On Wed, May 24, 2023 at 09:23:59PM +0200, Ilya Maximets wrote: > On 5/24/23 15:39, Frode Nordahl wrote: > > The bareudp tests depend on specific kernel configuration to > > succeed. Skip the test if the feature is not enabled in the > > running kernel. > > > > Signed-off-by: Frode Nordahl <frode.nord...@canonical.com> > > --- > > tests/system-kmod-macros.at | 10 ++++++++++ > > tests/system-layer3-tunnels.at | 2 ++ > > tests/system-userspace-macros.at | 8 ++++++++ > > 3 files changed, 20 insertions(+) > > > > diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at > > index fb15a5a7c..55e7821ce 100644 > > --- a/tests/system-kmod-macros.at > > +++ b/tests/system-kmod-macros.at > > @@ -237,3 +237,13 @@ m4_define([CHECK_L3L4_CONNTRACK_REASM]) > > # > > # The kernel module tests do not use TC offload. > > m4_define([CHECK_NO_TC_OFFLOAD]) > > + > > +# OVS_CHECK_BAREUDP() > > +# > > +# The feature needs to be enabled in the kernel configuration > > (CONFIG_BAREUDP) > > +# to work. > > +m4_define([OVS_CHECK_BAREUDP], > > +[ > > + AT_SKIP_IF([! ip link add dev bareudp0 type bareudp dstport 6635 > > ethertype mpls_uc 2>&1 >/dev/null]) > > + AT_CHECK([ip link del dev bareudp0]) > > Maybe call it ovs_bareudp0 or something like that? Just to decrease chances > for random collisions. > > > +]) > > diff --git a/tests/system-layer3-tunnels.at b/tests/system-layer3-tunnels.at > > index c37852b21..5546bc879 100644 > > --- a/tests/system-layer3-tunnels.at > > +++ b/tests/system-layer3-tunnels.at > > @@ -155,6 +155,7 @@ AT_CLEANUP > > > > AT_SETUP([layer3 - ping over MPLS Bareudp]) > > OVS_CHECK_MIN_KERNEL(5, 7) > > +OVS_CHECK_BAREUDP() > > This new check supersedes the kernel version check. Should we remove it? > The original idea was that we can create bareudp tunnels even if the ip > utility > didn't support them at a time. But, I suppose, enough time passed since then > and the ip utility available in distributions caught up, so we can just check > it instead.
I'd say that kernel checks are unreliable in the presence of backports. Whereas tool checks suffer the problem you describe. At this point I'd lean towards a tool-based check. As you say, some time has passed by now. FWIIW, I believe we've always relied on a tool based check for PPS rate limiting checks, which are similar. Perhaps unwisely. But in practice the problems I'm aware of there is with the implementation of the check (hopefully fixed by now) not the tool vs kernel issue. > > OVS_TRAFFIC_VSWITCHD_START([_ADD_BR([br1])]) > > ADD_NAMESPACES(at_ns0, at_ns1) > > > > @@ -203,6 +204,7 @@ AT_CLEANUP > > > > AT_SETUP([layer3 - ping over Bareudp]) > > OVS_CHECK_MIN_KERNEL(5, 7) > > Same here. > > > +OVS_CHECK_BAREUDP() > > OVS_TRAFFIC_VSWITCHD_START([_ADD_BR([br1])]) > > ADD_NAMESPACES(at_ns0, at_ns1) > > > > diff --git a/tests/system-userspace-macros.at > > b/tests/system-userspace-macros.at > > index 482079386..1cb67d6f6 100644 > > --- a/tests/system-userspace-macros.at > > +++ b/tests/system-userspace-macros.at > > @@ -336,3 +336,11 @@ m4_define([CHECK_L3L4_CONNTRACK_REASM], > > # > > # Userspace tests do not use TC offload. > > m4_define([CHECK_NO_TC_OFFLOAD]) > > + > > +# OVS_CHECK_BAREUDP() > > +# > > +# The userspace skips all tests that check kernel configuration. > > This should probably say that userspace datapath doesn't support bareudp > tunnels > instead. > > > +m4_define([OVS_CHECK_BAREUDP], > > +[ > > + AT_SKIP_IF([:]) > > +]) > _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
