Re: [PATCH net-next 1/6] bpf: introduce BPF_PROG_TEST_RUN command
On 2017/3/31 11:24, Alexei Starovoitov wrote: On 3/30/17 8:12 PM, Wangnan (F) wrote: On 2017/3/31 10:57, Alexei Starovoitov wrote: On 3/30/17 7:53 PM, Wangnan (F) wrote: I suggest using a CONFIG option to enable/disable code in test_run.o to reduce attack plane. attack plane? what attack do you see and how config helps? I think all testing features are not required to be compiled for a production system. A feature which should never be used looks dangerous to me. It is required on production system, since xdp testing and xdp production has to use the same kernel. We cannot keep rebooting the server back and forth to test and then to run. It's not testing the kernel features, it's testing bpf programs which are technically user space components. Okay. Now I understand it is a production feature. Thank you.
Re: [PATCH net-next 1/6] bpf: introduce BPF_PROG_TEST_RUN command
On 3/30/17 8:12 PM, Wangnan (F) wrote: On 2017/3/31 10:57, Alexei Starovoitov wrote: On 3/30/17 7:53 PM, Wangnan (F) wrote: I suggest using a CONFIG option to enable/disable code in test_run.o to reduce attack plane. attack plane? what attack do you see and how config helps? I think all testing features are not required to be compiled for a production system. A feature which should never be used looks dangerous to me. It is required on production system, since xdp testing and xdp production has to use the same kernel. We cannot keep rebooting the server back and forth to test and then to run. It's not testing the kernel features, it's testing bpf programs which are technically user space components. I suggest adding a CONFIG option like CONFIG_BPF_PROGRAM_TEST_RUN to control whether the kernel should be compiled with this feature or not. We can enable by default, and give people a chance to turn it off. At least in my company people tends to turn all unneeded features off. If you don't provide a config option they will make one by themselves. Using this logic huawei should be turning off xdp as well. Sorry we're not going stub xdp facility out of the core and the drivers just because you don't use it.
Re: [PATCH net-next 1/6] bpf: introduce BPF_PROG_TEST_RUN command
On 2017/3/31 10:57, Alexei Starovoitov wrote: On 3/30/17 7:53 PM, Wangnan (F) wrote: I suggest using a CONFIG option to enable/disable code in test_run.o to reduce attack plane. attack plane? what attack do you see and how config helps? I think all testing features are not required to be compiled for a production system. A feature which should never be used looks dangerous to me. I suggest adding a CONFIG option like CONFIG_BPF_PROGRAM_TEST_RUN to control whether the kernel should be compiled with this feature or not. We can enable by default, and give people a chance to turn it off. At least in my company people tends to turn all unneeded features off. If you don't provide a config option they will make one by themselves. Thank you.
Re: [PATCH net-next 1/6] bpf: introduce BPF_PROG_TEST_RUN command
On 3/30/17 7:53 PM, Wangnan (F) wrote: I suggest using a CONFIG option to enable/disable code in test_run.o to reduce attack plane. attack plane? what attack do you see and how config helps?
Re: [PATCH net-next 1/6] bpf: introduce BPF_PROG_TEST_RUN command
On 2017/3/31 9:31, Alexei Starovoitov wrote: development and testing of networking bpf programs is quite cumbersome. Despite availability of user space bpf interpreters the kernel is the ultimate authority and execution environment. Current test frameworks for TC include creation of netns, veth, qdiscs and use of various packet generators just to test functionality of a bpf program. XDP testing is even more complicated, since qemu needs to be started with gro/gso disabled and precise queue configuration, transferring of xdp program from host into guest, attaching to virtio/eth0 and generating traffic from the host while capturing the results from the guest. Moreover analyzing performance bottlenecks in XDP program is impossible in virtio environment, since cost of running the program is tiny comparing to the overhead of virtio packet processing, so performance testing can only be done on physical nic with another server generating traffic. Furthermore ongoing changes to user space control plane of production applications cannot be run on the test servers leaving bpf programs stubbed out for testing. Last but not least, the upstream llvm changes are validated by the bpf backend testsuite which has no ability to test the code generated. To improve this situation introduce BPF_PROG_TEST_RUN command to test and performance benchmark bpf programs. Joint work with Daniel Borkmann. Signed-off-by: Alexei StarovoitovAcked-by: Daniel Borkmann Acked-by: Martin KaFai Lau --- include/linux/bpf.h | 7 ++ include/uapi/linux/bpf.h | 12 kernel/bpf/syscall.c | 27 +++- net/Makefile | 2 +- net/bpf/Makefile | 1 + net/bpf/test_run.c | 172 +++ net/core/filter.c| 5 ++ 7 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 net/bpf/Makefile create mode 100644 net/bpf/test_run.c [SNIP] diff --git a/net/Makefile b/net/Makefile index 9b681550e3a3..9086ffbb5085 100644 --- a/net/Makefile +++ b/net/Makefile @@ -12,7 +12,7 @@ obj-$(CONFIG_NET) += $(tmp-y) # LLC has to be linked before the files in net/802/ obj-$(CONFIG_LLC) += llc/ -obj-$(CONFIG_NET) += ethernet/ 802/ sched/ netlink/ +obj-$(CONFIG_NET) += ethernet/ 802/ sched/ netlink/ bpf/ obj-$(CONFIG_NETFILTER) += netfilter/ obj-$(CONFIG_INET)+= ipv4/ obj-$(CONFIG_XFRM)+= xfrm/ diff --git a/net/bpf/Makefile b/net/bpf/Makefile new file mode 100644 index ..27b2992a0692 --- /dev/null +++ b/net/bpf/Makefile @@ -0,0 +1 @@ +obj-y := test_run.o I suggest using a CONFIG option to enable/disable code in test_run.o to reduce attack plane. Thank you.
[PATCH net-next 1/6] bpf: introduce BPF_PROG_TEST_RUN command
development and testing of networking bpf programs is quite cumbersome. Despite availability of user space bpf interpreters the kernel is the ultimate authority and execution environment. Current test frameworks for TC include creation of netns, veth, qdiscs and use of various packet generators just to test functionality of a bpf program. XDP testing is even more complicated, since qemu needs to be started with gro/gso disabled and precise queue configuration, transferring of xdp program from host into guest, attaching to virtio/eth0 and generating traffic from the host while capturing the results from the guest. Moreover analyzing performance bottlenecks in XDP program is impossible in virtio environment, since cost of running the program is tiny comparing to the overhead of virtio packet processing, so performance testing can only be done on physical nic with another server generating traffic. Furthermore ongoing changes to user space control plane of production applications cannot be run on the test servers leaving bpf programs stubbed out for testing. Last but not least, the upstream llvm changes are validated by the bpf backend testsuite which has no ability to test the code generated. To improve this situation introduce BPF_PROG_TEST_RUN command to test and performance benchmark bpf programs. Joint work with Daniel Borkmann. Signed-off-by: Alexei StarovoitovAcked-by: Daniel Borkmann Acked-by: Martin KaFai Lau --- include/linux/bpf.h | 7 ++ include/uapi/linux/bpf.h | 12 kernel/bpf/syscall.c | 27 +++- net/Makefile | 2 +- net/bpf/Makefile | 1 + net/bpf/test_run.c | 172 +++ net/core/filter.c| 5 ++ 7 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 net/bpf/Makefile create mode 100644 net/bpf/test_run.c diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 2ae39a3e9ead..bbb513da5075 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -169,6 +169,8 @@ struct bpf_verifier_ops { const struct bpf_insn *src, struct bpf_insn *dst, struct bpf_prog *prog); + int (*test_run)(struct bpf_prog *prog, const union bpf_attr *kattr, + union bpf_attr __user *uattr); }; struct bpf_prog_type_list { @@ -233,6 +235,11 @@ typedef unsigned long (*bpf_ctx_copy_t)(void *dst, const void *src, u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size, void *ctx, u64 ctx_size, bpf_ctx_copy_t ctx_copy); +int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, + union bpf_attr __user *uattr); +int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, + union bpf_attr __user *uattr); + #ifdef CONFIG_BPF_SYSCALL DECLARE_PER_CPU(int, bpf_prog_active); diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 28317a04c34d..a1d95386f562 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -81,6 +81,7 @@ enum bpf_cmd { BPF_OBJ_GET, BPF_PROG_ATTACH, BPF_PROG_DETACH, + BPF_PROG_TEST_RUN, }; enum bpf_map_type { @@ -189,6 +190,17 @@ union bpf_attr { __u32 attach_type; __u32 attach_flags; }; + + struct { /* anonymous struct used by BPF_PROG_TEST_RUN command */ + __u32 prog_fd; + __u32 retval; + __u32 data_size_in; + __u32 data_size_out; + __aligned_u64 data_in; + __aligned_u64 data_out; + __u32 repeat; + __u32 duration; + } test; } __attribute__((aligned(8))); /* BPF helper function descriptions: diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index c35ebfe6d84d..ab0cf4c43690 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -973,6 +973,28 @@ static int bpf_prog_detach(const union bpf_attr *attr) } #endif /* CONFIG_CGROUP_BPF */ +#define BPF_PROG_TEST_RUN_LAST_FIELD test.duration + +static int bpf_prog_test_run(const union bpf_attr *attr, +union bpf_attr __user *uattr) +{ + struct bpf_prog *prog; + int ret = -ENOTSUPP; + + if (CHECK_ATTR(BPF_PROG_TEST_RUN)) + return -EINVAL; + + prog = bpf_prog_get(attr->test.prog_fd); + if (IS_ERR(prog)) + return PTR_ERR(prog); + + if (prog->aux->ops->test_run) + ret = prog->aux->ops->test_run(prog, attr, uattr); + + bpf_prog_put(prog); + return ret; +} + SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size) { union bpf_attr attr = {};