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