[PATCH net 0/2] net: ti: icssg-prueth: fix port config and rx_mode drops

[PATCH net 0/2] net: ti: icssg-prueth: fix port config and rx_mode drops

[Rishi Kumar]

Two independent fixes for the ICSSG PRUETH driver, both hit during
switch offload testing with ports opened/closed in varying order.


Patch 1 fixes icssg_enable_fw_offload() running only once, on the
first port to open (gated by !emacs_initialized), so a sibling port
that opens later never gets its FDB/VLAN table/PVID configured. Each
port now configures itself at open time.

Patch 2 fixes icssg_set_port_state() commands (e.g. promisc/allmulti
flood enable from rx_mode_work) being silently dropped when they race
a PRU/RTU/txPRU firmware reboot, by re-queuing rx_mode_work for all
running ports once the firmware is confirmed back up.

Rishi Kumar (2):
  net: ti: icssg-prueth: fix missing port config on second interface
    open
  net: ti: icssg-prueth: re-apply rx_mode state after firmware reboot

 drivers/net/ethernet/ti/icssg/icssg_prueth.c | 89 ++++++++++++++------
 1 file changed, 61 insertions(+), 28 deletions(-)

-- 
2.53.0

[PATCH net 1/2] net: ti: icssg-prueth: fix missing port config on second interface open

[Rishi Kumar]

icssg_enable_fw_offload() is gated by !prueth->emacs_initialized, so it
runs only once, on whichever port opens first. If the sibling port is
not yet running at that point, the netif_running() check skips it and
its FDB entry and VLAN table membership are never configured. If one
port is later brought down and up while the sibling stays up,
emacs_initialized is still non-zero, so the function never runs again
and the port is left unconfigured, depending on bring-up order.

Extract the per-port FDB, VLAN table, PVID and VLAN-aware setup into a
new helper icssg_enable_fw_offload_emac(), and the HSR Rx offload
setup into icssg_update_hsr_rx_offload(). Call both unconditionally
from emac_ndo_open() for the port that is actually opening, so each
port configures itself at open time regardless of sibling port state.

icssg_enable_fw_offload() now uses the same helpers and is kept for
icssg_change_mode(), which needs to reconfigure all running ports
after a full firmware restart.

prefix logs: https://gist.github.com/r-kumar12/2279fbfeb5b451a427c4c9778cdfca4a
postfix logs: https://gist.github.com/r-kumar12/071bd3625040fc1fb71207df4e652f58

Fixes: 01792bc3e5bd ("net: ti: icssg-prueth: Fix HSR and switch offload Enablement during firwmare reload.")
Signed-off-by: Rishi Kumar <r-kumar12@ti.com>
---
 drivers/net/ethernet/ti/icssg/icssg_prueth.c | 72 ++++++++++++--------
 1 file changed, 44 insertions(+), 28 deletions(-)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index f43c1394179f..745178fe7cf3 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -206,6 +206,44 @@ static void prueth_emac_stop(struct prueth *prueth)
 	}
 }
 
