[dpdk-dev] net/liquidio: do not touch mbuf initialized fields
Checks
Commit Message
Avoid re-initializing of mbuf fields which are set while in pool.
Replaced lio_recv_buffer_alloc with rte_pktmbuf_alloc.
See commit 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool").
Signed-off-by: Shijith Thotton <shijith.thotton@caviumnetworks.com>
---
drivers/net/liquidio/lio_rxtx.c | 37 ++++---------------------------------
drivers/net/liquidio/lio_struct.h | 2 +-
2 files changed, 5 insertions(+), 34 deletions(-)
Comments
On 6/8/2017 12:22 PM, Shijith Thotton wrote:
> Avoid re-initializing of mbuf fields which are set while in pool.
> Replaced lio_recv_buffer_alloc with rte_pktmbuf_alloc.
>
> See commit 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool").
>
> Signed-off-by: Shijith Thotton <shijith.thotton@caviumnetworks.com>
<...>
> @@ -489,9 +466,6 @@
> droq->refill_count++;
>
> if (likely(nicbuf != NULL)) {
> - nicbuf->data_off = RTE_PKTMBUF_HEADROOM;
Hi Shijith, Olivier,
nb_segs and next are initialized properly when mbuf allocated, but is
"data_off = RTE_PKTMBUF_HEADROOM" guaranteed?
> - nicbuf->nb_segs = 1;
> - nicbuf->next = NULL;
> /* We don't have a way to pass flags yet */
> nicbuf->ol_flags = 0;
> if (rh->r_dh.has_hash) {
<...>
Hi Ferruh,
On Fri, 9 Jun 2017 12:24:56 +0100, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 6/8/2017 12:22 PM, Shijith Thotton wrote:
> > Avoid re-initializing of mbuf fields which are set while in pool.
> > Replaced lio_recv_buffer_alloc with rte_pktmbuf_alloc.
> >
> > See commit 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool").
> >
> > Signed-off-by: Shijith Thotton <shijith.thotton@caviumnetworks.com>
>
> <...>
>
> > @@ -489,9 +466,6 @@
> > droq->refill_count++;
> >
> > if (likely(nicbuf != NULL)) {
> > - nicbuf->data_off = RTE_PKTMBUF_HEADROOM;
>
> Hi Shijith, Olivier,
>
> nb_segs and next are initialized properly when mbuf allocated, but is
> "data_off = RTE_PKTMBUF_HEADROOM" guaranteed?
>
> > - nicbuf->nb_segs = 1;
> > - nicbuf->next = NULL;
> > /* We don't have a way to pass flags yet */
> > nicbuf->ol_flags = 0;
> > if (rh->r_dh.has_hash) {
>
> <...>
It's not guaranteed when using a raw allocation, i.e. rte_mbuf_raw_alloc()
or rte_mempool_get(). The PMD usually use these functions in Rx path to
fully initialize the mbuf fields on their own.
In this driver, it looks it uses rte_pktmbuf_alloc(), which calls
rte_pktmbuf_reset(), fully initializing the mbuf struct. So data_off is
properly set.
As a side note, I wonder if lio_recv_buffer_alloc() replaced by
rte_pktmbuf_alloc() everywhere in the code.
Olivier
On 6/8/2017 12:22 PM, Shijith Thotton wrote:
> Avoid re-initializing of mbuf fields which are set while in pool.
> Replaced lio_recv_buffer_alloc with rte_pktmbuf_alloc.
>
> See commit 8f094a9ac5d7 ("mbuf: set mbuf fields while in pool").
>
> Signed-off-by: Shijith Thotton <shijith.thotton@caviumnetworks.com>
Applied to dpdk-next-net/master, thanks.
@@ -81,28 +81,6 @@
lio_droq_reset_indices(droq);
}
-static void *
-lio_recv_buffer_alloc(struct lio_device *lio_dev, int q_no)
-{
- struct lio_droq *droq = lio_dev->droq[q_no];
- struct rte_mempool *mpool = droq->mpool;
- struct rte_mbuf *m;
-
- m = rte_pktmbuf_alloc(mpool);
- if (m == NULL) {
- lio_dev_err(lio_dev, "Cannot allocate\n");
- return NULL;
- }
-
- rte_mbuf_refcnt_set(m, 1);
- m->next = NULL;
- m->data_off = RTE_PKTMBUF_HEADROOM;
- m->nb_segs = 1;
- m->pool = mpool;
-
- return m;
-}
-
static int
lio_droq_setup_ring_buffers(struct lio_device *lio_dev,
struct lio_droq *droq)
@@ -112,7 +90,7 @@
void *buf;
for (i = 0; i < droq->max_count; i++) {
- buf = lio_recv_buffer_alloc(lio_dev, droq->q_no);
+ buf = rte_pktmbuf_alloc(droq->mpool);
if (buf == NULL) {
lio_dev_err(lio_dev, "buffer alloc failed\n");
droq->stats.rx_alloc_failure++;
@@ -378,7 +356,6 @@
/* lio_droq_refill
*
- * @param lio_dev - pointer to the lio device structure
* @param droq - droq in which descriptors require new buffers.
*
* Description:
@@ -394,7 +371,7 @@
* This routine is called with droq->lock held.
*/
static uint32_t
-lio_droq_refill(struct lio_device *lio_dev, struct lio_droq *droq)
+lio_droq_refill(struct lio_droq *droq)
{
struct lio_droq_desc *desc_ring;
uint32_t desc_refilled = 0;
@@ -407,7 +384,7 @@
* reuse the buffer, else allocate.
*/
if (droq->recv_buf_list[droq->refill_idx].buffer == NULL) {
- buf = lio_recv_buffer_alloc(lio_dev, droq->q_no);
+ buf = rte_pktmbuf_alloc(droq->mpool);
/* If a buffer could not be allocated, no point in
* continuing
*/
@@ -489,9 +466,6 @@
droq->refill_count++;
if (likely(nicbuf != NULL)) {
- nicbuf->data_off = RTE_PKTMBUF_HEADROOM;
- nicbuf->nb_segs = 1;
- nicbuf->next = NULL;
/* We don't have a way to pass flags yet */
nicbuf->ol_flags = 0;
if (rh->r_dh.has_hash) {
@@ -545,9 +519,6 @@
if (!pkt_len)
first_buf = nicbuf;
- nicbuf->data_off = RTE_PKTMBUF_HEADROOM;
- nicbuf->nb_segs = 1;
- nicbuf->next = NULL;
nicbuf->port = lio_dev->port_id;
/* We don't have a way to pass
* flags yet
@@ -617,7 +588,7 @@
}
if (droq->refill_count >= droq->refill_threshold) {
- int desc_refilled = lio_droq_refill(lio_dev, droq);
+ int desc_refilled = lio_droq_refill(droq);
/* Flush the droq descriptor data to memory to be sure
* that when we update the credits the data in memory is
@@ -94,7 +94,7 @@ struct lio_droq_stats {
/** Num of vxlan packets received; */
uint64_t rx_vxlan;
- /** Num of failures of lio_recv_buffer_alloc() */
+ /** Num of failures of rte_pktmbuf_alloc() */
uint64_t rx_alloc_failure;
};