Re: [PATCH net 1/1] ip6_tunnel: use skb_cow_head() in ip6_tnl_xmit()
Ido Schimmel <[email protected]>
| Newsgroups | gmane.linux.network |
|---|---|
| Message-ID | <20260806181918.GA2060872@shredder> |
On Thu, Aug 06, 2026 at 06:28:00PM +0200, Paolo Abeni wrote: > On 8/6/26 11:06 AM, zhilin zou wrote: > > On Thu, Aug 6, 2026 at 4:52 PM Ido Schimmel <[email protected]> wrote: > >> > >> On Wed, Aug 05, 2026 at 05:29:02PM -0700, Jakub Kicinski wrote: > >>> On Wed, 5 Aug 2026 12:07:21 +0300 Ido Schimmel wrote: > >>>> __gre6_xmit() and ip6erspan_tunnel_xmit() already call skb_cow_head() > >>>> before calling ip6_tnl_xmit(). > >>> > >>> Probably just me but can't they all be buggy? > >>> PKTGEN likes to send shared skbs around in funny ways. > >>> Can we get a good explanation in the commit msg or maybe let's > >>> keep the check? > >> > >> ip6_tnl_xmit() is accessible via two Ethernet devices (pktgen doesn't > >> support other types) and they both clear IFF_TX_SKB_SHARING, so if > >> pktgen sends them shared skbs, I would say that it's a pktgen bug and > >> not a reason to block this patch. Note that pktgen is not available to > >> unprivileged users, so it's a less severe bug. > >> > >> The patch also makes ip6_tnl_xmit() consistent with its IPv4 counterpart > >> (ip_tunnel_xmit()) which is already using skb_cow_head(). > >> > >> Zhiling, please add a note in the commit message that ip6gretap and > >> ip6erspan do not expect to be handed shared skbs given that they clear > >> IFF_TX_SKB_SHARING. > > > > Thanks, Ido. I'll add a note to the commit message explaining that > > ip6gretap and ip6erspan clear IFF_TX_SKB_SHARING and therefore do not > > expect shared skbs, and send a v2 with your Reviewed-by tag retained. > > I'm not sure if it's too late, but since a v2 is required, I think it > would be better to keep the skb_shared check: that will avoid a later > patch for the pktgen/shared issue that inevitably will land, possibly > via the security channel. What do you mean by "keep the skb_shared check"? Return an error if the skb is shared before calling skb_cow_head()? How would that avoid "a later patch for the pktgen/shared issue" when other tunnels are already calling skb_cow_head() without an skb_shared() check (they clear IFF_TX_SKB_SHARING)? Not saying that there isn't a possible problem with pktgen, but it should be fixed there and not papered over in various drivers.