Hi Oliver, On Mon. 16 May 2022 at 04:17, Oliver Hartkopp <socketcan@xxxxxxxxxxxx> 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@xxxxxxxxxxxxxx/ 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
