On Tue, 8 Mar 2022 15:12:45 +0100 Ilya Maximets wrote:
> >> diff --git a/include/uapi/linux/openvswitch.h
> >> b/include/uapi/linux/openvswitch.h
> >> index 9d1710f20505..ab6755621e02 100644
> >> --- a/include/uapi/linux/openvswitch.h
> >> +++ b/include/uapi/linux/openvswitch.h
> >> @@ -351,11 +351,16 @@ enum ovs_key_attr {
> >> OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV4, /* struct ovs_key_ct_tuple_ipv4
> >> */
> >> OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6, /* struct ovs_key_ct_tuple_ipv6
> >> */
> >> OVS_KEY_ATTR_NSH, /* Nested set of ovs_nsh_key_* */
> >> - OVS_KEY_ATTR_IPV6_EXTHDRS, /* struct ovs_key_ipv6_exthdr */
> >>
> >> #ifdef __KERNEL__
> >> OVS_KEY_ATTR_TUNNEL_INFO, /* struct ip_tunnel_info */
> >> #endif
> >> + /* User space decided to squat on types 30 and 31 */
> >> + OVS_KEY_ATTR_IPV6_EXTHDRS = 32, /* struct ovs_key_ipv6_exthdr */
> >> + /* WARNING: <scary warning to avoid the problem coming back> */
>
> Yes, that is something that I had in mind too. The only thing that makes
> me uncomfortable is OVS_KEY_ATTR_TUNNEL_INFO = 30 here. Even though it
> doesn't make a lot of difference, I'd better keep the kernel-only attributes
> at the end of the enumeration. Is there a better way to handle kernel-only
> attribute?
My thought was to leave the kernel/userspace only types "behind" to
avoid perpetuating the same constructs.
Johannes's point about userspace to userspace messages makes the whole
thing a little less of an aberration.
Is there a reason for the types to be hidden under __KERNEL__?
Or someone did that in a misguided attempt to save space in attr arrays
when parsing?
> Also, the OVS_KEY_ATTR_ND_EXTENSIONS (31) attribute used to store IPv6
> Neighbor
> Discovery extensions is currently implemented only for userspace, but nothing
> actually prevents us having the kernel implementation. So, we need a way to
> make it usable by the kernel in the future.
The "= 32" leaves the earlier attr types as reserved so nothing
prevents us from defining them later. But..
> > It might be nicer to actually document here in what's at least supposed
> > to be the canonical documentation of the API what those types were used
> > for.
>
> I agree with that.
Should we add the user space types to the kernel header and remove the
ifdef __KERNEL__ around TUNNEL_INFO, then?
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
