Re: [PATCH net-next v4 05/10] net: airoha: Move qos_sq_bmap in airoha_qdma struct

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 5004 bytes --]

On May 07, Lorenzo Bianconi wrote:
> Since now multiple net_devices connected to different QDMA blocks can
> share the same GDM port, qos_sq_bmap field can be overwritten with the
> configuration obtained from a net_device connected to a different QDMA
> block. In order to fix the issue move qos_sq_bmap field from
> airoha_gdm_port struct to airoha_qdma one.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  drivers/net/ethernet/airoha/airoha_eth.c | 20 ++++++++++----------
>  drivers/net/ethernet/airoha/airoha_eth.h |  4 ++--
>  2 files changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 080705e2f58d..69a4c2e0d58b 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -2600,7 +2600,7 @@ static int airoha_tc_htb_alloc_leaf_queue(struct net_device *netdev,
>  	u32 rate = div_u64(opt->rate, 1000) << 3; /* kbps */
>  	int err, num_tx_queues = netdev->real_num_tx_queues;
>  	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> -	struct airoha_gdm_port *port = dev->port;
> +	struct airoha_qdma *qdma = dev->qdma;
>  
>  	if (opt->parent_classid != TC_HTB_CLASSID_ROOT) {
>  		NL_SET_ERR_MSG_MOD(opt->extack, "invalid parent classid");
> @@ -2627,7 +2627,7 @@ static int airoha_tc_htb_alloc_leaf_queue(struct net_device *netdev,
>  		return err;
>  	}
>  
> -	set_bit(channel, port->qos_sq_bmap);
> +	set_bit(channel, qdma->qos_sq_bmap);
>  	opt->qid = AIROHA_NUM_TX_RING + channel;
>  
>  	return 0;
> @@ -2811,11 +2811,11 @@ static int airoha_dev_setup_tc_block(struct net_device *dev,
>  static void airoha_tc_remove_htb_queue(struct net_device *netdev, int queue)
>  {
>  	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> -	struct airoha_gdm_port *port = dev->port;
> +	struct airoha_qdma *qdma = dev->qdma;
>  
>  	netif_set_real_num_tx_queues(netdev, netdev->real_num_tx_queues - 1);
>  	airoha_qdma_set_tx_rate_limit(netdev, queue + 1, 0, 0);
> -	clear_bit(queue, port->qos_sq_bmap);
> +	clear_bit(queue, qdma->qos_sq_bmap);
>  }
>  
>  static int airoha_tc_htb_delete_leaf_queue(struct net_device *netdev,
> @@ -2823,9 +2823,9 @@ static int airoha_tc_htb_delete_leaf_queue(struct net_device *netdev,
>  {
>  	u32 channel = TC_H_MIN(opt->classid) % AIROHA_NUM_QOS_CHANNELS;
>  	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> -	struct airoha_gdm_port *port = dev->port;
> +	struct airoha_qdma *qdma = dev->qdma;
>  
> -	if (!test_bit(channel, port->qos_sq_bmap)) {
> +	if (!test_bit(channel, qdma->qos_sq_bmap)) {
>  		NL_SET_ERR_MSG_MOD(opt->extack, "invalid queue id");
>  		return -EINVAL;
>  	}
> @@ -2838,10 +2838,10 @@ static int airoha_tc_htb_delete_leaf_queue(struct net_device *netdev,
>  static int airoha_tc_htb_destroy(struct net_device *netdev)
>  {
>  	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> -	struct airoha_gdm_port *port = dev->port;
> +	struct airoha_qdma *qdma = dev->qdma;
>  	int q;
>  
> -	for_each_set_bit(q, port->qos_sq_bmap, AIROHA_NUM_QOS_CHANNELS)
> +	for_each_set_bit(q, qdma->qos_sq_bmap, AIROHA_NUM_QOS_CHANNELS)
>  		airoha_tc_remove_htb_queue(netdev, q);
>  
>  	return 0;
> @@ -2852,9 +2852,9 @@ static int airoha_tc_get_htb_get_leaf_queue(struct net_device *netdev,
>  {
>  	u32 channel = TC_H_MIN(opt->classid) % AIROHA_NUM_QOS_CHANNELS;
>  	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> -	struct airoha_gdm_port *port = dev->port;
> +	struct airoha_qdma *qdma = dev->qdma;
>  
> -	if (!test_bit(channel, port->qos_sq_bmap)) {
> +	if (!test_bit(channel, qdma->qos_sq_bmap)) {
>  		NL_SET_ERR_MSG_MOD(opt->extack, "invalid queue id");
>  		return -EINVAL;
>  	}
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
> index 18a9dfd75d44..34f1fef51640 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.h
> +++ b/drivers/net/ethernet/airoha/airoha_eth.h
> @@ -531,6 +531,8 @@ struct airoha_qdma {
>  
>  	struct airoha_queue q_tx[AIROHA_NUM_TX_RING];
>  	struct airoha_queue q_rx[AIROHA_NUM_RX_RING];
> +
> +	DECLARE_BITMAP(qos_sq_bmap, AIROHA_NUM_QOS_CHANNELS);
>  };
>  
>  struct airoha_gdm_dev {
> @@ -547,8 +549,6 @@ struct airoha_gdm_port {
>  
>  	struct airoha_hw_stats stats;
>  
> -	DECLARE_BITMAP(qos_sq_bmap, AIROHA_NUM_QOS_CHANNELS);
> -
>  	/* qos stats counters */
>  	u64 cpu_tx_packets;
>  	u64 fwd_tx_packets;
> 
> -- 
> 2.54.0
> 

commenting on sashiko report:
https://sashiko.dev/#/patchset/20260507-airoha-eth-multi-serdes-v4-0-af613b61ae02%40kernel.org

- Following up on the real_num_tx_queues logic above, does blindly decrementing
  this by 1 dynamically shrink the queue range and instantly invalidate other
  active queues whose qid might now fall out of bounds?
  - This issue has not been introduced by this series and I will fix it with a
    separated patch

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]