+static void icssg_update_hsr_rx_offload(struct prueth_emac *emac)
+{
+	struct prueth *prueth = emac->prueth;
+
+	if (!prueth->is_hsr_offload_mode)
+		return;
+
+	if (emac->ndev->features & NETIF_F_HW_HSR_TAG_RM)
+		icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE);
+	else
+		icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE);
+}
+
+static void icssg_enable_fw_offload_emac(struct prueth_emac *emac)
+{
+	struct prueth *prueth = emac->prueth;
+
+	if (prueth->is_switch_mode || prueth->is_hsr_offload_mode) {
+		icssg_fdb_add_del(emac, eth_stp_addr, prueth->default_vlan,
+				  ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
+				  ICSSG_FDB_ENTRY_P1_MEMBERSHIP |
+				  ICSSG_FDB_ENTRY_P2_MEMBERSHIP |
+				  ICSSG_FDB_ENTRY_BLOCK,
+				  true);
+		icssg_vtbl_modify(emac, emac->port_vlan | DEFAULT_VID,
+				  BIT(emac->port_id) | DEFAULT_PORT_MASK,
+				  BIT(emac->port_id) | DEFAULT_UNTAG_MASK,
+				  true);
+		if (prueth->is_hsr_offload_mode)
+			icssg_vtbl_modify(emac, DEFAULT_VID,
+					  DEFAULT_PORT_MASK,
+					  DEFAULT_UNTAG_MASK, true);
+		icssg_set_pvid(prueth, emac->port_vlan, emac->port_id);
+		if (prueth->is_switch_mode)
+			icssg_set_port_state(emac, ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE);
+	}
+}
+
 static void icssg_enable_fw_offload(struct prueth *prueth)
 {
 	struct prueth_emac *emac;
@@ -213,34 +251,11 @@ static void icssg_enable_fw_offload(struct prueth *prueth)
 
 	for (mac = PRUETH_MAC0; mac < PRUETH_NUM_MACS; mac++) {
 		emac = prueth->emac[mac];
-		if (prueth->is_hsr_offload_mode) {
-			if (emac->ndev->features & NETIF_F_HW_HSR_TAG_RM)
-				icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_ENABLE);
-			else
-				icssg_set_port_state(emac, ICSSG_EMAC_HSR_RX_OFFLOAD_DISABLE);
-		}
 
-		if (prueth->is_switch_mode || prueth->is_hsr_offload_mode) {
-			if (netif_running(emac->ndev)) {
-				icssg_fdb_add_del(emac, eth_stp_addr, prueth->default_vlan,
-						  ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
-						  ICSSG_FDB_ENTRY_P1_MEMBERSHIP |
-						  ICSSG_FDB_ENTRY_P2_MEMBERSHIP |
-						  ICSSG_FDB_ENTRY_BLOCK,
-						  true);
-				icssg_vtbl_modify(emac, emac->port_vlan | DEFAULT_VID,
-						  BIT(emac->port_id) | DEFAULT_PORT_MASK,
-						  BIT(emac->port_id) | DEFAULT_UNTAG_MASK,
-						  true);
-				if (prueth->is_hsr_offload_mode)
-					icssg_vtbl_modify(emac, DEFAULT_VID,
-							  DEFAULT_PORT_MASK,
-							  DEFAULT_UNTAG_MASK, true);
-				icssg_set_pvid(prueth, emac->port_vlan, emac->port_id);
-				if (prueth->is_switch_mode)
-					icssg_set_port_state(emac, ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE);
-			}
-		}
+		icssg_update_hsr_rx_offload(emac);
+
+		if (netif_running(emac->ndev))
+			icssg_enable_fw_offload_emac(emac);
 	}
 }
 
@@ -1073,8 +1088,9 @@ static int emac_ndo_open(struct net_device *ndev)
 		ret = prueth_emac_common_start(prueth);
 		if (ret)
 			goto free_rx_irq;
-		icssg_enable_fw_offload(prueth);
 	}
+	icssg_update_hsr_rx_offload(emac);
+	icssg_enable_fw_offload_emac(emac);
 
 	flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
 	writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);
-- 
2.53.0

Re: [PATCH net 1/2] net: ti: icssg-prueth: fix missing port config on second interface open

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 0 (1)

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Reopening one port while its sibling stays up now resets that
  port's live VLAN configuration to the driver defaults.

Pre-existing issues:
- [Low] The two join paths set emac->port_vlan differently.

--

net: ti: icssg-prueth: fix missing port config on second interface open

This commit moves the per-port FDB, VLAN table, PVID and VLAN-aware setup
out of icssg_enable_fw_offload() into a new icssg_enable_fw_offload_emac().
It also moves the HSR Rx offload setup into icssg_update_hsr_rx_offload().
emac_ndo_open() now calls both helpers unconditionally for the opening
port, and icssg_enable_fw_offload() is kept for icssg_change_mode().

> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index 591be5c8056b4..cd4c918fe2b89 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -206,6 +206,44 @@ static void prueth_emac_stop(struct prueth *prueth)

[ ... ]

> +static void icssg_enable_fw_offload_emac(struct prueth_emac *emac)
> +{
> +	struct prueth *prueth = emac->prueth;
> +
> +	if (prueth->is_switch_mode || prueth->is_hsr_offload_mode) {
> +		icssg_fdb_add_del(emac, eth_stp_addr, prueth->default_vlan,
> +				  ICSSG_FDB_ENTRY_P0_MEMBERSHIP |
> +				  ICSSG_FDB_ENTRY_P1_MEMBERSHIP |
> +				  ICSSG_FDB_ENTRY_P2_MEMBERSHIP |
> +				  ICSSG_FDB_ENTRY_BLOCK,
> +				  true);
> +		icssg_vtbl_modify(emac, emac->port_vlan | DEFAULT_VID,
> +				  BIT(emac->port_id) | DEFAULT_PORT_MASK,
> +				  BIT(emac->port_id) | DEFAULT_UNTAG_MASK,
> +				  true);
> +		if (prueth->is_hsr_offload_mode)
> +			icssg_vtbl_modify(emac, DEFAULT_VID,
> +					  DEFAULT_PORT_MASK,
> +					  DEFAULT_UNTAG_MASK, true);
> +		icssg_set_pvid(prueth, emac->port_vlan, emac->port_id);

[Severity: Low]
This isn't a bug introduced by this patch, but is emac->port_vlan set
for both ports in switch mode?

In prueth_netdevice_port_link(), only the port whose join completes
br_members gets port_vlan set:

	if (!prueth->is_switch_mode) {
		if (prueth->br_members & BIT(PRUETH_PORT_MII0) &&
		    prueth->br_members & BIT(PRUETH_PORT_MII1)) {
			prueth->is_switch_mode = true;
			prueth->default_vlan = PRUETH_DFLT_VLAN_SW;
			emac->port_vlan = prueth->default_vlan;
			icssg_change_mode(prueth);
		}
	}

prueth_hsr_port_link() sets it on both emac0 and emac1.

This means the port that joined the bridge first keeps port_vlan == 0.
icssg_set_pvid() here then programs PVID 0 for that port and PVID 1 for
the other one. Both ports still get VID 1 table membership, because
0 | DEFAULT_VID == 1.

The unlink paths have the same asymmetry. The switch unlink path resets
port_vlan on one emac, while the HSR unlink path resets both.

icssg_change_mode()->icssg_enable_fw_offload() already behaved this way
before this patch. This patch adds another call site that applies the
value.

A concrete forwarding failure from this hasn't been shown. With 8021q
loaded, NETDEV_UP already sets PVID 0 via ndo_vlan_rx_add_vid(0). How the
firmware handles PVID 0 is not known.

Should prueth_netdevice_port_link() set port_vlan on both ports, the
same way the HSR path does?

> +		if (prueth->is_switch_mode)
> +			icssg_set_port_state(emac, ICSSG_EMAC_PORT_VLAN_AWARE_ENABLE);
> +	}
> +}

[ ... ]

> @@ -1067,8 +1082,9 @@ static int emac_ndo_open(struct net_device *ndev)
>  		ret = prueth_emac_common_start(prueth);
>  		if (ret)
>  			goto free_rx_irq;
> -		icssg_enable_fw_offload(prueth);
>  	}
> +	icssg_update_hsr_rx_offload(emac);
> +	icssg_enable_fw_offload_emac(emac);

[Severity: Medium]
Can this reset a port's live VLAN configuration when the port is
reopened while its sibling stays up?

When the sibling is up, emac_ndo_stop() does not stop the firmware and
does not remove any FDB, VLAN or PVID state:

	/* stop PRUs */
	if (prueth->emacs_initialized == 1)
		prueth_emac_common_stop(prueth);

