Date:   Mon, 19 Jul 2021 10:41:48 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Florian Fainelli <f.fainelli@...il.com>
Cc:     Vladimir Oltean <vladimir.oltean@....com>, netdev@...r.kernel.org,
        Jakub Kicinski <kuba@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Andrew Lunn <andrew@...n.ch>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Ido Schimmel <idosch@...sch.org>,
        Tobias Waldekranz <tobias@...dekranz.com>,
        Roopa Prabhu <roopa@...dia.com>,
        Nikolay Aleksandrov <nikolay@...dia.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        bridge@...ts.linux-foundation.org,
        Grygorii Strashko <grygorii.strashko@...com>,
        Marek Behun <kabel@...ckhole.sk>,
        DENG Qingfang <dqfext@...il.com>
Subject: Re: [PATCH v4 net-next 15/15] net: dsa: tag_dsa: offload the bridge
 forwarding process

On Sun, Jul 18, 2021 at 07:47:22PM -0700, Florian Fainelli wrote:
> On 7/18/2021 2:44 PM, Vladimir Oltean wrote:
> > From: Tobias Waldekranz <tobias@...dekranz.com>
> >
> > Allow the DSA tagger to generate FORWARD frames for offloaded skbs
> > sent from a bridge that we offload, allowing the switch to handle any
> > frame replication that may be required. This also means that source
> > address learning takes place on packets sent from the CPU, meaning
> > that return traffic no longer needs to be flooded as unknown unicast.
> >
> > Signed-off-by: Tobias Waldekranz <tobias@...dekranz.com>
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@....com>
>
> This looks pretty complicated to but if this is how it has to work, it has
> to. For tag_brcm.c we can simply indicate that the frame to be transmitted
> should have a specific bitmask of egress ports.

Complicated in the sense that we need to nail the VLAN ID so that
the FDB / MDB is looked up correctly by the accelerator, to ensure that
it produces a result that is in sync with the software tables?

What you are proposing is not really TX forwarding offload but TX
replication offload. A CPU-injected packet targeting multiple egress
ports is still a control plane packet nonetheless, with all features
that characterize one:
- Ingress stage of the CPU port is bypassed (no hardware address
  learning for that MAC SA)
- FDB lookup is bypassed (we trust the software). This is also perhaps
  an advantage, because for example, if we have a MAC address learned
  towards the CPU port, and then we inject a packet from the CPU towards
  that destination MAC address, then a data plane packet would be
  dropped due to source port pruning (source == destination port), but a
  control plane packet would be sent regardless.
- Can inject into a BLOCKING egress port (we trust the software not to
  do that)

Whereas this patch set is really about laying the ground for data plane
packets to be safely created and sent by the network stack. There are
switches which have a clear distinction between the control plane and
the data plane, and injecting a control packet is a fairly expensive
operation. So it would be very good to support this operating mode,
regardless of whatever else we do.

I can look into adding support for your use case with just the
replication offload, since it should be possible nonetheless, and if you
really don't have the option to send a data plane packet then it is a
valid approach too, however I believe that the brick wall will be where
to encode the destination bit mask in the egress skb. For the full TX
forwarding offload we managed to dodge that because we already had
skb->offload_fwd_mark, but that's just one bit and we would need more.
I'm thinking we would need to add another bit (skb->offload_tx_replication)
and then add a struct list_head tx_dev to the skb which contains all the
net devices that the packet was not cloned to?

Powered by blists - more mailing lists