netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] dpaa2-switch: small improvements
@ 2023-12-04 16:35 Ioana Ciornei
  2023-12-04 16:35 ` [PATCH 1/8] dpaa2-switch: set interface MAC address only on endpoint change Ioana Ciornei
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-04 16:35 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, netdev; +Cc: Ioana Ciornei

This patch set consists of a series of small improvements on the
dpaa2-switch driver ranging from adding some more verbosity when
encountering errors to reorganizing code to be easily extensible.

Ioana Ciornei (8):
  dpaa2-switch: set interface MAC address only on endpoint change
  dpaa2-switch: declare the netdev as IFF_LIVE_ADDR_CHANGE capable
  dpaa2-switch: print an error when the vlan is already configured
  dpaa2-switch: add ENDPOINT_CHANGED to the irq_mask
  dpaa2-switch: do not clear any interrupts automatically
  dpaa2-switch: reorganize the [pre]changeupper events
  dpaa2-switch: move a check to the prechangeupper stage
  dpaa2-switch: cleanup the egress flood of an unused FDB

 .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 131 +++++++++++-------
 1 file changed, 84 insertions(+), 47 deletions(-)

-- 
2.25.1


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH 1/8] dpaa2-switch: set interface MAC address only on endpoint change
  2023-12-04 16:35 [PATCH 0/8] dpaa2-switch: small improvements Ioana Ciornei
@ 2023-12-04 16:35 ` Ioana Ciornei
  2023-12-04 16:35 ` [PATCH 2/8] dpaa2-switch: declare the netdev as IFF_LIVE_ADDR_CHANGE capable Ioana Ciornei
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-04 16:35 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, netdev; +Cc: Ioana Ciornei

There is no point in updating the MAC address of a switch interface each
time the link state changes, this only needs to happen in case the
endpoint changes (the switch interface is [dis]connected from/to a MAC).

Just move the call to dpaa2_switch_port_set_mac_addr() under
DPSW_IRQ_EVENT_ENDPOINT_CHANGED.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 97d3151076d5..08a4d64c1c7d 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -1523,12 +1523,11 @@ static irqreturn_t dpaa2_switch_irq0_handler_thread(int irq_num, void *arg)
 	if_id = (status & 0xFFFF0000) >> 16;
 	port_priv = ethsw->ports[if_id];
 
-	if (status & DPSW_IRQ_EVENT_LINK_CHANGED) {
+	if (status & DPSW_IRQ_EVENT_LINK_CHANGED)
 		dpaa2_switch_port_link_state_update(port_priv->netdev);
-		dpaa2_switch_port_set_mac_addr(port_priv);
-	}
 
 	if (status & DPSW_IRQ_EVENT_ENDPOINT_CHANGED) {
+		dpaa2_switch_port_set_mac_addr(port_priv);
 		/* We can avoid locking because the "endpoint changed" IRQ
 		 * handler is the only one who changes priv->mac at runtime,
 		 * so we are not racing with anyone.
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 2/8] dpaa2-switch: declare the netdev as IFF_LIVE_ADDR_CHANGE capable
  2023-12-04 16:35 [PATCH 0/8] dpaa2-switch: small improvements Ioana Ciornei
  2023-12-04 16:35 ` [PATCH 1/8] dpaa2-switch: set interface MAC address only on endpoint change Ioana Ciornei
@ 2023-12-04 16:35 ` Ioana Ciornei
  2023-12-04 16:35 ` [PATCH 3/8] dpaa2-switch: print an error when the vlan is already configured Ioana Ciornei
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-04 16:35 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, netdev; +Cc: Ioana Ciornei

There is no restriction around the change of the MAC address on the
switch ports, thus declare the interface netdevs IFF_LIVE_ADDR_CHANGE
capable.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 08a4d64c1c7d..5b0ab06b40a7 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -3300,6 +3300,7 @@ static int dpaa2_switch_probe_port(struct ethsw_core *ethsw,
 	port_netdev->features = NETIF_F_HW_VLAN_CTAG_FILTER |
 				NETIF_F_HW_VLAN_STAG_FILTER |
 				NETIF_F_HW_TC;
+	port_netdev->priv_flags |= IFF_LIVE_ADDR_CHANGE;
 
 	err = dpaa2_switch_port_init(port_priv, port_idx);
 	if (err)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 3/8] dpaa2-switch: print an error when the vlan is already configured
  2023-12-04 16:35 [PATCH 0/8] dpaa2-switch: small improvements Ioana Ciornei
  2023-12-04 16:35 ` [PATCH 1/8] dpaa2-switch: set interface MAC address only on endpoint change Ioana Ciornei
  2023-12-04 16:35 ` [PATCH 2/8] dpaa2-switch: declare the netdev as IFF_LIVE_ADDR_CHANGE capable Ioana Ciornei