The next open also skips prueth_emac_common_start(). So the shared-RAM
VLAN table and the per-port PVID word still hold whatever the bridge or
8021q programmed. icssg_enable_fw_offload_emac() then runs:

	icssg_vtbl_modify(emac, emac->port_vlan | DEFAULT_VID,
			  BIT(emac->port_id) | DEFAULT_PORT_MASK,
			  BIT(emac->port_id) | DEFAULT_UNTAG_MASK,
			  true);
	...
	icssg_set_pvid(prueth, emac->port_vlan, emac->port_id);

This adds the port back to VID 1 with untagged egress, and overwrites the
PVID with emac->port_vlan.

emac->port_vlan is only ever 0 or the mode default. The runtime VLAN
paths write the hardware directly and never update it:

prueth_switchdev_vlan_add()
	icssg_vtbl_modify(emac, vid, port_mask, untag_mask, true);
	icssg_set_pvid(emac->prueth, vid, emac->port_id);

prueth_switchdev_vlan_del()
	icssg_vtbl_modify(emac, vid, port_mask, 0, false);

emac_ndo_vlan_rx_add_vid()
	icssg_set_pvid(emac->prueth, vid, emac->port_id);

For example, on an offloaded bridge, the admin runs "bridge vlan del dev
eth1 vid 1" or makes VID 1 tagged on eth1. Then "ip link set eth1 down;
ip link set eth1 up" runs while eth2 is still up. In hardware, eth1 ends
up as an untagged member of VID 1 again. The software bridge still shows
the old configuration, and nothing replays the switchdev VLAN objects on
NETDEV_UP.

Without 8021q, a bridge-configured PVID such as "bridge vlan add dev eth1
vid 10 pvid untagged" is also replaced, with 1 or with 0 for the port
that joined the bridge first. With 8021q loaded, its NETDEV_UP handler
already sets PVID 0 through ndo_vlan_rx_add_vid(0) on every up, both
before and after this patch.

Before this patch, none of this ran on a single-port reopen, so VLAN
membership and tagging survived it. The later commit in the series, "net:
ti: icssg-prueth: re-apply rx_mode state after firmware reboot", only
adds rx_mode_work scheduling in prueth_emac_common_start(). It does not
change icssg_enable_fw_offload_emac() or the open path.

The commit message also says:

    If one port is later brought down and up while the sibling stays up,
    emacs_initialized is still non-zero, so the function never runs again
    and the port is left unconfigured, depending on bring-up order.

Is that accurate? emac_ndo_stop() never removes the port's FDB, VLAN or
PVID state, so a port configured at first open stays configured. Only
the port opened second seems to have been missing its configuration.

Should the open path avoid overwriting state that the firmware still
holds while the sibling port is running?

>  
>  	flow_cfg = emac->dram.va + ICSSG_CONFIG_OFFSET + PSI_L_REGULAR_FLOW_ID_BASE_OFFSET;
>  	writew(emac->rx_flow_id_base, &flow_cfg->rx_base_flow);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014125948.230571-1-r-kumar12%40ti.com

[PATCH net 2/2] net: ti: icssg-prueth: re-apply rx_mode state after firmware reboot

[Rishi Kumar]

prueth_emac_common_start() reboots the PRU/RTU/txPRU cores whenever
the first port opens, or on a full mode change via
icssg_change_mode(). Any icssg_set_port_state() command sent while a
core is stopped or still booting times out waiting for firmware to
ack it ("timeout waiting for command done"), and the write is lost
with no retry.

emac_ndo_set_rx_mode_work() sends exactly this kind of command to
enable/disable UC/MC flooding based on IFF_PROMISC/IFF_ALLMULTI. If
a port's rx_mode_work runs while firmware is being rebooted, the
flood-enable command can be dropped, leaving that port without
multicast flooding.

