From: Ferruh Yigit <ferruh.yigit@intel.com>
To: Allain Legacy <allain.legacy@windriver.com>,
helin.zhang@intel.com, jingjing.wu@intel.com
Cc: dev@dpdk.org, matt.peters@windriver.com
Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: mbuf alloc failed counter not incremented
Date: Mon, 24 Apr 2017 06:46:31 +0100 [thread overview]
Message-ID: <fc7fa349-d9b8-1330-b175-ec861a223bc7@intel.com> (raw)
In-Reply-To: <20170421231352.133855-1-allain.legacy@windriver.com>
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];
>
next prev parent reply other threads:[~2017-04-24 5:46 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-03-27 16:30 [dpdk-dev] [PATCH] " Allain Legacy
2017-03-28 8:52 ` Zhang, Helin
2017-03-29 22:42 ` Legacy, Allain
2017-04-21 23:13 ` [dpdk-dev] [PATCH v2] " Allain Legacy
2017-04-24 5:46 ` Ferruh Yigit [this message]
2017-04-24 11:21 ` Legacy, Allain
2017-04-25 11:01 ` Zhang, Helin
2017-04-25 12:28 ` [dpdk-dev] [PATCH v3] " Allain Legacy
2017-04-25 17:31 ` Zhang, Helin
2017-04-26 8:21 ` Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=fc7fa349-d9b8-1330-b175-ec861a223bc7@intel.com \
--to=ferruh.yigit@intel.com \
--cc=allain.legacy@windriver.com \
--cc=dev@dpdk.org \
--cc=helin.zhang@intel.com \
--cc=jingjing.wu@intel.com \
--cc=matt.peters@windriver.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).