mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Matthieu Baerts <matttbe@kernel.org>
To: netdev-bot+sashiko@kernel.org
Cc: martineau@kernel.org, geliang@kernel.org, davem@davemloft.net,
	edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
	horms@kernel.org, netdev@vger.kernel.org, mptcp@lists.linux.dev,
	linux-kernel@vger.kernel.org, quanyeyang@proton.me
Subject: Re: [PATCH net-next 5/5] selftests: mptcp: convert iptables to nftables for mptcp_join.sh
Date: Mon, 28 Sep 2026 13:24:20 +0200	[thread overview]
Message-ID: <deb5e3b3-d1d7-4d1e-8bb6-95050769d656@kernel.org> (raw)
In-Reply-To: <179058242453.3145.1450534124183352369@kernel.org>

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.

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


  reply	other threads:[~2026-09-28 11:24 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-26 15:30 [PATCH net-next 0/5] mptcp: misc improvements for v7.4 Matthieu Baerts (NGI0)
2026-09-26 15:30 ` [PATCH net-next 1/5] mptcp: remove thmac from subflow ctx Matthieu Baerts (NGI0)
2026-09-26 15:30 ` [PATCH net-next 2/5] mptcp: split FASTCLOSE key from rcvr_key Matthieu Baerts (NGI0)
2026-09-26 15:30 ` [PATCH net-next 3/5] mptcp: shrink struct mptcp_options_received Matthieu Baerts (NGI0)
2026-09-26 15:30 ` [PATCH net-next 4/5] selftests: mptcp: convert iptables to nftables for mptcp_sockopt.sh Matthieu Baerts (NGI0)
2026-09-28  8:00   ` netdev-bot+sashiko
2026-09-28 11:24     ` Matthieu Baerts
2026-09-26 15:30 ` [PATCH net-next 5/5] selftests: mptcp: convert iptables to nftables for mptcp_join.sh Matthieu Baerts (NGI0)
2026-09-28  8:00   ` netdev-bot+sashiko
2026-09-28 11:24     ` Matthieu Baerts [this message]
2026-09-28 21:17       ` Matthieu Baerts

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=deb5e3b3-d1d7-4d1e-8bb6-95050769d656@kernel.org \
    --to=matttbe@kernel.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=geliang@kernel.org \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=martineau@kernel.org \
    --cc=mptcp@lists.linux.dev \
    --cc=netdev-bot+sashiko@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=quanyeyang@proton.me \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®