Re: [dpdk-dev] [PATCH v2] efd: support lookup using neon intrinsics

[Jianbo Liu <jianbo.liu@linaro.org>]

On 28 April 2017 at 18:38, Sekhar, Ashwin <Ashwin.Sekhar@cavium.com> wrote:
> On Friday 28 April 2017 03:36 PM, Jianbo Liu wrote:
>> On 27 April 2017 at 20:44, Ashwin Sekhar T K
>> <ashwin.sekhar@caviumnetworks.com> wrote:
>>> * Added file lib/librte_efd/rte_efd_arm64.h to hold arm64
>>>   specific definitions
>>> * Verified the changes with efd_autotest unit test case
>>>
>>> Signed-off-by: Ashwin Sekhar T K <ashwin.sekhar@caviumnetworks.com>
>>> ---
>>> v2:
>>> * Slightly modified the content of the commit message body
>>> * Added prefix [dpdk-dev] to the email subject line
>>>
>>>  MAINTAINERS                    |  1 +
>>>  lib/librte_efd/rte_efd.c       | 22 ++++++++++++
>>>  lib/librte_efd/rte_efd_arm64.h | 76 ++++++++++++++++++++++++++++++++++++++++++
>>>  3 files changed, 99 insertions(+)
>>>  create mode 100644 lib/librte_efd/rte_efd_arm64.h
>>>
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index b6495d2..7d708ae 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -147,6 +147,7 @@ F: lib/librte_eal/common/include/arch/arm/*_64.h
>>>  F: lib/librte_acl/acl_run_neon.*
>>>  F: lib/librte_lpm/rte_lpm_neon.h
>>>  F: lib/librte_hash/rte*_arm64.h
>>> +F: lib/librte_efd/rte*_arm64.h
>>>  F: drivers/net/ixgbe/ixgbe_rxtx_vec_neon.c
>>>  F: drivers/net/i40e/i40e_rxtx_vec_neon.c
>>>  F: drivers/net/virtio/virtio_rxtx_simple_neon.c
>>> diff --git a/lib/librte_efd/rte_efd.c b/lib/librte_efd/rte_efd.c
>>> index f601d62..4d9a088 100644
>>> --- a/lib/librte_efd/rte_efd.c
>>> +++ b/lib/librte_efd/rte_efd.c
>>> @@ -53,6 +53,8 @@
>>>  #include "rte_efd.h"
>>>  #if defined(RTE_ARCH_X86)
>>>  #include "rte_efd_x86.h"
>>> +#elif defined(RTE_ARCH_ARM64)
>>> +#include "rte_efd_arm64.h"
>>>  #endif
>>>
>>>  #define EFD_KEY(key_idx, table) (table->keys + ((key_idx) * table->key_len))
>>> @@ -103,6 +105,7 @@ allocated memory
>>>  enum efd_lookup_internal_function {
>>>         EFD_LOOKUP_SCALAR = 0,
>>>         EFD_LOOKUP_AVX2,
>>> +       EFD_LOOKUP_NEON,
>>
>> Should it be included in "if defined(RTE_ARCH_ARM64)"?
>>
> The enum can be wrapped under "if defined(RTE_ARCH_ARM64)" with no
> issues, as all its usages are also under "if defined(RTE_ARCH_ARM64)".
> I followed EFD_LOOKUP_AVX2 and defined EFD_LOOKUP_NEON on the same lines.
> Please advise on whether this change is to be made. Will follow your advice.

Yes, please do that.