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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AM7PR04MB6885CBAB63363CA93E50A207F8419@AM7PR04MB6885.eurprd04.prod.outlook.com>
Date:   Tue, 27 Apr 2021 04:13:36 +0000
From:   "Y.b. Lu" <yangbo.lu@....com>
To:     Vladimir Oltean <olteanv@...il.com>,
        Florian Fainelli <f.fainelli@...il.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Richard Cochran <richardcochran@...il.com>,
        Vladimir Oltean <vladimir.oltean@....com>,
        "David S . Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jonathan Corbet <corbet@....net>,
        Kurt Kanzenbach <kurt@...utronix.de>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [net-next, v2, 3/7] net: dsa: free skb->cb usage in core driver



> -----Original Message-----
> From: Vladimir Oltean <olteanv@...il.com>
> Sent: 2021年4月27日 2:17
> To: Y.b. Lu <yangbo.lu@....com>
> Cc: netdev@...r.kernel.org; Richard Cochran <richardcochran@...il.com>;
> Vladimir Oltean <vladimir.oltean@....com>; David S . Miller
> <davem@...emloft.net>; Jakub Kicinski <kuba@...nel.org>; Jonathan Corbet
> <corbet@....net>; Kurt Kanzenbach <kurt@...utronix.de>; Andrew Lunn
> <andrew@...n.ch>; Vivien Didelot <vivien.didelot@...il.com>; Florian
> Fainelli <f.fainelli@...il.com>; Claudiu Manoil <claudiu.manoil@....com>;
> Alexandre Belloni <alexandre.belloni@...tlin.com>;
> UNGLinuxDriver@...rochip.com; linux-doc@...r.kernel.org;
> linux-kernel@...r.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@....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.