On Tue, Jul 15, 2014 at 5:15 PM, Joe Stringer <joestrin...@nicira.com> wrote:
> Signed-off-by: Joe Stringer <joestrin...@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 <pshe...@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@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
