From: Simon Horman <horms@kernel.org>
To: Lorenzo Bianconi <lorenzo@kernel.org>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>,
Jozsef Kadlecsik <kadlec@netfilter.org>,
Florian Westphal <fw@strlen.de>, Phil Sutter <phil@nwl.cc>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Shuah Khan <shuah@kernel.org>,
netfilter-devel@vger.kernel.org, coreteam@netfilter.org,
netdev@vger.kernel.org, linux-kselftest@vger.kernel.org
Subject: Re: [PATCH nf-next] selftests: netfilter: nft_flowtable.sh: Add the capability to send IPv6 TCP traffic
Date: Thu, 27 Nov 2025 11:59:09 +0000 [thread overview]
Message-ID: <aSg9DbG4_NlLxEcy@horms.kernel.org> (raw)
In-Reply-To: <aSg1MWLUx0GyCWij@lore-desk>
On Thu, Nov 27, 2025 at 12:25:37PM +0100, Lorenzo Bianconi wrote:
> > On Sat, Nov 22, 2025 at 07:41:38PM +0100, Lorenzo Bianconi wrote:
> > > Introduce the capability to send TCP traffic over IPv6 to
> > > nft_flowtable netfilter selftest.
> > >
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > > ---
> > > .../selftests/net/netfilter/nft_flowtable.sh | 47 +++++++++++++++-------
> > > 1 file changed, 33 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/net/netfilter/nft_flowtable.sh b/tools/testing/selftests/net/netfilter/nft_flowtable.sh
> > > index 1fbfc8ad8dcdc5db2ab1a1ea9310f655d09eee83..24b4e60b91451e7ea7f6a041b0335233047c6242 100755
> > > --- a/tools/testing/selftests/net/netfilter/nft_flowtable.sh
> > > +++ b/tools/testing/selftests/net/netfilter/nft_flowtable.sh
> > > @@ -127,6 +127,8 @@ ip -net "$nsr1" addr add fee1:2::1/64 dev veth1 nodad
> > > ip -net "$nsr2" addr add 192.168.10.2/24 dev veth0
> > > ip -net "$nsr2" addr add fee1:2::2/64 dev veth0 nodad
> > >
> > > +ip netns exec "$nsr1" sysctl net.ipv6.conf.all.forwarding=1 > /dev/null
> > > +ip netns exec "$nsr2" sysctl net.ipv6.conf.all.forwarding=1 > /dev/null
> > > for i in 0 1; do
> > > ip netns exec "$nsr1" sysctl net.ipv4.conf.veth$i.forwarding=1 > /dev/null
> > > ip netns exec "$nsr2" sysctl net.ipv4.conf.veth$i.forwarding=1 > /dev/null
> > > @@ -153,7 +155,9 @@ ip -net "$ns1" route add default via dead:1::1
> > > ip -net "$ns2" route add default via dead:2::1
> > >
> > > ip -net "$nsr1" route add default via 192.168.10.2
> > > +ip -6 -net "$nsr1" route add default via fee1:2::2
> > > ip -net "$nsr2" route add default via 192.168.10.1
> > > +ip -6 -net "$nsr2" route add default via fee1:2::1
> > >
> > > ip netns exec "$nsr1" nft -f - <<EOF
> > > table inet filter {
> > > @@ -352,8 +356,9 @@ test_tcp_forwarding_ip()
> > > local nsa=$1
> > > local nsb=$2
> > > local pmtu=$3
> > > - local dstip=$4
> > > - local dstport=$5
> > > + local proto=$4
> > > + local dstip=$5
> > > + local dstport=$6
> > > local lret=0
> > > local socatc
> > > local socatl
> > > @@ -363,12 +368,12 @@ test_tcp_forwarding_ip()
> > > infile="$nsin_small"
> > > fi
> > >
> > > - timeout "$SOCAT_TIMEOUT" ip netns exec "$nsb" socat -4 TCP-LISTEN:12345,reuseaddr STDIO < "$infile" > "$ns2out" &
> > > + timeout "$SOCAT_TIMEOUT" ip netns exec "$nsb" socat -${proto} TCP${proto}-LISTEN:12345,reuseaddr STDIO < "$infile" > "$ns2out" &
> >
> > Hi Lorenzo,
> >
> > Some minor nits:
> >
> > 1. This line is (and was) excessively long.
> > Maybe it can be addressed as the line is being modified anyway.
> >
> > Flagged by checkpatch
> >
> > 2. Prior to this patch, variables on this line were enclosed in "" to
> > guard against word splitting when expansion occurs.
> > This is no longer the case.
> >
> > Flagged by shellcheck
> >
> > > lpid=$!
> > >
> > > busywait 1000 listener_ready
> > >
> > > - timeout "$SOCAT_TIMEOUT" ip netns exec "$nsa" socat -4 TCP:"$dstip":"$dstport" STDIO < "$infile" > "$ns1out"
> > > + timeout "$SOCAT_TIMEOUT" ip netns exec "$nsa" socat -${proto} TCP${proto}:"$dstip":"$dstport" STDIO < "$infile" > "$ns1out"
> > > socatc=$?
> >
> > Likewise here.
> >
> > >
> > > wait $lpid
> >
> > Otherwise this LGTM.
>
> Hi Simon,
>
> ack, fine. Is it ok to address them in subsequent patch or do you prefer to
> address them here?
Hi Lorenzo,
No preference on my side.
And feel free to include the following either way.
Reviewed-by: Simon Horman <horms@kernel.org>
> @Pablo: what do you prefer?
prev parent reply other threads:[~2025-11-27 11:59 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-11-22 18:41 [PATCH nf-next] selftests: netfilter: nft_flowtable.sh: Add the capability to send IPv6 TCP traffic Lorenzo Bianconi
2025-11-27 10:19 ` Simon Horman
2025-11-27 11:25 ` Lorenzo Bianconi
2025-11-27 11:59 ` Simon Horman [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aSg9DbG4_NlLxEcy@horms.kernel.org \
--to=horms@kernel.org \
--cc=coreteam@netfilter.org \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=fw@strlen.de \
--cc=kadlec@netfilter.org \
--cc=kuba@kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=lorenzo@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=netfilter-devel@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=pablo@netfilter.org \
--cc=phil@nwl.cc \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.