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

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

[Ioana Ciornei]

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.

Fixes: 45035febc495 ("net: dpaa2-switch: refactor prechangeupper sanity checks")
Signed-off-by: Ioana Ciornei <ioana.ciornei@nxp.com>
---
 drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c b/drivers/net/ethernet/freescale/dpaa2/dpaa2-switch.c
index 52c1cb9cb7e0..46ae81c2fa01 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;
 	}
 
 	netdev_for_each_lower_dev(upper_dev, other_dev, iter) {
-- 
2.25.1

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

[Simon Horman]

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?

>  	}
>  
>  	netdev_for_each_lower_dev(upper_dev, other_dev, iter) {

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.