[PATCH v2 net-next 00/14] LAG offload for Ocelot DSA switches

[PATCH v2 net-next 00/14] LAG offload for Ocelot DSA switches

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

This patch series reworks the ocelot switchdev driver such that it could
share the same implementation for LAG offload as the felix DSA driver.

Testing has been done in the following topology:

         +----------------------------------+
         | Board 1         br0              |
         |             +---------+          |
         |            /           \         |
         |            |           |         |
         |            |         bond0       |
         |            |        +-----+      |
         |            |       /       \     |
         |  eno0     swp0    swp1    swp2   |
         +---|--------|-------|-------|-----+
             |        |       |       |
             +--------+       |       |
               Cable          |       |
                         Cable|       |Cable
               Cable          |       |
             +--------+       |       |
             |        |       |       |
         +---|--------|-------|-------|-----+
         |  eno0     swp0    swp1    swp2   |
         |            |       \       /     |
         |            |        +-----+      |
         |            |         bond0       |
         |            |           |         |
         |            \           /         |
         |             +---------+          |
         | Board 2         br0              |
         +----------------------------------+

The same script can be run on both Board 1 and Board 2 to set this up:

#!/bin/bash

ip link del bond0
ip link add bond0 type bond mode 802.3ad
ip link set swp1 down && ip link set swp1 master bond0 && ip link set swp1 up
ip link set swp2 down && ip link set swp2 master bond0 && ip link set swp2 up
ip link del br0
ip link add br0 type bridge
ip link set bond0 master br0
ip link set swp0 master br0

Then traffic can be tested between eno0 of Board 1 and eno0 of Board 2.

Note: series applies on top of:
https://patchwork.kernel.org/project/netdevbpf/cover/20210115021120.3055988-1-olteanv@gmail.com/

Vladimir Oltean (14):
  net: mscc: ocelot: allow offloading of bridge on top of LAG
  net: mscc: ocelot: rename ocelot_netdevice_port_event to
    ocelot_netdevice_changeupper
  net: mscc: ocelot: use a switch-case statement in
    ocelot_netdevice_event
  net: mscc: ocelot: don't refuse bonding interfaces we can't offload
  net: mscc: ocelot: use ipv6 in the aggregation code
  net: mscc: ocelot: set up the bonding mask in a way that avoids a
    net_device
  net: mscc: ocelot: avoid unneeded "lp" variable in LAG join
  net: mscc: ocelot: use "lag" variable name in
    ocelot_bridge_stp_state_set
  net: mscc: ocelot: reapply bridge forwarding mask on bonding
    join/leave
  net: mscc: ocelot: set up logical port IDs centrally
  net: mscc: ocelot: drop the use of the "lags" array
  net: mscc: ocelot: rename aggr_count to num_ports_in_lag
  net: mscc: ocelot: rebalance LAGs on link up/down events
  net: dsa: felix: propagate the LAG offload ops towards the ocelot lib

 drivers/net/dsa/ocelot/felix.c         |  28 +++
 drivers/net/ethernet/mscc/ocelot.c     | 256 ++++++++++++++-----------
 drivers/net/ethernet/mscc/ocelot.h     |   4 -
 drivers/net/ethernet/mscc/ocelot_net.c | 131 ++++++++-----
 include/soc/mscc/ocelot.h              |  11 +-
 5 files changed, 265 insertions(+), 165 deletions(-)

-- 
2.25.1

