> -----Original Message----- > From: David Marchand <david.march...@redhat.com> > Sent: Wednesday, October 2, 2024 9:42 AM > To: Kusztal, ArkadiuszX <arkadiuszx.kusz...@intel.com> > Cc: dev@dpdk.org; ferruh.yi...@amd.com; Ji, Kai <kai...@intel.com>; Dooley, > Brian <brian.doo...@intel.com> > Subject: Re: [PATCH v2 1/3] net: add thread-safe crc api > > On Tue, Oct 1, 2024 at 9:27 PM Arkadiusz Kusztal > <arkadiuszx.kusz...@intel.com> wrote: > > > > The current net CRC API is not thread-safe, this patch solves this by > > adding another, thread-safe API functions. > > This API is also safe to use across multiple processes, yet with > > limitations on max-simd-bitwidth, which will be checked only by the > > process that created the CRC context; all other processes will use the > > same CRC function when used with the same CRC context. > > It is an undefined behavior when process binaries are compiled with > > different SIMD capabilities when the same CRC context is used. > > > > Signed-off-by: Arkadiusz Kusztal <arkadiuszx.kusz...@intel.com> > > I am lost... do you mean thread-safe ? or DPDK multi process safe?
In this case both. > > For now, I don't see why we need a new API (and especially why we should > break the existing one..). In the current one, there exists a race condition that causes multi-threaded applications to crash; currently only QAT PMD is using it. But I know it is late for such a change, so I probably should defer it. We can manage to fix it in a different way. > > > > --- > > lib/net/net_crc.h | 19 ++-- > > lib/net/rte_net_crc.c | 309 > > +++++++++++++++----------------------------------- > > lib/net/rte_net_crc.h | 40 ++----- > > lib/net/version.map | 18 ++- > > 4 files changed, 124 insertions(+), 262 deletions(-) > > > > diff --git a/lib/net/net_crc.h b/lib/net/net_crc.h index > > 7a74d5406c..d220200685 100644 > > --- a/lib/net/net_crc.h > > +++ b/lib/net/net_crc.h > > @@ -5,40 +5,41 @@ > > #ifndef _NET_CRC_H_ > > #define _NET_CRC_H_ > > > > -/* > > - * Different implementations of CRC > > - */ > > - > > -/* SSE4.2 */ > > +#include <rte_compat.h> > > > > +__rte_internal > > void > > rte_net_crc_sse42_init(void); > > > > +__rte_internal > > uint32_t > > rte_crc16_ccitt_sse42_handler(const uint8_t *data, uint32_t > > data_len); > > > > +__rte_internal > > uint32_t > > rte_crc32_eth_sse42_handler(const uint8_t *data, uint32_t data_len); > > > > -/* AVX512 */ > > - > > +__rte_internal > > void > > rte_net_crc_avx512_init(void); > > > > +__rte_internal > > uint32_t > > rte_crc16_ccitt_avx512_handler(const uint8_t *data, uint32_t > > data_len); > > > > +__rte_internal > > uint32_t > > rte_crc32_eth_avx512_handler(const uint8_t *data, uint32_t data_len); > > > > -/* NEON */ > > - > > +__rte_internal > > void > > rte_net_crc_neon_init(void); > > > > +__rte_internal > > uint32_t > > rte_crc16_ccitt_neon_handler(const uint8_t *data, uint32_t data_len); > > > > +__rte_internal > > uint32_t > > rte_crc32_eth_neon_handler(const uint8_t *data, uint32_t data_len); > > Exporting internals but not using them out of the library makes no sense. > So there must have been a misunderstanding on my part, the initial idea was to prevent the user from calling these functions. > > -- > David Marchand
