Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Ioana Ciornei <ioana.ciornei@nxp.com>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
	Jakub Kicinski <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	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" 
	<bridge@lists.linux-foundation.org>,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	Marek Behun <kabel@blackhole.sk>,
	DENG Qingfang <dqfext@gmail.com>,
	Vadym Kochan <vkochan@marvell.com>,
	Taras Chornyi <tchornyi@marvell.com>,
	Lars Povlsen <lars.povlsen@microchip.com>,
	Steen Hegelund <Steen.Hegelund@microchip.com>,
	"UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>
Subject: Re: [PATCH v4 net-next 10/15] net: bridge: switchdev object replay helpers for everybody
Date: Mon, 19 Jul 2021 09:26:26 +0000	[thread overview]
Message-ID: <20210719092625.tnbgghblfqiwtrwl@skbuf> (raw)
In-Reply-To: <20210718214434.3938850-11-vladimir.oltean@nxp.com>

On Mon, Jul 19, 2021 at 12:44:29AM +0300, Vladimir Oltean wrote:
> Starting with commit 4f2673b3a2b6 ("net: bridge: add helper to replay
> port and host-joined mdb entries"), DSA has introduced some bridge
> helpers that replay switchdev events (FDB/MDB/VLAN additions and
> deletions) that can be lost by the switchdev drivers in a variety of
> circumstances:
> 
> - an IP multicast group was host-joined on the bridge itself before any
>   switchdev port joined the bridge, leading to the host MDB entries
>   missing in the hardware database.
> - during the bridge creation process, the MAC address of the bridge was
>   added to the FDB as an entry pointing towards the bridge device
>   itself, but with no switchdev ports being part of the bridge yet, this
>   local FDB entry would remain unknown to the switchdev hardware
>   database.
> - a VLAN/FDB/MDB was added to a bridge port that is a LAG interface,
>   before any switchdev port joined that LAG, leading to the hardware
>   database missing those entries.
> - a switchdev port left a LAG that is a bridge port, while the LAG
>   remained part of the bridge, and all FDB/MDB/VLAN entries remained
>   installed in the hardware database of the switchdev port.
> 
> Also, since commit 0d2cfbd41c4a ("net: bridge: ignore switchdev events
> for LAG ports which didn't request replay"), DSA introduced a method,
> based on a const void *ctx, to ensure that two switchdev ports under the
> same LAG that is a bridge port do not see the same MDB/VLAN entry being
> replayed twice by the bridge, once for every bridge port that joins the
> LAG.
> 
> With so many ordering corner cases being possible, it seems unreasonable
> to expect a switchdev driver writer to get it right from the first try.
> Therefore, now that we are past the beta testing period for the bridge
> replay helpers used in DSA only, it is time to roll them out to all
> switchdev drivers.
> 
> To convert the switchdev object replay helpers from "pull mode" (where
> the driver asks for them) to a "push mode" (where the bridge offers them
> automatically), the biggest problem is that the bridge needs to be aware
> when a switchdev port joins and leaves, even when the switchdev is only
> indirectly a bridge port (for example when the bridge port is a LAG
> upper of the switchdev).
> 
> Luckily, we already have a hook for that, in the form of the newly
> introduced switchdev_bridge_port_offload() and
> switchdev_bridge_port_unoffload() calls. These offer a natural place for
> hooking the object addition and deletion replays.
> 
> Extend the above 2 functions with:
> - pointers to the switchdev atomic notifier (for FDB replays) and the
>   blocking notifier (for MDB and VLAN replays).
> - the "const void *ctx" argument required for drivers to be able to
>   disambiguate between which port is targeted, when multiple ports are
>   lowers of the same LAG that is a bridge port. Most of the drivers pass
>   NULL to this argument, except the ones that support LAG offload and have
>   the proper context check already in place in the switchdev blocking
>   notifier handler.
> 
> am65_cpsw and cpsw had the same name for the switchdev notifiers, so I
> renamed the am65_cpsw ones with an am65_ prefix.
> 
> Also unexport the replay helpers, since nobody except the bridge calls
> them directly now.
> 
> Note that:
> (a) we abuse the terminology slightly, because FDB entries are not
>     "switchdev objects", but we count them as objects nonetheless.
>     With no direct way to prove it, I think they are not modeled as
>     switchdev objects because those can only be installed by the bridge
>     to the hardware (as opposed to FDB entries which can be propagated
>     in the other direction too). This is merely an abuse of terms, FDB
>     entries are replayed too, despite not being objects.
> (b) the bridge does not attempt to sync port attributes to newly joined
>     ports, just the countable stuff (the objects). The reason for this
>     is simple: no universal and symmetric way to sync and unsync them is
>     known. For example, VLAN filtering: what to do on unsync, disable or
>     leave it enabled? Similarly, STP state, ageing timer, etc etc. What
>     a switchdev port does when it becomes standalone again is not really
>     up to the bridge's competence, and the driver should deal with it.
>     On the other hand, replaying deletions of switchdev objects can be
>     seen a matter of cleanup and therefore be treated by the bridge,
>     hence this patch.
> (c) I do not expect a lot of functional change introduced for drivers in
>     this patch, because:
>     - nbp_vlan_init() is called _after_ netdev_master_upper_dev_link(),
>       so br_vlan_replay() should not do anything for the new drivers on
>       which we call it. The existing drivers where there was even a
>       slight possibility for there to exist a VLAN on a bridge port
>       before they join it are already guarded against this: mlxsw and
>       prestera deny joining LAG interfaces that are members of a bridge.
>     - br_fdb_replay() should now notify of local FDB entries, but I
>       patched all drivers except DSA to ignore these new entries in
>       commit 2c4eca3ef716 ("net: bridge: switchdev: include local flag
>       in FDB notifications"). Driver authors can lift this restriction
>       as they wish.
>     - br_mdb_replay() should now fix the issue described in commit
>       2c4eca3ef716 ("net: bridge: switchdev: include local flag in FDB
>       notifications") for all drivers, I don't see any downside.
> 
> Cc: Vadym Kochan <vkochan@marvell.com>
> Cc: Taras Chornyi <tchornyi@marvell.com>
> Cc: Ioana Ciornei <ioana.ciornei@nxp.com>
> Cc: Lars Povlsen <lars.povlsen@microchip.com>
> Cc: Steen Hegelund <Steen.Hegelund@microchip.com>
> Cc: UNGLinuxDriver@microchip.com
> Cc: Claudiu Manoil <claudiu.manoil@nxp.com>
> Cc: Alexandre Belloni <alexandre.belloni@bootlin.com>
> Cc: Grygorii Strashko <grygorii.strashko@ti.com>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>

Acked-by: Ioana Ciornei <ioana.ciornei@nxp.com> # dpaa2-switch

  parent reply	other threads:[~2021-07-19  9:26 UTC|newest]

Thread overview: 37+ 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 ` [PATCH v4 net-next 01/15] net: dpaa2-switch: use extack in dpaa2_switch_port_bridge_join Vladimir Oltean
2021-07-19  9:17   ` Ioana Ciornei
2021-07-18 21:44 ` [PATCH v4 net-next 02/15] net: dpaa2-switch: refactor prechangeupper sanity checks Vladimir Oltean
2021-07-19  9:18   ` Ioana Ciornei
2021-07-18 21:44 ` [PATCH v4 net-next 03/15] mlxsw: spectrum: " 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-19  2:16   ` Florian Fainelli
2021-07-18 21:44 ` [PATCH v4 net-next 05/15] net: marvell: prestera: refactor prechangeupper sanity checks Vladimir Oltean
2021-07-19  2:20   ` 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-19  2:17   ` Florian Fainelli
2021-07-18 21:44 ` [PATCH v4 net-next 07/15] net: bridge: disambiguate offload_fwd_mark Vladimir Oltean
2021-07-19  2:26   ` 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 ` [PATCH v4 net-next 09/15] net: bridge: switchdev: let drivers inform which bridge ports are offloaded Vladimir Oltean
2021-07-19  9:23   ` Ioana Ciornei
2021-07-20  7:53   ` Horatiu Vultur
2021-07-20  8:45     ` 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-19  8:19   ` Vladimir Oltean
2021-07-19  9:26   ` Ioana Ciornei [this message]
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-19  2:43   ` Florian Fainelli
2021-07-19  7:22   ` 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-19  2:54   ` 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-19  2:51   ` 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-19  2:52   ` 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-19  2:47   ` Florian Fainelli
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 13:20   ` Vladimir Oltean
2021-07-20 13:51     ` 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=20210719092625.tnbgghblfqiwtrwl@skbuf \
    --to=ioana.ciornei@nxp.com \
    --cc=Steen.Hegelund@microchip.com \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=bridge@lists.linux-foundation.org \
    --cc=claudiu.manoil@nxp.com \
    --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=lars.povlsen@microchip.com \
    --cc=netdev@vger.kernel.org \
    --cc=nikolay@nvidia.com \
    --cc=roopa@nvidia.com \
    --cc=stephen@networkplumber.org \
    --cc=tchornyi@marvell.com \
    --cc=tobias@waldekranz.com \
    --cc=vivien.didelot@gmail.com \
    --cc=vkochan@marvell.com \
    --cc=vladimir.oltean@nxp.com \
    --subject='Re: [PATCH v4 net-next 10/15] net: bridge: switchdev object replay helpers for everybody' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).