Re: [PATCH net] net: airoha: fix HTB class modification offload

[Lorenzo Bianconi <lorenzo@kernel.org>]

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

> HTB core does not populate parent_classid for TC_HTB_NODE_MODIFY.
> Airoha currently checks parent_classid against TC_HTB_CLASSID_ROOT
> in a helper shared by both TC_HTB_LEAF_ALLOC_QUEUE and
> TC_HTB_NODE_MODIFY. Since the modify path leaves parent_classid as
> zero, the check always fails and changing parameters of an already
> offloaded HTB class is rejected with -EINVAL.
> 
> Move the root-parent check into the allocation path and validate
> modify requests using the per-netdev QoS channel bitmap, consistent
> with the delete and query paths.
> 
> Fixes: ef1ca9271313 ("net: airoha: Add sched HTB offload support")
> Signed-off-by: Wayen Yan <win847@gmail.com>

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> ---
>  drivers/net/ethernet/airoha/airoha_eth.c | 32 +++++++++++++++++-------
>  1 file changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
> index 59001fd4b6f7..8e4e79c1e4c2 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.c
> +++ b/drivers/net/ethernet/airoha/airoha_eth.c
> @@ -2766,18 +2766,13 @@ static int airoha_qdma_set_tx_rate_limit(struct net_device *netdev,
>  	return 0;
>  }
>  
> -static int airoha_tc_htb_modify_queue(struct net_device *dev,
> -				      struct tc_htb_qopt_offload *opt)
> +static int airoha_tc_htb_set_rate(struct net_device *dev,
> +				  struct tc_htb_qopt_offload *opt,
> +				  u32 channel)
>  {
> -	u32 channel = TC_H_MIN(opt->classid) % AIROHA_NUM_QOS_CHANNELS;
>  	u32 rate = div_u64(opt->rate, 1000) << 3; /* kbps */
>  	int err;
>  
> -	if (opt->parent_classid != TC_HTB_CLASSID_ROOT) {
> -		NL_SET_ERR_MSG_MOD(opt->extack, "invalid parent classid");
> -		return -EINVAL;
> -	}
> -
>  	err = airoha_qdma_set_tx_rate_limit(dev, channel, rate, opt->quantum);
>  	if (err)
>  		NL_SET_ERR_MSG_MOD(opt->extack,
> @@ -2786,6 +2781,20 @@ static int airoha_tc_htb_modify_queue(struct net_device *dev,
>  	return err;
>  }
>  
> +static int airoha_tc_htb_modify_queue(struct net_device *netdev,
> +				      struct tc_htb_qopt_offload *opt)
> +{
> +	u32 channel = TC_H_MIN(opt->classid) % AIROHA_NUM_QOS_CHANNELS;
> +	struct airoha_gdm_dev *dev = netdev_priv(netdev);
> +
> +	if (!test_bit(channel, dev->qos_sq_bmap)) {
> +		NL_SET_ERR_MSG_MOD(opt->extack, "invalid queue id");
> +		return -EINVAL;
> +	}
> +
> +	return airoha_tc_htb_set_rate(netdev, opt, channel);
> +}
> +
>  static int airoha_tc_htb_alloc_leaf_queue(struct net_device *netdev,
>  					  struct tc_htb_qopt_offload *opt)
>  {
> @@ -2794,6 +2803,11 @@ static int airoha_tc_htb_alloc_leaf_queue(struct net_device *netdev,
>  	struct airoha_gdm_dev *dev = netdev_priv(netdev);
>  	struct airoha_qdma *qdma = dev->qdma;
>  
> +	if (opt->parent_classid != TC_HTB_CLASSID_ROOT) {
> +		NL_SET_ERR_MSG_MOD(opt->extack, "invalid parent classid");
> +		return -EINVAL;
> +	}
> +
>  	/* Here we need to check the requested QDMA channel is not already
>  	 * in use by another net_device running on the same QDMA block.
>  	 */
> @@ -2803,7 +2817,7 @@ static int airoha_tc_htb_alloc_leaf_queue(struct net_device *netdev,
>  		return -EBUSY;
>  	}
>  
> -	err = airoha_tc_htb_modify_queue(netdev, opt);
> +	err = airoha_tc_htb_set_rate(netdev, opt, channel);
>  	if (err)
>  		goto error;
>  
> -- 
> 2.51.0
> 
> 

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