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

[Jakub Kicinski]

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;
>  	}