This functionality was deprecated in OVS 3.7, so we should remove it. The legacy kernel vport types were not used for many years with the upstream Linux kernel since introduction of COLLECT_METADATA support for standard kernel tunnel devices in Linux 4.3. These days we're just creating those standard kernel tunnel devices with RTM_NEWLINK and then adding them into OVS datapath as OVS_VPORT_TYPE_NETDEV.
It is also planned to fully remove OVS_VPORT_TYPE_GRE/VXLAN/GENEVE from the upstream kernel as well. The function naming was also confusing as compat() function was handling all the non-tunnel devices, even though there is nothing 'compat' about them. And 'rtnl' functions are only handling tunnels creating confusion for the readers of the dpif_netlink_port_add(). Now renamed appropriately. Attempts for module loading are removed from tests and dev scripts. The documentation is mentioning only the removal of the legacy tunnel configuration support and not the general "support for using OOT kernel module", since technically it still can be used, only the tunnels will not work in cases where the rest of the kernel doesn't support them. The legacy tunnel types were the only ones that supported options, so parsing and formatting OVS_VPORT_ATTR_OPTIONS is also removed. Signed-off-by: Ilya Maximets <[email protected]> --- Documentation/faq/releases.rst | 18 ++- NEWS | 6 + lib/dpif-netlink-rtnl.c | 112 +--------------- lib/dpif-netlink-rtnl.h | 18 +-- lib/dpif-netlink.c | 148 ++++++---------------- lib/dpif-netlink.h | 5 +- tests/system-kmod-macros.at | 7 - tests/system-offloads-testsuite-macros.at | 4 - utilities/docker/ovs-override.conf | 2 - utilities/docker/start-ovs | 1 - 10 files changed, 66 insertions(+), 255 deletions(-) diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst index 42d2c8e2a..e08c61138 100644 --- a/Documentation/faq/releases.rst +++ b/Documentation/faq/releases.rst @@ -91,9 +91,9 @@ Q: Are all features available with all datapaths? Conntrack NAT 4.6 2.8 Conntrack NAT6 4.6 2.8 Conntrack Helper Persist. YES 3.3 - Tunnel - GRE 3.11 2.4 - Tunnel - VXLAN 3.12 2.4 - Tunnel - Geneve 3.18 2.4 + Tunnel - GRE 4.3 2.4 + Tunnel - VXLAN 4.3 2.4 + Tunnel - Geneve 4.3 2.4 Tunnel - GRE-IPv6 4.18 2.6 Tunnel - VXLAN-IPv6 4.3 2.6 Tunnel - Geneve-IPv6 4.4 2.6 @@ -205,12 +205,18 @@ Q: Why do tunnels not work when using a Linux kernel module? ======== ============ Protocol Linux Kernel ======== ============ - GRE 3.11 - VXLAN 3.12 - Geneve 3.18 + GRE 4.3 + VXLAN 4.3 + Geneve 4.3 ERSPAN 4.18 ======== ============ + Open vSwitch 3.7 and earlier supported GRE, VXLAN and Geneve on kernels + older than specified above by utilizing the legacy OVS-specific kernel + vport types. Newer versions of Open vSwitch no longer support that method + and fully rely on standard kernel tunnel devices with ``COLLECT_METADATA`` + support. This makes Linux 4.3 a minimal requirement for these tunnels. + Q: Why are UDP tunnel checksums not computed for VXLAN or Geneve? A: Generating outer UDP checksums requires kernel support that was not part diff --git a/NEWS b/NEWS index 779447c74..b37a2006c 100644 --- a/NEWS +++ b/NEWS @@ -19,6 +19,12 @@ Post-v3.7.0 - Python: * The ovstest Python module is now removed. It was only used by the ovs-test and ovs-l3ping utilities. + - Removed previously deprecated support for running Open vSwitch with the + legacy tunnel vport implementation (e.g., from the old OOT kernel module). + Tunnel ports in the kernel datapath now require support for standard + kernel tunnel devices with COLLECT_METADATA. And so the minimum kernel + version for GRE, VXLAN, and Geneve tunnels is raised from 3.11/3.12/3.18 + to 4.3. - The following deprecated AVX512-specific features of the userspace datapath are now removed: * AVX512-optimized DPCLS lookups. diff --git a/lib/dpif-netlink-rtnl.c b/lib/dpif-netlink-rtnl.c index 31ca35aae..4ac53a62a 100644 --- a/lib/dpif-netlink-rtnl.c +++ b/lib/dpif-netlink-rtnl.c @@ -445,7 +445,7 @@ exit: } int -dpif_netlink_rtnl_port_create(struct netdev *netdev) +dpif_netlink_rtnl_tunnel_create(struct netdev *netdev) { const struct netdev_tunnel_config *tnl_cfg; char namebuf[NETDEV_VPORT_NAME_BUFSIZE]; @@ -507,7 +507,7 @@ dpif_netlink_rtnl_port_create(struct netdev *netdev) } int -dpif_netlink_rtnl_port_destroy(const char *name, const char *type) +dpif_netlink_rtnl_tunnel_destroy(const char *name, const char *type) { switch (netdev_to_ovs_vport_type(type)) { case OVS_VPORT_TYPE_VXLAN: @@ -529,111 +529,3 @@ dpif_netlink_rtnl_port_destroy(const char *name, const char *type) } return 0; } - -/** - * Probe for whether the modules are out-of-tree (openvswitch) or in-tree - * (upstream kernel). - * - * We probe for "ovs_geneve" via rtnetlink. As long as this returns something - * other than EOPNOTSUPP we know that the module in use is the out-of-tree one. - * This will be used to determine which netlink interface to use when creating - * ports; rtnetlink or compat/genetlink. - * - * See ovs_tunnels_out_of_tree - */ -static bool -dpif_netlink_rtnl_probe_oot_tunnels__(void) -{ - char namebuf[NETDEV_VPORT_NAME_BUFSIZE]; - struct netdev *netdev = NULL; - bool out_of_tree = false; - const char *name; - int error; - - error = netdev_open("ovs-system-probe", "geneve", &netdev); - if (!error) { - struct ofpbuf *reply; - const struct netdev_tunnel_config *tnl_cfg; - - tnl_cfg = netdev_get_tunnel_config(netdev); - if (!tnl_cfg) { - netdev_close(netdev); - return true; - } - - name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf); - - /* The geneve module exists when ovs-vswitchd crashes - * and restarts, handle the case here. - */ - error = dpif_netlink_rtnl_getlink(name, &reply); - if (!error) { - - struct nlattr *linkinfo[ARRAY_SIZE(linkinfo_policy)]; - struct nlattr *rtlink[ARRAY_SIZE(rtlink_policy)]; - const char *kind; - - if (!nl_policy_parse(reply, - NLMSG_HDRLEN + sizeof(struct ifinfomsg), - rtlink_policy, rtlink, - ARRAY_SIZE(rtlink_policy)) - || !nl_parse_nested(rtlink[IFLA_LINKINFO], linkinfo_policy, - linkinfo, ARRAY_SIZE(linkinfo_policy))) { - VLOG_ABORT("Error fetching Geneve tunnel device %s " - "linkinfo", name); - } - - kind = nl_attr_get_string(linkinfo[IFLA_INFO_KIND]); - - if (!strcmp(kind, "ovs_geneve")) { - out_of_tree = true; - } else if (!strcmp(kind, "geneve")) { - out_of_tree = false; - } else { - VLOG_ABORT("Geneve tunnel device %s with kind %s" - " not supported", name, kind); - } - - ofpbuf_delete(reply); - netdev_close(netdev); - - return out_of_tree; - } - - error = dpif_netlink_rtnl_create(tnl_cfg, name, OVS_VPORT_TYPE_GENEVE, - "ovs_geneve", - (NLM_F_REQUEST | NLM_F_ACK - | NLM_F_CREATE)); - /* EOPNOTSUPP indicates that OOT tunnel support is not present - * EPERM indicates insufficient permissions to add a tunnel. - * This may occur when OVS is run by an unprivileged user, - * e.g. when running make check. - * As this case doesn't use kernel tunnels, assume that they - * are not present for the sake of logic that warns if they are - * used. - */ - if (error != EOPNOTSUPP && error != EPERM) { - if (!error) { - dpif_netlink_rtnl_destroy(name); - } - out_of_tree = true; - } - netdev_close(netdev); - } - - return out_of_tree; -} - -bool -dpif_netlink_rtnl_probe_oot_tunnels(void) -{ - bool out_of_tree = dpif_netlink_rtnl_probe_oot_tunnels__(); - - if (out_of_tree) { - VLOG_WARN_ONCE("Use of the OOT Kernel datapath module is deprecated. " - "Please use the module provided by the upstream " - "Kernel instead."); - } - - return out_of_tree; -} diff --git a/lib/dpif-netlink-rtnl.h b/lib/dpif-netlink-rtnl.h index 5c790e0bc..e8e577de8 100644 --- a/lib/dpif-netlink-rtnl.h +++ b/lib/dpif-netlink-rtnl.h @@ -22,33 +22,25 @@ #include "netdev.h" /* Declare these to keep sparse happy. */ -int dpif_netlink_rtnl_port_create(struct netdev *netdev); -int dpif_netlink_rtnl_port_destroy(const char *name, const char *type); - -bool dpif_netlink_rtnl_probe_oot_tunnels(void); +int dpif_netlink_rtnl_tunnel_create(struct netdev *netdev); +int dpif_netlink_rtnl_tunnel_destroy(const char *name, const char *type); #ifndef __linux__ /* Dummy implementations for non Linux builds. */ static inline int -dpif_netlink_rtnl_port_create(struct netdev *netdev OVS_UNUSED) +dpif_netlink_rtnl_tunnel_create(struct netdev *netdev OVS_UNUSED) { return EOPNOTSUPP; } static inline int -dpif_netlink_rtnl_port_destroy(const char *name OVS_UNUSED, - const char *type OVS_UNUSED) +dpif_netlink_rtnl_tunnel_destroy(const char *name OVS_UNUSED, + const char *type OVS_UNUSED) { return EOPNOTSUPP; } -static inline bool -dpif_netlink_rtnl_probe_oot_tunnels(void) -{ - return true; -} - #endif #endif /* DPIF_NETLINK_RTNL_H */ diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c index 1caac4f6d..18677fba5 100644 --- a/lib/dpif-netlink.c +++ b/lib/dpif-netlink.c @@ -226,12 +226,6 @@ static int ovs_ct_limit_family; * Initialized by dpif_netlink_init(). */ static unsigned int ovs_vport_mcgroup; -/* If true, tunnel devices are created using OVS compat/genetlink. - * If false, tunnel devices are created with rtnetlink and using light weight - * tunnels. If we fail to create the tunnel the rtnetlink+LWT, then we fallback - * to using the compat interface. */ -static bool ovs_tunnels_out_of_tree = true; - static int dpif_netlink_init(void); static int open_dpif(const struct dpif_netlink_dp *, struct dpif **); static uint32_t dpif_netlink_port_get_pid(const struct dpif *, @@ -860,9 +854,7 @@ netdev_to_ovs_vport_type(const char *type) static int dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, - enum ovs_vport_type type, - struct ofpbuf *options, - odp_port_t *port_nop) + enum ovs_vport_type type, odp_port_t *port_nop) OVS_REQ_WRLOCK(dpif->upcall_lock) { struct dpif_netlink_vport request, reply; @@ -894,11 +886,6 @@ dpif_netlink_port_add__(struct dpif_netlink *dpif, const char *name, request.n_upcall_pids = 1; request.upcall_pids = &upcall_pids; - if (options) { - request.options = options->data; - request.options_len = options->size; - } - error = dpif_netlink_vport_transact(&request, &reply, &buf); if (!error) { *port_nop = reply.port_no; @@ -936,104 +923,56 @@ exit: } static int -dpif_netlink_port_add_compat(struct dpif_netlink *dpif, struct netdev *netdev, - odp_port_t *port_nop) - OVS_REQ_WRLOCK(dpif->upcall_lock) +dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev, + odp_port_t *port_nop) { - const struct netdev_tunnel_config *tnl_cfg; - char namebuf[NETDEV_VPORT_NAME_BUFSIZE]; + struct dpif_netlink *dpif = dpif_netlink_cast(dpif_); const char *type = netdev_get_type(netdev); - uint64_t options_stub[64 / 8]; + char namebuf[NETDEV_VPORT_NAME_BUFSIZE]; enum ovs_vport_type ovs_type; - struct ofpbuf options; const char *name; + bool is_tunnel; + int error; - name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf); - - ovs_type = netdev_to_ovs_vport_type(netdev_get_type(netdev)); - if (ovs_type == OVS_VPORT_TYPE_UNSPEC) { - VLOG_WARN_RL(&error_rl, "%s: cannot create port `%s' because it has " - "unsupported type `%s'", - dpif_name(&dpif->dpif), name, type); - return EINVAL; - } + fat_rwlock_wrlock(&dpif->upcall_lock); - if (ovs_type == OVS_VPORT_TYPE_NETDEV) { - netdev_linux_ethtool_set_flag(netdev, ETH_FLAG_LRO, "LRO", false); - } + name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf); + is_tunnel = netdev_get_tunnel_config(netdev) != NULL; - tnl_cfg = netdev_get_tunnel_config(netdev); - if (tnl_cfg && (tnl_cfg->dst_port != 0 || tnl_cfg->exts)) { - ofpbuf_use_stack(&options, options_stub, sizeof options_stub); - if (tnl_cfg->dst_port) { - nl_msg_put_u16(&options, OVS_TUNNEL_ATTR_DST_PORT, - ntohs(tnl_cfg->dst_port)); - } - if (tnl_cfg->exts) { - size_t ext_ofs; - int i; - - ext_ofs = nl_msg_start_nested(&options, OVS_TUNNEL_ATTR_EXTENSION); - for (i = 0; i < 32; i++) { - if (tnl_cfg->exts & (UINT32_C(1) << i)) { - nl_msg_put_flag(&options, i); - } + if (is_tunnel) { + /* Tunnel port: create the tunnel device via rtnetlink then add it + * as OVS_VPORT_TYPE_NETDEV. */ + error = dpif_netlink_rtnl_tunnel_create(netdev); + if (error) { + if (error != EOPNOTSUPP) { + VLOG_WARN_RL(&error_rl, + "Failed to create %s with rtnetlink: %s", + netdev_get_name(netdev), ovs_strerror(error)); } - nl_msg_end_nested(&options, ext_ofs); + goto out; } - return dpif_netlink_port_add__(dpif, name, ovs_type, &options, - port_nop); + ovs_type = OVS_VPORT_TYPE_NETDEV; } else { - return dpif_netlink_port_add__(dpif, name, ovs_type, NULL, port_nop); - } - -} - -static int -dpif_netlink_rtnl_port_create_and_add(struct dpif_netlink *dpif, - struct netdev *netdev, - odp_port_t *port_nop) - OVS_REQ_WRLOCK(dpif->upcall_lock) -{ - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 20); - char namebuf[NETDEV_VPORT_NAME_BUFSIZE]; - const char *name; - int error; - - error = dpif_netlink_rtnl_port_create(netdev); - if (error) { - if (error != EOPNOTSUPP) { - VLOG_WARN_RL(&rl, "Failed to create %s with rtnetlink: %s", - netdev_get_name(netdev), ovs_strerror(error)); + ovs_type = netdev_to_ovs_vport_type(type); + if (ovs_type == OVS_VPORT_TYPE_UNSPEC) { + VLOG_WARN_RL(&error_rl, "%s: cannot create port '%s' " + "because it has unsupported type '%s'", + dpif_name(&dpif->dpif), name, type); + error = EINVAL; + goto out; + } + if (ovs_type == OVS_VPORT_TYPE_NETDEV) { + netdev_linux_ethtool_set_flag(netdev, ETH_FLAG_LRO, "LRO", false); } - return error; } - name = netdev_vport_get_dpif_port(netdev, namebuf, sizeof namebuf); - error = dpif_netlink_port_add__(dpif, name, OVS_VPORT_TYPE_NETDEV, NULL, - port_nop); - if (error) { - dpif_netlink_rtnl_port_destroy(name, netdev_get_type(netdev)); + error = dpif_netlink_port_add__(dpif, name, ovs_type, port_nop); + if (error && is_tunnel) { + dpif_netlink_rtnl_tunnel_destroy(name, type); } - return error; -} -static int -dpif_netlink_port_add(struct dpif *dpif_, struct netdev *netdev, - odp_port_t *port_nop) -{ - struct dpif_netlink *dpif = dpif_netlink_cast(dpif_); - int error = EOPNOTSUPP; - - fat_rwlock_wrlock(&dpif->upcall_lock); - if (!ovs_tunnels_out_of_tree) { - error = dpif_netlink_rtnl_port_create_and_add(dpif, netdev, port_nop); - } - if (error) { - error = dpif_netlink_port_add_compat(dpif, netdev, port_nop); - } +out: fat_rwlock_unlock(&dpif->upcall_lock); - return error; } @@ -1059,8 +998,9 @@ dpif_netlink_port_del__(struct dpif_netlink *dpif, odp_port_t port_no) vport_del_channels(dpif, port_no); - if (!error && !ovs_tunnels_out_of_tree) { - error = dpif_netlink_rtnl_port_destroy(dpif_port.name, dpif_port.type); + if (!error) { + error = dpif_netlink_rtnl_tunnel_destroy(dpif_port.name, + dpif_port.type); if (error == EOPNOTSUPP) { error = 0; } @@ -3861,8 +3801,6 @@ dpif_netlink_init(void) "the conntrack limit feature.", OVS_CT_LIMIT_FAMILY); } - ovs_tunnels_out_of_tree = dpif_netlink_rtnl_probe_oot_tunnels(); - unixctl_command_register("dpif-netlink/dispatch-mode", "", 0, 0, dpif_netlink_unixctl_dispatch_mode, NULL); @@ -3907,7 +3845,6 @@ dpif_netlink_vport_from_ofpbuf(struct dpif_netlink_vport *vport, [OVS_VPORT_ATTR_UPCALL_PID] = { .type = NL_A_UNSPEC }, [OVS_VPORT_ATTR_STATS] = { NL_POLICY_FOR(struct ovs_vport_stats), .optional = true }, - [OVS_VPORT_ATTR_OPTIONS] = { .type = NL_A_NESTED, .optional = true }, [OVS_VPORT_ATTR_NETNSID] = { .type = NL_A_U32, .optional = true }, [OVS_VPORT_ATTR_UPCALL_STATS] = { .type = NL_A_NESTED, .optional = true }, @@ -3957,10 +3894,6 @@ dpif_netlink_vport_from_ofpbuf(struct dpif_netlink_vport *vport, vport->upcall_success = UINT64_MAX; vport->upcall_fail = UINT64_MAX; } - if (a[OVS_VPORT_ATTR_OPTIONS]) { - vport->options = nl_attr_get(a[OVS_VPORT_ATTR_OPTIONS]); - vport->options_len = nl_attr_get_size(a[OVS_VPORT_ATTR_OPTIONS]); - } if (a[OVS_VPORT_ATTR_NETNSID]) { netnsid_set(&vport->netnsid, nl_attr_get_u32(a[OVS_VPORT_ATTR_NETNSID])); @@ -4006,11 +3939,6 @@ dpif_netlink_vport_to_ofpbuf(const struct dpif_netlink_vport *vport, nl_msg_put_unspec(buf, OVS_VPORT_ATTR_STATS, vport->stats, sizeof *vport->stats); } - - if (vport->options) { - nl_msg_put_nested(buf, OVS_VPORT_ATTR_OPTIONS, - vport->options, vport->options_len); - } } /* Clears 'vport' to "empty" values. */ diff --git a/lib/dpif-netlink.h b/lib/dpif-netlink.h index 4909fe160..ea283f766 100644 --- a/lib/dpif-netlink.h +++ b/lib/dpif-netlink.h @@ -39,6 +39,9 @@ struct dpif_netlink_vport { * * The 'stats' member points to 64-bit data that might only be aligned on * 32-bit boundaries, so use get_unaligned_u64() to access its values. + * + * Note: There are currently no supported vport types that support legacy + * options, so the OVS_VPORT_ATTR_OPTIONS is not represented here. */ const char *name; /* OVS_VPORT_ATTR_NAME. */ uint32_t n_upcall_pids; @@ -46,8 +49,6 @@ struct dpif_netlink_vport { const struct ovs_vport_stats *stats; /* OVS_VPORT_ATTR_STATS. */ uint64_t upcall_success; /* OVS_VPORT_UPCALL_ATTR_SUCCESS. */ uint64_t upcall_fail; /* OVS_VPORT_UPCALL_ATTR_FAIL. */ - const struct nlattr *options; /* OVS_VPORT_ATTR_OPTIONS. */ - size_t options_len; }; void dpif_netlink_vport_init(struct dpif_netlink_vport *); diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at index 9a4607ef7..75ef7642d 100644 --- a/tests/system-kmod-macros.at +++ b/tests/system-kmod-macros.at @@ -14,16 +14,9 @@ m4_define([_ADD_BR], [[add-br $1 -- set Bridge $1 protocols=OpenFlow10,OpenFlow1 # specifies the expected output after filtering through uuidfilt. # 'dbinit-aux-args' are passed as additional commands to 'ovs-vsctl init' # before starting ovs-vswitchd. -# -# Best-effort loading of all available vport modules is performed. -# m4_define([OVS_TRAFFIC_VSWITCHD_START], [AT_CHECK([modprobe openvswitch]) on_exit 'modprobe -r openvswitch' - m4_foreach([mod], [[vport_geneve], [vport_gre], [vport_vxlan]], - [modprobe -q mod || echo "Module mod not loaded." - on_exit 'modprobe -q -r mod' - ]) on_exit 'ovs-dpctl del-dp ovs-system' on_exit 'ovs-appctl dpctl/flush-conntrack' _OVS_VSWITCHD_START([], [$3]) diff --git a/tests/system-offloads-testsuite-macros.at b/tests/system-offloads-testsuite-macros.at index 2ce3f4d6f..9d22c6043 100644 --- a/tests/system-offloads-testsuite-macros.at +++ b/tests/system-offloads-testsuite-macros.at @@ -18,10 +18,6 @@ limitations under the License.]) m4_define([OVS_TRAFFIC_VSWITCHD_START], [AT_CHECK([modprobe openvswitch]) on_exit 'modprobe -r openvswitch' - m4_foreach([mod], [[vport_geneve], [vport_gre], [vport_vxlan]], - [modprobe -q mod || echo "Module mod not loaded." - on_exit 'modprobe -q -r mod' - ]) on_exit 'ovs-dpctl del-dp ovs-system' on_exit 'ovs-appctl dpctl/flush-conntrack' _OVS_VSWITCHD_START([], [-- set Open_vSwitch . other_config:hw-offload=true $3]) diff --git a/utilities/docker/ovs-override.conf b/utilities/docker/ovs-override.conf index 682cb5251..855702e06 100644 --- a/utilities/docker/ovs-override.conf +++ b/utilities/docker/ovs-override.conf @@ -1,3 +1 @@ override openvswitch * extra -override vport-geneve * extra -override vport-* * extra diff --git a/utilities/docker/start-ovs b/utilities/docker/start-ovs index 054c7f163..fd21ef08b 100755 --- a/utilities/docker/start-ovs +++ b/utilities/docker/start-ovs @@ -27,7 +27,6 @@ case $1 in ;; "ovs-vswitchd") depmod -a modprobe openvswitch - modprobe vport_geneve /usr/share/openvswitch/scripts/ovs-ctl \ --no-ovsdb-server start /usr/share/openvswitch/scripts/ovs-ctl \ -- 2.54.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
