On 9/15/21 5:08 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 <[email protected]> > Reported-at: https://bugzilla.redhat.com/show_bug.cgi?id=2004083 > Suggested-by: Ilya Maximets <[email protected]> > Signed-off-by: Mark Gray <[email protected]> > ---
This works for me, for this version: Tested-by: Dumitru Ceara <[email protected]> I do have a small comment below, thanks! > > Notes: > v2: Fix case raised by Dumitru in which kernel is already configured with > unsupported features. > > lib/dpif-netlink.c | 72 ++++++++++++++++++++++++++++++++-------------- > 1 file changed, 50 insertions(+), 22 deletions(-) > > diff --git a/lib/dpif-netlink.c b/lib/dpif-netlink.c > index 34fc04237333..5f4b60c5a6d6 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,62 @@ 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; Nit: Sorry, I missed this on v1, but I guess we could just remove the lines that set OVS_DP_F_UNALIGNED and OVS_DP_F_VPORT_PIDS here, and just request OVS_DP_F_UNSUPPORTED. We set the correct features anyway, in all cases, below. As far as I can tell this doesn't affect functionality; I tested with the two lines removed and it looks OK to me. If you send a v3 removing these two lines please feel free to keep the "tested-by". > error = dpif_netlink_dp_transact(&dp_request, &dp, &buf); > if (error) { > - dp_request.user_features &= ~OVS_DP_F_DISPATCH_UPCALL_PER_CPU; > + /* 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_UNSUPPORTED; > + 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) { > + 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."); > + dp_request.cmd = OVS_DP_CMD_SET; > + dp_request.user_features = 0; > + dp_request.user_features |= OVS_DP_F_UNALIGNED; > dp_request.user_features |= OVS_DP_F_VPORT_PIDS; > error = dpif_netlink_dp_transact(&dp_request, &dp, &buf); > - } > - if (error) { > - return error; > + if (error) { > + return error; > + } > + error = open_dpif(&dp, dpifp); > } > > - error = open_dpif(&dp, dpifp); > - dpif_netlink_set_features(*dpifp, OVS_DP_F_TC_RECIRC_SHARING); > ofpbuf_delete(buf); > > if (create) { > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
