Thanks, I fixed that up.

On Tue, May 08, 2012 at 01:42:07PM -0700, Ethan Jackson wrote:
> 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

Reply via email to