Re: [net-next v4 4/5] net: mtip: The L2 switch driver for imx287

Andrew Lunn
2025-04-09T22:58:12Z

> +static int mtip_ndev_port_link(struct net_device *ndev,
> + struct net_device *br_ndev,
> + struct netlink_ext_ack *extack)
> +{
> + struct mtip_ndev_priv *priv = netdev_priv(ndev), *other_priv;
> + struct switch_enet_private *fep = priv->fep;
> + struct net_device *other_ndev;
> +
> + /* Check if one port of MTIP switch is already bridged */
> + if (fep->br_members && !fep->br_offload) {
> + /* Get the second bridge ndev */
> + other_ndev = fep->ndev[fep->br_members - 1];
> + other_priv = netdev_priv(other_ndev);
> + if (other_priv->master_dev != br_ndev) {
> + NL_SET_ERR_MSG_MOD(extack,
> + "L2 offloading only possible for the same bridge!");
> + return notifier_from_errno(-EOPNOTSUPP);

This is not an error condition as such. The switch cannot do it, so
-EOPNOTSUPP is correct, but the Linux software bridge can, and it
will. So there is no need to use NL_SET_ERR_MSG_MOD().

> + }
> +
> + fep->br_offload = 1;
> + mtip_switch_dis_port_separation(fep);
> + mtip_clear_atable(fep);
> + }
> +
> + if (!priv->master_dev)
> + priv->master_dev = br_ndev;
> +
> + fep->br_members |= BIT(priv->portnum - 1);
> +
> + dev_dbg(&ndev->dev,
> + "%s: ndev: %s br: %s fep: 0x%x members: 0x%x offload: %d\n",
> + __func__, ndev->name, br_ndev->name, (unsigned int)fep,
> + fep->br_members, fep->br_offload);
> +
> + return NOTIFY_DONE;
> +}
> +
> +static void mtip_netdevice_port_unlink(struct net_device *ndev)
> +{
> + struct mtip_ndev_priv *priv = netdev_priv(ndev);
> + struct switch_enet_private *fep = priv->fep;
> +
> + dev_dbg(&ndev->dev, "%s: ndev: %s members: 0x%x\n", __func__,
> + ndev->name, fep->br_members);
> +
> + fep->br_members &= ~BIT(priv->portnum - 1);
> + priv->master_dev = NULL;
> +
> + if (!fep->br_members) {
> + fep->br_offload = 0;
> + mtip_switch_en_port_separation(fep);
> + mtip_clear_atable(fep);
> + }

This does not look quite correct. So you disable port separation once
both ports are a member of the same bridge. So you should enable port
separation as soon as one leaves. So if fep->br_members has 0 or 1
bits set, you need to enable port separation.

Andrew