@ 2023-12-04 16:35 ` Ioana Ciornei
  2023-12-06  3:59   ` Jakub Kicinski
  2023-12-04 16:35 ` [PATCH 4/8] dpaa2-switch: add ENDPOINT_CHANGED to the irq_mask Ioana Ciornei
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-04 16:35 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, netdev; +Cc: Ioana Ciornei

Print a netdev error when we hit a case in which a specific VLAN is
already configured on the port.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 5b0ab06b40a7..654dd10df307 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -289,7 +289,7 @@ static int dpaa2_switch_port_add_vlan(struct ethsw_port_priv *port_priv,
 	int err;
 
 	if (port_priv->vlans[vid]) {
-		netdev_warn(netdev, "VLAN %d already configured\n", vid);
+		netdev_err(netdev, "VLAN %d already configured\n", vid);
 		return -EEXIST;
 	}
 
@@ -1774,8 +1774,10 @@ int dpaa2_switch_port_vlans_add(struct net_device *netdev,
 	/* Make sure that the VLAN is not already configured
 	 * on the switch port
 	 */
-	if (port_priv->vlans[vlan->vid] & ETHSW_VLAN_MEMBER)
+	if (port_priv->vlans[vlan->vid] & ETHSW_VLAN_MEMBER) {
+		netdev_err(netdev, "VLAN %d already configured\n", vlan->vid);
 		return -EEXIST;
+	}
 
 	/* Check if there is space for a new VLAN */
 	err = dpsw_get_attributes(ethsw->mc_io, 0, ethsw->dpsw_handle,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 4/8] dpaa2-switch: add ENDPOINT_CHANGED to the irq_mask
  2023-12-04 16:35 [PATCH 0/8] dpaa2-switch: small improvements Ioana Ciornei
                   ` (2 preceding siblings ...)
  2023-12-04 16:35 ` [PATCH 3/8] dpaa2-switch: print an error when the vlan is already configured Ioana Ciornei
@ 2023-12-04 16:35 ` Ioana Ciornei
  2023-12-06  3:58   ` Jakub Kicinski
  2023-12-04 16:35 ` [PATCH 5/8] dpaa2-switch: do not clear any interrupts automatically Ioana Ciornei
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-04 16:35 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, netdev; +Cc: Ioana Ciornei

Add the ENDPOINT_CHANGED irq to the irq_mask since it was omitted in the
blamed commit.

Fixes: 84cba72956fd ("dpaa2-switch: integrate the MAC endpoint support")
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 654dd10df307..e91ade7c7c93 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -1550,9 +1550,9 @@ static irqreturn_t dpaa2_switch_irq0_handler_thread(int irq_num, void *arg)
 
 static int dpaa2_switch_setup_irqs(struct fsl_mc_device *sw_dev)
 {
+	u32 mask = DPSW_IRQ_EVENT_LINK_CHANGED | DPSW_IRQ_EVENT_ENDPOINT_CHANGED;
 	struct device *dev = &sw_dev->dev;
 	struct ethsw_core *ethsw = dev_get_drvdata(dev);
-	u32 mask = DPSW_IRQ_EVENT_LINK_CHANGED;
 	struct fsl_mc_device_irq *irq;
 	int err;
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 5/8] dpaa2-switch: do not clear any interrupts automatically
  2023-12-04 16:35 [PATCH 0/8] dpaa2-switch: small improvements Ioana Ciornei
                   ` (3 preceding siblings ...)
  2023-12-04 16:35 ` [PATCH 4/8] dpaa2-switch: add ENDPOINT_CHANGED to the irq_mask Ioana Ciornei
