> From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Ananyev, > Konstantin > Sent: Wednesday, February 5, 2020 10:16 PM > > > > > As I mentioned in my FOSDEM talk the current DPDK eBPF handling is > > not usable for packet filters. I have ported the classic BPF to eBPF > code > > and the generated code is not usable by DPDK. > > > > The problem is that DPDK eBPF does not implement all the opcodes. > > BPF_ABS is not implemented and must be. It is in the Linux kernel. > > Yep, it doesn't. > This is not a generic eBPF instruction, but sort of implicit function > call > to access skb data. At initial stage of librte_bpf development, > I didn't think much about cBPF conversion, and to simplify things > decided to put all special skb features aside. > But sure, if that will enable DPDK cBPF support, let's add it in. > Please fill a Bugzilla ticket to track that issue, and I'll try to > find some time within 20.05 to look at it. > Unless of course, you or someone else would like to volunteer for it. > > Though at first step, we probably need to decide what should be > our requirements for it in terms of DPDK. > From https://www.kernel.org/doc/Documentation/networking/filter.txt: > "eBPF has two non-generic instructions: (BPF_ABS | <size> | BPF_LD) and > (BPF_IND | <size> | BPF_LD) which are used to access packet data. > They had to be carried over from classic to have strong performance of > socket filters running in eBPF interpreter. These instructions can only > be used when interpreter context is a pointer to 'struct sk_buff' and > have seven implicit operands. Register R6 is an implicit input that > must > contain pointer to sk_buff. Register R0 is an implicit output which > contains > the data fetched from the packet. Registers R1-R5 are scratch registers > and must not be used to store the data across BPF_ABS | BPF_LD or > BPF_IND | BPF_LD instructions. > These instructions have implicit program exit condition as well. When > eBPF program is trying to access the data beyond the packet boundary, > the interpreter will abort the execution of the program. JIT compilers > therefore must preserve this property. src_reg and imm32 fields are > explicit inputs to these instructions. > For example: > BPF_IND | BPF_W | BPF_LD means: > R0 = ntohl(*(u32 *) (((struct sk_buff *) R6)->data + src_reg + imm32)) > and R1 - R5 were scratched." > > For RTE_BPF_ARG_PTR_MBUF context we probably want behavior similar > to linux, i.e. BPF_IND | BPF_W | BPF_LD would mean something like: > > 1) uint32_t tmp; > R0 = &tmp; > R0 = rte_pktmbuf_read((const struct rte_mbuf *)R6, src_reg + > imm32, > sizeof(uint32_t), R0); > if (R0 == NULL) return FAILED; > R0 = ntohl(*(uint32_t *)R0); > > But what to do with when ctx is raw data buffer (RTE_BPF_ARG_PTR)? > Should it be just: > 2) R0 = ntohl(*(uint32_t *)(R6 + src_reg + imm32)); > 3) not allow LD_ABS/LD_IND in this mode at all. >
I think that 1) is the correct choice for the "cBPF filter" use case. In that context I consider both 2) and 3) irrelevant because the RTE_BPF_ARG_PTR_MBUF type should be used for cBPF filtering. I have argued for this before. Others have argued for using the RTE_BPF_ARG_PTR type instead. Let's consider using the RTE_BPF_ARG_PTR for a moment... Is there an implicit understanding that the data points to packet data? Then a range check in 2) might be relevant. However, if the RTE_BPF_ARG_PTR_MBUF type is supported and used for cBPF->eBPF conversion, we would not need to support LD_ABS/LD_IND for the RTE_BPF_ARG_PTR type. So between 2) and 3), I support 3). > Second question is implementation. > I can see two main options here: > a) if we plan to have our own cBPF->eBPF converter and support only it, > we can add these extra instructions generation into converter itself. > I.E. cBPF->eBPF conversion for LD_ABS/LD_IND will generate series > of generic eBPF instructions. > b) support eBPF LD_ABS/LD_IND in eBPF interpreter/jit > > (a) probably a simpler way (eBPF interpreter/jit/verifier would remain > unchanged), > but seems way too limited. So I think (b) is a better choice, even more > work implied > (interpreter seems more or less straightforward, jit would probably > need some effort). > This is going to be used in the fast path, probably on all packets on an interface. So clearly b). > Any thoughts/opinions? > Konstantin One more piece of information: Linux cBPF supports Auxiliary data (VLAN ID, Interface Index, etc.), i.e. metadata that are not part of the packet data, but can be found in the sk_buff/mbuf. Going for 1) and b) might make it easier adding support for these later. Med venlig hilsen / kind regards - Morten Brørup
