Re: [dpdk-dev] [dpdk-stable] [PATCH v3 3/5] net/tap: fix check for mbuf's nb_segs failure

[wangyunjian <wangyunjian@huawei.com>]

> -----Original Message-----
> From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> Sent: Wednesday, April 8, 2020 12:08 AM
> To: Ferruh Yigit <ferruh.yigit@intel.com>
> Cc: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org;
> keith.wiles@intel.com; Lilijun (Jerry) <jerry.lilijun@huawei.com>; xudingke
> <xudingke@huawei.com>; stable@dpdk.org
> Subject: Re: [dpdk-dev] [dpdk-stable] [PATCH v3 3/5] net/tap: fix check for
> mbuf's nb_segs failure
> 
> On Tue, 7 Apr 2020 16:45:59 +0100
> Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> 
> > On 4/7/2020 4:38 PM, Stephen Hemminger wrote:
> > > On Tue, 7 Apr 2020 16:15:16 +0100
> > > Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> > >
> > >>> +static void
> > >>> +tap_rxq_pool_free(struct rte_mbuf *pool) {
> > >>> +	struct rte_mbuf *mbuf = pool;
> > >>> +	uint16_t nb_segs = 1;
> > >>> +
> > >>> +	if (mbuf == NULL)
> > >>> +		return;
> > >>> +
> > >>> +	while (mbuf->next) {
> > >>> +		mbuf = mbuf->next;
> > >>> +		nb_segs++;
> > >>> +	}
> > >>> +	pool->nb_segs = nb_segs;
> > >>> +	rte_pktmbuf_free(pool);
> > >>> +}
> > >
> > > Since mbuf is going to be free, why bother with nb_segs.
> > > Since rte_pktmbuf_free takes NULL as an argument, and frees the
> > > m->next chain I don't see why not just
> > > 	rte_pktmbuf_free(pool)
> > >
> >
> > Chain is not constructed properly, 'nb_segs' is wrong, only
> 'rte_pktmbuf_free()'
> > call won't free all the chain but first mbuf.
> >
> > This implementation is fixing 'nb_segs' sot that 'rte_pktmbuf_free()'
> > can work as you suggested.
> >
> > Or I suggest iterate the list and fix all mbufs, instead of fixing
> > 'nb_segs', this may be one iteration less.
> 
> If you look at implementation of rte_pktmbuf_free() in current DPDK version it
> does not care what nb_segs is set to.

I found this problem with mbuf debug enabled.

Thanks
Yunjian