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-stepCould 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>