RE: [net-next,v3,13/13] net: renesas: rswitch: add vlan aware switching

[Michael Dege]

Hello Paolo,

Thank you for your review comments.

> -----Original Message-----
> From: Paolo Abeni <pabeni@xxxxxxxxxx>
> Sent: Thursday, April 2, 2026 3:00 PM
> To: Michael Dege <michael.dege@xxxxxxxxxxx>
> Cc: paul@xxxxxxxxxxx; niklas.soderlund <niklas.soderlund@xxxxxxxxxxxx>; kuba@xxxxxxxxxx; Yoshihiro
> Shimoda <yoshihiro.shimoda.uh@xxxxxxxxxxx>; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> edumazet@xxxxxxxxxx; pabeni@xxxxxxxxxx; linux-renesas-soc@xxxxxxxxxxxxxxx; andrew+netdev@xxxxxxx;
> davem@xxxxxxxxxxxxx
> Subject: Re: [net-next,v3,13/13] net: renesas: rswitch: add vlan aware switching
>
> This is an AI-generated review of your patch. The human sending this email has considered the AI
> review valid, or at least plausible.
>
> For local reproduction steps see
> https://netdev-ai.bots.linux.dev/ai-
> local.html&data=05%7C02%7Cmichael.dege%40renesas.com%7C10d1c680d41c4e17dc2408de90b7bf4a%7C53d82571da19
> 47e49cb4625a166a4a2a%7C0%7C0%7C639107316024723538%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYi
> OiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=KVia3Rm%2FIDae%2Fq
> NCrKk08fsOpcv%2FnSMZoApYSZsLCYI%3D&reserved=0
> ---
> net: renesas: rswitch: add vlan aware switching
>
> This commit adds vlan support to the L2 HW bridge, with limited support on R-Car S4 (not fully 802.1Q
> compliant).
>
> > diff --git a/drivers/net/ethernet/renesas/Kconfig
> > b/drivers/net/ethernet/renesas/Kconfig
> > index 9b7559c88bee..2494ff60e8c2 100644
> > --- a/drivers/net/ethernet/renesas/Kconfig
> > +++ b/drivers/net/ethernet/renesas/Kconfig
>
> [ ... ]
>
> > diff --git a/drivers/net/ethernet/renesas/rswitch_l2.c
> > b/drivers/net/ethernet/renesas/rswitch_l2.c
> > index ea95a87ed234..b7d0e7f94ecd 100644
> > --- a/drivers/net/ethernet/renesas/rswitch_l2.c
> > +++ b/drivers/net/ethernet/renesas/rswitch_l2.c
>
> [ ... ]
>
> > @@ -212,61 +197,397 @@ static int rswitch_netdevice_event(struct
> > notifier_block *nb,
>
> [ ... ]
>
> > +static int rswitch_handle_port_obj_add(struct net_device *ndev,
> > +                                  struct notifier_block *nb,
> > +                                  struct switchdev_notifier_port_obj_info *info) {
> > +   struct switchdev_obj_port_vlan *p_vlan = SWITCHDEV_OBJ_PORT_VLAN(info->obj);
> > +   struct rswitch_private *priv;
> > +   int err;
> > +
> > +   priv = container_of(nb, struct rswitch_private,
> > +rswitch_switchdev_blocking_nb);
> > +
> > +   if ((p_vlan->flags & BRIDGE_VLAN_INFO_MASTER) ||
> > +       (p_vlan->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) ||
> > +       (p_vlan->flags & BRIDGE_VLAN_INFO_RANGE_END) ||
> > +       (p_vlan->flags & BRIDGE_VLAN_INFO_ONLY_OPTS))
> > +           return NOTIFY_DONE;
> > +
> > +   switch (info->obj->id) {
> > +   case SWITCHDEV_OBJ_ID_PORT_VLAN:
>
> Can the flag checks access the wrong memory? The code casts info->obj to switchdev_obj_port_vlan
> before checking info->obj->id. When the notifier delivers a non-VLAN object (such as
> SWITCHDEV_OBJ_ID_PORT_MDB for multicast group changes), SWITCHDEV_OBJ_PORT_VLAN() performs an
> incorrect container_of cast, and p_vlan->flags reads bytes from a different struct type.
>
> For switchdev_obj_port_mdb, the p_vlan->flags field overlaps with the first
> 2 bytes of the addr[ETH_ALEN] MAC address field. Multicast MAC addresses always have bit 0 of the
> first byte set, which matches BRIDGE_VLAN_INFO_MASTER (1<<0), causing the function to return
> NOTIFY_DONE instead of falling through to the default case that returns -EOPNOTSUPP.
>
> Other switchdev drivers (lan966x, mlxsw, cpsw) perform the SWITCHDEV_OBJ_PORT_VLAN cast inside the
> SWITCHDEV_OBJ_ID_PORT_VLAN case, not before the type switch.
>
> The same issue exists in rswitch_handle_port_obj_del().

Thank you this makes sense, I changed it.

Best regards,

Michael
>
> > +           if (!is_rdev(ndev))
> > +                   err = rswitch_port_obj_do_add_gwca(ndev, priv, p_vlan);
> > +           else
> > +                   err = rswitch_port_obj_do_add(ndev, p_vlan);
> > +
> > +           if (err < 0)
> > +                   return err;
> > +
> > +           break;
> > +   default:
> > +           return -EOPNOTSUPP;
> > +   }
> > +
> > +   info->handled = true;
> > +
> > +   return NOTIFY_DONE;
> > +}
> > +
> > +static int rswitch_handle_port_obj_del(struct net_device *ndev,
> > +                                  struct notifier_block *nb,
> > +                                  struct switchdev_notifier_port_obj_info *info) {
> > +   struct switchdev_obj_port_vlan *p_vlan = SWITCHDEV_OBJ_PORT_VLAN(info->obj);
> > +   struct rswitch_private *priv;
> > +   int err;
> > +
> > +   priv = container_of(nb, struct rswitch_private,
> > +rswitch_switchdev_blocking_nb);
> > +
> > +   if ((p_vlan->flags & BRIDGE_VLAN_INFO_MASTER) ||
> > +       (p_vlan->flags & BRIDGE_VLAN_INFO_RANGE_BEGIN) ||
> > +       (p_vlan->flags & BRIDGE_VLAN_INFO_RANGE_END) ||
> > +       (p_vlan->flags & BRIDGE_VLAN_INFO_ONLY_OPTS))
> > +           return NOTIFY_DONE;
> > +
> > +   switch (info->obj->id) {
> > +   case SWITCHDEV_OBJ_ID_PORT_VLAN:
> > +           if (!is_rdev(ndev))
> > +                   err = rswitch_port_obj_do_del_gwca(ndev, priv, p_vlan);
> > +           else
> > +                   err = rswitch_port_obj_do_del(ndev, p_vlan);
> > +
> > +           if (err < 0)
> > +                   return err;
> > +
> > +           break;
> > +   default:
> > +           return -EOPNOTSUPP;
> > +   }
> > +
> > +   info->handled = true;
> > +
> > +   return NOTIFY_DONE;
> > +}
>
> [ ... ]