[ovs-dev] [PATCH] dpif-netlink: Fix feature negotiation for older kernels
Dumitru Ceara
dceara at redhat.com
Wed Sep 15 10:27:02 UTC 2021
On 9/15/21 12:01 PM, Mark Gray wrote:
> Older kernels do not reject unsupported features. This can lead
> to a situation in which 'ovs-vswitchd' believes that a feature is
> supported when, in fact, it is not.
>
> This patch probes for this by attempting to set a known unsupported
> feature.
>
> Reported-by: Dumitru Ceara <dceara at redhat.com>
> Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2004083
> Suggested-by: Ilya Maximets <i.maximets at ovn.org>
> Signed-off-by: Mark Gray <mark.d.gray at redhat.com>
> ---
Hi Mark,
Thanks for working on this, it fixes the issue I was having when
starting with a fresh configuration on an old kernel.
There is however another case this patch doesn't seem to cover, although
I'm not 100% sure we need to address it. Please see below.
> lib/dpif-netlink.c | 66 ++++++++++++++++++++++++++++++----------------
> 1 file changed, 43 insertions(+), 23 deletions(-)
>
> diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c
> index 34fc04237333..c16323f7ee21 100644
> --- a/lib/dpif-netlink.c
> +++ b/lib/dpif-netlink.c
> @@ -84,6 +84,8 @@ enum { MAX_PORTS = USHRT_MAX };
> #define EPOLLEXCLUSIVE (1u << 28)
> #endif
>
> +#define OVS_DP_F_UNSUPPORTED (1 << 31);
> +
> /* This PID is not used by the kernel datapath when using dispatch per CPU,
> * but it is required to be set (not zero). */
> #define DPIF_NETLINK_PER_CPU_PID UINT32_MAX
> @@ -382,36 +384,54 @@ dpif_netlink_open(const struct dpif_class *class OVS_UNUSED, const char *name,
> dp_request.cmd = OVS_DP_CMD_SET;
> }
>
> - /* The Open vSwitch kernel module has two modes for dispatching upcalls:
> - * per-vport and per-cpu.
> - *
> - * When dispatching upcalls per-vport, the kernel will
> - * send the upcall via a Netlink socket that has been selected based on the
> - * vport that received the packet that is causing the upcall.
> - *
> - * When dispatching upcall per-cpu, the kernel will send the upcall via
> - * a Netlink socket that has been selected based on the cpu that received
> - * the packet that is causing the upcall.
> - *
> - * First we test to see if the kernel module supports per-cpu dispatching
> - * (the preferred method). If it does not support per-cpu dispatching, we
> - * fall back to the per-vport dispatch mode.
> + /* Some older kernels will not reject unknown features. This will cause
> + * 'ovs-vswitchd' to incorrectly assume a feature is supported. In order to
> + * test for that, we attempt to set a feature that we know is not supported
> + * by any kernel. If this feature is not rejected, we can assume we are
> + * running on one of these older kernels.
> */
> dp_request.user_features |= OVS_DP_F_UNALIGNED;
> - dp_request.user_features &= ~OVS_DP_F_VPORT_PIDS;
> - dp_request.user_features |= OVS_DP_F_DISPATCH_UPCALL_PER_CPU;
> + dp_request.user_features |= OVS_DP_F_VPORT_PIDS;
> + dp_request.user_features |= OVS_DP_F_UNSUPPORTED;
> error = dpif_netlink_dp_transact(&dp_request, &dp, &buf);
> + dp_request.user_features &= ~OVS_DP_F_UNSUPPORTED;
> if (error) {
> - dp_request.user_features &= ~OVS_DP_F_DISPATCH_UPCALL_PER_CPU;
> - dp_request.user_features |= OVS_DP_F_VPORT_PIDS;
> + /* The Open vSwitch kernel module has two modes for dispatching
> + * upcalls: per-vport and per-cpu.
> + *
> + * When dispatching upcalls per-vport, the kernel will
> + * send the upcall via a Netlink socket that has been selected based on
> + * the vport that received the packet that is causing the upcall.
> + *
> + * When dispatching upcall per-cpu, the kernel will send the upcall via
> + * a Netlink socket that has been selected based on the cpu that
> + * received the packet that is causing the upcall.
> + *
> + * First we test to see if the kernel module supports per-cpu
> + * dispatching (the preferred method). If it does not support per-cpu
> + * dispatching, we fall back to the per-vport dispatch mode.
> + */
> + dp_request.user_features |= OVS_DP_F_UNALIGNED;
> + dp_request.user_features &= ~OVS_DP_F_VPORT_PIDS;
> + dp_request.user_features |= OVS_DP_F_DISPATCH_UPCALL_PER_CPU;
> error = dpif_netlink_dp_transact(&dp_request, &dp, &buf);
> - }
> - if (error) {
> - return error;
> + if (error) {
> + dp_request.user_features &= ~OVS_DP_F_DISPATCH_UPCALL_PER_CPU;
> + dp_request.user_features |= OVS_DP_F_VPORT_PIDS;
> + error = dpif_netlink_dp_transact(&dp_request, &dp, &buf);
> + }
> + if (error) {
> + return error;
> + }
> +
> + error = open_dpif(&dp, dpifp);
> + dpif_netlink_set_features(*dpifp, OVS_DP_F_TC_RECIRC_SHARING);
> + } else {
> + VLOG_INFO("Kernel does not correctly support feature negotiation. "
> + "Using standard features");
> + error = open_dpif(&dp, dpifp);
If the datapath was already configured with
OVS_DP_F_DISPATCH_UPCALL_PER_CPU (e.g., running code that didn't include
this patch), but the kernel doesn't support it *and* also doesn't reject
it, we need to reset it. I guess we need another call to
dpif_netlink_dp_transact() here.
Regards,
Dumitru
> }
>
> - error = open_dpif(&dp, dpifp);
> - dpif_netlink_set_features(*dpifp, OVS_DP_F_TC_RECIRC_SHARING);
> ofpbuf_delete(buf);
>
> if (create) {
>
More information about the dev
mailing list