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

[sashiko-bot@kernel.org]

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?

>  
>  		# RST packets might be handled by a internal dummy socket
> -		ip netns exec $ns $t -A OUTPUT -p tcp --tcp-flags RST RST -m mark --mark 0 -j ACCEPT
> +		ip netns exec "$ns" nft add rule "$t" filter OUTPUT \
> +			tcp flags \& rst == rst meta mark 0x0 accept
> +
> +		ip netns exec "$ns" nft add rule "$t" filter OUTPUT \
> +			meta l4proto tcp meta mark "$m" accept
> +		ip netns exec "$ns" nft add rule "$t" filter OUTPUT \
> +			meta l4proto tcp meta mark 0 counter drop
>  
> -		ip netns exec $ns $t -A OUTPUT -p tcp -m mark --mark $m -j ACCEPT
> -		ip netns exec $ns $t -A OUTPUT -p tcp -m mark --mark 0 -j DROP
>  	done
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903-mptcp_nft-v2-0-66283e4b9c3b@kylinos.cn?part=1