Re-queue rx_mode_work for every running port at the end of the
firmware start sequence in prueth_emac_common_start(), so the current
IFF_PROMISC/IFF_ALLMULTI state is re-applied after a reboot. Also
cancel the pending work in the stop_pruss error path so it cannot run
after the cores are stopped again.

prefix logs: https://gist.github.com/r-kumar12/91ee6fecd40ea3f22dcc65aba42705e1
postfix logs: https://gist.github.com/r-kumar12/4d5ade22d783020552b8a1e0c6d7102f

Fixes: 9facce84f406 ("net: ti: icssg-prueth: Fix firmware load sequence.")
Signed-off-by: Rishi Kumar <r-kumar12@ti.com>
---
 drivers/net/ethernet/ti/icssg/icssg_prueth.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
index 745178fe7cf3..96ad6227f405 100644
--- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
+++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
@@ -311,6 +311,18 @@ static int prueth_emac_common_start(struct prueth *prueth)
 	if (ret)
 		goto disable_class;
 
+	/* PRU/RTU/txPRU are up now: re-sync any port
+	 * config (e.g. flood/promisc flags) that may have been
+	 * dropped while firmware was still rebooting above.
+	 */
+	for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
+		emac = prueth->emac[slice];
+		if (!emac)
+			continue;
+		if (netif_running(emac->ndev))
+			schedule_work(&emac->rx_mode_work);
+	}
+
 	emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
 	       prueth->emac[ICSS_SLICE1];
 	ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
@@ -323,6 +335,11 @@ static int prueth_emac_common_start(struct prueth *prueth)
 	return 0;
 
 stop_pruss:
+	for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
+		emac = prueth->emac[slice];
+		if (emac)
+			cancel_work_sync(&emac->rx_mode_work);
+	}
 	prueth_emac_stop(prueth);
 
 disable_class:
-- 
2.53.0

Re: [PATCH net 2/2] net: ti: icssg-prueth: re-apply rx_mode state after firmware reboot

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 2 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] net: silent hardware state loss due to rtnl_trylock failure in
  rx_mode_work
- [Medium] prueth_emac_common_start() now queues rx_mode_work for every
  running port before returning success.
- [Medium] The new schedule_work() loop in prueth_emac_common_start()
  lets the opening port's rx_mode_work run while emac_ndo_open() is…

--

net: ti: icssg-prueth: re-apply rx_mode state after firmware reboot

At the end of prueth_emac_common_start(), this commit re-queues
rx_mode_work for every running port. The aim is to re-apply the
IFF_PROMISC/IFF_ALLMULTI flood state after the PRU/RTU/txPRU firmware
reboots. It also cancels the pending work in the stop_pruss error path.

> diff --git a/drivers/net/ethernet/ti/icssg/icssg_prueth.c b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> index cd4c918fe2b89..9d38cbbdaf27f 100644
> --- a/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> +++ b/drivers/net/ethernet/ti/icssg/icssg_prueth.c
> @@ -311,6 +311,18 @@ static int prueth_emac_common_start(struct prueth *prueth)
>  	if (ret)
>  		goto disable_class;
>  
> +	/* PRU/RTU/txPRU are up now: re-sync any port
> +	 * config (e.g. flood/promisc flags) that may have been
> +	 * dropped while firmware was still rebooting above.
> +	 */
> +	for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
> +		emac = prueth->emac[slice];
> +		if (!emac)
> +			continue;
> +		if (netif_running(emac->ndev))
> +			schedule_work(&emac->rx_mode_work);
> +	}

[Severity: High]
Does the VLAN multicast part of the re-sync actually happen when the
work is queued from here?

prueth_emac_common_start() is called with rtnl_lock held in both
paths. It is called from emac_ndo_open() via __dev_open(), and from
icssg_change_mode() via the NETDEV_CHANGEUPPER notifier. The caller
still holds rtnl while it runs icss_iep_init() and the rest of
ndo_open or the mode change.