[PATCH v2 net-next 01/14] net: mscc: ocelot: allow offloading of bridge on top of LAG

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Commit 7afb3e575e5a ("net: mscc: ocelot: don't handle netdev events for
other netdevs") was too aggressive, and it made ocelot_netdevice_event
react only to network interface events emitted for the ocelot switch
ports.

In fact, only the PRECHANGEUPPER should have had that check.

When we ignore all events that are not for us, we miss the fact that the
upper of the LAG changes, and the bonding interface gets enslaved to a
bridge. This is an operation we could offload under certain conditions.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes in v2:
None.

 drivers/net/ethernet/mscc/ocelot_net.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index a520fd485912..467170363ab2 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -1153,10 +1153,8 @@ static int ocelot_netdevice_event(struct notifier_block *unused,
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
 	int ret = 0;
 
-	if (!ocelot_netdevice_dev_check(dev))
-		return 0;
-
 	if (event == NETDEV_PRECHANGEUPPER &&
+	    ocelot_netdevice_dev_check(dev) &&
 	    netif_is_lag_master(info->upper_dev)) {
 		struct netdev_lag_upper_info *lag_upper_info = info->upper_info;
 		struct netlink_ext_ack *extack;
-- 
2.25.1

Re: [PATCH v2 net-next 01/14] net: mscc: ocelot: allow offloading of bridge on top of LAG

[Jakub Kicinski]

On Sat, 16 Jan 2021 02:59:30 +0200 Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> Commit 7afb3e575e5a ("net: mscc: ocelot: don't handle netdev events for
> other netdevs") was too aggressive, and it made ocelot_netdevice_event
> react only to network interface events emitted for the ocelot switch
> ports.
> 
> In fact, only the PRECHANGEUPPER should have had that check.
> 
> When we ignore all events that are not for us, we miss the fact that the
> upper of the LAG changes, and the bonding interface gets enslaved to a
> bridge. This is an operation we could offload under certain conditions.

I see the commit in question is in net, perhaps worth spelling out why
this is not a fix? Perhaps add some "in the future" to the last
sentence if it's the case that this will only matter with the following
patches applied?

Re: [PATCH v2 net-next 01/14] net: mscc: ocelot: allow offloading of bridge on top of LAG

[Vladimir Oltean]

On Sat, Jan 16, 2021 at 05:26:23PM -0800, Jakub Kicinski wrote:
> On Sat, 16 Jan 2021 02:59:30 +0200 Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> >
> > Commit 7afb3e575e5a ("net: mscc: ocelot: don't handle netdev events for
> > other netdevs") was too aggressive, and it made ocelot_netdevice_event
> > react only to network interface events emitted for the ocelot switch
> > ports.
> >
> > In fact, only the PRECHANGEUPPER should have had that check.
> >
> > When we ignore all events that are not for us, we miss the fact that the
> > upper of the LAG changes, and the bonding interface gets enslaved to a
> > bridge. This is an operation we could offload under certain conditions.
>
> I see the commit in question is in net, perhaps worth spelling out why
> this is not a fix? Perhaps add some "in the future" to the last
> sentence if it's the case that this will only matter with the following
> patches applied?

It is a fix. However, so is patch 13/14 "net: mscc: ocelot: rebalance
LAGs on link up/down events", but I didn't see an easy way to backport
that. Honestly the reasons why I did not attempt to split this series
into a part for "net" and one for "net-next" are:
(a) It would unnecessarily complicate my work for felix DSA, where this
    is considered a new feature as opposed to ocelot switchdev where it
    was supposedly already working (although.. not quite, due to the
    lack of rebalancing, a link down would throw off the LAG).
    I don't really think that anybody was seriously using LAG offload on
    ocelot so far.
(b) Even if I were to split this patch, it can only be trivially
    backported as far as commit 9c90eea310f8 ("net: mscc: ocelot: move
    net_device related functions to ocelot_net.c") from June 2020
    anyway.
(c) I cannot test the mscc_ocelot.ko switchdev driver with traffic,
    since I don't have the hardware (I just have a local patch that
    I keep rebasing on top of net-next which makes me able to at least
    probe it and access its registers on a different switch revision,
    but the traffic I/O procedure there is completely different). So I
    can not really confirm what is the state I'm leaving the mscc_ocelot
    driver in, for stable kernels. At least now, I've made the entry
    points into the control code path very similar to those of DSA, and
    I've exercised the switchdev driver in blind (without traffic), so I
    have a bit more confidence that it should work.
(d) Had the AUTOSEL guys picked up this patch, I would have probably had
    no objection (since my belief is that there's nothing to break and
    nothing to fix in stable kernels).

That being said, if we want to engage in a rigid demonstration of
procedures, sure we can do that. I have other patches anyway to fill the
pipeline until "net" is merged back into "net-next" :)

Re: [PATCH v2 net-next 01/14] net: mscc: ocelot: allow offloading of bridge on top of LAG

[Jakub Kicinski]

On Sun, 17 Jan 2021 14:37:44 +0200 Vladimir Oltean wrote:
> That being said, if we want to engage in a rigid demonstration of
> procedures, sure we can do that. I have other patches anyway to fill the
> pipeline until "net" is merged back into "net-next" :)

If you don't mind I'd rather apply the fix to net, and the rest on
Thu/Fri after the trees get merged.

Re: [PATCH v2 net-next 01/14] net: mscc: ocelot: allow offloading of bridge on top of LAG

[Vladimir Oltean]

On Mon, Jan 18, 2021 at 11:04:47AM -0800, Jakub Kicinski wrote:
> On Sun, 17 Jan 2021 14:37:44 +0200 Vladimir Oltean wrote:
> > That being said, if we want to engage in a rigid demonstration of
> > procedures, sure we can do that. I have other patches anyway to fill the
> > pipeline until "net" is merged back into "net-next" :)
> 
> If you don't mind I'd rather apply the fix to net, and the rest on
> Thu/Fri after the trees get merged.

Sure, I already split this patch and sent it to "net":
https://patchwork.kernel.org/project/netdevbpf/patch/20210118135210.2666246-1-olteanv@gmail.com/

[PATCH v2 net-next 02/14] net: mscc: ocelot: rename ocelot_netdevice_port_event to ocelot_netdevice_changeupper

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

ocelot_netdevice_port_event treats a single event, NETDEV_CHANGEUPPER.
So we can remove the check for the type of event, and rename the
function to be more suggestive, since there already is a function with a
very similar name of ocelot_netdevice_event.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
Fixed build by removing the "event" argument of ocelot_netdevice_changeupper.

 drivers/net/ethernet/mscc/ocelot_net.c | 59 ++++++++++++--------------
 1 file changed, 27 insertions(+), 32 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 467170363ab2..f15f38f45249 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -1109,9 +1109,8 @@ static int ocelot_port_obj_del(struct net_device *dev,
 	return ret;
 }
 
-static int ocelot_netdevice_port_event(struct net_device *dev,
-				       unsigned long event,
-				       struct netdev_notifier_changeupper_info *info)
+static int ocelot_netdevice_changeupper(struct net_device *dev,
+					struct netdev_notifier_changeupper_info *info)
 {
 	struct ocelot_port_private *priv = netdev_priv(dev);
 	struct ocelot_port *ocelot_port = &priv->port;
@@ -1119,28 +1118,22 @@ static int ocelot_netdevice_port_event(struct net_device *dev,
 	int port = priv->chip_port;
 	int err = 0;
 
-	switch (event) {
-	case NETDEV_CHANGEUPPER:
-		if (netif_is_bridge_master(info->upper_dev)) {
-			if (info->linking) {
-				err = ocelot_port_bridge_join(ocelot, port,
-							      info->upper_dev);
-			} else {
-				err = ocelot_port_bridge_leave(ocelot, port,
-							       info->upper_dev);
-			}
-		}
-		if (netif_is_lag_master(info->upper_dev)) {
-			if (info->linking)
-				err = ocelot_port_lag_join(ocelot, port,
-							   info->upper_dev);
-			else
-				ocelot_port_lag_leave(ocelot, port,
+	if (netif_is_bridge_master(info->upper_dev)) {
+		if (info->linking) {
+			err = ocelot_port_bridge_join(ocelot, port,
 						      info->upper_dev);
+		} else {
+			err = ocelot_port_bridge_leave(ocelot, port,
+						       info->upper_dev);
 		}
-		break;
-	default:
-		break;
+	}
+	if (netif_is_lag_master(info->upper_dev)) {
+		if (info->linking)
+			err = ocelot_port_lag_join(ocelot, port,
+						   info->upper_dev);
+		else
+			ocelot_port_lag_leave(ocelot, port,
+					      info->upper_dev);
 	}
 
 	return err;
@@ -1169,17 +1162,19 @@ static int ocelot_netdevice_event(struct notifier_block *unused,
 		}
 	}
 
-	if (netif_is_lag_master(dev)) {
-		struct net_device *slave;
-		struct list_head *iter;
+	if (event == NETDEV_CHANGEUPPER) {
+		if (netif_is_lag_master(dev)) {
+			struct net_device *slave;
+			struct list_head *iter;
 
-		netdev_for_each_lower_dev(dev, slave, iter) {
-			ret = ocelot_netdevice_port_event(slave, event, info);
-			if (ret)
-				goto notify;
+			netdev_for_each_lower_dev(dev, slave, iter) {
+				ret = ocelot_netdevice_changeupper(slave, info);
+				if (ret)
+					goto notify;
+			}
+		} else {
+			ret = ocelot_netdevice_changeupper(dev, info);
 		}
-	} else {
-		ret = ocelot_netdevice_port_event(dev, event, info);
 	}
 
 notify:
-- 
2.25.1

[PATCH v2 net-next 03/14] net: mscc: ocelot: use a switch-case statement in ocelot_netdevice_event

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Make ocelot's net device event handler more streamlined by structuring
it in a similar way with others. The inspiration here was
dsa_slave_netdevice_event.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>

---
Changes in v2:
Addressed Alex's feedback:
> This changes the return value in case of error, I'm not sure how
> important this is.
by keeping the return code of notifier_from_errno(-EINVAL)

 drivers/net/ethernet/mscc/ocelot_net.c | 68 +++++++++++++++++---------
 1 file changed, 45 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index f15f38f45249..b80a5bb95163 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -1136,49 +1136,71 @@ static int ocelot_netdevice_changeupper(struct net_device *dev,
 					      info->upper_dev);
 	}
 
-	return err;
+	return notifier_from_errno(err);
+}
+
+static int
+ocelot_netdevice_lag_changeupper(struct net_device *dev,
+				 struct netdev_notifier_changeupper_info *info)
+{
+	struct net_device *lower;
+	struct list_head *iter;
+	int err = NOTIFY_DONE;
+
+	netdev_for_each_lower_dev(dev, lower, iter) {
+		err = ocelot_netdevice_changeupper(lower, info);
+		if (err)
+			return notifier_from_errno(err);
+	}
+
+	return NOTIFY_DONE;
 }
 
 static int ocelot_netdevice_event(struct notifier_block *unused,
 				  unsigned long event, void *ptr)
 {
-	struct netdev_notifier_changeupper_info *info = ptr;
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
-	int ret = 0;
 
-	if (event == NETDEV_PRECHANGEUPPER &&
-	    ocelot_netdevice_dev_check(dev) &&
-	    netif_is_lag_master(info->upper_dev)) {
-		struct netdev_lag_upper_info *lag_upper_info = info->upper_info;
+	switch (event) {
+	case NETDEV_PRECHANGEUPPER: {
+		struct netdev_notifier_changeupper_info *info = ptr;
+		struct netdev_lag_upper_info *lag_upper_info;
 		struct netlink_ext_ack *extack;
 
+		if (!ocelot_netdevice_dev_check(dev))
+			break;
+
+		if (!netif_is_lag_master(info->upper_dev))
+			break;
+
+		lag_upper_info = info->upper_info;
+
 		if (lag_upper_info &&
 		    lag_upper_info->tx_type != NETDEV_LAG_TX_TYPE_HASH) {
 			extack = netdev_notifier_info_to_extack(&info->info);
 			NL_SET_ERR_MSG_MOD(extack, "LAG device using unsupported Tx type");
 
-			ret = -EINVAL;
-			goto notify;
+			return notifier_from_errno(-EINVAL);
 		}
+
+		break;
 	}
+	case NETDEV_CHANGEUPPER: {
+		struct netdev_notifier_changeupper_info *info = ptr;
 
-	if (event == NETDEV_CHANGEUPPER) {
-		if (netif_is_lag_master(dev)) {
-			struct net_device *slave;
-			struct list_head *iter;
+		if (ocelot_netdevice_dev_check(dev))
+			return ocelot_netdevice_changeupper(dev, info);
 
-			netdev_for_each_lower_dev(dev, slave, iter) {
-				ret = ocelot_netdevice_changeupper(slave, info);
-				if (ret)
-					goto notify;
-			}
-		} else {
-			ret = ocelot_netdevice_changeupper(dev, info);
-		}
+		if (netif_is_lag_master(dev))
+			return ocelot_netdevice_lag_changeupper(dev, info);
+
+		break;
+	}
+	default:
+		break;
 	}
 
-notify:
-	return notifier_from_errno(ret);
+	return NOTIFY_DONE;
 }
 
 struct notifier_block ocelot_netdevice_nb __read_mostly = {
-- 
2.25.1

[PATCH v2 net-next 04/14] net: mscc: ocelot: don't refuse bonding interfaces we can't offload

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Since switchdev/DSA exposes network interfaces that fulfill many of the
same user space expectations that dedicated NICs do, it makes sense to
not deny bonding interfaces with a bonding policy that we cannot offload,
but instead allow the bonding driver to select the egress interface in
software.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes in v2:
Adapted to the merged version of the DSA API for LAG offload (i.e.
rejecting a bonding interface due to tx_type now done within the
.port_lag_join callback, caller is supposed to handle -EOPNOTSUPP).

 drivers/net/ethernet/mscc/ocelot.c     |  6 ++++-
 drivers/net/ethernet/mscc/ocelot.h     |  3 ++-
 drivers/net/ethernet/mscc/ocelot_net.c | 36 +++++++-------------------
 3 files changed, 17 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index a560d6be2a44..d3a92c46f610 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1251,12 +1251,16 @@ static void ocelot_setup_lag(struct ocelot *ocelot, int lag)
 }
 
 int ocelot_port_lag_join(struct ocelot *ocelot, int port,
-			 struct net_device *bond)
+			 struct net_device *bond,
+			 struct netdev_lag_upper_info *info)
 {
 	struct net_device *ndev;
 	u32 bond_mask = 0;
 	int lag, lp;
 
+	if (info->tx_type != NETDEV_LAG_TX_TYPE_HASH)
+		return -EOPNOTSUPP;
+
 	rcu_read_lock();
 	for_each_netdev_in_bond_rcu(bond, ndev) {
 		struct ocelot_port_private *priv = netdev_priv(ndev);
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index e8621dbc14f7..b6c9ddcee554 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -110,7 +110,8 @@ int ocelot_mact_learn(struct ocelot *ocelot, int port,
 int ocelot_mact_forget(struct ocelot *ocelot,
 		       const unsigned char mac[ETH_ALEN], unsigned int vid);
 int ocelot_port_lag_join(struct ocelot *ocelot, int port,
-			 struct net_device *bond);
+			 struct net_device *bond,
+			 struct netdev_lag_upper_info *info);
 void ocelot_port_lag_leave(struct ocelot *ocelot, int port,
 			   struct net_device *bond);
 struct net_device *ocelot_port_to_netdev(struct ocelot *ocelot, int port);
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index b80a5bb95163..f246f8fc535d 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -1128,12 +1128,19 @@ static int ocelot_netdevice_changeupper(struct net_device *dev,
 		}
 	}
 	if (netif_is_lag_master(info->upper_dev)) {
-		if (info->linking)
+		if (info->linking) {
 			err = ocelot_port_lag_join(ocelot, port,
-						   info->upper_dev);
-		else
+						   info->upper_dev,
+						   info->upper_info);
+			if (err == -EOPNOTSUPP) {
+				NL_SET_ERR_MSG_MOD(info->info.extack,
+						   "Offloading not supported");
+				err = 0;
+			}
+		} else {
 			ocelot_port_lag_leave(ocelot, port,
 					      info->upper_dev);
+		}
 	}
 
 	return notifier_from_errno(err);
@@ -1162,29 +1169,6 @@ static int ocelot_netdevice_event(struct notifier_block *unused,
 	struct net_device *dev = netdev_notifier_info_to_dev(ptr);
 
 	switch (event) {
-	case NETDEV_PRECHANGEUPPER: {
-		struct netdev_notifier_changeupper_info *info = ptr;
-		struct netdev_lag_upper_info *lag_upper_info;
-		struct netlink_ext_ack *extack;
-
-		if (!ocelot_netdevice_dev_check(dev))
-			break;
-
-		if (!netif_is_lag_master(info->upper_dev))
-			break;
-
-		lag_upper_info = info->upper_info;
-
-		if (lag_upper_info &&
-		    lag_upper_info->tx_type != NETDEV_LAG_TX_TYPE_HASH) {
-			extack = netdev_notifier_info_to_extack(&info->info);
-			NL_SET_ERR_MSG_MOD(extack, "LAG device using unsupported Tx type");
-
-			return notifier_from_errno(-EINVAL);
-		}
-
-		break;
-	}
 	case NETDEV_CHANGEUPPER: {
 		struct netdev_notifier_changeupper_info *info = ptr;
 
-- 
2.25.1

[PATCH v2 net-next 05/14] net: mscc: ocelot: use ipv6 in the aggregation code

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

IPv6 header information is not currently part of the entropy source for
the 4-bit aggregation code used for LAG offload, even though it could be.
The hardware reference manual says about these fields:

ANA::AGGR_CFG.AC_IP6_TCPUDP_PORT_ENA
Use IPv6 TCP/UDP port when calculating aggregation code. Configure
identically for all ports. Recommended value is 1.

ANA::AGGR_CFG.AC_IP6_FLOW_LBL_ENA
Use IPv6 flow label when calculating AC. Configure identically for all
ports. Recommended value is 1.

Integration with the xmit_hash_policy of the bonding interface is TBD.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes in v2:
None.

 drivers/net/ethernet/mscc/ocelot.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index d3a92c46f610..2b542f286739 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1548,7 +1548,10 @@ int ocelot_init(struct ocelot *ocelot)
 	ocelot_write(ocelot, ANA_AGGR_CFG_AC_SMAC_ENA |
 			     ANA_AGGR_CFG_AC_DMAC_ENA |
 			     ANA_AGGR_CFG_AC_IP4_SIPDIP_ENA |
-			     ANA_AGGR_CFG_AC_IP4_TCPUDP_ENA, ANA_AGGR_CFG);
+			     ANA_AGGR_CFG_AC_IP4_TCPUDP_ENA |
+			     ANA_AGGR_CFG_AC_IP6_FLOW_LBL_ENA |
+			     ANA_AGGR_CFG_AC_IP6_TCPUDP_ENA,
+			     ANA_AGGR_CFG);
 
 	/* Set MAC age time to default value. The entry is aged after
 	 * 2*AGE_PERIOD
-- 
2.25.1

[PATCH v2 net-next 06/14] net: mscc: ocelot: set up the bonding mask in a way that avoids a net_device

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Since this code should be called from pure switchdev as well as from
DSA, we must find a way to determine the bonding mask not by looking
directly at the net_device lowers of the bonding interface, since those
could have different private structures.

We keep a pointer to the bonding upper interface, if present, in struct
ocelot_port. Then the bonding mask becomes the bitwise OR of all ports
that have the same bonding upper interface. This adds a duplication of
functionality with the current "lags" array, but the duplication will be
short-lived, since further patches will remove the latter completely.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes in v2:
Adapted to the merged version of the DSA API for LAG offload (i.e.
rejecting a bonding interface due to tx_type now done within the
.port_lag_join callback, caller is supposed to handle -EOPNOTSUPP).

 drivers/net/ethernet/mscc/ocelot.c | 29 ++++++++++++++++++++++-------
 include/soc/mscc/ocelot.h          |  2 ++
 2 files changed, 24 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 2b542f286739..e2744b921a97 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -876,6 +876,24 @@ int ocelot_get_ts_info(struct ocelot *ocelot, int port,
 }
 EXPORT_SYMBOL(ocelot_get_ts_info);
 
+static u32 ocelot_get_bond_mask(struct ocelot *ocelot, struct net_device *bond)
+{
+	u32 bond_mask = 0;
+	int port;
+
+	for (port = 0; port < ocelot->num_phys_ports; port++) {
+		struct ocelot_port *ocelot_port = ocelot->ports[port];
+
+		if (!ocelot_port)
+			continue;
+
+		if (ocelot_port->bond == bond)
+			bond_mask |= BIT(port);
+	}
+
+	return bond_mask;
+}
+
 void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state)
 {
 	u32 port_cfg;
@@ -1254,20 +1272,15 @@ int ocelot_port_lag_join(struct ocelot *ocelot, int port,
 			 struct net_device *bond,
 			 struct netdev_lag_upper_info *info)
 {
-	struct net_device *ndev;
 	u32 bond_mask = 0;
 	int lag, lp;
 
 	if (info->tx_type != NETDEV_LAG_TX_TYPE_HASH)
 		return -EOPNOTSUPP;
 
-	rcu_read_lock();
-	for_each_netdev_in_bond_rcu(bond, ndev) {
-		struct ocelot_port_private *priv = netdev_priv(ndev);
+	ocelot->ports[port]->bond = bond;
 
-		bond_mask |= BIT(priv->chip_port);
-	}
-	rcu_read_unlock();
+	bond_mask = ocelot_get_bond_mask(ocelot, bond);
 
 	lp = __ffs(bond_mask);
 
@@ -1300,6 +1313,8 @@ void ocelot_port_lag_leave(struct ocelot *ocelot, int port,
 	u32 port_cfg;
 	int i;
 
+	ocelot->ports[port]->bond = NULL;
+
 	/* Remove port from any lag */
 	for (i = 0; i < ocelot->num_phys_ports; i++)
 		ocelot->lags[i] &= ~BIT(port);
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index cdc33fa05660..d2c587f099c8 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -610,6 +610,8 @@ struct ocelot_port {
 	phy_interface_t			phy_mode;
 
 	u8				*xmit_template;
+
+	struct net_device		*bond;
 };
 
 struct ocelot {
-- 
2.25.1

[PATCH v2 net-next 07/14] net: mscc: ocelot: avoid unneeded "lp" variable in LAG join

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The index of the LAG is equal to the logical port ID that all the
physical port members have, which is further equal to the index of the
first physical port that is a member of the LAG.

The code gets a bit carried away with logic like this:

	if (a == b)
		c = a;
	else
		c = b;

which can be simplified, of course, into:

	c = b;

(with a being port, b being lp, c being lag)

This further makes the "lp" variable redundant, since we can use "lag"
everywhere where "lp" (logical port) was used. So instead of a "c = b"
assignment, we can do a complete deletion of b. Only one comment here:

		if (bond_mask) {
			lp = __ffs(bond_mask);
			ocelot->lags[lp] = 0;
		}

lp was clobbered before, because it was used as a temporary variable to
hold the new smallest port ID from the bond. Now that we don't have "lp"
any longer, we'll just avoid the temporary variable and zeroize the
bonding mask directly.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes in v2:
None.

 drivers/net/ethernet/mscc/ocelot.c | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index e2744b921a97..ce52bf892f9b 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1273,7 +1273,7 @@ int ocelot_port_lag_join(struct ocelot *ocelot, int port,
 			 struct netdev_lag_upper_info *info)
 {
 	u32 bond_mask = 0;
-	int lag, lp;
+	int lag;
 
 	if (info->tx_type != NETDEV_LAG_TX_TYPE_HASH)
 		return -EOPNOTSUPP;
@@ -1282,22 +1282,18 @@ int ocelot_port_lag_join(struct ocelot *ocelot, int port,
 
 	bond_mask = ocelot_get_bond_mask(ocelot, bond);
 
-	lp = __ffs(bond_mask);
+	lag = __ffs(bond_mask);
 
 	/* If the new port is the lowest one, use it as the logical port from
 	 * now on
 	 */
-	if (port == lp) {
-		lag = port;
+	if (port == lag) {
 		ocelot->lags[port] = bond_mask;
 		bond_mask &= ~BIT(port);
-		if (bond_mask) {
-			lp = __ffs(bond_mask);
-			ocelot->lags[lp] = 0;
-		}
+		if (bond_mask)
+			ocelot->lags[__ffs(bond_mask)] = 0;
 	} else {
-		lag = lp;
-		ocelot->lags[lp] |= BIT(port);
+		ocelot->lags[lag] |= BIT(port);
 	}
 
 	ocelot_setup_lag(ocelot, lag);
-- 
2.25.1

[PATCH v2 net-next 08/14] net: mscc: ocelot: use "lag" variable name in ocelot_bridge_stp_state_set

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

In anticipation of further simplification, make it more clear what we're
iterating over.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes in v2:
None.

 drivers/net/ethernet/mscc/ocelot.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index ce52bf892f9b..915cf81f602a 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -897,7 +897,7 @@ static u32 ocelot_get_bond_mask(struct ocelot *ocelot, struct net_device *bond)
 void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state)
 {
 	u32 port_cfg;
-	int p, i;
+	int p;
 
 	if (!(BIT(port) & ocelot->bridge_mask))
 		return;
@@ -921,14 +921,17 @@ void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state)
 	ocelot_write_gix(ocelot, port_cfg, ANA_PORT_PORT_CFG, port);
 
 	/* Apply FWD mask. The loop is needed to add/remove the current port as
-	 * a source for the other ports.
+	 * a source for the other ports. If the source port is in a bond, then
+	 * all the other ports from that bond need to be removed from this
+	 * source port's forwarding mask.
 	 */
 	for (p = 0; p < ocelot->num_phys_ports; p++) {
 		if (ocelot->bridge_fwd_mask & BIT(p)) {
 			unsigned long mask = ocelot->bridge_fwd_mask & ~BIT(p);
+			int lag;
 
-			for (i = 0; i < ocelot->num_phys_ports; i++) {
-				unsigned long bond_mask = ocelot->lags[i];
+			for (lag = 0; lag < ocelot->num_phys_ports; lag++) {
+				unsigned long bond_mask = ocelot->lags[lag];
 
 				if (!bond_mask)
 					continue;
-- 
2.25.1

[PATCH v2 net-next 09/14] net: mscc: ocelot: reapply bridge forwarding mask on bonding join/leave

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

Applying the bridge forwarding mask currently is done only on the STP
state changes for any port. But it depends on both STP state changes,
and bonding interface state changes. Export the bit that recalculates
the forwarding mask so that it could be reused, and call it when a port
starts and stops offloading a bonding interface.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
Reviewed-by: Alexandre Belloni <alexandre.belloni@bootlin.com>
---
Changes in v2:
Found a better strategic placement for the ocelot_apply_bridge_fwd_mask
function (i.e. just code ordering).

 drivers/net/ethernet/mscc/ocelot.c | 68 +++++++++++++++++-------------
 1 file changed, 38 insertions(+), 30 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 915cf81f602a..8ab74325eb78 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -894,40 +894,18 @@ static u32 ocelot_get_bond_mask(struct ocelot *ocelot, struct net_device *bond)
 	return bond_mask;
 }
 
-void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state)
+static void ocelot_apply_bridge_fwd_mask(struct ocelot *ocelot)
 {
-	u32 port_cfg;
-	int p;
-
-	if (!(BIT(port) & ocelot->bridge_mask))
-		return;
-
-	port_cfg = ocelot_read_gix(ocelot, ANA_PORT_PORT_CFG, port);
-
-	switch (state) {
-	case BR_STATE_FORWARDING:
-		ocelot->bridge_fwd_mask |= BIT(port);
-		fallthrough;
-	case BR_STATE_LEARNING:
-		port_cfg |= ANA_PORT_PORT_CFG_LEARN_ENA;
-		break;
-
-	default:
-		port_cfg &= ~ANA_PORT_PORT_CFG_LEARN_ENA;
-		ocelot->bridge_fwd_mask &= ~BIT(port);
-		break;
-	}
-
-	ocelot_write_gix(ocelot, port_cfg, ANA_PORT_PORT_CFG, port);
+	int port;
 
 	/* Apply FWD mask. The loop is needed to add/remove the current port as
 	 * a source for the other ports. If the source port is in a bond, then
 	 * all the other ports from that bond need to be removed from this
 	 * source port's forwarding mask.
 	 */
-	for (p = 0; p < ocelot->num_phys_ports; p++) {
-		if (ocelot->bridge_fwd_mask & BIT(p)) {
-			unsigned long mask = ocelot->bridge_fwd_mask & ~BIT(p);
+	for (port = 0; port < ocelot->num_phys_ports; port++) {
+		if (ocelot->bridge_fwd_mask & BIT(port)) {
+			unsigned long mask = ocelot->bridge_fwd_mask & ~BIT(port);
 			int lag;
 
 			for (lag = 0; lag < ocelot->num_phys_ports; lag++) {
@@ -936,20 +914,48 @@ void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state)
 				if (!bond_mask)
 					continue;
 
-				if (bond_mask & BIT(p)) {
+				if (bond_mask & BIT(port)) {
 					mask &= ~bond_mask;
 					break;
 				}
 			}
 
 			ocelot_write_rix(ocelot, mask,
-					 ANA_PGID_PGID, PGID_SRC + p);
+					 ANA_PGID_PGID, PGID_SRC + port);
 		} else {
 			ocelot_write_rix(ocelot, 0,
-					 ANA_PGID_PGID, PGID_SRC + p);
+					 ANA_PGID_PGID, PGID_SRC + port);
 		}
 	}
 }
+
+void ocelot_bridge_stp_state_set(struct ocelot *ocelot, int port, u8 state)
+{
+	u32 port_cfg;
+
+	if (!(BIT(port) & ocelot->bridge_mask))
+		return;
+
+	port_cfg = ocelot_read_gix(ocelot, ANA_PORT_PORT_CFG, port);
+
+	switch (state) {
+	case BR_STATE_FORWARDING:
+		ocelot->bridge_fwd_mask |= BIT(port);
+		fallthrough;
+	case BR_STATE_LEARNING:
+		port_cfg |= ANA_PORT_PORT_CFG_LEARN_ENA;
+		break;
+
+	default:
+		port_cfg &= ~ANA_PORT_PORT_CFG_LEARN_ENA;
+		ocelot->bridge_fwd_mask &= ~BIT(port);
+		break;
+	}
+
+	ocelot_write_gix(ocelot, port_cfg, ANA_PORT_PORT_CFG, port);
+
+	ocelot_apply_bridge_fwd_mask(ocelot);
+}
 EXPORT_SYMBOL(ocelot_bridge_stp_state_set);
 
 void ocelot_set_ageing_time(struct ocelot *ocelot, unsigned int msecs)
@@ -1300,6 +1306,7 @@ int ocelot_port_lag_join(struct ocelot *ocelot, int port,
 	}
 
 	ocelot_setup_lag(ocelot, lag);
+	ocelot_apply_bridge_fwd_mask(ocelot);
 	ocelot_set_aggr_pgids(ocelot);
 
 	return 0;
@@ -1335,6 +1342,7 @@ void ocelot_port_lag_leave(struct ocelot *ocelot, int port,
 	ocelot_write_gix(ocelot, port_cfg | ANA_PORT_PORT_CFG_PORTID_VAL(port),
 			 ANA_PORT_PORT_CFG, port);
 
+	ocelot_apply_bridge_fwd_mask(ocelot);
 	ocelot_set_aggr_pgids(ocelot);
 }
 EXPORT_SYMBOL(ocelot_port_lag_leave);
-- 
2.25.1

[PATCH v2 net-next 10/14] net: mscc: ocelot: set up logical port IDs centrally

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The setup of logical port IDs is done in two places: from the inconclusively
named ocelot_setup_lag and from ocelot_port_lag_leave, a function that
also calls ocelot_setup_lag (which apparently does an incomplete setup
of the LAG).

To improve this situation, we can rename ocelot_setup_lag into
ocelot_setup_logical_port_ids, and drop the "lag" argument. It will now
set up the logical port IDs of all switch ports, which may be just
slightly more inefficient but more maintainable.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
None.

 drivers/net/ethernet/mscc/ocelot.c | 47 ++++++++++++++++++------------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 8ab74325eb78..1bcf68bd1bff 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1260,20 +1260,36 @@ static void ocelot_set_aggr_pgids(struct ocelot *ocelot)
 	}
 }
 
-static void ocelot_setup_lag(struct ocelot *ocelot, int lag)
+/* When offloading a bonding interface, the switch ports configured under the
+ * same bond must have the same logical port ID, equal to the physical port ID
+ * of the lowest numbered physical port in that bond. Otherwise, in standalone/
+ * bridged mode, each port has a logical port ID equal to its physical port ID.
+ */
+static void ocelot_setup_logical_port_ids(struct ocelot *ocelot)
 {
-	unsigned long bond_mask = ocelot->lags[lag];
-	unsigned int p;
+	int port;
 
-	for_each_set_bit(p, &bond_mask, ocelot->num_phys_ports) {
-		u32 port_cfg = ocelot_read_gix(ocelot, ANA_PORT_PORT_CFG, p);
+	for (port = 0; port < ocelot->num_phys_ports; port++) {
+		struct ocelot_port *ocelot_port = ocelot->ports[port];
+		struct net_device *bond;
+
+		if (!ocelot_port)
+			continue;
 
-		port_cfg &= ~ANA_PORT_PORT_CFG_PORTID_VAL_M;
+		bond = ocelot_port->bond;
+		if (bond) {
+			int lag = __ffs(ocelot_get_bond_mask(ocelot, bond));
 
-		/* Use lag port as logical port for port i */
-		ocelot_write_gix(ocelot, port_cfg |
-				 ANA_PORT_PORT_CFG_PORTID_VAL(lag),
-				 ANA_PORT_PORT_CFG, p);
+			ocelot_rmw_gix(ocelot,
+				       ANA_PORT_PORT_CFG_PORTID_VAL(lag),
+				       ANA_PORT_PORT_CFG_PORTID_VAL_M,
+				       ANA_PORT_PORT_CFG, port);
+		} else {
+			ocelot_rmw_gix(ocelot,
+				       ANA_PORT_PORT_CFG_PORTID_VAL(port),
+				       ANA_PORT_PORT_CFG_PORTID_VAL_M,
+				       ANA_PORT_PORT_CFG, port);
+		}
 	}
 }
 
@@ -1305,7 +1321,7 @@ int ocelot_port_lag_join(struct ocelot *ocelot, int port,
 		ocelot->lags[lag] |= BIT(port);
 	}
 
-	ocelot_setup_lag(ocelot, lag);
+	ocelot_setup_logical_port_ids(ocelot);
 	ocelot_apply_bridge_fwd_mask(ocelot);
 	ocelot_set_aggr_pgids(ocelot);
 
@@ -1316,7 +1332,6 @@ EXPORT_SYMBOL(ocelot_port_lag_join);
 void ocelot_port_lag_leave(struct ocelot *ocelot, int port,
 			   struct net_device *bond)
 {
-	u32 port_cfg;
 	int i;
 
 	ocelot->ports[port]->bond = NULL;
@@ -1333,15 +1348,9 @@ void ocelot_port_lag_leave(struct ocelot *ocelot, int port,
 
 		ocelot->lags[n] = ocelot->lags[port];
 		ocelot->lags[port] = 0;
-
-		ocelot_setup_lag(ocelot, n);
 	}
 
-	port_cfg = ocelot_read_gix(ocelot, ANA_PORT_PORT_CFG, port);
-	port_cfg &= ~ANA_PORT_PORT_CFG_PORTID_VAL_M;
-	ocelot_write_gix(ocelot, port_cfg | ANA_PORT_PORT_CFG_PORTID_VAL(port),
-			 ANA_PORT_PORT_CFG, port);
-
+	ocelot_setup_logical_port_ids(ocelot);
 	ocelot_apply_bridge_fwd_mask(ocelot);
 	ocelot_set_aggr_pgids(ocelot);
 }
-- 
2.25.1

[PATCH v2 net-next 11/14] net: mscc: ocelot: drop the use of the "lags" array

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

We can now simplify the implementation by always using ocelot_get_bond_mask
to look up the other ports that are offloading the same bonding interface
as us.

In ocelot_set_aggr_pgids, the code had a way to uniquely iterate through
LAGs. We need to achieve the same behavior by marking each LAG as visited,
which we do now by temporarily allocating an array of pointers to bonding
uppers of each port, and marking each bonding upper as NULL once it has
been treated by the first port that is a member. And because we now do
some dynamic allocation, we need to propagate errors from
ocelot_set_aggr_pgid all the way to ocelot_port_lag_leave.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
Context looks a bit different.

 drivers/net/ethernet/mscc/ocelot.c     | 104 ++++++++++---------------
 drivers/net/ethernet/mscc/ocelot.h     |   4 +-
 drivers/net/ethernet/mscc/ocelot_net.c |   4 +-
 include/soc/mscc/ocelot.h              |   2 -
 4 files changed, 47 insertions(+), 67 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 1bcf68bd1bff..fa2ebfbb33eb 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -904,21 +904,17 @@ static void ocelot_apply_bridge_fwd_mask(struct ocelot *ocelot)
 	 * source port's forwarding mask.
 	 */
 	for (port = 0; port < ocelot->num_phys_ports; port++) {
-		if (ocelot->bridge_fwd_mask & BIT(port)) {
-			unsigned long mask = ocelot->bridge_fwd_mask & ~BIT(port);
-			int lag;
+		struct ocelot_port *ocelot_port = ocelot->ports[port];
 
-			for (lag = 0; lag < ocelot->num_phys_ports; lag++) {
-				unsigned long bond_mask = ocelot->lags[lag];
+		if (!ocelot_port)
+			continue;
 
-				if (!bond_mask)
-					continue;
+		if (ocelot->bridge_fwd_mask & BIT(port)) {
+			unsigned long mask = ocelot->bridge_fwd_mask & ~BIT(port);
+			struct net_device *bond = ocelot_port->bond;
 
-				if (bond_mask & BIT(port)) {
-					mask &= ~bond_mask;
-					break;
-				}
-			}
+			if (bond)
+				mask &= ~ocelot_get_bond_mask(ocelot, bond);
 
 			ocelot_write_rix(ocelot, mask,
 					 ANA_PGID_PGID, PGID_SRC + port);
@@ -1219,10 +1215,16 @@ int ocelot_port_bridge_leave(struct ocelot *ocelot, int port,
 }
 EXPORT_SYMBOL(ocelot_port_bridge_leave);
 
-static void ocelot_set_aggr_pgids(struct ocelot *ocelot)
+static int ocelot_set_aggr_pgids(struct ocelot *ocelot)
 {
+	struct net_device **bonds;
 	int i, port, lag;
 
+	bonds = kcalloc(ocelot->num_phys_ports, sizeof(struct net_device *),
+			GFP_KERNEL);
+	if (!bonds)
+		return -ENOMEM;
+
 	/* Reset destination and aggregation PGIDS */
 	for_each_unicast_dest_pgid(ocelot, port)
 		ocelot_write_rix(ocelot, BIT(port), ANA_PGID_PGID, port);
@@ -1231,16 +1233,26 @@ static void ocelot_set_aggr_pgids(struct ocelot *ocelot)
 		ocelot_write_rix(ocelot, GENMASK(ocelot->num_phys_ports - 1, 0),
 				 ANA_PGID_PGID, i);
 
+	for (port = 0; port < ocelot->num_phys_ports; port++) {
+		struct ocelot_port *ocelot_port = ocelot->ports[port];
+
+		if (!ocelot_port)
+			continue;
+
+		bonds[port] = ocelot_port->bond;
+	}
+
 	/* Now, set PGIDs for each LAG */
 	for (lag = 0; lag < ocelot->num_phys_ports; lag++) {
 		unsigned long bond_mask;
 		int aggr_count = 0;
 		u8 aggr_idx[16];
 
-		bond_mask = ocelot->lags[lag];
-		if (!bond_mask)
+		if (!bonds[lag])
 			continue;
 
+		bond_mask = ocelot_get_bond_mask(ocelot, bonds[lag]);
+
 		for_each_set_bit(port, &bond_mask, ocelot->num_phys_ports) {
 			// Destination mask
 			ocelot_write_rix(ocelot, bond_mask,
@@ -1257,7 +1269,19 @@ static void ocelot_set_aggr_pgids(struct ocelot *ocelot)
 			ac |= BIT(aggr_idx[i % aggr_count]);
 			ocelot_write_rix(ocelot, ac, ANA_PGID_PGID, i);
 		}
+
+		/* Mark the bonding interface as visited to avoid applying
+		 * the same config again
+		 */
+		for (i = lag + 1; i < ocelot->num_phys_ports; i++)
+			if (bonds[i] == bonds[lag])
+				bonds[i] = NULL;
+
+		bonds[lag] = NULL;
 	}
+
+	kfree(bonds);
+	return 0;
 }
 
 /* When offloading a bonding interface, the switch ports configured under the
@@ -1297,62 +1321,25 @@ int ocelot_port_lag_join(struct ocelot *ocelot, int port,
 			 struct net_device *bond,
 			 struct netdev_lag_upper_info *info)
 {
-	u32 bond_mask = 0;
-	int lag;
-
 	if (info->tx_type != NETDEV_LAG_TX_TYPE_HASH)
 		return -EOPNOTSUPP;
 
 	ocelot->ports[port]->bond = bond;
 
-	bond_mask = ocelot_get_bond_mask(ocelot, bond);
-
-	lag = __ffs(bond_mask);
-
-	/* If the new port is the lowest one, use it as the logical port from
-	 * now on
-	 */
-	if (port == lag) {
-		ocelot->lags[port] = bond_mask;
-		bond_mask &= ~BIT(port);
-		if (bond_mask)
-			ocelot->lags[__ffs(bond_mask)] = 0;
-	} else {
-		ocelot->lags[lag] |= BIT(port);
-	}
-
 	ocelot_setup_logical_port_ids(ocelot);
 	ocelot_apply_bridge_fwd_mask(ocelot);
-	ocelot_set_aggr_pgids(ocelot);
-
-	return 0;
+	return ocelot_set_aggr_pgids(ocelot);
 }
 EXPORT_SYMBOL(ocelot_port_lag_join);
 
-void ocelot_port_lag_leave(struct ocelot *ocelot, int port,
-			   struct net_device *bond)
+int ocelot_port_lag_leave(struct ocelot *ocelot, int port,
+			  struct net_device *bond)
 {
-	int i;
-
 	ocelot->ports[port]->bond = NULL;
 
-	/* Remove port from any lag */
-	for (i = 0; i < ocelot->num_phys_ports; i++)
-		ocelot->lags[i] &= ~BIT(port);
-
-	/* if it was the logical port of the lag, move the lag config to the
-	 * next port
-	 */
-	if (ocelot->lags[port]) {
-		int n = __ffs(ocelot->lags[port]);
-
-		ocelot->lags[n] = ocelot->lags[port];
-		ocelot->lags[port] = 0;
-	}
-
 	ocelot_setup_logical_port_ids(ocelot);
 	ocelot_apply_bridge_fwd_mask(ocelot);
-	ocelot_set_aggr_pgids(ocelot);
+	return ocelot_set_aggr_pgids(ocelot);
 }
 EXPORT_SYMBOL(ocelot_port_lag_leave);
 
@@ -1531,11 +1518,6 @@ int ocelot_init(struct ocelot *ocelot)
 		}
 	}
 
-	ocelot->lags = devm_kcalloc(ocelot->dev, ocelot->num_phys_ports,
-				    sizeof(u32), GFP_KERNEL);
-	if (!ocelot->lags)
-		return -ENOMEM;
-
 	ocelot->stats = devm_kcalloc(ocelot->dev,
 				     ocelot->num_phys_ports * ocelot->num_stats,
 				     sizeof(u64), GFP_KERNEL);
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index b6c9ddcee554..c018a54f4e49 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -112,8 +112,8 @@ int ocelot_mact_forget(struct ocelot *ocelot,
 int ocelot_port_lag_join(struct ocelot *ocelot, int port,
 			 struct net_device *bond,
 			 struct netdev_lag_upper_info *info);
-void ocelot_port_lag_leave(struct ocelot *ocelot, int port,
-			   struct net_device *bond);
+int ocelot_port_lag_leave(struct ocelot *ocelot, int port,
+			  struct net_device *bond);
 struct net_device *ocelot_port_to_netdev(struct ocelot *ocelot, int port);
 int ocelot_netdev_to_port(struct net_device *dev);
 
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index f246f8fc535d..871bfbaa8ff1 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -1138,8 +1138,8 @@ static int ocelot_netdevice_changeupper(struct net_device *dev,
 				err = 0;
 			}
 		} else {
-			ocelot_port_lag_leave(ocelot, port,
-					      info->upper_dev);
+			err = ocelot_port_lag_leave(ocelot, port,
+						    info->upper_dev);
 		}
 	}
 
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index d2c587f099c8..1bb16527f490 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -656,8 +656,6 @@ struct ocelot {
 	enum ocelot_tag_prefix		inj_prefix;
 	enum ocelot_tag_prefix		xtr_prefix;
 
-	u32				*lags;
-
 	struct list_head		multicast;
 	struct list_head		pgids;
 
-- 
2.25.1

[PATCH v2 net-next 12/14] net: mscc: ocelot: rename aggr_count to num_ports_in_lag

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

It makes it a bit easier to read and understand the code that deals with
balancing the 16 aggregation codes among the ports in a certain LAG.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
None.

 drivers/net/ethernet/mscc/ocelot.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index fa2ebfbb33eb..78a468d39e9b 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1244,8 +1244,8 @@ static int ocelot_set_aggr_pgids(struct ocelot *ocelot)
 
 	/* Now, set PGIDs for each LAG */
 	for (lag = 0; lag < ocelot->num_phys_ports; lag++) {
+		int num_ports_in_lag = 0;
 		unsigned long bond_mask;
-		int aggr_count = 0;
 		u8 aggr_idx[16];
 
 		if (!bonds[lag])
@@ -1257,8 +1257,7 @@ static int ocelot_set_aggr_pgids(struct ocelot *ocelot)
 			// Destination mask
 			ocelot_write_rix(ocelot, bond_mask,
 					 ANA_PGID_PGID, port);
-			aggr_idx[aggr_count] = port;
-			aggr_count++;
+			aggr_idx[num_ports_in_lag++] = port;
 		}
 
 		for_each_aggr_pgid(ocelot, i) {
@@ -1266,7 +1265,7 @@ static int ocelot_set_aggr_pgids(struct ocelot *ocelot)
 
 			ac = ocelot_read_rix(ocelot, ANA_PGID_PGID, i);
 			ac &= ~bond_mask;
-			ac |= BIT(aggr_idx[i % aggr_count]);
+			ac |= BIT(aggr_idx[i % num_ports_in_lag]);
 			ocelot_write_rix(ocelot, ac, ANA_PGID_PGID, i);
 		}
 
-- 
2.25.1

[PATCH v2 net-next 13/14] net: mscc: ocelot: rebalance LAGs on link up/down events

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

At present there is an issue when ocelot is offloading a bonding
interface, but one of the links of the physical ports goes down. Traffic
keeps being hashed towards that destination, and of course gets dropped
on egress.

Monitor the netdev notifier events emitted by the bonding driver for
changes in the physical state of lower interfaces, to determine which
ports are active and which ones are no longer.

Then extend ocelot_get_bond_mask to return either the configured bonding
interfaces, or the active ones, depending on a boolean argument. The
code that does rebalancing only needs to do so among the active ports,
whereas the bridge forwarding mask and the logical port IDs still need
to look at the permanently bonded ports.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
- Adapt to the merged version of the DSA API, which now passes just a
  bool lag_tx_active in .port_lag_change instead of the full struct
  netdev_lag_lower_state_info *info.
- Renamed "just_active_ports" -> "only_active_ports"

 drivers/net/ethernet/mscc/ocelot.c     | 38 ++++++++++++++++++++------
 drivers/net/ethernet/mscc/ocelot.h     |  1 +
 drivers/net/ethernet/mscc/ocelot_net.c | 30 ++++++++++++++++++++
 include/soc/mscc/ocelot.h              |  1 +
 4 files changed, 61 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index 78a468d39e9b..c31a63d9fe2a 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -876,7 +876,8 @@ int ocelot_get_ts_info(struct ocelot *ocelot, int port,
 }
 EXPORT_SYMBOL(ocelot_get_ts_info);
 
-static u32 ocelot_get_bond_mask(struct ocelot *ocelot, struct net_device *bond)
+static u32 ocelot_get_bond_mask(struct ocelot *ocelot, struct net_device *bond,
+				bool only_active_ports)
 {
 	u32 bond_mask = 0;
 	int port;
@@ -887,8 +888,12 @@ static u32 ocelot_get_bond_mask(struct ocelot *ocelot, struct net_device *bond)
 		if (!ocelot_port)
 			continue;
 
-		if (ocelot_port->bond == bond)
+		if (ocelot_port->bond == bond) {
+			if (only_active_ports && !ocelot_port->lag_tx_active)
+				continue;
+
 			bond_mask |= BIT(port);
+		}
 	}
 
 	return bond_mask;
@@ -914,7 +919,7 @@ static void ocelot_apply_bridge_fwd_mask(struct ocelot *ocelot)
 			struct net_device *bond = ocelot_port->bond;
 
 			if (bond)
-				mask &= ~ocelot_get_bond_mask(ocelot, bond);
+				mask &= ~ocelot_get_bond_mask(ocelot, bond, false);
 
 			ocelot_write_rix(ocelot, mask,
 					 ANA_PGID_PGID, PGID_SRC + port);
@@ -1242,22 +1247,22 @@ static int ocelot_set_aggr_pgids(struct ocelot *ocelot)
 		bonds[port] = ocelot_port->bond;
 	}
 
-	/* Now, set PGIDs for each LAG */
+	/* Now, set PGIDs for each active LAG */
 	for (lag = 0; lag < ocelot->num_phys_ports; lag++) {
-		int num_ports_in_lag = 0;
+		int num_active_ports = 0;
 		unsigned long bond_mask;
 		u8 aggr_idx[16];
 
 		if (!bonds[lag])
 			continue;
 
-		bond_mask = ocelot_get_bond_mask(ocelot, bonds[lag]);
+		bond_mask = ocelot_get_bond_mask(ocelot, bonds[lag], true);
 
 		for_each_set_bit(port, &bond_mask, ocelot->num_phys_ports) {
 			// Destination mask
 			ocelot_write_rix(ocelot, bond_mask,
 					 ANA_PGID_PGID, port);
-			aggr_idx[num_ports_in_lag++] = port;
+			aggr_idx[num_active_ports++] = port;
 		}
 
 		for_each_aggr_pgid(ocelot, i) {
@@ -1265,7 +1270,11 @@ static int ocelot_set_aggr_pgids(struct ocelot *ocelot)
 
 			ac = ocelot_read_rix(ocelot, ANA_PGID_PGID, i);
 			ac &= ~bond_mask;
-			ac |= BIT(aggr_idx[i % num_ports_in_lag]);
+			/* Don't do division by zero if there was no active
+			 * port. Just make all aggregation codes zero.
+			 */
+			if (num_active_ports)
+				ac |= BIT(aggr_idx[i % num_active_ports]);
 			ocelot_write_rix(ocelot, ac, ANA_PGID_PGID, i);
 		}
 
@@ -1301,7 +1310,8 @@ static void ocelot_setup_logical_port_ids(struct ocelot *ocelot)
 
 		bond = ocelot_port->bond;
 		if (bond) {
-			int lag = __ffs(ocelot_get_bond_mask(ocelot, bond));
+			int lag = __ffs(ocelot_get_bond_mask(ocelot, bond,
+							     false));
 
 			ocelot_rmw_gix(ocelot,
 				       ANA_PORT_PORT_CFG_PORTID_VAL(lag),
@@ -1342,6 +1352,16 @@ int ocelot_port_lag_leave(struct ocelot *ocelot, int port,
 }
 EXPORT_SYMBOL(ocelot_port_lag_leave);
 
+int ocelot_port_lag_change(struct ocelot *ocelot, int port, bool lag_tx_active)
+{
+	struct ocelot_port *ocelot_port = ocelot->ports[port];
+
+	ocelot_port->lag_tx_active = lag_tx_active;
+
+	/* Rebalance the LAGs */
+	return ocelot_set_aggr_pgids(ocelot);
+}
+
 /* Configure the maximum SDU (L2 payload) on RX to the value specified in @sdu.
  * The length of VLAN tags is accounted for automatically via DEV_MAC_TAGS_CFG.
  * In the special case that it's the NPI port that we're configuring, the
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index c018a54f4e49..fc055525fd88 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -114,6 +114,7 @@ int ocelot_port_lag_join(struct ocelot *ocelot, int port,
 			 struct netdev_lag_upper_info *info);
 int ocelot_port_lag_leave(struct ocelot *ocelot, int port,
 			  struct net_device *bond);
+int ocelot_port_lag_change(struct ocelot *ocelot, int port, bool lag_tx_active);
 struct net_device *ocelot_port_to_netdev(struct ocelot *ocelot, int port);
 int ocelot_netdev_to_port(struct net_device *dev);
 
diff --git a/drivers/net/ethernet/mscc/ocelot_net.c b/drivers/net/ethernet/mscc/ocelot_net.c
index 871bfbaa8ff1..3cba1c2de6fe 100644
--- a/drivers/net/ethernet/mscc/ocelot_net.c
+++ b/drivers/net/ethernet/mscc/ocelot_net.c
@@ -1163,6 +1163,27 @@ ocelot_netdevice_lag_changeupper(struct net_device *dev,
 	return NOTIFY_DONE;
 }
 
+static int
+ocelot_netdevice_changelowerstate(struct net_device *dev,
+				  struct netdev_lag_lower_state_info *info)
+{
+	struct ocelot_port_private *priv = netdev_priv(dev);
+	bool is_active = info->link_up && info->tx_enabled;
+	struct ocelot_port *ocelot_port = &priv->port;
+	struct ocelot *ocelot = ocelot_port->ocelot;
+	int port = priv->chip_port;
+	int err;
+
+	if (!ocelot_port->bond)
+		return NOTIFY_DONE;
+
+	if (ocelot_port->lag_tx_active == is_active)
+		return 0;
+
+	err = ocelot_port_lag_change(ocelot, port, is_active);
+	return notifier_from_errno(err);
+}
+
 static int ocelot_netdevice_event(struct notifier_block *unused,
 				  unsigned long event, void *ptr)
 {
@@ -1180,6 +1201,15 @@ static int ocelot_netdevice_event(struct notifier_block *unused,
 
 		break;
 	}
+	case NETDEV_CHANGELOWERSTATE: {
+		struct netdev_notifier_changelowerstate_info *info = ptr;
+
+		if (!ocelot_netdevice_dev_check(dev))
+			break;
+
+		return ocelot_netdevice_changelowerstate(dev,
+							 info->lower_state_info);
+	}
 	default:
 		break;
 	}
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 1bb16527f490..7bd7c58a7490 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -612,6 +612,7 @@ struct ocelot_port {
 	u8				*xmit_template;
 
 	struct net_device		*bond;
+	bool				lag_tx_active;
 };
 
 struct ocelot {
-- 
2.25.1

[PATCH v2 net-next 14/14] net: dsa: felix: propagate the LAG offload ops towards the ocelot lib

[Vladimir Oltean]

From: Vladimir Oltean <vladimir.oltean@nxp.com>

The ocelot switch has been supporting LAG offload since its initial
commit, however felix could not make use of that, due to lack of a LAG
abstraction in DSA. Now that we have that, let's forward DSA's calls
towards the ocelot library, who will deal with setting up the bonding.

Note that ocelot_port_lag_leave can return an error due to memory
allocation but we are currently ignoring that, because the DSA method
returns void.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
Changes in v2:
s/lag_dev/bond/g

 drivers/net/dsa/ocelot/felix.c     | 28 ++++++++++++++++++++++++++++
 drivers/net/ethernet/mscc/ocelot.c |  1 +
 drivers/net/ethernet/mscc/ocelot.h |  6 ------
 include/soc/mscc/ocelot.h          |  6 ++++++
 4 files changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/ocelot/felix.c b/drivers/net/dsa/ocelot/felix.c
index 2c6ac2b137ec..af867535f9eb 100644
--- a/drivers/net/dsa/ocelot/felix.c
+++ b/drivers/net/dsa/ocelot/felix.c
@@ -105,6 +105,31 @@ static void felix_bridge_leave(struct dsa_switch *ds, int port,
 	ocelot_port_bridge_leave(ocelot, port, br);
 }
 
+static int felix_lag_join(struct dsa_switch *ds, int port,
+			  struct net_device *bond,
+			  struct netdev_lag_upper_info *info)
+{
+	struct ocelot *ocelot = ds->priv;
+
+	return ocelot_port_lag_join(ocelot, port, bond, info);
+}
+
+static int felix_lag_leave(struct dsa_switch *ds, int port,
+			   struct net_device *bond)
+{
+	struct ocelot *ocelot = ds->priv;
+
+	return ocelot_port_lag_leave(ocelot, port, bond);
+}
+
+static int felix_lag_change(struct dsa_switch *ds, int port)
+{
+	struct dsa_port *dp = dsa_to_port(ds, port);
+	struct ocelot *ocelot = ds->priv;
+
+	return ocelot_port_lag_change(ocelot, port, dp->lag_tx_enabled);
+}
+
 static int felix_vlan_prepare(struct dsa_switch *ds, int port,
 			      const struct switchdev_obj_port_vlan *vlan)
 {
@@ -881,6 +906,9 @@ const struct dsa_switch_ops felix_switch_ops = {
 	.port_mdb_del			= felix_mdb_del,
 	.port_bridge_join		= felix_bridge_join,
 	.port_bridge_leave		= felix_bridge_leave,
+	.port_lag_join			= felix_lag_join,
+	.port_lag_leave			= felix_lag_leave,
+	.port_lag_change		= felix_lag_change,
 	.port_stp_state_set		= felix_bridge_stp_state_set,
 	.port_vlan_filtering		= felix_vlan_filtering,
 	.port_vlan_add			= felix_vlan_add,
diff --git a/drivers/net/ethernet/mscc/ocelot.c b/drivers/net/ethernet/mscc/ocelot.c
index c31a63d9fe2a..b2fdee8a713c 100644
--- a/drivers/net/ethernet/mscc/ocelot.c
+++ b/drivers/net/ethernet/mscc/ocelot.c
@@ -1361,6 +1361,7 @@ int ocelot_port_lag_change(struct ocelot *ocelot, int port, bool lag_tx_active)
 	/* Rebalance the LAGs */
 	return ocelot_set_aggr_pgids(ocelot);
 }
+EXPORT_SYMBOL(ocelot_port_lag_change);
 
 /* Configure the maximum SDU (L2 payload) on RX to the value specified in @sdu.
  * The length of VLAN tags is accounted for automatically via DEV_MAC_TAGS_CFG.
diff --git a/drivers/net/ethernet/mscc/ocelot.h b/drivers/net/ethernet/mscc/ocelot.h
index fc055525fd88..e3352ae7b11f 100644
--- a/drivers/net/ethernet/mscc/ocelot.h
+++ b/drivers/net/ethernet/mscc/ocelot.h
@@ -109,12 +109,6 @@ int ocelot_mact_learn(struct ocelot *ocelot, int port,
 		      unsigned int vid, enum macaccess_entry_type type);
 int ocelot_mact_forget(struct ocelot *ocelot,
 		       const unsigned char mac[ETH_ALEN], unsigned int vid);
-int ocelot_port_lag_join(struct ocelot *ocelot, int port,
-			 struct net_device *bond,
-			 struct netdev_lag_upper_info *info);
-int ocelot_port_lag_leave(struct ocelot *ocelot, int port,
-			  struct net_device *bond);
-int ocelot_port_lag_change(struct ocelot *ocelot, int port, bool lag_tx_active);
 struct net_device *ocelot_port_to_netdev(struct ocelot *ocelot, int port);
 int ocelot_netdev_to_port(struct net_device *dev);
 
diff --git a/include/soc/mscc/ocelot.h b/include/soc/mscc/ocelot.h
index 7bd7c58a7490..d9b5d31bb181 100644
--- a/include/soc/mscc/ocelot.h
+++ b/include/soc/mscc/ocelot.h
@@ -796,6 +796,12 @@ int ocelot_port_mdb_add(struct ocelot *ocelot, int port,
 			const struct switchdev_obj_port_mdb *mdb);
 int ocelot_port_mdb_del(struct ocelot *ocelot, int port,
 			const struct switchdev_obj_port_mdb *mdb);
+int ocelot_port_lag_join(struct ocelot *ocelot, int port,
+			 struct net_device *bond,
+			 struct netdev_lag_upper_info *info);
+int ocelot_port_lag_leave(struct ocelot *ocelot, int port,
+			  struct net_device *bond);
+int ocelot_port_lag_change(struct ocelot *ocelot, int port, bool lag_tx_active);
 
 int ocelot_devlink_sb_register(struct ocelot *ocelot);
 void ocelot_devlink_sb_unregister(struct ocelot *ocelot);
-- 
2.25.1

Re: [PATCH v2 net-next 00/14] LAG offload for Ocelot DSA switches

[Vladimir Oltean]

On Sat, Jan 16, 2021 at 02:59:29AM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> This patch series reworks the ocelot switchdev driver such that it could
> share the same implementation for LAG offload as the felix DSA driver.

Jakub, I sent these patches a few hours early because I didn't want to
wait for the devlink-sb series to get accepted. Now that it did, can you
move the patches back from the RFC state into review, or do I need to
resend them?

Re: [PATCH v2 net-next 00/14] LAG offload for Ocelot DSA switches

[Jakub Kicinski]

On Sat, 16 Jan 2021 17:51:03 +0200 Vladimir Oltean wrote:
> On Sat, Jan 16, 2021 at 02:59:29AM +0200, Vladimir Oltean wrote:
> > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > 
> > This patch series reworks the ocelot switchdev driver such that it could
> > share the same implementation for LAG offload as the felix DSA driver.  
> 
> Jakub, I sent these patches a few hours early because I didn't want to
> wait for the devlink-sb series to get accepted. Now that it did, can you
> move the patches back from the RFC state into review, or do I need to
> resend them?

I tried to convince the build bot to take a look at this series again,
but failed :( Let me look at the patches now, but you'll have to repost
to get them merged.

Re: [PATCH v2 net-next 00/14] LAG offload for Ocelot DSA switches

[Jakub Kicinski]

On Sat, 16 Jan 2021 17:25:10 -0800 Jakub Kicinski wrote:
> On Sat, 16 Jan 2021 17:51:03 +0200 Vladimir Oltean wrote:
> > On Sat, Jan 16, 2021 at 02:59:29AM +0200, Vladimir Oltean wrote:  
> > > From: Vladimir Oltean <vladimir.oltean@nxp.com>
> > > 
> > > This patch series reworks the ocelot switchdev driver such that it could
> > > share the same implementation for LAG offload as the felix DSA driver.    
> > 
> > Jakub, I sent these patches a few hours early because I didn't want to
> > wait for the devlink-sb series to get accepted. Now that it did, can you
> > move the patches back from the RFC state into review, or do I need to
> > resend them?  
> 
> I tried to convince the build bot to take a look at this series again,
> but failed :( Let me look at the patches now, but you'll have to repost
> to get them merged.

The code LGTM, FWIW. I'm a little surprised you opted in for allocation
in ocelot_set_aggr_pgids() but admittedly that makes the code much
simpler than trying to for instance use lower bits of pointers as
markers, or even a bitmask on the stack..