Re: [PATCH net] dpaa2-switch: fix VLAN upper check not rejecting bridge join

[Ioana Ciornei]

On Wed, Jun 17, 2026 at 12:45:18PM +0100, Simon Horman 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.
> Full review at: https://sashiko.dev
> ---
> dpaa2-switch: fix VLAN upper check not rejecting bridge join
> 
> The blamed commit refactored the prechangeupper event handling but
> failed to actually return an error in case
> dpaa2_switch_prevent_bridging_with_8021q_upper() detected a 802.1q upper
> on a port which tries to join a bridge. Fix this by returning err
> instead of 0.
> 
> > diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> > index 52c1cb9cb7e02..46ae81c2fa013 100644
> > --- a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> > +++ b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
> > @@ -2177,7 +2177,7 @@ dpaa2_switch_prechangeupper_sanity_checks(struct net_device *netdev,
> >  	if (err) {
> >  		NL_SET_ERR_MSG_MOD(extack,
> >  				   "Cannot join a bridge while VLAN uppers are present");
> > -		return 0;
> > +		return err;
> 
> [Severity: High]
> If a user adds an 802.1q VLAN upper device to a port after it has already
> successfully joined a bridge, won't this check trigger when the port leaves
> the bridge?
> 
> In drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c,
> dpaa2_switch_prechangeupper_sanity_checks() is called for both bridge joins
> and unlinks from dpaa2_switch_port_prechangeupper():
> 
>     err = dpaa2_switch_prechangeupper_sanity_checks(netdev, upper_dev, extack);
>     if (err)
>         return err;
> 
>     if (!info->linking)
>         dpaa2_switch_port_pre_bridge_leave(netdev);
> 
> Since returning an error aborts the notifier chain, an unlink notification
> would fail this check, preventing dpaa2_switch_port_pre_bridge_leave() from
> running. Does this lead to state inconsistency and skipped switchdev
> unoffloads?
> 
> Could this check be guarded so it only runs when info->linking is true?

Valid point but the fix should be to prevent adding VLAN uppers on
bridged dpaa2-switch ports since the driver does not support the usecase
altogether.