Re: [PATCH v4 net-next 8/9] net: dsa: netc: add bridge mode support

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Static FDB entries with VID 0 on a VLAN-aware bridge are incorrectly mapped to the bridge's internal VLAN-unaware PVID, breaking forwarding for unspecified VLANs.
--

commit 2c77a1dbe137ce8f6001205692208e4092133f9f
Author: Wei Fang <wei.fang@nxp.com>

net: dsa: netc: add bridge mode support

This commit wires up the port_bridge_join, port_bridge_leave, and
port_vlan_filtering DSA callbacks to support both VLAN-unaware and
VLAN-aware bridge modes. It introduces a dedicated internal PVID for
VLAN-unaware bridges and manages dynamic FDB entry flushing.

> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> index fcca22da08d87..299a9e76b9aa9 100644
> --- a/drivers/net/dsa/netc/netc_main.c
> +++ b/drivers/net/dsa/netc/netc_main.c
[ ... ]
> @@ -1510,17 +1554,50 @@ static int netc_port_max_mtu(struct dsa_switch *ds, int port)
[ ... ]
>  static int netc_port_fdb_add(struct dsa_switch *ds, int port,
>  			     const unsigned char *addr, u16 vid,
>  			     struct dsa_db db)
>  {
> +	struct net_device *br_ndev = netc_classify_db(db);
>  	struct netc_port *np = NETC_PORT(ds, port);
>  
> -	/* Currently, only support standalone port mode, so only
> -	 * NETC_STANDALONE_PVID (= 0) is supported here.
> -	 */
> -	if (vid != NETC_STANDALONE_PVID)
> -		return -EOPNOTSUPP;
> +	if (IS_ERR(br_ndev))
> +		return PTR_ERR(br_ndev);
> +
> +	if (!vid)
> +		vid = netc_vlan_unaware_pvid(br_ndev ? &db.bridge : NULL);

[Severity: High]
Will this unconditionally map VID 0 entries to the bridge's internal
VLAN-unaware PVID, even if the bridge has VLAN filtering enabled?

When an FDB entry is added without a specific VLAN (VID 0) - such as local
port MAC addresses automatically added by the bridge core - it appears this
code translates it to the unaware PVID (e.g., 4095). However, if the bridge
is VLAN-aware, the port's ingress traffic uses the user-configured PVID or
explicitly tagged VLANs, not the unaware PVID.

Since this patch also removes the host flood rule when a port joins a bridge,
traffic destined to the local machine would miss the FDB lookup and be treated
as unknown unicast. Could this result in local traffic being leaked to other
ports via flooding, or dropped entirely if unknown unicast flooding is
disabled?

>  
>  	return netc_port_set_fdb_entry(np, addr, vid);
>  }
> @@ -1529,10 +1606,14 @@ static int netc_port_fdb_del(struct dsa_switch *ds, int port,
>  			     const unsigned char *addr, u16 vid,
>  			     struct dsa_db db)
>  {
> +	struct net_device *br_ndev = netc_classify_db(db);
>  	struct netc_port *np = NETC_PORT(ds, port);
>  
> -	if (vid != NETC_STANDALONE_PVID)
> -		return -EOPNOTSUPP;
> +	if (IS_ERR(br_ndev))
> +		return PTR_ERR(br_ndev);
> +
> +	if (!vid)
> +		vid = netc_vlan_unaware_pvid(br_ndev ? &db.bridge : NULL);

[Severity: High]
Does the same unconditional translation happen here during FDB entry deletion,
potentially preventing VID 0 entries from being removed from the hardware if
the bridge is VLAN-aware?

>  
>  	return netc_port_del_fdb_entry(np, addr, vid);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609032955.2066089-1-wei.fang@oss.nxp.com?part=8