Hi, > -----Original Message----- > From: dev <[email protected]> On Behalf Of Pavan Nikhilesh > Bhagavatula > Sent: Tuesday, April 23, 2019 17:13 > To: [email protected] > Cc: [email protected]; [email protected]; Marko Kovacevic > <[email protected]>; Ori Kam <[email protected]>; Bruce > Richardson <[email protected]>; Pablo de Lara > <[email protected]>; Radu Nicolau <[email protected]>; > [email protected]; Tomasz Kantecki <[email protected]>; > [email protected] > Subject: Re: [dpdk-dev] [PATCH] examples/l3fwd: fix em mode datapath > selection > > > > >-----Original Message----- > >From: dev <[email protected]> On Behalf Of Thomas Monjalon > >Sent: Tuesday, April 23, 2019 2:05 PM > >To: Pavan Nikhilesh Bhagavatula <[email protected]> > >Cc: [email protected]; Jerin Jacob Kollanukkaran <[email protected]>; Marko > >Kovacevic <[email protected]>; Ori Kam <[email protected]>; > >Bruce Richardson <[email protected]>; Pablo de Lara > ><[email protected]>; Radu Nicolau > ><[email protected]>; Akhil Goyal <[email protected]>; Tomasz > >Kantecki <[email protected]>; [email protected] > >Subject: Re: [dpdk-dev] [PATCH] examples/l3fwd: fix em mode datapath > >selection > > > >23/04/2019 04:47, Pavan Nikhilesh Bhagavatula: > >>From: Thomas Monjalon <[email protected]> > >> >10/04/2019 09:29, Pavan Nikhilesh Bhagavatula: > >> >> From: Pavan Nikhilesh <[email protected]> > >> >> > >> >> Currently, l3wfd em mode has two datapath modes em_sequential > and > >> >> em_hlm. We can select either of them by defining > >> >NO_HASH_MULTI_LOOKUP > >> >> to one or zero. > >> >> The code checks if NO_HASH_MULTI_LOOKUP is defined or not > instead > >> >> of checking for the value. > >> >> > >> >> Fixes: 52c97adc1f0f ("examples/l3fwd: fix exact match > >> >> performance") > >> >> Cc: [email protected] > >> >> > >> >> Signed-off-by: Pavan Nikhilesh <[email protected]> > >> >> --- > >> >> --- a/examples/l3fwd/l3fwd_em.c > >> >> +++ b/examples/l3fwd/l3fwd_em.c > >> >> #if defined RTE_ARCH_X86 || defined > RTE_MACHINE_CPUFLAG_NEON > >-#if > >> >> defined(NO_HASH_MULTI_LOOKUP) > >> >> +#if NO_HASH_MULTI_LOOKUP > >> > > >> >A quick grep shows that it used in another place with #ifdef: > >> > > >> >examples/l3fwd/l3fwd.h:#if !defined(NO_HASH_MULTI_LOOKUP) && > >> >defined(RTE_MACHINE_CPUFLAG_NEON) > >> > > >> > > >> > >> #if !defined(NO_HASH_MULTI_LOOKUP) && > >> defined(RTE_MACHINE_CPUFLAG_NEON) #define > >NO_HASH_MULTI_LOOKUP 1 > >> #endif > >> > >> This macro is used to set l3fwd_em_sequential as the default EM > >> datapath on AARCH64 as its performance is better. > >> (http://patches.dpdk.org/patch/49372/) > >> > >> make -C examples/l3fwd #Selects l3fwd_em_sequential by default > on > >AARCH 64 > >> > >> Currently, we cannot select em_hlm without manually editing the macro > >> as using the below command still sets em_sequential as the default > >> datapath because the macro modified in the patch that selects the > >> datapath > >checks if NO_HASH_MULTI_LOOKUP is defined or not rather than its value. > >> > >> EXTRA_CFLAGS='-DNO_HASH_MULTI_LOOKUP=0' make -C > examples/l3fwd > >> > >> I hope I cleared up things a bit. > > > >In my understanding, we should check the value in the other case too, > >instead of #if defined. > > That will lead to undefined and redefined error: > > [dpdk] # make -C examples/l3fwd > make: Entering directory '/root/pavan/dpdk-int/examples/l3fwd' > CC main.o > CC l3fwd_lpm.o > In file included from /root/dpdk/examples/l3fwd/l3fwd_lpm.c:28:0: > /root/dpdk/examples/l3fwd/l3fwd.h:14:6: error: > "NO_HASH_MULTI_LOOKUP" is not defined, evaluates to 0 [-Werror=undef] > #if !NO_HASH_MULTI_LOOKUP && > defined(RTE_MACHINE_CPUFLAG_NEON) > > [dpdk] # EXTRA_CFLAGS='-DNO_HASH_MULTI_LOOKUP=0' make -C > examples/l3fwd > make: Entering directory '/root/dpdk/examples/l3fwd' > CC main.o > ] > #define NO_HASH_MULTI_LOOKUP 1 > n file included from /root/dpdk/examples/l3fwd/l3fwd_lpm.c:28:0: > <command-line>:0:0: note: > > ] #define > NO_HASH_MULTI_LOOKUP 1is the location of the previous definition > > > > >
Actually I looked at this EM lookup mode selection issue as well, and my first solution is the same as yours :) I didn't find out a way to enable selection without using compile options (-DNO_HASH_MULTI_LOOKUP etc.). Maybe we can use both HASH_MULTI_LOOKUP and NO_HASH_MULTI_LOOKUP like below: --- a/examples/l3fwd/l3fwd.h +++ b/examples/l3fwd/l3fwd.h @@ -11,8 +11,8 @@ #define RTE_LOGTYPE_L3FWD RTE_LOGTYPE_USER1 -#if !defined(NO_HASH_MULTI_LOOKUP) && defined(RTE_MACHINE_CPUFLAG_NEON) -#define NO_HASH_MULTI_LOOKUP 1 +#if !defined(HASH_MULTI_LOOKUP) && defined(RTE_MACHINE_CPUFLAG_NEON) +#define NO_HASH_MULTI_LOOKUP #endif Not sure if this is a good idea. PS: if we are controlling selection using compile options, it is better to be documented somewhere.
