| Message ID | 20190203091217.8459-1-elibr@mellanox.com |
|---|---|
| State | Rejected |
| Delegated to: | David Miller |
| Headers | show |
| Series | [net-next,V2,1/1] openvswitch: Declare ovs key structures using macros | expand |
On Sun, Feb 3, 2019 at 1:12 AM Eli Britstein <elibr@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@mellanox.com> > Reviewed-by: Roi Dayan <roid@mellanox.com> Thanks. Acked-by: Pravin B Shelar <pshelar@ovn.org>
On Sun, Feb 3, 2019 at 1:13 AM Eli Britstein <elibr@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@mellanox.com> > Reviewed-by: Roi Dayan <roid@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
On 2/3/2019 1:12 AM, Eli Britstein 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@mellanox.com> > Reviewed-by: Roi Dayan <roid@mellanox.com> Obscuring the structures with these macros is awful. I'm opposed but I see it has already been accepted upstream so I guess that's that. Ugh... - Greg > --- > 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_ETHERNET_FIELDS \ > + OVS_KEY_FIELD_ARR(__u8, eth_src, ETH_ALEN) \ > + OVS_KEY_FIELD_ARR(__u8, eth_dst, ETH_ALEN) > + > struct ovs_key_ethernet { > - __u8 eth_src[ETH_ALEN]; > - __u8 eth_dst[ETH_ALEN]; > + OVS_KEY_ETHERNET_FIELDS > }; > > struct ovs_key_mpls { > __be32 mpls_lse; > }; > > +#define OVS_KEY_IPV4_FIELDS \ > + OVS_KEY_FIELD(__be32, ipv4_src) \ > + OVS_KEY_FIELD(__be32, ipv4_dst) \ > + OVS_KEY_FIELD(__u8, ipv4_proto) \ > + OVS_KEY_FIELD(__u8, ipv4_tos) \ > + OVS_KEY_FIELD(__u8, ipv4_ttl) \ > + OVS_KEY_FIELD(__u8, ipv4_frag /* One of OVS_FRAG_TYPE_*. */) > + > struct ovs_key_ipv4 { > - __be32 ipv4_src; > - __be32 ipv4_dst; > - __u8 ipv4_proto; > - __u8 ipv4_tos; > - __u8 ipv4_ttl; > - __u8 ipv4_frag; /* One of OVS_FRAG_TYPE_*. */ > + OVS_KEY_IPV4_FIELDS > }; > > +#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 > }; > > +#define OVS_KEY_TCP_FIELDS \ > + OVS_KEY_FIELD(__be16, tcp_src) \ > + OVS_KEY_FIELD(__be16, tcp_dst) > + > struct ovs_key_tcp { > - __be16 tcp_src; > - __be16 tcp_dst; > + OVS_KEY_TCP_FIELDS > }; > > +#define OVS_KEY_UDP_FIELDS \ > + OVS_KEY_FIELD(__be16, udp_src) \ > + OVS_KEY_FIELD(__be16, udp_dst) > + > struct ovs_key_udp { > - __be16 udp_src; > - __be16 udp_dst; > + OVS_KEY_UDP_FIELDS > }; > > +#define OVS_KEY_SCTP_FIELDS \ > + OVS_KEY_FIELD(__be16, sctp_src) \ > + OVS_KEY_FIELD(__be16, sctp_dst) > + > struct ovs_key_sctp { > - __be16 sctp_src; > - __be16 sctp_dst; > + OVS_KEY_SCTP_FIELDS > }; > > +#define OVS_KEY_ICMP_FIELDS \ > + OVS_KEY_FIELD(__u8, icmp_type) \ > + OVS_KEY_FIELD(__u8, icmp_code) > + > struct ovs_key_icmp { > - __u8 icmp_type; > - __u8 icmp_code; > + OVS_KEY_ICMP_FIELDS > }; > > +#define OVS_KEY_ICMPV6_FIELDS \ > + OVS_KEY_FIELD(__u8, icmpv6_type) \ > + OVS_KEY_FIELD(__u8, icmpv6_code) > + > struct ovs_key_icmpv6 { > - __u8 icmpv6_type; > - __u8 icmpv6_code; > + OVS_KEY_ICMPV6_FIELDS > }; > > +#define OVS_KEY_ARP_FIELDS \ > + OVS_KEY_FIELD(__be32, arp_sip) \ > + OVS_KEY_FIELD(__be32, arp_tip) \ > + OVS_KEY_FIELD(__be16, arp_op) \ > + OVS_KEY_FIELD_ARR(__u8, arp_sha, ETH_ALEN) \ > + OVS_KEY_FIELD_ARR(__u8, arp_tha, ETH_ALEN) > + > struct ovs_key_arp { > - __be32 arp_sip; > - __be32 arp_tip; > - __be16 arp_op; > - __u8 arp_sha[ETH_ALEN]; > - __u8 arp_tha[ETH_ALEN]; > + OVS_KEY_ARP_FIELDS > }; > > +#define OVS_KEY_ND_FIELDS \ > + OVS_KEY_FIELD_ARR(__be32, nd_target, 4) \ > + OVS_KEY_FIELD_ARR(__u8, nd_sll, ETH_ALEN) \ > + OVS_KEY_FIELD_ARR(__u8, nd_tll, ETH_ALEN) > + > struct ovs_key_nd { > - __be32 nd_target[4]; > - __u8 nd_sll[ETH_ALEN]; > - __u8 nd_tll[ETH_ALEN]; > + OVS_KEY_ND_FIELDS > }; > > +#undef OVS_KEY_FIELD_ARR > +#undef OVS_KEY_FIELD > + > #define OVS_CT_LABELS_LEN_32 4 > #define OVS_CT_LABELS_LEN (OVS_CT_LABELS_LEN_32 * sizeof(__u32)) > struct ovs_key_ct_labels {
From: Yi-Hung Wei <yihung.wei@gmail.com> Date: Mon, 4 Feb 2019 10:47:18 -0800 > 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. I completely agree. Unless this is totally unavoidable, I do not want to apply a patch which makes reading and auditing the networking code more difficult.
From: Gregory Rose <gvrose8192@gmail.com> Date: Mon, 4 Feb 2019 11:41:29 -0800 > > On 2/3/2019 1:12 AM, Eli Britstein 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@mellanox.com> >> Reviewed-by: Roi Dayan <roid@mellanox.com> > > Obscuring the structures with these macros is awful. I'm opposed but > I see it has already been > accepted upstream so I guess that's that. I am personally in no way obligated to apply this patch to my tree just because "upstream" did, and I absolutely have no plans to do so at this point. This patch is absolutely awful.
On 2/4/2019 10:07 PM, David Miller wrote: > From: Yi-Hung Wei <yihung.wei@gmail.com> > Date: Mon, 4 Feb 2019 10:47:18 -0800 > >> 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. > I completely agree. > > Unless this is totally unavoidable, I do not want to apply a patch > which makes reading and auditing the networking code more difficult. This technique is discussed for example in https://stackoverflow.com/questions/6635851/real-world-use-of-x-macros, and I found existing examples of using it in the kernel tree: __BPF_FUNC_MAPPER in commit ebb676daa1a34 ("bpf: Print function name in addition to function id") __AAL_STAT_ITEMS and __SONET_ITEMS in commit 607ca46e97a1b ("UAPI: (Scripted) Disintegrate include/linux"), the successor of commit 1da177e4c3f4 ("Linux-2.6.12-rc2"). I didn't dig deeper to the past. I can agree it makes that H file a bit more complicated, but for sure less than ## macros that are widely used. However, I think the alternatives of generating such defines by some scripts, or having the fields in more than one place are even worse, so it is a kind of unavoidable. Please reconsider regarding applying this patch.
On 2/5/2019 4:02 AM, Eli Britstein wrote: > On 2/4/2019 10:07 PM, David Miller wrote: >> From: Yi-Hung Wei <yihung.wei@gmail.com> >> Date: Mon, 4 Feb 2019 10:47:18 -0800 >> >>> 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. >> I completely agree. >> >> Unless this is totally unavoidable, I do not want to apply a patch >> which makes reading and auditing the networking code more difficult. > This technique is discussed for example in > https://stackoverflow.com/questions/6635851/real-world-use-of-x-macros, > and I found existing examples of using it in the kernel tree: > > __BPF_FUNC_MAPPER in commit ebb676daa1a34 ("bpf: Print function name in > addition to function id") > > __AAL_STAT_ITEMS and __SONET_ITEMS in commit 607ca46e97a1b ("UAPI: > (Scripted) Disintegrate include/linux"), the successor of commit > 1da177e4c3f4 ("Linux-2.6.12-rc2"). I didn't dig deeper to the past. > > I can agree it makes that H file a bit more complicated, but for sure > less than ## macros that are widely used. > > However, I think the alternatives of generating such defines by some > scripts, or having the fields in more than one place are even worse, so > it is a kind of unavoidable. Why is using a script to generate defines for the requirements of your application or driver so bad? Your patch turns openvswitch.h into some hardly readable code - using a script and having a machine output the macros your application or driver needs seems like a much better alternative to me. - Greg > > Please reconsider regarding applying this patch. > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
On Tue, Feb 05, 2019 at 10:22:09AM -0800, Gregory Rose wrote: > > On 2/5/2019 4:02 AM, Eli Britstein wrote: > > On 2/4/2019 10:07 PM, David Miller wrote: > > > From: Yi-Hung Wei <yihung.wei@gmail.com> > > > Date: Mon, 4 Feb 2019 10:47:18 -0800 > > > > > > > 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. > > > I completely agree. > > > > > > Unless this is totally unavoidable, I do not want to apply a patch > > > which makes reading and auditing the networking code more difficult. > > This technique is discussed for example in > > https://stackoverflow.com/questions/6635851/real-world-use-of-x-macros, > > and I found existing examples of using it in the kernel tree: > > > > __BPF_FUNC_MAPPER in commit ebb676daa1a34 ("bpf: Print function name in > > addition to function id") > > > > __AAL_STAT_ITEMS and __SONET_ITEMS in commit 607ca46e97a1b ("UAPI: > > (Scripted) Disintegrate include/linux"), the successor of commit > > 1da177e4c3f4 ("Linux-2.6.12-rc2"). I didn't dig deeper to the past. > > > > I can agree it makes that H file a bit more complicated, but for sure > > less than ## macros that are widely used. > > > > However, I think the alternatives of generating such defines by some > > scripts, or having the fields in more than one place are even worse, so > > it is a kind of unavoidable. > > Why is using a script to generate defines for the requirements of your > application or driver so bad? Your patch > turns openvswitch.h into some hardly readable code - using a script and > having a machine output the macros > your application or driver needs seems like a much better alternative to me. In addition, one of the reasons that developers prefer to avoid duplication is because of the need to synchronize copies when one of them changes. This doesn't apply in the same way to these structures, because they are ABIs that will not change.
On 2/5/2019 10:23 PM, Ben Pfaff wrote: > On Tue, Feb 05, 2019 at 10:22:09AM -0800, Gregory Rose wrote: >> On 2/5/2019 4:02 AM, Eli Britstein wrote: >>> On 2/4/2019 10:07 PM, David Miller wrote: >>>> From: Yi-Hung Wei <yihung.wei@gmail.com> >>>> Date: Mon, 4 Feb 2019 10:47:18 -0800 >>>> >>>>> 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. >>>> I completely agree. >>>> >>>> Unless this is totally unavoidable, I do not want to apply a patch >>>> which makes reading and auditing the networking code more difficult. >>> This technique is discussed for example in >>> https://stackoverflow.com/questions/6635851/real-world-use-of-x-macros, >>> and I found existing examples of using it in the kernel tree: >>> >>> __BPF_FUNC_MAPPER in commit ebb676daa1a34 ("bpf: Print function name in >>> addition to function id") >>> >>> __AAL_STAT_ITEMS and __SONET_ITEMS in commit 607ca46e97a1b ("UAPI: >>> (Scripted) Disintegrate include/linux"), the successor of commit >>> 1da177e4c3f4 ("Linux-2.6.12-rc2"). I didn't dig deeper to the past. >>> >>> I can agree it makes that H file a bit more complicated, but for sure >>> less than ## macros that are widely used. >>> >>> However, I think the alternatives of generating such defines by some >>> scripts, or having the fields in more than one place are even worse, so >>> it is a kind of unavoidable. >> Why is using a script to generate defines for the requirements of your >> application or driver so bad? Your patch >> turns openvswitch.h into some hardly readable code - using a script and >> having a machine output the macros >> your application or driver needs seems like a much better alternative to me. OK, let's abandon this patch. I'll go with the script alternative. > In addition, one of the reasons that developers prefer to avoid > duplication is because of the need to synchronize copies when one of > them changes. This doesn't apply in the same way to these structures, > because they are ABIs that will not change. This is correct, but still, though it is not likely to change, it might will, so I think we must avoid duplication.
On Mon, Feb 04, 2019 at 12:09:00PM -0800, David Miller wrote: > From: Gregory Rose <gvrose8192@gmail.com> > Date: Mon, 4 Feb 2019 11:41:29 -0800 > > > > > On 2/3/2019 1:12 AM, Eli Britstein 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@mellanox.com> > >> Reviewed-by: Roi Dayan <roid@mellanox.com> > > > > Obscuring the structures with these macros is awful. I'm opposed but > > I see it has already been > > accepted upstream so I guess that's that. > > I am personally in no way obligated to apply this patch to my tree > just because "upstream" did, and I absolutely have no plans to do so > at this point. > > This patch is absolutely awful. I hate to jump on a bandwagon, but this patch makes the code much less readable.
On 2/8/2019 9:59 AM, Simon Horman wrote: > On Mon, Feb 04, 2019 at 12:09:00PM -0800, David Miller wrote: >> From: Gregory Rose <gvrose8192@gmail.com> >> Date: Mon, 4 Feb 2019 11:41:29 -0800 >> >>> On 2/3/2019 1:12 AM, Eli Britstein 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@mellanox.com> >>>> Reviewed-by: Roi Dayan <roid@mellanox.com> >>> Obscuring the structures with these macros is awful. I'm opposed but >>> I see it has already been >>> accepted upstream so I guess that's that. >> I am personally in no way obligated to apply this patch to my tree >> just because "upstream" did, and I absolutely have no plans to do so >> at this point. >> >> This patch is absolutely awful. > I hate to jump on a bandwagon, but this patch makes the code much > less readable. Please review the alternative I have posted: https://mail.openvswitch.org/pipermail/ovs-dev/2019-February/356000.html
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_ETHERNET_FIELDS \ + OVS_KEY_FIELD_ARR(__u8, eth_src, ETH_ALEN) \ + OVS_KEY_FIELD_ARR(__u8, eth_dst, ETH_ALEN) + struct ovs_key_ethernet { - __u8 eth_src[ETH_ALEN]; - __u8 eth_dst[ETH_ALEN]; + OVS_KEY_ETHERNET_FIELDS }; struct ovs_key_mpls { __be32 mpls_lse; }; +#define OVS_KEY_IPV4_FIELDS \ + OVS_KEY_FIELD(__be32, ipv4_src) \ + OVS_KEY_FIELD(__be32, ipv4_dst) \ + OVS_KEY_FIELD(__u8, ipv4_proto) \ + OVS_KEY_FIELD(__u8, ipv4_tos) \ + OVS_KEY_FIELD(__u8, ipv4_ttl) \ + OVS_KEY_FIELD(__u8, ipv4_frag /* One of OVS_FRAG_TYPE_*. */) + struct ovs_key_ipv4 { - __be32 ipv4_src; - __be32 ipv4_dst; - __u8 ipv4_proto; - __u8 ipv4_tos; - __u8 ipv4_ttl; - __u8 ipv4_frag; /* One of OVS_FRAG_TYPE_*. */ + OVS_KEY_IPV4_FIELDS }; +#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 }; +#define OVS_KEY_TCP_FIELDS \ + OVS_KEY_FIELD(__be16, tcp_src) \ + OVS_KEY_FIELD(__be16, tcp_dst) + struct ovs_key_tcp { - __be16 tcp_src; - __be16 tcp_dst; + OVS_KEY_TCP_FIELDS }; +#define OVS_KEY_UDP_FIELDS \ + OVS_KEY_FIELD(__be16, udp_src) \ + OVS_KEY_FIELD(__be16, udp_dst) + struct ovs_key_udp { - __be16 udp_src; - __be16 udp_dst; + OVS_KEY_UDP_FIELDS }; +#define OVS_KEY_SCTP_FIELDS \ + OVS_KEY_FIELD(__be16, sctp_src) \ + OVS_KEY_FIELD(__be16, sctp_dst) + struct ovs_key_sctp { - __be16 sctp_src; - __be16 sctp_dst; + OVS_KEY_SCTP_FIELDS }; +#define OVS_KEY_ICMP_FIELDS \ + OVS_KEY_FIELD(__u8, icmp_type) \ + OVS_KEY_FIELD(__u8, icmp_code) + struct ovs_key_icmp { - __u8 icmp_type; - __u8 icmp_code; + OVS_KEY_ICMP_FIELDS }; +#define OVS_KEY_ICMPV6_FIELDS \ + OVS_KEY_FIELD(__u8, icmpv6_type) \ + OVS_KEY_FIELD(__u8, icmpv6_code) + struct ovs_key_icmpv6 { - __u8 icmpv6_type; - __u8 icmpv6_code; + OVS_KEY_ICMPV6_FIELDS }; +#define OVS_KEY_ARP_FIELDS \ + OVS_KEY_FIELD(__be32, arp_sip) \ + OVS_KEY_FIELD(__be32, arp_tip) \ + OVS_KEY_FIELD(__be16, arp_op) \ + OVS_KEY_FIELD_ARR(__u8, arp_sha, ETH_ALEN) \ + OVS_KEY_FIELD_ARR(__u8, arp_tha, ETH_ALEN) + struct ovs_key_arp { - __be32 arp_sip; - __be32 arp_tip; - __be16 arp_op; - __u8 arp_sha[ETH_ALEN]; - __u8 arp_tha[ETH_ALEN]; + OVS_KEY_ARP_FIELDS }; +#define OVS_KEY_ND_FIELDS \ + OVS_KEY_FIELD_ARR(__be32, nd_target, 4) \ + OVS_KEY_FIELD_ARR(__u8, nd_sll, ETH_ALEN) \ + OVS_KEY_FIELD_ARR(__u8, nd_tll, ETH_ALEN) + struct ovs_key_nd { - __be32 nd_target[4]; - __u8 nd_sll[ETH_ALEN]; - __u8 nd_tll[ETH_ALEN]; + OVS_KEY_ND_FIELDS }; +#undef OVS_KEY_FIELD_ARR +#undef OVS_KEY_FIELD + #define OVS_CT_LABELS_LEN_32 4 #define OVS_CT_LABELS_LEN (OVS_CT_LABELS_LEN_32 * sizeof(__u32)) struct ovs_key_ct_labels {
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@mellanox.com> Reviewed-by: Roi Dayan <roid@mellanox.com> --- include/uapi/linux/openvswitch.h | 102 ++++++++++++++++++++++++++------------- 1 file changed, 69 insertions(+), 33 deletions(-)