summaryrefslogtreecommitdiff
path: root/datapath
diff options
context:
space:
mode:
authorIlya Maximets <i.maximets@ovn.org>2022-03-25 14:48:23 +0100
committerIlya Maximets <i.maximets@ovn.org>2022-03-25 20:32:12 +0100
commitd96d14b14733c557d11768ebfe3f92d858b0a79f (patch)
tree301d9ce22727dc9369fe753e3a5e7a50c01af4ae /datapath
parent9d86459516826aae25cff1e99108ed53a9b92d23 (diff)
downloadopenvswitch-d96d14b14733c557d11768ebfe3f92d858b0a79f.tar.gz
openvswitch.h: Align uAPI definition with the kernel.
Upstream commit: commit 1926407a4ab0e59d5a27bed7b82029b356d80fa0 Author: Ilya Maximets <i.maximets@ovn.org> Date: Wed Mar 9 23:20:33 2022 +0100 net: openvswitch: fix uAPI incompatibility with existing user space Few years ago OVS user space made a strange choice in the commit [1] to define types only valid for the user space inside the copy of a kernel uAPI header. '#ifndef __KERNEL__' and another attribute was added later. This leads to the inevitable clash between user space and kernel types when the kernel uAPI is extended. The issue was unveiled with the addition of a new type for IPv6 extension header in kernel uAPI. When kernel provides the OVS_KEY_ATTR_IPV6_EXTHDRS attribute to the older user space application, application tries to parse it as OVS_KEY_ATTR_PACKET_TYPE and discards the whole netlink message as malformed. Since OVS_KEY_ATTR_IPV6_EXTHDRS is supplied along with every IPv6 packet that goes to the user space, IPv6 support is fully broken. Fixing that by bringing these user space attributes to the kernel uAPI to avoid the clash. Strictly speaking this is not the problem of the kernel uAPI, but changing it is the only way to avoid breakage of the older user space applications at this point. These 2 types are explicitly rejected now since they should not be passed to the kernel. Additionally, OVS_KEY_ATTR_TUNNEL_INFO moved out from the '#ifdef __KERNEL__' as there is no good reason to hide it from the userspace. And it's also explicitly rejected now, because it's for in-kernel use only. Comments with warnings were added to avoid the problem coming back. (1 << type) converted to (1ULL << type) to avoid integer overflow on OVS_KEY_ATTR_IPV6_EXTHDRS, since it equals 32 now. [1] beb75a40fdc2 ("userspace: Switching of L3 packets in L2 pipeline") Fixes: 28a3f0601727 ("net: openvswitch: IPv6: Add IPv6 extension header support") Link: https://lore.kernel.org/netdev/3adf00c7-fe65-3ef4-b6d7-6d8a0cad8a5f@nvidia.com Link: https://github.com/openvswitch/ovs/commit/beb75a40fdc295bfd6521b0068b4cd12f6de507c Reported-by: Roi Dayan <roid@nvidia.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org> Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> Acked-by: Aaron Conole <aconole@redhat.com> Link: https://lore.kernel.org/r/20220309222033.3018976-1-i.maximets@ovn.org Signed-off-by: Jakub Kicinski <kuba@kernel.org> Not adding OVS_KEY_ATTR_IPV6_EXTHDRS in this commit as this is not necessary. Will be added along with the actual userspace implementation. This change should help avoiding incompatibility issues in the future. Acked-by: Aaron Conole <aconole@redhat.com> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
Diffstat (limited to 'datapath')
-rw-r--r--datapath/flow_netlink.c9
-rw-r--r--datapath/linux/compat/include/linux/openvswitch.h20
2 files changed, 19 insertions, 10 deletions
diff --git a/datapath/flow_netlink.c b/datapath/flow_netlink.c
index 996041602..caed44386 100644
--- a/datapath/flow_netlink.c
+++ b/datapath/flow_netlink.c
@@ -359,7 +359,7 @@ 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 != 29);
+ BUILD_BUG_ON(OVS_KEY_ATTR_MAX != 31);
return nla_total_size(4) /* OVS_KEY_ATTR_PRIORITY */
+ nla_total_size(0) /* OVS_KEY_ATTR_TUNNEL */
@@ -491,6 +491,13 @@ static int __parse_flow_nlattrs(const struct nlattr *attr,
return -EINVAL;
}
+ if (type == OVS_KEY_ATTR_PACKET_TYPE ||
+ type == OVS_KEY_ATTR_ND_EXTENSIONS ||
+ type == OVS_KEY_ATTR_TUNNEL_INFO) {
+ OVS_NLERR(log, "Key type %d is not supported", type);
+ return -EINVAL;
+ }
+
if (attrs & (1ULL << type)) {
OVS_NLERR(log, "Duplicate key (type %d).", type);
return -EINVAL;
diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h
index 8d9300091..8bb5abdc8 100644
--- a/datapath/linux/compat/include/linux/openvswitch.h
+++ b/datapath/linux/compat/include/linux/openvswitch.h
@@ -388,17 +388,19 @@ enum ovs_key_attr {
OVS_KEY_ATTR_CT_ORIG_TUPLE_IPV6, /* struct ovs_key_ct_tuple_ipv6 */
OVS_KEY_ATTR_NSH, /* Nested set of ovs_nsh_key_* */
-#ifdef __KERNEL__
- /* Only used within kernel data path. */
- OVS_KEY_ATTR_TUNNEL_INFO, /* struct ovs_tunnel_info */
-#endif
-
-#ifndef __KERNEL__
- /* Only used within userspace data path. */
- OVS_KEY_ATTR_PACKET_TYPE, /* be32 packet type */
+ /* User space decided to squat on types 29 and 30. They are defined
+ * below, but should not be sent to the kernel.
+ *
+ * WARNING: No new types should be added unless they are defined
+ * for both kernel and user space (no 'ifdef's). It's hard
+ * to keep compatibility otherwise.
+ */
+ OVS_KEY_ATTR_PACKET_TYPE, /* be32 packet type */
OVS_KEY_ATTR_ND_EXTENSIONS, /* struct ovs_key_nd_extensions */
-#endif
+ OVS_KEY_ATTR_TUNNEL_INFO, /* struct ip_tunnel_info.
+ * For in-kernel use only.
+ */
__OVS_KEY_ATTR_MAX
};