RE: [PATCH net-next v4 10/13] net: renesas: rswitch: clean up is_rdev rswitch_device checking

[Michael Dege]

Hello Jakub,

Thank you for your comments.

> -----Original Message-----
> From: Jakub Kicinski <kuba@xxxxxxxxxx>
> Sent: Wednesday, May 13, 2026 3:36 AM
> To: Michael Dege <michael.dege@xxxxxxxxxxx>
> Cc: Jakub Kicinski <kuba@xxxxxxxxxx>; Yoshihiro Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>;
> andrew+netdev@xxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; pabeni@xxxxxxxxxx; niklas.soderlund
> <niklas.soderlund@xxxxxxxxxxxx>; paul@xxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-renesas-
> soc@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH net-next v4 10/13] net: renesas: rswitch: clean up is_rdev rswitch_device checking
> 
> This is an AI-generated review of your patch. The human sending this email has considered the AI
> review valid, or at least plausible.
> ---
> net: renesas: rswitch: clean up is_rdev rswitch_device checking
> 
> Add null pointer checking for ndev and remove unnecessay
> rswitch_port_check() wrapper function.
> 
> > Add null pointer checking for ndev and remove unnecessay
> > rswitch_port_check() wrapper function.
> 
> This isn't a bug, but there's a typo (unnecessay) in the commit message.
> 
> > diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c
> > b/drivers/net/ethernet/renesas/rswitch_l2.c
> > index 3cc0ce8762f3c..dd8aecbcb2a8b 100644
> > --- a/drivers/net/ethernet/renesas/rswitch_l2.c
> > +++ b/drivers/net/ethernet/renesas/rswitch_l2.c
> [ ... ]
> > @@ -278,11 +273,11 @@ static int rswitch_switchdev_blocking_event(struct notifier_block *nb,
> >  		return -EOPNOTSUPP;
> 
> This wasn't introduced by this patch, but does returning -EOPNOTSUPP directly here abort the global
> switchdev blocking notifier chain?
> 
> In a 32-bit signed integer representation, -EOPNOTSUPP (-95) has the 15th bit set (0xFFFFFFA1), which
> inadvertently matches NOTIFY_STOP_MASK (0x8000).
> 
> Because this happens without verifying if the target device belongs to the driver via is_rdev(ndev),
> it seems like this will silently break switchdev object offloading (like VLANs, MDBs, MRP) for all
> other NICs system-wide as long as this module is loaded.
> 
> Additionally, notifier_to_errno() strips the stop mask, transforming -95 to -32863. Since this is
> negative, it returns 0, which could trick the switchdev core into thinking the event was unhandled
> rather than explicitly unsupported.
> 
> Even though subsequent patches in this series (like "net: renesas: rswitch: add vlan aware switching")
> refactor this block, they still return -EOPNOTSUPP for non-VLAN objects by wrapping it in
> notifier_from_errno(-EOPNOTSUPP).
> 
> Because notifier_from_errno(-EOPNOTSUPP) produces NOTIFY_STOP_MASK | 96, doesn't the system-wide chain
> breakage persist?
> 
> >  	case SWITCHDEV_PORT_ATTR_SET:
> >  		ret = switchdev_handle_port_attr_set(ndev, ptr,
> > -						     rswitch_port_check,
> > +						     is_rdev,
> >  						     rswitch_port_attr_set);
> >  		break;
> >  	default:
> > -		if (!rswitch_port_check(ndev))
> > +		if (!is_rdev(ndev))
> >  			return NOTIFY_DONE;
> >  		ret = -EOPNOTSUPP;
> >  	}

I have reworked the event handler and -EOPNOTSUPP is not returned anymore.

Best regards,

Michael