> -----Original Message-----
> From: Vladimir Oltean <olteanv@xxxxxxxxx>
> Sent: 2021年4月27日 2:17
> To: Y.b. Lu <yangbo.lu@xxxxxxx>
> Cc: netdev@xxxxxxxxxxxxxxx; Richard Cochran <richardcochran@xxxxxxxxx>;
> 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, 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@xxxxxxx>
> > ---
> > 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.
