All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vladimir Oltean <olteanv@gmail.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Vladimir Oltean <vladimir.oltean@nxp.com>,
	netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Jiri Pirko <jiri@resnulli.us>, Ido Schimmel <idosch@idosch.org>,
	Tobias Waldekranz <tobias@waldekranz.com>,
	Roopa Prabhu <roopa@nvidia.com>,
	Nikolay Aleksandrov <nikolay@nvidia.com>,
	Stephen Hemminger <stephen@networkplumber.org>,
	bridge@lists.linux-foundation.org,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	Marek Behun <kabel@blackhole.sk>,
	DENG Qingfang <dqfext@gmail.com>
Subject: Re: [PATCH v4 net-next 15/15] net: dsa: tag_dsa: offload the bridge forwarding process
Date: Mon, 19 Jul 2021 10:41:48 +0300	[thread overview]
Message-ID: <20210719074148.xlm7syfm76fuzsxy@skbuf> (raw)
In-Reply-To: <7c2b81e8-db72-4665-fe81-7254cba1e797@gmail.com>

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@waldekranz.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@waldekranz.com>
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.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?

WARNING: multiple messages have this Message-ID (diff)
From: Vladimir Oltean <olteanv@gmail.com>
To: Florian Fainelli <f.fainelli@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	bridge@lists.linux-foundation.org, Jiri Pirko <jiri@resnulli.us>,
	DENG Qingfang <dqfext@gmail.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Roopa Prabhu <roopa@nvidia.com>,
	"David S. Miller" <davem@davemloft.net>,
	Ido Schimmel <idosch@idosch.org>,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	Nikolay Aleksandrov <nikolay@nvidia.com>,
	netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	Marek Behun <kabel@blackhole.sk>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Tobias Waldekranz <tobias@waldekranz.com>