@ 2023-12-04 16:35 ` Ioana Ciornei
  2023-12-06  4:02   ` Jakub Kicinski
  2023-12-04 16:35 ` [PATCH 6/8] dpaa2-switch: reorganize the [pre]changeupper events Ioana Ciornei
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-04 16:35 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, netdev; +Cc: Ioana Ciornei

The dpsw_get_irq_status() can clear interrupts automatically based on
the value stored in the 'status' variable passed to it. We don't want
that to happen because we could get into a situation when we are
clearing more interrupts that we actually handled.

Just resort to manually clearing interrupts after we received them using
the dpsw_clear_irq_status().

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index e91ade7c7c93..d9906573f71f 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -1509,7 +1509,7 @@ static irqreturn_t dpaa2_switch_irq0_handler_thread(int irq_num, void *arg)
 	struct device *dev = (struct device *)arg;
 	struct ethsw_core *ethsw = dev_get_drvdata(dev);
 	struct ethsw_port_priv *port_priv;
-	u32 status = ~0;
+	u32 status = 0;
 	int err, if_id;
 	bool had_mac;
 
@@ -1539,12 +1539,12 @@ static irqreturn_t dpaa2_switch_irq0_handler_thread(int irq_num, void *arg)
 			dpaa2_switch_port_connect_mac(port_priv);
 	}
 
-out:
 	err = dpsw_clear_irq_status(ethsw->mc_io, 0, ethsw->dpsw_handle,
 				    DPSW_IRQ_INDEX_IF, status);
 	if (err)
 		dev_err(dev, "Can't clear irq status (err %d)\n", err);
 
+out:
 	return IRQ_HANDLED;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 6/8] dpaa2-switch: reorganize the [pre]changeupper events
  2023-12-04 16:35 [PATCH 0/8] dpaa2-switch: small improvements Ioana Ciornei
                   ` (4 preceding siblings ...)
  2023-12-04 16:35 ` [PATCH 5/8] dpaa2-switch: do not clear any interrupts automatically Ioana Ciornei
@ 2023-12-04 16:35 ` Ioana Ciornei
  2023-12-04 16:35 ` [PATCH 7/8] dpaa2-switch: move a check to the prechangeupper stage Ioana Ciornei
  2023-12-04 16:35 ` [PATCH 8/8] dpaa2-switch: cleanup the egress flood of an unused FDB Ioana Ciornei
  7 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-04 16:35 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, netdev; +Cc: Ioana Ciornei

Create separate functions, dpaa2_switch_port_prechangeupper and
dpaa2_switch_port_changeupper, to be called directly when a DPSW port
changes its upper device.

This way we are not open-coding everything in the main event callback
and we can easily extent when necessary.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 76 +++++++++++++------
 1 file changed, 52 insertions(+), 24 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index d9906573f71f..58c0baee2d61 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -2180,51 +2180,79 @@ dpaa2_switch_prechangeupper_sanity_checks(struct net_device *netdev,
 	return 0;
 }
 
