Re: [PATCH net-next v5 09/12] net: airoha: Do not stop GDM port if it is shared

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `GDM_STAG_EN_MASK` configuration is port-global but is modified unconditionally per net_device, leading to silent clobbering when multiple devices share a port.
- [Medium] The hardware MTU (`REG_GDM_LEN_CFG`) is not recalculated when a device sharing the port is stopped, leaving the MAC configured to accept inappropriately large frames.
--

commit b896e320dcf0d56db7e0654191126c23e6f485fd
Author: Lorenzo Bianconi <lorenzo@kernel.org>
net: airoha: Do not stop GDM port if it is shared

This commit introduces a users counter to the GDM port structure to allow
multiple independent net_device instances to share the same hardware port
without prematurely stopping it. It also conditionally updates the MTU so
that the largest MTU requested by any sharing device is used.

> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index be31f639f4be3..ebb24e60f1de3 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -1733,10 +1733,20 @@ static int airoha_dev_open(struct net_device *netdev)
>  		airoha_fe_clear(qdma->eth, REG_GDM_INGRESS_CFG(port->id),
>  				GDM_STAG_EN_MASK);

Since GDM_STAG_EN_MASK is a port-global configuration, will this clear the 
flag for the entire port if a secondary non-DSA net_device is brought up?

If the primary master device relies on this hardware tag parsing, it appears
a secondary device could silently break it by clearing the mask during
its open routine. Should this configuration be protected or refcounted when
multiple devices share a port?

> -	airoha_fe_rmw(qdma->eth, REG_GDM_LEN_CFG(port->id),
> -		      GDM_SHORT_LEN_MASK | GDM_LONG_LEN_MASK,
> -		      FIELD_PREP(GDM_SHORT_LEN_MASK, 60) |
> -		      FIELD_PREP(GDM_LONG_LEN_MASK, len));
> +	cur_len = airoha_fe_get(qdma->eth, REG_GDM_LEN_CFG(port->id),
> +				GDM_LONG_LEN_MASK);
> +	if (!port->users || len > cur_len) {
> +		/* Opening a sibling net_device with a larger MTU updates the
> +		 * MTU of already running devices. This is required to allow
> +		 * multiple net_devices wht different MTUs to share the same
> +		 * QDMA block.
> +		 */
> +		airoha_fe_rmw(qdma->eth, REG_GDM_LEN_CFG(port->id),
> +			      GDM_SHORT_LEN_MASK | GDM_LONG_LEN_MASK,
> +			      FIELD_PREP(GDM_SHORT_LEN_MASK, 60) |
> +			      FIELD_PREP(GDM_LONG_LEN_MASK, len));
> +	}
> +	port->users++;

[ ... ]

> @@ -1766,8 +1776,10 @@ static int airoha_dev_stop(struct net_device *netdev)
>  	for (i = 0; i < netdev->num_tx_queues; i++)
>  		netdev_tx_reset_subqueue(netdev, i);
>  
> -	airoha_set_gdm_port_fwd_cfg(qdma->eth, REG_GDM_FWD_CFG(port->id),
> -				    FE_PSE_PORT_DROP);
> +	if (!--port->users)
> +		airoha_set_gdm_port_fwd_cfg(qdma->eth,
> +					    REG_GDM_FWD_CFG(port->id),
> +					    FE_PSE_PORT_DROP);

If multiple devices are sharing the port and the device with the largest MTU
is stopped, the hardware MTU in REG_GDM_LEN_CFG doesn't appear to be
recalculated or shrunk.

Since the MTU is only expanded in airoha_dev_open() and airoha_dev_change_mtu(),
does this leave the MAC configured to accept inappropriately large frames
for the remaining devices indefinitely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260509-airoha-eth-multi-serdes-v5-0-805e38edc2aa@kernel.org?part=9