Netdev List
 help / color / mirror / Atom feed
From: Julian Anastasov <ja@ssi.bg>
To: Yizhou Zhao <zhaoyz24@mails.tsinghua.edu.cn>
Cc: netdev@vger.kernel.org, Simon Horman <horms@verge.net.au>,
	Pablo Neira Ayuso <pablo@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>,
	lvs-devel@vger.kernel.org, netfilter-devel@vger.kernel.org,
	coreteam@netfilter.org, linux-kernel@vger.kernel.org,
	Yuxiang Yang <yangyx22@mails.tsinghua.edu.cn>,
	Ao Wang <wangao@seu.edu.cn>, Xuewei Feng <fengxw06@126.com>,
	Qi Li <qli01@tsinghua.edu.cn>, Ke Xu <xuke@tsinghua.edu.cn>,
	stable@vger.kernel.org
Subject: Re: [PATCH nf] ipvs: skip IPv6 extension headers in SCTP state lookup
Date: Mon, 6 Jul 2026 17:17:38 +0300 (EEST)	[thread overview]
Message-ID: <bcd7f0e6-b2fc-4e54-f9ac-e1c8b1b0c497@ssi.bg> (raw)
In-Reply-To: <87E04893-B2B2-485F-B242-9CACF2F176D6@mails.tsinghua.edu.cn>


	Hello,

On Mon, 6 Jul 2026, Yizhou Zhao wrote:

> > On Jul 6, 2026, at 01:35, Julian Anastasov <ja@ssi.bg> wrote:
> > 
> > May be it is better starting from ip_vs_set_state()
> > to provide new arg 'int iph_len/offset' (set to iph.len), down to
> > state_transition(), sctp_state_transition() and set_sctp_state().
> > Same for all protos. It should cost less stack and ipv6_find_hdr()
> > calls and what matters most, correct iph context in case we
> > have IP+ICMP+TCP (with just two ports or even with TCP flags)
> > and are scheduling ICMP, i.e. not IP+TCP as usually.
> 
> I agree that the already parsed transport-header offset should be 
> passed from ip_vs_set_state() down to the protocol state_transition() 
> callbacks, instead of reparsing the skb in set_sctp_state(). We will 
> send a v2 that does this for SCTP, TCP and the other IPVS protocols 
> in one combined fix.
> 
> > But what I see is that ip_vs_in_icmp*() are missing
> > the ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd) call just
> > after ip_vs_in_stats() and before ip_vs_icmp_xmit() where
> > we should provide ciph.len. That is why we don't reach the
> > set_tcp_state() calls to set correct cp->state and timeout
> > when scheduling related ICMP. So, this should be fixed too.
> 
> For the ICMP path, I agree that the missing ip_vs_set_state() call is
> worth looking at, but using ICMP errors to drive the upper L4 state 
> needs some care, because spoofed ICMP packets can match an 
> existing embedded tuple before the endpoint TCP/SCTP stack 
> performs its own validation. Maybe this change needs further
> discussion?

	May be only for the schedule_icmp case while the other
ICMP replies should not change it:

diff --git a/net/netfilter/ipvs/ip_vs_core.c b/net/netfilter/ipvs/ip_vs_core.c
index 7f93239898ff..05fdcf4ce2c0 100644
--- a/net/netfilter/ipvs/ip_vs_core.c
+++ b/net/netfilter/ipvs/ip_vs_core.c
@@ -1971,6 +1971,8 @@ ip_vs_in_icmp(struct netns_ipvs *ipvs, struct sk_buff *skb, int *related,
 
 	/* do the statistics and put it back */
 	ip_vs_in_stats(cp, skb);
+	if (new_cp)
+		ip_vs_set_state(cp, IP_VS_DIR_INPUT, skb, pd, offset);
 	if (IPPROTO_TCP == cih->protocol || IPPROTO_UDP == cih->protocol ||
 	    IPPROTO_SCTP == cih->protocol)
 		offset += 2 * sizeof(__u16);

	Here is why schedule_icmp was added:

https://archive.linuxvirtualserver.org/html/lvs-devel/2015-08/msg00015.html

	But the inner TCP header should be generated by the
real server, not from the client, so things can go wrong. We can
leave it as it is now - we will forward the ICMP to the right real 
server by using short conn timeout...

Regards

--
Julian Anastasov <ja@ssi.bg>


      reply	other threads:[~2026-07-06 14:17 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-05 12:30 [PATCH nf] ipvs: skip IPv6 extension headers in SCTP state lookup Yizhou Zhao
2026-07-05 17:35 ` Julian Anastasov
2026-07-06  8:22   ` Yizhou Zhao
2026-07-06 14:17     ` Julian Anastasov [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=bcd7f0e6-b2fc-4e54-f9ac-e1c8b1b0c497@ssi.bg \
    --to=ja@ssi.bg \
    --cc=coreteam@netfilter.org \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fengxw06@126.com \
    --cc=fw@strlen.de \
    --cc=horms@verge.net.au \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lvs-devel@vger.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=qli01@tsinghua.edu.cn \
    --cc=stable@vger.kernel.org \
    --cc=wangao@seu.edu.cn \
    --cc=xuke@tsinghua.edu.cn \
    --cc=yangyx22@mails.tsinghua.edu.cn \
    --cc=zhaoyz24@mails.tsinghua.edu.cn \
    /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