Re: [PATCH mptcp-next v2 1/2] selftests: mptcp: convert iptables to nftables for mptcp_sockopt.sh

[Hangbin Liu <hangbin.liu@linux.dev>]

On Thu, Sep 03, 2026 at 01:19:11AM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The debug rule for SYN packets lacks a counter in the nftables conversion, rendering it completely dead code and defeating its stated purpose.
> --
> 
> commit 10b242a411677a92a0a291c8ae35773ac1ef8e56
> Author: Hangbin Liu <liuhangbin@kylinos.cn>
> 
> selftests: mptcp: convert iptables to nftables for mptcp_sockopt.sh
>     
> During the conversion, we retain the same filter and chain names previously
> used by iptables/ip6tables. Counters are not added to accept rules because
> the test does not inspect them. After conversion, the generated output
> matches the original iptables/ip6tables behavior.
> 
> > diff --git a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> > index e850a87429b61..a2c20483986dc 100755
> > --- a/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> > +++ b/tools/testing/selftests/net/mptcp/mptcp_sockopt.sh
> > @@ -50,15 +48,25 @@ add_mark_rules()
> >  	local m=$2
> >  
> >  	local t
> > -	for t in ${iptables} ${ip6tables}; do
> > +	for t in ip ip6; do
> > +		ip netns exec "$ns" nft add table "$t" filter
> > +		ip netns exec "$ns" nft add chain "$t" filter OUTPUT \
> > +			'{ type filter hook output priority 0; policy accept; }'
> > +
> >  		# just to debug: check we have multiple subflows connection requests
> > -		ip netns exec $ns $t -A OUTPUT -p tcp --syn -m mark --mark $m -j ACCEPT
> > +		ip netns exec "$ns" nft add rule "$t" filter OUTPUT \
> > +			tcp flags \& \(fin \| syn \| rst \| ack\) == syn \
> > +			meta mark "$m" accept
> 
> [Severity: Medium]
> Since this rule doesn't have a counter statement, does it become entirely
> dead code?
> 
> Because the SYN packets it matches are accepted anyway by the broader rule
> below (`meta l4proto tcp meta mark "$m" accept`), omitting the counter seems
> to eliminate the debugging capability mentioned in the comment. Should a
> counter be included here so the rule can still provide packet counts for
> manual inspection?

The --sync rule counter is not checked anywhere in the script. If someone
want do a manual inspection, they can do it manually in the script. So I
think there is no need to add counter for the debug rule.

Thanks
Hangbin