Re: [PATCH net-next v3 12/12] dpaa2-switch: add support for imprecise source port
From: Ioana Ciornei
Date: Mon Jun 08 2026 - 08:41:57 EST
On Fri, Jun 05, 2026 at 07:11:05PM -0700, Jakub Kicinski wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> ---
> dpaa2-switch: add support for imprecise source port
>
> Switch ports configured as part of a LAG group are not able to provide
> a precise source port for all packets which reach the control interface.
>
> The only frames which will have a precise source port are those that are
> explicitly trapped, for example STP and LCAP frames. For any other
> frames (for example, those which are flooded) we can only know the
> ingress LAG group.
>
> Take into account the DPAA2_ETHSW_FLC_IMPRECISE_IF_ID bit and based on
> its value target the bond device or the specific source netdevice.
>
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> > index 7aef34778e220..163f6c51c1606 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> > @@ -3270,8 +3272,10 @@ static void dpaa2_switch_rx(struct dpaa2_switch_fq *fq,
> > rcu_read_lock();
> >
> > lag = rcu_dereference(port_priv->lag);
> > -
> > - skb->dev = netdev;
> > + if (DPAA2_ETHSW_FLC_IMPRECISE_IF_ID(flc) && lag)
> > + skb->dev = lag->bond_dev;
>
> [Severity: High]
> By assigning skb->dev = lag->bond_dev here, isn't there a risk of a
> use-after-free on PREEMPT_RT?
>
> The explicit rcu_read_lock() is dropped before handing the SKB over to
> netif_receive_skb(). While NAPI's local_bh_disable() provides RCU protection
> on standard kernels, softirqs are threaded on PREEMPT_RT and do not provide
> an implicit RCU read-side critical section.
>
> Between the rcu_read_unlock() and the internal rcu_read_lock() taken by
> netif_receive_skb_core(), the threaded softirq can be preempted. If the bond
> device is concurrently unregistered, synchronize_net() will not wait for the
> NAPI thread, allowing bond_dev to be freed before netif_receive_skb()
> dereferences it.
>
>From what I know, the NAPI context already has an explicit
rcu_read_lock() taken from local_bh_disable() even under PREEMPT_RT.
So the feedback is incorrect from my point of view and will not get
addressed.