lkml.org 
[lkml]   [2022]   [May]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
Date
SubjectRe: [PATCH v3 3/4] can: skb:: move can_dropped_invalid_skb and can_skb_headroom_valid to skb.c
Hi Oliver,

On Mon. 16 May 2022 at 04:17, Oliver Hartkopp <socketcan@hartkopp.net> wrote:
> Hi Vincent,
>
> On 14.05.22 16:16, Vincent Mailhol wrote:
> > The functions can_dropped_invalid_skb() and can_skb_headroom_valid()
> > grew a lot over the years to a point which it does not make much sense
> > to have them defined as static inline in header files. Move those two
> > functions to the .c counterpart of skb.h.
> >
> > can_skb_headroom_valid() only caller being can_dropped_invalid_skb(),
> > the declaration is removed from the header. Only
> > can_dropped_invalid_skb() gets its symbol exported.
>
> I can see your point but the need for can-dev was always given for
> hardware specific stuff like bitrates, TDC, transceivers, whatever.

I also see your point :)
Actually, I raised the exact same idea in a previous message:
https://lore.kernel.org/linux-can/CAMZ6RqLU-Wg0Cau3cM=QsU-t-7Lyzmo1nJ_VAA4Mbw3u0jnNtw@mail.gmail.com/

But you were not in CC and it seems that there is a lot of congestion
recently on the mailing list so I wouldn’t be surprised if you tell me
you did not receive it.

> As there would be more things in slcan (e.g. dev_alloc_skb() could be
> unified with alloc_can_skb())

And also the can_{put,get}_echo_skb() I guess.

> would it probably make sense to
> introduce a new can-skb module that could be used by all CAN
> virtual/software interfaces?
>
> Or some other split-up ... any idea?

My concern is: what would be the merrit? If we do not split, the users
of slcan and v(x)can would have to load the can-dev module which will
be slightly bloated for their use, but is this really an issue? I do
not see how this can become a performance bottleneck, so what is the
problem?
I could also argue that most of the devices do not depend on
rx-offload.o. So should we also split this one out of can-dev on the
same basis and add another module dependency?
The benefit (not having to load a bloated module for three drivers)
does not outweigh the added complexity: all hardware modules will have
one additional modprobe dependency on the tiny can-skb module.

But as said above, I am not fully opposed to the split, I am just
strongly divided. If we go for the split, creating a can-skb module is
the natural and only option I see.
If the above argument does not convince you, I will send a v3 with that split.


Yours sincerely,
Vincent Mailhol

\
 
 \ /
  Last update: 2022-05-17 03:51    [W:0.145 / U:0.472 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site