Re: [PATCH v2 1/2] net: extend drop reasons for multiple subsystems

[Jakub Kicinski <kuba@kernel.org>]

On Fri, 14 Apr 2023 11:25:08 +0200 Johannes Berg wrote:
> On Fri, 2023-03-31 at 21:36 -0700, Jakub Kicinski wrote:
> >   
> > > +/* Note: due to dynamic registrations, access must be under RCU */
> > > +extern const struct drop_reason_list __rcu *
> > > +drop_reasons_by_subsys[SKB_DROP_REASON_SUBSYS_NUM];
> > > +
> > > +void drop_reasons_register_subsys(enum skb_drop_reason_subsys subsys,
> > > +				  const struct drop_reason_list *list);
> > > +void drop_reasons_unregister_subsys(enum skb_drop_reason_subsys subsys);  
> > 
> > dropreason.h is included by skbuff.h because history, but I don't think
> > any of the new stuff must be visible in skbuff.h.
> > 
> > Could you make a new header, and put as much of this stuff there as
> > possible? Our future selves will thank us for shorter rebuild times..  
> 
> Sure. Not sure it'll make a big difference in rebuild, but we'll see :)
> 
> I ended up moving dropreason.h to dropreason-core.h first, that way we
> also have a naming scheme for non-core dropreason files should they
> become visible outside of the subsystem (i.e. mac80211 just has them
> internally).
> 
> Dunno, let me know if you prefer something else, I just couldn't come up
> with a non-confusing longer name for the new thing.

Sounds good.

> > Weak preference to also take the code out of skbuff.c but that's not as
> > important.  
> 
> I guess I can create a new dropreason.c, but is that worth it? It's only
> a few lines. Let me know, then I can resend.

It's hard to tell. Most additions to the core are small at the start so
we end up chucking all of them into a handful of existing source files.
And those files grow and grow. Splitting the later is extra work and
makes backports harder.

It's a game of predicting which code will likely grow into a reasonable
~500+ LoC at some point, and which code will not. I have the feeling
that dropreason code will grow. But yes, it's still fairly small, we 
can defer.

> > You To'd both wireless and netdev, who are you expecting to apply this?
> > :S  
> 
> Good question :)
> 
> The first patch (patches in v3) really should go through net-next I
> suppose, and I wouldn't mind if the other one did as well, it doesn't
> right now touch anything likely to change.

SG!