Message-ID: <20221115115023.hgc4ynrx3kylf6p3@skbuf>
Date:   Tue, 15 Nov 2022 11:50:23 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Maxime Chevallier <maxime.chevallier@...tlin.com>
CC:     Felix Fietkau <nbd@....name>, Jakub Kicinski <kuba@...nel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Eric Dumazet <edumazet@...gle.com>,
        Paolo Abeni <pabeni@...hat.com>,
        "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>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>
Subject: Re: [PATCH net-next v8 3/5] net: dsa: add out-of-band tagging
 protocol

On Tue, Nov 15, 2022 at 10:29:24AM +0100, Maxime Chevallier wrote:
> Hello everyone,
> 
> Felix, thanks for the feedback !
> 
> On Tue, 8 Nov 2022 13:22:17 +0100
> Felix Fietkau <nbd@....name> wrote:
> 
> [...]
> 
> > FYI, I'm currently working on hardware DSA untagging on the mediatek
> > mtk_eth_soc driver. On this hardware, I definitely need to keep the
> > custom DSA tag driver, as hardware untagging is not always available.
> > For the receive side, I came up with this patch (still untested) for
> > using METADATA_HW_PORT_MUX.
> > It has the advantage of being able to skip the tag protocol rcv ops
> > call for offload-enabled packets.
> > 
> > Maybe for the transmit side we could have some kind of netdev feature
> > or capability that indicates offload support and allows skipping the
> > tag xmit function as well.
> > In that case, ipqess could simply use a no-op tag driver.
> 
> If I'm not mistaken, Florian also proposed a while ago an offload
> mechanism for taggin/untagging :
> 
> https://lore.kernel.org/lkml/1438322920.20182.144.camel@edumazet-glaptop2.roam.corp.google.com/T/
> 
> It uses some of the points you're mentionning, such as the netdev
> feature :)
> 
> All in all, I'm still a bit confused about the next steps. If I can
> summarize a bit, we have a lot of approaches, all with advantages and
> inconvenients, I'll try to summarize the state :
> 
>  - We could simply use the skb extensions as-is, rename the tagger
>    something like "DSA_TAG_IPQDMA" and consider this a way to perform
>    tagging on this specific class of hardware, without trying too hard
>    to make it generic.

For Felix, using skb extensions would be inconvenient, since it would
involve per packet allocations which are now avoided with the metadata
dsts.

>  - We could try to move forward with this mechanism of offloading
>    tagging and untagging from the MAC driver, this would address
>    Florian's first try at this, Felix's use-case and would fit well the
>    IPQESS case

Someone would need to take things from where Felix left them:
https://patchwork.kernel.org/project/netdevbpf/patch/20221114124214.58199-2-nbd@nbd.name/
and add TX tag offloading support as well. Here there would need to be
a mechanism through which DSA asks "hey, this is my tagging protocol,
can the master offload it in the TX direction or am I just going to push
the tag into the packet?". I tried to sketch here something along those
lines:
https://patchwork.kernel.org/project/netdevbpf/patch/20221109163426.76164-10-nbd@nbd.name/#25084481

>  - There's the option discussed by Vlad and Jakub to add several
>    frontends, one being a switchev driver, here I'm a bit lost TBH, if
>    we go this way I could definitely use a few pointers from Vlad :)

The assumption being here that there is more functionality to cover by
the metadata dst than a port mux. I'm really not clear what is the
hardware design truly, hopefully you could give more details about that.

The mechanism is quite simple, it's not rocket science. Take something
like a bridge join operation, the proposal is to do something like this:

    dsa_slave_netdevice_event
        (net/dsa/slave.c)
               |
               v
      dsa_slave_changeupper
       (net/dsa/slave.c)
               |
               v
       dsa_port_bridge_join                         ocelot_netdevice_event
        (net/dsa/port.c)                  (drivers/net/ethernet/mscc/ocelot_net.c)
               |                                           |
               v                                           v
     dsa_switch_bridge_join                     ocelot_netdevice_changeupper
       (net/dsa/switch.c)                 (drivers/net/ethernet/mscc/ocelot_net.c)
               |                                           |
               v                                           v
       felix_bridge_join                        ocelot_netdevice_bridge_join
(drivers/net/dsa/ocelot/felix.c)          (drivers/net/ethernet/mscc/ocelot_net.c)
               |                                           |
               |                                           |
               +---------------------+---------------------+
                                     |
                                     v
                           ocelot_port_bridge_join
                      (drivers/net/ethernet/mscc/ocelot.c)

with you maintaining the entire right branch that represents the switchdev frontend,
and more or less duplicates part of DSA.

The advantage of this approach is that you can register your own NAPI
handler where you can treat packets in whichever way you like, and have
your own ndo_start_xmit. This driver would treat the aggregate of the
ess DMA engine and the ipq switch as a single device, and expose it as a
switch with DMA, basically.

Powered by blists - more mailing lists