This patch adds a redundant break to format_odp_userspace_action(), otherwise looks good, thanks.
Ethan On Sat, May 5, 2012 at 11:10 AM, Ben Pfaff <[email protected]> wrote: > Signed-off-by: Ben Pfaff <[email protected]> > --- > lib/odp-util.c | 21 ++++++------- > lib/odp-util.h | 10 +++--- > ofproto/ofproto-dpif-sflow.c | 13 +-------- > ofproto/ofproto-dpif.c | 65 ++++++++++++++++++++++++----------------- > 4 files changed, 54 insertions(+), 55 deletions(-) > > diff --git a/lib/odp-util.c b/lib/odp-util.c > index 8d0e95f..5f65b62 100644 > --- a/lib/odp-util.c > +++ b/lib/odp-util.c > @@ -191,10 +191,11 @@ format_odp_userspace_action(struct ds *ds, const struct > nlattr *attr) > > switch (cookie.type) { > case USER_ACTION_COOKIE_SFLOW: > - ds_put_format(ds, ",sFlow,n_output=%"PRIu8"," > - "vid=%"PRIu16",pcp=%"PRIu8",ifindex=%"PRIu32, > - cookie.n_output, vlan_tci_to_vid(cookie.vlan_tci), > - vlan_tci_to_pcp(cookie.vlan_tci), cookie.data); > + ds_put_format(ds, ",sFlow," > + "vid=%"PRIu16",pcp=%"PRIu8",output=%"PRIu32, > + vlan_tci_to_vid(cookie.vlan_tci), > + vlan_tci_to_pcp(cookie.vlan_tci), cookie.output); > + break; > break; > > case USER_ACTION_COOKIE_UNSPEC: > @@ -336,18 +337,17 @@ parse_odp_action(const char *s, const struct shash > *port_names, > > { > unsigned long long int pid; > - unsigned long long int ifindex; > + unsigned long long int output; > char userdata_s[32]; > - int n_output; > int vid, pcp; > int n = -1; > > if (sscanf(s, "userspace(pid=%lli)%n", &pid, &n) > 0 && n > 0) { > odp_put_userspace_action(pid, NULL, actions); > return n; > - } else if (sscanf(s, "userspace(pid=%lli,sFlow,n_output=%i,vid=%i," > - "pcp=%i,ifindex=%lli)%n", &pid, &n_output, > - &vid, &pcp, &ifindex, &n) > 0 && n > 0) { > + } else if (sscanf(s, "userspace(pid=%lli,sFlow,vid=%i," > + "pcp=%i,output=%lli)%n", > + &pid, &vid, &pcp, &output, &n) > 0 && n > 0) { > struct user_action_cookie cookie; > uint16_t tci; > > @@ -357,9 +357,8 @@ parse_odp_action(const char *s, const struct shash > *port_names, > } > > cookie.type = USER_ACTION_COOKIE_SFLOW; > - cookie.n_output = n_output; > cookie.vlan_tci = htons(tci); > - cookie.data = ifindex; > + cookie.output = output; > odp_put_userspace_action(pid, &cookie, actions); > return n; > } else if (sscanf(s, "userspace(pid=%lli,userdata=" > diff --git a/lib/odp-util.h b/lib/odp-util.h > index 3d337a7..cdafbe4 100644 > --- a/lib/odp-util.h > +++ b/lib/odp-util.h > @@ -124,11 +124,11 @@ enum user_action_cookie_type { > /* user_action_cookie is passed as argument to OVS_ACTION_ATTR_USERSPACE. > * Since it is passed to kernel as u64, its size has to be 8 bytes. */ > struct user_action_cookie { > - uint8_t type; /* enum user_action_cookie_type. */ > - uint8_t n_output; /* No of output ports. used by sflow. */ > - ovs_be16 vlan_tci; /* Used by sFlow */ > - uint32_t data; /* Data is len for OFPP_CONTROLLER > action. > - For sFlow it is port_ifindex. */ > + uint16_t type; /* enum user_action_cookie_type. */ > + > + /* The following members are used only by USER_ACTION_COOKIE_SFLOW. */ > + ovs_be16 vlan_tci; /* Destination VLAN TCI. */ > + uint32_t output; /* SFL_FLOW_SAMPLE_TYPE 'output' value. */ > }; > > BUILD_ASSERT_DECL(sizeof(struct user_action_cookie) == 8); > diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c > index fb09b2f..78a6dd6 100644 > --- a/ofproto/ofproto-dpif-sflow.c > +++ b/ofproto/ofproto-dpif-sflow.c > @@ -552,18 +552,7 @@ dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf > *packet, > switchElem.flowType.sw.dst_vlan = vlan_tci_to_vid(cookie->vlan_tci); > switchElem.flowType.sw.dst_priority = vlan_tci_to_pcp(cookie->vlan_tci); > > - /* Set output port, as defined by > http://www.sflow.org/sflow_version_5.txt > - (search for "Input/output port information"). */ > - if (!cookie->n_output) { > - /* This value indicates that the packet was dropped for an unknown > - * reason. */ > - fs.output = 0x40000000 | 256; > - } else if (cookie->n_output > 1 || !cookie->data) { > - /* Setting the high bit means "multiple output ports". */ > - fs.output = 0x80000000 | cookie->n_output; > - } else { > - fs.output = cookie->data; > - } > + fs.output = cookie->output; > > /* Submit the flow sample to be encoded into the next datagram. */ > SFLADD_ELEMENT(&fs, &hdrElem); > diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c > index 5a8edcd..e86ff59 100644 > --- a/ofproto/ofproto-dpif.c > +++ b/ofproto/ofproto-dpif.c > @@ -4523,6 +4523,38 @@ put_userspace_action(const struct ofproto_dpif > *ofproto, > return odp_put_userspace_action(pid, cookie, odp_actions); > } > > +static void > +compose_sflow_cookie(const struct ofproto_dpif *ofproto, > + ovs_be16 vlan_tci, uint32_t odp_port, > + unsigned int n_outputs, struct user_action_cookie > *cookie) > +{ > + int ifindex; > + > + cookie->type = USER_ACTION_COOKIE_SFLOW; > + cookie->vlan_tci = vlan_tci; > + > + /* See http://www.sflow.org/sflow_version_5.txt (search for "Input/output > + * port information") for the interpretation of cookie->output. */ > + switch (n_outputs) { > + case 0: > + /* 0x40000000 | 256 means "packet dropped for unknown reason". */ > + cookie->output = 0x40000000 | 256; > + break; > + > + case 1: > + ifindex = dpif_sflow_odp_port_to_ifindex(ofproto->sflow, odp_port); > + if (ifindex) { > + cookie->output = ifindex; > + break; > + } > + /* Fall through. */ > + default: > + /* 0x80000000 means "multiple output ports. */ > + cookie->output = 0x80000000 | n_outputs; > + break; > + } > +} > + > /* Compose SAMPLE action for sFlow. */ > static size_t > compose_sflow_action(const struct ofproto_dpif *ofproto, > @@ -4530,24 +4562,15 @@ compose_sflow_action(const struct ofproto_dpif > *ofproto, > const struct flow *flow, > uint32_t odp_port) > { > - uint32_t port_ifindex; > uint32_t probability; > struct user_action_cookie cookie; > size_t sample_offset, actions_offset; > - int cookie_offset, n_output; > + int cookie_offset; > > if (!ofproto->sflow || flow->in_port == OFPP_NONE) { > return 0; > } > > - if (odp_port == OVSP_NONE) { > - port_ifindex = 0; > - n_output = 0; > - } else { > - port_ifindex = dpif_sflow_odp_port_to_ifindex(ofproto->sflow, > odp_port); > - n_output = 1; > - } > - > sample_offset = nl_msg_start_nested(odp_actions, OVS_ACTION_ATTR_SAMPLE); > > /* Number of packets out of UINT_MAX to sample. */ > @@ -4555,11 +4578,8 @@ compose_sflow_action(const struct ofproto_dpif > *ofproto, > nl_msg_put_u32(odp_actions, OVS_SAMPLE_ATTR_PROBABILITY, probability); > > actions_offset = nl_msg_start_nested(odp_actions, > OVS_SAMPLE_ATTR_ACTIONS); > - > - cookie.type = USER_ACTION_COOKIE_SFLOW; > - cookie.data = port_ifindex; > - cookie.n_output = n_output; > - cookie.vlan_tci = 0; > + compose_sflow_cookie(ofproto, htons(0), odp_port, > + odp_port == OVSP_NONE ? 0 : 1, &cookie); > cookie_offset = put_userspace_action(ofproto, odp_actions, flow, &cookie); > > nl_msg_end_nested(odp_actions, actions_offset); > @@ -4594,20 +4614,11 @@ fix_sflow_action(struct action_xlate_ctx *ctx) > } > > cookie = ofpbuf_at(ctx->odp_actions, ctx->user_cookie_offset, > - sizeof(*cookie)); > - assert(cookie != NULL); > + sizeof(*cookie)); > assert(cookie->type == USER_ACTION_COOKIE_SFLOW); > > - if (ctx->sflow_n_outputs) { > - cookie->data = dpif_sflow_odp_port_to_ifindex(ctx->ofproto->sflow, > - ctx->sflow_odp_port); > - } > - if (ctx->sflow_n_outputs >= 255) { > - cookie->n_output = 255; > - } else { > - cookie->n_output = ctx->sflow_n_outputs; > - } > - cookie->vlan_tci = base->vlan_tci; > + compose_sflow_cookie(ctx->ofproto, base->vlan_tci, > + ctx->sflow_odp_port, ctx->sflow_n_outputs, cookie); > } > > static void > -- > 1.7.2.5 > > _______________________________________________ > dev mailing list > [email protected] > http://openvswitch.org/mailman/listinfo/dev _______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
