* [PATCH bpf-next 0/3] support flow dissector in BPF_PROG_TEST_RUN
@ 2018-12-03 18:59 Stanislav Fomichev
2018-12-03 18:59 ` [PATCH bpf-next 1/3] net/flow_dissector: move bpf case into __skb_flow_bpf_dissect Stanislav Fomichev
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Stanislav Fomichev @ 2018-12-03 18:59 UTC (permalink / raw)
To: netdev; +Cc: davem, ast, daniel, simon.horman, Stanislav Fomichev
This patch series adds support for testing flow dissector BPF programs by
extending already existing BPF_PROG_TEST_RUN. The goal is to have a
packet as an input and `struct bpf_flow_key' as an output. That way
we can easily test flow dissector programs' behavior.
I've also modified existing test_progs.c test to do a simple flow
dissector run as well.
* first patch introduces new __skb_flow_bpf_dissect to simplify
sharing between __skb_flow_bpf_dissect and BPF_PROG_TEST_RUN
* second patch adds actual BPF_PROG_TEST_RUN support
* third patch adds example usage to the selftests
Stanislav Fomichev (3):
net/flow_dissector: move bpf case into __skb_flow_bpf_dissect
bpf: add BPF_PROG_TEST_RUN support for flow dissector
selftests/bpf: add simple BPF_PROG_TEST_RUN examples for flow
dissector
include/linux/bpf.h | 3 +
include/linux/skbuff.h | 5 ++
net/bpf/test_run.c | 76 +++++++++++++++--
net/core/filter.c | 1 +
net/core/flow_dissector.c | 83 +++++++++++--------
tools/testing/selftests/bpf/Makefile | 3 +
.../selftests/bpf/flow_dissector_load.c | 43 ++--------
.../selftests/bpf/flow_dissector_load.h | 55 ++++++++++++
tools/testing/selftests/bpf/test_progs.c | 76 ++++++++++++++++-
9 files changed, 265 insertions(+), 80 deletions(-)
create mode 100644 tools/testing/selftests/bpf/flow_dissector_load.h
--
2.20.0.rc1.387.gf8505762e3-goog
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH bpf-next 1/3] net/flow_dissector: move bpf case into __skb_flow_bpf_dissect 2018-12-03 18:59 [PATCH bpf-next 0/3] support flow dissector in BPF_PROG_TEST_RUN Stanislav Fomichev @ 2018-12-03 18:59 ` Stanislav Fomichev 2018-12-03 18:59 ` [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector Stanislav Fomichev 2018-12-03 18:59 ` [PATCH bpf-next 3/3] selftests/bpf: add simple BPF_PROG_TEST_RUN examples " Stanislav Fomichev 2 siblings, 0 replies; 10+ messages in thread From: Stanislav Fomichev @ 2018-12-03 18:59 UTC (permalink / raw) To: netdev; +Cc: davem, ast, daniel, simon.horman, Stanislav Fomichev This way, we can reuse it for flow dissector in BPF_PROG_TEST_RUN. No functional changes. Signed-off-by: Stanislav Fomichev <sdf@google.com> --- include/linux/skbuff.h | 5 +++ net/core/flow_dissector.c | 83 +++++++++++++++++++++++---------------- 2 files changed, 54 insertions(+), 34 deletions(-) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 73902acf2b71..c10b27bb0cab 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1215,6 +1215,11 @@ static inline int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr) } #endif +struct bpf_flow_keys; +bool __skb_flow_bpf_dissect(struct bpf_prog *prog, + const struct sk_buff *skb, + struct flow_dissector *flow_dissector, + struct bpf_flow_keys *flow_keys); bool __skb_flow_dissect(const struct sk_buff *skb, struct flow_dissector *flow_dissector, void *target_container, diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c index 2e8d91e54179..b465f894ec20 100644 --- a/net/core/flow_dissector.c +++ b/net/core/flow_dissector.c @@ -683,6 +683,39 @@ static void __skb_flow_bpf_to_target(const struct bpf_flow_keys *flow_keys, } } +bool __skb_flow_bpf_dissect(struct bpf_prog *prog, + const struct sk_buff *skb, + struct flow_dissector *flow_dissector, + struct bpf_flow_keys *flow_keys) +{ + struct bpf_skb_data_end cb_saved; + struct bpf_skb_data_end *cb; + u32 result; + + /* Note that even though the const qualifier is discarded + * throughout the execution of the BPF program, all changes(the + * control block) are reverted after the BPF program returns. + * Therefore, __skb_flow_dissect does not alter the skb. + */ + + cb = (struct bpf_skb_data_end *)skb->cb; + + /* Save Control Block */ + memcpy(&cb_saved, cb, sizeof(cb_saved)); + memset(cb, 0, sizeof(cb_saved)); + + /* Pass parameters to the BPF program */ + cb->qdisc_cb.flow_keys = flow_keys; + + bpf_compute_data_pointers((struct sk_buff *)skb); + result = BPF_PROG_RUN(prog, skb); + + /* Restore state */ + memcpy(cb, &cb_saved, sizeof(cb_saved)); + + return result == BPF_OK; +} + /** * __skb_flow_dissect - extract the flow_keys struct and return it * @skb: sk_buff to extract the flow from, can be NULL if the rest are specified @@ -714,7 +747,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb, struct flow_dissector_key_vlan *key_vlan; enum flow_dissect_ret fdret; enum flow_dissector_key_id dissector_vlan = FLOW_DISSECTOR_KEY_MAX; - struct bpf_prog *attached = NULL; int num_hdrs = 0; u8 ip_proto = 0; bool ret; @@ -754,49 +786,32 @@ bool __skb_flow_dissect(const struct sk_buff *skb, FLOW_DISSECTOR_KEY_BASIC, target_container); - rcu_read_lock(); if (skb) { + struct bpf_flow_keys flow_keys = {}; + struct bpf_prog *attached = NULL; + + rcu_read_lock(); + if (skb->dev) attached = rcu_dereference(dev_net(skb->dev)->flow_dissector_prog); else if (skb->sk) attached = rcu_dereference(sock_net(skb->sk)->flow_dissector_prog); else WARN_ON_ONCE(1); - } - if (attached) { - /* Note that even though the const qualifier is discarded - * throughout the execution of the BPF program, all changes(the - * control block) are reverted after the BPF program returns. - * Therefore, __skb_flow_dissect does not alter the skb. - */ - struct bpf_flow_keys flow_keys = {}; - struct bpf_skb_data_end cb_saved; - struct bpf_skb_data_end *cb; - u32 result; - - cb = (struct bpf_skb_data_end *)skb->cb; - /* Save Control Block */ - memcpy(&cb_saved, cb, sizeof(cb_saved)); - memset(cb, 0, sizeof(cb_saved)); - - /* Pass parameters to the BPF program */ - cb->qdisc_cb.flow_keys = &flow_keys; - flow_keys.nhoff = nhoff; - - bpf_compute_data_pointers((struct sk_buff *)skb); - result = BPF_PROG_RUN(attached, skb); - - /* Restore state */ - memcpy(cb, &cb_saved, sizeof(cb_saved)); - - __skb_flow_bpf_to_target(&flow_keys, flow_dissector, - target_container); - key_control->thoff = min_t(u16, key_control->thoff, skb->len); + if (attached) { + ret = __skb_flow_bpf_dissect(attached, skb, + flow_dissector, + &flow_keys); + __skb_flow_bpf_to_target(&flow_keys, flow_dissector, + target_container); + key_control->thoff = min_t(u16, key_control->thoff, + skb->len); + rcu_read_unlock(); + return ret; + } rcu_read_unlock(); - return result == BPF_OK; } - rcu_read_unlock(); if (dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_ETH_ADDRS)) { -- 2.20.0.rc1.387.gf8505762e3-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector 2018-12-03 18:59 [PATCH bpf-next 0/3] support flow dissector in BPF_PROG_TEST_RUN Stanislav Fomichev 2018-12-03 18:59 ` [PATCH bpf-next 1/3] net/flow_dissector: move bpf case into __skb_flow_bpf_dissect Stanislav Fomichev @ 2018-12-03 18:59 ` Stanislav Fomichev 2018-12-03 22:28 ` Song Liu 2018-12-03 18:59 ` [PATCH bpf-next 3/3] selftests/bpf: add simple BPF_PROG_TEST_RUN examples " Stanislav Fomichev 2 siblings, 1 reply; 10+ messages in thread From: Stanislav Fomichev @ 2018-12-03 18:59 UTC (permalink / raw) To: netdev; +Cc: davem, ast, daniel, simon.horman, Stanislav Fomichev The input is packet data, the output is struct bpf_flow_key. This should make it easy to test flow dissector programs without elaborate setup. Signed-off-by: Stanislav Fomichev <sdf@google.com> --- include/linux/bpf.h | 3 ++ net/bpf/test_run.c | 76 +++++++++++++++++++++++++++++++++++++++++---- net/core/filter.c | 1 + 3 files changed, 74 insertions(+), 6 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index e82b7039fc66..7a572d15d5dd 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -373,6 +373,9 @@ 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); +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, + const union bpf_attr *kattr, + union bpf_attr __user *uattr); /* an array of programs to be executed under rcu_lock. * diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index c89c22c49015..bfa05d31c6e3 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -14,21 +14,33 @@ #include <net/tcp.h> static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx, - struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE]) + struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE], + struct bpf_flow_keys *flow_keys) { u32 ret; preempt_disable(); rcu_read_lock(); bpf_cgroup_storage_set(storage); - ret = BPF_PROG_RUN(prog, ctx); + + switch (prog->type) { + case BPF_PROG_TYPE_FLOW_DISSECTOR: + ret = __skb_flow_bpf_dissect(prog, ctx, &flow_keys_dissector, + flow_keys); + break; + default: + ret = BPF_PROG_RUN(prog, ctx); + break; + } + rcu_read_unlock(); preempt_enable(); return ret; } -static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) +static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time, + struct bpf_flow_keys *flow_keys) { struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { 0 }; enum bpf_cgroup_storage_type stype; @@ -49,7 +61,7 @@ static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) repeat = 1; time_start = ktime_get_ns(); for (i = 0; i < repeat; i++) { - ret = bpf_test_run_one(prog, ctx, storage); + ret = bpf_test_run_one(prog, ctx, storage, flow_keys); if (need_resched()) { if (signal_pending(current)) break; @@ -165,7 +177,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, __skb_push(skb, hh_len); if (is_direct_pkt_access) bpf_compute_data_pointers(skb); - retval = bpf_test_run(prog, skb, repeat, &duration); + retval = bpf_test_run(prog, skb, repeat, &duration, NULL); if (!is_l2) { if (skb_headroom(skb) < hh_len) { int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb)); @@ -212,7 +224,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, rxqueue = __netif_get_rx_queue(current->nsproxy->net_ns->loopback_dev, 0); xdp.rxq = &rxqueue->xdp_rxq; - retval = bpf_test_run(prog, &xdp, repeat, &duration); + retval = bpf_test_run(prog, &xdp, repeat, &duration, NULL); if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN || xdp.data_end != xdp.data + size) size = xdp.data_end - xdp.data; @@ -220,3 +232,55 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, kfree(data); return ret; } + +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, + const union bpf_attr *kattr, + union bpf_attr __user *uattr) +{ + struct bpf_flow_keys flow_keys = {}; + u32 size = kattr->test.data_size_in; + u32 repeat = kattr->test.repeat; + u32 retval, duration; + struct sk_buff *skb; + struct sock *sk; + void *data; + int ret; + + if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR) + return -EINVAL; + + data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN, + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); + if (IS_ERR(data)) + return PTR_ERR(data); + + sk = kzalloc(sizeof(*sk), GFP_USER); + if (!sk) { + kfree(data); + return -ENOMEM; + } + sock_net_set(sk, current->nsproxy->net_ns); + sock_init_data(NULL, sk); + + skb = build_skb(data, 0); + if (!skb) { + kfree(data); + kfree(sk); + return -ENOMEM; + } + skb->sk = sk; + + skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); + __skb_put(skb, size); + skb->protocol = eth_type_trans(skb, + current->nsproxy->net_ns->loopback_dev); + skb_reset_network_header(skb); + + retval = bpf_test_run(prog, skb, repeat, &duration, &flow_keys); + + ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys), + retval, duration); + kfree_skb(skb); + kfree(sk); + return ret; +} diff --git a/net/core/filter.c b/net/core/filter.c index bd0df75dc7b6..4eae6102399d 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -7657,6 +7657,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = { }; const struct bpf_prog_ops flow_dissector_prog_ops = { + .test_run = bpf_prog_test_run_flow_dissector, }; int sk_detach_filter(struct sock *sk) -- 2.20.0.rc1.387.gf8505762e3-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector 2018-12-03 18:59 ` [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector Stanislav Fomichev @ 2018-12-03 22:28 ` Song Liu 2018-12-03 23:08 ` Stanislav Fomichev 0 siblings, 1 reply; 10+ messages in thread From: Song Liu @ 2018-12-03 22:28 UTC (permalink / raw) To: sdf Cc: Networking, David S . Miller, Alexei Starovoitov, Daniel Borkmann, simon.horman On Mon, Dec 3, 2018 at 11:00 AM Stanislav Fomichev <sdf@google.com> wrote: > > The input is packet data, the output is struct bpf_flow_key. This should > make it easy to test flow dissector programs without elaborate > setup. > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > --- > include/linux/bpf.h | 3 ++ > net/bpf/test_run.c | 76 +++++++++++++++++++++++++++++++++++++++++---- > net/core/filter.c | 1 + > 3 files changed, 74 insertions(+), 6 deletions(-) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index e82b7039fc66..7a572d15d5dd 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -373,6 +373,9 @@ 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); > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > + const union bpf_attr *kattr, > + union bpf_attr __user *uattr); > > /* an array of programs to be executed under rcu_lock. > * > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > index c89c22c49015..bfa05d31c6e3 100644 > --- a/net/bpf/test_run.c > +++ b/net/bpf/test_run.c > @@ -14,21 +14,33 @@ > #include <net/tcp.h> > > static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx, > - struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE]) > + struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE], > + struct bpf_flow_keys *flow_keys) > { > u32 ret; > > preempt_disable(); > rcu_read_lock(); > bpf_cgroup_storage_set(storage); > - ret = BPF_PROG_RUN(prog, ctx); > + > + switch (prog->type) { > + case BPF_PROG_TYPE_FLOW_DISSECTOR: > + ret = __skb_flow_bpf_dissect(prog, ctx, &flow_keys_dissector, > + flow_keys); > + break; > + default: > + ret = BPF_PROG_RUN(prog, ctx); > + break; > + } > + Is it possible to fold the logic above into bpf_prog_test_run_flow_dissector()? In that way, the logic flow is similar to other bpf_prog_test_run_XXX() functions. Thanks, Song > rcu_read_unlock(); > preempt_enable(); > > return ret; > } > > -static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) > +static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time, > + struct bpf_flow_keys *flow_keys) > { > struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { 0 }; > enum bpf_cgroup_storage_type stype; > @@ -49,7 +61,7 @@ static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) > repeat = 1; > time_start = ktime_get_ns(); > for (i = 0; i < repeat; i++) { > - ret = bpf_test_run_one(prog, ctx, storage); > + ret = bpf_test_run_one(prog, ctx, storage, flow_keys); > if (need_resched()) { > if (signal_pending(current)) > break; > @@ -165,7 +177,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, > __skb_push(skb, hh_len); > if (is_direct_pkt_access) > bpf_compute_data_pointers(skb); > - retval = bpf_test_run(prog, skb, repeat, &duration); > + retval = bpf_test_run(prog, skb, repeat, &duration, NULL); > if (!is_l2) { > if (skb_headroom(skb) < hh_len) { > int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb)); > @@ -212,7 +224,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > rxqueue = __netif_get_rx_queue(current->nsproxy->net_ns->loopback_dev, 0); > xdp.rxq = &rxqueue->xdp_rxq; > > - retval = bpf_test_run(prog, &xdp, repeat, &duration); > + retval = bpf_test_run(prog, &xdp, repeat, &duration, NULL); > if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN || > xdp.data_end != xdp.data + size) > size = xdp.data_end - xdp.data; > @@ -220,3 +232,55 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > kfree(data); > return ret; > } > + > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > + const union bpf_attr *kattr, > + union bpf_attr __user *uattr) > +{ > + struct bpf_flow_keys flow_keys = {}; > + u32 size = kattr->test.data_size_in; > + u32 repeat = kattr->test.repeat; > + u32 retval, duration; > + struct sk_buff *skb; > + struct sock *sk; > + void *data; > + int ret; > + > + if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR) > + return -EINVAL; > + > + data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN, > + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); > + if (IS_ERR(data)) > + return PTR_ERR(data); > + > + sk = kzalloc(sizeof(*sk), GFP_USER); > + if (!sk) { > + kfree(data); > + return -ENOMEM; > + } > + sock_net_set(sk, current->nsproxy->net_ns); > + sock_init_data(NULL, sk); > + > + skb = build_skb(data, 0); > + if (!skb) { > + kfree(data); > + kfree(sk); > + return -ENOMEM; > + } > + skb->sk = sk; > + > + skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); > + __skb_put(skb, size); > + skb->protocol = eth_type_trans(skb, > + current->nsproxy->net_ns->loopback_dev); > + skb_reset_network_header(skb); > + > + retval = bpf_test_run(prog, skb, repeat, &duration, &flow_keys); > + > + ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys), > + retval, duration); > + kfree_skb(skb); > + kfree(sk); > + return ret; > +} > diff --git a/net/core/filter.c b/net/core/filter.c > index bd0df75dc7b6..4eae6102399d 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -7657,6 +7657,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = { > }; > > const struct bpf_prog_ops flow_dissector_prog_ops = { > + .test_run = bpf_prog_test_run_flow_dissector, > }; > > int sk_detach_filter(struct sock *sk) > -- > 2.20.0.rc1.387.gf8505762e3-goog > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector 2018-12-03 22:28 ` Song Liu @ 2018-12-03 23:08 ` Stanislav Fomichev 2018-12-04 3:54 ` Stanislav Fomichev 2018-12-04 23:25 ` Song Liu 0 siblings, 2 replies; 10+ messages in thread From: Stanislav Fomichev @ 2018-12-03 23:08 UTC (permalink / raw) To: Song Liu Cc: sdf, Networking, David S . Miller, Alexei Starovoitov, Daniel Borkmann, simon.horman On 12/03, Song Liu wrote: > On Mon, Dec 3, 2018 at 11:00 AM Stanislav Fomichev <sdf@google.com> wrote: > > > > The input is packet data, the output is struct bpf_flow_key. This should > > make it easy to test flow dissector programs without elaborate > > setup. > > > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > --- > > include/linux/bpf.h | 3 ++ > > net/bpf/test_run.c | 76 +++++++++++++++++++++++++++++++++++++++++---- > > net/core/filter.c | 1 + > > 3 files changed, 74 insertions(+), 6 deletions(-) > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > index e82b7039fc66..7a572d15d5dd 100644 > > --- a/include/linux/bpf.h > > +++ b/include/linux/bpf.h > > @@ -373,6 +373,9 @@ 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); > > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > > + const union bpf_attr *kattr, > > + union bpf_attr __user *uattr); > > > > /* an array of programs to be executed under rcu_lock. > > * > > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > > index c89c22c49015..bfa05d31c6e3 100644 > > --- a/net/bpf/test_run.c > > +++ b/net/bpf/test_run.c > > @@ -14,21 +14,33 @@ > > #include <net/tcp.h> > > > > static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx, > > - struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE]) > > + struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE], > > + struct bpf_flow_keys *flow_keys) > > { > > u32 ret; > > > > preempt_disable(); > > rcu_read_lock(); > > bpf_cgroup_storage_set(storage); > > - ret = BPF_PROG_RUN(prog, ctx); > > + > > + switch (prog->type) { > > + case BPF_PROG_TYPE_FLOW_DISSECTOR: > > + ret = __skb_flow_bpf_dissect(prog, ctx, &flow_keys_dissector, > > + flow_keys); > > + break; > > + default: > > + ret = BPF_PROG_RUN(prog, ctx); > > + break; > > + } > > + > > Is it possible to fold the logic above into bpf_prog_test_run_flow_dissector()? > In that way, the logic flow is similar to other bpf_prog_test_run_XXX() > functions. I can probably do everything that __skb_flow_bpf_dissect does inside of bpf_prog_test_run_flow_dissector (pass flow_keys in cb); that would remove this ugly program type check down the stack. But my additional goal here was to test __skb_flow_bpf_dissect itself as well. Attached some possible prototype below. The only issue I see with that approach is that we need to clear flow_keys on each test iteration. I think in my current patch I'm actually missing a memset(&flow_keys, 0, ...) for each iteration of __skb_flow_bpf_dissect; I haven't tested multiple runs :-/ So I'd prefer to continue doing this prog type check (plus, add a missing memset for flow_keys on each iteration in v2). WDYT? --- diff --git a/include/linux/bpf.h b/include/linux/bpf.h index e82b7039fc66..7a572d15d5dd 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -373,6 +373,9 @@ 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); +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, + const union bpf_attr *kattr, + union bpf_attr __user *uattr); /* an array of programs to be executed under rcu_lock. * diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c index c89c22c49015..04387ef1f95a 100644 --- a/net/bpf/test_run.c +++ b/net/bpf/test_run.c @@ -220,3 +220,60 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, kfree(data); return ret; } + +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, + const union bpf_attr *kattr, + union bpf_attr __user *uattr) +{ + struct bpf_flow_keys flow_keys = {}; + u32 size = kattr->test.data_size_in; + u32 repeat = kattr->test.repeat; + struct bpf_skb_data_end *cb; + u32 retval, duration; + struct sk_buff *skb; + struct sock *sk; + void *data; + int ret; + + if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR) + return -EINVAL; + + data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN, + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); + if (IS_ERR(data)) + return PTR_ERR(data); + + sk = kzalloc(sizeof(*sk), GFP_USER); + if (!sk) { + kfree(data); + return -ENOMEM; + } + sock_net_set(sk, current->nsproxy->net_ns); + sock_init_data(NULL, sk); + + skb = build_skb(data, 0); + if (!skb) { + kfree(data); + kfree(sk); + return -ENOMEM; + } + skb->sk = sk; + + skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); + __skb_put(skb, size); + skb->protocol = eth_type_trans(skb, + current->nsproxy->net_ns->loopback_dev); + skb_reset_network_header(skb); + + cb = (struct bpf_skb_data_end *)skb->cb; + memset(cb, 0, sizeof(cb)); + cb->qdisc_cb.flow_keys = &flow_keys; + + retval = bpf_test_run(prog, skb, repeat, &duration); + + ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys), + retval, duration); + kfree_skb(skb); + kfree(sk); + return ret; +} diff --git a/net/core/filter.c b/net/core/filter.c index bd0df75dc7b6..4eae6102399d 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -7657,6 +7657,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = { }; const struct bpf_prog_ops flow_dissector_prog_ops = { + .test_run = bpf_prog_test_run_flow_dissector, }; int sk_detach_filter(struct sock *sk) > Thanks, > Song > > > > rcu_read_unlock(); > > preempt_enable(); > > > > return ret; > > } > > > > -static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) > > +static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time, > > + struct bpf_flow_keys *flow_keys) > > { > > struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { 0 }; > > enum bpf_cgroup_storage_type stype; > > @@ -49,7 +61,7 @@ static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) > > repeat = 1; > > time_start = ktime_get_ns(); > > for (i = 0; i < repeat; i++) { > > - ret = bpf_test_run_one(prog, ctx, storage); > > + ret = bpf_test_run_one(prog, ctx, storage, flow_keys); > > if (need_resched()) { > > if (signal_pending(current)) > > break; > > @@ -165,7 +177,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, > > __skb_push(skb, hh_len); > > if (is_direct_pkt_access) > > bpf_compute_data_pointers(skb); > > - retval = bpf_test_run(prog, skb, repeat, &duration); > > + retval = bpf_test_run(prog, skb, repeat, &duration, NULL); > > if (!is_l2) { > > if (skb_headroom(skb) < hh_len) { > > int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb)); > > @@ -212,7 +224,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > > rxqueue = __netif_get_rx_queue(current->nsproxy->net_ns->loopback_dev, 0); > > xdp.rxq = &rxqueue->xdp_rxq; > > > > - retval = bpf_test_run(prog, &xdp, repeat, &duration); > > + retval = bpf_test_run(prog, &xdp, repeat, &duration, NULL); > > if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN || > > xdp.data_end != xdp.data + size) > > size = xdp.data_end - xdp.data; > > @@ -220,3 +232,55 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > > kfree(data); > > return ret; > > } > > + > > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > > + const union bpf_attr *kattr, > > + union bpf_attr __user *uattr) > > +{ > > + struct bpf_flow_keys flow_keys = {}; > > + u32 size = kattr->test.data_size_in; > > + u32 repeat = kattr->test.repeat; > > + u32 retval, duration; > > + struct sk_buff *skb; > > + struct sock *sk; > > + void *data; > > + int ret; > > + > > + if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR) > > + return -EINVAL; > > + > > + data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN, > > + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); > > + if (IS_ERR(data)) > > + return PTR_ERR(data); > > + > > + sk = kzalloc(sizeof(*sk), GFP_USER); > > + if (!sk) { > > + kfree(data); > > + return -ENOMEM; > > + } > > + sock_net_set(sk, current->nsproxy->net_ns); > > + sock_init_data(NULL, sk); > > + > > + skb = build_skb(data, 0); > > + if (!skb) { > > + kfree(data); > > + kfree(sk); > > + return -ENOMEM; > > + } > > + skb->sk = sk; > > + > > + skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); > > + __skb_put(skb, size); > > + skb->protocol = eth_type_trans(skb, > > + current->nsproxy->net_ns->loopback_dev); > > + skb_reset_network_header(skb); > > + > > + retval = bpf_test_run(prog, skb, repeat, &duration, &flow_keys); > > + > > + ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys), > > + retval, duration); > > + kfree_skb(skb); > > + kfree(sk); > > + return ret; > > +} > > diff --git a/net/core/filter.c b/net/core/filter.c > > index bd0df75dc7b6..4eae6102399d 100644 > > --- a/net/core/filter.c > > +++ b/net/core/filter.c > > @@ -7657,6 +7657,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = { > > }; > > > > const struct bpf_prog_ops flow_dissector_prog_ops = { > > + .test_run = bpf_prog_test_run_flow_dissector, > > }; > > > > int sk_detach_filter(struct sock *sk) > > -- > > 2.20.0.rc1.387.gf8505762e3-goog > > ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector 2018-12-03 23:08 ` Stanislav Fomichev @ 2018-12-04 3:54 ` Stanislav Fomichev 2018-12-04 23:25 ` Song Liu 1 sibling, 0 replies; 10+ messages in thread From: Stanislav Fomichev @ 2018-12-04 3:54 UTC (permalink / raw) To: Song Liu Cc: sdf, Networking, David S . Miller, Alexei Starovoitov, Daniel Borkmann, simon.horman On 12/03, Stanislav Fomichev wrote: > On 12/03, Song Liu wrote: > > On Mon, Dec 3, 2018 at 11:00 AM Stanislav Fomichev <sdf@google.com> wrote: > > > > > > The input is packet data, the output is struct bpf_flow_key. This should > > > make it easy to test flow dissector programs without elaborate > > > setup. > > > > > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > > --- > > > include/linux/bpf.h | 3 ++ > > > net/bpf/test_run.c | 76 +++++++++++++++++++++++++++++++++++++++++---- > > > net/core/filter.c | 1 + > > > 3 files changed, 74 insertions(+), 6 deletions(-) > > > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > > index e82b7039fc66..7a572d15d5dd 100644 > > > --- a/include/linux/bpf.h > > > +++ b/include/linux/bpf.h > > > @@ -373,6 +373,9 @@ 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); > > > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > > > + const union bpf_attr *kattr, > > > + union bpf_attr __user *uattr); > > > > > > /* an array of programs to be executed under rcu_lock. > > > * > > > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > > > index c89c22c49015..bfa05d31c6e3 100644 > > > --- a/net/bpf/test_run.c > > > +++ b/net/bpf/test_run.c > > > @@ -14,21 +14,33 @@ > > > #include <net/tcp.h> > > > > > > static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx, > > > - struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE]) > > > + struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE], > > > + struct bpf_flow_keys *flow_keys) > > > { > > > u32 ret; > > > > > > preempt_disable(); > > > rcu_read_lock(); > > > bpf_cgroup_storage_set(storage); > > > - ret = BPF_PROG_RUN(prog, ctx); > > > + > > > + switch (prog->type) { > > > + case BPF_PROG_TYPE_FLOW_DISSECTOR: > > > + ret = __skb_flow_bpf_dissect(prog, ctx, &flow_keys_dissector, > > > + flow_keys); > > > + break; > > > + default: > > > + ret = BPF_PROG_RUN(prog, ctx); > > > + break; > > > + } > > > + > > > > Is it possible to fold the logic above into bpf_prog_test_run_flow_dissector()? > > In that way, the logic flow is similar to other bpf_prog_test_run_XXX() > > functions. > I can probably do everything that __skb_flow_bpf_dissect does inside of > bpf_prog_test_run_flow_dissector (pass flow_keys in cb); that would remove > this ugly program type check down the stack. But my additional goal here was > to test __skb_flow_bpf_dissect itself as well. > > Attached some possible prototype below. The only issue I see with that > approach is that we need to clear flow_keys on each test iteration. > I think in my current patch I'm actually missing a memset(&flow_keys, 0, ...) > for each iteration of __skb_flow_bpf_dissect; > I haven't tested multiple runs :-/ > > So I'd prefer to continue doing this prog type check (plus, add a missing > memset for flow_keys on each iteration in v2). WDYT? I think I found another problem with not using __skb_flow_bpf_dissect: I want to add some clamping to the thoff/nhoff returned by the bpf program (to be safe); it's probably better to keep all this logic in one place. I can probably pass flow_keys in the skb->cb, so I don't have to add flow_keys to the bpf_test_run_one/bpf_test_run signatures. I'll send v2 shortly, let me know if it addresses your point. > --- > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index e82b7039fc66..7a572d15d5dd 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -373,6 +373,9 @@ 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); > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > + const union bpf_attr *kattr, > + union bpf_attr __user *uattr); > > /* an array of programs to be executed under rcu_lock. > * > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > index c89c22c49015..04387ef1f95a 100644 > --- a/net/bpf/test_run.c > +++ b/net/bpf/test_run.c > @@ -220,3 +220,60 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > kfree(data); > return ret; > } > + > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > + const union bpf_attr *kattr, > + union bpf_attr __user *uattr) > +{ > + struct bpf_flow_keys flow_keys = {}; > + u32 size = kattr->test.data_size_in; > + u32 repeat = kattr->test.repeat; > + struct bpf_skb_data_end *cb; > + u32 retval, duration; > + struct sk_buff *skb; > + struct sock *sk; > + void *data; > + int ret; > + > + if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR) > + return -EINVAL; > + > + data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN, > + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); > + if (IS_ERR(data)) > + return PTR_ERR(data); > + > + sk = kzalloc(sizeof(*sk), GFP_USER); > + if (!sk) { > + kfree(data); > + return -ENOMEM; > + } > + sock_net_set(sk, current->nsproxy->net_ns); > + sock_init_data(NULL, sk); > + > + skb = build_skb(data, 0); > + if (!skb) { > + kfree(data); > + kfree(sk); > + return -ENOMEM; > + } > + skb->sk = sk; > + > + skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); > + __skb_put(skb, size); > + skb->protocol = eth_type_trans(skb, > + current->nsproxy->net_ns->loopback_dev); > + skb_reset_network_header(skb); > + > + cb = (struct bpf_skb_data_end *)skb->cb; > + memset(cb, 0, sizeof(cb)); > + cb->qdisc_cb.flow_keys = &flow_keys; > + > + retval = bpf_test_run(prog, skb, repeat, &duration); > + > + ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys), > + retval, duration); > + kfree_skb(skb); > + kfree(sk); > + return ret; > +} > diff --git a/net/core/filter.c b/net/core/filter.c > index bd0df75dc7b6..4eae6102399d 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -7657,6 +7657,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = { > }; > > const struct bpf_prog_ops flow_dissector_prog_ops = { > + .test_run = bpf_prog_test_run_flow_dissector, > }; > > int sk_detach_filter(struct sock *sk) > > > Thanks, > > Song > > > > > > > rcu_read_unlock(); > > > preempt_enable(); > > > > > > return ret; > > > } > > > > > > -static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) > > > +static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time, > > > + struct bpf_flow_keys *flow_keys) > > > { > > > struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { 0 }; > > > enum bpf_cgroup_storage_type stype; > > > @@ -49,7 +61,7 @@ static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) > > > repeat = 1; > > > time_start = ktime_get_ns(); > > > for (i = 0; i < repeat; i++) { > > > - ret = bpf_test_run_one(prog, ctx, storage); > > > + ret = bpf_test_run_one(prog, ctx, storage, flow_keys); > > > if (need_resched()) { > > > if (signal_pending(current)) > > > break; > > > @@ -165,7 +177,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, > > > __skb_push(skb, hh_len); > > > if (is_direct_pkt_access) > > > bpf_compute_data_pointers(skb); > > > - retval = bpf_test_run(prog, skb, repeat, &duration); > > > + retval = bpf_test_run(prog, skb, repeat, &duration, NULL); > > > if (!is_l2) { > > > if (skb_headroom(skb) < hh_len) { > > > int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb)); > > > @@ -212,7 +224,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > > > rxqueue = __netif_get_rx_queue(current->nsproxy->net_ns->loopback_dev, 0); > > > xdp.rxq = &rxqueue->xdp_rxq; > > > > > > - retval = bpf_test_run(prog, &xdp, repeat, &duration); > > > + retval = bpf_test_run(prog, &xdp, repeat, &duration, NULL); > > > if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN || > > > xdp.data_end != xdp.data + size) > > > size = xdp.data_end - xdp.data; > > > @@ -220,3 +232,55 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > > > kfree(data); > > > return ret; > > > } > > > + > > > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > > > + const union bpf_attr *kattr, > > > + union bpf_attr __user *uattr) > > > +{ > > > + struct bpf_flow_keys flow_keys = {}; > > > + u32 size = kattr->test.data_size_in; > > > + u32 repeat = kattr->test.repeat; > > > + u32 retval, duration; > > > + struct sk_buff *skb; > > > + struct sock *sk; > > > + void *data; > > > + int ret; > > > + > > > + if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR) > > > + return -EINVAL; > > > + > > > + data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN, > > > + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); > > > + if (IS_ERR(data)) > > > + return PTR_ERR(data); > > > + > > > + sk = kzalloc(sizeof(*sk), GFP_USER); > > > + if (!sk) { > > > + kfree(data); > > > + return -ENOMEM; > > > + } > > > + sock_net_set(sk, current->nsproxy->net_ns); > > > + sock_init_data(NULL, sk); > > > + > > > + skb = build_skb(data, 0); > > > + if (!skb) { > > > + kfree(data); > > > + kfree(sk); > > > + return -ENOMEM; > > > + } > > > + skb->sk = sk; > > > + > > > + skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); > > > + __skb_put(skb, size); > > > + skb->protocol = eth_type_trans(skb, > > > + current->nsproxy->net_ns->loopback_dev); > > > + skb_reset_network_header(skb); > > > + > > > + retval = bpf_test_run(prog, skb, repeat, &duration, &flow_keys); > > > + > > > + ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys), > > > + retval, duration); > > > + kfree_skb(skb); > > > + kfree(sk); > > > + return ret; > > > +} > > > diff --git a/net/core/filter.c b/net/core/filter.c > > > index bd0df75dc7b6..4eae6102399d 100644 > > > --- a/net/core/filter.c > > > +++ b/net/core/filter.c > > > @@ -7657,6 +7657,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = { > > > }; > > > > > > const struct bpf_prog_ops flow_dissector_prog_ops = { > > > + .test_run = bpf_prog_test_run_flow_dissector, > > > }; > > > > > > int sk_detach_filter(struct sock *sk) > > > -- > > > 2.20.0.rc1.387.gf8505762e3-goog > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector 2018-12-03 23:08 ` Stanislav Fomichev 2018-12-04 3:54 ` Stanislav Fomichev @ 2018-12-04 23:25 ` Song Liu 2018-12-04 23:36 ` Stanislav Fomichev 1 sibling, 1 reply; 10+ messages in thread From: Song Liu @ 2018-12-04 23:25 UTC (permalink / raw) To: sdf Cc: Stanislav Fomichev, Networking, David S . Miller, Alexei Starovoitov, Daniel Borkmann, simon.horman On Mon, Dec 3, 2018 at 3:08 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > On 12/03, Song Liu wrote: > > On Mon, Dec 3, 2018 at 11:00 AM Stanislav Fomichev <sdf@google.com> wrote: > > > > > > The input is packet data, the output is struct bpf_flow_key. This should > > > make it easy to test flow dissector programs without elaborate > > > setup. > > > > > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > > --- > > > include/linux/bpf.h | 3 ++ > > > net/bpf/test_run.c | 76 +++++++++++++++++++++++++++++++++++++++++---- > > > net/core/filter.c | 1 + > > > 3 files changed, 74 insertions(+), 6 deletions(-) > > > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > > index e82b7039fc66..7a572d15d5dd 100644 > > > --- a/include/linux/bpf.h > > > +++ b/include/linux/bpf.h > > > @@ -373,6 +373,9 @@ 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); > > > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > > > + const union bpf_attr *kattr, > > > + union bpf_attr __user *uattr); > > > > > > /* an array of programs to be executed under rcu_lock. > > > * > > > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > > > index c89c22c49015..bfa05d31c6e3 100644 > > > --- a/net/bpf/test_run.c > > > +++ b/net/bpf/test_run.c > > > @@ -14,21 +14,33 @@ > > > #include <net/tcp.h> > > > > > > static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx, > > > - struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE]) > > > + struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE], > > > + struct bpf_flow_keys *flow_keys) > > > { > > > u32 ret; > > > > > > preempt_disable(); > > > rcu_read_lock(); > > > bpf_cgroup_storage_set(storage); > > > - ret = BPF_PROG_RUN(prog, ctx); > > > + > > > + switch (prog->type) { > > > + case BPF_PROG_TYPE_FLOW_DISSECTOR: > > > + ret = __skb_flow_bpf_dissect(prog, ctx, &flow_keys_dissector, > > > + flow_keys); > > > + break; > > > + default: > > > + ret = BPF_PROG_RUN(prog, ctx); > > > + break; > > > + } > > > + > > > > Is it possible to fold the logic above into bpf_prog_test_run_flow_dissector()? > > In that way, the logic flow is similar to other bpf_prog_test_run_XXX() > > functions. > I can probably do everything that __skb_flow_bpf_dissect does inside of > bpf_prog_test_run_flow_dissector (pass flow_keys in cb); that would remove > this ugly program type check down the stack. But my additional goal here was > to test __skb_flow_bpf_dissect itself as well. > > Attached some possible prototype below. The only issue I see with that > approach is that we need to clear flow_keys on each test iteration. > I think in my current patch I'm actually missing a memset(&flow_keys, 0, ...) > for each iteration of __skb_flow_bpf_dissect; > I haven't tested multiple runs :-/ > > So I'd prefer to continue doing this prog type check (plus, add a missing > memset for flow_keys on each iteration in v2). WDYT? I think v2 is still using original logic? Actually, after a second thought, I think it is OK to change bpf_test_run_one() (unless Alexei and/or Daniel has better suggestions. Thanks, Song > > --- > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index e82b7039fc66..7a572d15d5dd 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -373,6 +373,9 @@ 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); > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > + const union bpf_attr *kattr, > + union bpf_attr __user *uattr); > > /* an array of programs to be executed under rcu_lock. > * > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > index c89c22c49015..04387ef1f95a 100644 > --- a/net/bpf/test_run.c > +++ b/net/bpf/test_run.c > @@ -220,3 +220,60 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > kfree(data); > return ret; > } > + > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > + const union bpf_attr *kattr, > + union bpf_attr __user *uattr) > +{ > + struct bpf_flow_keys flow_keys = {}; > + u32 size = kattr->test.data_size_in; > + u32 repeat = kattr->test.repeat; > + struct bpf_skb_data_end *cb; > + u32 retval, duration; > + struct sk_buff *skb; > + struct sock *sk; > + void *data; > + int ret; > + > + if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR) > + return -EINVAL; > + > + data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN, > + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); > + if (IS_ERR(data)) > + return PTR_ERR(data); > + > + sk = kzalloc(sizeof(*sk), GFP_USER); > + if (!sk) { > + kfree(data); > + return -ENOMEM; > + } > + sock_net_set(sk, current->nsproxy->net_ns); > + sock_init_data(NULL, sk); > + > + skb = build_skb(data, 0); > + if (!skb) { > + kfree(data); > + kfree(sk); > + return -ENOMEM; > + } > + skb->sk = sk; > + > + skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); > + __skb_put(skb, size); > + skb->protocol = eth_type_trans(skb, > + current->nsproxy->net_ns->loopback_dev); > + skb_reset_network_header(skb); > + > + cb = (struct bpf_skb_data_end *)skb->cb; > + memset(cb, 0, sizeof(cb)); > + cb->qdisc_cb.flow_keys = &flow_keys; > + > + retval = bpf_test_run(prog, skb, repeat, &duration); > + > + ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys), > + retval, duration); > + kfree_skb(skb); > + kfree(sk); > + return ret; > +} > diff --git a/net/core/filter.c b/net/core/filter.c > index bd0df75dc7b6..4eae6102399d 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -7657,6 +7657,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = { > }; > > const struct bpf_prog_ops flow_dissector_prog_ops = { > + .test_run = bpf_prog_test_run_flow_dissector, > }; > > int sk_detach_filter(struct sock *sk) > > > Thanks, > > Song > > > > > > > rcu_read_unlock(); > > > preempt_enable(); > > > > > > return ret; > > > } > > > > > > -static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) > > > +static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time, > > > + struct bpf_flow_keys *flow_keys) > > > { > > > struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { 0 }; > > > enum bpf_cgroup_storage_type stype; > > > @@ -49,7 +61,7 @@ static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) > > > repeat = 1; > > > time_start = ktime_get_ns(); > > > for (i = 0; i < repeat; i++) { > > > - ret = bpf_test_run_one(prog, ctx, storage); > > > + ret = bpf_test_run_one(prog, ctx, storage, flow_keys); > > > if (need_resched()) { > > > if (signal_pending(current)) > > > break; > > > @@ -165,7 +177,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, > > > __skb_push(skb, hh_len); > > > if (is_direct_pkt_access) > > > bpf_compute_data_pointers(skb); > > > - retval = bpf_test_run(prog, skb, repeat, &duration); > > > + retval = bpf_test_run(prog, skb, repeat, &duration, NULL); > > > if (!is_l2) { > > > if (skb_headroom(skb) < hh_len) { > > > int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb)); > > > @@ -212,7 +224,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > > > rxqueue = __netif_get_rx_queue(current->nsproxy->net_ns->loopback_dev, 0); > > > xdp.rxq = &rxqueue->xdp_rxq; > > > > > > - retval = bpf_test_run(prog, &xdp, repeat, &duration); > > > + retval = bpf_test_run(prog, &xdp, repeat, &duration, NULL); > > > if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN || > > > xdp.data_end != xdp.data + size) > > > size = xdp.data_end - xdp.data; > > > @@ -220,3 +232,55 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > > > kfree(data); > > > return ret; > > > } > > > + > > > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > > > + const union bpf_attr *kattr, > > > + union bpf_attr __user *uattr) > > > +{ > > > + struct bpf_flow_keys flow_keys = {}; > > > + u32 size = kattr->test.data_size_in; > > > + u32 repeat = kattr->test.repeat; > > > + u32 retval, duration; > > > + struct sk_buff *skb; > > > + struct sock *sk; > > > + void *data; > > > + int ret; > > > + > > > + if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR) > > > + return -EINVAL; > > > + > > > + data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN, > > > + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); > > > + if (IS_ERR(data)) > > > + return PTR_ERR(data); > > > + > > > + sk = kzalloc(sizeof(*sk), GFP_USER); > > > + if (!sk) { > > > + kfree(data); > > > + return -ENOMEM; > > > + } > > > + sock_net_set(sk, current->nsproxy->net_ns); > > > + sock_init_data(NULL, sk); > > > + > > > + skb = build_skb(data, 0); > > > + if (!skb) { > > > + kfree(data); > > > + kfree(sk); > > > + return -ENOMEM; > > > + } > > > + skb->sk = sk; > > > + > > > + skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); > > > + __skb_put(skb, size); > > > + skb->protocol = eth_type_trans(skb, > > > + current->nsproxy->net_ns->loopback_dev); > > > + skb_reset_network_header(skb); > > > + > > > + retval = bpf_test_run(prog, skb, repeat, &duration, &flow_keys); > > > + > > > + ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys), > > > + retval, duration); > > > + kfree_skb(skb); > > > + kfree(sk); > > > + return ret; > > > +} > > > diff --git a/net/core/filter.c b/net/core/filter.c > > > index bd0df75dc7b6..4eae6102399d 100644 > > > --- a/net/core/filter.c > > > +++ b/net/core/filter.c > > > @@ -7657,6 +7657,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = { > > > }; > > > > > > const struct bpf_prog_ops flow_dissector_prog_ops = { > > > + .test_run = bpf_prog_test_run_flow_dissector, > > > }; > > > > > > int sk_detach_filter(struct sock *sk) > > > -- > > > 2.20.0.rc1.387.gf8505762e3-goog > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector 2018-12-04 23:25 ` Song Liu @ 2018-12-04 23:36 ` Stanislav Fomichev 0 siblings, 0 replies; 10+ messages in thread From: Stanislav Fomichev @ 2018-12-04 23:36 UTC (permalink / raw) To: Song Liu Cc: Stanislav Fomichev, Networking, David S . Miller, Alexei Starovoitov, Daniel Borkmann, simon.horman On 12/04, Song Liu wrote: > On Mon, Dec 3, 2018 at 3:08 PM Stanislav Fomichev <sdf@fomichev.me> wrote: > > > > On 12/03, Song Liu wrote: > > > On Mon, Dec 3, 2018 at 11:00 AM Stanislav Fomichev <sdf@google.com> wrote: > > > > > > > > The input is packet data, the output is struct bpf_flow_key. This should > > > > make it easy to test flow dissector programs without elaborate > > > > setup. > > > > > > > > Signed-off-by: Stanislav Fomichev <sdf@google.com> > > > > --- > > > > include/linux/bpf.h | 3 ++ > > > > net/bpf/test_run.c | 76 +++++++++++++++++++++++++++++++++++++++++---- > > > > net/core/filter.c | 1 + > > > > 3 files changed, 74 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > > > index e82b7039fc66..7a572d15d5dd 100644 > > > > --- a/include/linux/bpf.h > > > > +++ b/include/linux/bpf.h > > > > @@ -373,6 +373,9 @@ 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); > > > > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > > > > + const union bpf_attr *kattr, > > > > + union bpf_attr __user *uattr); > > > > > > > > /* an array of programs to be executed under rcu_lock. > > > > * > > > > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > > > > index c89c22c49015..bfa05d31c6e3 100644 > > > > --- a/net/bpf/test_run.c > > > > +++ b/net/bpf/test_run.c > > > > @@ -14,21 +14,33 @@ > > > > #include <net/tcp.h> > > > > > > > > static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx, > > > > - struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE]) > > > > + struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE], > > > > + struct bpf_flow_keys *flow_keys) > > > > { > > > > u32 ret; > > > > > > > > preempt_disable(); > > > > rcu_read_lock(); > > > > bpf_cgroup_storage_set(storage); > > > > - ret = BPF_PROG_RUN(prog, ctx); > > > > + > > > > + switch (prog->type) { > > > > + case BPF_PROG_TYPE_FLOW_DISSECTOR: > > > > + ret = __skb_flow_bpf_dissect(prog, ctx, &flow_keys_dissector, > > > > + flow_keys); > > > > + break; > > > > + default: > > > > + ret = BPF_PROG_RUN(prog, ctx); > > > > + break; > > > > + } > > > > + > > > > > > Is it possible to fold the logic above into bpf_prog_test_run_flow_dissector()? > > > In that way, the logic flow is similar to other bpf_prog_test_run_XXX() > > > functions. > > I can probably do everything that __skb_flow_bpf_dissect does inside of > > bpf_prog_test_run_flow_dissector (pass flow_keys in cb); that would remove > > this ugly program type check down the stack. But my additional goal here was > > to test __skb_flow_bpf_dissect itself as well. > > > > Attached some possible prototype below. The only issue I see with that > > approach is that we need to clear flow_keys on each test iteration. > > I think in my current patch I'm actually missing a memset(&flow_keys, 0, ...) > > for each iteration of __skb_flow_bpf_dissect; > > I haven't tested multiple runs :-/ > > > > So I'd prefer to continue doing this prog type check (plus, add a missing > > memset for flow_keys on each iteration in v2). WDYT? > > I think v2 is still using original logic? Actually, after a second > thought, I think In a sense, it does. v2 still passes flow_dissector to the lower level bpf_test_run_one routine, it just passes it in the skb->cb, not explicitly in the function arguments. > it is OK to change bpf_test_run_one() (unless Alexei and/or Daniel has better > suggestions. The only preference I have is that I want to call __skb_flow_bpf_dissect from bpf_test_run_one (i.e. do NOT do elaborate setup in bpf_prog_test_run_flow_dissector so BPF_PROG_RUN in bpf_test_run_one just magically works). I don't really care how we pass flow_keys there (skb->cb vs plumbing the argument) :-) > > Thanks, > Song > > > > > --- > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > index e82b7039fc66..7a572d15d5dd 100644 > > --- a/include/linux/bpf.h > > +++ b/include/linux/bpf.h > > @@ -373,6 +373,9 @@ 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); > > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > > + const union bpf_attr *kattr, > > + union bpf_attr __user *uattr); > > > > /* an array of programs to be executed under rcu_lock. > > * > > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c > > index c89c22c49015..04387ef1f95a 100644 > > --- a/net/bpf/test_run.c > > +++ b/net/bpf/test_run.c > > @@ -220,3 +220,60 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > > kfree(data); > > return ret; > > } > > + > > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > > + const union bpf_attr *kattr, > > + union bpf_attr __user *uattr) > > +{ > > + struct bpf_flow_keys flow_keys = {}; > > + u32 size = kattr->test.data_size_in; > > + u32 repeat = kattr->test.repeat; > > + struct bpf_skb_data_end *cb; > > + u32 retval, duration; > > + struct sk_buff *skb; > > + struct sock *sk; > > + void *data; > > + int ret; > > + > > + if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR) > > + return -EINVAL; > > + > > + data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN, > > + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); > > + if (IS_ERR(data)) > > + return PTR_ERR(data); > > + > > + sk = kzalloc(sizeof(*sk), GFP_USER); > > + if (!sk) { > > + kfree(data); > > + return -ENOMEM; > > + } > > + sock_net_set(sk, current->nsproxy->net_ns); > > + sock_init_data(NULL, sk); > > + > > + skb = build_skb(data, 0); > > + if (!skb) { > > + kfree(data); > > + kfree(sk); > > + return -ENOMEM; > > + } > > + skb->sk = sk; > > + > > + skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); > > + __skb_put(skb, size); > > + skb->protocol = eth_type_trans(skb, > > + current->nsproxy->net_ns->loopback_dev); > > + skb_reset_network_header(skb); > > + > > + cb = (struct bpf_skb_data_end *)skb->cb; > > + memset(cb, 0, sizeof(cb)); > > + cb->qdisc_cb.flow_keys = &flow_keys; > > + > > + retval = bpf_test_run(prog, skb, repeat, &duration); > > + > > + ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys), > > + retval, duration); > > + kfree_skb(skb); > > + kfree(sk); > > + return ret; > > +} > > diff --git a/net/core/filter.c b/net/core/filter.c > > index bd0df75dc7b6..4eae6102399d 100644 > > --- a/net/core/filter.c > > +++ b/net/core/filter.c > > @@ -7657,6 +7657,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = { > > }; > > > > const struct bpf_prog_ops flow_dissector_prog_ops = { > > + .test_run = bpf_prog_test_run_flow_dissector, > > }; > > > > int sk_detach_filter(struct sock *sk) > > > > > Thanks, > > > Song > > > > > > > > > > rcu_read_unlock(); > > > > preempt_enable(); > > > > > > > > return ret; > > > > } > > > > > > > > -static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) > > > > +static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time, > > > > + struct bpf_flow_keys *flow_keys) > > > > { > > > > struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { 0 }; > > > > enum bpf_cgroup_storage_type stype; > > > > @@ -49,7 +61,7 @@ static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 *time) > > > > repeat = 1; > > > > time_start = ktime_get_ns(); > > > > for (i = 0; i < repeat; i++) { > > > > - ret = bpf_test_run_one(prog, ctx, storage); > > > > + ret = bpf_test_run_one(prog, ctx, storage, flow_keys); > > > > if (need_resched()) { > > > > if (signal_pending(current)) > > > > break; > > > > @@ -165,7 +177,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr, > > > > __skb_push(skb, hh_len); > > > > if (is_direct_pkt_access) > > > > bpf_compute_data_pointers(skb); > > > > - retval = bpf_test_run(prog, skb, repeat, &duration); > > > > + retval = bpf_test_run(prog, skb, repeat, &duration, NULL); > > > > if (!is_l2) { > > > > if (skb_headroom(skb) < hh_len) { > > > > int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb)); > > > > @@ -212,7 +224,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > > > > rxqueue = __netif_get_rx_queue(current->nsproxy->net_ns->loopback_dev, 0); > > > > xdp.rxq = &rxqueue->xdp_rxq; > > > > > > > > - retval = bpf_test_run(prog, &xdp, repeat, &duration); > > > > + retval = bpf_test_run(prog, &xdp, repeat, &duration, NULL); > > > > if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN || > > > > xdp.data_end != xdp.data + size) > > > > size = xdp.data_end - xdp.data; > > > > @@ -220,3 +232,55 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const union bpf_attr *kattr, > > > > kfree(data); > > > > return ret; > > > > } > > > > + > > > > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog, > > > > + const union bpf_attr *kattr, > > > > + union bpf_attr __user *uattr) > > > > +{ > > > > + struct bpf_flow_keys flow_keys = {}; > > > > + u32 size = kattr->test.data_size_in; > > > > + u32 repeat = kattr->test.repeat; > > > > + u32 retval, duration; > > > > + struct sk_buff *skb; > > > > + struct sock *sk; > > > > + void *data; > > > > + int ret; > > > > + > > > > + if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR) > > > > + return -EINVAL; > > > > + > > > > + data = bpf_test_init(kattr, size, NET_SKB_PAD + NET_IP_ALIGN, > > > > + SKB_DATA_ALIGN(sizeof(struct skb_shared_info))); > > > > + if (IS_ERR(data)) > > > > + return PTR_ERR(data); > > > > + > > > > + sk = kzalloc(sizeof(*sk), GFP_USER); > > > > + if (!sk) { > > > > + kfree(data); > > > > + return -ENOMEM; > > > > + } > > > > + sock_net_set(sk, current->nsproxy->net_ns); > > > > + sock_init_data(NULL, sk); > > > > + > > > > + skb = build_skb(data, 0); > > > > + if (!skb) { > > > > + kfree(data); > > > > + kfree(sk); > > > > + return -ENOMEM; > > > > + } > > > > + skb->sk = sk; > > > > + > > > > + skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN); > > > > + __skb_put(skb, size); > > > > + skb->protocol = eth_type_trans(skb, > > > > + current->nsproxy->net_ns->loopback_dev); > > > > + skb_reset_network_header(skb); > > > > + > > > > + retval = bpf_test_run(prog, skb, repeat, &duration, &flow_keys); > > > > + > > > > + ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys), > > > > + retval, duration); > > > > + kfree_skb(skb); > > > > + kfree(sk); > > > > + return ret; > > > > +} > > > > diff --git a/net/core/filter.c b/net/core/filter.c > > > > index bd0df75dc7b6..4eae6102399d 100644 > > > > --- a/net/core/filter.c > > > > +++ b/net/core/filter.c > > > > @@ -7657,6 +7657,7 @@ const struct bpf_verifier_ops flow_dissector_verifier_ops = { > > > > }; > > > > > > > > const struct bpf_prog_ops flow_dissector_prog_ops = { > > > > + .test_run = bpf_prog_test_run_flow_dissector, > > > > }; > > > > > > > > int sk_detach_filter(struct sock *sk) > > > > -- > > > > 2.20.0.rc1.387.gf8505762e3-goog > > > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH bpf-next 3/3] selftests/bpf: add simple BPF_PROG_TEST_RUN examples for flow dissector 2018-12-03 18:59 [PATCH bpf-next 0/3] support flow dissector in BPF_PROG_TEST_RUN Stanislav Fomichev 2018-12-03 18:59 ` [PATCH bpf-next 1/3] net/flow_dissector: move bpf case into __skb_flow_bpf_dissect Stanislav Fomichev 2018-12-03 18:59 ` [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector Stanislav Fomichev @ 2018-12-03 18:59 ` Stanislav Fomichev 2 siblings, 0 replies; 10+ messages in thread From: Stanislav Fomichev @ 2018-12-03 18:59 UTC (permalink / raw) To: netdev; +Cc: davem, ast, daniel, simon.horman, Stanislav Fomichev Use existing pkt_v4 and pkt_v6 to make sure flow_keys are what we want. Also, add new bpf_flow_load routine (and flow_dissector_load.h header) that loads bpf_flow.o program and does all required setup. Signed-off-by: Stanislav Fomichev <sdf@google.com> --- tools/testing/selftests/bpf/Makefile | 3 + .../selftests/bpf/flow_dissector_load.c | 43 ++--------- .../selftests/bpf/flow_dissector_load.h | 55 ++++++++++++++ tools/testing/selftests/bpf/test_progs.c | 76 ++++++++++++++++++- 4 files changed, 137 insertions(+), 40 deletions(-) create mode 100644 tools/testing/selftests/bpf/flow_dissector_load.h diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile index 73aa6d8f4a2f..387763e6d137 100644 --- a/tools/testing/selftests/bpf/Makefile +++ b/tools/testing/selftests/bpf/Makefile @@ -19,6 +19,9 @@ all: $(TEST_CUSTOM_PROGS) $(TEST_CUSTOM_PROGS): $(OUTPUT)/%: %.c $(CC) -o $(TEST_CUSTOM_PROGS) -static $< -Wl,--build-id +flow_dissector_load.c: flow_dissector_load.h +test_run.c: flow_dissector_load.h + # Order correspond to 'make run_tests' order TEST_GEN_PROGS = test_verifier test_tag test_maps test_lru_map test_lpm_map test_progs \ test_align test_verifier_log test_dev_cgroup test_tcpbpf_user \ diff --git a/tools/testing/selftests/bpf/flow_dissector_load.c b/tools/testing/selftests/bpf/flow_dissector_load.c index ae8180b11d5f..77cafa66d048 100644 --- a/tools/testing/selftests/bpf/flow_dissector_load.c +++ b/tools/testing/selftests/bpf/flow_dissector_load.c @@ -12,6 +12,7 @@ #include <bpf/libbpf.h> #include "bpf_rlimit.h" +#include "flow_dissector_load.h" const char *cfg_pin_path = "/sys/fs/bpf/flow_dissector"; const char *cfg_map_name = "jmp_table"; @@ -21,46 +22,13 @@ char *cfg_path_name; static void load_and_attach_program(void) { - struct bpf_program *prog, *main_prog; - struct bpf_map *prog_array; - int i, fd, prog_fd, ret; + int prog_fd, ret; struct bpf_object *obj; - int prog_array_fd; - ret = bpf_prog_load(cfg_path_name, BPF_PROG_TYPE_FLOW_DISSECTOR, &obj, - &prog_fd); + ret = bpf_flow_load(&obj, cfg_path_name, cfg_section_name, + cfg_map_name, &prog_fd); if (ret) - error(1, 0, "bpf_prog_load %s", cfg_path_name); - - main_prog = bpf_object__find_program_by_title(obj, cfg_section_name); - if (!main_prog) - error(1, 0, "bpf_object__find_program_by_title %s", - cfg_section_name); - - prog_fd = bpf_program__fd(main_prog); - if (prog_fd < 0) - error(1, 0, "bpf_program__fd"); - - prog_array = bpf_object__find_map_by_name(obj, cfg_map_name); - if (!prog_array) - error(1, 0, "bpf_object__find_map_by_name %s", cfg_map_name); - - prog_array_fd = bpf_map__fd(prog_array); - if (prog_array_fd < 0) - error(1, 0, "bpf_map__fd %s", cfg_map_name); - - i = 0; - bpf_object__for_each_program(prog, obj) { - fd = bpf_program__fd(prog); - if (fd < 0) - error(1, 0, "bpf_program__fd"); - - if (fd != prog_fd) { - printf("%d: %s\n", i, bpf_program__title(prog, false)); - bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY); - ++i; - } - } + error(1, 0, "bpf_flow_load %s", cfg_path_name); ret = bpf_prog_attach(prog_fd, 0 /* Ignore */, BPF_FLOW_DISSECTOR, 0); if (ret) @@ -69,7 +37,6 @@ static void load_and_attach_program(void) ret = bpf_object__pin(obj, cfg_pin_path); if (ret) error(1, 0, "bpf_object__pin %s", cfg_pin_path); - } static void detach_program(void) diff --git a/tools/testing/selftests/bpf/flow_dissector_load.h b/tools/testing/selftests/bpf/flow_dissector_load.h new file mode 100644 index 000000000000..9f2fd38e30b0 --- /dev/null +++ b/tools/testing/selftests/bpf/flow_dissector_load.h @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef FLOW_DISSECTOR_LOAD +#define FLOW_DISSECTOR_LOAD + +#include <bpf/bpf.h> +#include <bpf/libbpf.h> + +static inline int bpf_flow_load(struct bpf_object **obj, + const char *path, + const char *section_name, + const char *map_name, + int *prog_fd) +{ + struct bpf_program *prog, *main_prog; + struct bpf_map *prog_array; + int prog_array_fd; + int ret, fd, i; + + ret = bpf_prog_load(path, BPF_PROG_TYPE_FLOW_DISSECTOR, obj, + prog_fd); + if (ret) + return ret; + + main_prog = bpf_object__find_program_by_title(*obj, section_name); + if (!main_prog) + return ret; + + *prog_fd = bpf_program__fd(main_prog); + if (*prog_fd < 0) + return ret; + + prog_array = bpf_object__find_map_by_name(*obj, map_name); + if (!prog_array) + return ret; + + prog_array_fd = bpf_map__fd(prog_array); + if (prog_array_fd < 0) + return ret; + + i = 0; + bpf_object__for_each_program(prog, *obj) { + fd = bpf_program__fd(prog); + if (fd < 0) + return fd; + + if (fd != *prog_fd) { + bpf_map_update_elem(prog_array_fd, &i, &fd, BPF_ANY); + ++i; + } + } + + return 0; +} + +#endif /* FLOW_DISSECTOR_LOAD */ diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 1c57abbe0945..fa8a307bba9c 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -39,6 +39,7 @@ typedef __u16 __sum16; #include "bpf_endian.h" #include "bpf_rlimit.h" #include "trace_helpers.h" +#include "flow_dissector_load.h" static int error_cnt, pass_cnt; static bool jit_enabled; @@ -53,7 +54,7 @@ static struct { } __packed pkt_v4 = { .eth.h_proto = bpf_htons(ETH_P_IP), .iph.ihl = 5, - .iph.protocol = 6, + .iph.protocol = IPPROTO_TCP, .iph.tot_len = bpf_htons(MAGIC_BYTES), .tcp.urg_ptr = 123, }; @@ -65,7 +66,7 @@ static struct { struct tcphdr tcp; } __packed pkt_v6 = { .eth.h_proto = bpf_htons(ETH_P_IPV6), - .iph.nexthdr = 6, + .iph.nexthdr = IPPROTO_TCP, .iph.payload_len = bpf_htons(MAGIC_BYTES), .tcp.urg_ptr = 123, }; @@ -1830,6 +1831,76 @@ static void test_queue_stack_map(int type) bpf_object__close(obj); } +#define CHECK_FLOW_KEYS(desc, got, expected) \ + CHECK(memcmp(&got, &expected, sizeof(got)) != 0, \ + desc, \ + "nhoff=%u/%u " \ + "thoff=%u/%u " \ + "addr_proto=0x%x/0x%x " \ + "is_frag=%u/%u " \ + "is_first_frag=%u/%u " \ + "is_encap=%u/%u " \ + "n_proto=0x%x/0x%x " \ + "sport=%u/%u " \ + "dport=%u/%u\n", \ + got.nhoff, expected.nhoff, \ + got.thoff, expected.thoff, \ + got.addr_proto, expected.addr_proto, \ + got.is_frag, expected.is_frag, \ + got.is_first_frag, expected.is_first_frag, \ + got.is_encap, expected.is_encap, \ + got.n_proto, expected.n_proto, \ + got.sport, expected.sport, \ + got.dport, expected.dport) + +static struct bpf_flow_keys pkt_v4_flow_keys = { + .nhoff = sizeof(struct iphdr), + .thoff = 0, + .addr_proto = ETH_P_IP, + .ip_proto = IPPROTO_TCP, + .n_proto = bpf_htons(ETH_P_IP), +}; + +static struct bpf_flow_keys pkt_v6_flow_keys = { + .nhoff = sizeof(struct ipv6hdr), + .thoff = 0, + .addr_proto = ETH_P_IPV6, + .ip_proto = IPPROTO_TCP, + .n_proto = bpf_htons(ETH_P_IPV6), +}; + +static void test_flow_dissector(void) +{ + struct bpf_flow_keys flow_keys; + struct bpf_object *obj; + __u32 duration, retval; + int err, prog_fd; + __u32 size; + + err = bpf_flow_load(&obj, "./bpf_flow.o", "flow_dissector", + "jmp_table", &prog_fd); + if (err) { + error_cnt++; + return; + } + + err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4), + &flow_keys, &size, &retval, &duration); + CHECK(size != sizeof(flow_keys) || err || retval, "ipv4", + "err %d errno %d retval %d duration %d size %u/%lu\n", + err, errno, retval, duration, size, sizeof(flow_keys)); + CHECK_FLOW_KEYS("ipv4_flow_keys", flow_keys, pkt_v4_flow_keys); + + err = bpf_prog_test_run(prog_fd, 1, &pkt_v6, sizeof(pkt_v6), + &flow_keys, &size, &retval, &duration); + CHECK(size != sizeof(flow_keys) || err || retval, "ipv6", + "err %d errno %d retval %d duration %d size %u/%lu\n", + err, errno, retval, duration, size, sizeof(flow_keys)); + CHECK_FLOW_KEYS("ipv6_flow_keys", flow_keys, pkt_v6_flow_keys); + + bpf_object__close(obj); +} + int main(void) { srand(time(NULL)); @@ -1856,6 +1927,7 @@ int main(void) test_reference_tracking(); test_queue_stack_map(QUEUE); test_queue_stack_map(STACK); + test_flow_dissector(); printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt); return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS; -- 2.20.0.rc1.387.gf8505762e3-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH bpf-next 0/3] support flow dissector in BPF_PROG_TEST_RUN
@ 2019-01-22 21:23 Stanislav Fomichev
2019-01-22 21:23 ` [PATCH bpf-next 1/3] net/flow_dissector: move bpf case into __skb_flow_bpf_dissect Stanislav Fomichev
0 siblings, 1 reply; 10+ messages in thread
From: Stanislav Fomichev @ 2019-01-22 21:23 UTC (permalink / raw)
To: netdev; +Cc: davem, ast, daniel, Stanislav Fomichev
This patch series adds support for testing flow dissector BPF programs by
extending already existing BPF_PROG_TEST_RUN. The goal is to have a
packet as an input and `struct bpf_flow_key' as an output. That way
we can easily test flow dissector programs' behavior.
I've also modified existing test_progs.c test to do a simple flow
dissector run as well.
* first patch introduces new __skb_flow_bpf_dissect to simplify
sharing between __skb_flow_bpf_dissect and BPF_PROG_TEST_RUN
* second patch adds actual BPF_PROG_TEST_RUN support
* third patch adds example usage to the selftests
Stanislav Fomichev (3):
net/flow_dissector: move bpf case into __skb_flow_bpf_dissect
bpf: add BPF_PROG_TEST_RUN support for flow dissector
selftests/bpf: add simple BPF_PROG_TEST_RUN examples for flow
dissector
include/linux/bpf.h | 3 +
include/linux/skbuff.h | 5 +
net/bpf/test_run.c | 75 ++++++++++++++-
net/core/filter.c | 1 +
net/core/flow_dissector.c | 92 +++++++++++--------
tools/testing/selftests/bpf/Makefile | 3 +
.../selftests/bpf/flow_dissector_load.c | 43 +--------
.../selftests/bpf/flow_dissector_load.h | 55 +++++++++++
tools/testing/selftests/bpf/test_progs.c | 78 +++++++++++++++-
9 files changed, 276 insertions(+), 79 deletions(-)
create mode 100644 tools/testing/selftests/bpf/flow_dissector_load.h
--
2.20.1.321.g9e740568ce-goog
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH bpf-next 1/3] net/flow_dissector: move bpf case into __skb_flow_bpf_dissect 2019-01-22 21:23 [PATCH bpf-next 0/3] support flow dissector in BPF_PROG_TEST_RUN Stanislav Fomichev @ 2019-01-22 21:23 ` Stanislav Fomichev 0 siblings, 0 replies; 10+ messages in thread From: Stanislav Fomichev @ 2019-01-22 21:23 UTC (permalink / raw) To: netdev; +Cc: davem, ast, daniel, Stanislav Fomichev This way, we can reuse it for flow dissector in BPF_PROG_TEST_RUN. No functional changes. Signed-off-by: Stanislav Fomichev <sdf@google.com> --- include/linux/skbuff.h | 5 +++ net/core/flow_dissector.c | 92 +++++++++++++++++++++++---------------- 2 files changed, 59 insertions(+), 38 deletions(-) diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 93f56fddd92a..be762fc34ff3 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -1221,6 +1221,11 @@ static inline int skb_flow_dissector_bpf_prog_detach(const union bpf_attr *attr) } #endif +struct bpf_flow_keys; +bool __skb_flow_bpf_dissect(struct bpf_prog *prog, + const struct sk_buff *skb, + struct flow_dissector *flow_dissector, + struct bpf_flow_keys *flow_keys); bool __skb_flow_dissect(const struct sk_buff *skb, struct flow_dissector *flow_dissector, void *target_container, diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c index 9f2840510e63..bb1a54747d64 100644 --- a/net/core/flow_dissector.c +++ b/net/core/flow_dissector.c @@ -683,6 +683,46 @@ static void __skb_flow_bpf_to_target(const struct bpf_flow_keys *flow_keys, } } +bool __skb_flow_bpf_dissect(struct bpf_prog *prog, + const struct sk_buff *skb, + struct flow_dissector *flow_dissector, + struct bpf_flow_keys *flow_keys) +{ + struct bpf_skb_data_end cb_saved; + struct bpf_skb_data_end *cb; + u32 result; + + /* Note that even though the const qualifier is discarded + * throughout the execution of the BPF program, all changes(the + * control block) are reverted after the BPF program returns. + * Therefore, __skb_flow_dissect does not alter the skb. + */ + + cb = (struct bpf_skb_data_end *)skb->cb; + + /* Save Control Block */ + memcpy(&cb_saved, cb, sizeof(cb_saved)); + memset(cb, 0, sizeof(*cb)); + + /* Pass parameters to the BPF program */ + memset(flow_keys, 0, sizeof(*flow_keys)); + cb->qdisc_cb.flow_keys = flow_keys; + flow_keys->nhoff = skb_network_offset(skb); + flow_keys->thoff = flow_keys->nhoff; + + bpf_compute_data_pointers((struct sk_buff *)skb); + result = BPF_PROG_RUN(prog, skb); + + /* Restore state */ + memcpy(cb, &cb_saved, sizeof(cb_saved)); + + flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff, 0, skb->len); + flow_keys->thoff = clamp_t(u16, flow_keys->thoff, + flow_keys->nhoff, skb->len); + + return result == BPF_OK; +} + /** * __skb_flow_dissect - extract the flow_keys struct and return it * @skb: sk_buff to extract the flow from, can be NULL if the rest are specified @@ -714,7 +754,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb, struct flow_dissector_key_vlan *key_vlan; enum flow_dissect_ret fdret; enum flow_dissector_key_id dissector_vlan = FLOW_DISSECTOR_KEY_MAX; - struct bpf_prog *attached = NULL; int num_hdrs = 0; u8 ip_proto = 0; bool ret; @@ -754,53 +793,30 @@ bool __skb_flow_dissect(const struct sk_buff *skb, FLOW_DISSECTOR_KEY_BASIC, target_container); - rcu_read_lock(); if (skb) { + struct bpf_flow_keys flow_keys; + struct bpf_prog *attached = NULL; + + rcu_read_lock(); + if (skb->dev) attached = rcu_dereference(dev_net(skb->dev)->flow_dissector_prog); else if (skb->sk) attached = rcu_dereference(sock_net(skb->sk)->flow_dissector_prog); else WARN_ON_ONCE(1); - } - if (attached) { - /* Note that even though the const qualifier is discarded - * throughout the execution of the BPF program, all changes(the - * control block) are reverted after the BPF program returns. - * Therefore, __skb_flow_dissect does not alter the skb. - */ - struct bpf_flow_keys flow_keys = {}; - struct bpf_skb_data_end cb_saved; - struct bpf_skb_data_end *cb; - u32 result; - - cb = (struct bpf_skb_data_end *)skb->cb; - - /* Save Control Block */ - memcpy(&cb_saved, cb, sizeof(cb_saved)); - memset(cb, 0, sizeof(cb_saved)); - /* Pass parameters to the BPF program */ - cb->qdisc_cb.flow_keys = &flow_keys; - flow_keys.nhoff = nhoff; - flow_keys.thoff = nhoff; - - bpf_compute_data_pointers((struct sk_buff *)skb); - result = BPF_PROG_RUN(attached, skb); - - /* Restore state */ - memcpy(cb, &cb_saved, sizeof(cb_saved)); - - flow_keys.nhoff = clamp_t(u16, flow_keys.nhoff, 0, skb->len); - flow_keys.thoff = clamp_t(u16, flow_keys.thoff, - flow_keys.nhoff, skb->len); - - __skb_flow_bpf_to_target(&flow_keys, flow_dissector, - target_container); + if (attached) { + ret = __skb_flow_bpf_dissect(attached, skb, + flow_dissector, + &flow_keys); + __skb_flow_bpf_to_target(&flow_keys, flow_dissector, + target_container); + rcu_read_unlock(); + return ret; + } rcu_read_unlock(); - return result == BPF_OK; } - rcu_read_unlock(); if (dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_ETH_ADDRS)) { -- 2.20.1.321.g9e740568ce-goog ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-01-22 21:23 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-12-03 18:59 [PATCH bpf-next 0/3] support flow dissector in BPF_PROG_TEST_RUN Stanislav Fomichev 2018-12-03 18:59 ` [PATCH bpf-next 1/3] net/flow_dissector: move bpf case into __skb_flow_bpf_dissect Stanislav Fomichev 2018-12-03 18:59 ` [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector Stanislav Fomichev 2018-12-03 22:28 ` Song Liu 2018-12-03 23:08 ` Stanislav Fomichev 2018-12-04 3:54 ` Stanislav Fomichev 2018-12-04 23:25 ` Song Liu 2018-12-04 23:36 ` Stanislav Fomichev 2018-12-03 18:59 ` [PATCH bpf-next 3/3] selftests/bpf: add simple BPF_PROG_TEST_RUN examples " Stanislav Fomichev -- strict thread matches above, loose matches on Subject: below -- 2019-01-22 21:23 [PATCH bpf-next 0/3] support flow dissector in BPF_PROG_TEST_RUN Stanislav Fomichev 2019-01-22 21:23 ` [PATCH bpf-next 1/3] net/flow_dissector: move bpf case into __skb_flow_bpf_dissect Stanislav Fomichev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).