Hi Jerin,
> > >
> > > > +/*
> > > > + * Marks given callback as used by datapath.
> > > > + */
> > > > +static __rte_always_inline void
> > > > +bpf_eth_cbi_inuse(struct bpf_eth_cbi *cbi)
> > > > +{
> > > > + cbi->use++;
> > > > + /* make sure no store/load reordering could happen */
> > > > + rte_smp_mb();
> > > > +}
> > > > +
> > > > +/*
> > > > + * Marks given callback list as not used by datapath.
> > > > + */
> > > > +static __rte_always_inline void
> > > > +bpf_eth_cbi_unuse(struct bpf_eth_cbi *cbi)
> > > > +{
> > > > + /* make sure all previous loads are completed */
> > > > + rte_smp_rmb();
> > >
> > > We earlier discussed this barrier. Will following scheme works out to
> > > fix the bpf_eth_cbi_wait() without cbi->use scheme?
> > >
> > > #ie. We need to exit from jitted or interpreted code irrespective of its
> > > state. IMO, We can do that by an _arch_ specific function to fill jitted
> > > memory with
> > > "exit" opcode(value:0x95, exit, return r0),so that above code needs to be
> > > come out i n anycase,
> > > on next instruction execution. I know, jitted memory is read-only in your
> > > design, I think, we can change the permission to "write" to the fill
> > > "exit" opcode(both jitted or interpreted case) for termination.
> > >
> > > What you think?
> >
> > Not sure I understand your proposal...
>
> If I understand it correctly, bpf_eth_cbi_wait() is used to _wait_ until
> eBPF program exits? Right?
Kind off, but not only.
After bpf_eth_cbi_wait() finishes it is guaranteed that data-path wouldn't try
to access the resources associated with given bpf_eth_cbi (bpf, jit), so we
can proceed with freeing them.
> . Instead of using bpf_eth_cbi_[un]use()
> scheme which involves the barrier. How about,
>
> in bpf_eth_cbi_wait()
> {
>
> memset the EBPF "program memory" with 0x95 value. Which is an "exit" and
> "return r0" EPBF opcode, Which makes program to terminate by it own
> as on 0x95 instruction, CPU decodes and it gets out from EPBF program.
>
> }
>
> In jitted case, it is not 0x95 instruction, which will be an arch
> specific instructions, We can have arch abstraction to generated
> such instruction for "exit" opcode. And use common code to fill the
> instructions
> to exit from EPBF program provided by arch code.
>
> Does that makes sense?
There is no much point in doing it.
What we need is a guarantee that after some point data-path wouldn't try to
access
given bpf context, so we can destroy it.
Konstantin
>
>
> > Are you suggesting to change bpf_exec() and bpf_jit() to make them execute
> > sync primitives in an arch specific manner?
> > But some users probably will use bpf_exec/jitted program in the environment
> > that wouldn't require such synchronization.
> > For these people it would be just unnecessary slowdown.
> >
> > If you are looking for a ways to replace 'smp_rmb' in bpf_eth_cbi_unuse()
> > with something arch specific, then
> > I can make cbi_inuse/cbi_unuse - arch specific with keeping current
> > implementation as generic one.
> > Would that help?
> >
> > Konstantin
> >
> > >
> > > > + cbi->use++;
> > > > +}
> > > > +
> > > > +/*
> > > > + * Waits till datapath finished using given callback.
> > > > + */
> > > > +static void
> > > > +bpf_eth_cbi_wait(const struct bpf_eth_cbi *cbi)
> > > > +{
> > > > + uint32_t nuse, puse;
> > > > +
> > > > + /* make sure all previous loads and stores are completed */
> > > > + rte_smp_mb();
> > > > +
> > > > + puse = cbi->use;
> > > > +
> > > > + /* 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);
> > > > + }
> > > > +}
