> -----Original Message-----
> From: William Tu <[email protected]>
> Sent: Tuesday, June 16, 2020 4:41 PM
> To: Van Haaren, Harry <[email protected]>
> Cc: ovs-dev <[email protected]>; Stokes, Ian <[email protected]>;
> Ilya Maximets <[email protected]>; Federico Iezzi <[email protected]>
> 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);
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to