lkml.org 
[lkml]   [2021]   [Apr]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRE: [net-next, v2, 3/7] net: dsa: free skb->cb usage in core driver
Date


> -----Original Message-----
> From: Vladimir Oltean <olteanv@gmail.com>
> Sent: 2021年4月27日 2:17
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; Richard Cochran <richardcochran@gmail.com>;
> Vladimir Oltean <vladimir.oltean@nxp.com>; David S . Miller
> <davem@davemloft.net>; Jakub Kicinski <kuba@kernel.org>; Jonathan Corbet
> <corbet@lwn.net>; Kurt Kanzenbach <kurt@linutronix.de>; Andrew Lunn
> <andrew@lunn.ch>; Vivien Didelot <vivien.didelot@gmail.com>; Florian
> Fainelli <f.fainelli@gmail.com>; Claudiu Manoil <claudiu.manoil@nxp.com>;
> Alexandre Belloni <alexandre.belloni@bootlin.com>;
> 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 05:37:58PM +0800, Yangbo Lu 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
> > 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>
> > ---
> > Changes for v2:
> > - Added this patch.
> > ---
> > drivers/net/dsa/ocelot/felix.c | 1 +
> > drivers/net/dsa/sja1105/sja1105_main.c | 2 +-
> > drivers/net/dsa/sja1105/sja1105_ptp.c | 4 +++-
> > drivers/net/ethernet/mscc/ocelot.c | 6 +++---
> > drivers/net/ethernet/mscc/ocelot_net.c | 2 +-
> > include/linux/dsa/sja1105.h | 3 ++-
> > include/net/dsa.h | 14 --------------
> > include/soc/mscc/ocelot.h | 8 ++++++++
> > net/dsa/slave.c | 3 +--
> > net/dsa/tag_ocelot.c | 8 ++++----
> > net/dsa/tag_ocelot_8021q.c | 8 ++++----
> > 11 files changed, 28 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/net/dsa/ocelot/felix.c
> > b/drivers/net/dsa/ocelot/felix.c index d679f023dc00..8980d56ee793
> > 100644
> > --- a/drivers/net/dsa/ocelot/felix.c
> > +++ b/drivers/net/dsa/ocelot/felix.c
> > @@ -1403,6 +1403,7 @@ static bool felix_txtstamp(struct dsa_switch
> > *ds, int port,
> >
> > if (ocelot->ptp && ocelot_port->ptp_cmd ==
> IFH_REW_OP_TWO_STEP_PTP) {
> > ocelot_port_add_txtstamp_skb(ocelot, port, clone);
> > + OCELOT_SKB_CB(skb)->clone = clone;
> > return true;
> > }
> >
>
> Uh-oh, this patch fails to build:
>
> In file included from ./include/soc/mscc/ocelot_vcap.h:9:0,
> from drivers/net/dsa/ocelot/felix.c:9:
> drivers/net/dsa/ocelot/felix.c: In function ‘felix_txtstamp’:
> drivers/net/dsa/ocelot/felix.c:1406:17: error: ‘skb’ undeclared (first use in
> this function)
> OCELOT_SKB_CB(skb)->clone = clone;
> ^
> ./include/soc/mscc/ocelot.h:698:29: note: in definition of macro
> ‘OCELOT_SKB_CB’
> ((struct ocelot_skb_cb *)((skb)->cb))
> ^~~
> drivers/net/dsa/ocelot/felix.c:1406:17: note: each undeclared identifier is
> reported only once for each function it appears in
> OCELOT_SKB_CB(skb)->clone = clone;
> ^
> ./include/soc/mscc/ocelot.h:698:29: note: in definition of macro
> ‘OCELOT_SKB_CB’
> ((struct ocelot_skb_cb *)((skb)->cb))
> ^~~
>
> It depends on changes made in patch 3.

Oh.. That's a sequence problem.
I switched patch #3 and patch #4 with rebasing to fix up in v3 version.
Thanks.
\
 
 \ /
  Last update: 2021-04-27 06:15    [W:0.126 / U:5.424 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site