[ovs-dev] [PATCH v3 4/7] dpcls: enable cpu feature detection
Van Haaren, Harry
harry.van.haaren at intel.com
Wed Jun 17 17:06:34 UTC 2020
> -----Original Message-----
> From: William Tu <u9012063 at gmail.com>
> Sent: Tuesday, June 16, 2020 4:41 PM
> To: Van Haaren, Harry <harry.van.haaren at intel.com>
> Cc: ovs-dev <ovs-dev at openvswitch.org>; Stokes, Ian <ian.stokes at intel.com>;
> Ilya Maximets <i.maximets at ovn.org>; Federico Iezzi <fiezzi at redhat.com>
> Subject: Re: [PATCH v3 4/7] dpcls: enable cpu feature detection
>
> btw, remember to add "." at the end of the commit title.
> so
> "dpcls: enable cpu feature detection."
Ah thanks, will do.
<snip patch details>
> > + CHECK_CPU_FEATURE(feature, "avx512f", RTE_CPUFLAG_AVX512F);
> > + CHECK_CPU_FEATURE(feature, "bmi2", RTE_CPUFLAG_BMI2);
>
> why are "avx512f" and "bmi2" hard-coded here?
> I thought this function "dpdk_get_cpu_has_isa" allows you to check any
> cpu feature.
The strings are hard-coded here to abstract the OVS APIs from DPDK rte_cpu_flag_t.
Indeed the implementation here can be extended to check any CPU flags, but would
require some additional CHECK_CPU_FEATURE() macros. The number of ISA combinations
actually used is generally limited to < 10, so I see no issue with implementing as individual checks.
It is possible to change the API to directly use RTE_CPUFLAG_*, however I'm not sure
that's a clean API. DPDK uses an "__extension__ enum rte_cpu_flag_t" to define CPU flags,
which feels like it would better be contained in a .c file inside OVS.
To contain the rte_cpu_flag_t in a .c I wrapped it into a "string arch, string cpu_feature" combo,
which I believe is much more flexible, and a more generic API for OVS.
/* An alternative API, but IMO it is a worse solution due to DPDK datatype in an OVS API. */
int dpdk_get_cpu_has_isa(const char *arch, enum rte_cpu_flag_t cpuflag);
More information about the dev
mailing list