[ovs-dev] [PATCH net-next V2 1/1] openvswitch: Declare ovs key structures using macros

[Yi-Hung Wei]

On Sun, Feb 3, 2019 at 1:13 AM Eli Britstein <elibr at mellanox.com> wrote:
>
> Declare ovs key structures using macros as a pre-step towards to
> enable retrieving fields information, as a work done in proposed
> commit in the OVS tree https://patchwork.ozlabs.org/patch/1023406/
> ("odp-util: Do not rewrite fields with the same values as matched"),
> with no functional change.
>
> Signed-off-by: Eli Britstein <elibr at mellanox.com>
> Reviewed-by: Roi Dayan <roid at mellanox.com>
> ---
>  include/uapi/linux/openvswitch.h | 102 ++++++++++++++++++++++++++-------------
>  1 file changed, 69 insertions(+), 33 deletions(-)
>
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index dbe0cbe4f1b7..dc8246f871fd 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -387,73 +387,109 @@ enum ovs_frag_type {
>
>  #define OVS_FRAG_TYPE_MAX (__OVS_FRAG_TYPE_MAX - 1)
>
> +#define OVS_KEY_FIELD_ARR(type, name, elements) type name[elements];
> +#define OVS_KEY_FIELD(type, name) type name;
......
> +#define OVS_KEY_IPV6_FIELDS \
> +    OVS_KEY_FIELD_ARR(__be32, ipv6_src, 4) \
> +    OVS_KEY_FIELD_ARR(__be32, ipv6_dst, 4) \
> +    OVS_KEY_FIELD(__be32, ipv6_label /* 20-bits in least-significant bits. */) \
> +    OVS_KEY_FIELD(__u8, ipv6_proto) \
> +    OVS_KEY_FIELD(__u8, ipv6_tclass) \
> +    OVS_KEY_FIELD(__u8, ipv6_hlimit) \
> +    OVS_KEY_FIELD(__u8, ipv6_frag /* One of OVS_FRAG_TYPE_*. */)
> +
>  struct ovs_key_ipv6 {
> -       __be32 ipv6_src[4];
> -       __be32 ipv6_dst[4];
> -       __be32 ipv6_label;      /* 20-bits in least-significant bits. */
> -       __u8   ipv6_proto;
> -       __u8   ipv6_tclass;
> -       __u8   ipv6_hlimit;
> -       __u8   ipv6_frag;       /* One of OVS_FRAG_TYPE_*. */
> +    OVS_KEY_IPV6_FIELDS
>  };

Hi Eli,

Thanks for the patch.  In my personal opinion, I feel this patch makes
the header file harder to read.

For example, to see how 'struct ovs_key_ipv6' is defined, now we need
to trace how OVS_KEY_IPV6_FIELDS is defined, and how OVS_KEY_FIELD_ARR
and OVS_KEY_FIELD defined.  I think it makes the header file to be
more complicated.

There are also some discussion on ovs-dev mailing list about this
patch: https://patchwork.ozlabs.org/cover/1023404/

Thanks,

-Yi-Hung