|
Hello,
On Mon, 6 Jul 2026, Yizhou Zhao wrote:
> > On Jul 6, 2026, at 01:35, Julian Anastasov <ja@xxxxxx> 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@xxxxxx>
|
