[net-next,3/4] net: dsa: populate dp->link_dp for cascade ports

Commit Message

Vladimir Oltean Sept. 13, 2024, 1:15 p.m. UTC

Drivers may need to walk the tree hop by hop, activity which is
currently impossible. This is because dst->rtable offers no guarantee as
to whether we are looking at a dsa_link that represents a direct
connection or not.

Partially address the long-standing TODO that we have, and do introduce
a link_dp member in struct dsa_port. This will actually represent the
adjacent cascade port.

Signed-off-by: Vladimir Oltean <[e-mail]>
---
 include/net/dsa.h |  9 +++++----
 net/dsa/dsa.c     | 22 ++++++++++++++++++++--
 2 files changed, 25 insertions(+), 6 deletions(-)

Comments

Simon Horman Sept. 14, 2024, 8:50 a.m. UTC | #1

On Fri, Sep 13, 2024 at 04:15:06PM +0300, Vladimir Oltean wrote:
> Drivers may need to walk the tree hop by hop, activity which is
> currently impossible. This is because dst->rtable offers no guarantee as
> to whether we are looking at a dsa_link that represents a direct
> connection or not.
> 
> Partially address the long-standing TODO that we have, and do introduce
> a link_dp member in struct dsa_port. This will actually represent the
> adjacent cascade port.
> 
> Signed-off-by: Vladimir Oltean <[e-mail]>

...

> @@ -307,9 +308,23 @@ static struct dsa_link *dsa_link_touch(struct dsa_port *dp,
>  	INIT_LIST_HEAD(&dl->list);
>  	list_add_tail(&dl->list, &dst->rtable);
>  
> +	if (adjacent)
> +		dp->link_dp = link_dp;
> +
>  	return dl;
>  }
>  
> +/**
> + * dsa_port_setup_routing_table(): Set up tree routing table based on
> + *	information from this cascade port
> + * @dp: cascade port
> + *
> + * Parse the device tree node for the "link" array of phandles to other cascade
> + * ports, creating routing table elements from this source to each destination
> + * list element found. One assumption is being made, which is backed by the
> + * device tree bindings: that the first "link" element is the directly
> + * connected cascade port.
> + */

Hi Vladimir,

Another minor nit from my side (I think this is the last one).

Please consider documenting the return value of functions that return
a value using a "Return:" or "Returns:" section.

Flagged by ./scripts/kernel-doc -none -Wall

>  static bool dsa_port_setup_routing_table(struct dsa_port *dp)
>  {
>  	struct dsa_switch *ds = dp->ds;
> @@ -317,6 +332,7 @@ static bool dsa_port_setup_routing_table(struct dsa_port *dp)
>  	struct device_node *dn = dp->dn;
>  	struct of_phandle_iterator it;
>  	struct dsa_port *link_dp;
> +	bool adjacent = true;
>  	struct dsa_link *dl;
>  	int err;
>  

...

Patch

diff --git a/include/net/dsa.h b/include/net/dsa.h
index d7a6c2930277..586efb76f67d 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -265,6 +265,7 @@  struct dsa_port {
 
 	const char		*name;
 	struct dsa_port		*cpu_dp;
+	struct dsa_port		*link_dp;
 	u8			mac[ETH_ALEN];
 
 	u8			stp_state;
@@ -332,10 +333,10 @@  dsa_phylink_to_port(struct phylink_config *config)
 	return container_of(config, struct dsa_port, pl_config);
 }
 
-/* TODO: ideally DSA ports would have a single dp->link_dp member,
- * and no dst->rtable nor this struct dsa_link would be needed,
- * but this would require some more complex tree walking,
- * so keep it stupid at the moment and list them all.
+/* TODO: DSA ports do have a dp->link_dp member which represents their direct
+ * connection. However, dsa_routing_port() requires full routing information,
+ * which _could_ be deduced based on just the adjacency, but it requires some
+ * complex tree walking. So keep it stupid at the moment and list them all.
  */
 struct dsa_link {
 	struct dsa_port *dp;
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index a543ddaefdd8..8b4ec00de521 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -285,7 +285,8 @@  static struct dsa_port *dsa_tree_find_port_by_node(struct dsa_switch_tree *dst,
 }
 
 static struct dsa_link *dsa_link_touch(struct dsa_port *dp,
-				       struct dsa_port *link_dp)
+				       struct dsa_port *link_dp,
+				       bool adjacent)
 {
 	struct dsa_switch *ds = dp->ds;
 	struct dsa_switch_tree *dst;
@@ -307,9 +308,23 @@  static struct dsa_link *dsa_link_touch(struct dsa_port *dp,
 	INIT_LIST_HEAD(&dl->list);
 	list_add_tail(&dl->list, &dst->rtable);
 
+	if (adjacent)
+		dp->link_dp = link_dp;
+
 	return dl;
 }
 
+/**
+ * dsa_port_setup_routing_table(): Set up tree routing table based on
+ *	information from this cascade port
+ * @dp: cascade port
+ *
+ * Parse the device tree node for the "link" array of phandles to other cascade
+ * ports, creating routing table elements from this source to each destination
+ * list element found. One assumption is being made, which is backed by the
+ * device tree bindings: that the first "link" element is the directly
+ * connected cascade port.
+ */
 static bool dsa_port_setup_routing_table(struct dsa_port *dp)
 {
 	struct dsa_switch *ds = dp->ds;
@@ -317,6 +332,7 @@  static bool dsa_port_setup_routing_table(struct dsa_port *dp)
 	struct device_node *dn = dp->dn;
 	struct of_phandle_iterator it;
 	struct dsa_port *link_dp;
+	bool adjacent = true;
 	struct dsa_link *dl;
 	int err;
 
@@ -327,11 +343,13 @@  static bool dsa_port_setup_routing_table(struct dsa_port *dp)
 			return false;
 		}
 
-		dl = dsa_link_touch(dp, link_dp);
+		dl = dsa_link_touch(dp, link_dp, adjacent);
 		if (!dl) {
 			of_node_put(it.node);
 			return false;
 		}
+
+		adjacent = false;
 	}
 
 	return true;