RE: [PATCH v5] net: add thread-safe crc api

["Kusztal, ArkadiuszX" <arkadiuszx.kusztal@intel.com>]

> -----Original Message-----
> From: Stephen Hemminger <stephen@networkplumber.org>
> Sent: Monday, February 10, 2025 8:57 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Cc: dev@dpdk.org; ferruh.yigit@amd.com; Ji, Kai <kai.ji@intel.com>; Dooley,
> Brian <brian.dooley@intel.com>
> Subject: Re: [PATCH v5] net: add thread-safe crc api
> 
> On Fri,  7 Feb 2025 18:24:43 +0000
> Arkadiusz Kusztal <arkadiuszx.kusztal@intel.com> wrote:
> 
> > +static struct
> > +{
> > +	rte_net_crc_handler f[RTE_NET_CRC_REQS]; }
> > +handlers_dpdk26[RTE_NET_CRC_AVX512 + 1];
> > +
> 
> Should have { after the struct
+1
> 
> +void rte_net_crc_free(struct rte_net_crc *crc) {
> +	rte_free(crc);
> +}
> 
> Add rte_net_crc_free to the coccinelle script that checks for null free.
Added in v6.
> Also add function attributes please.
Not sure about this one, other `free` functions are not declared with any attributes.
> 
> > diff --git a/lib/net/rte_net_crc.h b/lib/net/rte_net_crc.h index
> > 72d3e10ff6..ffac8c2f1f 100644
> > --- a/lib/net/rte_net_crc.h
> > +++ b/lib/net/rte_net_crc.h
> > @@ -1,5 +1,5 @@
> >  /* SPDX-License-Identifier: BSD-3-Clause
> > - * Copyright(c) 2017-2020 Intel Corporation
> > + * Copyright(c) 2017-2025 Intel Corporation
> >   */
> 
> Not sure what DPDK policy is around copyright date updates.
> The Linux kernel has said no to this (excess churn).
I reverted those changes in v6.
> 
> > +struct rte_net_crc *rte_net_crc_set_alg_v26(enum rte_net_crc_alg alg,
> > +	enum rte_net_crc_type type)
> 
> Since this now an allocator should add function attributes __rte_malloc and
> __rte_dealloc(rte_crc_free, 1).
Added in v6.
> 
> 
> > +{
> > +	uint16_t max_simd_bitwidth;
> > +	struct rte_net_crc *crc;
> > +
> > +	crc = rte_zmalloc(NULL, sizeof(struct rte_net_crc), 0);
> > +	if (crc == NULL)
> > +		return NULL;
> 
> Are you using rte_malloc here because it can be shared between
> primary/secondary?
Yes, this pointer may be referenced by another process, so memory it points to must be available to other processes.
> Otherwise regular malloc has more checks.