All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] net/i40e: mbuf alloc failed counter not incremented
@ 2017-03-27 16:30 Allain Legacy
  2017-03-28  8:52 ` Zhang, Helin
  2017-04-21 23:13 ` [PATCH v2] " Allain Legacy
  0 siblings, 2 replies; 10+ messages in thread
From: Allain Legacy @ 2017-03-27 16:30 UTC (permalink / raw)
  To: helin.zhang, jingjing.wu; +Cc: dev, matt.peters

From: Matt Peters <matt.peters@windriver.com>

When an mbuf alloc fails during the mempool get operation for the
i40e bulk alloc receive function, the rx_mbuf_alloc_failed counter
is not incremented to record the error.

This fix ensures consistency with the other i40e receive procedures and
other net drivers.

Signed-off-by: Matt Peters <matt.peters@windriver.com>
Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
---
 drivers/net/i40e/i40e_rxtx.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index ec64a20eb..794d8717e 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -610,6 +610,7 @@ static inline uint16_t
 rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 {
 	struct i40e_rx_queue *rxq = (struct i40e_rx_queue *)rx_queue;
+	struct rte_eth_dev *dev;
 	uint16_t nb_rx = 0;
 
 	if (!nb_pkts)
@@ -630,6 +631,10 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 			PMD_RX_LOG(DEBUG, "Rx mbuf alloc failed for "
 				   "port_id=%u, queue_id=%u",
 				   rxq->port_id, rxq->queue_id);
+
+			dev = &rte_eth_devices[rxq->port_id];
+			dev->data->rx_mbuf_alloc_failed += rxq->rx_free_thresh;
+
 			rxq->rx_nb_avail = 0;
 			rxq->rx_tail = (uint16_t)(rxq->rx_tail - nb_rx);
 			for (i = 0, j = rxq->rx_tail; i < nb_rx; i++, j++)
-- 
2.12.1

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

* Re: [PATCH] net/i40e: mbuf alloc failed counter not incremented
  2017-03-27 16:30 [PATCH] net/i40e: mbuf alloc failed counter not incremented Allain Legacy
@ 2017-03-28  8:52 ` Zhang, Helin
  2017-03-29 22:42   ` Legacy, Allain
  2017-04-21 23:13 ` [PATCH v2] " Allain Legacy
  1 sibling, 1 reply; 10+ messages in thread
From: Zhang, Helin @ 2017-03-28  8:52 UTC (permalink / raw)
  To: Legacy, Allain (Wind River), Wu, Jingjing; +Cc: dev, Peters, Matt (Wind River)



> -----Original Message-----
> From: Allain Legacy [mailto:allain.legacy@windriver.com]
> Sent: Tuesday, March 28, 2017 12:31 AM
> To: Zhang, Helin; Wu, Jingjing
> Cc: dev@dpdk.org; Peters, Matt (Wind River)
> Subject: [PATCH] net/i40e: mbuf alloc failed counter not incremented
> 
> From: Matt Peters <matt.peters@windriver.com>
> 
> When an mbuf alloc fails during the mempool get operation for the i40e bulk
> alloc receive function, the rx_mbuf_alloc_failed counter is not incremented to
> record the error.
> 
> This fix ensures consistency with the other i40e receive procedures and other
> net drivers.
> 
> Signed-off-by: Matt Peters <matt.peters@windriver.com>
> Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
> ---
>  drivers/net/i40e/i40e_rxtx.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index
> ec64a20eb..794d8717e 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -610,6 +610,7 @@ static inline uint16_t  rx_recv_pkts(void *rx_queue,
> struct rte_mbuf **rx_pkts, uint16_t nb_pkts)  {
>  	struct i40e_rx_queue *rxq = (struct i40e_rx_queue *)rx_queue;
> +	struct rte_eth_dev *dev;
>  	uint16_t nb_rx = 0;
> 
>  	if (!nb_pkts)
> @@ -630,6 +631,10 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts, uint16_t nb_pkts)
>  			PMD_RX_LOG(DEBUG, "Rx mbuf alloc failed for "
>  				   "port_id=%u, queue_id=%u",
>  				   rxq->port_id, rxq->queue_id);
> +
> +			dev = &rte_eth_devices[rxq->port_id];
> +			dev->data->rx_mbuf_alloc_failed += rxq-
> >rx_free_thresh;
> +
I think there are more places to have the similar code. Would you help to complete all?
e.g. rte_mbuf_raw_alloc() in i40e_recv_pkts.

Thank you very much for the helps!
/Helin

>  			rxq->rx_nb_avail = 0;
>  			rxq->rx_tail = (uint16_t)(rxq->rx_tail - nb_rx);
>  			for (i = 0, j = rxq->rx_tail; i < nb_rx; i++, j++)
> --
> 2.12.1

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

* Re: [PATCH] net/i40e: mbuf alloc failed counter not incremented
  2017-03-28  8:52 ` Zhang, Helin
@ 2017-03-29 22:42   ` Legacy, Allain
  0 siblings, 0 replies; 10+ messages in thread
From: Legacy, Allain @ 2017-03-29 22:42 UTC (permalink / raw)
  To: ZHANG, HELIN, WU, JINGJING; +Cc: dev, Peters, Matt

> -----Original Message-----
> From: Zhang, Helin [mailto:helin.zhang@intel.com]
> Sent: Tuesday, March 28, 2017 4:52 AM

<...>
> > +
> > +			dev = &rte_eth_devices[rxq->port_id];
> > +			dev->data->rx_mbuf_alloc_failed += rxq-
> > >rx_free_thresh;
> > +
> I think there are more places to have the similar code. Would you help to
> complete all?

Sure, we'll submit a v2 in the near future.

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

* [PATCH v2] net/i40e: mbuf alloc failed counter not incremented
  2017-03-27 16:30 [PATCH] net/i40e: mbuf alloc failed counter not incremented Allain Legacy
  2017-03-28  8:52 ` Zhang, Helin
@ 2017-04-21 23:13 ` Allain Legacy
  2017-04-24  5:46   ` Ferruh Yigit
  2017-04-25 12:28   ` [PATCH v3] " Allain Legacy
  1 sibling, 2 replies; 10+ messages in thread
From: Allain Legacy @ 2017-04-21 23:13 UTC (permalink / raw)
  To: helin.zhang, jingjing.wu; +Cc: dev, matt.peters

From: Matt Peters <matt.peters@windriver.com>

When an mbuf alloc fails during the mempool get operation for the
i40e bulk alloc receive function, the rx_mbuf_alloc_failed counter
is not incremented to record the error.

This fix ensures consistency with the other i40e receive procedures and
other net drivers.

Signed-off-by: Matt Peters <matt.peters@windriver.com>
Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
---
 drivers/net/i40e/i40e_rxtx.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index e5471b143..4131902a9 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -610,6 +610,7 @@ static inline uint16_t
 rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 {
 	struct i40e_rx_queue *rxq = (struct i40e_rx_queue *)rx_queue;
+	struct rte_eth_dev *dev;
 	uint16_t nb_rx = 0;
 
 	if (!nb_pkts)
@@ -630,6 +631,11 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 			PMD_RX_LOG(DEBUG, "Rx mbuf alloc failed for "
 				   "port_id=%u, queue_id=%u",
 				   rxq->port_id, rxq->queue_id);
+
+			dev = &rte_eth_devices[rxq->port_id];
+			dev->data->rx_mbuf_alloc_failed +=
+				rxq->rx_free_thresh;
+
 			rxq->rx_nb_avail = 0;
 			rxq->rx_tail = (uint16_t)(rxq->rx_tail - nb_rx);
 			for (i = 0, j = rxq->rx_tail; i < nb_rx; i++, j++)
@@ -691,6 +697,7 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	union i40e_rx_desc rxd;
 	struct i40e_rx_entry *sw_ring;
 	struct i40e_rx_entry *rxe;
+	struct rte_eth_dev *dev;
 	struct rte_mbuf *rxm;
 	struct rte_mbuf *nmb;
 	uint16_t nb_rx;
@@ -721,10 +728,16 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 			break;
 
 		nmb = rte_mbuf_raw_alloc(rxq->mp);
-		if (unlikely(!nmb))
+		if (unlikely(!nmb)) {
+			PMD_RX_LOG(DEBUG, "RX mbuf alloc failed port_id=%u "
+				   "queue_id=%u", (unsigned int)rxq->port_id,
+				   (unsigned int)rxq->queue_id);
+			dev = &rte_eth_devices[rxq->port_id];
+			dev->data->rx_mbuf_alloc_failed++;
 			break;
-		rxd = *rxdp;
+		}
 
+		rxd = *rxdp;
 		nb_hold++;
 		rxe = &sw_ring[rx_id];
 		rx_id++;
@@ -816,6 +829,7 @@ i40e_recv_scattered_pkts(void *rx_queue,
 	struct rte_mbuf *nmb, *rxm;
 	uint16_t rx_id = rxq->rx_tail;
 	uint16_t nb_rx = 0, nb_hold = 0, rx_packet_len;
+	struct rte_eth_dev *dev;
 	uint32_t rx_status;
 	uint64_t qword1;
 	uint64_t dma_addr;
@@ -833,8 +847,15 @@ i40e_recv_scattered_pkts(void *rx_queue,
 			break;
 
 		nmb = rte_mbuf_raw_alloc(rxq->mp);
-		if (unlikely(!nmb))
+		if (unlikely(!nmb)) {
+			PMD_RX_LOG(DEBUG, "RX mbuf alloc failed port_id=%u "
+				   "queue_id=%u", (unsigned int)rxq->port_id,
+				   (unsigned int)rxq->queue_id);
+			dev = &rte_eth_devices[rxq->port_id];
+			dev->data->rx_mbuf_alloc_failed++;
 			break;
+		}
+
 		rxd = *rxdp;
 		nb_hold++;
 		rxe = &sw_ring[rx_id];
-- 
2.12.1

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

* Re: [PATCH v2] net/i40e: mbuf alloc failed counter not incremented
  2017-04-21 23:13 ` [PATCH v2] " Allain Legacy
@ 2017-04-24  5:46   ` Ferruh Yigit
  2017-04-24 11:21     ` Legacy, Allain
  2017-04-25 11:01     ` Zhang, Helin
  2017-04-25 12:28   ` [PATCH v3] " Allain Legacy
  1 sibling, 2 replies; 10+ messages in thread
From: Ferruh Yigit @ 2017-04-24  5:46 UTC (permalink / raw)
  To: Allain Legacy, helin.zhang, jingjing.wu; +Cc: dev, matt.peters

On 4/22/2017 12:13 AM, Allain Legacy wrote:
> From: Matt Peters <matt.peters@windriver.com>
> 
> When an mbuf alloc fails during the mempool get operation for the
> i40e bulk alloc receive function, the rx_mbuf_alloc_failed counter
> is not incremented to record the error.
> 
> This fix ensures consistency with the other i40e receive procedures and
> other net drivers.
> 
> Signed-off-by: Matt Peters <matt.peters@windriver.com>
> Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
> ---
>  drivers/net/i40e/i40e_rxtx.c | 27 ++++++++++++++++++++++++---
>  1 file changed, 24 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> index e5471b143..4131902a9 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -610,6 +610,7 @@ static inline uint16_t
>  rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>  {
>  	struct i40e_rx_queue *rxq = (struct i40e_rx_queue *)rx_queue;
> +	struct rte_eth_dev *dev;
>  	uint16_t nb_rx = 0;
>  
>  	if (!nb_pkts)
> @@ -630,6 +631,11 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>  			PMD_RX_LOG(DEBUG, "Rx mbuf alloc failed for "
>  				   "port_id=%u, queue_id=%u",
>  				   rxq->port_id, rxq->queue_id);
> +
> +			dev = &rte_eth_devices[rxq->port_id];

The question is mostly to the driver maintainer, instead of using global
variable, would it be better to use rxq->vsi->adapter->eth_dev to access
rte_eth_device struct?

> +			dev->data->rx_mbuf_alloc_failed +=
> +				rxq->rx_free_thresh;
> +
>  			rxq->rx_nb_avail = 0;
>  			rxq->rx_tail = (uint16_t)(rxq->rx_tail - nb_rx);
>  			for (i = 0, j = rxq->rx_tail; i < nb_rx; i++, j++)
> @@ -691,6 +697,7 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>  	union i40e_rx_desc rxd;
>  	struct i40e_rx_entry *sw_ring;
>  	struct i40e_rx_entry *rxe;
> +	struct rte_eth_dev *dev;
>  	struct rte_mbuf *rxm;
>  	struct rte_mbuf *nmb;
>  	uint16_t nb_rx;
> @@ -721,10 +728,16 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
>  			break;
>  
>  		nmb = rte_mbuf_raw_alloc(rxq->mp);
> -		if (unlikely(!nmb))
> +		if (unlikely(!nmb)) {
> +			PMD_RX_LOG(DEBUG, "RX mbuf alloc failed port_id=%u "
> +				   "queue_id=%u", (unsigned int)rxq->port_id,
> +				   (unsigned int)rxq->queue_id);

Do we really want debug print here?
When you think the speeds we are dealing with, if mbuf alloc starts
failing we may hit this lines millions per second, which may make app
unusable?

> +			dev = &rte_eth_devices[rxq->port_id];
> +			dev->data->rx_mbuf_alloc_failed++;
>  			break;
> -		rxd = *rxdp;
> +		}
>  
> +		rxd = *rxdp;
>  		nb_hold++;
>  		rxe = &sw_ring[rx_id];
>  		rx_id++;
> @@ -816,6 +829,7 @@ i40e_recv_scattered_pkts(void *rx_queue,
>  	struct rte_mbuf *nmb, *rxm;
>  	uint16_t rx_id = rxq->rx_tail;
>  	uint16_t nb_rx = 0, nb_hold = 0, rx_packet_len;
> +	struct rte_eth_dev *dev;
>  	uint32_t rx_status;
>  	uint64_t qword1;
>  	uint64_t dma_addr;
> @@ -833,8 +847,15 @@ i40e_recv_scattered_pkts(void *rx_queue,
>  			break;
>  
>  		nmb = rte_mbuf_raw_alloc(rxq->mp);
> -		if (unlikely(!nmb))
> +		if (unlikely(!nmb)) {
> +			PMD_RX_LOG(DEBUG, "RX mbuf alloc failed port_id=%u "
> +				   "queue_id=%u", (unsigned int)rxq->port_id,
> +				   (unsigned int)rxq->queue_id);
> +			dev = &rte_eth_devices[rxq->port_id];
> +			dev->data->rx_mbuf_alloc_failed++;
>  			break;
> +		}
> +
>  		rxd = *rxdp;
>  		nb_hold++;
>  		rxe = &sw_ring[rx_id];
> 

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

* Re: [PATCH v2] net/i40e: mbuf alloc failed counter not incremented
  2017-04-24  5:46   ` Ferruh Yigit
@ 2017-04-24 11:21     ` Legacy, Allain
  2017-04-25 11:01     ` Zhang, Helin
  1 sibling, 0 replies; 10+ messages in thread
From: Legacy, Allain @ 2017-04-24 11:21 UTC (permalink / raw)
  To: YIGIT, FERRUH, ZHANG, HELIN, WU, JINGJING; +Cc: dev, Peters, Matt

> -----Original Message-----
> From: Ferruh Yigit [mailto:ferruh.yigit@intel.com]
> Sent: Monday, April 24, 2017 1:47 AM
<...>
> >  		nmb = rte_mbuf_raw_alloc(rxq->mp);
> > -		if (unlikely(!nmb))
> > +		if (unlikely(!nmb)) {
> > +			PMD_RX_LOG(DEBUG, "RX mbuf alloc failed
> port_id=%u "
> > +				   "queue_id=%u", (unsigned int)rxq-
> >port_id,
> > +				   (unsigned int)rxq->queue_id);
> 
> Do we really want debug print here?
> When you think the speeds we are dealing with, if mbuf alloc starts failing we
> may hit this lines millions per second, which may make app unusable?
> 
Sure, but isn't that the purpose of the PMD_RX_LOG/PMD_TX_LOG (i.e., to make it possible to log events in the rx/tx path when that becomes necessary)?  Presumably, this would be a debug option only when necessary during development and not used to debug an issue live while the application needs to be useable/responsive. 




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

* Re: [PATCH v2] net/i40e: mbuf alloc failed counter not incremented
  2017-04-24  5:46   ` Ferruh Yigit
  2017-04-24 11:21     ` Legacy, Allain
@ 2017-04-25 11:01     ` Zhang, Helin
  1 sibling, 0 replies; 10+ messages in thread
From: Zhang, Helin @ 2017-04-25 11:01 UTC (permalink / raw)
  To: Yigit, Ferruh, Legacy, Allain (Wind River), Wu, Jingjing
  Cc: dev, Peters, Matt (Wind River)



> -----Original Message-----
> From: Yigit, Ferruh
> Sent: Monday, April 24, 2017 1:47 PM
> To: Legacy, Allain (Wind River); Zhang, Helin; Wu, Jingjing
> Cc: dev@dpdk.org; Peters, Matt (Wind River)
> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: mbuf alloc failed counter not
> incremented
Actually it is a fix, right? I think you need to start the title with 'fix'?
Then don't miss the 'Fix' line, as required.
> 
> On 4/22/2017 12:13 AM, Allain Legacy wrote:
> > From: Matt Peters <matt.peters@windriver.com>
> >
> > When an mbuf alloc fails during the mempool get operation for the i40e
> > bulk alloc receive function, the rx_mbuf_alloc_failed counter is not
> > incremented to record the error.
> >
> > This fix ensures consistency with the other i40e receive procedures
> > and other net drivers.
> >
> > Signed-off-by: Matt Peters <matt.peters@windriver.com>
> > Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
> > ---
> >  drivers/net/i40e/i40e_rxtx.c | 27 ++++++++++++++++++++++++---
> >  1 file changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/i40e/i40e_rxtx.c
> > b/drivers/net/i40e/i40e_rxtx.c index e5471b143..4131902a9 100644
> > --- a/drivers/net/i40e/i40e_rxtx.c
> > +++ b/drivers/net/i40e/i40e_rxtx.c
> > @@ -610,6 +610,7 @@ static inline uint16_t  rx_recv_pkts(void
> > *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)  {
> >  	struct i40e_rx_queue *rxq = (struct i40e_rx_queue *)rx_queue;
> > +	struct rte_eth_dev *dev;
> >  	uint16_t nb_rx = 0;
> >
> >  	if (!nb_pkts)
> > @@ -630,6 +631,11 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts, uint16_t nb_pkts)
> >  			PMD_RX_LOG(DEBUG, "Rx mbuf alloc failed for "
> >  				   "port_id=%u, queue_id=%u",
> >  				   rxq->port_id, rxq->queue_id);
> > +
> > +			dev = &rte_eth_devices[rxq->port_id];
> 
> The question is mostly to the driver maintainer, instead of using global variable,
> would it be better to use rxq->vsi->adapter->eth_dev to access rte_eth_device
> struct?
Yes, I agree with Ferruh. I40E_VSI_TO_ETH_DEV(rxq->vsi) can be used here.
> 
> > +			dev->data->rx_mbuf_alloc_failed +=
> > +				rxq->rx_free_thresh;
> > +
> >  			rxq->rx_nb_avail = 0;
> >  			rxq->rx_tail = (uint16_t)(rxq->rx_tail - nb_rx);
> >  			for (i = 0, j = rxq->rx_tail; i < nb_rx; i++, j++) @@ -691,6
> > +697,7 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> uint16_t nb_pkts)
> >  	union i40e_rx_desc rxd;
> >  	struct i40e_rx_entry *sw_ring;
> >  	struct i40e_rx_entry *rxe;
> > +	struct rte_eth_dev *dev;
> >  	struct rte_mbuf *rxm;
> >  	struct rte_mbuf *nmb;
> >  	uint16_t nb_rx;
> > @@ -721,10 +728,16 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts, uint16_t nb_pkts)
> >  			break;
> >
> >  		nmb = rte_mbuf_raw_alloc(rxq->mp);
> > -		if (unlikely(!nmb))
> > +		if (unlikely(!nmb)) {
> > +			PMD_RX_LOG(DEBUG, "RX mbuf alloc failed
> port_id=%u "
> > +				   "queue_id=%u", (unsigned int)rxq->port_id,
> > +				   (unsigned int)rxq->queue_id);
> 
> Do we really want debug print here?
> When you think the speeds we are dealing with, if mbuf alloc starts failing we
> may hit this lines millions per second, which may make app unusable?
> 
> > +			dev = &rte_eth_devices[rxq->port_id];
> > +			dev->data->rx_mbuf_alloc_failed++;
> >  			break;
> > -		rxd = *rxdp;
> > +		}
> >
> > +		rxd = *rxdp;
> >  		nb_hold++;
> >  		rxe = &sw_ring[rx_id];
> >  		rx_id++;
> > @@ -816,6 +829,7 @@ i40e_recv_scattered_pkts(void *rx_queue,
> >  	struct rte_mbuf *nmb, *rxm;
> >  	uint16_t rx_id = rxq->rx_tail;
> >  	uint16_t nb_rx = 0, nb_hold = 0, rx_packet_len;
> > +	struct rte_eth_dev *dev;
> >  	uint32_t rx_status;
> >  	uint64_t qword1;
> >  	uint64_t dma_addr;
> > @@ -833,8 +847,15 @@ i40e_recv_scattered_pkts(void *rx_queue,
> >  			break;
> >
> >  		nmb = rte_mbuf_raw_alloc(rxq->mp);
> > -		if (unlikely(!nmb))
> > +		if (unlikely(!nmb)) {
> > +			PMD_RX_LOG(DEBUG, "RX mbuf alloc failed
> port_id=%u "
> > +				   "queue_id=%u", (unsigned int)rxq->port_id,
> > +				   (unsigned int)rxq->queue_id);
> > +			dev = &rte_eth_devices[rxq->port_id];
> > +			dev->data->rx_mbuf_alloc_failed++;
> >  			break;
> > +		}
> > +
> >  		rxd = *rxdp;
> >  		nb_hold++;
> >  		rxe = &sw_ring[rx_id];
> >


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

* [PATCH v3] net/i40e: mbuf alloc failed counter not incremented
  2017-04-21 23:13 ` [PATCH v2] " Allain Legacy
  2017-04-24  5:46   ` Ferruh Yigit
@ 2017-04-25 12:28   ` Allain Legacy
  2017-04-25 17:31     ` Zhang, Helin
  1 sibling, 1 reply; 10+ messages in thread
From: Allain Legacy @ 2017-04-25 12:28 UTC (permalink / raw)
  To: helin.zhang, jingjing.wu; +Cc: dev, matt.peters

From: Matt Peters <matt.peters@windriver.com>

When an mbuf alloc fails during the mempool get operation for the
i40e bulk alloc receive function, the rx_mbuf_alloc_failed counter
is not incremented to record the error.

This fix ensures consistency with the other i40e receive procedures and
other net drivers.

Signed-off-by: Matt Peters <matt.peters@windriver.com>
Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
---
 drivers/net/i40e/i40e_rxtx.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index e5471b143..74055bb04 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -610,6 +610,7 @@ static inline uint16_t
 rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 {
 	struct i40e_rx_queue *rxq = (struct i40e_rx_queue *)rx_queue;
+	struct rte_eth_dev *dev;
 	uint16_t nb_rx = 0;
 
 	if (!nb_pkts)
@@ -627,9 +628,10 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		if (i40e_rx_alloc_bufs(rxq) != 0) {
 			uint16_t i, j;
 
-			PMD_RX_LOG(DEBUG, "Rx mbuf alloc failed for "
-				   "port_id=%u, queue_id=%u",
-				   rxq->port_id, rxq->queue_id);
+			dev = I40E_VSI_TO_ETH_DEV(rxq->vsi);
+			dev->data->rx_mbuf_alloc_failed +=
+				rxq->rx_free_thresh;
+
 			rxq->rx_nb_avail = 0;
 			rxq->rx_tail = (uint16_t)(rxq->rx_tail - nb_rx);
 			for (i = 0, j = rxq->rx_tail; i < nb_rx; i++, j++)
@@ -691,6 +693,7 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 	union i40e_rx_desc rxd;
 	struct i40e_rx_entry *sw_ring;
 	struct i40e_rx_entry *rxe;
+	struct rte_eth_dev *dev;
 	struct rte_mbuf *rxm;
 	struct rte_mbuf *nmb;
 	uint16_t nb_rx;
@@ -721,10 +724,13 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 			break;
 
 		nmb = rte_mbuf_raw_alloc(rxq->mp);
-		if (unlikely(!nmb))
+		if (unlikely(!nmb)) {
+			dev = I40E_VSI_TO_ETH_DEV(rxq->vsi);
+			dev->data->rx_mbuf_alloc_failed++;
 			break;
-		rxd = *rxdp;
+		}
 
+		rxd = *rxdp;
 		nb_hold++;
 		rxe = &sw_ring[rx_id];
 		rx_id++;
@@ -816,6 +822,7 @@ i40e_recv_scattered_pkts(void *rx_queue,
 	struct rte_mbuf *nmb, *rxm;
 	uint16_t rx_id = rxq->rx_tail;
 	uint16_t nb_rx = 0, nb_hold = 0, rx_packet_len;
+	struct rte_eth_dev *dev;
 	uint32_t rx_status;
 	uint64_t qword1;
 	uint64_t dma_addr;
@@ -833,8 +840,12 @@ i40e_recv_scattered_pkts(void *rx_queue,
 			break;
 
 		nmb = rte_mbuf_raw_alloc(rxq->mp);
-		if (unlikely(!nmb))
+		if (unlikely(!nmb)) {
+			dev = I40E_VSI_TO_ETH_DEV(rxq->vsi);
+			dev->data->rx_mbuf_alloc_failed++;
 			break;
+		}
+
 		rxd = *rxdp;
 		nb_hold++;
 		rxe = &sw_ring[rx_id];
-- 
2.12.1

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

* Re: [PATCH v3] net/i40e: mbuf alloc failed counter not incremented
  2017-04-25 12:28   ` [PATCH v3] " Allain Legacy
@ 2017-04-25 17:31     ` Zhang, Helin
  2017-04-26  8:21       ` Ferruh Yigit
  0 siblings, 1 reply; 10+ messages in thread
From: Zhang, Helin @ 2017-04-25 17:31 UTC (permalink / raw)
  To: Legacy, Allain (Wind River), Wu, Jingjing; +Cc: dev, Peters, Matt (Wind River)



> -----Original Message-----
> From: Allain Legacy [mailto:allain.legacy@windriver.com]
> Sent: Tuesday, April 25, 2017 8:29 PM
> To: Zhang, Helin; Wu, Jingjing
> Cc: dev@dpdk.org; Peters, Matt (Wind River)
> Subject: [PATCH v3] net/i40e: mbuf alloc failed counter not incremented
> 
> From: Matt Peters <matt.peters@windriver.com>
> 
> When an mbuf alloc fails during the mempool get operation for the i40e bulk
> alloc receive function, the rx_mbuf_alloc_failed counter is not incremented to
> record the error.
> 
> This fix ensures consistency with the other i40e receive procedures and other
> net drivers.
> 
> Signed-off-by: Matt Peters <matt.peters@windriver.com>
> Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>

> ---
>  drivers/net/i40e/i40e_rxtx.c | 23 +++++++++++++++++------
>  1 file changed, 17 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c index
> e5471b143..74055bb04 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -610,6 +610,7 @@ static inline uint16_t  rx_recv_pkts(void *rx_queue,
> struct rte_mbuf **rx_pkts, uint16_t nb_pkts)  {
>  	struct i40e_rx_queue *rxq = (struct i40e_rx_queue *)rx_queue;
> +	struct rte_eth_dev *dev;
>  	uint16_t nb_rx = 0;
> 
>  	if (!nb_pkts)
> @@ -627,9 +628,10 @@ rx_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts, uint16_t nb_pkts)
>  		if (i40e_rx_alloc_bufs(rxq) != 0) {
>  			uint16_t i, j;
> 
> -			PMD_RX_LOG(DEBUG, "Rx mbuf alloc failed for "
> -				   "port_id=%u, queue_id=%u",
> -				   rxq->port_id, rxq->queue_id);
> +			dev = I40E_VSI_TO_ETH_DEV(rxq->vsi);
> +			dev->data->rx_mbuf_alloc_failed +=
> +				rxq->rx_free_thresh;
> +
>  			rxq->rx_nb_avail = 0;
>  			rxq->rx_tail = (uint16_t)(rxq->rx_tail - nb_rx);
>  			for (i = 0, j = rxq->rx_tail; i < nb_rx; i++, j++) @@ -691,6
> +693,7 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
> uint16_t nb_pkts)
>  	union i40e_rx_desc rxd;
>  	struct i40e_rx_entry *sw_ring;
>  	struct i40e_rx_entry *rxe;
> +	struct rte_eth_dev *dev;
>  	struct rte_mbuf *rxm;
>  	struct rte_mbuf *nmb;
>  	uint16_t nb_rx;
> @@ -721,10 +724,13 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf
> **rx_pkts, uint16_t nb_pkts)
>  			break;
> 
>  		nmb = rte_mbuf_raw_alloc(rxq->mp);
> -		if (unlikely(!nmb))
> +		if (unlikely(!nmb)) {
> +			dev = I40E_VSI_TO_ETH_DEV(rxq->vsi);
> +			dev->data->rx_mbuf_alloc_failed++;
>  			break;
> -		rxd = *rxdp;
> +		}
> 
> +		rxd = *rxdp;
>  		nb_hold++;
>  		rxe = &sw_ring[rx_id];
>  		rx_id++;
> @@ -816,6 +822,7 @@ i40e_recv_scattered_pkts(void *rx_queue,
>  	struct rte_mbuf *nmb, *rxm;
>  	uint16_t rx_id = rxq->rx_tail;
>  	uint16_t nb_rx = 0, nb_hold = 0, rx_packet_len;
> +	struct rte_eth_dev *dev;
>  	uint32_t rx_status;
>  	uint64_t qword1;
>  	uint64_t dma_addr;
> @@ -833,8 +840,12 @@ i40e_recv_scattered_pkts(void *rx_queue,
>  			break;
> 
>  		nmb = rte_mbuf_raw_alloc(rxq->mp);
> -		if (unlikely(!nmb))
> +		if (unlikely(!nmb)) {
> +			dev = I40E_VSI_TO_ETH_DEV(rxq->vsi);
> +			dev->data->rx_mbuf_alloc_failed++;
>  			break;
> +		}
> +
>  		rxd = *rxdp;
>  		nb_hold++;
>  		rxe = &sw_ring[rx_id];
> --
> 2.12.1

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

* Re: [PATCH v3] net/i40e: mbuf alloc failed counter not incremented
  2017-04-25 17:31     ` Zhang, Helin
@ 2017-04-26  8:21       ` Ferruh Yigit
  0 siblings, 0 replies; 10+ messages in thread
From: Ferruh Yigit @ 2017-04-26  8:21 UTC (permalink / raw)
  To: Zhang, Helin, Legacy, Allain (Wind River), Wu, Jingjing
  Cc: dev, Peters, Matt (Wind River)

On 4/25/2017 6:31 PM, Zhang, Helin wrote:
> 
> 
>> -----Original Message-----
>> From: Allain Legacy [mailto:allain.legacy@windriver.com]
>> Sent: Tuesday, April 25, 2017 8:29 PM
>> To: Zhang, Helin; Wu, Jingjing
>> Cc: dev@dpdk.org; Peters, Matt (Wind River)
>> Subject: [PATCH v3] net/i40e: mbuf alloc failed counter not incremented
>>
>> From: Matt Peters <matt.peters@windriver.com>
>>
>> When an mbuf alloc fails during the mempool get operation for the i40e bulk
>> alloc receive function, the rx_mbuf_alloc_failed counter is not incremented to
>> record the error.
>>
>> This fix ensures consistency with the other i40e receive procedures and other
>> net drivers.
>>
>> Signed-off-by: Matt Peters <matt.peters@windriver.com>
>> Signed-off-by: Allain Legacy <allain.legacy@windriver.com>
> Acked-by: Helin Zhang <helin.zhang@intel.com>

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

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

end of thread, other threads:[~2017-04-26  8:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-27 16:30 [PATCH] net/i40e: mbuf alloc failed counter not incremented Allain Legacy
2017-03-28  8:52 ` Zhang, Helin
2017-03-29 22:42   ` Legacy, Allain
2017-04-21 23:13 ` [PATCH v2] " Allain Legacy
2017-04-24  5:46   ` Ferruh Yigit
2017-04-24 11:21     ` Legacy, Allain
2017-04-25 11:01     ` Zhang, Helin
2017-04-25 12:28   ` [PATCH v3] " Allain Legacy
2017-04-25 17:31     ` Zhang, Helin
2017-04-26  8:21       ` Ferruh Yigit

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.