-static int dpaa2_switch_port_netdevice_event(struct notifier_block *nb,
-					     unsigned long event, void *ptr)
+static int dpaa2_switch_port_prechangeupper(struct net_device *netdev,
+					    struct netdev_notifier_changeupper_info *info)
 {
-	struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
-	struct netdev_notifier_changeupper_info *info = ptr;
 	struct netlink_ext_ack *extack;
 	struct net_device *upper_dev;
 	int err = 0;
 
 	if (!dpaa2_switch_port_dev_check(netdev))
-		return NOTIFY_DONE;
+		return 0;
 
 	extack = netdev_notifier_info_to_extack(&info->info);
-
-	switch (event) {
-	case NETDEV_PRECHANGEUPPER:
-		upper_dev = info->upper_dev;
-		if (!netif_is_bridge_master(upper_dev))
-			break;
-
+	upper_dev = info->upper_dev;
+	if (netif_is_bridge_master(upper_dev)) {
 		err = dpaa2_switch_prechangeupper_sanity_checks(netdev,
 								upper_dev,
 								extack);
 		if (err)
-			goto out;
+			return err;
 
 		if (!info->linking)
 			dpaa2_switch_port_pre_bridge_leave(netdev);
+	}
+
+	return 0;
+}
+
+static int dpaa2_switch_port_changeupper(struct net_device *netdev,
+					 struct netdev_notifier_changeupper_info *info)
+{
+	struct netlink_ext_ack *extack;
+	struct net_device *upper_dev;
+	int err = 0;
+
+	if (!dpaa2_switch_port_dev_check(netdev))
+		return 0;
+
+	extack = netdev_notifier_info_to_extack(&info->info);
+
+	upper_dev = info->upper_dev;
+	if (netif_is_bridge_master(upper_dev)) {
+		if (info->linking)
+			return dpaa2_switch_port_bridge_join(netdev,
+							     upper_dev,
+							     extack);
+		else
+			return dpaa2_switch_port_bridge_leave(netdev);
+	}
+
+	return err;
+}
+
+static int dpaa2_switch_port_netdevice_event(struct notifier_block *nb,
+					     unsigned long event, void *ptr)
+{
+	struct net_device *netdev = netdev_notifier_info_to_dev(ptr);
+	int err = 0;
+
+	switch (event) {
+	case NETDEV_PRECHANGEUPPER:
+		err = dpaa2_switch_port_prechangeupper(netdev, ptr);
+		if (err)
+			return notifier_from_errno(err);
 
 		break;
 	case NETDEV_CHANGEUPPER:
-		upper_dev = info->upper_dev;
-		if (netif_is_bridge_master(upper_dev)) {
-			if (info->linking)
-				err = dpaa2_switch_port_bridge_join(netdev,
-								    upper_dev,
-								    extack);
-			else
-				err = dpaa2_switch_port_bridge_leave(netdev);
-		}
+		err = dpaa2_switch_port_changeupper(netdev, ptr);
+		if (err)
+			return notifier_from_errno(err);
+
 		break;
 	}
 
-out:
-	return notifier_from_errno(err);
+	return NOTIFY_DONE;
 }
 
 struct ethsw_switchdev_event_work {
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 7/8] dpaa2-switch: move a check to the prechangeupper stage
  2023-12-04 16:35 [PATCH 0/8] dpaa2-switch: small improvements Ioana Ciornei
                   ` (5 preceding siblings ...)
  2023-12-04 16:35 ` [PATCH 6/8] dpaa2-switch: reorganize the [pre]changeupper events Ioana Ciornei
@ 2023-12-04 16:35 ` Ioana Ciornei
  2023-12-04 16:35 ` [PATCH 8/8] dpaa2-switch: cleanup the egress flood of an unused FDB Ioana Ciornei
  7 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-04 16:35 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, netdev; +Cc: Ioana Ciornei

Two different DPAA2 switch ports from two different DPSW instances
cannot be under the same bridge. Instead of checking for this
unsupported configuration in the CHANGEUPPER event, check it as early as
possible in the PRECHANGEUPPER one.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 .../ethernet/freescale/dpaa2/dpaa2-switch.c   | 31 ++++++++++---------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 58c0baee2d61..dd878e87eef1 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -2008,24 +2008,9 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
 {
 	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
 	struct ethsw_core *ethsw = port_priv->ethsw_data;
-	struct ethsw_port_priv *other_port_priv;
-	struct net_device *other_dev;
-	struct list_head *iter;
 	bool learn_ena;
 	int err;
 
-	netdev_for_each_lower_dev(upper_dev, other_dev, iter) {
-		if (!dpaa2_switch_port_dev_check(other_dev))
-			continue;
-
-		other_port_priv = netdev_priv(other_dev);
-		if (other_port_priv->ethsw_data != port_priv->ethsw_data) {
-			NL_SET_ERR_MSG_MOD(extack,
-					   "Interface from a different DPSW is in the bridge already");
-			return -EINVAL;
-		}
-	}
-
 	/* Delete the previously manually installed VLAN 1 */
 	err = dpaa2_switch_port_del_vlan(port_priv, 1);
 	if (err)
@@ -2163,6 +2148,10 @@ dpaa2_switch_prechangeupper_sanity_checks(struct net_device *netdev,
 					  struct net_device *upper_dev,
 					  struct netlink_ext_ack *extack)
 {
+	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
+	struct ethsw_port_priv *other_port_priv;
+	struct net_device *other_dev;
+	struct list_head *iter;
 	int err;
 
 	if (!br_vlan_enabled(upper_dev)) {
@@ -2177,6 +2166,18 @@ dpaa2_switch_prechangeupper_sanity_checks(struct net_device *netdev,
 		return 0;
 	}
 
+	netdev_for_each_lower_dev(upper_dev, other_dev, iter) {
+		if (!dpaa2_switch_port_dev_check(other_dev))
+			continue;
+
+		other_port_priv = netdev_priv(other_dev);
+		if (other_port_priv->ethsw_data != port_priv->ethsw_data) {
+			NL_SET_ERR_MSG_MOD(extack,
+					   "Interface from a different DPSW is in the bridge already");
+			return -EINVAL;
+		}
+	}
+
 	return 0;
 }
 
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* [PATCH 8/8] dpaa2-switch: cleanup the egress flood of an unused FDB
  2023-12-04 16:35 [PATCH 0/8] dpaa2-switch: small improvements Ioana Ciornei
                   ` (6 preceding siblings ...)
  2023-12-04 16:35 ` [PATCH 7/8] dpaa2-switch: move a check to the prechangeupper stage Ioana Ciornei
@ 2023-12-04 16:35 ` Ioana Ciornei
  2023-12-06  4:04   ` Jakub Kicinski
  7 siblings, 1 reply; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-04 16:35 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, netdev; +Cc: Ioana Ciornei

In case a switch interface is joining a bridge, its FDB might change. In
case this happens, we have to recreate the egress flood setup of the FDB
that we just left. For this to happen, keep track of the old FDB and
just call dpaa2_switch_fdb_set_egress_flood() on it.

Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index dd878e87eef1..35f71c3668ba 100644
--- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
+++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
@@ -2008,6 +2008,7 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
 {
 	struct ethsw_port_priv *port_priv = netdev_priv(netdev);
 	struct ethsw_core *ethsw = port_priv->ethsw_data;
+	struct dpaa2_switch_fdb *old_fdb = port_priv->fdb;
 	bool learn_ena;
 	int err;
 
@@ -2028,6 +2029,11 @@ static int dpaa2_switch_port_bridge_join(struct net_device *netdev,
 	if (err)
 		goto err_egress_flood;
 
+	/* Recreate the egress flood domain of the FDB that we just left. */
+	err = dpaa2_switch_fdb_set_egress_flood(ethsw, old_fdb->fdb_id);
+	if (err)
+		goto err_egress_flood;
+
 	err = switchdev_bridge_port_offload(netdev, netdev, NULL,
 					    &dpaa2_switch_port_switchdev_nb,
 					    &dpaa2_switch_port_switchdev_blocking_nb,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/8] dpaa2-switch: add ENDPOINT_CHANGED to the irq_mask
  2023-12-04 16:35 ` [PATCH 4/8] dpaa2-switch: add ENDPOINT_CHANGED to the irq_mask Ioana Ciornei
@ 2023-12-06  3:58   ` Jakub Kicinski
  2023-12-12 12:13     ` Ioana Ciornei
  0 siblings, 1 reply; 18+ messages in thread
From: Jakub Kicinski @ 2023-12-06  3:58 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: davem, edumazet, pabeni, netdev

On Mon,  4 Dec 2023 18:35:24 +0200 Ioana Ciornei wrote:
> Add the ENDPOINT_CHANGED irq to the irq_mask since it was omitted in the
> blamed commit.

Any user-visible impact? What's the observable problem?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/8] dpaa2-switch: print an error when the vlan is already configured
  2023-12-04 16:35 ` [PATCH 3/8] dpaa2-switch: print an error when the vlan is already configured Ioana Ciornei
@ 2023-12-06  3:59   ` Jakub Kicinski
  2023-12-12 12:24     ` Ioana Ciornei
  0 siblings, 1 reply; 18+ messages in thread
From: Jakub Kicinski @ 2023-12-06  3:59 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: davem, edumazet, pabeni, netdev

On Mon,  4 Dec 2023 18:35:23 +0200 Ioana Ciornei wrote:
> Print a netdev error when we hit a case in which a specific VLAN is
> already configured on the port.

Would be nice to cover the "why" - I'm a bit curious what difference
upgrading from warn to err makes. Is it just for consistency with the
newly added case?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/8] dpaa2-switch: do not clear any interrupts automatically
  2023-12-04 16:35 ` [PATCH 5/8] dpaa2-switch: do not clear any interrupts automatically Ioana Ciornei
@ 2023-12-06  4:02   ` Jakub Kicinski
  2023-12-12 12:17     ` Ioana Ciornei
  0 siblings, 1 reply; 18+ messages in thread
From: Jakub Kicinski @ 2023-12-06  4:02 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: davem, edumazet, pabeni, netdev

On Mon,  4 Dec 2023 18:35:25 +0200 Ioana Ciornei wrote:
> The dpsw_get_irq_status() can clear interrupts automatically based on
> the value stored in the 'status' variable passed to it. We don't want
> that to happen because we could get into a situation when we are
> clearing more interrupts that we actually handled.
> 
> Just resort to manually clearing interrupts after we received them using
> the dpsw_clear_irq_status().

Currently it can't cause any issues?
We won't get into an IRQ storm if some unexpected IRQ fires?

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 8/8] dpaa2-switch: cleanup the egress flood of an unused FDB
  2023-12-04 16:35 ` [PATCH 8/8] dpaa2-switch: cleanup the egress flood of an unused FDB Ioana Ciornei
@ 2023-12-06  4:04   ` Jakub Kicinski
  2023-12-12 12:09     ` Ioana Ciornei
  0 siblings, 1 reply; 18+ messages in thread
From: Jakub Kicinski @ 2023-12-06  4:04 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: davem, edumazet, pabeni, netdev

On Mon,  4 Dec 2023 18:35:28 +0200 Ioana Ciornei wrote:
> In case a switch interface is joining a bridge, its FDB might change. In
> case this happens, we have to recreate the egress flood setup of the FDB
> that we just left. For this to happen, keep track of the old FDB and
> just call dpaa2_switch_fdb_set_egress_flood() on it.

Is this not a fix? FWIW the commit message is a bit hard to parse,
rephrasing would help..

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 8/8] dpaa2-switch: cleanup the egress flood of an unused FDB
  2023-12-06  4:04   ` Jakub Kicinski
@ 2023-12-12 12:09     ` Ioana Ciornei
  2023-12-12 16:25       ` Jakub Kicinski
  0 siblings, 1 reply; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-12 12:09 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, edumazet, pabeni, netdev

On Tue, Dec 05, 2023 at 08:04:55PM -0800, Jakub Kicinski wrote:
> On Mon,  4 Dec 2023 18:35:28 +0200 Ioana Ciornei wrote:
> > In case a switch interface is joining a bridge, its FDB might change. In
> > case this happens, we have to recreate the egress flood setup of the FDB
> > that we just left. For this to happen, keep track of the old FDB and
> > just call dpaa2_switch_fdb_set_egress_flood() on it.
> 
> Is this not a fix? FWIW the commit message is a bit hard to parse,
> rephrasing would help..

This is not actually fixing up an issue which can be seen but maybe it's
better to just have it through the net tree. I will split the patch set
and send some of them through net.

I'll rephrase the commit message and add a bit more information.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 4/8] dpaa2-switch: add ENDPOINT_CHANGED to the irq_mask
  2023-12-06  3:58   ` Jakub Kicinski
@ 2023-12-12 12:13     ` Ioana Ciornei
  0 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-12 12:13 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, edumazet, pabeni, netdev

On Tue, Dec 05, 2023 at 07:58:19PM -0800, Jakub Kicinski wrote:
> On Mon,  4 Dec 2023 18:35:24 +0200 Ioana Ciornei wrote:
> > Add the ENDPOINT_CHANGED irq to the irq_mask since it was omitted in the
> > blamed commit.
> 
> Any user-visible impact? What's the observable problem?

No user-visible impact but that is only because the firmware sets both
events anytime a switch interface is connected to a MAC. We shouldn't
rely on this behavior since it's not documented. And this is why I
didn't catch the problem when I initially sent the patch.


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 5/8] dpaa2-switch: do not clear any interrupts automatically
  2023-12-06  4:02   ` Jakub Kicinski
@ 2023-12-12 12:17     ` Ioana Ciornei
  0 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-12 12:17 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, edumazet, pabeni, netdev

On Tue, Dec 05, 2023 at 08:02:32PM -0800, Jakub Kicinski wrote:
> On Mon,  4 Dec 2023 18:35:25 +0200 Ioana Ciornei wrote:
> > The dpsw_get_irq_status() can clear interrupts automatically based on
> > the value stored in the 'status' variable passed to it. We don't want
> > that to happen because we could get into a situation when we are
> > clearing more interrupts that we actually handled.
> > 
> > Just resort to manually clearing interrupts after we received them using
> > the dpsw_clear_irq_status().
> 
> Currently it can't cause any issues?
> We won't get into an IRQ storm if some unexpected IRQ fires?

We don't get into an IRQ storm because we can configure which event
sources can raise the IRQ. This means that the driver can ask only for
the events which are handled.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 3/8] dpaa2-switch: print an error when the vlan is already configured
  2023-12-06  3:59   ` Jakub Kicinski
@ 2023-12-12 12:24     ` Ioana Ciornei
  0 siblings, 0 replies; 18+ messages in thread
From: Ioana Ciornei @ 2023-12-12 12:24 UTC (permalink / raw)
  To: Jakub Kicinski; +Cc: davem, edumazet, pabeni, netdev

On Tue, Dec 05, 2023 at 07:59:33PM -0800, Jakub Kicinski wrote:
> On Mon,  4 Dec 2023 18:35:23 +0200 Ioana Ciornei wrote:
> > Print a netdev error when we hit a case in which a specific VLAN is
> > already configured on the port.
> 
> Would be nice to cover the "why" - I'm a bit curious what difference
> upgrading from warn to err makes. Is it just for consistency with the
> newly added case?

Yes, it's more about consistency with the newly added print. I just
chose the err instead of warn because we actually error out in that case,
we don't just print a warning and continue.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 8/8] dpaa2-switch: cleanup the egress flood of an unused FDB
  2023-12-12 12:09     ` Ioana Ciornei
@ 2023-12-12 16:25       ` Jakub Kicinski
  0 siblings, 0 replies; 18+ messages in thread
From: Jakub Kicinski @ 2023-12-12 16:25 UTC (permalink / raw)
  To: Ioana Ciornei; +Cc: davem, edumazet, pabeni, netdev

On Tue, 12 Dec 2023 14:09:43 +0200 Ioana Ciornei wrote:
> > Is this not a fix? FWIW the commit message is a bit hard to parse,
> > rephrasing would help..  
> 
> This is not actually fixing up an issue which can be seen but maybe it's
> better to just have it through the net tree. I will split the patch set
> and send some of them through net.
> 
> I'll rephrase the commit message and add a bit more information.

Same goes for all the other changes. You don't have to send the patches
to net if there's not user visible impact, just please add the
explanations to the commit messages.

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2023-12-12 16:25 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-12-04 16:35 [PATCH 0/8] dpaa2-switch: small improvements Ioana Ciornei
2023-12-04 16:35 ` [PATCH 1/8] dpaa2-switch: set interface MAC address only on endpoint change Ioana Ciornei
2023-12-04 16:35 ` [PATCH 2/8] dpaa2-switch: declare the netdev as IFF_LIVE_ADDR_CHANGE capable Ioana Ciornei
2023-12-04 16:35 ` [PATCH 3/8] dpaa2-switch: print an error when the vlan is already configured Ioana Ciornei
2023-12-06  3:59   ` Jakub Kicinski
2023-12-12 12:24     ` Ioana Ciornei
2023-12-04 16:35 ` [PATCH 4/8] dpaa2-switch: add ENDPOINT_CHANGED to the irq_mask Ioana Ciornei
2023-12-06  3:58   ` Jakub Kicinski
2023-12-12 12:13     ` Ioana Ciornei
2023-12-04 16:35 ` [PATCH 5/8] dpaa2-switch: do not clear any interrupts automatically Ioana Ciornei
2023-12-06  4:02   ` Jakub Kicinski
2023-12-12 12:17     ` Ioana Ciornei
2023-12-04 16:35 ` [PATCH 6/8] dpaa2-switch: reorganize the [pre]changeupper events Ioana Ciornei
2023-12-04 16:35 ` [PATCH 7/8] dpaa2-switch: move a check to the prechangeupper stage Ioana Ciornei
2023-12-04 16:35 ` [PATCH 8/8] dpaa2-switch: cleanup the egress flood of an unused FDB Ioana Ciornei
2023-12-06  4:04   ` Jakub Kicinski
2023-12-12 12:09     ` Ioana Ciornei
2023-12-12 16:25       ` Jakub Kicinski

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).