Paolo Valerio <pvale...@redhat.com> writes:
> Simon Horman <ho...@ovn.org> writes:
>
>> On Wed, Aug 28, 2024 at 07:14:06PM +0200, Paolo Valerio wrote:
>>> As Long reported, kernels built without CONFIG_NETFILTER_CONNCOUNT
>>> result in the unexpected failure of the following tests:
>>>
>>> conntrack - multiple zones, local
>>> conntrack - multi-stage pipeline, local
>>> conntrack - can match and clear ct_state from outside OVS
>>>
>>> this happens because the nf_conncount turns on connection tracking and
>>> the above tests rely on this side effect. However, this behavior may
>>> be corrected in the kernel, which could, in turn, cause the tests to
>>> fail.
>>>
>>> The patch removes the assumption by adding explicit iptables rules to
>>> attach an nf_conn template to the skb resulting tracked once hit the
>>> OvS pipeline.
>>>
>>> While at it, introduce $HAVE_IPTABLES and skip tests if iptables
>>> binary is not present.
>>>
>>> Reported-by: Xin Long <lucien....@gmail.com>
>>> Reported-at: https://issues.redhat.com/browse/FDP-708
>>> Signed-off-by: Paolo Valerio <pvale...@redhat.com>
>>
>> Hi Paolo,
>>
>> I exercised this using vng with net-next compiled using
>> tools/testing/selftests/net/config from the upstream kernel tree [1].
>>
>> [1]
>> https://github.com/linux-netdev/nipa/wiki/How-to-run-netdev-selftests-CI-style
>>
>> The resulting config does not have CONFIG_NETFILTER_CONNCOUNT set.
>>
>
> Hi Simon,
>
> I used vng with net-next as well, but with a different config. In my
> case I simply reused a previously used config which is essentially my
> local one updated with olddefconfig and manually (to remove/add things).
>
>> Some observations:
>>
>> * CONFIG_NETFILTER_XT_TARGET_CT is required for -j CT
>>
>> I don't think this is a problem (other than my own problem
>> of it taking me a long time to figure that out). But it seems
>> worth noting (see parentheses in previous sentence:).
>>
>
> It's something I was thinking about right after the patch submission.
> Although extensions are fairly common in the main distros, maybe we may
> consider checking they are present.
>
> The only ways that comes to mind to reliably check whether the
> extensions (both kernel and user space) are present is simply by
> applying a rule.
>
> I guess we can (added a diff at the bottom), given the nft discussion
> and the potential follow-up, change things a bit in order to better
> accomodate a potential migration/follow-up.
>
> WDYT?
>
>> * Of the tests that are updated by this patch,
>> I only observed that the last one,
>> "conntrack - can match and clear ct_state from outside OVS",
>> fails without this patch applied.
>>
>> I am unsure if that is something that warrants updating this
>> patch or not. Or if, rather, there is an error in my testing.
>
> Thanks for testing it.
>
> Interesting.
> I tried the following config against net-next and it seems I can't
> reproduce that behaviour:
>
> vng --build --config tools/testing/selftests/net/config \
> --configitem CONFIG_NF_CONNTRACK_ZONES=y \
> --configitem CONFIG_NETFILTER_XT_TARGET_CT=m -v
>
> ---
> diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
> index 5203b1df8..6b5eb32fc 100644
> --- a/tests/system-kmod-macros.at
> +++ b/tests/system-kmod-macros.at
> @@ -267,3 +267,22 @@ m4_define([OVS_CHECK_BAREUDP],
> AT_SKIP_IF([! ip link add dev ovs_bareudp0 type bareudp dstport 6635
> ethertype mpls_uc 2>&1 >/dev/null])
> AT_CHECK([ip link del dev ovs_bareudp0])
> ])
> +
> +# CHECK_EXTERNAL_CT()
> +#
> +# Checks if packets can be tracked outside OvS.
> +m4_define([CHECK_EXTERNAL_CT],
> +[
> + dnl Kernel config (CONFIG_NETFILTER_XT_TARGET_CT)
> + dnl and user space extensions need to be present.
> + AT_SKIP_IF([! iptables -t raw -I OUTPUT 1 -j CT])
> + AT_CHECK([iptables -t raw -D OUTPUT 1])
> +])
> +
> +# ADD_EXTERNAL_CT()
> +#
> +# Let conntrack start tracking the packets outside OvS.
> +m4_define([ADD_EXTERNAL_CT],
> +[
> + AT_CHECK([iptables -t raw -I OUTPUT 1 -o $1 -j CT])
> +])
on_exit here got lost for some reason.
Below the corrected diff.
---
diff --git a/tests/system-kmod-macros.at b/tests/system-kmod-macros.at
index 5203b1df8..ab89ea24b 100644
--- a/tests/system-kmod-macros.at
+++ b/tests/system-kmod-macros.at
@@ -267,3 +267,23 @@ m4_define([OVS_CHECK_BAREUDP],
AT_SKIP_IF([! ip link add dev ovs_bareudp0 type bareudp dstport 6635
ethertype mpls_uc 2>&1 >/dev/null])
AT_CHECK([ip link del dev ovs_bareudp0])
])
+
+# CHECK_EXTERNAL_CT()
+#
+# Checks if packets can be tracked outside OvS.
+m4_define([CHECK_EXTERNAL_CT],
+[
+ dnl Kernel config (CONFIG_NETFILTER_XT_TARGET_CT)
+ dnl and user space extensions need to be present.
+ AT_SKIP_IF([! iptables -t raw -I OUTPUT 1 -j CT])
+ AT_CHECK([iptables -t raw -D OUTPUT 1])
+])
+
+# ADD_EXTERNAL_CT()
+#
+# Let conntrack start tracking the packets outside OvS.
+m4_define([ADD_EXTERNAL_CT],
+[
+ AT_CHECK([iptables -t raw -I OUTPUT 1 -o $1 -j CT])
+ on_exit 'iptables -t raw -D OUTPUT 1'
+])
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 46a9414d4..5435a6241 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -5458,12 +5458,12 @@ OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([conntrack - multiple zones, local])
-AT_SKIP_IF([test $HAVE_IPTABLES = no])
+CHECK_EXTERNAL_CT()
CHECK_CONNTRACK()
CHECK_CONNTRACK_LOCAL_STACK()
OVS_TRAFFIC_VSWITCHD_START()
-IPTABLES_CT([br0])
+ADD_EXTERNAL_CT([br0])
ADD_NAMESPACES(at_ns0)
AT_CHECK([ip addr add dev br0 "10.1.1.1/24"])
@@ -5509,12 +5509,12 @@ OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([conntrack - multi-stage pipeline, local])
-AT_SKIP_IF([test $HAVE_IPTABLES = no])
+CHECK_EXTERNAL_CT()
CHECK_CONNTRACK()
CHECK_CONNTRACK_LOCAL_STACK()
OVS_TRAFFIC_VSWITCHD_START()
-IPTABLES_CT([br0])
+ADD_EXTERNAL_CT([br0])
ADD_NAMESPACES(at_ns0)
AT_CHECK([ip addr add dev br0 "10.1.1.1/24"])
@@ -8392,7 +8392,7 @@ OVS_TRAFFIC_VSWITCHD_STOP
AT_CLEANUP
AT_SETUP([conntrack - can match and clear ct_state from outside OVS])
-AT_SKIP_IF([test $HAVE_IPTABLES = no])
+CHECK_EXTERNAL_CT()
CHECK_CONNTRACK_LOCAL_STACK()
OVS_CHECK_GENEVE()
@@ -8403,7 +8403,7 @@ AT_CHECK([ovs-ofctl add-flow br0 "actions=normal"])
AT_CHECK([ovs-ofctl add-flow br-underlay
"priority=100,ct_state=+trk,actions=ct_clear,resubmit(,0)"])
AT_CHECK([ovs-ofctl add-flow br-underlay "priority=10,actions=normal"])
-IPTABLES_CT([br0])
+ADD_EXTERNAL_CT([br0])
ADD_NAMESPACES(at_ns0)
dnl Set up underlay link from host into the namespace using veth pair.
diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at
index d9b5b7e4c..c1be97347 100644
--- a/tests/system-userspace-macros.at
+++ b/tests/system-userspace-macros.at
@@ -357,3 +357,19 @@ m4_define([OVS_CHECK_BAREUDP],
[
AT_SKIP_IF([:])
])
+
+# CHECK_EXTERNAL_CT()
+#
+# The userspace datapath does not support external ct.
+m4_define([CHECK_EXTERNAL_CT],
+[
+ AT_SKIP_IF([:])
+])
+
+# ADD_EXTERNAL_CT()
+#
+# The userspace datapath does not support external ct.
+m4_define([ADD_EXTERNAL_CT],
+[
+ AT_SKIP_IF([:])
+])
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
