[ovs-dev] [PATCH] ofpbuf: Abstract 'l2' pointer and document usage conventions.
Jarno Rajahalme
jrajahalme at nicira.com
Wed Apr 2 17:59:29 UTC 2014
Rename 'l2' to 'frame' and add new ofpbuf_set_frame() and
ofpbuf_get_l2(). ofpbuf_set_frame() alse resets all the layer
offsets. ofpbuf_get_l2() returns NULL if the packet has no Ethernet
header, as indicated either by unset l3 offset or NULL frame pointer.
Caller's of ofpbuf_get_l2() are supposed to check the return value,
unless they can otherwise be sure that the packet has a valid Ethernet
header.
The recent commit 437d0d22 made some assumptions that were not valid
regarding the use of the 'l2' pointer in rconn module and by
compose_rarp(). This is now fixed as follows: rconn now relies on the
fact that once OpenFlow messages are given to rconn for transport, the
frame pointer is no longer needed to refer to the OpenFlow header; and
compose_rarp() now sets the frame pointer and offsets as expected.
In addition to storing network frames, ofpbufs are also used for
handling OpenFlow messages and action lists. lib/ofpbuf.h now has a
comment documenting the current usage conventions and invariants.
Signed-off-by: Jarno Rajahalme <jrajahalme at nicira.com>
---
lib/bundle.c | 4 +--
lib/cfm.c | 2 +-
lib/flow.c | 7 ++----
lib/learn.c | 4 +--
lib/odp-execute.c | 8 +++---
lib/ofp-actions.c | 10 ++++----
lib/ofp-msgs.c | 65 ++++++++++++++++++++++++------------------------
lib/ofp-parse.c | 3 ---
lib/ofp-util.c | 39 ++++++++++++++---------------
lib/ofpbuf.c | 20 +++++++--------
lib/ofpbuf.h | 63 ++++++++++++++++++++++++++++++++++++----------
lib/packets.c | 22 ++++++++++------
lib/rconn.c | 13 +++++-----
lib/stp.c | 3 ++-
ofproto/ofproto-dpif.c | 14 +++++------
utilities/ovs-ofctl.c | 2 +-
16 files changed, 157 insertions(+), 122 deletions(-)
diff --git a/lib/bundle.c b/lib/bundle.c
index 4a40a6a..60a360e 100644
--- a/lib/bundle.c
+++ b/lib/bundle.c
@@ -177,7 +177,7 @@ bundle_from_openflow(const struct nx_action_bundle *nab,
ofpbuf_put(ofpacts, &ofp_port, sizeof ofp_port);
}
- bundle = ofpacts->l2;
+ bundle = ofpacts->frame;
ofpact_update_len(ofpacts, &bundle->ofpact);
if (!error) {
@@ -288,7 +288,7 @@ bundle_parse__(const char *s, char **save_ptr,
}
ofpbuf_put(ofpacts, &slave_port, sizeof slave_port);
- bundle = ofpacts->l2;
+ bundle = ofpacts->frame;
bundle->n_slaves++;
}
ofpact_update_len(ofpacts, &bundle->ofpact);
diff --git a/lib/cfm.c b/lib/cfm.c
index dcdaa0e..2ef3d10 100644
--- a/lib/cfm.c
+++ b/lib/cfm.c
@@ -718,7 +718,7 @@ cfm_process_heartbeat(struct cfm *cfm, const struct ofpbuf *p)
ovs_mutex_lock(&mutex);
- eth = p->l2;
+ eth = ofpbuf_get_l2(p);
ccm = ofpbuf_at(p, (uint8_t *)ofpbuf_get_l3(p) - (uint8_t *)p->data,
CCM_ACCEPT_LEN);
diff --git a/lib/flow.c b/lib/flow.c
index 314c1c7..7f00a36 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -371,10 +371,7 @@ flow_extract(struct ofpbuf *packet, const struct pkt_metadata *md,
flow->pkt_mark = md->pkt_mark;
}
- packet->l2 = b.data;
- ofpbuf_set_l2_5(packet, NULL);
- ofpbuf_set_l3(packet, NULL);
- ofpbuf_set_l4(packet, NULL);
+ ofpbuf_set_frame(packet, packet->data);
if (b.size < sizeof *eth) {
return;
@@ -1330,7 +1327,7 @@ flow_compose(struct ofpbuf *b, const struct flow *flow)
/* eth_compose() sets l3 pointer and makes sure it is 32-bit aligned. */
eth_compose(b, flow->dl_dst, flow->dl_src, ntohs(flow->dl_type), 0);
if (flow->dl_type == htons(FLOW_DL_TYPE_NONE)) {
- struct eth_header *eth = b->l2;
+ struct eth_header *eth = ofpbuf_get_l2(b);
eth->eth_type = htons(b->size);
return;
}
diff --git a/lib/learn.c b/lib/learn.c
index 61799c9..c82b618 100644
--- a/lib/learn.c
+++ b/lib/learn.c
@@ -127,7 +127,7 @@ learn_from_openflow(const struct nx_action_learn *nal, struct ofpbuf *ofpacts)
}
spec = ofpbuf_put_zeros(ofpacts, sizeof *spec);
- learn = ofpacts->l2;
+ learn = ofpacts->frame;
learn->n_specs++;
spec->src_type = header & NX_LEARN_SRC_MASK;
@@ -585,7 +585,7 @@ learn_parse__(char *orig, char *arg, struct ofpbuf *ofpacts)
char *error;
spec = ofpbuf_put_zeros(ofpacts, sizeof *spec);
- learn = ofpacts->l2;
+ learn = ofpacts->frame;
learn->n_specs++;
error = learn_parse_spec(orig, name, value, spec);
diff --git a/lib/odp-execute.c b/lib/odp-execute.c
index e5aa0ce..8dd5701 100644
--- a/lib/odp-execute.c
+++ b/lib/odp-execute.c
@@ -33,10 +33,12 @@ static void
odp_eth_set_addrs(struct ofpbuf *packet,
const struct ovs_key_ethernet *eth_key)
{
- struct eth_header *eh = packet->l2;
+ struct eth_header *eh = ofpbuf_get_l2(packet);
- memcpy(eh->eth_src, eth_key->eth_src, sizeof eh->eth_src);
- memcpy(eh->eth_dst, eth_key->eth_dst, sizeof eh->eth_dst);
+ if (eh) {
+ memcpy(eh->eth_src, eth_key->eth_src, sizeof eh->eth_src);
+ memcpy(eh->eth_dst, eth_key->eth_dst, sizeof eh->eth_dst);
+ }
}
static void
diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
index 23d89d3..a20e1eb 100644
--- a/lib/ofp-actions.c
+++ b/lib/ofp-actions.c
@@ -225,7 +225,7 @@ dec_ttl_from_openflow(struct ofpbuf *out, enum ofputil_action_code compat)
ids->ofpact.compat = compat;
ids->n_controllers = 1;
ofpbuf_put(out, &id, sizeof id);
- ids = out->l2;
+ ids = out->frame;
ofpact_update_len(out, &ids->ofpact);
return error;
}
@@ -258,7 +258,7 @@ dec_ttl_cnt_ids_from_openflow(const struct nx_action_cnt_ids *nac_ids,
for (i = 0; i < ids->n_controllers; i++) {
uint16_t id = ntohs(((ovs_be16 *)(nac_ids + 1))[i]);
ofpbuf_put(out, &id, sizeof id);
- ids = out->l2;
+ ids = out->frame;
}
ofpact_update_len(out, &ids->ofpact);
@@ -1077,7 +1077,7 @@ static void
set_field_to_openflow(const struct ofpact_set_field *sf,
struct ofpbuf *openflow)
{
- struct ofp_header *oh = (struct ofp_header *)openflow->l2;
+ struct ofp_header *oh = (struct ofp_header *)openflow->frame;
if (oh->version >= OFP12_VERSION) {
set_field_to_openflow12(sf, openflow);
@@ -3608,7 +3608,7 @@ ofpact_put(struct ofpbuf *ofpacts, enum ofpact_type type, size_t len)
struct ofpact *ofpact;
ofpact_pad(ofpacts);
- ofpact = ofpacts->l2 = ofpbuf_put_uninit(ofpacts, len);
+ ofpact = ofpacts->frame = ofpbuf_put_uninit(ofpacts, len);
ofpact_init(ofpact, type, len);
return ofpact;
}
@@ -3631,7 +3631,7 @@ ofpact_init(struct ofpact *ofpact, enum ofpact_type type, size_t len)
void
ofpact_update_len(struct ofpbuf *ofpacts, struct ofpact *ofpact)
{
- ovs_assert(ofpact == ofpacts->l2);
+ ovs_assert(ofpact == ofpacts->frame);
ofpact->len = (char *) ofpbuf_tail(ofpacts) - (char *) ofpact;
}
diff --git a/lib/ofp-msgs.c b/lib/ofp-msgs.c
index 677d359..7db5719 100644
--- a/lib/ofp-msgs.c
+++ b/lib/ofp-msgs.c
@@ -380,9 +380,8 @@ ofpraw_decode_assert(const struct ofp_header *oh)
*
* In addition to setting '*rawp', this function pulls off the OpenFlow header
* (including the stats headers, vendor header, and any subtype header) with
- * ofpbuf_pull(). It also sets 'msg->l2' to the start of the OpenFlow header
- * and 'msg->l3' just beyond the headers (that is, to the final value of
- * msg->data). */
+ * ofpbuf_pull(). It also sets 'msg->frame' to the start of the OpenFlow
+ * header and 'l3' to 'msg->data', just beyond the headers. */
enum ofperr
ofpraw_pull(enum ofpraw *rawp, struct ofpbuf *msg)
{
@@ -399,7 +398,7 @@ ofpraw_pull(enum ofpraw *rawp, struct ofpbuf *msg)
enum ofpraw raw;
/* Set default outputs. */
- msg->l2 = msg->data;
+ msg->frame = msg->data;
ofpbuf_set_l3(msg, msg->data);
*rawp = 0;
@@ -416,7 +415,7 @@ ofpraw_pull(enum ofpraw *rawp, struct ofpbuf *msg)
info = raw_info_get(raw);
instance = raw_instance_get(info, hdrs.version);
- msg->l2 = ofpbuf_pull(msg, instance->hdrs_len);
+ msg->frame = ofpbuf_pull(msg, instance->hdrs_len);
ofpbuf_set_l3(msg, msg->data);
min_len = instance->hdrs_len + info->min_body;
@@ -511,10 +510,10 @@ static void ofpraw_put__(enum ofpraw, uint8_t version, ovs_be32 xid,
* Each 'raw' value is valid only for certain OpenFlow versions. The caller
* must specify a valid (raw, version) pair.
*
- * In the returned ofpbuf, 'l2' points to the beginning of the OpenFlow header
- * and 'l3' points just after it, to where the message's body will start. The
- * caller must actually allocate the body into the space reserved for it,
- * e.g. with ofpbuf_put_uninit().
+ * In the returned ofpbuf, 'frame' points to the beginning of the
+ * OpenFlow header and 'l3' points just after it, to where the
+ * message's body will start. The caller must actually allocate the
+ * body into the space reserved for it, e.g. with ofpbuf_put_uninit().
*
* The caller owns the returned ofpbuf and must free it when it is no longer
* needed, e.g. with ofpbuf_delete(). */
@@ -558,10 +557,10 @@ ofpraw_alloc_reply(enum ofpraw raw, const struct ofp_header *request,
* value. Every stats request has a corresponding reply, so the (raw, version)
* pairing pitfalls of the other ofpraw_alloc_*() functions don't apply here.
*
- * In the returned ofpbuf, 'l2' points to the beginning of the OpenFlow header
- * and 'l3' points just after it, to where the message's body will start. The
- * caller must actually allocate the body into the space reserved for it,
- * e.g. with ofpbuf_put_uninit().
+ * In the returned ofpbuf, 'frame' points to the beginning of the
+ * OpenFlow header and 'l3' points just after it, to where the
+ * message's body will start. The caller must actually allocate the
+ * body into the space reserved for it, e.g. with ofpbuf_put_uninit().
*
* The caller owns the returned ofpbuf and must free it when it is no longer
* needed, e.g. with ofpbuf_delete(). */
@@ -591,7 +590,7 @@ ofpraw_alloc_stats_reply(const struct ofp_header *request,
* Each 'raw' value is valid only for certain OpenFlow versions. The caller
* must specify a valid (raw, version) pair.
*
- * Upon return, 'buf->l2' points to the beginning of the OpenFlow header and
+ * Upon return, 'buf->frame' points to the beginning of the OpenFlow header and
* 'buf->l3' points just after it, to where the message's body will start. The
* caller must actually allocating the body into the space reserved for it,
* e.g. with ofpbuf_put_uninit(). */
@@ -631,10 +630,10 @@ ofpraw_put_reply(enum ofpraw raw, const struct ofp_header *request,
* value. Every stats request has a corresponding reply, so the (raw, version)
* pairing pitfalls of the other ofpraw_alloc_*() functions don't apply here.
*
- * In the returned ofpbuf, 'l2' points to the beginning of the OpenFlow header
- * and 'l3' points just after it, to where the message's body will start. The
- * caller must actually allocate the body into the space reserved for it,
- * e.g. with ofpbuf_put_uninit().
+ * In the returned ofpbuf, 'frame' points to the beginning of the
+ * OpenFlow header and 'l3' points just after it, to where the
+ * message's body will start. The caller must actually allocate the
+ * body into the space reserved for it, e.g. with ofpbuf_put_uninit().
*
* The caller owns the returned ofpbuf and must free it when it is no longer
* needed, e.g. with ofpbuf_delete(). */
@@ -664,17 +663,17 @@ ofpraw_put__(enum ofpraw raw, uint8_t version, ovs_be32 xid,
ofpbuf_prealloc_tailroom(buf, (instance->hdrs_len + info->min_body
+ extra_tailroom));
- buf->l2 = ofpbuf_put_uninit(buf, instance->hdrs_len);
+ buf->frame = ofpbuf_put_uninit(buf, instance->hdrs_len);
ofpbuf_set_l3(buf, ofpbuf_tail(buf));
- oh = buf->l2;
+ oh = buf->frame;
oh->version = version;
oh->type = hdrs->type;
oh->length = htons(buf->size);
oh->xid = xid;
if (hdrs->type == OFPT_VENDOR) {
- struct nicira_header *nh = buf->l2;
+ struct nicira_header *nh = buf->frame;
ovs_assert(hdrs->vendor == NX_VENDOR_ID);
nh->vendor = htonl(hdrs->vendor);
@@ -682,17 +681,17 @@ ofpraw_put__(enum ofpraw raw, uint8_t version, ovs_be32 xid,
} else if (version == OFP10_VERSION
&& (hdrs->type == OFPT10_STATS_REQUEST ||
hdrs->type == OFPT10_STATS_REPLY)) {
- struct ofp10_stats_msg *osm = buf->l2;
+ struct ofp10_stats_msg *osm = buf->frame;
osm->type = htons(hdrs->stat);
osm->flags = htons(0);
if (hdrs->stat == OFPST_VENDOR) {
- struct ofp10_vendor_stats_msg *ovsm = buf->l2;
+ struct ofp10_vendor_stats_msg *ovsm = buf->frame;
ovsm->vendor = htonl(hdrs->vendor);
if (hdrs->vendor == NX_VENDOR_ID) {
- struct nicira10_stats_msg *nsm = buf->l2;
+ struct nicira10_stats_msg *nsm = buf->frame;
nsm->subtype = htonl(hdrs->subtype);
memset(nsm->pad, 0, sizeof nsm->pad);
@@ -703,18 +702,18 @@ ofpraw_put__(enum ofpraw raw, uint8_t version, ovs_be32 xid,
} else if (version != OFP10_VERSION
&& (hdrs->type == OFPT11_STATS_REQUEST ||
hdrs->type == OFPT11_STATS_REPLY)) {
- struct ofp11_stats_msg *osm = buf->l2;
+ struct ofp11_stats_msg *osm = buf->frame;
osm->type = htons(hdrs->stat);
osm->flags = htons(0);
memset(osm->pad, 0, sizeof osm->pad);
if (hdrs->stat == OFPST_VENDOR) {
- struct ofp11_vendor_stats_msg *ovsm = buf->l2;
+ struct ofp11_vendor_stats_msg *ovsm = buf->frame;
ovsm->vendor = htonl(hdrs->vendor);
if (hdrs->vendor == NX_VENDOR_ID) {
- struct nicira11_stats_msg *nsm = buf->l2;
+ struct nicira11_stats_msg *nsm = buf->frame;
nsm->subtype = htonl(hdrs->subtype);
} else {
@@ -797,11 +796,11 @@ ofptype_decode(enum ofptype *typep, const struct ofp_header *oh)
* This function checks that the message has a valid length for its particular
* type of message, and returns an error if not.
*
- * In addition to setting '*typep', this function pulls off the OpenFlow header
- * (including the stats headers, vendor header, and any subtype header) with
- * ofpbuf_pull(). It also sets 'msg->l2' to the start of the OpenFlow header
- * and 'msg->l3' just beyond the headers (that is, to the final value of
- * msg->data). */
+ * In addition to setting '*typep', this function pulls off the
+ * OpenFlow header (including the stats headers, vendor header, and
+ * any subtype header) with ofpbuf_pull(). It also sets 'msg->frame'
+ * to the start of the OpenFlow header and 'msg->l3' just beyond the
+ * headers (that is, to the final value of msg->data). */
enum ofperr
ofptype_pull(enum ofptype *typep, struct ofpbuf *buf)
{
@@ -893,7 +892,7 @@ ofpmp_reserve(struct list *replies, size_t len)
next = ofpbuf_new(MAX(1024, hdrs_len + len));
ofpbuf_put(next, msg->data, hdrs_len);
- next->l2 = next->data;
+ next->frame = next->data;
ofpbuf_set_l3(next, ofpbuf_tail(next));
list_push_back(replies, &next->list_node);
diff --git a/lib/ofp-parse.c b/lib/ofp-parse.c
index 5c5bb06..dfb308d 100644
--- a/lib/ofp-parse.c
+++ b/lib/ofp-parse.c
@@ -291,7 +291,6 @@ parse_note(const char *arg, struct ofpbuf *ofpacts)
}
ofpbuf_put(ofpacts, &byte, 1);
- note = ofpacts->l2;
note->length++;
arg += 2;
@@ -400,7 +399,6 @@ parse_noargs_dec_ttl(struct ofpbuf *b)
ids = ofpact_put_DEC_TTL(b);
ofpbuf_put(b, &id, sizeof id);
- ids = b->l2;
ids->n_controllers++;
ofpact_update_len(b, &ids->ofpact);
}
@@ -426,7 +424,6 @@ parse_dec_ttl(struct ofpbuf *b, char *arg)
uint16_t id = atoi(cntr);
ofpbuf_put(b, &id, sizeof id);
- ids = b->l2;
ids->n_controllers++;
}
if (!ids->n_controllers) {
diff --git a/lib/ofp-util.c b/lib/ofp-util.c
index d0002d5..14edbb3 100644
--- a/lib/ofp-util.c
+++ b/lib/ofp-util.c
@@ -1918,7 +1918,7 @@ ofputil_decode_meter_config(struct ofpbuf *msg,
enum ofperr err;
/* Pull OpenFlow headers for the first call. */
- if (!msg->l2) {
+ if (!msg->frame) {
ofpraw_pull_assert(msg);
}
@@ -1994,7 +1994,7 @@ ofputil_decode_meter_stats(struct ofpbuf *msg,
enum ofperr err;
/* Pull OpenFlow headers for the first call. */
- if (!msg->l2) {
+ if (!msg->frame) {
ofpraw_pull_assert(msg);
}
@@ -2475,7 +2475,7 @@ ofputil_pull_queue_get_config_reply(struct ofpbuf *reply,
queue->min_rate = UINT16_MAX;
queue->max_rate = UINT16_MAX;
- oh = reply->l2;
+ oh = reply->frame;
if (oh->version < OFP12_VERSION) {
const struct ofp10_packet_queue *opq10;
@@ -2680,13 +2680,13 @@ ofputil_decode_flow_stats_reply(struct ofputil_flow_stats *fs,
enum ofperr error;
enum ofpraw raw;
- error = (msg->l2
- ? ofpraw_decode(&raw, msg->l2)
+ error = (msg->frame
+ ? ofpraw_decode(&raw, msg->frame)
: ofpraw_pull(&raw, msg));
if (error) {
return error;
}
- oh = msg->l2;
+ oh = msg->frame;
if (!msg->size) {
return EOF;
@@ -4400,8 +4400,7 @@ ofputil_decode_table_features(struct ofpbuf *msg,
struct ofp13_table_features *otf;
unsigned int len;
- if (!msg->l2) {
- msg->l2 = msg->data;
+ if (!msg->frame) {
ofpraw_pull_assert(msg);
}
@@ -4942,8 +4941,7 @@ ofputil_decode_flow_monitor_request(struct ofputil_flow_monitor_request *rq,
struct nx_flow_monitor_request *nfmr;
uint16_t flags;
- if (!msg->l2) {
- msg->l2 = msg->data;
+ if (!msg->frame) {
ofpraw_pull_assert(msg);
}
@@ -5027,8 +5025,7 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update,
unsigned int length;
struct ofp_header *oh;
- if (!msg->l2) {
- msg->l2 = msg->data;
+ if (!msg->frame) {
ofpraw_pull_assert(msg);
}
@@ -5040,7 +5037,7 @@ ofputil_decode_flow_update(struct ofputil_flow_update *update,
goto bad_len;
}
- oh = msg->l2;
+ oh = msg->frame;
nfuh = msg->data;
update->event = ntohs(nfuh->event);
@@ -5155,7 +5152,7 @@ ofputil_append_flow_update(const struct ofputil_flow_update *update,
msg = ofpbuf_from_list(list_back(replies));
start_ofs = msg->size;
- version = ((struct ofp_header *)msg->l2)->version;
+ version = ((struct ofp_header *)msg->frame)->version;
if (update->event == NXFME_ABBREV) {
struct nx_flow_update_abbrev *nfua;
@@ -6126,8 +6123,8 @@ ofputil_decode_port_stats(struct ofputil_port_stats *ps, struct ofpbuf *msg)
enum ofperr error;
enum ofpraw raw;
- error = (msg->l2
- ? ofpraw_decode(&raw, msg->l2)
+ error = (msg->frame
+ ? ofpraw_decode(&raw, msg->frame)
: ofpraw_pull(&raw, msg));
if (error) {
return error;
@@ -6451,8 +6448,8 @@ ofputil_decode_group_stats_reply(struct ofpbuf *msg,
size_t i;
gs->bucket_stats = NULL;
- error = (msg->l2
- ? ofpraw_decode(&raw, msg->l2)
+ error = (msg->frame
+ ? ofpraw_decode(&raw, msg->frame)
: ofpraw_pull(&raw, msg));
if (error) {
return error;
@@ -6631,7 +6628,7 @@ ofputil_decode_group_desc_reply(struct ofputil_group_desc *gd,
struct ofp11_group_desc_stats *ogds;
size_t length;
- if (!msg->l2) {
+ if (!msg->frame) {
ofpraw_pull_assert(msg);
}
@@ -6934,8 +6931,8 @@ ofputil_decode_queue_stats(struct ofputil_queue_stats *qs, struct ofpbuf *msg)
enum ofperr error;
enum ofpraw raw;
- error = (msg->l2
- ? ofpraw_decode(&raw, msg->l2)
+ error = (msg->frame
+ ? ofpraw_decode(&raw, msg->frame)
: ofpraw_pull(&raw, msg));
if (error) {
return error;
diff --git a/lib/ofpbuf.c b/lib/ofpbuf.c
index 56e1ec6..140d7c6 100644
--- a/lib/ofpbuf.c
+++ b/lib/ofpbuf.c
@@ -30,7 +30,7 @@ ofpbuf_use__(struct ofpbuf *b, void *base, size_t allocated,
b->allocated = allocated;
b->source = source;
b->size = 0;
- b->l2 = NULL;
+ b->frame = NULL;
b->l2_5_ofs = b->l3_ofs = b->l4_ofs = UINT16_MAX;
list_poison(&b->list_node);
}
@@ -167,10 +167,10 @@ ofpbuf_clone_with_headroom(const struct ofpbuf *buffer, size_t headroom)
new_buffer = ofpbuf_clone_data_with_headroom(buffer->data, buffer->size,
headroom);
- if (buffer->l2) {
+ if (buffer->frame) {
uintptr_t data_delta = (char *)new_buffer->data - (char *)buffer->data;
- new_buffer->l2 = (char *) buffer->l2 + data_delta;
+ new_buffer->frame = (char *) buffer->frame + data_delta;
}
new_buffer->l2_5_ofs = buffer->l2_5_ofs;
new_buffer->l3_ofs = buffer->l3_ofs;
@@ -255,12 +255,12 @@ ofpbuf_resize__(struct ofpbuf *b, size_t new_headroom, size_t new_tailroom)
new_data = (char *) new_base + new_headroom;
if (b->data != new_data) {
- uintptr_t data_delta = (char *) new_data - (char *) b->data;
+ if (b->frame) {
+ uintptr_t data_delta = (char *) new_data - (char *) b->data;
- b->data = new_data;
- if (b->l2) {
- b->l2 = (char *) b->l2 + data_delta;
+ b->frame = (char *) b->frame + data_delta;
}
+ b->data = new_data;
}
}
@@ -516,12 +516,12 @@ ofpbuf_resize_l2_5(struct ofpbuf *b, int increment)
ofpbuf_pull(b, -increment);
}
- b->l2 = b->data;
+ b->frame = b->data;
/* Adjust layer offsets after l2_5. */
ofpbuf_adjust_layer_offset(&b->l3_ofs, increment);
ofpbuf_adjust_layer_offset(&b->l4_ofs, increment);
- return b->l2;
+ return b->frame;
}
/* Adjust the size of the l2 portion of the ofpbuf, updating the l2
@@ -532,5 +532,5 @@ ofpbuf_resize_l2(struct ofpbuf *b, int increment)
{
ofpbuf_resize_l2_5(b, increment);
ofpbuf_adjust_layer_offset(&b->l2_5_ofs, increment);
- return b->l2;
+ return b->frame;
}
diff --git a/lib/ofpbuf.h b/lib/ofpbuf.h
index 8d1cb11..fa8a3f0 100644
--- a/lib/ofpbuf.h
+++ b/lib/ofpbuf.h
@@ -36,26 +36,46 @@ enum OVS_PACKED_ENUM ofpbuf_source {
};
/* Buffer for holding arbitrary data. An ofpbuf is automatically reallocated
- * as necessary if it grows too large for the available memory. */
+ * as necessary if it grows too large for the available memory.
+ *
+ * 'frame' and offset conventions:
+ *
+ * Network frames (aka "packets"): 'frame' MUST be set to the start of the
+ * packet, layer offsets MAY be set as appropriate for the packet.
+ * Additionally, we assume in many places that the 'frame' and 'data' are
+ * the same for packets.
+ *
+ * OpenFlow messages: 'frame' points to the start of the OpenFlow
+ * header, while 'l3_ofs' is the length of the OpenFlow header.
+ * When parsing, the 'data' will move past these, as data is being
+ * pulled from the OpenFlow message.
+ *
+ * Actions: When encoding OVS action lists, the 'frame' is used
+ * as a pointer to the beginning of the current action (see ofpact_put()).
+ *
+ * rconn: Reuses 'frame' as a private pointer while queuing.
+ */
struct ofpbuf {
void *base; /* First byte of allocated space. */
uint32_t allocated; /* Number of bytes allocated. */
uint32_t size; /* Number of bytes in use. */
void *data; /* First byte actually in use. */
- void *l2; /* Link-level header. */
- uint16_t l2_5_ofs; /* MPLS label stack offset from l2, or
+ void *frame; /* Packet frame start, or NULL. */
+ uint16_t l2_5_ofs; /* MPLS label stack offset from 'packet', or
* UINT16_MAX */
- uint16_t l3_ofs; /* Network-level header offset from l2, or
- * UINT16_MAX. */
- uint16_t l4_ofs; /* Transport-level header offset from l2, or
- UINT16_MAX. */
+ uint16_t l3_ofs; /* Network-level header offset from 'packet',
+ or UINT16_MAX. */
+ uint16_t l4_ofs; /* Transport-level header offset from 'packet',
+ or UINT16_MAX. */
enum ofpbuf_source source; /* Source of memory allocated as 'base'. */
struct list list_node; /* Private list element for use by owner. */
};
void * ofpbuf_resize_l2(struct ofpbuf *, int increment);
void * ofpbuf_resize_l2_5(struct ofpbuf *, int increment);
+static inline void * ofpbuf_get_l2(const struct ofpbuf *);
+static inline void ofpbuf_set_frame(struct ofpbuf *, void *);
static inline void * ofpbuf_get_l2_5(const struct ofpbuf *);
static inline void ofpbuf_set_l2_5(struct ofpbuf *, void *);
static inline void * ofpbuf_get_l3(const struct ofpbuf *);
@@ -226,34 +246,51 @@ static inline bool ofpbuf_equal(const struct ofpbuf *a, const struct ofpbuf *b)
return a->size == b->size && memcmp(a->data, b->data, a->size) == 0;
}
+/* Get the start if the Ethernet frame. 'l3_ofs' marks the end of the l2
+ * headers, so return NULL if it is not set. */
+static inline void * ofpbuf_get_l2(const struct ofpbuf *b)
+{
+ return (b->l3_ofs != UINT16_MAX) ? b->frame : NULL;
+}
+
+/* Sets the packet frame start pointer and resets all layer offsets.
+ * l3 offset must be set before 'l2' can be retrieved. */
+static inline void ofpbuf_set_frame(struct ofpbuf *b, void *packet)
+{
+ b->frame = packet;
+ b->l2_5_ofs = UINT16_MAX;
+ b->l3_ofs = UINT16_MAX;
+ b->l4_ofs = UINT16_MAX;
+}
+
static inline void * ofpbuf_get_l2_5(const struct ofpbuf *b)
{
- return b->l2_5_ofs != UINT16_MAX ? (char *)b->l2 + b->l2_5_ofs : NULL;
+ return b->l2_5_ofs != UINT16_MAX ? (char *)b->frame + b->l2_5_ofs : NULL;
}
static inline void ofpbuf_set_l2_5(struct ofpbuf *b, void *l2_5)
{
- b->l2_5_ofs = l2_5 ? (char *)l2_5 - (char *)b->l2 : UINT16_MAX;
+ b->l2_5_ofs = l2_5 ? (char *)l2_5 - (char *)b->frame : UINT16_MAX;
}
static inline void * ofpbuf_get_l3(const struct ofpbuf *b)
{
- return b->l3_ofs != UINT16_MAX ? (char *)b->l2 + b->l3_ofs : NULL;
+ return b->l3_ofs != UINT16_MAX ? (char *)b->frame + b->l3_ofs : NULL;
}
static inline void ofpbuf_set_l3(struct ofpbuf *b, void *l3)
{
- b->l3_ofs = l3 ? (char *)l3 - (char *)b->l2 : UINT16_MAX;
+ b->l3_ofs = l3 ? (char *)l3 - (char *)b->frame : UINT16_MAX;
}
static inline void * ofpbuf_get_l4(const struct ofpbuf *b)
{
- return b->l4_ofs != UINT16_MAX ? (char *)b->l2 + b->l4_ofs : NULL;
+ return b->l4_ofs != UINT16_MAX ? (char *)b->frame + b->l4_ofs : NULL;
}
static inline void ofpbuf_set_l4(struct ofpbuf *b, void *l4)
{
- b->l4_ofs = l4 ? (char *)l4 - (char *)b->l2 : UINT16_MAX;
+ b->l4_ofs = l4 ? (char *)l4 - (char *)b->frame : UINT16_MAX;
}
static inline size_t ofpbuf_get_l4_size(const struct ofpbuf *b)
diff --git a/lib/packets.c b/lib/packets.c
index 3366089..10954ac 100644
--- a/lib/packets.c
+++ b/lib/packets.c
@@ -168,13 +168,15 @@ compose_rarp(struct ofpbuf *b, const uint8_t eth_src[ETH_ADDR_LEN])
put_16aligned_be32(&arp->ar_spa, htonl(0));
memcpy(arp->ar_tha, eth_src, ETH_ADDR_LEN);
put_16aligned_be32(&arp->ar_tpa, htonl(0));
+
+ ofpbuf_set_frame(b, eth);
+ ofpbuf_set_l3(b, arp);
}
/* Insert VLAN header according to given TCI. Packet passed must be Ethernet
* packet. Ignores the CFI bit of 'tci' using 0 instead.
*
- * Also sets 'packet->l2' to point to the new Ethernet header and adjusts
- * the layer offsets accordingly. */
+ * Also adjusts the layer offsets accordingly. */
void
eth_push_vlan(struct ofpbuf *packet, ovs_be16 tpid, ovs_be16 tci)
{
@@ -194,9 +196,9 @@ eth_push_vlan(struct ofpbuf *packet, ovs_be16 tpid, ovs_be16 tci)
void
eth_pop_vlan(struct ofpbuf *packet)
{
- struct vlan_eth_header *veh = packet->l2;
+ struct vlan_eth_header *veh = ofpbuf_get_l2(packet);
- if (packet->size >= sizeof *veh
+ if (veh && packet->size >= sizeof *veh
&& veh->veth_type == htons(ETH_TYPE_VLAN)) {
memmove((char *)veh + VLAN_HEADER_LEN, veh, 2 * ETH_ADDR_LEN);
@@ -208,7 +210,11 @@ eth_pop_vlan(struct ofpbuf *packet)
static void
set_ethertype(struct ofpbuf *packet, ovs_be16 eth_type)
{
- struct eth_header *eh = packet->l2;
+ struct eth_header *eh = ofpbuf_get_l2(packet);
+
+ if (!eh) {
+ return;
+ }
if (eh->eth_type == htons(ETH_TYPE_VLAN)) {
ovs_be16 *p;
@@ -545,8 +551,8 @@ ipv6_is_cidr(const struct in6_addr *netmask)
/* Populates 'b' with an Ethernet II packet headed with the given 'eth_dst',
* 'eth_src' and 'eth_type' parameters. A payload of 'size' bytes is allocated
* in 'b' and returned. This payload may be populated with appropriate
- * information by the caller. Sets 'b''s 'l2' pointer and 'l3' offset to the
- * Ethernet header and payload respectively. Aligns b->l3 on a 32-bit
+ * information by the caller. Sets 'b''s 'frame' pointer and 'l3' offset to
+ * the Ethernet header and payload respectively. Aligns b->l3 on a 32-bit
* boundary.
*
* The returned packet has enough headroom to insert an 802.1Q VLAN header if
@@ -572,7 +578,7 @@ eth_compose(struct ofpbuf *b, const uint8_t eth_dst[ETH_ADDR_LEN],
memcpy(eth->eth_src, eth_src, ETH_ADDR_LEN);
eth->eth_type = htons(eth_type);
- b->l2 = eth;
+ ofpbuf_set_frame(b, eth);
ofpbuf_set_l3(b, data);
return data;
diff --git a/lib/rconn.c b/lib/rconn.c
index a5368d5..b1b1626 100644
--- a/lib/rconn.c
+++ b/lib/rconn.c
@@ -721,9 +721,8 @@ rconn_send__(struct rconn *rc, struct ofpbuf *b,
rconn_packet_counter_inc(counter, b->size);
}
- /* Use 'l2' as a private pointer while 'b' is in txq. */
- ovs_assert(b->l2 == b->data);
- b->l2 = counter;
+ /* Reuse 'frame' as a private pointer while 'b' is in txq. */
+ ofpbuf_set_frame(b, counter);
list_push_back(&rc->txq, &b->list_node);
@@ -1108,18 +1107,18 @@ try_send(struct rconn *rc)
{
struct ofpbuf *msg = ofpbuf_from_list(rc->txq.next);
unsigned int n_bytes = msg->size;
- struct rconn_packet_counter *counter = msg->l2;
+ struct rconn_packet_counter *counter = msg->frame;
int retval;
/* Eagerly remove 'msg' from the txq. We can't remove it from the list
* after sending, if sending is successful, because it is then owned by the
* vconn, which might have freed it already. */
list_remove(&msg->list_node);
- msg->l2 = msg->data; /* Restore 'l2'. */
+ ofpbuf_set_frame(msg, NULL);
retval = vconn_send(rc->vconn, msg);
if (retval) {
- msg->l2 = counter; /* 'l2' is a private pointer while msg is in txq. */
+ ofpbuf_set_frame(msg, counter);
list_push_front(&rc->txq, &msg->list_node);
if (retval != EAGAIN) {
report_error(rc, retval);
@@ -1212,7 +1211,7 @@ flush_queue(struct rconn *rc)
}
while (!list_is_empty(&rc->txq)) {
struct ofpbuf *b = ofpbuf_from_list(list_pop_front(&rc->txq));
- struct rconn_packet_counter *counter = b->l2;
+ struct rconn_packet_counter *counter = b->frame;
if (counter) {
rconn_packet_counter_dec(counter, b->size);
}
diff --git a/lib/stp.c b/lib/stp.c
index 78dacb3..64cf698 100644
--- a/lib/stp.c
+++ b/lib/stp.c
@@ -1535,8 +1535,9 @@ stp_send_bpdu(struct stp_port *p, const void *bpdu, size_t bpdu_size)
/* Skeleton. */
pkt = ofpbuf_new(ETH_HEADER_LEN + LLC_HEADER_LEN + bpdu_size);
- pkt->l2 = eth = ofpbuf_put_zeros(pkt, sizeof *eth);
+ eth = ofpbuf_put_zeros(pkt, sizeof *eth);
llc = ofpbuf_put_zeros(pkt, sizeof *llc);
+ ofpbuf_set_frame(pkt, eth);
ofpbuf_set_l3(pkt, ofpbuf_put(pkt, bpdu, bpdu_size));
/* 802.2 header. */
diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
index 7172cb2..c22de8f 100644
--- a/ofproto/ofproto-dpif.c
+++ b/ofproto/ofproto-dpif.c
@@ -1768,7 +1768,7 @@ send_bpdu_cb(struct ofpbuf *pkt, int port_num, void *ofproto_)
VLOG_WARN_RL(&rl, "%s: cannot send BPDU on unknown port %d",
ofproto->up.name, port_num);
} else {
- struct eth_header *eth = pkt->l2;
+ struct eth_header *eth = ofpbuf_get_l2(pkt);
netdev_get_etheraddr(ofport->up.netdev, eth->eth_src);
if (eth_addr_is_zero(eth->eth_src)) {
@@ -2419,9 +2419,9 @@ bundle_send_learning_packets(struct ofbundle *bundle)
learning_packet = bond_compose_learning_packet(bundle->bond,
e->mac, e->vlan,
&port_void);
- /* Temporarily use l2 as a private pointer (see below). */
- ovs_assert(learning_packet->l2 == learning_packet->data);
- learning_packet->l2 = port_void;
+ /* Temporarily use 'frame' as a private pointer (see below). */
+ ovs_assert(learning_packet->frame == learning_packet->data);
+ learning_packet->frame = port_void;
list_push_back(&packets, &learning_packet->list_node);
}
}
@@ -2430,10 +2430,10 @@ bundle_send_learning_packets(struct ofbundle *bundle)
error = n_packets = n_errors = 0;
LIST_FOR_EACH (learning_packet, list_node, &packets) {
int ret;
- void *port_void = learning_packet->l2;
+ void *port_void = learning_packet->frame;
- /* Restore l2. */
- learning_packet->l2 = learning_packet->data;
+ /* Restore 'frame'. */
+ learning_packet->frame = learning_packet->data;
ret = ofproto_dpif_send_packet(port_void, learning_packet);
if (ret) {
error = ret;
diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c
index 1294247..98e7286 100644
--- a/utilities/ovs-ofctl.c
+++ b/utilities/ovs-ofctl.c
@@ -2395,7 +2395,7 @@ recv_flow_stats_reply(struct vconn *vconn, ovs_be32 send_xid,
return true;
case EOF:
- more = ofpmp_more(reply->l2);
+ more = ofpmp_more(reply->frame);
ofpbuf_delete(reply);
reply = NULL;
if (!more) {
--
1.7.10.4
More information about the dev
mailing list