RE: [net-next 1/3] net: dsa: optimize tx timestamp request handling

From: Y.b. Lu
Date: Tue Apr 20 2021 - 03:39:48 EST


> -----Original Message-----
> From: Richard Cochran <richardcochran@xxxxxxxxx>
> Sent: 2021年4月18日 23:11
> To: Vladimir Oltean <olteanv@xxxxxxxxx>
> Cc: Y.b. Lu <yangbo.lu@xxxxxxx>; netdev@xxxxxxxxxxxxxxx; Vladimir Oltean
> <vladimir.oltean@xxxxxxx>; David S . Miller <davem@xxxxxxxxxxxxx>; Jakub
> Kicinski <kuba@xxxxxxxxxx>; Jonathan Corbet <corbet@xxxxxxx>; Kurt
> Kanzenbach <kurt@xxxxxxxxxxxxx>; Andrew Lunn <andrew@xxxxxxx>; Vivien
> Didelot <vivien.didelot@xxxxxxxxx>; Florian Fainelli <f.fainelli@xxxxxxxxx>;
> Claudiu Manoil <claudiu.manoil@xxxxxxx>; Alexandre Belloni
> <alexandre.belloni@xxxxxxxxxxx>; UNGLinuxDriver@xxxxxxxxxxxxx;
> linux-doc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [net-next 1/3] net: dsa: optimize tx timestamp request handling
>
> On Sun, Apr 18, 2021 at 12:18:42PM +0300, Vladimir Oltean wrote:
> >
> > How about not passing "clone" back to DSA as an argument by reference,
> > but instead require the driver to populate DSA_SKB_CB(skb)->clone if
> > it needs to do so?
> >
> > Also, how about changing the return type to void? Returning true or
> > false makes no difference.

Thank you. That's good idea.
And how about letting driver store the skb clone pointer, or not? I copied my comments in patch #3 here,

" Can we totally drop dsa_skb_cb in dsa core? The only usage of it is holding a skb clone pointer, for only felix and sja1105.
Actually we can move such pointer in <device>_skb_cb, instead of reserving the space of skb for any drivers."

>
> +1
>
> Thanks,
> Richard