netdev - Re: [PATCH net-next v2 2/5] net: dsa: add out-of-band tagging protocol

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: <20220519145221.odisjsmjojrpuutp@skbuf>
Date:   Thu, 19 May 2022 14:52:22 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Maxime Chevallier <maxime.chevallier@...tlin.com>
CC:     "davem@...emloft.net" <davem@...emloft.net>,
        Rob Herring <robh+dt@...nel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "thomas.petazzoni@...tlin.com" <thomas.petazzoni@...tlin.com>,
        Andrew Lunn <andrew@...n.ch>,
        Florian Fainelli <f.fainelli@...il.com>,
        Heiner Kallweit <hkallweit1@...il.com>,
        Russell King <linux@...linux.org.uk>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        Luka Perkov <luka.perkov@...tura.hr>,
        Robert Marko <robert.marko@...tura.hr>
Subject: Re: [PATCH net-next v2 2/5] net: dsa: add out-of-band tagging
 protocol

On Tue, May 17, 2022 at 09:01:56AM +0200, Maxime Chevallier wrote:
> Hi Vlad,
> 
> On Sat, 14 May 2022 22:40:03 +0000
> Vladimir Oltean <vladimir.oltean@....com> wrote:
> 
> > On Sat, May 14, 2022 at 05:06:53PM +0200, Maxime Chevallier wrote:
> > > This tagging protocol is designed for the situation where the link
> > > between the MAC and the Switch is designed such that the Destination
> > > Port, which is usually embedded in some part of the Ethernet
> > > Header, is sent out-of-band, and isn't present at all in the
> > > Ethernet frame.
> > > 
> > > This can happen when the MAC and Switch are tightly integrated on an
> > > SoC, as is the case with the Qualcomm IPQ4019 for example, where
> > > the DSA tag is inserted directly into the DMA descriptors. In that
> > > case, the MAC driver is responsible for sending the tag to the
> > > switch using the out-of-band medium. To do so, the MAC driver needs
> > > to have the information of the destination port for that skb.
> > > 
> > > This out-of-band tagging protocol is using the very beggining of
> > > the skb headroom to store the tag. The drawback of this approch is
> > > that the headroom isn't initialized upon allocating it, therefore
> > > we have a chance that the garbage data that lies there at
> > > allocation time actually ressembles a valid oob tag. This is only
> > > problematic if we are sending/receiving traffic on the master port,
> > > which isn't a valid DSA use-case from the beggining. When dealing
> > > from traffic to/from a slave port, then the oob tag will be
> > > initialized properly by the tagger or the mac driver through the
> > > use of the dsa_oob_tag_push() call.
> > > 
> > > Signed-off-by: Maxime Chevallier <maxime.chevallier@...tlin.com>
> > > ---  
> > 
> > Why put the DSA pseudo-header at skb->head rather than push it using
> > skb_push()? I thought you were going to check for the presence of a
> > DSA header using something like skb->mac_len == ETH_HLEN + tag len,
> > but right now it sounds like treating garbage in the headroom as a
> > valid DSA tag is indeed a potential problem. If you can't sort that
> > out using information from the header offsets alone, maybe an skb
> > extension is required?
> 
> Indeed, I thought of that, the main reason is that pushing/poping in
> itself is not enough, you also have to move the whole mac_header to
> leave room for the tag, and then re-set it in it's original location.
> There's nothing wrong with this, but it looked a bit cumbersome just to
> insert a dummy tag that gets removed rightaway. Does that make sense ?

You're thinking about inserting a header before the EtherType. But what
has been said was to _prepend_ a header, i.e. put it before the Ethernet
MAC DA. That way you don't need to move the Ethernet header.

But anyway, too much talk for mostly nothing, see below.

> But yes I would really like to get a way to know wether the tag is
> there or not, I'll dig a bit more to see if I can find a way to get
> this info from the various skb offsets in a reliable way.

Without an skb extension, this seems like an impossible task to me
(which should also answer Florian's request for feedback on the proposal
to share skb->cb with GRO, the qdisc, and whomever else there might be
in the path between the DSA master and the switch).