Hi,

Yes you are right - 
Maybe with this way?

#if defined(RTE_ARCH_ARM64)
                vector_mode = IPSEC_MB_ARM64;
 #elif defined(RTE_ARCH_X86_64)
        if (vector_mode == IPSEC_MB_NOT_SUPPORTED) {
                /* Check CPU for supported vector instruction set */
                if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F))
                        vector_mode = IPSEC_MB_AVX512;
                else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
                        vector_mode = IPSEC_MB_AVX2;
                else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX))
                        vector_mode = IPSEC_MB_AVX;
                else
                        vector_mode = IPSEC_MB_SSE;
#else
                /* error return */
#endif

The only reason I suggest this is to avoid forgetting why it is implemented 
this way in the future...

Regards,
Fan

> -----Original Message-----
> From: Ashwin Sekhar Thalakalath Kottilveetil <asek...@marvell.com>
> Sent: Tuesday, June 21, 2022 7:38 AM
> To: Zhang, Roy Fan <roy.fan.zh...@intel.com>; dev@dpdk.org
> Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Sunil Kumar Kori
> <sk...@marvell.com>; Satha Koteswara Rao Kottidi
> <skotesh...@marvell.com>; Pavan Nikhilesh Bhagavatula
> <pbhagavat...@marvell.com>; Kiran Kumar Kokkilagadda
> <kirankum...@marvell.com>; Satheesh Paul Antonysamy
> <psathe...@marvell.com>; Anoob Joseph <ano...@marvell.com>; Akhil Goyal
> <gak...@marvell.com>; Harman Kalra <hka...@marvell.com>; Nithin Kumar
> Dabilpuram <ndabilpu...@marvell.com>; De Lara Guarch, Pablo
> <pablo.de.lara.gua...@intel.com>
> Subject: RE: [PATCH] crypto/ipsec_mb: enable compilation for non x86 arch
> 
> Hi Fan,
> 
> Thanks for the suggestion. However that will not work for ARM64.
> 
> Please note that the #defines RTE_CPUFLAG_AVX512F, RTE_CPUFLAG_AVX2
> and RTE_CPUFLAG_AVX are not available for ARM64. So the compilation will
> break.
> 
> I can introduce IPSEC_MB_ARM64, but still the current branch needs to be
> completely wrapped under #ifdef to avoid compilation failure.
> 
> Does the below code look OK ?
> 
>       if (vector_mode == IPSEC_MB_NOT_SUPPORTED) {
> #if defined(RTE_ARCH_X86_64)
>               /* Check CPU for supported vector instruction set */
>               if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F))
>                       vector_mode = IPSEC_MB_AVX512;
>               else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
>                       vector_mode = IPSEC_MB_AVX2;
>               else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX))
>                       vector_mode = IPSEC_MB_AVX;
>               else
>                       vector_mode = IPSEC_MB_SSE;
> #elif defined(RTE_ARCH_ARM64)
>               vector_mode = IPSEC_MB_ARM64;
> #else
>               /* error return */
> #endif
>       }
> 
> Ashwin Sekhar T K
> 
> > -----Original Message-----
> > From: Zhang, Roy Fan <roy.fan.zh...@intel.com>
> > Sent: Monday, June 20, 2022 7:10 PM
> > To: Ashwin Sekhar Thalakalath Kottilveetil <asek...@marvell.com>;
> > dev@dpdk.org
> > Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Sunil Kumar Kori
> > <sk...@marvell.com>; Satha Koteswara Rao Kottidi
> > <skotesh...@marvell.com>; Pavan Nikhilesh Bhagavatula
> > <pbhagavat...@marvell.com>; Kiran Kumar Kokkilagadda
> > <kirankum...@marvell.com>; Satheesh Paul Antonysamy
> > <psathe...@marvell.com>; Anoob Joseph <ano...@marvell.com>; Akhil
> > Goyal <gak...@marvell.com>; Harman Kalra <hka...@marvell.com>; Nithin
> > Kumar Dabilpuram <ndabilpu...@marvell.com>; De Lara Guarch, Pablo
> > <pablo.de.lara.gua...@intel.com>
> > Subject: [EXT] RE: [PATCH] crypto/ipsec_mb: enable compilation for non x86
> > arch
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > Hi,
> >
> > Thank you for the explanation.
> > In that case I'd suggest instead of warping the branches completely with
> > macro, wrapping on the last "else" instead.
> > Something like
> >
> >     if (vector_mode == IPSEC_MB_NOT_SUPPORTED) {
> >             /* Check CPU for supported vector instruction set */
> >             if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F))
> >                     vector_mode = IPSEC_MB_AVX512;
> >             else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
> >                     vector_mode = IPSEC_MB_AVX2;
> >             else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX))
> >                     vector_mode = IPSEC_MB_AVX;
> >             else
> > #ifdef RTE_ARCH_X86_64
> >                     vector_mode = IPSEC_MB_SSE;
> > #elif RTE_ARCH_ARM64 ?
> >                     Vector_mode = IPSEC_MB_ARM64; /* newly
> > introduced ARM vector mode */ #else
> >                     /* error return */
> > #endif
> >     }
> >
> > This should also help setting RTE_CRYPTODEV_FF_CPU_ARM_CE in device
> > feature flag bitmap later.
> >
> > Regards,
> > Fan
> >
> > > -----Original Message-----
> > > From: Ashwin Sekhar Thalakalath Kottilveetil <asek...@marvell.com>
> > > Sent: Friday, June 17, 2022 11:34 AM
> > > To: Zhang, Roy Fan <roy.fan.zh...@intel.com>; dev@dpdk.org
> > > Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Sunil Kumar Kori
> > > <sk...@marvell.com>; Satha Koteswara Rao Kottidi
> > > <skotesh...@marvell.com>; Pavan Nikhilesh Bhagavatula
> > > <pbhagavat...@marvell.com>; Kiran Kumar Kokkilagadda
> > > <kirankum...@marvell.com>; Satheesh Paul Antonysamy
> > > <psathe...@marvell.com>; Anoob Joseph <ano...@marvell.com>; Akhil
> > > Goyal <gak...@marvell.com>; Harman Kalra <hka...@marvell.com>; Nithin
> > > Kumar Dabilpuram <ndabilpu...@marvell.com>
> > > Subject: RE: [PATCH] crypto/ipsec_mb: enable compilation for non x86
> > > arch
> > >
> > > Hi Fan,
> > >
> > > There is an ARM64 port for IPSEC-MB
> > > https://urldefense.proofpoint.com/v2/url?u=https-
> > 3A__gitlab.arm.com_ar
> > > m-2Dreference-2D&d=DwIF-g&c=nKjWec2b6R0mOyPaz7xtfQ&r=pYk-
> > QOhvnkU-_75y0
> > >
> > NKSn535ZotEGI_E69Py3Ppondk&m=hJ5ewsPh0KMcvMFylomkCk_hiNsuTm-
> > HfHad1UoQy
> > > K7MHqUyXvJSwSgyAQQThWyj&s=mL8bOzY63JQ-
> > 6CBhruqHv9bE5sEWT5PfXRFv4zbI6mw&
> > > e=
> > > solutions/ipsec-mb .
> > >
> > > When we compile DPDK with this IPSEC-MB port for ARM64, the
> > > vector_mode value doesn't matter.
> > >
> > > And the cpuflag #defines RTE_CPUFLAG_AVX512F, RTE_CPUFLAG_AVX2
> > etc.
> > > are not available in ARM64. So these need to be made x86 specific.
> > >
> > > Thank  you
> > > Ashwin
> > >
> > > Ashwin Sekhar T K
> > >
> > > > -----Original Message-----
> > > > From: Zhang, Roy Fan <roy.fan.zh...@intel.com>
> > > > Sent: Friday, June 17, 2022 3:53 PM
> > > > To: Ashwin Sekhar Thalakalath Kottilveetil <asek...@marvell.com>;
> > > > dev@dpdk.org
> > > > Cc: Jerin Jacob Kollanukkaran <jer...@marvell.com>; Sunil Kumar Kori
> > > > <sk...@marvell.com>; Satha Koteswara Rao Kottidi
> > > > <skotesh...@marvell.com>; Pavan Nikhilesh Bhagavatula
> > > > <pbhagavat...@marvell.com>; Kiran Kumar Kokkilagadda
> > > > <kirankum...@marvell.com>; Satheesh Paul Antonysamy
> > > > <psathe...@marvell.com>; Anoob Joseph <ano...@marvell.com>;
> > Akhil
> > > > Goyal <gak...@marvell.com>; Harman Kalra <hka...@marvell.com>;
> > > > Nithin Kumar Dabilpuram <ndabilpu...@marvell.com>
> > > > Subject: [EXT] RE: [PATCH] crypto/ipsec_mb: enable compilation for
> > > > non x86 arch
> > > >
> > > > External Email
> > > >
> > > > --------------------------------------------------------------------
> > > > --
> > > > Hi,
> > > >
> > > > IPsec-mb PMD should not be built at all if the library is not installed.
> > > > Also, the code you are warping with macro only prevents initializing
> > > > the vector mode param to SSE which is later used to add feature flag
> > > > bits. To me this change does not make much sense.
> > > > Can you share with me the purpose of this change?
> > > >
> > > > Regards,
> > > > Fan
> > > >
> > > > > -----Original Message-----
> > > > > From: Ashwin Sekhar T K <asek...@marvell.com>
> > > > > Sent: Friday, June 10, 2022 5:21 PM
> > > > > To: dev@dpdk.org
> > > > > Cc: jer...@marvell.com; sk...@marvell.com; skotesh...@marvell.com;
> > > > > pbhagavat...@marvell.com; kirankum...@marvell.com;
> > > > > psathe...@marvell.com; asek...@marvell.com; ano...@marvell.com;
> > > > > gak...@marvell.com; hka...@marvell.com; ndabilpu...@marvell.com
> > > > > Subject: [PATCH] crypto/ipsec_mb: enable compilation for non x86
> > > > > arch
> > > > >
> > > > > Enable compilation for non x86 architectures by conditionally
> > > > > compiling x86 specific code.
> > > > >
> > > > > Signed-off-by: Ashwin Sekhar T K <asek...@marvell.com>
> > > > > ---
> > > > >  drivers/crypto/ipsec_mb/ipsec_mb_private.c | 2 ++
> > > > >  1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/crypto/ipsec_mb/ipsec_mb_private.c
> > > > > b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
> > > > > index aab42c360c..9ea1110aaf 100644
> > > > > --- a/drivers/crypto/ipsec_mb/ipsec_mb_private.c
> > > > > +++ b/drivers/crypto/ipsec_mb/ipsec_mb_private.c
> > > > > @@ -53,6 +53,7 @@ ipsec_mb_create(struct rte_vdev_device *vdev,
> > > > >       const char *name, *args;
> > > > >       int retval;
> > > > >
> > > > > +#ifdef RTE_ARCH_X86_64
> > > > >       if (vector_mode == IPSEC_MB_NOT_SUPPORTED) {
> > > > >               /* Check CPU for supported vector instruction set */
> > > > >               if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX512F))
> > > > > @@ -64,6 +65,7 @@ ipsec_mb_create(struct rte_vdev_device *vdev,
> > > > >               else
> > > > >                       vector_mode = IPSEC_MB_SSE;
> > > > >       }
> > > > > +#endif
> > > > >
> > > > >       init_params.private_data_size = sizeof(struct
> > > > ipsec_mb_dev_private) +
> > > > >               pmd_data->internals_priv_size;
> > > > > --
> > > > > 2.25.1

Reply via email to