DPDK patches and discussions
 help / color / Atom feed
* Re: [dpdk-dev] [PATCH] net/i40e: fix unchecked return value
  2020-02-11 19:02 [dpdk-dev] [PATCH] net/i40e: fix unchecked return value Beilei Xing
@ 2020-02-11 11:42 ` Bruce Richardson
  2020-02-12  2:33   ` Xing, Beilei
  2020-02-12 12:36 ` [dpdk-dev] [PATCH v2] " Beilei Xing
  1 sibling, 1 reply; 5+ messages in thread
From: Bruce Richardson @ 2020-02-11 11:42 UTC (permalink / raw)
  To: Beilei Xing; +Cc: dev, qi.z.zhang

On Wed, Feb 12, 2020 at 03:02:00AM +0800, Beilei Xing wrote:
> Check the return value of the i40e_xmit_cleanup function.
> 
> Coverity issue: 353617
> Fixes: 4861cde46116 ("i40e: new poll mode driver")
> 
> Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> ---
>  drivers/net/i40e/i40e_rxtx.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> index fd1ae80..f43fc0f 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -1038,8 +1038,9 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
>  	txe = &sw_ring[tx_id];
>  
>  	/* Check if the descriptor ring needs to be cleaned. */
> -	if (txq->nb_tx_free < txq->tx_free_thresh)
> -		i40e_xmit_cleanup(txq);
> +	if ((txq->nb_tx_free < txq->tx_free_thresh) &&
> +	    (i40e_xmit_cleanup(txq) != 0))
> +		return 0;
>  

I don't think this should be fixed, and the original code is correct.

This cleanup is opportunistic and may not cause problems if it fails. For
example, if tx_free_thresh is 32, and nb_tx_free is 24, there is no reason
to return zero here if the total packets to be sent it 16 - since all
packets can feasibly fit. Even if we had 32 to transmit, we still should
not quit here, since any packets that can be transmitted should be sent,
and there is a subsequent cleanup call at line 1084 to handle failed
cleanup when it does become a problem.

Regards,
/Bruce

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dpdk-dev] [PATCH] net/i40e: fix unchecked return value
@ 2020-02-11 19:02 Beilei Xing
  2020-02-11 11:42 ` Bruce Richardson
  2020-02-12 12:36 ` [dpdk-dev] [PATCH v2] " Beilei Xing
  0 siblings, 2 replies; 5+ messages in thread
From: Beilei Xing @ 2020-02-11 19:02 UTC (permalink / raw)
  To: dev, qi.z.zhang

Check the return value of the i40e_xmit_cleanup function.

Coverity issue: 353617
Fixes: 4861cde46116 ("i40e: new poll mode driver")

Signed-off-by: Beilei Xing <beilei.xing@intel.com>
---
 drivers/net/i40e/i40e_rxtx.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index fd1ae80..f43fc0f 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1038,8 +1038,9 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	txe = &sw_ring[tx_id];
 
 	/* Check if the descriptor ring needs to be cleaned. */
-	if (txq->nb_tx_free < txq->tx_free_thresh)
-		i40e_xmit_cleanup(txq);
+	if ((txq->nb_tx_free < txq->tx_free_thresh) &&
+	    (i40e_xmit_cleanup(txq) != 0))
+		return 0;
 
 	for (nb_tx = 0; nb_tx < nb_pkts; nb_tx++) {
 		td_cmd = 0;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH] net/i40e: fix unchecked return value
  2020-02-11 11:42 ` Bruce Richardson
