On Fri, Sep 13, 2024 at 04:15:07PM +0300, Vladimir Oltean wrote:
> The SJA1105 management route concept was previously explained in commits
> 227d07a07ef1 ("net: dsa: sja1105: Add support for traffic through
> standalone ports") and 0a51826c6e05 ("net: dsa: sja1105: Always send
> through management routes in slot 0").
>
> In a daisy chained topology with at least 2 switches, sending link-local
> frames belonging to the downstream switch should program 2 management
> routes: one on the upstream switch and one on the leaf switch. In the
> general case, each switch along the TX path of the packet, starting from
> the CPU, need a one-shot management route installed over SPI.
>
> The driver currently does not handle this, but instead limits link-local
> traffic support to a single switch, due to 2 major blockers:
>
> 1. There was no way up until now to calculate the path (the management
> route itself) between the CPU and a leaf user port. Sure, we can start
> with dp->cpu_dp and use dsa_routing_port() to figure out the cascade
> port that targets the next switch. But we cannot make the jump from
> one switch to the next. The dst->rtable is fundamentally flawed by
> construction. It contains not only directly-connected link_dp entries,
> but links to _all_ other cascade ports in the tree. For trees with 3
> or more switches, this means that we don't know, by following
> dst->rtable, if the link_dp that we pick is really one hop away, or
> more than one hop away. So we might skip programming some switches
> along the packet's path.
>
> 2. The current priv->mgmt_lock does not serialize enough code to work in
> a cross-chip scenario. When sending a packet across a tree, we want
> to block updates to the management route tables for all switches
> along that path, not just for the leaf port (because link-local
> traffic might be transmitted concurrently towards other ports).
> Keeping this lock where it is (in struct sja1105_private, which is
> per switch) will not work, because sja1105_port_deferred_xmit() would
> have to acquire and then release N locks, and that's simply
> impossible to do without risking AB/BA deadlocks.
>
> To solve 1, recent changes have introduced struct dsa_port :: link_dp in
> the DSA core, to make the hop-by-hop traversal of the DSA tree possible.
> Using that information, we statically compute management routes for each
> user port at switch setup time.
>
> To solve 2, we go for the much more complex scheme of allocating a
> tree-wide structure for managing the management routes, which holds a
> single lock.
>
> Signed-off-by: Vladimir Oltean <vladimir.oltean@xxxxxxx>
> ---
> drivers/net/dsa/sja1105/sja1105.h | 43 ++++-
> drivers/net/dsa/sja1105/sja1105_main.c | 253 ++++++++++++++++++++++---
> 2 files changed, 263 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/net/dsa/sja1105/sja1105.h b/drivers/net/dsa/sja1105/sja1105.h
> index 8c66d3bf61f0..7753b4d62bc6 100644
> --- a/drivers/net/dsa/sja1105/sja1105.h
> +++ b/drivers/net/dsa/sja1105/sja1105.h
> @@ -245,6 +245,43 @@ struct sja1105_flow_block {
> int num_virtual_links;
> };
>
> +/**
> + * sja1105_mgmt_route_port: Representation of one port in a management route
Hi Vladimir,
As this series has been deferred, a minor nit from my side.
Tooling seems to want the keyword struct at the beginning of the
short description. So I suggest something like this:
* struct sja1105_mgmt_route_port: One port in a management route
Likewise for the two Kernel docs for structures below.
Flagged by ./scripts/kernel-doc -none
> + * @dp: DSA user or cascade port
> + * @list: List node element for the mgmt_route->ports list membership
> + */
> +struct sja1105_mgmt_route_port {
> + struct dsa_port *dp;
> + struct list_head list;
> +};
> +
> +/**
> + * sja1105_mgmt_route: Structure to represent a SJA1105 management route
> + * @ports: List of ports on which the management route needs to be installed,
> + * starting with the downstream-facing cascade port of the switch which
> + * has the CPU connection, and ending with the user port of the leaf
> + * switch.
> + * @list: List node element for the mgmt_tree->routes list membership.
> + */
> +struct sja1105_mgmt_route {
> + struct list_head ports;
> + struct list_head list;
> +};
> +
> +/**
> + * sja1105_mgmt_tree: DSA switch tree-level structure for management routes
> + * @lock: Serializes transmission of management frames across the tree, so that
> + * the switches don't confuse them with one another.
> + * @routes: List of sja1105_mgmt_route structures, one for each user port in
> + * the tree.
> + * @refcount: Reference count.
> + */
> +struct sja1105_mgmt_tree {
> + struct mutex lock;
> + struct list_head routes;
> + refcount_t refcount;
> +};
> +
> struct sja1105_private {
> struct sja1105_static_config static_config;
> int rgmii_rx_delay_ps[SJA1105_MAX_NUM_PORTS];
...
