Re: [dpdk-dev] [PATCH] bpf: fix to allow ptr stack program type

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

> > Hi Jerin,
> 
> Hi Konstantin,
> 
> >
> > >
> > > bpf_validate does not allow to execute RTE_BPF_ARG_PTR_STACK for no
> > > reason.
> >
> > I believe there is a reason,
> > ARG_PTR_STACK is reserved for memory within BPF program internal stack
> > only.
> > User that calls BPF program should never pass parameter with that type.
> 
> OK.
> Shouldn't we remove that from public header file
> (lib/librte_bpf/rte_bpf.h) then ?

Probably... or might be just put extra comments to mark it as internal?
The reason I left it here, so we can add new public values for enum here,
before RTE_BPF_ARG_PTR_STACK.
Of course in theory we can use for RTE_BPF_ARG_PTR_STACK some reserved
value instead.

> 
> > If the user allocates parameter for bpf function on the stack, he can still use
> > RTE_BPF_ARG_PTR for it.
> 
> I see the _stack_ is only allocated under RTE_BPF_ARG_PTR_STACK checks in bpf_validate.c.
> Can you check? I agree that stack should be allocated for RTE_BPF_ARG_PTR as well.

Not sure I understand your query here...
Each BPF program can use up to MAX_BPF_STACK_SIZE bytes for stack.
Though to avoid JIT to allocate unused space for the stack, in bpf_validate.c
we figure out does given BPF program really allocate things on the stack and if yes,
how many bytes is needed.
This info is stored in rte_bpf.stack_sz and can be used later by the JIT.
Let say for x86 jit is used in  emit_prolog().

> 
> I am writing the arm64 JIT support now, I see always stack of size of 0. I did not spend  much
> Time on the generic piece of ebpf code(Focusing only on JIT side now).
> 
> Can you share more detail the stack allocation scheme, Is validate code parse the eBPF opcode and
> Figure out the stack depth it by its own and pass to JIT function where Arch code can allocate
> enough stack.

Yep, see above.
Konstantin


> 
> 
> >
> > > Fix it by enhancing the prog_arg.type check.
> > >
> > > Fixes: 6e12ec4c4d6d ("bpf: add more checks")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Jerin Jacob <jerinj@marvell.com>
> > > ---
> > >  lib/librte_bpf/bpf_validate.c | 1 +
> > >  1 file changed, 1 insertion(+)
> > >
> > > diff --git a/lib/librte_bpf/bpf_validate.c
> > > b/lib/librte_bpf/bpf_validate.c index 0cf41fa27..c75777b6d 100644
> > > --- a/lib/librte_bpf/bpf_validate.c
> > > +++ b/lib/librte_bpf/bpf_validate.c
> > > @@ -2216,6 +2216,7 @@ bpf_validate(struct rte_bpf *bpf)
> > >
> > >  	/* check input argument type, don't allow mbuf ptr on 32-bit */
> > >  	if (bpf->prm.prog_arg.type != RTE_BPF_ARG_RAW &&
> > > +			bpf->prm.prog_arg.type !=
> > RTE_BPF_ARG_PTR_STACK &&
> > >  			bpf->prm.prog_arg.type != RTE_BPF_ARG_PTR &&
> > >  			(sizeof(uint64_t) != sizeof(uintptr_t) ||
> > >  			bpf->prm.prog_arg.type !=
> > RTE_BPF_ARG_PTR_MBUF)) {
> > > --
> > > 2.22.0