Thread (8 messages) 8 messages, 5 authors, 2026-08-25

Re: [PATCH net v2 1/1] ip6_tunnel: snapshot encap in xmit

flat view

From: Zixuan Chai <hidden>
Date: 2026-08-25 07:23:54

Dear Artem,

Thanks for the detailed feedback.

I agree that the RCU-protected config object is the better long-term
direction, and it also addresses the control-plane issue where a
rejected changelink can leave encapsulation partially applied.

For the immediate bug, my intent with v3 is only to keep a minimal fix
for the transmit-side crash that seems suitable for net:
one packet uses one consistent encapsulation snapshot in xmit, without
trying to solve the larger config / lookup / synchronize_net() questions
in the same patch.

I have already dropped the headroom change around t->hlen, so v3 keeps
that line unchanged and only fixes the xmit-side inconsistency that can
lead to skb_under_panic().

At this point, it seems there are two possible ways forward:
1. take this minimal fix through net for the crash, and do the RCU config
   conversion separately in net-next; or
2. follow your proposed approach and address this as part of the RCU
   config conversion.

Please feel free to choose whichever approach you think is the most
appropriate and elegant.

If we go with your approach, could you also please keep the following
attribution tags?

Reported-by: Zixuan Chai <redacted>
Reported-by: Vega <redacted>

Thank you for your time and review.

Best regards,
Zixuan Chai

Artem Lytkin [off-list ref] 于2026年8月25日周二 03:47写道:
On Sun, Aug 09, 2026 at 03:33:35PM +0300, Ido Schimmel wrote:
quoted
It's on my TODO list since last week, but I don't have the time to work
on it right now. It's a very large change (see the geneve change) that
is needed across all the IP tunnels, not something that I consider
suitable for net.
I'd like to pick this up for net-next, unless you have already started.

The shape I have in mind follows the geneve conversion: a separately
allocated config struct hanging off the tunnel under an __rcu pointer,
holding encap together with encap_hlen, tun_hlen and hlen. xmit
dereferences it once and passes the pointer down to ip6_tnl_encap() and
the header length helpers. changelink builds a new one, validates it,
and only then publishes it with rcu_assign_pointer(). That also stops a
rejected changelink from leaving the encap half applied, which is what
happens today when ip6_tnl_encap_setup() runs before the collect_md and
duplicate checks.

The synchronize_net() in ip6_tnl_update() and its siblings is a
separate question. It is there for hash coherence: ip6_tnl_lookup()
matches on parms.laddr, raddr and link while walking the bucket. It
does nothing for the header geometry xmit reads, so it can only go once
parms is in the config as well and the lookups dereference it.

Two things I'd settle before writing code. Header geometry only in a
first series, with parms following, or everything at once? And the
order: ip6_tunnel first, since that is where the report landed and
ip6_gre builds on it, then ip_tunnel.c for ipip and gre, and sit's own
copy in net/ipv6/sit.c after that. vti has no encap and no hlen, so it
needs nothing.

If you have notes or a half-done branch I'd gladly build on them.

Artem
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help