On 5/25/23 09:19, Simon Horman wrote: > 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 <[email protected]> >>> --- >>> 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.
Yes, I agree. We may remove the kernel version check though as it is not necessary in the presence of a tool-based check. > > 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 [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
