[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