Re: [dpdk-stable] [dpdk-dev] [PATCH 3/3] net/i40e: fix segment fault in AVX512

["Lu, Wenzhuo" <wenzhuo.lu@intel.com>]

Hi David,
Sorry for the late response.

> > +	if (!cache)
> > +		goto normal;
> 
> [DC] Like in IAVF and ICE, should we also check for cache->len == 0, like is
> done in Tx path?
Not necessary because below code tries to get more buffer is length is not as long as needed, it covers the case even if the length is 0.

> > +	/* Initialize the mbufs in vector, process 4 mbufs in one loop */
> 
> [DC] Comment should say 2 mbufs
Thanks. Will correct it.

> > _mm512_set1_epi64(RTE_PKTMBUF_HEADROOM);
> > +	/* Initialize the mbufs in vector, process 4 mbufs in one loop */
> 
> [DC] Comment should say 8 mbufs
Thanks. Will correct it.

> > +		/**
> > +		 * merge 0 & 1, by casting 0 to 256-bit and inserting 1
> > +		 * into the high lanes. Similarly for 2 & 3
> > +		 */
> 
> [DC] Comment should say "Similarly for 2 & 3, 4 & 5, 6 & 7"
Thanks. Will correct it.

> 
> > +		vaddr0_1 =
> > +
> > 	_mm256_inserti128_si256(_mm256_castsi128_si256(vaddr0),
> > +						vaddr1, 1);
> 
> <snip>
> 
> > +		/* flush desc with pa dma_addr */
> > +		_mm512_store_si512((__m512i *)&rxdp->read,
> > dma_addr0_3);
> > +		_mm512_store_si512((__m512i *)&(rxdp + 4)->read,
> > dma_addr4_7);
> > +	}
> > +#endif
> 
> [DC] Again, there's common code here with the avx2 file and also with the
> IAVF and ICE PMDs.
> 
> As I said in other reviews, maybe it's not practical to share code across PMDs.
> But might be good to have some common functions within each PMD for
> avx2 and avx512 paths
Most of the code looks same but not all. In this avx512 file, some avx512 instructions are used to replace the avx2 ones.
Let me check if some common code can be saved.