From mboxrd@z Thu Jan 1 00:00:00 1970 From: Olivier Matz Subject: Re: [PATCH v8 1/2] librte_net: add crc compute APIs Date: Thu, 30 Mar 2017 16:40:48 +0200 Message-ID: <20170330164048.1c92bf81@platinum> References: <1490807704-211859-2-git-send-email-jasvinder.singh@intel.com> <1490873422-13734-1-git-send-email-jasvinder.singh@intel.com> <1490873422-13734-2-git-send-email-jasvinder.singh@intel.com> <2601191342CEEE43887BDE71AB9772583FAE243B@IRSMSX109.ger.corp.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable Cc: "Singh, Jasvinder" , "dev@dpdk.org" , "Doherty, Declan" , "De Lara Guarch, Pablo" To: "Ananyev, Konstantin" Return-path: Received: from mail-wr0-f182.google.com (mail-wr0-f182.google.com [209.85.128.182]) by dpdk.org (Postfix) with ESMTP id C9EF1D326 for ; Thu, 30 Mar 2017 16:40:51 +0200 (CEST) Received: by mail-wr0-f182.google.com with SMTP id w11so62575865wrc.3 for ; Thu, 30 Mar 2017 07:40:51 -0700 (PDT) In-Reply-To: <2601191342CEEE43887BDE71AB9772583FAE243B@IRSMSX109.ger.corp.intel.com> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Jasvinder, On Thu, 30 Mar 2017 11:31:54 +0000, "Ananyev, Konstantin" wrote: > Hi Jasvinder, >=20 > > diff --git a/lib/librte_net/rte_net_crc.h b/lib/librte_net/rte_net_crc.h > > new file mode 100644 > > index 0000000..dd6c110 > > --- /dev/null > > +++ b/lib/librte_net/rte_net_crc.h > > @@ -0,0 +1,104 @@ > > +/*- > > + * BSD LICENSE > > + * > > + * Copyright(c) 2017 Intel Corporation. > > + * All rights reserved. > > + * > > + * Redistribution and use in source and binary forms, with or without > > + * modification, are permitted provided that the following conditions > > + * are met: > > + * > > + * * Redistributions of source code must retain the above copyright > > + * notice, this list of conditions and the following disclaimer. > > + * * Redistributions in binary form must reproduce the above copyr= ight > > + * notice, this list of conditions and the following disclaimer = in > > + * the documentation and/or other materials provided with the > > + * distribution. > > + * * Neither the name of Intel Corporation nor the names of its > > + * contributors may be used to endorse or promote products deriv= ed > > + * from this software without specific prior written permission. > > + * > > + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTO= RS > > + * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT > > + * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS= FOR > > + * A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRI= GHT > > + * OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDEN= TAL, > > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT > > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF = USE, > > + * DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON= ANY > > + * THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TO= RT > > + * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE= USE > > + * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMA= GE. > > + */ > > + > > +#ifndef _RTE_NET_CRC_H_ > > +#define _RTE_NET_CRC_H_ > > + > > +#ifdef __cplusplus > > +extern "C" { > > +#endif > > + > > +#include > > + > > +#include =20 >=20 > As a nit: you probably don't need that include. > Konstantin >=20 > > + > > +/** CRC polynomials */ > > +#define CRC32_ETH_POLYNOMIAL 0x04c11db7UL > > +#define CRC16_CCITT_POLYNOMIAL 0x1021U > > + > > +#define CRC_LUT_SIZE 256 > > + > > +/** CRC types */ > > +enum rte_net_crc_type { > > + RTE_NET_CRC16_CCITT =3D 0, > > + RTE_NET_CRC32_ETH, > > + RTE_NET_CRC_REQS > > +}; > > + > > +/** CRC compute algorithm */ > > +enum rte_net_crc_alg { > > + RTE_NET_CRC_SCALAR =3D 0, > > + RTE_NET_CRC_SSE42, > > +}; > > + > > +/** > > + * This API set the CRC computation algorithm (i.e. scalar version, > > + * x86 64-bit sse4.2 intrinsic version, etc.) and internal data > > + * structure. > > + * > > + * @param alg > > + * This parameter is used to select the CRC implementation version. > > + * - RTE_NET_CRC_SCALAR > > + * - RTE_NET_CRC_SSE42 (Use 64-bit SSE4.2 intrinsic) > > + */ > > +void > > +rte_net_crc_set_alg(enum rte_net_crc_alg alg); > > + > > +/** > > + * CRC compute API > > + * > > + * @param data > > + * Pointer to the packet data for CRC computation > > + * @param data_len > > + * Data length for CRC computation > > + * @param type > > + * CRC type (enum rte_net_crc_type) > > + * > > + * @return > > + * CRC value > > + */ > > +uint32_t > > +rte_net_crc_calc(const void *data, > > + uint32_t data_len, > > + enum rte_net_crc_type type); > > + > > +#if defined(RTE_ARCH_X86_64) && defined(RTE_MACHINE_CPUFLAG_SSE4_2) > > +#include > > +#endif I think this include should not be included from rte_net_crc.h. =46rom what I see, the API is the same for sse and non-sse, so this include could be private, included only from the .c file. If you also remove the include to rte_mbuf.h as suggested by Konstantin, it will require the following includes in rte_net_crc.c: #include #include =20 #include #include #include #include #include #if defined(RTE_ARCH_X86_64) && defined(RTE_MACHINE_CPUFLAG_SSE4_2) #include #endif If the sse file is only used in the .c, this line could also be removed in the Makefile: SYMLINK-$(CONFIG_RTE_LIBRTE_NET)-include +=3D rte_net_crc_sse.h I'm not very familiar with crc and sse code. Could you add yourself as maintainer for these files in MAINTAINERS? Thanks Olivier