| Message ID | 1513895115-35879-4-git-send-email-jpettit@ovn.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,no-slow,1/6] ofproto-dpif: Add ability to look up an ofproto by UUID. | expand |
On 12/21/2017 2:25 PM, Justin Pettit wrote: > Previously, the ofproto instance and OpenFlow port have been derived > based on the datapath port number. This change explicitly declares them > both, which will be helpful in future commits that no longer can depend > on having a unique datapath port (e.g., a source port that represents > the controller). > > Signed-off-by: Justin Pettit <jpettit@ovn.org> > --- > lib/odp-util.c | 27 ++++++++++------- > lib/odp-util.h | 24 ++++++++++----- > ofproto/ofproto-dpif-upcall.c | 70 +++++++++++++++++++++++++++---------------- > ofproto/ofproto-dpif-xlate.c | 19 ++++++++---- > ofproto/ofproto-dpif.c | 5 +--- > ofproto/ofproto-dpif.h | 2 ++ > 6 files changed, 95 insertions(+), 52 deletions(-) > > diff --git a/lib/odp-util.c b/lib/odp-util.c > index a3211118c17c..173a868e0728 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -431,7 +431,7 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr, > bool userdata_unspec = true; > union user_action_cookie cookie; > > - if (userdata_len >= sizeof cookie.type > + if (userdata_len >= sizeof cookie.header.type > && userdata_len <= sizeof cookie) { > > memset(&cookie, 0, sizeof cookie); > @@ -440,20 +440,20 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr, > userdata_unspec = false; > > if (userdata_len == sizeof cookie.sflow > - && cookie.type == USER_ACTION_COOKIE_SFLOW) { > + && cookie.header.type == USER_ACTION_COOKIE_SFLOW) { > ds_put_format(ds, ",sFlow(" > "vid=%"PRIu16",pcp=%d,output=%"PRIu32")", > vlan_tci_to_vid(cookie.sflow.vlan_tci), > vlan_tci_to_pcp(cookie.sflow.vlan_tci), > cookie.sflow.output); > } else if (userdata_len == sizeof cookie.slow_path > - && cookie.type == USER_ACTION_COOKIE_SLOW_PATH) { > + && cookie.header.type == USER_ACTION_COOKIE_SLOW_PATH) { > ds_put_cstr(ds, ",slow_path("); > format_flags(ds, slow_path_reason_to_string, > cookie.slow_path.reason, ','); > ds_put_format(ds, ")"); > } else if (userdata_len == sizeof cookie.flow_sample > - && cookie.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { > + && cookie.header.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { > ds_put_format(ds, ",flow_sample(probability=%"PRIu16 > ",collector_set_id=%"PRIu32 > ",obs_domain_id=%"PRIu32 > @@ -472,7 +472,7 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr, > } > ds_put_char(ds, ')'); > } else if (userdata_len >= sizeof cookie.ipfix > - && cookie.type == USER_ACTION_COOKIE_IPFIX) { > + && cookie.header.type == USER_ACTION_COOKIE_IPFIX) { > ds_put_format(ds, ",ipfix(output_port="); > odp_portno_name_format(portno_names, > cookie.ipfix.output_odp_port, ds); > @@ -1132,7 +1132,9 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) > tci |= VLAN_CFI; > } > > - cookie.type = USER_ACTION_COOKIE_SFLOW; > + cookie.header.type = USER_ACTION_COOKIE_SFLOW; > + cookie.header.ofp_in_port = OFPP_NONE; > + cookie.header.ofproto_uuid = UUID_ZERO; > cookie.sflow.vlan_tci = htons(tci); > cookie.sflow.output = output; > user_data = &cookie; > @@ -1140,8 +1142,9 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) > } else if (ovs_scan(&s[n], ",slow_path(%n", > &n1)) { > n += n1; > - cookie.type = USER_ACTION_COOKIE_SLOW_PATH; > - cookie.slow_path.unused = 0; > + cookie.header.type = USER_ACTION_COOKIE_SLOW_PATH; > + cookie.header.ofp_in_port = OFPP_NONE; > + cookie.header.ofproto_uuid = UUID_ZERO; > cookie.slow_path.reason = 0; > > res = parse_odp_flags(&s[n], slow_path_reason_to_string, > @@ -1164,7 +1167,9 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) > &output, &n1)) { > n += n1; > > - cookie.type = USER_ACTION_COOKIE_FLOW_SAMPLE; > + cookie.header.type = USER_ACTION_COOKIE_FLOW_SAMPLE; > + cookie.header.ofp_in_port = OFPP_NONE; > + cookie.header.ofproto_uuid = UUID_ZERO; > cookie.flow_sample.probability = probability; > cookie.flow_sample.collector_set_id = collector_set_id; > cookie.flow_sample.obs_domain_id = obs_domain_id; > @@ -1190,7 +1195,9 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) > } else if (ovs_scan(&s[n], ",ipfix(output_port=%"SCNi32")%n", > &output, &n1) ) { > n += n1; > - cookie.type = USER_ACTION_COOKIE_IPFIX; > + cookie.header.type = USER_ACTION_COOKIE_IPFIX; > + cookie.header.ofp_in_port = OFPP_NONE; > + cookie.header.ofproto_uuid = UUID_ZERO; > cookie.ipfix.output_odp_port = u32_to_odp(output); > user_data = &cookie; > user_data_size = sizeof cookie.ipfix; > diff --git a/lib/odp-util.h b/lib/odp-util.h > index 72a3c30d6d37..040334695835 100644 > --- a/lib/odp-util.h > +++ b/lib/odp-util.h > @@ -25,6 +25,7 @@ > #include "hash.h" > #include "openvswitch/hmap.h" > #include "openvswitch/ofp-actions.h" > +#include "openvswitch/uuid.h" > #include "odp-netlink.h" > #include "openflow/openflow.h" > #include "util.h" > @@ -295,24 +296,32 @@ enum user_action_cookie_type { > USER_ACTION_COOKIE_IPFIX, /* Packet for per-bridge IPFIX sampling. */ > }; > > +struct user_action_cookie_header { > + enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ > + ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ > + struct uuid ofproto_uuid; /* UUID of ofproto-dpif. */ > +}; > + > /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. */ > union user_action_cookie { > - uint16_t type; /* enum user_action_cookie_type. */ > + struct user_action_cookie_header header; > > struct { > - uint16_t type; /* USER_ACTION_COOKIE_SFLOW. */ > + /* USER_ACTION_COOKIE_SFLOW. */ > + struct user_action_cookie_header header; > ovs_be16 vlan_tci; /* Destination VLAN TCI. */ > uint32_t output; /* SFL_FLOW_SAMPLE_TYPE 'output' value. */ > } sflow; > > struct { > - uint16_t type; /* USER_ACTION_COOKIE_SLOW_PATH. */ > - uint16_t unused; > + /* USER_ACTION_COOKIE_SLOW_PATH. */ > + struct user_action_cookie_header header; > uint32_t reason; /* enum slow_path_reason. */ > } slow_path; > > struct { > - uint16_t type; /* USER_ACTION_COOKIE_FLOW_SAMPLE. */ > + /* USER_ACTION_COOKIE_FLOW_SAMPLE. */ > + struct user_action_cookie_header header; > uint16_t probability; /* Sampling probability. */ > uint32_t collector_set_id; /* ID of IPFIX collector set. */ > uint32_t obs_domain_id; /* Observation Domain ID. */ > @@ -322,11 +331,12 @@ union user_action_cookie { > } flow_sample; > > struct { > - uint16_t type; /* USER_ACTION_COOKIE_IPFIX. */ > + /* USER_ACTION_COOKIE_IPFIX. */ > + struct user_action_cookie_header header; > odp_port_t output_odp_port; /* The output odp port. */ > } ipfix; > }; > -BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 24); > +BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 48); > > size_t odp_put_userspace_action(uint32_t pid, > const void *userdata, size_t userdata_size, > diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c > index 46b75fe35a2b..88501803b5b8 100644 > --- a/ofproto/ofproto-dpif-upcall.c > +++ b/ofproto/ofproto-dpif-upcall.c > @@ -38,6 +38,7 @@ > #include "packets.h" > #include "openvswitch/poll-loop.h" > #include "seq.h" > +#include "tunnel.h" > #include "unixctl.h" > #include "openvswitch/vlog.h" > > @@ -207,7 +208,7 @@ struct upcall { > const ovs_u128 *ufid; /* Unique identifier for 'flow'. */ > unsigned pmd_id; /* Datapath poll mode driver id. */ > const struct dp_packet *packet; /* Packet associated with this upcall. */ > - ofp_port_t in_port; /* OpenFlow in port, or OFPP_NONE. */ > + ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ > uint16_t mru; /* If !0, Maximum receive unit of > fragmented IP packet */ > > @@ -998,7 +999,7 @@ classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata, > return BAD_UPCALL; > } > userdata_len = nl_attr_get_size(userdata); > - if (userdata_len < sizeof cookie->type > + if (userdata_len < sizeof cookie->header.type > || userdata_len > sizeof *cookie) { > VLOG_WARN_RL(&rl, "action upcall cookie has unexpected size %"PRIuSIZE, > userdata_len); > @@ -1007,21 +1008,21 @@ classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata, > > memcpy(cookie, nl_attr_get(userdata), userdata_len); > > - if (userdata_len == MAX(8, sizeof cookie->sflow) > - && cookie->type == USER_ACTION_COOKIE_SFLOW) { > + if (userdata_len == sizeof cookie->sflow > + && cookie->header.type == USER_ACTION_COOKIE_SFLOW) { > return SFLOW_UPCALL; > - } else if (userdata_len == MAX(8, sizeof cookie->slow_path) > - && cookie->type == USER_ACTION_COOKIE_SLOW_PATH) { > + } else if (userdata_len == sizeof cookie->slow_path > + && cookie->header.type == USER_ACTION_COOKIE_SLOW_PATH) { > return SLOW_PATH_UPCALL; > - } else if (userdata_len == MAX(8, sizeof cookie->flow_sample) > - && cookie->type == USER_ACTION_COOKIE_FLOW_SAMPLE) { > + } else if (userdata_len == sizeof cookie->flow_sample > + && cookie->header.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { > return FLOW_SAMPLE_UPCALL; > - } else if (userdata_len == MAX(8, sizeof cookie->ipfix) > - && cookie->type == USER_ACTION_COOKIE_IPFIX) { > + } else if (userdata_len == sizeof cookie->ipfix > + && cookie->header.type == USER_ACTION_COOKIE_IPFIX) { > return IPFIX_UPCALL; > } else { > VLOG_WARN_RL(&rl, "invalid user cookie of type %"PRIu16 > - " and size %"PRIuSIZE, cookie->type, userdata_len); > + " and size %"PRIuSIZE, cookie->header.type, userdata_len); > return BAD_UPCALL; > } > } > @@ -1030,16 +1031,18 @@ classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata, > * initialized with at least 128 bytes of space. */ > static void > compose_slow_path(struct udpif *udpif, struct xlate_out *xout, > - const struct flow *flow, odp_port_t odp_in_port, > + const struct flow *flow, > + odp_port_t odp_in_port, ofp_port_t ofp_in_port, > struct ofpbuf *buf, uint32_t slowpath_meter_id, > - uint32_t controller_meter_id) > + uint32_t controller_meter_id, struct uuid *ofproto_uuid) > { > union user_action_cookie cookie; > odp_port_t port; > uint32_t pid; > > - cookie.type = USER_ACTION_COOKIE_SLOW_PATH; > - cookie.slow_path.unused = 0; > + cookie.header.type = USER_ACTION_COOKIE_SLOW_PATH; > + cookie.header.ofp_in_port = ofp_in_port; > + cookie.header.ofproto_uuid = *ofproto_uuid; > cookie.slow_path.reason = xout->slow; > > port = xout->slow & (SLOW_CFM | SLOW_BFD | SLOW_LACP | SLOW_STP) > @@ -1086,12 +1089,23 @@ upcall_receive(struct upcall *upcall, const struct dpif_backer *backer, > upcall->type = classify_upcall(type, userdata, &upcall->cookie); > if (upcall->type == BAD_UPCALL) { > return EAGAIN; > - } > - > - error = xlate_lookup(backer, flow, &upcall->ofproto, &upcall->ipfix, > - &upcall->sflow, NULL, &upcall->in_port); > - if (error) { > - return error; > + } else if (upcall->type == MISS_UPCALL) { > + error = xlate_lookup(backer, flow, &upcall->ofproto, &upcall->ipfix, > + &upcall->sflow, NULL, &upcall->ofp_in_port); > + if (error) { > + return error; > + } > + } else { > + struct ofproto_dpif *ofproto > + = ofproto_dpif_lookup_by_uuid(&upcall->cookie.header.ofproto_uuid); > + if (!ofproto) { > + VLOG_INFO_RL(&rl, "upcall could not find ofproto"); > + return ENODEV; > + } > + upcall->ofproto = ofproto; > + upcall->ipfix = ofproto->ipfix; > + upcall->sflow = ofproto->sflow; > + upcall->ofp_in_port = upcall->cookie.header.ofp_in_port; > } > > upcall->recirc = NULL; > @@ -1132,7 +1146,7 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall, > > xlate_in_init(&xin, upcall->ofproto, > ofproto_dpif_get_tables_version(upcall->ofproto), > - upcall->flow, upcall->in_port, NULL, > + upcall->flow, upcall->ofp_in_port, NULL, > stats.tcp_flags, upcall->packet, wc, odp_actions); > > if (upcall->type == MISS_UPCALL) { > @@ -1177,8 +1191,9 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall, > uint32_t cmid = upcall->ofproto->up.controller_meter_id; > /* upcall->put_actions already initialized by upcall_receive(). */ > compose_slow_path(udpif, &upcall->xout, upcall->flow, > - upcall->flow->in_port.odp_port, > - &upcall->put_actions, smid, cmid); > + upcall->flow->in_port.odp_port, upcall->ofp_in_port, > + &upcall->put_actions, smid, cmid, > + &upcall->ofproto->uuid); > } > > /* This function is also called for slow-pathed flows. As we are only > @@ -2045,13 +2060,16 @@ revalidate_ukey__(struct udpif *udpif, const struct udpif_key *ukey, > > if (xoutp->slow) { > struct ofproto_dpif *ofproto; > - ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, NULL); > + ofp_port_t ofp_in_port; > + > + ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, > + &ofp_in_port); > uint32_t smid = ofproto ? ofproto->up.slowpath_meter_id : UINT32_MAX; > uint32_t cmid = ofproto ? ofproto->up.controller_meter_id : UINT32_MAX; > > ofpbuf_clear(odp_actions); > compose_slow_path(udpif, xoutp, &ctx.flow, ctx.flow.in_port.odp_port, > - odp_actions, smid, cmid); > + ofp_in_port, odp_actions, smid, cmid, &ofproto->uuid); > } > > if (odp_flow_key_to_mask(ukey->mask, ukey->mask_len, &dp_mask, &ctx.flow) > diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c > index 3bdd64d4d552..ec92c6a730bf 100644 > --- a/ofproto/ofproto-dpif-xlate.c > +++ b/ofproto/ofproto-dpif-xlate.c > @@ -2927,7 +2927,13 @@ compose_sflow_action(struct xlate_ctx *ctx) > return 0; > } > > - union user_action_cookie cookie = { .type = USER_ACTION_COOKIE_SFLOW }; > + union user_action_cookie cookie = { > + .sflow = { > + .header.type = USER_ACTION_COOKIE_SFLOW, > + .header.ofp_in_port = ctx->xin->flow.in_port.ofp_port, > + .header.ofproto_uuid = ctx->xbridge->ofproto->uuid > + } > + }; I don't know how old of a compiler we're supposed to support but this breaks the 4.8.4 compiler that comes with the Ubuntu 14.04 distro when warnings are treated as errors. ../ofproto/ofproto-dpif-xlate.c: In function 'compose_sflow_action': ../ofproto/ofproto-dpif-xlate.c:2933:13: error: missing initializer for field 'ofp_in_port' of 'struct user_action_cookie_header' [-Werror=missing-field-initializers] .header.ofp_in_port = ctx->xin->flow.in_port.ofp_port, ^ In file included from ../ofproto/ofproto-dpif-xlate.h:21:0, from ../ofproto/ofproto-dpif-xlate.c:17: ../lib/odp-util.h:306:16: note: 'ofp_in_port' declared here ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ ^ ../ofproto/ofproto-dpif-xlate.c:2934:13: error: missing initializer for field 'ofproto_uuid' of 'struct user_action_cookie_header' [-Werror=missing-field-initializers] .header.ofproto_uuid = ctx->xbridge->ofproto->uuid ^ In file included from ../ofproto/ofproto-dpif-xlate.h:21:0, from ../ofproto/ofproto-dpif-xlate.c:17: ../lib/odp-util.h:307:17: note: 'ofproto_uuid' declared here struct uuid ofproto_uuid; /* UUID of ofproto-dpif. */ ^ ../ofproto/ofproto-dpif-xlate.c: In function 'compose_ipfix_action': ../ofproto/ofproto-dpif-xlate.c:2980:13: error: missing initializer for field 'ofp_in_port' of 'struct user_action_cookie_header' [-Werror=missing-field-initializers] .header.ofp_in_port = ctx->xin->flow.in_port.ofp_port, ^ In file included from ../ofproto/ofproto-dpif-xlate.h:21:0, from ../ofproto/ofproto-dpif-xlate.c:17: ../lib/odp-util.h:306:16: note: 'ofp_in_port' declared here ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ ^ ../ofproto/ofproto-dpif-xlate.c:2981:13: error: missing initializer for field 'ofproto_uuid' of 'struct user_action_cookie_header' [-Werror=missing-field-initializers] .header.ofproto_uuid = ctx->xbridge->ofproto->uuid, ^ In file included from ../ofproto/ofproto-dpif-xlate.h:21:0, from ../ofproto/ofproto-dpif-xlate.c:17: ../lib/odp-util.h:307:17: note: 'ofproto_uuid' declared here struct uuid ofproto_uuid; /* UUID of ofproto-dpif. */ ^ ../ofproto/ofproto-dpif-xlate.c: In function 'xlate_sample_action': ../ofproto/ofproto-dpif-xlate.c:5116:13: error: missing initializer for field 'ofp_in_port' of 'struct user_action_cookie_header' [-Werror=missing-field-initializers] .header.ofp_in_port = ctx->xin->flow.in_port.ofp_port, ^ In file included from ../ofproto/ofproto-dpif-xlate.h:21:0, from ../ofproto/ofproto-dpif-xlate.c:17: ../lib/odp-util.h:306:16: note: 'ofp_in_port' declared here ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ ^ ../ofproto/ofproto-dpif-xlate.c:5117:13: error: missing initializer for field 'ofproto_uuid' of 'struct user_action_cookie_header' [-Werror=missing-field-initializers] .header.ofproto_uuid = ctx->xbridge->ofproto->uuid, ^ In file included from ../ofproto/ofproto-dpif-xlate.h:21:0, from ../ofproto/ofproto-dpif-xlate.c:17: ../lib/odp-util.h:307:17: note: 'ofproto_uuid' declared here struct uuid ofproto_uuid; /* UUID of ofproto-dpif. */ ^ cc1: all warnings being treated as errors make[2]: *** [ofproto/ofproto_libofproto_la-ofproto-dpif-xlate.lo] Error 1 make[2]: Leaving directory `/home/gvrose/prj/ovs-experimental/_build' make[1]: *** [all-recursive] Error 1 make[1]: Leaving directory `/home/gvrose/prj/ovs-experimental/_build' make: *** [all] Error 2 Works fine on gcc 7. Thanks, - Greg > return compose_sample_action(ctx, dpif_sflow_get_probability(sflow), > &cookie, sizeof cookie.sflow, ODPP_NONE, > true); > @@ -2970,7 +2976,9 @@ compose_ipfix_action(struct xlate_ctx *ctx, odp_port_t output_odp_port) > > union user_action_cookie cookie = { > .ipfix = { > - .type = USER_ACTION_COOKIE_IPFIX, > + .header.type = USER_ACTION_COOKIE_IPFIX, > + .header.ofp_in_port = ctx->xin->flow.in_port.ofp_port, > + .header.ofproto_uuid = ctx->xbridge->ofproto->uuid, > .output_odp_port = output_odp_port, > } > }; > @@ -2992,9 +3000,8 @@ fix_sflow_action(struct xlate_ctx *ctx, unsigned int user_cookie_offset) > > cookie = ofpbuf_at(ctx->odp_actions, user_cookie_offset, > sizeof cookie->sflow); > - ovs_assert(cookie->type == USER_ACTION_COOKIE_SFLOW); > + ovs_assert(cookie->header.type == USER_ACTION_COOKIE_SFLOW); > > - cookie->type = USER_ACTION_COOKIE_SFLOW; > cookie->sflow.vlan_tci = base->vlans[0].tci; > > /* See http://www.sflow.org/sflow_version_5.txt (search for "Input/output > @@ -5265,7 +5272,9 @@ xlate_sample_action(struct xlate_ctx *ctx, > > union user_action_cookie cookie = { > .flow_sample = { > - .type = USER_ACTION_COOKIE_FLOW_SAMPLE, > + .header.type = USER_ACTION_COOKIE_FLOW_SAMPLE, > + .header.ofp_in_port = ctx->xin->flow.in_port.ofp_port, > + .header.ofproto_uuid = ctx->xbridge->ofproto->uuid, > .probability = os->probability, > .collector_set_id = os->collector_set_id, > .obs_domain_id = os->obs_domain_id, > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index 7d628e11328a..b0a1390d816b 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -161,9 +161,6 @@ struct ofport_dpif { > static odp_port_t ofp_port_to_odp_port(const struct ofproto_dpif *, > ofp_port_t); > > -static ofp_port_t odp_port_to_ofp_port(const struct ofproto_dpif *, > - odp_port_t); > - > static struct ofport_dpif * > ofport_dpif_cast(const struct ofport *ofport) > { > @@ -5622,7 +5619,7 @@ odp_port_to_ofport(const struct dpif_backer *backer, odp_port_t odp_port) > return NULL; > } > > -static ofp_port_t > +ofp_port_t > odp_port_to_ofp_port(const struct ofproto_dpif *ofproto, odp_port_t odp_port) > { > struct ofport_dpif *port; > diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h > index 0bb07638c15e..0f7086824f3d 100644 > --- a/ofproto/ofproto-dpif.h > +++ b/ofproto/ofproto-dpif.h > @@ -248,6 +248,8 @@ struct dpif_backer { > extern struct shash all_dpif_backers; > > struct ofport_dpif *odp_port_to_ofport(const struct dpif_backer *, odp_port_t); > + > +ofp_port_t odp_port_to_ofp_port(const struct ofproto_dpif *, odp_port_t); > > /* A bridge based on a "dpif" datapath. */ >
On Thu, Dec 21, 2017 at 02:25:13PM -0800, Justin Pettit wrote: > Previously, the ofproto instance and OpenFlow port have been derived > based on the datapath port number. This change explicitly declares them > both, which will be helpful in future commits that no longer can depend > on having a unique datapath port (e.g., a source port that represents > the controller). > > Signed-off-by: Justin Pettit <jpettit@ovn.org> Some checkpatch warnings look legit: WARNING: Line length is >79-characters long #34 FILE: lib/odp-util.c:457: && cookie.header.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { WARNING: Line length is >79-characters long #259 FILE: ofproto/ofproto-dpif-upcall.c:1101: = ofproto_dpif_lookup_by_uuid(&upcall->cookie.header.ofproto_uuid); WARNING: Line length is >79-characters long #307 FILE: ofproto/ofproto-dpif-upcall.c:2072: ofp_in_port, odp_actions, smid, cmid, &ofproto->uuid); There's a stray " in this line in odp-util.h: + enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ Maybe you like the way you did it, which is fine, but an alternative way to arrange user_action_cookie would be to make it a struct with the standard header followed by embedding further structs inside an anonymous union, like below. Then you could omit lots of ".header." stuff although you'd need to s/union/struct/ in other places. /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. */ struct user_action_cookie { enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ struct uuid ofproto_uuid; /* UUID of ofproto-dpif. */ union { struct { /* USER_ACTION_COOKIE_SFLOW. */ ovs_be16 vlan_tci; /* Destination VLAN TCI. */ uint32_t output; /* SFL_FLOW_SAMPLE_TYPE 'output' value. */ } sflow; struct { /* USER_ACTION_COOKIE_SLOW_PATH. */ uint32_t reason; /* enum slow_path_reason. */ } slow_path; struct { /* USER_ACTION_COOKIE_FLOW_SAMPLE. */ uint16_t probability; /* Sampling probability. */ uint32_t collector_set_id; /* ID of IPFIX collector set. */ uint32_t obs_domain_id; /* Observation Domain ID. */ uint32_t obs_point_id; /* Observation Point ID. */ odp_port_t output_odp_port; /* The output odp port. */ enum nx_action_sample_direction direction; } flow_sample; struct { /* USER_ACTION_COOKIE_IPFIX. */ odp_port_t output_odp_port; /* The output odp port. */ } ipfix; }; }; It looks like the shortest user_action_cookie variation is 24 bytes, the longest is 48. It may not be worth it to treat user_action_cookie as variable-length now. I think that it was done this way to better tolerate old datapaths that had a short, fixed maximum length for userdata. It would be simpler to just always ship sizeof(union user_action_cookie) bytes to the datapath and always require sizeof(union user_action_cookie) back. You could then skip a lot of userdata_len checks and updates for each type of cookie. Acked-by: Ben Pfaff <blp@ovn.org>
> On Jan 2, 2018, at 10:13 AM, Ben Pfaff <blp@ovn.org> wrote: > > On Thu, Dec 21, 2017 at 02:25:13PM -0800, Justin Pettit wrote: >> Previously, the ofproto instance and OpenFlow port have been derived >> based on the datapath port number. This change explicitly declares them >> both, which will be helpful in future commits that no longer can depend >> on having a unique datapath port (e.g., a source port that represents >> the controller). >> >> Signed-off-by: Justin Pettit <jpettit@ovn.org> > > Some checkpatch warnings look legit: > > WARNING: Line length is >79-characters long > #34 FILE: lib/odp-util.c:457: > && cookie.header.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { > > WARNING: Line length is >79-characters long > #259 FILE: ofproto/ofproto-dpif-upcall.c:1101: > = ofproto_dpif_lookup_by_uuid(&upcall->cookie.header.ofproto_uuid); > > WARNING: Line length is >79-characters long > #307 FILE: ofproto/ofproto-dpif-upcall.c:2072: > ofp_in_port, odp_actions, smid, cmid, &ofproto->uuid); > > There's a stray " in this line in odp-util.h: > + enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ I addressed those issues. > Maybe you like the way you did it, which is fine, but an alternative way > to arrange user_action_cookie would be to make it a struct with the > standard header followed by embedding further structs inside an > anonymous union, like below. Then you could omit lots of ".header." > stuff although you'd need to s/union/struct/ in other places. > > /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. */ > struct user_action_cookie { > enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ > ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ > struct uuid ofproto_uuid; /* UUID of ofproto-dpif. */ > > union { > struct { > /* USER_ACTION_COOKIE_SFLOW. */ > ovs_be16 vlan_tci; /* Destination VLAN TCI. */ > uint32_t output; /* SFL_FLOW_SAMPLE_TYPE 'output' value. */ > } sflow; > > struct { > /* USER_ACTION_COOKIE_SLOW_PATH. */ > uint32_t reason; /* enum slow_path_reason. */ > } slow_path; > > struct { > /* USER_ACTION_COOKIE_FLOW_SAMPLE. */ > uint16_t probability; /* Sampling probability. */ > uint32_t collector_set_id; /* ID of IPFIX collector set. */ > uint32_t obs_domain_id; /* Observation Domain ID. */ > uint32_t obs_point_id; /* Observation Point ID. */ > odp_port_t output_odp_port; /* The output odp port. */ > enum nx_action_sample_direction direction; > } flow_sample; > > struct { > /* USER_ACTION_COOKIE_IPFIX. */ > odp_port_t output_odp_port; /* The output odp port. */ > } ipfix; > }; > }; > > It looks like the shortest user_action_cookie variation is 24 bytes, the > longest is 48. It may not be worth it to treat user_action_cookie as > variable-length now. I think that it was done this way to better > tolerate old datapaths that had a short, fixed maximum length for > userdata. It would be simpler to just always ship sizeof(union > user_action_cookie) bytes to the datapath and always require > sizeof(union user_action_cookie) back. You could then skip a lot of > userdata_len checks and updates for each type of cookie. These are good suggestions. I want to send it as a separate patch because it contains more subtlety than I'd want to do as an incremental. > Acked-by: Ben Pfaff <blp@ovn.org> Thanks! --Justin
On Thu, Dec 21, 2017 at 02:25:13PM -0800, Justin Pettit wrote: > Previously, the ofproto instance and OpenFlow port have been derived > based on the datapath port number. This change explicitly declares them > both, which will be helpful in future commits that no longer can depend > on having a unique datapath port (e.g., a source port that represents > the controller). > > Signed-off-by: Justin Pettit <jpettit@ovn.org> When I applied more scrutiny to this patch, I noticed that the changes to ofproto-dpif.c and .h aren't needed. Thanks, Ben.
> On Jan 2, 2018, at 10:13 AM, Ben Pfaff <blp@ovn.org> wrote: > > On Thu, Dec 21, 2017 at 02:25:13PM -0800, Justin Pettit wrote: >> Previously, the ofproto instance and OpenFlow port have been derived >> based on the datapath port number. This change explicitly declares them >> both, which will be helpful in future commits that no longer can depend >> on having a unique datapath port (e.g., a source port that represents >> the controller). >> >> Signed-off-by: Justin Pettit <jpettit@ovn.org> > > Some checkpatch warnings look legit: > > WARNING: Line length is >79-characters long > #34 FILE: lib/odp-util.c:457: > && cookie.header.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { > > WARNING: Line length is >79-characters long > #259 FILE: ofproto/ofproto-dpif-upcall.c:1101: > = ofproto_dpif_lookup_by_uuid(&upcall->cookie.header.ofproto_uuid); > > WARNING: Line length is >79-characters long > #307 FILE: ofproto/ofproto-dpif-upcall.c:2072: > ofp_in_port, odp_actions, smid, cmid, &ofproto->uuid); Thanks. I fixed those and ran checkpatch on the v2 of the series. > There's a stray " in this line in odp-util.h: > + enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ Good catch. > Maybe you like the way you did it, which is fine, but an alternative way > to arrange user_action_cookie would be to make it a struct with the > standard header followed by embedding further structs inside an > anonymous union, like below. Then you could omit lots of ".header." > stuff although you'd need to s/union/struct/ in other places. > > /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. */ > struct user_action_cookie { > enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ > ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ > struct uuid ofproto_uuid; /* UUID of ofproto-dpif. */ > > union { > struct { > /* USER_ACTION_COOKIE_SFLOW. */ > ovs_be16 vlan_tci; /* Destination VLAN TCI. */ > uint32_t output; /* SFL_FLOW_SAMPLE_TYPE 'output' value. */ > } sflow; > > struct { > /* USER_ACTION_COOKIE_SLOW_PATH. */ > uint32_t reason; /* enum slow_path_reason. */ > } slow_path; > > struct { > /* USER_ACTION_COOKIE_FLOW_SAMPLE. */ > uint16_t probability; /* Sampling probability. */ > uint32_t collector_set_id; /* ID of IPFIX collector set. */ > uint32_t obs_domain_id; /* Observation Domain ID. */ > uint32_t obs_point_id; /* Observation Point ID. */ > odp_port_t output_odp_port; /* The output odp port. */ > enum nx_action_sample_direction direction; > } flow_sample; > > struct { > /* USER_ACTION_COOKIE_IPFIX. */ > odp_port_t output_odp_port; /* The output odp port. */ > } ipfix; > }; > }; Yes, I do think that is an improvement. > It looks like the shortest user_action_cookie variation is 24 bytes, the > longest is 48. It may not be worth it to treat user_action_cookie as > variable-length now. I think that it was done this way to better > tolerate old datapaths that had a short, fixed maximum length for > userdata. It would be simpler to just always ship sizeof(union > user_action_cookie) bytes to the datapath and always require > sizeof(union user_action_cookie) back. You could then skip a lot of > userdata_len checks and updates for each type of cookie. I reworked the series in v2 to make it fixed length. I agree it makes the code cleaner. Thanks for the suggestion! --Justin
> On Dec 28, 2017, at 4:13 PM, Gregory Rose <gvrose8192@gmail.com> wrote: > >> diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c >> index 3bdd64d4d552..ec92c6a730bf 100644 >> --- a/ofproto/ofproto-dpif-xlate.c >> +++ b/ofproto/ofproto-dpif-xlate.c >> @@ -2927,7 +2927,13 @@ compose_sflow_action(struct xlate_ctx *ctx) >> return 0; >> } >> - union user_action_cookie cookie = { .type = USER_ACTION_COOKIE_SFLOW }; >> + union user_action_cookie cookie = { >> + .sflow = { >> + .header.type = USER_ACTION_COOKIE_SFLOW, >> + .header.ofp_in_port = ctx->xin->flow.in_port.ofp_port, >> + .header.ofproto_uuid = ctx->xbridge->ofproto->uuid >> + } >> + }; > > I don't know how old of a compiler we're supposed to support but this breaks the 4.8.4 compiler > that comes with the Ubuntu 14.04 distro when warnings are treated as errors. Based on Ben's suggestions, I ended up removing these nested structs. I was able to build cleanly on a 4.8.5 compiler on Ubuntu 16.04. Let me know if you still see breakage in v2. Thanks, --Justin
diff --git a/lib/odp-util.c b/lib/odp-util.c index a3211118c17c..173a868e0728 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -431,7 +431,7 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr, bool userdata_unspec = true; union user_action_cookie cookie; - if (userdata_len >= sizeof cookie.type + if (userdata_len >= sizeof cookie.header.type && userdata_len <= sizeof cookie) { memset(&cookie, 0, sizeof cookie); @@ -440,20 +440,20 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr, userdata_unspec = false; if (userdata_len == sizeof cookie.sflow - && cookie.type == USER_ACTION_COOKIE_SFLOW) { + && cookie.header.type == USER_ACTION_COOKIE_SFLOW) { ds_put_format(ds, ",sFlow(" "vid=%"PRIu16",pcp=%d,output=%"PRIu32")", vlan_tci_to_vid(cookie.sflow.vlan_tci), vlan_tci_to_pcp(cookie.sflow.vlan_tci), cookie.sflow.output); } else if (userdata_len == sizeof cookie.slow_path - && cookie.type == USER_ACTION_COOKIE_SLOW_PATH) { + && cookie.header.type == USER_ACTION_COOKIE_SLOW_PATH) { ds_put_cstr(ds, ",slow_path("); format_flags(ds, slow_path_reason_to_string, cookie.slow_path.reason, ','); ds_put_format(ds, ")"); } else if (userdata_len == sizeof cookie.flow_sample - && cookie.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { + && cookie.header.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { ds_put_format(ds, ",flow_sample(probability=%"PRIu16 ",collector_set_id=%"PRIu32 ",obs_domain_id=%"PRIu32 @@ -472,7 +472,7 @@ format_odp_userspace_action(struct ds *ds, const struct nlattr *attr, } ds_put_char(ds, ')'); } else if (userdata_len >= sizeof cookie.ipfix - && cookie.type == USER_ACTION_COOKIE_IPFIX) { + && cookie.header.type == USER_ACTION_COOKIE_IPFIX) { ds_put_format(ds, ",ipfix(output_port="); odp_portno_name_format(portno_names, cookie.ipfix.output_odp_port, ds); @@ -1132,7 +1132,9 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) tci |= VLAN_CFI; } - cookie.type = USER_ACTION_COOKIE_SFLOW; + cookie.header.type = USER_ACTION_COOKIE_SFLOW; + cookie.header.ofp_in_port = OFPP_NONE; + cookie.header.ofproto_uuid = UUID_ZERO; cookie.sflow.vlan_tci = htons(tci); cookie.sflow.output = output; user_data = &cookie; @@ -1140,8 +1142,9 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) } else if (ovs_scan(&s[n], ",slow_path(%n", &n1)) { n += n1; - cookie.type = USER_ACTION_COOKIE_SLOW_PATH; - cookie.slow_path.unused = 0; + cookie.header.type = USER_ACTION_COOKIE_SLOW_PATH; + cookie.header.ofp_in_port = OFPP_NONE; + cookie.header.ofproto_uuid = UUID_ZERO; cookie.slow_path.reason = 0; res = parse_odp_flags(&s[n], slow_path_reason_to_string, @@ -1164,7 +1167,9 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) &output, &n1)) { n += n1; - cookie.type = USER_ACTION_COOKIE_FLOW_SAMPLE; + cookie.header.type = USER_ACTION_COOKIE_FLOW_SAMPLE; + cookie.header.ofp_in_port = OFPP_NONE; + cookie.header.ofproto_uuid = UUID_ZERO; cookie.flow_sample.probability = probability; cookie.flow_sample.collector_set_id = collector_set_id; cookie.flow_sample.obs_domain_id = obs_domain_id; @@ -1190,7 +1195,9 @@ parse_odp_userspace_action(const char *s, struct ofpbuf *actions) } else if (ovs_scan(&s[n], ",ipfix(output_port=%"SCNi32")%n", &output, &n1) ) { n += n1; - cookie.type = USER_ACTION_COOKIE_IPFIX; + cookie.header.type = USER_ACTION_COOKIE_IPFIX; + cookie.header.ofp_in_port = OFPP_NONE; + cookie.header.ofproto_uuid = UUID_ZERO; cookie.ipfix.output_odp_port = u32_to_odp(output); user_data = &cookie; user_data_size = sizeof cookie.ipfix; diff --git a/lib/odp-util.h b/lib/odp-util.h index 72a3c30d6d37..040334695835 100644 --- a/lib/odp-util.h +++ b/lib/odp-util.h @@ -25,6 +25,7 @@ #include "hash.h" #include "openvswitch/hmap.h" #include "openvswitch/ofp-actions.h" +#include "openvswitch/uuid.h" #include "odp-netlink.h" #include "openflow/openflow.h" #include "util.h" @@ -295,24 +296,32 @@ enum user_action_cookie_type { USER_ACTION_COOKIE_IPFIX, /* Packet for per-bridge IPFIX sampling. */ }; +struct user_action_cookie_header { + enum user_action_cookie_type type; /* One of USER_ACTION_COOKIE_*" */ + ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ + struct uuid ofproto_uuid; /* UUID of ofproto-dpif. */ +}; + /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. */ union user_action_cookie { - uint16_t type; /* enum user_action_cookie_type. */ + struct user_action_cookie_header header; struct { - uint16_t type; /* USER_ACTION_COOKIE_SFLOW. */ + /* USER_ACTION_COOKIE_SFLOW. */ + struct user_action_cookie_header header; ovs_be16 vlan_tci; /* Destination VLAN TCI. */ uint32_t output; /* SFL_FLOW_SAMPLE_TYPE 'output' value. */ } sflow; struct { - uint16_t type; /* USER_ACTION_COOKIE_SLOW_PATH. */ - uint16_t unused; + /* USER_ACTION_COOKIE_SLOW_PATH. */ + struct user_action_cookie_header header; uint32_t reason; /* enum slow_path_reason. */ } slow_path; struct { - uint16_t type; /* USER_ACTION_COOKIE_FLOW_SAMPLE. */ + /* USER_ACTION_COOKIE_FLOW_SAMPLE. */ + struct user_action_cookie_header header; uint16_t probability; /* Sampling probability. */ uint32_t collector_set_id; /* ID of IPFIX collector set. */ uint32_t obs_domain_id; /* Observation Domain ID. */ @@ -322,11 +331,12 @@ union user_action_cookie { } flow_sample; struct { - uint16_t type; /* USER_ACTION_COOKIE_IPFIX. */ + /* USER_ACTION_COOKIE_IPFIX. */ + struct user_action_cookie_header header; odp_port_t output_odp_port; /* The output odp port. */ } ipfix; }; -BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 24); +BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 48); size_t odp_put_userspace_action(uint32_t pid, const void *userdata, size_t userdata_size, diff --git a/ofproto/ofproto-dpif-upcall.c b/ofproto/ofproto-dpif-upcall.c index 46b75fe35a2b..88501803b5b8 100644 --- a/ofproto/ofproto-dpif-upcall.c +++ b/ofproto/ofproto-dpif-upcall.c @@ -38,6 +38,7 @@ #include "packets.h" #include "openvswitch/poll-loop.h" #include "seq.h" +#include "tunnel.h" #include "unixctl.h" #include "openvswitch/vlog.h" @@ -207,7 +208,7 @@ struct upcall { const ovs_u128 *ufid; /* Unique identifier for 'flow'. */ unsigned pmd_id; /* Datapath poll mode driver id. */ const struct dp_packet *packet; /* Packet associated with this upcall. */ - ofp_port_t in_port; /* OpenFlow in port, or OFPP_NONE. */ + ofp_port_t ofp_in_port; /* OpenFlow in port, or OFPP_NONE. */ uint16_t mru; /* If !0, Maximum receive unit of fragmented IP packet */ @@ -998,7 +999,7 @@ classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata, return BAD_UPCALL; } userdata_len = nl_attr_get_size(userdata); - if (userdata_len < sizeof cookie->type + if (userdata_len < sizeof cookie->header.type || userdata_len > sizeof *cookie) { VLOG_WARN_RL(&rl, "action upcall cookie has unexpected size %"PRIuSIZE, userdata_len); @@ -1007,21 +1008,21 @@ classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata, memcpy(cookie, nl_attr_get(userdata), userdata_len); - if (userdata_len == MAX(8, sizeof cookie->sflow) - && cookie->type == USER_ACTION_COOKIE_SFLOW) { + if (userdata_len == sizeof cookie->sflow + && cookie->header.type == USER_ACTION_COOKIE_SFLOW) { return SFLOW_UPCALL; - } else if (userdata_len == MAX(8, sizeof cookie->slow_path) - && cookie->type == USER_ACTION_COOKIE_SLOW_PATH) { + } else if (userdata_len == sizeof cookie->slow_path + && cookie->header.type == USER_ACTION_COOKIE_SLOW_PATH) { return SLOW_PATH_UPCALL; - } else if (userdata_len == MAX(8, sizeof cookie->flow_sample) - && cookie->type == USER_ACTION_COOKIE_FLOW_SAMPLE) { + } else if (userdata_len == sizeof cookie->flow_sample + && cookie->header.type == USER_ACTION_COOKIE_FLOW_SAMPLE) { return FLOW_SAMPLE_UPCALL; - } else if (userdata_len == MAX(8, sizeof cookie->ipfix) - && cookie->type == USER_ACTION_COOKIE_IPFIX) { + } else if (userdata_len == sizeof cookie->ipfix + && cookie->header.type == USER_ACTION_COOKIE_IPFIX) { return IPFIX_UPCALL; } else { VLOG_WARN_RL(&rl, "invalid user cookie of type %"PRIu16 - " and size %"PRIuSIZE, cookie->type, userdata_len); + " and size %"PRIuSIZE, cookie->header.type, userdata_len); return BAD_UPCALL; } } @@ -1030,16 +1031,18 @@ classify_upcall(enum dpif_upcall_type type, const struct nlattr *userdata, * initialized with at least 128 bytes of space. */ static void compose_slow_path(struct udpif *udpif, struct xlate_out *xout, - const struct flow *flow, odp_port_t odp_in_port, + const struct flow *flow, + odp_port_t odp_in_port, ofp_port_t ofp_in_port, struct ofpbuf *buf, uint32_t slowpath_meter_id, - uint32_t controller_meter_id) + uint32_t controller_meter_id, struct uuid *ofproto_uuid) { union user_action_cookie cookie; odp_port_t port; uint32_t pid; - cookie.type = USER_ACTION_COOKIE_SLOW_PATH; - cookie.slow_path.unused = 0; + cookie.header.type = USER_ACTION_COOKIE_SLOW_PATH; + cookie.header.ofp_in_port = ofp_in_port; + cookie.header.ofproto_uuid = *ofproto_uuid; cookie.slow_path.reason = xout->slow; port = xout->slow & (SLOW_CFM | SLOW_BFD | SLOW_LACP | SLOW_STP) @@ -1086,12 +1089,23 @@ upcall_receive(struct upcall *upcall, const struct dpif_backer *backer, upcall->type = classify_upcall(type, userdata, &upcall->cookie); if (upcall->type == BAD_UPCALL) { return EAGAIN; - } - - error = xlate_lookup(backer, flow, &upcall->ofproto, &upcall->ipfix, - &upcall->sflow, NULL, &upcall->in_port); - if (error) { - return error; + } else if (upcall->type == MISS_UPCALL) { + error = xlate_lookup(backer, flow, &upcall->ofproto, &upcall->ipfix, + &upcall->sflow, NULL, &upcall->ofp_in_port); + if (error) { + return error; + } + } else { + struct ofproto_dpif *ofproto + = ofproto_dpif_lookup_by_uuid(&upcall->cookie.header.ofproto_uuid); + if (!ofproto) { + VLOG_INFO_RL(&rl, "upcall could not find ofproto"); + return ENODEV; + } + upcall->ofproto = ofproto; + upcall->ipfix = ofproto->ipfix; + upcall->sflow = ofproto->sflow; + upcall->ofp_in_port = upcall->cookie.header.ofp_in_port; } upcall->recirc = NULL; @@ -1132,7 +1146,7 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall, xlate_in_init(&xin, upcall->ofproto, ofproto_dpif_get_tables_version(upcall->ofproto), - upcall->flow, upcall->in_port, NULL, + upcall->flow, upcall->ofp_in_port, NULL, stats.tcp_flags, upcall->packet, wc, odp_actions); if (upcall->type == MISS_UPCALL) { @@ -1177,8 +1191,9 @@ upcall_xlate(struct udpif *udpif, struct upcall *upcall, uint32_t cmid = upcall->ofproto->up.controller_meter_id; /* upcall->put_actions already initialized by upcall_receive(). */ compose_slow_path(udpif, &upcall->xout, upcall->flow, - upcall->flow->in_port.odp_port, - &upcall->put_actions, smid, cmid); + upcall->flow->in_port.odp_port, upcall->ofp_in_port, + &upcall->put_actions, smid, cmid, + &upcall->ofproto->uuid); } /* This function is also called for slow-pathed flows. As we are only @@ -2045,13 +2060,16 @@ revalidate_ukey__(struct udpif *udpif, const struct udpif_key *ukey, if (xoutp->slow) { struct ofproto_dpif *ofproto; - ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, NULL); + ofp_port_t ofp_in_port; + + ofproto = xlate_lookup_ofproto(udpif->backer, &ctx.flow, + &ofp_in_port); uint32_t smid = ofproto ? ofproto->up.slowpath_meter_id : UINT32_MAX; uint32_t cmid = ofproto ? ofproto->up.controller_meter_id : UINT32_MAX; ofpbuf_clear(odp_actions); compose_slow_path(udpif, xoutp, &ctx.flow, ctx.flow.in_port.odp_port, - odp_actions, smid, cmid); + ofp_in_port, odp_actions, smid, cmid, &ofproto->uuid); } if (odp_flow_key_to_mask(ukey->mask, ukey->mask_len, &dp_mask, &ctx.flow) diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 3bdd64d4d552..ec92c6a730bf 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -2927,7 +2927,13 @@ compose_sflow_action(struct xlate_ctx *ctx) return 0; } - union user_action_cookie cookie = { .type = USER_ACTION_COOKIE_SFLOW }; + union user_action_cookie cookie = { + .sflow = { + .header.type = USER_ACTION_COOKIE_SFLOW, + .header.ofp_in_port = ctx->xin->flow.in_port.ofp_port, + .header.ofproto_uuid = ctx->xbridge->ofproto->uuid + } + }; return compose_sample_action(ctx, dpif_sflow_get_probability(sflow), &cookie, sizeof cookie.sflow, ODPP_NONE, true); @@ -2970,7 +2976,9 @@ compose_ipfix_action(struct xlate_ctx *ctx, odp_port_t output_odp_port) union user_action_cookie cookie = { .ipfix = { - .type = USER_ACTION_COOKIE_IPFIX, + .header.type = USER_ACTION_COOKIE_IPFIX, + .header.ofp_in_port = ctx->xin->flow.in_port.ofp_port, + .header.ofproto_uuid = ctx->xbridge->ofproto->uuid, .output_odp_port = output_odp_port, } }; @@ -2992,9 +3000,8 @@ fix_sflow_action(struct xlate_ctx *ctx, unsigned int user_cookie_offset) cookie = ofpbuf_at(ctx->odp_actions, user_cookie_offset, sizeof cookie->sflow); - ovs_assert(cookie->type == USER_ACTION_COOKIE_SFLOW); + ovs_assert(cookie->header.type == USER_ACTION_COOKIE_SFLOW); - cookie->type = USER_ACTION_COOKIE_SFLOW; cookie->sflow.vlan_tci = base->vlans[0].tci; /* See http://www.sflow.org/sflow_version_5.txt (search for "Input/output @@ -5265,7 +5272,9 @@ xlate_sample_action(struct xlate_ctx *ctx, union user_action_cookie cookie = { .flow_sample = { - .type = USER_ACTION_COOKIE_FLOW_SAMPLE, + .header.type = USER_ACTION_COOKIE_FLOW_SAMPLE, + .header.ofp_in_port = ctx->xin->flow.in_port.ofp_port, + .header.ofproto_uuid = ctx->xbridge->ofproto->uuid, .probability = os->probability, .collector_set_id = os->collector_set_id, .obs_domain_id = os->obs_domain_id, diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c index 7d628e11328a..b0a1390d816b 100644 --- a/ofproto/ofproto-dpif.c +++ b/ofproto/ofproto-dpif.c @@ -161,9 +161,6 @@ struct ofport_dpif { static odp_port_t ofp_port_to_odp_port(const struct ofproto_dpif *, ofp_port_t); -static ofp_port_t odp_port_to_ofp_port(const struct ofproto_dpif *, - odp_port_t); - static struct ofport_dpif * ofport_dpif_cast(const struct ofport *ofport) { @@ -5622,7 +5619,7 @@ odp_port_to_ofport(const struct dpif_backer *backer, odp_port_t odp_port) return NULL; } -static ofp_port_t +ofp_port_t odp_port_to_ofp_port(const struct ofproto_dpif *ofproto, odp_port_t odp_port) { struct ofport_dpif *port; diff --git a/ofproto/ofproto-dpif.h b/ofproto/ofproto-dpif.h index 0bb07638c15e..0f7086824f3d 100644 --- a/ofproto/ofproto-dpif.h +++ b/ofproto/ofproto-dpif.h @@ -248,6 +248,8 @@ struct dpif_backer { extern struct shash all_dpif_backers; struct ofport_dpif *odp_port_to_ofport(const struct dpif_backer *, odp_port_t); + +ofp_port_t odp_port_to_ofp_port(const struct ofproto_dpif *, odp_port_t); /* A bridge based on a "dpif" datapath. */
Previously, the ofproto instance and OpenFlow port have been derived based on the datapath port number. This change explicitly declares them both, which will be helpful in future commits that no longer can depend on having a unique datapath port (e.g., a source port that represents the controller). Signed-off-by: Justin Pettit <jpettit@ovn.org> --- lib/odp-util.c | 27 ++++++++++------- lib/odp-util.h | 24 ++++++++++----- ofproto/ofproto-dpif-upcall.c | 70 +++++++++++++++++++++++++++---------------- ofproto/ofproto-dpif-xlate.c | 19 ++++++++---- ofproto/ofproto-dpif.c | 5 +--- ofproto/ofproto-dpif.h | 2 ++ 6 files changed, 95 insertions(+), 52 deletions(-)