[PATCH] net/nfp: fix the Tx descriptor free logic of nfd3

[PATCH] net/nfp: fix the Tx descriptor free logic of nfd3

[Chaoyong He]

In the tx descriptor free logic of nfd3, the former logic might force
cast a negative number into a very big unsigned number, and which will
cause potential problem in the xmit loop.

The xmit loop will continue in the place where it should break, and will
overwrite the Tx descriptor which is not free to use by the PMD.

Fixes: 74a640dac864 ("net/nfp: avoid modulo operations for handling ring wrapping")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
 drivers/net/nfp/nfp_rxtx.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfp_rxtx.h b/drivers/net/nfp/nfp_rxtx.h
index 5e651518ed..e642bc970a 100644
--- a/drivers/net/nfp/nfp_rxtx.h
+++ b/drivers/net/nfp/nfp_rxtx.h
@@ -401,10 +401,14 @@ nfp_net_mbuf_alloc_failed(struct nfp_net_rxq *rxq)
 static inline uint32_t
 nfp_net_nfd3_free_tx_desc(struct nfp_net_txq *txq)
 {
+	uint32_t free_desc;
+
 	if (txq->wr_p >= txq->rd_p)
-		return txq->tx_count - (txq->wr_p - txq->rd_p) - 8;
+		free_desc = txq->tx_count - (txq->wr_p - txq->rd_p);
 	else
-		return txq->rd_p - txq->wr_p - 8;
+		free_desc = txq->rd_p - txq->wr_p;
+
+	return (free_desc > 8) ? (free_desc - 8) : 0;
 }
 
 /*
-- 
2.39.1

Re: [PATCH] net/nfp: fix the Tx descriptor free logic of nfd3

[Ferruh Yigit]

On 5/12/2023 3:07 AM, Chaoyong He wrote:
> In the tx descriptor free logic of nfd3, the former logic might force
> cast a negative number into a very big unsigned number, and which will
> cause potential problem in the xmit loop.
> 
> The xmit loop will continue in the place where it should break, and will
> overwrite the Tx descriptor which is not free to use by the PMD.
> 
> Fixes: 74a640dac864 ("net/nfp: avoid modulo operations for handling ring wrapping")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> ---
>  drivers/net/nfp/nfp_rxtx.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/nfp/nfp_rxtx.h b/drivers/net/nfp/nfp_rxtx.h
> index 5e651518ed..e642bc970a 100644
> --- a/drivers/net/nfp/nfp_rxtx.h
> +++ b/drivers/net/nfp/nfp_rxtx.h
> @@ -401,10 +401,14 @@ nfp_net_mbuf_alloc_failed(struct nfp_net_rxq *rxq)
>  static inline uint32_t
>  nfp_net_nfd3_free_tx_desc(struct nfp_net_txq *txq)
>  {
> +	uint32_t free_desc;
> +
>  	if (txq->wr_p >= txq->rd_p)
> -		return txq->tx_count - (txq->wr_p - txq->rd_p) - 8;
> +		free_desc = txq->tx_count - (txq->wr_p - txq->rd_p);
>  	else
> -		return txq->rd_p - txq->wr_p - 8;
> +		free_desc = txq->rd_p - txq->wr_p;
> +
> +	return (free_desc > 8) ? (free_desc - 8) : 0;
>  }
>  
>  /*


Can you please rebase this patch on top of latest next-net?

'nfp_net_nfd3_free_tx_desc()' moved to 'drivers/net/nfp/nfd3/nfp_nfd3.h'
in commit '2344d6272fe9 ("net/nfp: move NFD3 logic to own source file")'

RE: [PATCH] net/nfp: fix the Tx descriptor free logic of nfd3

[Chaoyong He]

> On 5/12/2023 3:07 AM, Chaoyong He wrote:
> > In the tx descriptor free logic of nfd3, the former logic might force
> > cast a negative number into a very big unsigned number, and which will
> > cause potential problem in the xmit loop.
> >
> > The xmit loop will continue in the place where it should break, and
> > will overwrite the Tx descriptor which is not free to use by the PMD.
> >
> > Fixes: 74a640dac864 ("net/nfp: avoid modulo operations for handling
> > ring wrapping")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> > Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
> > ---
> >  drivers/net/nfp/nfp_rxtx.h | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/nfp/nfp_rxtx.h b/drivers/net/nfp/nfp_rxtx.h
> > index 5e651518ed..e642bc970a 100644
> > --- a/drivers/net/nfp/nfp_rxtx.h
> > +++ b/drivers/net/nfp/nfp_rxtx.h
> > @@ -401,10 +401,14 @@ nfp_net_mbuf_alloc_failed(struct nfp_net_rxq
> > *rxq)  static inline uint32_t  nfp_net_nfd3_free_tx_desc(struct
> > nfp_net_txq *txq)  {
> > +	uint32_t free_desc;
> > +
> >  	if (txq->wr_p >= txq->rd_p)
> > -		return txq->tx_count - (txq->wr_p - txq->rd_p) - 8;
> > +		free_desc = txq->tx_count - (txq->wr_p - txq->rd_p);
> >  	else
> > -		return txq->rd_p - txq->wr_p - 8;
> > +		free_desc = txq->rd_p - txq->wr_p;
> > +
> > +	return (free_desc > 8) ? (free_desc - 8) : 0;
> >  }
> >
> >  /*
> 
> 
> Can you please rebase this patch on top of latest next-net?
> 
> 'nfp_net_nfd3_free_tx_desc()' moved to 'drivers/net/nfp/nfd3/nfp_nfd3.h'
> in commit '2344d6272fe9 ("net/nfp: move NFD3 logic to own source file")'

Okay, I'll send out a v2 patch soon.

[PATCH v2] net/nfp: fix the Tx descriptor free logic of nfd3

[Chaoyong He]

In the Tx descriptor free logic of nfd3, the former logic might force
cast a negative number into a very big unsigned number, and which will
cause potential problem in the xmit loop.

The xmit loop will continue in the place where it should break, and will
overwrite the Tx descriptor which is not free to use by the PMD.

Fixes: 74a640dac864 ("net/nfp: avoid modulo operations for handling ring wrapping")
Cc: stable@dpdk.org

Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
---
v2:
* Rebase to the latest main branch.
---
 drivers/net/nfp/nfd3/nfp_nfd3.h | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/nfp/nfd3/nfp_nfd3.h b/drivers/net/nfp/nfd3/nfp_nfd3.h
index 7bf2349904..e772bc4711 100644
--- a/drivers/net/nfp/nfd3/nfp_nfd3.h
+++ b/drivers/net/nfp/nfd3/nfp_nfd3.h
@@ -52,10 +52,14 @@ struct nfp_net_nfd3_tx_desc {
 static inline uint32_t
 nfp_net_nfd3_free_tx_desc(struct nfp_net_txq *txq)
 {
+	uint32_t free_desc;
+
 	if (txq->wr_p >= txq->rd_p)
-		return txq->tx_count - (txq->wr_p - txq->rd_p) - 8;
+		free_desc = txq->tx_count - (txq->wr_p - txq->rd_p);
 	else
-		return txq->rd_p - txq->wr_p - 8;
+		free_desc = txq->rd_p - txq->wr_p;
+
+	return (free_desc > 8) ? (free_desc - 8) : 0;
 }
 
 /*
-- 
2.39.1

Re: [PATCH v2] net/nfp: fix the Tx descriptor free logic of nfd3

[Ferruh Yigit]

On 5/18/2023 12:02 PM, Chaoyong He wrote:
> In the Tx descriptor free logic of nfd3, the former logic might force
> cast a negative number into a very big unsigned number, and which will
> cause potential problem in the xmit loop.
> 
> The xmit loop will continue in the place where it should break, and will
> overwrite the Tx descriptor which is not free to use by the PMD.
> 
> Fixes: 74a640dac864 ("net/nfp: avoid modulo operations for handling ring wrapping")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chaoyong He <chaoyong.he@corigine.com>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@corigine.com>
>

Applied to dpdk-next-net/main, thanks.