Subject: Re: [Bridge] [PATCH v4 net-next 15/15] net: dsa: tag_dsa: offload the bridge forwarding process
Date: Mon, 19 Jul 2021 10:41:48 +0300	[thread overview]
Message-ID: <20210719074148.xlm7syfm76fuzsxy@skbuf> (raw)
In-Reply-To: <7c2b81e8-db72-4665-fe81-7254cba1e797@gmail.com>

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@waldekranz.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@waldekranz.com>
> > Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.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?

  reply	other threads:[~2021-07-19  7:41 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-18 21:44 [PATCH v4 net-next 00/15] Allow forwarding for the software bridge data path to be offloaded to capable devices Vladimir Oltean
2021-07-18 21:44 ` [Bridge] " Vladimir Oltean
2021-07-18 21:44 ` [PATCH v4 net-next 01/15] net: dpaa2-switch: use extack in dpaa2_switch_port_bridge_join Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  9:17   ` Ioana Ciornei
2021-07-19  9:17     ` [Bridge] " Ioana Ciornei
2021-07-18 21:44 ` [PATCH v4 net-next 02/15] net: dpaa2-switch: refactor prechangeupper sanity checks Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  9:18   ` Ioana Ciornei
2021-07-19  9:18     ` [Bridge] " Ioana Ciornei
2021-07-18 21:44 ` [PATCH v4 net-next 03/15] mlxsw: spectrum: " Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-18 21:44 ` [PATCH v4 net-next 04/15] mlxsw: spectrum: refactor leaving an 8021q upper that is a bridge port Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  2:16   ` Florian Fainelli
2021-07-19  2:16     ` [Bridge] " Florian Fainelli
2021-07-18 21:44 ` [PATCH v4 net-next 05/15] net: marvell: prestera: refactor prechangeupper sanity checks Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  2:20   ` Florian Fainelli
2021-07-19  2:20     ` [Bridge] " Florian Fainelli
2021-07-18 21:44 ` [PATCH v4 net-next 06/15] net: switchdev: guard drivers against multiple obj replays on same bridge port Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  2:17   ` Florian Fainelli
2021-07-19  2:17     ` [Bridge] " Florian Fainelli
2021-07-18 21:44 ` [PATCH v4 net-next 07/15] net: bridge: disambiguate offload_fwd_mark Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  2:26   ` Florian Fainelli
2021-07-19  2:26     ` [Bridge] " Florian Fainelli
2021-07-18 21:44 ` [PATCH v4 net-next 08/15] net: bridge: switchdev: recycle unused hwdoms Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-18 21:44 ` [PATCH v4 net-next 09/15] net: bridge: switchdev: let drivers inform which bridge ports are offloaded Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  9:23   ` Ioana Ciornei
2021-07-19  9:23     ` [Bridge] " Ioana Ciornei
2021-07-20  7:53   ` Horatiu Vultur
2021-07-20  7:53     ` [Bridge] " Horatiu Vultur
2021-07-20  8:45     ` Vladimir Oltean
2021-07-20  8:45       ` [Bridge] " Vladimir Oltean
2021-07-18 21:44 ` [PATCH v4 net-next 10/15] net: bridge: switchdev object replay helpers for everybody Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  8:19   ` Vladimir Oltean
2021-07-19  8:19     ` [Bridge] " Vladimir Oltean
2021-07-19  9:26   ` Ioana Ciornei
2021-07-19  9:26     ` [Bridge] " Ioana Ciornei
2021-07-18 21:44 ` [PATCH v4 net-next 11/15] net: bridge: switchdev: allow the TX data plane forwarding to be offloaded Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  2:43   ` Florian Fainelli
2021-07-19  2:43     ` [Bridge] " Florian Fainelli
2021-07-19  7:22   ` Vladimir Oltean
2021-07-19  7:22     ` [Bridge] " Vladimir Oltean
2021-07-18 21:44 ` [PATCH v4 net-next 12/15] net: dsa: track the number of switches in a tree Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  2:54   ` Florian Fainelli
2021-07-19  2:54     ` [Bridge] " Florian Fainelli
2021-07-18 21:44 ` [PATCH v4 net-next 13/15] net: dsa: add support for bridge TX forwarding offload Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  2:51   ` Florian Fainelli
2021-07-19  2:51     ` [Bridge] " Florian Fainelli
2021-07-18 21:44 ` [PATCH v4 net-next 14/15] net: dsa: mv88e6xxx: map virtual bridges with forwarding offload in the PVT Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  2:52   ` Florian Fainelli
2021-07-19  2:52     ` [Bridge] " Florian Fainelli
2021-07-18 21:44 ` [PATCH v4 net-next 15/15] net: dsa: tag_dsa: offload the bridge forwarding process Vladimir Oltean
2021-07-18 21:44   ` [Bridge] " Vladimir Oltean
2021-07-19  2:47   ` Florian Fainelli
2021-07-19  2:47     ` [Bridge] " Florian Fainelli
2021-07-19  7:41     ` Vladimir Oltean [this message]
2021-07-19  7:41       ` Vladimir Oltean
2021-07-20 11:24 ` [PATCH v4 net-next 00/15] Allow forwarding for the software bridge data path to be offloaded to capable devices Ido Schimmel
2021-07-20 11:24   ` [Bridge] " Ido Schimmel
2021-07-20 13:20   ` Vladimir Oltean
2021-07-20 13:20     ` [Bridge] " Vladimir Oltean
2021-07-20 13:51     ` Ido Schimmel
2021-07-20 13:51       ` [Bridge] " Ido Schimmel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210719074148.xlm7syfm76fuzsxy@skbuf \
    --to=olteanv@gmail.com \
    --cc=andrew@lunn.ch \
    --cc=bridge@lists.linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=dqfext@gmail.com \
    --cc=f.fainelli@gmail.com \
    --cc=grygorii.strashko@ti.com \
    --cc=idosch@idosch.org \
    --cc=jiri@resnulli.us \
    --cc=kabel@blackhole.sk \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@nvidia.com \
    --cc=roopa@nvidia.com \
    --cc=stephen@networkplumber.org \
    --cc=tobias@waldekranz.com \
    --cc=vivien.didelot@gmail.com \
    --cc=vladimir.oltean@nxp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.