[dpdk-dev] [PATCH v8 1/2] librte_net: add crc compute APIs

Singh, Jasvinder jasvinder.singh at intel.com
Thu Mar 30 17:14:21 CEST 2017


Hi Olivier,

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz at 6wind.com]
> Sent: Thursday, March 30, 2017 3:41 PM
> To: Ananyev, Konstantin <konstantin.ananyev at intel.com>
> Cc: Singh, Jasvinder <jasvinder.singh at intel.com>; dev at dpdk.org; Doherty,
> Declan <declan.doherty at intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch at intel.com>
> Subject: Re: [dpdk-dev] [PATCH v8 1/2] librte_net: add crc compute APIs


<snip>

> I think this include should not be included from rte_net_crc.h.
> 
> From 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 <stddef.h>
>  #include <string.h>
> 
>  #include <rte_common.h>
>  #include <rte_cpuflags.h>
>  #include <rte_branch_prediction.h>
>  #include <rte_vect.h>
>  #include <rte_net_crc.h>
>  #if defined(RTE_ARCH_X86_64) &&
> defined(RTE_MACHINE_CPUFLAG_SSE4_2)
>  #include <rte_net_crc_sse.h>
>  #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 += 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

Thank you for the review. I will make above suggested changes in the next version. 

Jasvinder


More information about the dev mailing list