[dpdk-dev] [PATCH v3 04/16] fm10k: add func to re-allocate mbuf for RX ring

[Liang, Cunming]

Hi Mark，


> -----Original Message-----
> From: Chen, Jing D
> Sent: Thursday, October 29, 2015 1:24 PM
> To: Liang, Cunming; dev at dpdk.org
> Cc: Tao, Zhe; He, Shaopeng; Ananyev, Konstantin; Richardson, Bruce
> Subject: RE: [PATCH v3 04/16] fm10k: add func to re-allocate mbuf for RX ring
> 
> Hi, Steve,
> 
> Best Regards,
> Mark
> 
> 
> > -----Original Message-----
> > From: Liang, Cunming
> > Sent: Wednesday, October 28, 2015 9:59 PM
> > To: Chen, Jing D; dev at dpdk.org
> > Cc: Tao, Zhe; He, Shaopeng; Ananyev, Konstantin; Richardson, Bruce
> > Subject: Re: [PATCH v3 04/16] fm10k: add func to re-allocate mbuf for RX ring
> >
> > Hi Mark,
> >
> > On 10/27/2015 5:46 PM, Chen Jing D(Mark) wrote:
> > > From: "Chen Jing D(Mark)" <jing.d.chen at intel.com>
> > >
> > > Add function fm10k_rxq_rearm to re-allocate mbuf for used desc
> > > in RX HW ring.
> > >
> > > Signed-off-by: Chen Jing D(Mark) <jing.d.chen at intel.com>
> > > ---
> > >   drivers/net/fm10k/fm10k.h          |    9 ++++
> > >   drivers/net/fm10k/fm10k_ethdev.c   |    3 +
> > >   drivers/net/fm10k/fm10k_rxtx_vec.c |   90
> > ++++++++++++++++++++++++++++++++++++
> > >   3 files changed, 102 insertions(+), 0 deletions(-)
> > [...]
> > > +static inline void
> > > +fm10k_rxq_rearm(struct fm10k_rx_queue *rxq)
> > > +{
> > > +	int i;
> > > +	uint16_t rx_id;
> > > +	volatile union fm10k_rx_desc *rxdp;
> > > +	struct rte_mbuf **mb_alloc = &rxq->sw_ring[rxq->rxrearm_start];
> > > +	struct rte_mbuf *mb0, *mb1;
> > > +	__m128i head_off = _mm_set_epi64x(
> > > +			RTE_PKTMBUF_HEADROOM +
> > FM10K_RX_DATABUF_ALIGN - 1,
> > > +			RTE_PKTMBUF_HEADROOM +
> > FM10K_RX_DATABUF_ALIGN - 1);
> > > +	__m128i dma_addr0, dma_addr1;
> > > +	/* Rx buffer need to be aligned with 512 byte */
> > > +	const __m128i hba_msk = _mm_set_epi64x(0,
> > > +				UINT64_MAX - FM10K_RX_DATABUF_ALIGN
> > + 1);
> > > +
> > > +	rxdp = rxq->hw_ring + rxq->rxrearm_start;
> > > +
> > > +	/* Pull 'n' more MBUFs into the software ring */
> > > +	if (rte_mempool_get_bulk(rxq->mp,
> > > +				 (void *)mb_alloc,
> > > +				 RTE_FM10K_RXQ_REARM_THRESH) < 0) {
> > Here's one potential issue when the failure happens. As tail won't
> > update, the head will equal to tail in the end. HW won't write back
> > anyway, however the SW recv_raw_pkts_vec only check DD bit, the old
> > 'dirty' descriptor(DD bit is not clean) will be taken and continue move
> > forward to check the next which even beyond the tail. I'm sorry didn't
> > catch it on the first time. /Steve
> 
> I have a different view on this. In case mbuf allocation always failed and tail
> equaled to head,
> then HW will stop to send new packet to HW ring, as you pointed out. Then,
> when
> Mbuf can be allocated, this function will refill HW ring and update tail. 
We can't guarantee it successful to recover and allocates new mbuf before the polling SW already move beyond the un-rearmed dirty entry. 
So, HW
> will
> resume to fill packet to HW ring. Receive functions will continue to work.
The point is HW is pending on that moment, but polling receive function won't wait, it just read next DD, but the value is 1 which hasn't cleared.
> Anything I missed?
> 
> > > +		rte_eth_devices[rxq->port_id].data->rx_mbuf_alloc_failed
> > +=
> > > +			RTE_FM10K_RXQ_REARM_THRESH;
> > > +		return;
> > > +	}
> > > +
> > > +