Looks good, thanks.

Ethan

On Sat, May 5, 2012 at 11:10 AM, Ben Pfaff <[email protected]> wrote:
> An upcoming commit will introduce a new type and a new use for the
> additional members.  It seems cleanest to use a union, rather that using
> the existing members multiple ways.
>
> Signed-off-by: Ben Pfaff <[email protected]>
> ---
>  lib/odp-util.c               |   17 +++++++++--------
>  lib/odp-util.h               |   14 ++++++++------
>  ofproto/ofproto-dpif-sflow.c |   10 ++++++----
>  ofproto/ofproto-dpif-sflow.h |    4 ++--
>  ofproto/ofproto-dpif.c       |   18 +++++++++---------
>  5 files changed, 34 insertions(+), 29 deletions(-)
>
> diff --git a/lib/odp-util.c b/lib/odp-util.c
> index 5f65b62..78bcf64 100644
> --- a/lib/odp-util.c
> +++ b/lib/odp-util.c
> @@ -185,7 +185,7 @@ format_odp_userspace_action(struct ds *ds, const struct 
> nlattr *attr)
>
>     if (a[OVS_USERSPACE_ATTR_USERDATA]) {
>         uint64_t userdata = nl_attr_get_u64(a[OVS_USERSPACE_ATTR_USERDATA]);
> -        struct user_action_cookie cookie;
> +        union user_action_cookie cookie;
>
>         memcpy(&cookie, &userdata, sizeof cookie);
>
> @@ -193,8 +193,9 @@ format_odp_userspace_action(struct ds *ds, const struct 
> nlattr *attr)
>         case USER_ACTION_COOKIE_SFLOW:
>             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);
> +                          vlan_tci_to_vid(cookie.sflow.vlan_tci),
> +                          vlan_tci_to_pcp(cookie.sflow.vlan_tci),
> +                          cookie.sflow.output);
>             break;
>             break;
>
> @@ -348,7 +349,7 @@ parse_odp_action(const char *s, const struct shash 
> *port_names,
>         } 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;
> +            union user_action_cookie cookie;
>             uint16_t tci;
>
>             tci = vid | (pcp << VLAN_PCP_SHIFT);
> @@ -357,14 +358,14 @@ parse_odp_action(const char *s, const struct shash 
> *port_names,
>             }
>
>             cookie.type = USER_ACTION_COOKIE_SFLOW;
> -            cookie.vlan_tci = htons(tci);
> -            cookie.output = output;
> +            cookie.sflow.vlan_tci = htons(tci);
> +            cookie.sflow.output = output;
>             odp_put_userspace_action(pid, &cookie, actions);
>             return n;
>         } else if (sscanf(s, "userspace(pid=%lli,userdata="
>                           "%31[x0123456789abcdefABCDEF])%n", &pid, userdata_s,
>                           &n) > 0 && n > 0) {
> -            struct user_action_cookie cookie;
> +            union user_action_cookie cookie;
>             uint64_t userdata;
>
>             userdata = strtoull(userdata_s, NULL, 0);
> @@ -1726,7 +1727,7 @@ odp_key_fitness_to_string(enum odp_key_fitness fitness)
>  * the start of the cookie.  (If 'cookie' is null, then the return value is 
> not
>  * meaningful.) */
>  size_t
> -odp_put_userspace_action(uint32_t pid, const struct user_action_cookie 
> *cookie,
> +odp_put_userspace_action(uint32_t pid, const union user_action_cookie 
> *cookie,
>                          struct ofpbuf *odp_actions)
>  {
>     size_t offset;
> diff --git a/lib/odp-util.h b/lib/odp-util.h
> index cdafbe4..c52e470 100644
> --- a/lib/odp-util.h
> +++ b/lib/odp-util.h
> @@ -123,18 +123,20 @@ 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 {
> +union user_action_cookie {
>     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. */
> +    struct {
> +        uint16_t type;          /* USER_ACTION_COOKIE_SFLOW. */
> +        ovs_be16 vlan_tci;      /* Destination VLAN TCI. */
> +        uint32_t output;        /* SFL_FLOW_SAMPLE_TYPE 'output' value. */
> +    } sflow;
>  };
>
> -BUILD_ASSERT_DECL(sizeof(struct user_action_cookie) == 8);
> +BUILD_ASSERT_DECL(sizeof(union user_action_cookie) == 8);
>
>  size_t odp_put_userspace_action(uint32_t pid,
> -                                const struct user_action_cookie *,
> +                                const union user_action_cookie *,
>                                 struct ofpbuf *odp_actions);
>
>  void commit_odp_actions(const struct flow *, struct flow *base,
> diff --git a/ofproto/ofproto-dpif-sflow.c b/ofproto/ofproto-dpif-sflow.c
> index 78a6dd6..611f89a 100644
> --- a/ofproto/ofproto-dpif-sflow.c
> +++ b/ofproto/ofproto-dpif-sflow.c
> @@ -491,7 +491,7 @@ dpif_sflow_odp_port_to_ifindex(const struct dpif_sflow 
> *ds,
>  void
>  dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf *packet,
>                     const struct flow *flow,
> -                    const struct user_action_cookie *cookie)
> +                    const union user_action_cookie *cookie)
>  {
>     SFL_FLOW_SAMPLE_TYPE fs;
>     SFLFlow_sample_element hdrElem;
> @@ -500,6 +500,7 @@ dpif_sflow_received(struct dpif_sflow *ds, struct ofpbuf 
> *packet,
>     SFLSampler *sampler;
>     struct dpif_sflow_port *in_dsp;
>     struct netdev_stats stats;
> +    ovs_be16 vlan_tci;
>     int error;
>
>     /* Build a flow sample */
> @@ -549,10 +550,11 @@ dpif_sflow_received(struct dpif_sflow *ds, struct 
> ofpbuf *packet,
>     switchElem.flowType.sw.src_priority = vlan_tci_to_pcp(flow->vlan_tci);
>
>     /* Retrieve data from user_action_cookie. */
> -    switchElem.flowType.sw.dst_vlan = vlan_tci_to_vid(cookie->vlan_tci);
> -    switchElem.flowType.sw.dst_priority = vlan_tci_to_pcp(cookie->vlan_tci);
> +    vlan_tci = cookie->sflow.vlan_tci;
> +    switchElem.flowType.sw.dst_vlan = vlan_tci_to_vid(vlan_tci);
> +    switchElem.flowType.sw.dst_priority = vlan_tci_to_pcp(vlan_tci);
>
> -    fs.output = cookie->output;
> +    fs.output = cookie->sflow.output;
>
>     /* Submit the flow sample to be encoded into the next datagram. */
>     SFLADD_ELEMENT(&fs, &hdrElem);
> diff --git a/ofproto/ofproto-dpif-sflow.h b/ofproto/ofproto-dpif-sflow.h
> index 8d388d0..6af8dc0 100644
> --- a/ofproto/ofproto-dpif-sflow.h
> +++ b/ofproto/ofproto-dpif-sflow.h
> @@ -1,6 +1,6 @@
>  /*
>  * Copyright (c) 2009, 2010 InMon Corp.
> - * Copyright (c) 2009 Nicira, Inc.
> + * Copyright (c) 2009, 2012 Nicira, Inc.
>  *
>  * Licensed under the Apache License, Version 2.0 (the "License");
>  * you may not use this file except in compliance with the License.
> @@ -46,7 +46,7 @@ void dpif_sflow_wait(struct dpif_sflow *);
>  void dpif_sflow_received(struct dpif_sflow *,
>                          struct ofpbuf *,
>                          const struct flow *,
> -                         const struct user_action_cookie *);
> +                         const union user_action_cookie *);
>
>  int dpif_sflow_odp_port_to_ifindex(const struct dpif_sflow *, uint16_t);
>
> diff --git a/ofproto/ofproto-dpif.c b/ofproto/ofproto-dpif.c
> index e86ff59..887db15 100644
> --- a/ofproto/ofproto-dpif.c
> +++ b/ofproto/ofproto-dpif.c
> @@ -3017,7 +3017,7 @@ static void
>  handle_userspace_upcall(struct ofproto_dpif *ofproto,
>                         struct dpif_upcall *upcall)
>  {
> -    struct user_action_cookie cookie;
> +    union user_action_cookie cookie;
>     enum odp_key_fitness fitness;
>     ovs_be16 initial_tci;
>     struct flow flow;
> @@ -4513,7 +4513,7 @@ static size_t
>  put_userspace_action(const struct ofproto_dpif *ofproto,
>                      struct ofpbuf *odp_actions,
>                      const struct flow *flow,
> -                     const struct user_action_cookie *cookie)
> +                     const union user_action_cookie *cookie)
>  {
>     uint32_t pid;
>
> @@ -4526,31 +4526,31 @@ put_userspace_action(const struct ofproto_dpif 
> *ofproto,
>  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)
> +                     unsigned int n_outputs, union user_action_cookie 
> *cookie)
>  {
>     int ifindex;
>
>     cookie->type = USER_ACTION_COOKIE_SFLOW;
> -    cookie->vlan_tci = vlan_tci;
> +    cookie->sflow.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;
> +        cookie->sflow.output = 0x40000000 | 256;
>         break;
>
>     case 1:
>         ifindex = dpif_sflow_odp_port_to_ifindex(ofproto->sflow, odp_port);
>         if (ifindex) {
> -            cookie->output = ifindex;
> +            cookie->sflow.output = ifindex;
>             break;
>         }
>         /* Fall through. */
>     default:
>         /* 0x80000000 means "multiple output ports. */
> -        cookie->output = 0x80000000 | n_outputs;
> +        cookie->sflow.output = 0x80000000 | n_outputs;
>         break;
>     }
>  }
> @@ -4563,7 +4563,7 @@ compose_sflow_action(const struct ofproto_dpif *ofproto,
>                      uint32_t odp_port)
>  {
>     uint32_t probability;
> -    struct user_action_cookie cookie;
> +    union user_action_cookie cookie;
>     size_t sample_offset, actions_offset;
>     int cookie_offset;
>
> @@ -4607,7 +4607,7 @@ static void
>  fix_sflow_action(struct action_xlate_ctx *ctx)
>  {
>     const struct flow *base = &ctx->base_flow;
> -    struct user_action_cookie *cookie;
> +    union user_action_cookie *cookie;
>
>     if (!ctx->user_cookie_offset) {
>         return;
> --
> 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