Re: [dpdk-dev] [PATCH v5 4/5] lib/bpf: use wait event scheme for Rx/Tx iteration

["Ananyev, Konstantin" <konstantin.ananyev@intel.com>]

Hi Feifei,

> > Instead of polling for cbi->use to be updated, use wait event scheme.
> >
> > Furthermore, delete 'const' for 'bpf_eth_cbi_wait'. This is because of a
> > compilation error:
> > -----------------------------------------------------------------------
> > ../lib/eal/include/rte_common.h:36:13: error: read-only variable ‘value’
> > used as ‘asm’ output
> >    36 | #define asm __asm__
> >       |             ^~~~~~~
> >
> > ../lib/eal/arm/include/rte_pause_64.h:66:3: note: in expansion of macro ‘asm’
> >    66 |   asm volatile("ldaxr %w[tmp], [%x[addr]]" \
> >       |   ^~~
> >
> > ../lib/eal/arm/include/rte_pause_64.h:96:3: note: in expansion of macro
> > ‘__LOAD_EXC_32’
> >    96 |   __LOAD_EXC_32((src), dst, memorder)     \
> >       |   ^~~~~~~~~~~~~
> >
> > ../lib/eal/arm/include/rte_pause_64.h:167:4: note: in expansion of macro
> > ‘__LOAD_EXC’
> >   167 |    __LOAD_EXC((addr), value, memorder, size) \
> >       |    ^~~~~~~~~~
> >
> > ../lib/bpf/bpf_pkt.c:125:3: note: in expansion of macro ‘rte_wait_event’
> >   125 |   rte_wait_event(&cbi->use, UINT32_MAX, ==, puse,
> > -----------------------------------------------------------------------
> >
> > Signed-off-by: Feifei Wang <feifei.wang2@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > ---
> >  lib/bpf/bpf_pkt.c | 11 ++++-------
> >  1 file changed, 4 insertions(+), 7 deletions(-)
> >
> > diff --git a/lib/bpf/bpf_pkt.c b/lib/bpf/bpf_pkt.c index
> > 6e8248f0d6..213d44a75a 100644
> > --- a/lib/bpf/bpf_pkt.c
> > +++ b/lib/bpf/bpf_pkt.c
> > @@ -111,9 +111,9 @@ bpf_eth_cbi_unuse(struct bpf_eth_cbi *cbi)
> >   * Waits till datapath finished using given callback.
> >   */
> >  static void
> > -bpf_eth_cbi_wait(const struct bpf_eth_cbi *cbi)
> > +bpf_eth_cbi_wait(struct bpf_eth_cbi *cbi)	
> 
> Hi, Konstantin
> 
> For this bpf patch, I delete 'const' through this is contrary to what we
> discussed earlier. This is because if  we keep 'constant' here and use 'rte_wait_event'
> new macro, compiler will report error. And earlier the arm version cannot be compiled
> due to I forgot enable "wfe" config in the meson file, so this issue can not happen before.


Honestly, I don't understand why we have to remove perfectly valid 'const' qualifier here.
If this macro can't be used with pointers to const (still don't understand why),
then let's just not use this macro here.
Strictly speaking I don't see much benefit here from it.

> 
> >  {
> > -	uint32_t nuse, puse;
> > +	uint32_t puse;
> >
> >  	/* make sure all previous loads and stores are completed */
> >  	rte_smp_mb();
> > @@ -122,11 +122,8 @@ bpf_eth_cbi_wait(const struct bpf_eth_cbi *cbi)
> >
> >  	/* in use, busy wait till current RX/TX iteration is finished */
> >  	if ((puse & BPF_ETH_CBI_INUSE) != 0) {
> > -		do {
> > -			rte_pause();
> > -			rte_compiler_barrier();
> > -			nuse = cbi->use;
> > -		} while (nuse == puse);
> > +		rte_wait_event(&cbi->use, UINT32_MAX, ==, puse,
> > +				__ATOMIC_RELAXED);
> >  	}
> >  }
> >
> > --
> > 2.25.1