Re: [PATCH v4 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] netc_port_update_vlan_egress_rule() incorrectly checks ett_eid instead of ect_eid before issuing an Egress Counter Table update, causing a hardware fault when ECT entries are exhausted.
--

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

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

Implement the DSA .port_vlan_add and .port_vlan_del operations to enable
VLAN-aware bridge offloading on the NETC switch.

> diff --git a/drivers/net/dsa/netc/netc_main.c b/drivers/net/dsa/netc/netc_main.c
> --- 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_update_vlan_egress_rule(struct netc_port *np,
> +					     struct netc_vlan_entry *entry)
> +{
> +	bool untagged = !!(entry->untagged_port_bitmap & BIT(np->dp->index));
> +	u32 num_ports = netc_num_available_ports(np->switch_priv);
> +	u32 ett_eid = le32_to_cpu(entry->cfge.et_eid);
> +	struct netc_switch *priv = np->switch_priv;
> +	u32 ect_eid = NTMP_NULL_ENTRY_ID;
> +	int err;

[ ... ]

> +	if (entry->ect_gid != NTMP_NULL_ENTRY_ID)
> +		/* Each ETT entry maps to an ECT entry if ect_gid is not NULL
> +		 * entry ID. The offset of the ECT entry corresponding to the
> +		 * port in the group is equal to ett_offset.
> +		 */
> +		ect_eid = entry->ect_gid * num_ports + np->ett_offset;
> +
> +	ett_eid += np->ett_offset;
> +	err = netc_update_ett_entry(priv, untagged, ett_eid, ect_eid);
> +	if (err) {
> +		dev_err(priv->dev,
> +			"Failed to update VLAN %u egress rule on port %d\n",
> +			entry->vid, np->dp->index);
> +		return err;
> +	}
> +
> +	if (ett_eid != NTMP_NULL_ENTRY_ID)
> +		ntmp_ect_update_entry(&priv->ntmp, ect_eid);

[Severity: High]
Should this check ect_eid instead of ett_eid before updating the counter
entry?

When Egress Counter Table (ECT) entries are exhausted, entry->ect_gid will
be NTMP_NULL_ENTRY_ID and ect_eid will keep its default value of
NTMP_NULL_ENTRY_ID. Because ett_eid was previously incremented by
np->ett_offset, it is guaranteed to be non-null here even if ECT entries
were exhausted.

As a result, ntmp_ect_update_entry() could be executed with
ect_eid = NTMP_NULL_ENTRY_ID, which would submit an out-of-bounds index
update to the switch hardware and cause a command bus error.

> +
> +	return 0;
> +}

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