Re: [PATCH net-next 5/5] selftests: mptcp: convert iptables to nftables for mptcp_join.sh

[Matthieu Baerts <matttbe@kernel.org>]

On 28/09/2026 13:24, Matthieu Baerts wrote:
> Hello,
> 
> On 28/09/2026 10:00, netdev-bot+sashiko@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 2 potential
>> issue(s) to consider.
>>
>> Critical: 0 · High: 0 · Medium: 1 · Low: 1
>>
>> If you believe none of the issues are legitimate, please reply to each
>> issue of Medium severity or higher and explain why the AI is wrong.
> 
> TL;DR: It looks like there are issues with NFTables, but no new version
> needed here for these MPTCP patches.
> 
>> If any of them are legitimate and you plan to respin, please let patchwork
>> know by including "pw-bot: cr" as a separate line at the end of your reply
>> (one such reply per series is enough).
>>
>> - [Medium] In endpoint_tests()
>>   (tools/testing/selftests/net/mptcp/mptcp_join.sh), the new rule `tcp…
>> - [Low] The patch removes CONFIG_NFT_COMPAT from
>>   tools/testing/selftests/net/mptcp/config, "since we don't need it now".
>>
>> --
>>
>> selftests: mptcp: convert iptables to nftables for mptcp_join.sh
>>
>> This converts the iptables/ip6tables rules in mptcp_join.sh to nftables.
>> It keeps the same table and chain names and adds init_nftables() to set
>> up the tables only when a test needs them. The cBPF bytecode that matched
>> the ADD_ADDR and RM_ADDR suboptions is replaced with "tcp option mptcp
>> subtype" matching. In the config fragment, CONFIG_NFT_COMPAT is swapped
>> for the nft modules that are now needed.
> 
> (...)
> 
>>> diff --git a/tools/testing/selftests/net/mptcp/mptcp_join.sh b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>>> index 18ce7136a2b0e..b16e24418e737 100755
>>> --- a/tools/testing/selftests/net/mptcp/mptcp_join.sh
>>> +++ b/tools/testing/selftests/net/mptcp/mptcp_join.sh
>>
>> [ ... ]
>>
>>> @@ -4402,12 +4372,10 @@ endpoint_tests()
>>>  
>>>  		# To make sure RM_ADDR are sent over a different subflow, but
>>>  		# allow the rest to quickly and cleanly close the subflow
>>> -		local ipt=1
>>> -		ip netns exec "${ns2}" ${iptables} -I OUTPUT -s "10.0.1.2" \
>>> -			-p tcp -m tcp --tcp-option 30 \
>>> -			-m bpf --bytecode \
>>> -			"$CBPF_MPTCP_SUBOPTION_RM_ADDR" \
>>> -			-j DROP || ipt=0
>>> +		local nft=1
>>> +		ip netns exec "${ns2}" nft insert rule ip filter OUTPUT \
>>> +			ip saddr 10.0.1.2 meta l4proto tcp \
>>> +			tcp option mptcp subtype remove-addr drop || nft=0
>>
>> [Severity: Medium]
>> Can this rule ever match an RM_ADDR packet on an established connection?
>>
>> nft_exthdr_tcp_eval() stops at the first TCP option whose kind matches
>> (30 for MPTCP). The subtype check therefore only ever sees the first
>> MPTCP suboption:
>>
>> net/netfilter/nft_exthdr.c:nft_exthdr_tcp_eval() {
>>     ...
>> 	for (i = sizeof(*tcph); i < tcphdr_len - 1; i += optl) {
>> 		optl = optlen(opt, i);
>>
>> 		if (priv->type != opt[i])
>> 			continue;
>> 		...
>> 		return;
>> 	}
>>     ...
>> }
> 
> Indeed, the RM_ADDR will be added in a second MPTCP option. It looks
> like Netfilter doesn't handle that case. But that seems to be an issue
> on the Netfilter side, rather than with the command that should work. I
> will follow up with Netfilter devs. If a fix cannot be added on their
> side, I will change the nft command to look at a specific offset.
> 
> Note that the command here is just to check there is no RM_ADDR sent on
> the wrong side: it shouldn't catch any packets here anyway.

After a discussion with Netfilter devs, it looks like a fix will not be
easy to have. Yet, I wonder if it wouldn't be better to apply this patch
like that (it doesn't break things), and have an explicit follow-up/fix
to document this issue somehow.

@Hangbin: do you plan to look at a fix for that? I guess a raw payload
looking at the same fields as what the previous cBPF rule was doing
would be enough.

Don't hesitate to fix the "low" priority comments as well.

https://lore.kernel.org/all/20260926-net-next-mptcp-misc-feat-7-4-v1-0-67af4ab37406@kernel.org/T/#u

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.