Re: [PATCH] net/enic: decrement Tx mbuf reference count before recycling

-----Original Message-----
From: Olivier Matz [mailto:olivier.matz@6wind.com]
Sent: Monday, July 11, 2016 3:04 AM
To: John Daley (johndale) <redacted>; dev@dpdk.org
Cc: bruce.richardson@intel.com
Subject: Re: [dpdk-dev] [PATCH] net/enic: decrement Tx mbuf reference
count before recycling

Hi John,

On 07/09/2016 12:22 AM, John Daley wrote:

quoted

In the Tx cleanup function, the reference count in mbufs to be
returned to the pool should to be decremented before they are
returned. Decrementing is not done by rte_mempool_put_bulk() so it
must be done separately using __rte_pktmbuf_prefree_seg().
If decrementing does not result in a 0 reference count the mbuf is not
returned to the pool and whatever has the last reference is
responsible for freeing.

Fixes: 36935afbc53c ("net/enic: refactor Tx mbuf recycling")
Reviewed-by: Nelson Escobar <neescoba@cisco.com>
Signed-off-by: John Daley <redacted>
---
Since reference counts are set to 0 when mbufs are reallocated from
the pool, and sending packets with reference count not equal to 1 is
probably an application error, this patch may not be critical. But a
debug ASSERT caught it and it would be nice to have it fixed in 16.07.

Sending a packet with refcnt != 1 is not an error. It can happen when using
mbuf clones. So indeed it would be better to have in 16.07.

For the same reason, I also wonder if enic_free_wq_buf() should also be
updated with:

-       rte_mempool_put(mbuf->pool, mbuf);
+       rte_pktmbuf_free(mbuf);

That is a very good point, thank you. I'll use rte_pktmubf_free_seg(mbuf) though, since we are walking an array of all mbuf segments. V2 coming momentarily.
-john

Regards,
Olivier