Re: [PATCH net-next v9 3/6] net: airoha: Support multiple net_devices for a single FE GDM port

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Hardware configuration clashing for shared GDM ports
- [Medium] Hardware statistics are incorrectly shared across all net_devices attached to the same GDM port
--

commit 18f4c136c0685846d452a3b0c6a493df8cf6dda0
Author: Lorenzo Bianconi <lorenzo@kernel.org>

net: airoha: Support multiple net_devices for a single FE GDM port

EN7581 or AN7583 SoCs support connecting multiple external SerDes (e.g.
Ethernet or USB SerDes) to GDM3 or GDM4 ports via a hw arbiter that
manages the traffic in a TDM manner. As a result multiple net_devices can
connect to the same GDM{3,4} port and there is a theoretical "1:n"
relation between GDM ports and net_devices.

> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 57a16de0a2ec7..1d088d95d5fbc 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
[ ... ]
> @@ -1742,11 +1745,9 @@ static int airoha_dev_open(struct net_device *netdev)
>  			GLOBAL_CFG_RX_DMA_EN_MASK);
>  	atomic_inc(&qdma->users);
>  
> -	if (port->id == AIROHA_GDM2_IDX &&
> -	    airoha_ppe_is_enabled(qdma->eth, 1)) {
> -		/* For PPE2 always use secondary cpu port. */
> +	if (!airoha_is_lan_gdm_dev(dev) &&
> +	    airoha_ppe_is_enabled(qdma->eth, 1))
>  		pse_port = FE_PSE_PORT_PPE2;
> -	}
>  	airoha_set_gdm_port_fwd_cfg(qdma->eth, REG_GDM_FWD_CFG(port->id),
>  				    pse_port);

[Severity: High]
Since multiple net_devices now share a single physical GDM port, does calling
this unconditionally overwrite the shared port's forwarding configuration?

If one device on the shared port is brought up as a LAN port and another as
a WAN port, it seems the last opened device will override the forwarding
destination for all sibling devices on that port.

Are there similar issues with the ingress configuration?  Earlier in
airoha_dev_open(), REG_GDM_INGRESS_CFG(port->id) is modified based on whether
the specific netdev uses DSA:

	if (netdev_uses_dsa(netdev))
		airoha_fe_set(qdma->eth, REG_GDM_INGRESS_CFG(port->id),
			      GDM_STAG_EN_MASK);
	else
		airoha_fe_clear(qdma->eth, REG_GDM_INGRESS_CFG(port->id),
				GDM_STAG_EN_MASK);

Also, do airoha_dev_stop() and airoha_dev_change_mtu() have similar
regressions?  They appear to blindly overwrite shared registers like
REG_GDM_FWD_CFG (setting it to FE_PSE_PORT_DROP) and REG_GDM_LEN_CFG(port->id).

I noticed the stop() and MTU issues might be fixed in a subsequent patch, but
should this intermediate patch be self-contained to avoid breaking
bisectability?

[ ... ]
> @@ -3080,15 +3100,51 @@ static int airoha_alloc_gdm_port(struct airoha_eth *eth,
>  	u64_stats_init(&port->stats.syncp);
>  	spin_lock_init(&port->stats.lock);
>  	port->id = id;
> -	/* XXX: Read nbq from DTS */
> -	port->nbq = id == AIROHA_GDM3_IDX && airoha_is_7581(eth) ? 4 : 0;
>  	eth->ports[p] = port;
>  
>  	err = airoha_metadata_dst_alloc(port);
>  	if (err)
>  		return err;
>  
> -	return airoha_alloc_gdm_device(eth, port, np);
> +	/* Default nbq value to ensure backward compatibility */
> +	nbq = id == AIROHA_GDM3_IDX && airoha_is_7581(eth) ? 4 : 0;
> +
> +	for_each_child_of_node(np, node) {
> +		/* Multiple external serdes connected to the FE GDM port via an
> +		 * external arbiter.
> +		 */
> +		const __be32 *nbq_ptr;

[Severity: Medium]
Will these multiplexed devices report duplicated, incorrect hardware statistics?

When userspace queries statistics for any single multiplexed device,
airoha_dev_get_stats64() fetches from port->stats, which is updated by
airoha_update_hw_stats() reading the clear-on-read MAC hardware counters for
the entire physical GDM port.

Since the port->stats structure is shared across all net_devices attached to
the port, each device will report the total traffic volume of all devices
sharing the port rather than its own traffic.

Should the driver rely on per-queue software counters to provide accurate
per-device statistics when hardware MACs are multiplexed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603-airoha-eth-multi-serdes-v9-0-5d476bc2f426@kernel.org?part=3