Re: [PATCH net] net: mv643xx_eth: implement descriptor cleanup in txq_submit_tso

[Paolo Abeni]

On 1/24/25 7:24 AM, Dheeraj Reddy Jonnalagadda wrote:
> Implement cleanup of used descriptors in the error path of txq_submit_tso.
> 
> Fixes: 3ae8f4e0b98b ("net: mv643xx_eth: Implement software TSO")
> Signed-off-by: Dheeraj Reddy Jonnalagadda <dheeraj.linuxdev@xxxxxxxxx>
> ---
>  drivers/net/ethernet/marvell/mv643xx_eth.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mv643xx_eth.c b/drivers/net/ethernet/marvell/mv643xx_eth.c
> index 67a6ff07c83d..8d217f8d451e 100644
> --- a/drivers/net/ethernet/marvell/mv643xx_eth.c
> +++ b/drivers/net/ethernet/marvell/mv643xx_eth.c
> @@ -881,10 +881,20 @@ static int txq_submit_tso(struct tx_queue *txq, struct sk_buff *skb,
>  	txq_enable(txq);
>  	txq->tx_desc_count += desc_count;
>  	return 0;
> +
>  err_release:
> -	/* TODO: Release all used data descriptors; header descriptors must not
> +	/* Release all used data descriptors; header descriptors must not
>  	 * be DMA-unmapped.
>  	 */
> +	for (int i = 0; i < desc_count; i++) {

Please move the 'i' variable definition into the initial definition for
this function.

> +		int desc_index = (txq->tx_curr_desc + i) % txq->tx_ring_size;

AFAICS we reach here when the first `desc_count - 1` descriptor for the
current TSO packet has been filled and filling the `desc_count`th one,
in the `txq->tx_curr_desc` index failed.

Given the above, you should free the previous `desc_count - 1`
descriptors, i.e. txq->tx_curr_desc - i.

> +		struct tx_desc *desc = &txq->tx_desc_area[desc_index];
> +		desc->cmd_sts = 0; /* Reset the descriptor */

It looks like you should still unmap data descriptor as done in
txq_reclaim()

/P