Re: [PATCH v5 net-next 7/9] net: dsa: netc: add VLAN filter table and egress treatment management

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unconditional ETT/ECT allocation wastes hardware entries and may cause VLAN-unaware bridge traffic to egress with internal VLAN tags.
--

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

net: dsa: netc: add VLAN filter table and egress treatment management

This commit implements the DSA .port_vlan_add and .port_vlan_del operations
to enable VLAN-aware bridge offloading on the NETC switch. It maintains
VLAN membership in the VLAN Filter Table and egress tagging through the
Egress Treatment Table.

> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> index d4475ad7ed6c2..9382e7b68e652 100644
> --- a/drivers/net/dsa/netc/netc_main.c
> +++ b/drivers/net/dsa/netc/netc_main.c

[ ... ]

> @@ -1025,6 +1054,385 @@ static void netc_switch_get_ip_revision(struct netc_switch *priv)

[ ... ]

> +static int netc_port_add_vlan_entry(struct netc_port *np, u16 vid,
> +				    bool untagged)
> +{
> +	struct netc_switch *priv = np->switch_priv;
> +	struct netc_vlan_entry *entry;
> +	struct vft_cfge_data *cfge;
> +	u32 index = np->dp->index;
> +	u32 bitmap_stg;
> +	int err;
> +	u16 cfg;

[ ... ]

> +	cfge = &entry->cfge;
> +	cfge->et_eid = cpu_to_le32(NTMP_NULL_ENTRY_ID);
> +	cfge->bitmap_stg = cpu_to_le32(bitmap_stg);
> +	cfge->fid = cpu_to_le16(vid);
> +	cfge->cfg = cpu_to_le16(cfg);
> +	cfge->eta_port_bitmap = cpu_to_le32(priv->port_bitmap);
> +
> +	if (untagged)
> +		entry->untagged_port_bitmap = BIT(index);
> +
> +	err = netc_add_vlan_egress_rule(priv, entry);
> +	if (err)
> +		goto free_vlan_entry;

[Severity: High]
Should this egress rule allocation be skipped for VLAN-unaware PVIDs?

In netc_port_vlan_egress_rule_changed(), there is a check noting that
VLAN-unaware VIDs have no egress rules. However, netc_port_add_vlan_entry()
unconditionally calls netc_add_vlan_egress_rule() for all VIDs.

When a port joins a VLAN-unaware bridge, netc_port_bridge_join() sets up
the internal PVID by calling netc_port_set_vlan_entry() with
untagged=false. Because ETT allocation is unconditional here, it appears
a limited Egress Treatment Table (ETT) and Egress Counter Table (ECT)
group is unnecessarily consumed for each VLAN-unaware bridge instance.

Since untagged=false is passed, does this incorrectly configure the ETT
entry to preserve the outer VLAN tag on egress (lacking the
FMTEID_VUDA_DEL_OTAG action)? This could result in hardware transmitting
untagged bridge traffic with the switch's internal PVID (e.g., 4095)
attached, potentially leaking internal configurations.

> +
> +	err = ntmp_vft_add_entry(&priv->ntmp, vid, cfge);
> +	if (err) {
> +		dev_err(priv->dev,
> +			"Failed to add VLAN %u entry on port %d\n",
> +			vid, index);
> +		goto delete_vlan_egress_rule;
> +	}

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