Thread (21 messages) 21 messages, 6 authors, 2021-05-07

RE: [net-next, v2, 3/7] net: dsa: free skb->cb usage in core driver

From: "Y.b. Lu" <yangbo.lu@nxp.com>
Date: 2021-05-07 11:26:47
Also in: lkml, netdev

-----Original Message-----
From: Tobias Waldekranz <tobias@waldekranz.com>
Sent: 2021年4月29日 4:30
To: Y.b. Lu <yangbo.lu@nxp.com>; netdev@vger.kernel.org
Cc: Y.b. Lu <yangbo.lu@nxp.com>; Richard Cochran
[off-list ref]; Vladimir Oltean [off-list ref];
David S . Miller [off-list ref]; Jakub Kicinski [off-list ref];
Jonathan Corbet [off-list ref]; Kurt Kanzenbach [off-list ref];
Andrew Lunn [off-list ref]; Vivien Didelot [off-list ref];
Florian Fainelli [off-list ref]; Claudiu Manoil
[off-list ref]; Alexandre Belloni
[off-list ref]; UNGLinuxDriver@microchip.com;
linux-doc@vger.kernel.org; linux-kernel@vger.kernel.org
Subject: Re: [net-next, v2, 3/7] net: dsa: free skb->cb usage in core driver

On Mon, Apr 26, 2021 at 17:37, Yangbo Lu [off-list ref] wrote:
quoted
Free skb->cb usage in core driver and let device drivers decide to use
or not. The reason having a DSA_SKB_CB(skb)->clone was because
dsa_skb_tx_timestamp() which may set the clone pointer was called
before p->xmit() which would use the clone if any, and the device
driver has no way to initialize the clone pointer.

Although for now putting memset(skb->cb, 0, 48) at beginning of
dsa_slave_xmit() by this patch is not very good, there is still way to
improve this. Otherwise, some other new features, like one-step
Could you please expand on this improvement?

This memset makes it impossible to carry control buffer information from
driver callbacks that run before .ndo_start_xmit, for
example .ndo_select_queue, to a tagger's .xmit.

It seems to me that if the drivers are to handle the CB internally from now on,
that should go for both setting and clearing of the required fields.
For the timestamping case, dsa_skb_tx_timestamp may not touch .port_txtstamp callback, so we had to put skb->cb initialization at beginning of dsa_slave_xmit.
To avoid breaking future skb->cb usage you mentioned, do you think we can back to Vladimir's idea initializing only field required, or even just add a callback for cb initialization for timestamping?

Thanks.

quoted
timestamp which needs a flag of skb marked in dsa_skb_tx_timestamp(),
and handles as one-step timestamp in p->xmit() will face same
situation.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help