[PATCH net-next] net: ti: icssm-prueth: use dev_close()/dev_open() in icssm_prueth_change_mode()

[PATCH net-next] net: ti: icssm-prueth: use dev_close()/dev_open() in icssm_prueth_change_mode()

[Sai Varun Mudepally]

In icssm_prueth_change_mode(), if ndev->netdev_ops->ndo_open(ndev) fails
and returns an error, the function aborts without restoring the NAPI state.
Later, when the interface is brought down, the kernel naturally invokes
ndo_stop() again. A second napi_disable() call on an already disabled NAPI
might trigger an infinite sleep waiting for NAPI_STATE_SCHED.

Use dev_close()/dev_open() so the core keeps IFF_UP consistent. If
dev_open() fails, close the ports already reopened and fall back to
EMAC mode.

Also undo the br_members update in icssm_prueth_ndev_port_link() when
icssm_prueth_port_offload_fwd_mark_update() fails.

Test Log:
https://gist.github.com/SaiVarun754/688a21038f1b2bd2b89d2db65224edb0

Fixes: eca327ae9aa6 ("net: ti: icssm-prueth: Add support for ICSSM RSTP switch")
Signed-off-by: Sai Varun Mudepally <saivarun@couthit.com>
---
 drivers/net/ethernet/ti/icssm/icssm_prueth.c | 38 ++++++++++++++------
 1 file changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssm/icssm_prueth.c b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
index b42c98e861e0..d50aa0992c33 100644
--- a/drivers/net/ethernet/ti/icssm/icssm_prueth.c
+++ b/drivers/net/ethernet/ti/icssm/icssm_prueth.c
@@ -1100,7 +1100,7 @@ static int icssm_prueth_change_mode(struct prueth *prueth,
 	bool portstatus[PRUETH_NUM_MACS];
 	struct prueth_emac *emac;
 	struct net_device *ndev;
-	int i, ret;
+	int i, j, ret;
 
 	for (i = 0; i < PRUETH_NUM_MACS; i++) {
 		if (!prueth->emac[i]) {
@@ -1115,11 +1115,7 @@ static int icssm_prueth_change_mode(struct prueth *prueth,
 		if (!portstatus[i])
 			continue;
 
-		ret = ndev->netdev_ops->ndo_stop(ndev);
-		if (ret < 0) {
-			netdev_err(ndev, "failed to stop: %d", ret);
-			return ret;
-		}
+		dev_close(ndev);
 	}
 
 	if (mode == PRUSS_ETHTYPE_EMAC || mode == PRUSS_ETHTYPE_SWITCH) {
@@ -1141,14 +1137,25 @@ static int icssm_prueth_change_mode(struct prueth *prueth,
 		if (!portstatus[i])
 			continue;
 
-		ret = ndev->netdev_ops->ndo_open(ndev);
+		ret = dev_open(ndev, NULL);
 		if (ret < 0) {
 			netdev_err(ndev, "failed to start: %d", ret);
-			return ret;
+			goto err_dev_open;
 		}
 	}
 
 	return 0;
+
+err_dev_open:
+	for (j = 0; j < i; j++) {
+		if (portstatus[j])
+			dev_close(prueth->emac[j]->ndev);
+	}
+
+	/* Fall back to EMAC mode */
+	prueth->eth_type = PRUSS_ETHTYPE_EMAC;
+
+	return ret;
 }
 
 /* VLAN-tag PCP to priority queue map for EMAC/Switch/HSR/PRP used by driver
@@ -1639,6 +1646,8 @@ bool icssm_prueth_sw_port_dev_check(const struct net_device *ndev)
 
 static int icssm_prueth_port_offload_fwd_mark_update(struct prueth *prueth)
 {
+	struct prueth_emac *emac;
+	unsigned long flags;
 	int set_val = 0;
 	int i, ret = 0;
 	u8 all_slaves;
@@ -1661,11 +1670,20 @@ static int icssm_prueth_port_offload_fwd_mark_update(struct prueth *prueth)
 	 */
 	if (set_val && !PRUETH_IS_SWITCH(prueth)) {
 		ret = icssm_prueth_change_mode(prueth, PRUSS_ETHTYPE_SWITCH);
-		if (ret < 0)
+		if (ret < 0) {
 			dev_err(prueth->dev, "Failed to enable Switch mode\n");
-		else
+			emac = prueth->emac[0];
+			spin_lock_irqsave(&emac->addr_lock, flags);
+
+			prueth->br_members &= ~BIT(emac->port_id);
+			if (!prueth->br_members)
+				prueth->hw_bridge_dev = NULL;
+
+			spin_unlock_irqrestore(&emac->addr_lock, flags);
+		} else {
 			dev_info(prueth->dev,
 				 "TI PRU ethernet now in Switch mode\n");
+		}
 	}
 
 	/* Bridge is deleted, switch to Dual EMAC mode */
-- 
2.43.0

Re: [PATCH net-next] net: ti: icssm-prueth: use dev_close()/dev_open() in icssm_prueth_change_mode()

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.