diff mbox series

[ovs-dev,2/2] tests: Use OVS_CHECK_XT6 for all applicable IPv6 tests.

Message ID 20241115-more-nft-v1-2-73d4f07a6e83@ovn.org
State Accepted
Commit 34868de0182b3b2ec2f5815184e5db5d55f54239
Delegated to: Ilya Maximets
Headers show
Series tests: Use OVS_CHECK_XT* for all applicable tests. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Simon Horman Nov. 15, 2024, 5:28 p.m. UTC
Commit d595473ccaae ("tests: Add nft accept support.") uses
nft, when available, instead of iptables to add an accept rule.

Unfortunately several such cases were missed by that patch.
This patchset seeks to address the IPv6 cases, all of which were missed.

It does so by:

1. Generalising NFT_ACCEPT() and IPTABLES_ACCEPT() to also handle IPv6.
2. Adding XT6_ACCEPT
3. Using XT6_ACCEPT and OVS_CHECK_XT in the relevant tests

Note that the use of OVS_CHECK_XT adds prerequisites checks to the
relevant tests, which were previously absent.

Reported-by: Paolo Valerio <pvalerio@redhat.com>
Signed-off-by: Simon Horman <horms@ovn.org>
---
 tests/ovs-macros.at     | 36 +++++++++++++++++++++++++++---------
 tests/system-traffic.at | 14 ++++----------
 2 files changed, 31 insertions(+), 19 deletions(-)

Comments

Paolo Valerio Dec. 1, 2024, 7:33 p.m. UTC | #1
Simon Horman <horms@ovn.org> writes:

> Commit d595473ccaae ("tests: Add nft accept support.") uses
> nft, when available, instead of iptables to add an accept rule.
>
> Unfortunately several such cases were missed by that patch.
> This patchset seeks to address the IPv6 cases, all of which were missed.
>
> It does so by:
>
> 1. Generalising NFT_ACCEPT() and IPTABLES_ACCEPT() to also handle IPv6.
> 2. Adding XT6_ACCEPT
> 3. Using XT6_ACCEPT and OVS_CHECK_XT in the relevant tests
>
> Note that the use of OVS_CHECK_XT adds prerequisites checks to the
> relevant tests, which were previously absent.
>
> Reported-by: Paolo Valerio <pvalerio@redhat.com>
> Signed-off-by: Simon Horman <horms@ovn.org>
> ---

Acked-by: Paolo Valerio <pvalerio@redhat.com>
Ilya Maximets Dec. 2, 2024, 11:36 p.m. UTC | #2
On 12/1/24 20:33, Paolo Valerio wrote:
> Simon Horman <horms@ovn.org> writes:
> 
>> Commit d595473ccaae ("tests: Add nft accept support.") uses
>> nft, when available, instead of iptables to add an accept rule.
>>
>> Unfortunately several such cases were missed by that patch.
>> This patchset seeks to address the IPv6 cases, all of which were missed.
>>
>> It does so by:
>>
>> 1. Generalising NFT_ACCEPT() and IPTABLES_ACCEPT() to also handle IPv6.
>> 2. Adding XT6_ACCEPT
>> 3. Using XT6_ACCEPT and OVS_CHECK_XT in the relevant tests
>>
>> Note that the use of OVS_CHECK_XT adds prerequisites checks to the
>> relevant tests, which were previously absent.
>>
>> Reported-by: Paolo Valerio <pvalerio@redhat.com>
>> Signed-off-by: Simon Horman <horms@ovn.org>
>> ---
> 
> Acked-by: Paolo Valerio <pvalerio@redhat.com>

Thanks, Simona and Paolo!

I went ahead and applied this set to main.  And then backported
down to branch 3.3 with all the prerequisites:

* ec2a950d7 2024-11-05 | tests: Handle marks using nft if available. [Simon Horman]
* 91ee06739 2024-11-05 | tests: Add nft support to ADD_EXTERNAL_CT. [Simon Horman]
* d595473cc 2024-11-05 | tests: Add nft accept support. [Simon Horman]
* 445991838 2024-10-07 | ovs-macros.at: Correctly delete iptables rule on_exit. [Paolo Valerio]
* 60917c822 2024-10-07 | system-traffic: Do not rely on conncount for already tracked packets. [Paolo Valerio]

Now we should be good in regards of conntrack and using nft in tests
on all these branches.

We're still using iptables in some scripts outside of tests, like ovs-ctl,
but that's a separate topic.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
index 90258ef07b59..738cda2e46ce 100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -361,30 +361,48 @@  m4_ifndef([AT_FAIL_IF],
     && exit 99 || exit 0], [0], [ignore], [ignore])])])
 
 dnl Add a rule to always accept the traffic.
