Re: [PATCH v5 net-next 5/9] net: dsa: netc: use entry ID instead of pointer to track host flood rule

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] netc: Sleeping in Atomic Context during port_set_host_flood
--

commit ec5410333ea86338656a97517d41df973b9cde3c
Author: Wei Fang <wei.fang@nxp.com>

    net: dsa: netc: use entry ID instead of pointer to track host flood rule
    
    Replace the struct ipft_entry_data pointer in struct netc_port with a
    plain u32 entry ID (ipft_hf_eid), using NTMP_NULL_ENTRY_ID as the
    sentinel value. The ipft_entry_data allocation is now freed immediately
    inside netc_port_add_host_flood_rule() after the hardware entry is
    committed, so no heap memory survives beyond that function. As a result,
    netc_free_host_flood_rules() is no longer needed and can be removed.

> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> index c639ae67c9b71..e33a184fd0054 100644
> --- a/drivers/net/dsa/netc/netc_main.c
> +++ b/drivers/net/dsa/netc/netc_main.c
[ ... ]
> @@ -1890,7 +1876,7 @@ static void netc_port_set_host_flood(struct dsa_switch *ds, int port,
>  				     bool uc, bool mc)
>  {
>  	struct netc_port *np = NETC_PORT(ds, port);
> -	struct ipft_entry_data *old_host_flood;
> +	u32 old_entry_id;
>  
>  	/* Do not add host flood rule to ingress port filter table when
>  	 * the port has joined a bridge. Otherwise, the ingress frames
> @@ -1898,7 +1884,7 @@ static void netc_port_set_host_flood(struct dsa_switch *ds, int port,
>  	 * will be redirected directly to the CPU port.
>  	 */
>  	if (dsa_port_bridge_dev_get(np->dp)) {
> -		netc_port_remove_host_flood(np, np->host_flood);
> +		netc_port_remove_host_flood(np, np->ipft_hf_eid);
>  
>  		return;
>  	}
> @@ -1908,20 +1894,18 @@ static void netc_port_set_host_flood(struct dsa_switch *ds, int port,
>  
>  	/* IPFT does not support in-place updates to the KEYE element,
>  	 * we need to add a new entry and then delete the old one. So
> -	 * save the old entry first.
> +	 * save the old entry ID first.
>  	 */
> -	old_host_flood = np->host_flood;
> -	np->host_flood = NULL;
> +	old_entry_id = np->ipft_hf_eid;
>  
>  	if (netc_port_add_host_flood_rule(np, uc, mc)) {

[Severity: High]
Does this code sleep in atomic context?

The port_set_host_flood callback in DSA drivers is invoked via
ndo_change_rx_flags (e.g., when toggling promiscuous or allmulti mode on the
interface). The networking core holds netif_addr_lock_bh() during this
operation, creating an atomic context where sleeping is prohibited.

However, netc_port_set_host_flood() calls netc_port_add_host_flood_rule(),
which performs a sleepable allocation:

	host_flood = kzalloc_obj(*host_flood);

Additionally, it calls ntmp_ipft_add_entry(), which allocates a DMA buffer
using dma_alloc_coherent() with GFP_KERNEL and calls read_poll_timeout()
with a 10us sleep (usleep_range). Executing these sleepable operations while
holding a spinlock with BH disabled will trigger a sleeping-in-atomic panic.

> -		np->host_flood = old_host_flood;
>  		dev_err(ds->dev, "Failed to add host flood rule on port %d\n",
>  			port);
>  		return;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012095924.1129312-1-wei.fang@oss.nxp.com?part=5