Re: [PATCH net-next v4 02/14] net: skb: use dstref for storing dst entry

[Sabrina Dubroca <sd@queasysnail.net>]

Eric, it seems your email didn't make it to netdev, quoting:

2025-11-13, 02:38:02 -0800, Eric Dumazet wrote:
> On Thu, Nov 13, 2025 at 1:37 AM Marek Mietus <mmietus97@yahoo.com> wrote:
> 
> > W dniu 11/12/25 o 18:09, Sabrina Dubroca pisze:
> > > 2025-11-12, 08:27:08 +0100, Marek Mietus wrote:
> > >> Use the newly introduced dstref object for storing the dst entry
> > >> in skb instead of using _skb_refdst, and remove code related
> > >> to _skb_refdst.
> > >
> > > This is an important change to a very core part of networking. You
> > > need to CC all the networking maintainers/reviewers for this series
> > > (ask scripts/get_maintainer.pl).
> >
> > Noted for next time.
> >
> > >
> > >> This is mostly a cosmetic improvement. It improves readability
> > >
> > > That rename, and the rest of the changes in this series. is causing
> > > some non-negligible churn and will take a while to review, to ensure
> > > all the conversions are correct.
> > >
> > > @Maintainers can I get some time to look at this in detail?
> > >
> >
> > I figured it would require a thorough review.
> > Thank you for taking the time to look at it!
> >
> > >
> > > Also, I'm not sure how we ended up from the previous proposal ("some
> > > tunnels are under RCU so they don't need a reference" [1]) to this.
> > >
> > > [1]
> > https://lore.kernel.org/netdev/20250922110622.10368-1-mmietus97@yahoo.com/
> > >
> >
> > As previously discussed with Jakub [2], tunnels that use
> > udp_tunnel_dst_lookup
> > add notable complexity because the returned dst could either be from
> > ip_route_output_key (referenced) or from the dst_cache (which I'm changing
> > to
> > be noref). There are also other tunnels that follow a similar pattern.

But IMO Jakub's comment about technical debt is not addressed by
pushing dstref all over the tunnel code.


> > The cleanest way to keep track of which dst is referenced and which isn't
> > is to borrow existing refdst concepts. This allows us to more easily track
> > the ref state of dst_entries in later flows to avoid unnecessarily taking
> > a reference. I played around with a couple implementations and this turned
> > out to be the most elegant. It's a big change, but it's mostly semantic.
> >
> > [2] https://lore.kernel.org/netdev/20250923184856.6cce6530@kernel.org/
> 
> 
> I have not seen the series, so I had to go to the archives.
> 
> Too much code churn for my taste, and a true nightmare for future backports
> to stable kernels.
> 
> Unless I am mistaken, this is your first submission to the linux kernel,
> please start with more manageable patches.

-- 
Sabrina