Hi Hariprasad,
On Sat, May 13, 2023 at 02:21:39PM +0530, Hariprasad Kelam wrote:
> 1. Upon txschq free request, the transmit schedular config in hardware
> is not getting reset. This patch adds necessary changes to do the same.
>
nit: s/schedular/scheduler/
> 2. Current implementation calls txschq alloc during interface
> initialization and in response handler updates the default txschq array.
> This creates a problem for htb offload where txsch alloc will be called
> for every tc class. This patch addresses the issue by reading txschq
> response in mbox caller function instead in the response handler.
>
> 3. Current otx2_txschq_stop routine tries to free all txschq nodes
> allocated to the interface. This creates a problem for htb offload.
> This patch introduces the otx2_txschq_free_one to free txschq in a
> given level.
This patch seems to be doing three things.
Could it be split into three patches?
...
> -int otx2_txschq_stop(struct otx2_nic *pfvf)
> +void otx2_txschq_free_one(struct otx2_nic *pfvf, u16 lvl, u16 schq)
> {
> struct nix_txsch_free_req *free_req;
> - int lvl, schq, err;
> + int err;
>
> mutex_lock(&pfvf->mbox.lock);
> - /* Free the transmit schedulers */
> +
> free_req = otx2_mbox_alloc_msg_nix_txsch_free(&pfvf->mbox);
Mainly for my own edification:
- otx2_mbox_alloc_msg_nix_txsch_free is created via the
M(_name, _id, _fn_name, _req_type, _rsp_type) macro
around line 844 of otx2_common.h
- It calls otx2_mbox_alloc_msg_rsp
- Which does not call any allocation functions such as kmalloc
> if (!free_req) {
> mutex_unlock(&pfvf->mbox.lock);
> - return -ENOMEM;
> + netdev_err(pfvf->netdev,
> + "Failed alloc txschq free req\n");
I think that given the above it's ok to log an error here.
As the allocation core won't have (because it's not used here.
But I wonder if it would be more consistent with how
allocation errors are usually handled to move the logging into
otx2_mbox_alloc_msg_rsp().
> + return;
> }
>
> - free_req->flags = TXSCHQ_FREE_ALL;
> + free_req->schq_lvl = lvl;
> + free_req->schq = schq;
> +
> err = otx2_sync_mbox_msg(&pfvf->mbox);
> + if (err) {
> + netdev_err(pfvf->netdev,
> + "Failed stop txschq %d at level %d\n", schq, lvl);
> + }
> +
> mutex_unlock(&pfvf->mbox.lock);
> +}
> +
> +void otx2_txschq_stop(struct otx2_nic *pfvf)
> +{
> + int lvl, schq;
> +
> + /* free non QOS TLx nodes */
> + for (lvl = 0; lvl < NIX_TXSCH_LVL_CNT; lvl++)
> + otx2_txschq_free_one(pfvf, lvl,
> + pfvf->hw.txschq_list[lvl][0]);
>
> /* Clear the txschq list */
> for (lvl = 0; lvl < NIX_TXSCH_LVL_CNT; lvl++) {
> for (schq = 0; schq < MAX_TXSCHQ_PER_FUNC; schq++)
> pfvf->hw.txschq_list[lvl][schq] = 0;
> }
> - return err;
> +
nit: no blank line here.
> }
>
> void otx2_sqb_flush(struct otx2_nic *pfvf)
...
