On Fri, May 07, 2021 at 11:26:32AM +0000, Y.b. Lu wrote: > > From: Tobias Waldekranz <tobias@xxxxxxxxxxxxxx> > > On Mon, Apr 26, 2021 at 17:37, Yangbo Lu <yangbo.lu@xxxxxxx> wrote: > > > 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? I would like to avoid overengineering, which a callback for skb->cb initialization would introduce, given the needs we have now. FWIW, we may not even be able to touch skb->cb in .ndo_select_queue for Tobias's use case, that discussion is here: https://patchwork.kernel.org/project/netdevbpf/patch/20210426170411.1789186-7-tobias@xxxxxxxxxxxxxx/
