Re: [PATCH v2 net-next 5/7] net: dsa: netc: enable ingress port filtering lookup by default

[email protected]
Newsgroups dev.linux.lists.imx
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The DSA `port_set_host_flood` callback executes sleepable operations in atomic context.
--

--- Patch [5]: [PATCH v2 net-next 5/7] net: dsa: netc: enable ingress port filtering lookup by default ---
commit eec39f0d058c5db3118dad52f6e11e6878ffa678
Author: Wei Fang <[email protected]>

net: dsa: netc: enable ingress port filtering lookup by default

This commit enables the ingress port filtering lookup by default for netc.
It simplifies the code logic by removing the need to track and toggle the
lookup state when adding or removing host flood rules.

> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> index d326a00104e1d..9cb9e618661e5 100644
> --- a/drivers/net/dsa/netc/netc_main.c
> +++ b/drivers/net/dsa/netc/netc_main.c

[ ... ]

> @@ -1753,8 +1757,6 @@ static int netc_port_add_host_flood_rule(struct netc_port *np,
>  	np->uc = uc;
>  	np->mc = mc;
>  	np->ipft_hf_eid = host_flood->entry_id;
> -	/* Enable ingress port filter table lookup */
> -	netc_port_wr(np, NETC_PIPFCR, PIPFCR_EN);
>  
>  free_host_flood:
>  	kfree(host_flood);

[Severity: High]
This isn't a bug introduced by this patch, but does
netc_port_add_host_flood_rule() execute sleepable operations in atomic context?

When userspace changes interface flags (e.g., ip link set dev ethX promisc on),
the network core calls ndo_change_rx_flags while holding netif_addr_lock_bh(),
which disables bottom halves and establishes an atomic context.

The DSA core delegates this to the port_set_host_flood callback, which calls
netc_port_set_host_flood() -> netc_port_add_host_flood_rule() synchronously.

Looking at the code, do these functions execute sleepable operations?

- Allocating memory using GFP_KERNEL (via kzalloc_obj() in 
  netc_port_add_host_flood_rule())
- Acquiring a sleeping lock (via mutex_lock() in ntmp_select_and_lock_cbdr())
- Yielding to the scheduler (via read_poll_timeout() with sleep=true in
  netc_xmit_ntmp_cmd())

Could this lead to a "scheduling while atomic" panic if
CONFIG_DEBUG_ATOMIC_SLEEP is enabled? Should this hardware configuration
be deferred to a workqueue?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.