+dnl The first argument to this macro should be the command to run:
+dnl iptables or ip6tables
+dnl The second argument to this macro should be the interface name (netdev)
 m4_define([IPTABLES_ACCEPT],
-  [AT_CHECK([iptables -I INPUT 1 -i $1 -j ACCEPT])
-   on_exit 'iptables -D INPUT 1'])
+  [AT_CHECK([$1 -I INPUT 1 -i $2 -j ACCEPT])
+   on_exit '$1 -D INPUT 1'])
 
 dnl Certain Linux distributions, like CentOS, have default iptable rules
 dnl to reject input traffic from bridges such as br-underlay.
-dnl This implies the existence of a ip filter INPUT chain.
-dnl If that chain exists then add a rule to it to always accept all traffic.
+dnl This implies the existence of a ip filter INPUT chain for IPv4 or an
+dnl ip6 filter INPUT chain for IPv6.  If that chain exists then add a rule
+dnl to it to always accept all traffic.
+dnl The first argument to this macro should be the filter chain: ip or ipv6
+dnl The second argument to this macro should be the interface name (netdev)
 m4_define([NFT_ACCEPT],
-  [if nft list chain ip filter INPUT > /dev/null 2>1; then
+  [if nft list chain $1 filter INPUT > /dev/null 2>1; then
      AT_CHECK([nft -ae \
-               "insert rule ip filter INPUT iifname \"$1\" counter accept"],
+               "insert rule $1 filter INPUT iifname \"$2\" counter accept"],
                [0], [stdout-nolog])
      dnl Extract handle, which is used to delete the rule
      AT_CHECK([sed -n 's/.*handle //; T; p' < stdout], [0], [stdout])
-     on_exit "nft \"delete rule ip filter INPUT handle $(cat stdout)\""
+     on_exit "nft \"delete rule $1 filter INPUT handle $(cat stdout)\""
    fi])
 
 dnl Certain Linux distributions, like CentOS, have default iptable rules
 dnl to reject input traffic from bridges such as br-underlay.
 dnl Add a rule to always accept the traffic.
+dnl IPv4 variant of this macro.
 m4_define([XT_ACCEPT],
   [if test $HAVE_NFT = yes; then
-       NFT_ACCEPT([$1])
+       NFT_ACCEPT([ip], [$1])
    else
-       IPTABLES_ACCEPT([$1])
+       IPTABLES_ACCEPT([iptables], [$1])
+   fi])
+
+dnl Certain Linux distributions, like CentOS, have default iptable rules
+dnl to reject input traffic from bridges such as br-underlay.
+dnl Add a rule to always accept the traffic.
+dnl IPv6 variant of this macro.
+m4_define([XT6_ACCEPT],
+  [if test $HAVE_NFT = yes; then
+       NFT_ACCEPT([ip6], [$1])
+   else
+       IPTABLES_ACCEPT([ip6tables], [$1])
    fi])
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index a45946e6ff0f..75a0740e3289 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -1332,6 +1332,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over ip6erspan v1 tunnel by simulated packets])
+OVS_CHECK_XT()
 OVS_CHECK_MIN_KERNEL(3, 10)
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -1355,11 +1356,7 @@  ADD_OVS_TUNNEL6([ip6erspan], [br0], [at_erspan0], [fc00:100::1], [10.1.1.100/24]
 
 OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 2 fc00:100::100])
 
-dnl Certain Linux distributions, like CentOS, have default iptable rules
-dnl to reject input traffic from br-underlay. Here we add a rule to walk
-dnl around it.
-ip6tables -I INPUT 1 -i br-underlay -j ACCEPT
-on_exit 'ip6tables -D INPUT 1'
+XT6_ACCEPT([br-underlay])
 
 NETNS_DAEMONIZE([at_ns0], [tcpdump -n -x -i p0 dst host fc00:100::1 -l > p0.pcap 2>/dev/null], [tcpdump.pid])
 sleep 1
@@ -1387,6 +1384,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over ip6erspan v2 tunnel by simulated packets])
+OVS_CHECK_XT()
 OVS_CHECK_MIN_KERNEL(3, 10)
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -1410,11 +1408,7 @@  ADD_OVS_TUNNEL6([ip6erspan], [br0], [at_erspan0], [fc00:100::1], [10.1.1.100/24]
 
 OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 2 fc00:100::100])
 
-dnl Certain Linux distributions, like CentOS, have default iptable rules
-dnl to reject input traffic from br-underlay. Here we add a rule to walk
-dnl around it.
-ip6tables -I INPUT 1 -i br-underlay -j ACCEPT
-on_exit 'ip6tables -D INPUT 1'
+XT6_ACCEPT([br-underlay])
 
 NETNS_DAEMONIZE([at_ns0], [tcpdump -n -x -i p0 dst host fc00:100::1 -l > p0.pcap 2>/dev/null], [tcpdump.pid])
 sleep 1