[dpdk-dev] i40e_rxtx_vec_avx2 code review

[dpdk-dev] i40e_rxtx_vec_avx2 code review

[Morten Brørup]

Jeff, Beilei,

Looking closer at i40e_rxtx_vec_avx2.c, I noticed:
1. i40e_rxq_rearm() uses RTE_I40E_DESCS_PER_LOOP, while all other functions in the file use RTE_I40E_DESCS_PER_LOOP_AVX. It may be correct, but please check.
2. The function descriptions of i40e_recv_pkts_vec_avx2(), i40e_recv_scattered_burst_vec_avx2() and i40e_recv_scattered_pkts_vec_avx2() refer to RTE_I40E_DESCS_PER_LOOP, but they should be referring to RTE_I40E_DESCS_PER_LOOP_AVX.
3. RTE_I40E_DESCS_PER_LOOP_AVX is defined in the file, unlike RTE_I40E_DESCS_PER_LOOP, which is defined in the i40e_rxtx.h header file. Your choice, just mentioning it.


Med venlig hilsen / kind regards
- Morten Brørup

Re: [dpdk-dev] i40e_rxtx_vec_avx2 code review

[Jeff Guo]

hi, mb

Seems that there are a referring issue here which you have found, i 
think we need to fix it, if there are on objection i will add that fix 
into the prior burst fixing patch, thanks very much.

On 8/29/2020 3:21 PM, Morten Brørup wrote:
> Jeff, Beilei,
>
> Looking closer at i40e_rxtx_vec_avx2.c, I noticed:
> 1. i40e_rxq_rearm() uses RTE_I40E_DESCS_PER_LOOP, while all other functions in the file use RTE_I40E_DESCS_PER_LOOP_AVX. It may be correct, but please check.
> 2. The function descriptions of i40e_recv_pkts_vec_avx2(), i40e_recv_scattered_burst_vec_avx2() and i40e_recv_scattered_pkts_vec_avx2() refer to RTE_I40E_DESCS_PER_LOOP, but they should be referring to RTE_I40E_DESCS_PER_LOOP_AVX.
> 3. RTE_I40E_DESCS_PER_LOOP_AVX is defined in the file, unlike RTE_I40E_DESCS_PER_LOOP, which is defined in the i40e_rxtx.h header file. Your choice, just mentioning it.
>
>
> Med venlig hilsen / kind regards
> - Morten Brørup
>

Re: [dpdk-dev] i40e_rxtx_vec_avx2 code review

[Morten Brørup]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jeff Guo
> Sent: Monday, August 31, 2020 8:30 AM
> 
> hi, mb
> 
> Seems that there are a referring issue here which you have found, i
> think we need to fix it, if there are on objection i will add that fix
> into the prior burst fixing patch, thanks very much.

I don't object if you put it in the same patch. I can't speak on behalf of others, though.

-Morten

> 
> On 8/29/2020 3:21 PM, Morten Brørup wrote:
> > Jeff, Beilei,
> >
> > Looking closer at i40e_rxtx_vec_avx2.c, I noticed:
> > 1. i40e_rxq_rearm() uses RTE_I40E_DESCS_PER_LOOP, while all other
> functions in the file use RTE_I40E_DESCS_PER_LOOP_AVX. It may be
> correct, but please check.
> > 2. The function descriptions of i40e_recv_pkts_vec_avx2(),
> i40e_recv_scattered_burst_vec_avx2() and
> i40e_recv_scattered_pkts_vec_avx2() refer to RTE_I40E_DESCS_PER_LOOP,
> but they should be referring to RTE_I40E_DESCS_PER_LOOP_AVX.
> > 3. RTE_I40E_DESCS_PER_LOOP_AVX is defined in the file, unlike
> RTE_I40E_DESCS_PER_LOOP, which is defined in the i40e_rxtx.h header
> file. Your choice, just mentioning it.
> >
> >
> > Med venlig hilsen / kind regards
> > - Morten Brørup
> >