The work queued here will usually run inside that window. The worker
first sends its icssg_set_port_state() commands, which wait for a
firmware ack. In the non-promisc, non-allmulti case,
emac_ndo_set_rx_mode_work() then ends with:

	if (rtnl_trylock()) {
		vlan_for_each(ndev, icssg_update_vlan_mcast, emac);
		rtnl_unlock();
	}

HSR offload mode does the same with prueth->hsr_dev. If the trylock
fails, the vlan_for_each() call is skipped silently. The work does
not requeue itself, and nothing records that the update was skipped.

The firmware restart has just cleared SHRAM, MSMC and the VLAN table.
The per-VLAN multicast entries tracked in emac->vlan_mcast_list are
exactly the state that needs to be reprogrammed.

In the icssg_change_mode() path, this loop is the only thing that
queues rx_mode_work for a port that is already running. For the
opening port, the ndo_set_rx_mode call that __dev_open() makes later
also runs under rtnl. If the trylock loses in these cases, are the
VLAN multicast groups left unprogrammed in the firmware FDB, so that
multicast reception on those VLANs breaks?

The rtnl_trylock() skip existed before this patch. However, the patch
now uses this work as the re-sync mechanism and queues it from
contexts that are known to hold rtnl. That turns the skip from a rare
race into the likely outcome.

Should the worker reschedule itself when rtnl_trylock() fails? Or
could the VLAN multicast re-sync be done directly from the start
path, which already holds rtnl?

[Severity: Medium]
Can the opening port's rx_mode_work now run while emac_ndo_open() is
still sending its own HWQ commands on the same slice?

Before this patch, the work could not run for a port still inside
ndo_open, because __dev_set_rx_mode() requires IFF_UP.

With this loop, take a first open in non-promisc, non-allmulti mode
with pending multicast entries. For example, IPv6 adds 33:33:00:00:00:01
at NETDEV_REGISTER. The work then does:

emac_ndo_set_rx_mode_work()
  __dev_mc_sync()
    icssg_prueth_add_mcast()
      icssg_fdb_lookup()
      icssg_fdb_add_del()

Meanwhile emac_ndo_open() continues with:

	icssg_enable_fw_offload_emac(emac);
	...
	ret = emac_fdb_flow_id_updated(emac);

In switch/HSR mode the first of these sends FDB add commands. The second
always runs. Both paths end up in icssg_send_fdb_msg(), which takes no
lock:

- It pops a command buffer, pushes the command, then polls the response
  queue and takes whichever response comes out first.
- icssg_fdb_lookup() reads its result from the single per-slice
  FDB_CMD_BUFFER in DRAM.
- prueth->icssg_hwcmdseq is incremented non-atomically.

Only icssg_set_port_state() is serialized, by cmd_lock.

With two commands in flight, could the responses get swapped? That
could lead to:

- the seqnum mismatch WARN_ON in emac_fdb_flow_id_updated();
- ndo_open failing because it got another command's status;
- a wrong port_mask written into the FDB/VLAN table from another
  command's lookup data.

The flood commands could also reach the firmware before the RX flow ID
base is configured.

The restart path seems to have a similar window. After
prueth_emac_restart(), icssg_change_mode() calls
icssg_enable_fw_offload(), and the work queued here for both ports may
be running at that point.