@ 2020-02-12  2:33   ` Xing, Beilei
  0 siblings, 0 replies; 5+ messages in thread
From: Xing, Beilei @ 2020-02-12  2:33 UTC (permalink / raw)
  To: Richardson, Bruce; +Cc: dev, Zhang, Qi Z

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Tuesday, February 11, 2020 7:42 PM
> To: Xing, Beilei <beilei.xing@intel.com>
> Cc: dev@dpdk.org; Zhang, Qi Z <qi.z.zhang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] net/i40e: fix unchecked return value
> 
> On Wed, Feb 12, 2020 at 03:02:00AM +0800, Beilei Xing wrote:
> > Check the return value of the i40e_xmit_cleanup function.
> >
> > Coverity issue: 353617
> > Fixes: 4861cde46116 ("i40e: new poll mode driver")
> >
> > Signed-off-by: Beilei Xing <beilei.xing@intel.com>
> > ---
> >  drivers/net/i40e/i40e_rxtx.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_rxtx.c
> > b/drivers/net/i40e/i40e_rxtx.c index fd1ae80..f43fc0f 100644
> > --- a/drivers/net/i40e/i40e_rxtx.c
> > +++ b/drivers/net/i40e/i40e_rxtx.c
> > @@ -1038,8 +1038,9 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf
> **tx_pkts, uint16_t nb_pkts)
> >  	txe = &sw_ring[tx_id];
> >
> >  	/* Check if the descriptor ring needs to be cleaned. */
> > -	if (txq->nb_tx_free < txq->tx_free_thresh)
> > -		i40e_xmit_cleanup(txq);
> > +	if ((txq->nb_tx_free < txq->tx_free_thresh) &&
> > +	    (i40e_xmit_cleanup(txq) != 0))
> > +		return 0;
> >
> 
> I don't think this should be fixed, and the original code is correct.
> 
> This cleanup is opportunistic and may not cause problems if it fails. For
> example, if tx_free_thresh is 32, and nb_tx_free is 24, there is no reason to
> return zero here if the total packets to be sent it 16 - since all packets can
> feasibly fit. Even if we had 32 to transmit, we still should not quit here, since
> any packets that can be transmitted should be sent, and there is a
> subsequent cleanup call at line 1084 to handle failed cleanup when it does
> become a problem.

Yes, agree. Will abandon the patch. Thanks.

> 
> Regards,
> /Bruce

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [dpdk-dev] [PATCH v2] net/i40e: fix unchecked return value
  2020-02-11 19:02 [dpdk-dev] [PATCH] net/i40e: fix unchecked return value Beilei Xing
  2020-02-11 11:42 ` Bruce Richardson
@ 2020-02-12 12:36 ` " Beilei Xing
  2020-02-14  9:45   ` Ye Xiaolong
  1 sibling, 1 reply; 5+ messages in thread
From: Beilei Xing @ 2020-02-12 12:36 UTC (permalink / raw)
  To: dev, qi.z.zhang

This patch fixes unchecked return value
of the i40e_xmit_cleanup function.

Coverity issue: 353617
Fixes: 4861cde46116 ("i40e: new poll mode driver")

Signed-off-by: Beilei Xing <beilei.xing@intel.com>
---
V2 change:
 - Use cast.

 drivers/net/i40e/i40e_rxtx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index fd1ae80..f6d23c9 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -1039,7 +1039,7 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 
 	/* Check if the descriptor ring needs to be cleaned. */
 	if (txq->nb_tx_free < txq->tx_free_thresh)
-		i40e_xmit_cleanup(txq);
+		(void)i40e_xmit_cleanup(txq);
 
 	for (nb_tx = 0; nb_tx < nb_pkts; nb_tx++) {
 		td_cmd = 0;
-- 
2.7.4


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [dpdk-dev] [PATCH v2] net/i40e: fix unchecked return value
  2020-02-12 12:36 ` [dpdk-dev] [PATCH v2] " Beilei Xing
@ 2020-02-14  9:45   ` Ye Xiaolong
  0 siblings, 0 replies; 5+ messages in thread
From: Ye Xiaolong @ 2020-02-14  9:45 UTC (permalink / raw)
  To: Beilei Xing; +Cc: dev, qi.z.zhang

On 02/12, Beilei Xing wrote:
>This patch fixes unchecked return value
>of the i40e_xmit_cleanup function.
>
>Coverity issue: 353617
>Fixes: 4861cde46116 ("i40e: new poll mode driver")
>
>Signed-off-by: Beilei Xing <beilei.xing@intel.com>
>---
>V2 change:
> - Use cast.
>
> drivers/net/i40e/i40e_rxtx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
>index fd1ae80..f6d23c9 100644
>--- a/drivers/net/i40e/i40e_rxtx.c
>+++ b/drivers/net/i40e/i40e_rxtx.c
>@@ -1039,7 +1039,7 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
> 
> 	/* Check if the descriptor ring needs to be cleaned. */
> 	if (txq->nb_tx_free < txq->tx_free_thresh)
>-		i40e_xmit_cleanup(txq);
>+		(void)i40e_xmit_cleanup(txq);
> 
> 	for (nb_tx = 0; nb_tx < nb_pkts; nb_tx++) {
> 		td_cmd = 0;
>-- 
>2.7.4
>

Acked-by: Xiaolong Ye <xiaolong.ye@intel.com>

Applied to dpdk-next-net-intel, Thanks.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-11 19:02 [dpdk-dev] [PATCH] net/i40e: fix unchecked return value Beilei Xing
2020-02-11 11:42 ` Bruce Richardson
2020-02-12  2:33   ` Xing, Beilei
2020-02-12 12:36 ` [dpdk-dev] [PATCH v2] " Beilei Xing
2020-02-14  9:45   ` Ye Xiaolong

DPDK patches and discussions

Archives are clonable:
	git clone --mirror http://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ http://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev


Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox