Re: [PATCH nf] ipvs: skip IPv6 extension headers in SCTP state lookup

Hi Julian,

> On Jul 6, 2026, at 22:17, Julian Anastasov <[email protected]> wrote:
>=20
>=20
> Hello,
>=20
> On Mon, 6 Jul 2026, Yizhou Zhao wrote:
>=20
>>> On Jul 6, 2026, at 01:35, Julian Anastasov <[email protected]> wrote:
>>>=20
>>> 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.
>>=20
>> I agree that the already parsed transport-header offset should be=20
>> passed from ip_vs_set_state() down to the protocol state_transition()=20=

>> callbacks, instead of reparsing the skb in set_sctp_state(). We will=20=

>> send a v2 that does this for SCTP, TCP and the other IPVS protocols=20=

>> in one combined fix.
>>=20
>>> 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.
>>=20
>> 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=20=

>> needs some care, because spoofed ICMP packets can match an=20
>> existing embedded tuple before the endpoint TCP/SCTP stack=20
>> performs its own validation. Maybe this change needs further
>> discussion?
>=20
> May be only for the schedule_icmp case while the other
> ICMP replies should not change it:
>=20
> 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,
>=20
> /* 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 =3D=3D cih->protocol || IPPROTO_UDP =3D=3D =
cih->protocol ||
>    IPPROTO_SCTP =3D=3D cih->protocol)
> offset +=3D 2 * sizeof(__u16);
>=20
> Here is why schedule_icmp was added:
>=20
> =
https://archive.linuxvirtualserver.org/html/lvs-devel/2015-08/msg00015.htm=
l
>=20
> 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=20
> server by using short conn timeout...

I agree. The embedded TCP/SCTP header in this path may not have the same
meaning as a normal client packet reaching the virtual service, so I do =
not
see a good fix for the ICMP state update part at the moment.

BTW, we have kept the ICMP handling unchanged and submitted a v2 series
for the parsed transport-offset issue only, following your earlier =
suggestion
to pass the already parsed offset from ip_vs_set_state() down to the =
protocol
state handlers. The v2 also puts the TCP and SCTP fixes in the same =
thread:

=
https://lore.kernel.org/netdev/[email protected]=
hua.edu.cn/

Thanks for the review.

>=20
> Regards
>=20
> --
> Julian Anastasov <[email protected]>

Thanks,
Yizhou=