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>
prev parent 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