linux-kernel - Re: [net-next,v21,7/7] net: mtip: Extend the L2 switch driver for imx287 with bridge operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260131015440.972531-1-kuba@kernel.org>
Date: Fri, 30 Jan 2026 17:54:40 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: lukasz.majewski@...lbox.org
Cc: Jakub Kicinski <kuba@...nel.org>,
	devicetree@...r.kernel.org,
	horms@...nel.org,
	krzk+dt@...nel.org,
	s.hauer@...gutronix.de,
	netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	kernel@...gutronix.de,
	richardcochran@...il.com,
	shawnguo@...nel.org,
	edumazet@...gle.com,
	linux-arm-kernel@...ts.infradead.org,
	andrew+netdev@...n.ch,
	pabeni@...hat.com,
	wahrenst@....net,
	davem@...emloft.net,
	festevam@...il.com,
	conor+dt@...nel.org,
	imx@...ts.linux.dev,
	andrew@...n.ch,
	robh@...nel.org
Subject: Re: [net-next,v21,7/7] net: mtip: Extend the L2 switch driver for imx287 with bridge operations

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
---
net: mtip: Extend the L2 switch driver for imx287 with bridge operations

This patch adds bridge operations support to the MTIP L2 switch driver,
enabling hardware offloading for packet switching when a bridge is
created on the switch interfaces.

> diff --git a/drivers/net/ethernet/freescale/mtipsw/mtipl2sw_br.c b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw_br.c
> --- /dev/null
> +++ b/drivers/net/ethernet/freescale/mtipsw/mtipl2sw_br.c

[ ... ]

> +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;
> +	int err;
> +
> +	/* 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 -EOPNOTSUPP;
> +		}
> +
> +		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);
> +
> +	err = switchdev_bridge_port_offload(ndev, ndev, NULL, NULL, NULL,
> +					    false, extack);
> +	if (err) {
> +		dev_err(&ndev->dev, "can't offload bridge port %s [err: %d]\n",
> +			ndev->name, err);
> +
> +		fep->br_members &= ~BIT(priv->portnum - 1);
> +		priv->master_dev = NULL;
> +
> +		return err;
> +	}

When switchdev_bridge_port_offload() fails, should the error path also
revert the state changes made earlier? Specifically, when the second port
joins the same bridge and passes the bridge check, the code sets
fep->br_offload = 1 and calls mtip_switch_dis_port_separation(). However,
the error handling only clears br_members and master_dev.

If switchdev_bridge_port_offload() fails at this point, fep->br_offload
remains set to 1 and port separation stays disabled, even though only
one port is actually bridged. This could leave the hardware in an
inconsistent state.

Looking at the TI cpsw driver for comparison, it modifies br_members
after switchdev_bridge_port_offload() succeeds, avoiding this ordering
issue.