Re: [RFC PATCH v2 2/5] ebpf: Added eBPF RSS program.

[Jason Wang <jasowang@redhat.com>]

On 2020/11/19 下午7:13, Andrew Melnychenko wrote:
> From: Andrew<andrew@daynix.com>
>
> RSS program and Makefile to build it.
> Also, added a python script that would generate '.h' file.
> The data in that file may be loaded by libbpf.
> EBPF compilation is not required for building qemu.
> You can use Makefile if you need to regenerate tun_rss_steering.h.
>
> NOTE: BPF program can't be loaded without debug/btf info.
> Which brings to huge tun_rss_steering.h file.
> In future, need to find proper way to shrink the file
> and leave only the program data and btf info.
>
> Signed-off-by: Yuri Benditovich<yuri.benditovich@daynix.com>
> Signed-off-by: Andrew Melnychenko<andrew@daynix.com>
> ---
>   ebpf/EbpfElf_to_C.py    |   36 +
>   ebpf/Makefile.ebpf      |   33 +
>   ebpf/rss.bpf.c          |  505 ++++


I think it's better to place them under tools/


>   ebpf/tun_rss_steering.h | 5439 +++++++++++++++++++++++++++++++++++++++


And move this to net/


>   4 files changed, 6013 insertions(+)
>   create mode 100644 ebpf/EbpfElf_to_C.py
>   create mode 100755 ebpf/Makefile.ebpf
>   create mode 100644 ebpf/rss.bpf.c
>   create mode 100644 ebpf/tun_rss_steering.h
>
> diff --git a/ebpf/EbpfElf_to_C.py b/ebpf/EbpfElf_to_C.py
> new file mode 100644
> index 0000000000..3a1b01aedc
> --- /dev/null
> +++ b/ebpf/EbpfElf_to_C.py
> @@ -0,0 +1,36 @@
> +#!/usr/bin/python3


Missing license.


> +
> +import sys
> +import argparse
> +
> +def process_file(filename, prog_name):
> +    print('Processing file:', filename)
> +    with open(filename, 'rb') as f:
> +        with open("%s.h" % prog_name, 'w') as w:
> +
> +            w.write('#ifndef %s\n' % prog_name.upper())
> +            w.write('#define %s\n\n' % prog_name.upper())
> +
> +            w.write("uint8_t data_%s[] = {\n" % prog_name)
> +
> +            data = f.read(8)
> +            while data:
> +                w.write("    " + ", ".join("0x%02x" % x for x in data) + ",\n")
> +                data = f.read(8)
> +
> +            w.write('};\n\n')
> +
> +            w.write('#endif /* %s */\n' % prog_name.upper())
> +
> +    return 0
> +
> +if __name__ == '__main__':
> +    parser = argparse.ArgumentParser(
> +        description='Convert eBPF ELF to C header. '
> +                    'Section name will be used in C namings.')
> +    parser.add_argument('--file', '-f', nargs=1, required=True,
> +                        help='eBPF ELF file')
> +    parser.add_argument('--section', '-s', nargs=1, required=True,
> +                        help='section in ELF with eBPF program.')
> +    args = parser.parse_args()
> +    sys.exit(process_file(args.file[0], args.section[0]))
> diff --git a/ebpf/Makefile.ebpf b/ebpf/Makefile.ebpf
> new file mode 100755
> index 0000000000..c09a8ac543
> --- /dev/null
> +++ b/ebpf/Makefile.ebpf
> @@ -0,0 +1,33 @@
> +OBJS = rss.bpf.o
> +
> +LLC ?= llc
> +CLANG ?= clang
> +INC_FLAGS = -nostdinc -isystem `$(CLANG) -print-file-name=include`
> +EXTRA_CFLAGS ?= -O2 -g -emit-llvm


Do we need a check of clang version to make sure BTF can be generated 
with "-g"?


> +
> +linuxhdrs = ~/src/kernel/master
> +
> +LINUXINCLUDE =  -I $(linuxhdrs)/arch/x86/include/uapi \
> +                -I $(linuxhdrs)/arch/x86/include/generated/uapi \
> +                -I $(linuxhdrs)/arch/x86/include/generated \
> +                -I $(linuxhdrs)/include/generated/uapi \
> +                -I $(linuxhdrs)/include/uapi \
> +                -I $(linuxhdrs)/include \
> +                -I $(linuxhdrs)/tools/lib
> +
> +all: $(OBJS)
> +
> +.PHONY: clean
> +
> +clean:
> +	rm -f $(OBJS)
> +
> +INC_FLAGS = -nostdinc -isystem `$(CLANG) -print-file-name=include`
> +
> +$(OBJS):  %.o:%.c
> +	$(CLANG) $(INC_FLAGS) \
> +                -D__KERNEL__ -D__ASM_SYSREG_H \
> +                -I../include $(LINUXINCLUDE) \
> +                $(EXTRA_CFLAGS) -c $< -o -| $(LLC) -march=bpf -filetype=obj -o $@
> +	python3 EbpfElf_to_C.py -f rss.bpf.o -s tun_rss_steering
> +
> diff --git a/ebpf/rss.bpf.c b/ebpf/rss.bpf.c
> new file mode 100644
> index 0000000000..3416bc72d0
> --- /dev/null
> +++ b/ebpf/rss.bpf.c
> @@ -0,0 +1,505 @@
> +/*
> + * eBPF RSS program
> + *
> + * Developed by Daynix Computing LTD (http://www.daynix.com)
> + *
> + * Authors:
> + *  Andrew Melnychenko<andrew@daynix.com>
> + *  Yuri Benditovich<yuri.benditovich@daynix.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2.  See
> + * the COPYING file in the top-level directory.
> + */
> +
> +#include <stddef.h>
> +#include <stdbool.h>
> +#include <linux/bpf.h>
> +
> +#include <linux/in.h>
> +#include <linux/if_ether.h>
> +#include <linux/ip.h>
> +#include <linux/ipv6.h>
> +
> +#include <linux/udp.h>
> +#include <linux/tcp.h>
> +
> +#include <bpf/bpf_helpers.h>
> +#include <linux/virtio_net.h>
> +
> +/*
> + * Prepare:
> + * Requires llvm, clang, python3, linux kernel tree
> + *
> + * Build tun_rss_steering.h:
> + * make -f Mefile.ebpf clean all
> + */


It's better to merge those instructions with the comments at the beginning.


> +
> +#define INDIRECTION_TABLE_SIZE 128
> +#define HASH_CALCULATION_BUFFER_SIZE 36
> +
> +struct rss_config_t {
> +    __u8 redirect;
> +    __u8 populate_hash;
> +    __u32 hash_types;
> +    __u16 indirections_len;
> +    __u16 default_queue;
> +};
> +
> +struct toeplitz_key_data_t {
> +    __u32 leftmost_32_bits;
> +    __u8 next_byte[HASH_CALCULATION_BUFFER_SIZE];
> +};
> +


[...]


> +
> +SEC("tun_rss_steering")
> +int tun_rss_steering_prog(struct __sk_buff *skb)
> +{
> +
> +    struct rss_config_t *config = 0;
> +    struct toeplitz_key_data_t *toe = 0;


There's no need to initialize the above two.

Thanks