[ovs-dev] [PATCHv2 1/2] datapath: Move key_attr_size() to flow_netlink.h.

Pravin Shelar pshelar at nicira.com
Wed Aug 13 22:22:44 UTC 2014


On Tue, Jul 15, 2014 at 5:15 PM, Joe Stringer <joestringer at nicira.com> wrote:
> Signed-off-by: Joe Stringer <joestringer at nicira.com>
> ---
> v2: First post.
>
> Side comment: OVS_KEY_ATTR_ETHERTYPE appears twice in this list. I guess
> it's not harmful to reserve a bit of extra space, but does anyone know
> why it's there twice?
> ---
>  datapath/datapath.c     |   37 +++----------------------------------
>  datapath/flow_netlink.c |   31 +++++++++++++++++++++++++++++++
>  datapath/flow_netlink.h |    2 ++
>  3 files changed, 36 insertions(+), 34 deletions(-)
>
> diff --git a/datapath/datapath.c b/datapath/datapath.c
> index 065356f..b1c757c 100644
> --- a/datapath/datapath.c
> +++ b/datapath/datapath.c
> @@ -376,43 +376,12 @@ static int queue_gso_packets(struct datapath *dp, struct sk_buff *skb,
>         return err;
>  }
>
> -static size_t key_attr_size(void)
> -{
> -       /* Whenever adding new OVS_KEY_ FIELDS, we should consider
> -        * updating this function.  */
> -       BUILD_BUG_ON(OVS_KEY_ATTR_TUNNEL_INFO != 22);
> -
> -       return    nla_total_size(4)   /* OVS_KEY_ATTR_PRIORITY */
> -               + nla_total_size(0)   /* OVS_KEY_ATTR_TUNNEL */
> -                 + nla_total_size(8)   /* OVS_TUNNEL_KEY_ATTR_ID */
> -                 + nla_total_size(4)   /* OVS_TUNNEL_KEY_ATTR_IPV4_SRC */
> -                 + nla_total_size(4)   /* OVS_TUNNEL_KEY_ATTR_IPV4_DST */
> -                 + nla_total_size(1)   /* OVS_TUNNEL_KEY_ATTR_TOS */
> -                 + nla_total_size(1)   /* OVS_TUNNEL_KEY_ATTR_TTL */
> -                 + nla_total_size(0)   /* OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT */
> -                 + nla_total_size(0)   /* OVS_TUNNEL_KEY_ATTR_CSUM */
> -                 + nla_total_size(0)   /* OVS_TUNNEL_KEY_ATTR_OAM */
> -                 + nla_total_size(256) /* OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS */
> -               + nla_total_size(4)   /* OVS_KEY_ATTR_IN_PORT */
> -               + nla_total_size(4)   /* OVS_KEY_ATTR_SKB_MARK */
> -               + nla_total_size(4)   /* OVS_KEY_ATTR_DP_HASH */
> -               + nla_total_size(4)   /* OVS_KEY_ATTR_RECIRC_ID */
> -               + nla_total_size(12)  /* OVS_KEY_ATTR_ETHERNET */
> -               + nla_total_size(2)   /* OVS_KEY_ATTR_ETHERTYPE */
> -               + nla_total_size(4)   /* OVS_KEY_ATTR_8021Q */
> -               + nla_total_size(0)   /* OVS_KEY_ATTR_ENCAP */
> -               + nla_total_size(2)   /* OVS_KEY_ATTR_ETHERTYPE */
> -               + nla_total_size(40)  /* OVS_KEY_ATTR_IPV6 */
> -               + nla_total_size(2)   /* OVS_KEY_ATTR_ICMPV6 */
> -               + nla_total_size(28); /* OVS_KEY_ATTR_ND */
> -}
> -
>  static size_t upcall_msg_size(const struct nlattr *userdata,
>                               unsigned int hdrlen)
>  {
>         size_t size = NLMSG_ALIGN(sizeof(struct ovs_header))
>                 + nla_total_size(hdrlen) /* OVS_PACKET_ATTR_PACKET */
> -               + nla_total_size(key_attr_size()); /* OVS_PACKET_ATTR_KEY */
> +               + nla_total_size(ovs_key_attr_size()); /* OVS_PACKET_ATTR_KEY */
>
>         /* OVS_PACKET_ATTR_USERDATA */
>         if (userdata)
> @@ -687,8 +656,8 @@ static void get_dp_stats(struct datapath *dp, struct ovs_dp_stats *stats,
>  static size_t ovs_flow_cmd_msg_size(const struct sw_flow_actions *acts)
>  {
>         return NLMSG_ALIGN(sizeof(struct ovs_header))
> -               + nla_total_size(key_attr_size()) /* OVS_FLOW_ATTR_KEY */
> -               + nla_total_size(key_attr_size()) /* OVS_FLOW_ATTR_MASK */
> +               + nla_total_size(ovs_key_attr_size()) /* OVS_FLOW_ATTR_KEY */
> +               + nla_total_size(ovs_key_attr_size()) /* OVS_FLOW_ATTR_MASK */
>                 + nla_total_size(sizeof(struct ovs_flow_stats)) /* OVS_FLOW_ATTR_STATS */
>                 + nla_total_size(1) /* OVS_FLOW_ATTR_TCP_FLAGS */
>                 + nla_total_size(8) /* OVS_FLOW_ATTR_USED */
> diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
> index 5f975a1..08b628a 100644
> --- a/datapath/flow_netlink.c
> +++ b/datapath/flow_netlink.c
> @@ -51,6 +51,37 @@
>
>  #include "flow_netlink.h"
>
> +size_t ovs_key_attr_size(void)
> +{
> +       /* Whenever adding new OVS_KEY_ FIELDS, we should consider
> +        * updating this function.  */
> +       BUILD_BUG_ON(OVS_KEY_ATTR_TUNNEL_INFO != 22);
> +
> +       return    nla_total_size(4)   /* OVS_KEY_ATTR_PRIORITY */
> +               + nla_total_size(0)   /* OVS_KEY_ATTR_TUNNEL */
> +                 + nla_total_size(8)   /* OVS_TUNNEL_KEY_ATTR_ID */
> +                 + nla_total_size(4)   /* OVS_TUNNEL_KEY_ATTR_IPV4_SRC */
> +                 + nla_total_size(4)   /* OVS_TUNNEL_KEY_ATTR_IPV4_DST */
> +                 + nla_total_size(1)   /* OVS_TUNNEL_KEY_ATTR_TOS */
> +                 + nla_total_size(1)   /* OVS_TUNNEL_KEY_ATTR_TTL */
> +                 + nla_total_size(0)   /* OVS_TUNNEL_KEY_ATTR_DONT_FRAGMENT */
> +                 + nla_total_size(0)   /* OVS_TUNNEL_KEY_ATTR_CSUM */
> +                 + nla_total_size(0)   /* OVS_TUNNEL_KEY_ATTR_OAM */
> +                 + nla_total_size(256) /* OVS_TUNNEL_KEY_ATTR_GENEVE_OPTS */
> +               + nla_total_size(4)   /* OVS_KEY_ATTR_IN_PORT */
> +               + nla_total_size(4)   /* OVS_KEY_ATTR_SKB_MARK */
> +               + nla_total_size(4)   /* OVS_KEY_ATTR_DP_HASH */
> +               + nla_total_size(4)   /* OVS_KEY_ATTR_RECIRC_ID */
> +               + nla_total_size(12)  /* OVS_KEY_ATTR_ETHERNET */
> +               + nla_total_size(2)   /* OVS_KEY_ATTR_ETHERTYPE */
> +               + nla_total_size(4)   /* OVS_KEY_ATTR_8021Q */
> +               + nla_total_size(0)   /* OVS_KEY_ATTR_ENCAP */
> +               + nla_total_size(2)   /* OVS_KEY_ATTR_ETHERTYPE */
> +               + nla_total_size(40)  /* OVS_KEY_ATTR_IPV6 */
> +               + nla_total_size(2)   /* OVS_KEY_ATTR_ICMPV6 */
> +               + nla_total_size(28); /* OVS_KEY_ATTR_ND */
> +}
> +
Can you move this definition next to ovs_key_lens definition.

otherwise looks good.
Acked-by: Pravin B Shelar <pshelar at nicira.com>

>  static void update_range__(struct sw_flow_match *match,
>                            size_t offset, size_t size, bool is_mask)
>  {
> diff --git a/datapath/flow_netlink.h b/datapath/flow_netlink.h
> index 0c20e86..274e43d 100644
> --- a/datapath/flow_netlink.h
> +++ b/datapath/flow_netlink.h
> @@ -37,6 +37,8 @@
>
>  #include "flow.h"
>
> +size_t ovs_key_attr_size(void);
> +
>  void ovs_match_init(struct sw_flow_match *match,
>                     struct sw_flow_key *key, struct sw_flow_mask *mask);
>
> --
> 1.7.10.4
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev



More information about the dev mailing list