On Wed, Dec 16, 2020 at 18:22, Vladimir Oltean [off-list ref] wrote:

On Wed, Dec 16, 2020 at 05:00:54PM +0100, Tobias Waldekranz wrote:

quoted

Monitor the following events and notify the driver when:

- A DSA port joins/leaves a LAG.
- A LAG, made up of DSA ports, joins/leaves a bridge.
- A DSA port in a LAG is enabled/disabled (enabled meaning
  "distributing" in 802.3ad LACP terms).

When a LAG joins a bridge, the DSA subsystem will treat that as each
individual port joining the bridge. The driver may look at the port's
LAG device pointer to see if it is associated with any LAG, if that is
required. This is analogue to how switchdev events are replicated out
to all lower devices when reaching e.g. a LAG.

Drivers can optionally request that DSA maintain a linear mapping from
a LAG ID to the corresponding netdev by setting ds->num_lag_ids to the
desired size.

In the event that the hardware is not capable of offloading a
particular LAG for any reason (the typical case being use of exotic
modes like broadcast), DSA will take a hands-off approach, allowing
the LAG to be formed as a pure software construct. This is reported
back through the extended ACK, but is otherwise transparent to the
user.

Signed-off-by: Tobias Waldekranz <tobias@waldekranz.com>
---

I tried in vain to win checkpatch's approval of the foreach macros, no
matter how many parentheses I added. Looking at existing macros, this
style seems to be widely accepted. Is this a known issue?

 include/net/dsa.h  | 60 +++++++++++++++++++++++++++++++++++
 net/dsa/dsa2.c     | 74 +++++++++++++++++++++++++++++++++++++++++++
 net/dsa/dsa_priv.h | 36 +++++++++++++++++++++
 net/dsa/port.c     | 79 ++++++++++++++++++++++++++++++++++++++++++++++
 net/dsa/slave.c    | 70 ++++++++++++++++++++++++++++++++++++----
 net/dsa/switch.c   | 50 +++++++++++++++++++++++++++++
 6 files changed, 362 insertions(+), 7 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 4e60d2610f20..9092c711a37c 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h

@@ -149,8 +149,41 @@ struct dsa_switch_tree {
 
 	/* List of DSA links composing the routing table */
 	struct list_head rtable;
+
+	/* Maps offloaded LAG netdevs to a zero-based linear ID for
+	 * drivers that need it.
+	 */
+	struct net_device **lags;
+	unsigned int lags_len;
 };
 
+#define dsa_lags_foreach_id(_id, _dst)				\
+	for ((_id) = 0; (_id) < (_dst)->lags_len; (_id)++)	\
+		if ((_dst)->lags[_id])
+
+#define dsa_lag_foreach_port(_dp, _dst, _lag)	       \
+	list_for_each_entry(_dp, &(_dst)->ports, list) \
+		if ((_dp)->lag_dev == (_lag))

ERROR: Macros with complex values should be enclosed in parentheses
#86: FILE: include/net/dsa.h:160:
+#define dsa_lags_foreach_id(_id, _dst)                         \
+       for ((_id) = 0; (_id) < (_dst)->lags_len; (_id)++)      \
+               if ((_dst)->lags[_id])
                                 ~~~
                                 missing parentheses

ERROR: Macros with complex values should be enclosed in parentheses
#90: FILE: include/net/dsa.h:164:
+#define dsa_lag_foreach_port(_dp, _dst, _lag)         \
+       list_for_each_entry(_dp, &(_dst)->ports, list) \
                            ~~~
                            missing parentheses
+               if ((_dp)->lag_dev == (_lag))

Please see my comment just before the diffstat. I tried adding these
parentheses, and about a gagillion other ones. But no matter what I did
I could not make checkpatch happy.

This is where I gave up:

ERROR: Macros with complex values should be enclosed in parentheses
#160: FILE: include/net/dsa.h:160:
+#define dsa_lags_foreach_id(_id, _dst)                         \
+       for (((_id) = 0); ((_id) < ((_dst)->lags_len)); ((_id)++))      \
+               if ((_dst)->lags[(_id)])

ERROR: Macros with complex values should be enclosed in parentheses
#164: FILE: include/net/dsa.h:164:
+#define dsa_lag_foreach_port(_dp, _dst, _lag)         \
+       list_for_each_entry((_dp), &((_dst)->ports), list)      \
+               if (((_dp)->lag_dev) == (_lag))