From cf3b7538666cd1efa314ce4944e4efdf3dd81d99 Mon Sep 17 00:00:00 2001 From: Jarno Rajahalme Date: Wed, 2 Apr 2014 15:44:21 -0700 Subject: [PATCH] ofpbuf: Abstract 'l2' pointer and document usage conventions. Rename 'l2' to 'frame' and add new ofpbuf_set_frame() and ofpbuf_l2(). ofpbuf_set_frame() alse resets all the layer offsets. ofpbuf_l2() returns NULL if the packet has no Ethernet header, as indicated either by unset l3 offset or NULL frame pointer. Callers of ofpbuf_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 Signed-off-by: Ben Pfaff --- 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 | 72 +++++++++++++++++++++--------------------- lib/ofp-parse.c | 3 -- lib/ofp-util.c | 39 +++++++++++------------ lib/ofpbuf.c | 23 +++++++------- lib/ofpbuf.h | 63 ++++++++++++++++++++++++++++-------- lib/packets.c | 24 ++++++++------ lib/rconn.c | 13 ++++---- lib/stp.c | 3 +- ofproto/ofproto-dpif.c | 14 ++++---- utilities/ovs-ofctl.c | 2 +- 16 files changed, 164 insertions(+), 127 deletions(-) diff --git a/lib/bundle.c b/lib/bundle.c index 4a40a6a57..60a360e1d 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 bdc79bd6c..ce0c471e3 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_l2(p); ccm = ofpbuf_at(p, (uint8_t *)ofpbuf_l3(p) - (uint8_t *)ofpbuf_data(p), CCM_ACCEPT_LEN); diff --git a/lib/flow.c b/lib/flow.c index 60b667ca5..e1ea75e51 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 = ofpbuf_data(&b); - ofpbuf_set_l2_5(packet, NULL); - ofpbuf_set_l3(packet, NULL); - ofpbuf_set_l4(packet, NULL); + ofpbuf_set_frame(packet, ofpbuf_data(packet)); if (ofpbuf_size(&b) < 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_l2(b); eth->eth_type = htons(ofpbuf_size(b)); return; } diff --git a/lib/learn.c b/lib/learn.c index 27b7105bd..8727a5539 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 cf6743513..ac0dac0dd 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_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 a644e6eff..5a3bf700c 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 fc91aaf1d..b67e47a7c 100644 --- a/lib/ofp-msgs.c +++ b/lib/ofp-msgs.c @@ -371,17 +371,17 @@ ofpraw_decode_assert(const struct ofp_header *oh) } /* Determines the OFPRAW_* type of the OpenFlow message in 'msg', which starts - * at 'ofpbuf_data(msg)' and has length 'ofpbuf_size(msg)' bytes. On success, returns 0 and - * stores the type into '*rawp'. On failure, returns an OFPERR_* error code - * and zeros '*rawp'. + * at 'ofpbuf_data(msg)' and has length 'ofpbuf_size(msg)' bytes. On success, + * returns 0 and stores the type into '*rawp'. On failure, returns an OFPERR_* + * error code and zeros '*rawp'. * * 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 '*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 + * 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 * ofpbuf_data(msg)). */ enum ofperr ofpraw_pull(enum ofpraw *rawp, struct ofpbuf *msg) @@ -399,8 +399,8 @@ ofpraw_pull(enum ofpraw *rawp, struct ofpbuf *msg) enum ofpraw raw; /* Set default outputs. */ - msg->l2 = ofpbuf_data(msg); - ofpbuf_set_l3(msg, ofpbuf_data(msg)); + msg->frame = ofpbuf_data(msg); + ofpbuf_set_l3(msg, msg->frame); *rawp = 0; len = ofpbuf_size(msg); @@ -416,7 +416,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, ofpbuf_data(msg)); min_len = instance->hdrs_len + info->min_body; @@ -511,10 +511,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 +558,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 +591,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 +631,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 +664,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(ofpbuf_size(buf)); 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 +682,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 +703,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 { @@ -790,17 +790,17 @@ ofptype_decode(enum ofptype *typep, const struct ofp_header *oh) } /* Determines the OFPTYPE_* type of the OpenFlow message in 'msg', which starts - * at 'ofpbuf_data(msg)' and has length 'ofpbuf_size(msg)' bytes. On success, returns 0 and - * stores the type into '*typep'. On failure, returns an OFPERR_* error code - * and zeros '*typep'. + * at 'ofpbuf_data(msg)' and has length 'ofpbuf_size(msg)' bytes. On success, + * returns 0 and stores the type into '*typep'. On failure, returns an + * OFPERR_* error code and zeros '*typep'. * * 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 + * 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 * ofpbuf_data(msg)). */ enum ofperr ofptype_pull(enum ofptype *typep, struct ofpbuf *buf) @@ -893,7 +893,7 @@ ofpmp_reserve(struct list *replies, size_t len) next = ofpbuf_new(MAX(1024, hdrs_len + len)); ofpbuf_put(next, ofpbuf_data(msg), hdrs_len); - next->l2 = ofpbuf_data(next); + next->frame = ofpbuf_data(next); 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 e100974c0..92fb40f95 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 9639ea490..b3d86e908 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 (!ofpbuf_size(msg)) { 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 = ofpbuf_data(msg); + 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 = ofpbuf_data(msg); + 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 = ofpbuf_data(msg); + 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 = ofpbuf_data(msg); 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 = ofpbuf_size(msg); - 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 6a44a30ac..1f4b61dc0 100644 --- a/lib/ofpbuf.c +++ b/lib/ofpbuf.c @@ -27,7 +27,7 @@ ofpbuf_init__(struct ofpbuf *b, size_t allocated, enum ofpbuf_source source) { b->allocated = allocated; b->source = source; - b->l2 = NULL; + b->frame = NULL; b->l2_5_ofs = b->l3_ofs = b->l4_ofs = UINT16_MAX; list_poison(&b->list_node); } @@ -186,10 +186,11 @@ ofpbuf_clone_with_headroom(const struct ofpbuf *buffer, size_t headroom) new_buffer = ofpbuf_clone_data_with_headroom(ofpbuf_data(buffer), ofpbuf_size(buffer), headroom); - if (buffer->l2) { - uintptr_t data_delta = (char *)ofpbuf_data(new_buffer) - (char *)ofpbuf_data(buffer); + if (buffer->frame) { + uintptr_t data_delta + = (char *)ofpbuf_data(new_buffer) - (char *)ofpbuf_data(buffer); - 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; @@ -274,12 +275,12 @@ ofpbuf_resize__(struct ofpbuf *b, size_t new_headroom, size_t new_tailroom) new_data = (char *) new_base + new_headroom; if (ofpbuf_data(b) != new_data) { - uintptr_t data_delta = (char *) new_data - (char *) ofpbuf_data(b); + if (b->frame) { + uintptr_t data_delta = (char *) new_data - (char *) ofpbuf_data(b); - ofpbuf_set_data(b, new_data); - if (b->l2) { - b->l2 = (char *) b->l2 + data_delta; + b->frame = (char *) b->frame + data_delta; } + ofpbuf_set_data(b, new_data); } } @@ -536,12 +537,12 @@ ofpbuf_resize_l2_5(struct ofpbuf *b, int increment) ofpbuf_pull(b, -increment); } - b->l2 = ofpbuf_data(b); + b->frame = ofpbuf_data(b); /* 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 @@ -552,5 +553,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 80c65475f..3312e9bc1 100644 --- a/lib/ofpbuf.h +++ b/lib/ofpbuf.h @@ -37,7 +37,25 @@ 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 { #ifdef DPDK_NETDEV struct rte_mbuf mbuf; /* DPDK mbuf */ @@ -48,13 +66,13 @@ struct ofpbuf { #endif uint32_t allocated; /* Number of bytes allocated. */ - 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. */ }; @@ -69,6 +87,8 @@ static inline void ofpbuf_set_size(struct ofpbuf *, uint32_t); void * ofpbuf_resize_l2(struct ofpbuf *, int increment); void * ofpbuf_resize_l2_5(struct ofpbuf *, int increment); +static inline void * ofpbuf_l2(const struct ofpbuf *); +static inline void ofpbuf_set_frame(struct ofpbuf *, void *); static inline void * ofpbuf_l2_5(const struct ofpbuf *); static inline void ofpbuf_set_l2_5(struct ofpbuf *, void *); static inline void * ofpbuf_l3(const struct ofpbuf *); @@ -247,34 +267,51 @@ static inline bool ofpbuf_equal(const struct ofpbuf *a, const struct ofpbuf *b) memcmp(ofpbuf_data(a), ofpbuf_data(b), ofpbuf_size(a)) == 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_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_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_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_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_l4_size(const struct ofpbuf *b) diff --git a/lib/packets.c b/lib/packets.c index 2c202bbff..89ee324f7 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_l2(packet); - if (ofpbuf_size(packet) >= sizeof *veh + if (veh && ofpbuf_size(packet) >= 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_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; @@ -850,7 +856,7 @@ packet_set_sctp_port(struct ofpbuf *packet, ovs_be16 src, ovs_be16 dst) { struct sctp_header *sh = ofpbuf_l4(packet); ovs_be32 old_csum, old_correct_csum, new_csum; - uint16_t tp_len = ofpbuf_size(packet) - ((uint8_t*)sh - (uint8_t*)ofpbuf_data(packet)); + uint16_t tp_len = ofpbuf_l4_size(packet); old_csum = sh->sctp_csum; sh->sctp_csum = 0; diff --git a/lib/rconn.c b/lib/rconn.c index b3f467940..cb3cdd5d4 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, ofpbuf_size(b)); } - /* Use 'l2' as a private pointer while 'b' is in txq. */ - ovs_assert(b->l2 == ofpbuf_data(b)); - 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 = ofpbuf_size(msg); - 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 = ofpbuf_data(msg); /* 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, ofpbuf_size(b)); } diff --git a/lib/stp.c b/lib/stp.c index b5688241d..dbe48e823 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 ee97707c4..9ec0237db 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_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 == ofpbuf_data(learning_packet)); - learning_packet->l2 = port_void; + /* Temporarily use 'frame' as a private pointer (see below). */ + ovs_assert(learning_packet->frame == ofpbuf_data(learning_packet)); + 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 = ofpbuf_data(learning_packet); + /* Restore 'frame'. */ + learning_packet->frame = ofpbuf_data(learning_packet); 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 a8704bf93..8be862686 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -2396,7 +2396,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) { -- 2.43.0