> +
>  	emac = prueth->emac[ICSS_SLICE0] ? prueth->emac[ICSS_SLICE0] :
>  	       prueth->emac[ICSS_SLICE1];
>  	ret = icss_iep_init(emac->iep, &prueth_iep_clockops,
> @@ -323,6 +335,11 @@ static int prueth_emac_common_start(struct prueth *prueth)
>  	return 0;
>  
>  stop_pruss:
> +	for (slice = 0; slice < PRUETH_NUM_MACS; slice++) {
> +		emac = prueth->emac[slice];
> +		if (emac)
> +			cancel_work_sync(&emac->rx_mode_work);
> +	}
>  	prueth_emac_stop(prueth);

[Severity: Medium]
The commit message says the cancel is there "so it cannot run after
the cores are stopped again". However, this label is reached only when
icss_iep_init() fails.

Is the newly queued work also cancelled when emac_ndo_open() fails
after prueth_emac_common_start() has already succeeded?

__dev_open() sets __LINK_STATE_START before calling ndo_open. So
netif_running() is already true for the port being opened, and its
rx_mode_work is queued by the loop above.

A later step in emac_ndo_open() can still fail. Examples are
request_threaded_irq() for tx_ts_irq, prueth_create_rxq(),
prueth_create_txq(), or:

	ret = emac_fdb_flow_id_updated(emac);

The error path then ends at:

stop:
	if (!prueth->emacs_initialized)
		prueth_emac_common_stop(prueth);

This shuts down the PRU/RTU/txPRU cores without cancelling
rx_mode_work.

emac_ndo_stop() does call cancel_work_sync(&emac->rx_mode_work) before
prueth_emac_common_stop(). But ndo_stop is not called after a failed
open.

A work item that is still pending or running would then send commands
to halted cores:

- icssg_set_port_state() commands, each timing out with "timeout
  waiting for command done";
- in the non-promisc case, icssg_send_fdb_msg() commands that can poll
  for up to 20 seconds.

The multicast list may also be affected after the failure.
icssg_prueth_add_mcast() always returns 0, so __dev_mc_sync() marks the
addresses as synced.

After a failed open, nothing calls __dev_mc_unsync(). That includes
the stop_pruss path, where cancel_work_sync() cannot undo a sync that
has already completed.

The next successful open clears SHRAM, MSMC and the VLAN table and
reboots the firmware. Its __dev_mc_sync() would then skip the addresses
that are still marked synced.

Could this leave those multicast groups unprogrammed in the firmware
FDB?

Before this patch, the opening port's rx_mode_work could not run during
its own ndo_open, so this problem appears to be new.

>  
>  disable_class:

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261014125948.230571-1-r-kumar12%40ti.com

Re: [PATCH net 0/2] net: ti: icssg-prueth: fix port config and rx_mode drops

[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:

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

Re: [PATCH net 0/2] net: ti: icssg-prueth: fix port config and rx_mode drops

[KUMAR, RISHI]

Hi All,

On 10/14/2026 6:35 PM, netdev-bot+sinfo@kernel.org wrote:
>   - 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.
> 

Both patches were tested on real hardware.

Board:    AM64x EVM (AM6442)

ICSSG:    REL.PRU-ICSS-ETHERNET-SWITCH_02.02.16.01 firmware

Software: TI SDK 12.01.00.05.03 rt-am64xx-evm root filesystem, with
             a kernel built from net/main plus these two patches

Setup:    eth1 and eth2 bridged in br0 (switch offload)

Patch 1 (fix missing port config on second interface open):
Brought up br0 with eth1/eth2, then cycled the links (eth1 down,
eth2 down, eth1 up, eth2 up, and the reverse order). Sent 30
multicast frames to each port.

Before: after the cycle, fid_c1 of the default VLAN was 0x3 (or 0x5
for the reversed order), so only P0 and one slave port were members.
Multicast forwarded in one direction only.
After: each port is configured when it opens (fid_c1 = 0x7, P0 + P1
+ P2), and forwarding works in both directions after the cycle.

Patch 2 (re-apply rx_mode state after firmware reboot):
Brought up the offloaded switch interfaces and sent 30 multicast
frames to each port in turn.

Before: NRT_HOST_RX_PKT_COUNT did not increment for one port.
After: it increments for both ports, and forwarding works in both
directions.

HSR regression: brought up hsr0, sent 30 HSR-tagged frames in both
directions, cycled eth1/eth2 down and up, and sent again. Forwarding
works in both directions and NRT_HOST_RX_PKT_COUNT increments both
times.

Logs are in the gist links in the commit messages.

Thanks,
Rishi Kumar
ICSS India