[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190126005613.GE26773@mini-arch>
Date: Fri, 25 Jan 2019 16:56:13 -0800
From: Stanislav Fomichev <sdf@...ichev.me>
To: Song Liu <liu.song.a23@...il.com>
Cc: Stanislav Fomichev <sdf@...gle.com>,
Networking <netdev@...r.kernel.org>,
"David S . Miller" <davem@...emloft.net>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>
Subject: Re: [PATCH bpf-next v2 2/3] bpf: add BPF_PROG_TEST_RUN support for
flow dissector
On 01/25, Song Liu wrote:
> On Thu, Jan 24, 2019 at 8:53 AM Stanislav Fomichev <sdf@...gle.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@...gle.com>
> > ---
> > include/linux/bpf.h | 3 ++
> > net/bpf/test_run.c | 82 +++++++++++++++++++++++++++++++++++++++++++++
> > net/core/filter.c | 1 +
> > 3 files changed, 86 insertions(+)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index e734f163bd0b..701ef954a258 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -397,6 +397,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 fa2644d276ef..2c5172b33209 100644
> > --- a/net/bpf/test_run.c
> > +++ b/net/bpf/test_run.c
> > @@ -240,3 +240,85 @@ 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)
>
> I think this function duplicates a lot of logic from bpf_prog_test_run_skb().
> Can we somehow reuse bpf_prog_test_run_skb()?
I did that initially
(https://marc.info/?l=linux-netdev&m=154830227529929&w=2), but then
Alexey suggested that there is not much to reuse (plus can hinder the
test performance of the existing types).
>
> Thanks,
> Song
>
>
> > +{
> > + u32 size = kattr->test.data_size_in;
> > + u32 repeat = kattr->test.repeat;
> > + struct bpf_flow_keys flow_keys;
> > + u64 time_start, time_spent = 0;
> > + struct bpf_skb_data_end *cb;
> > + u32 retval, duration;
> > + struct sk_buff *skb;
> > + struct sock *sk;
> > + void *data;
> > + int ret;
> > + u32 i;
> > +
> > + 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;
> > + cb->qdisc_cb.flow_keys = &flow_keys;
> > +
> > + if (!repeat)
> > + repeat = 1;
> > +
> > + time_start = ktime_get_ns();
> > + for (i = 0; i < repeat; i++) {
> > + preempt_disable();
> > + rcu_read_lock();
> > + retval = __skb_flow_bpf_dissect(prog, skb,
> > + &flow_keys_dissector,
> > + &flow_keys);
> > + rcu_read_unlock();
> > + preempt_enable();
> > +
> > + if (need_resched()) {
> > + if (signal_pending(current))
> > + break;
> > + time_spent += ktime_get_ns() - time_start;
> > + cond_resched();
> > + time_start = ktime_get_ns();
> > + }
> > + }
> > + time_spent += ktime_get_ns() - time_start;
> > + do_div(time_spent, repeat);
> > + duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> > +
> > + 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 2b3b436ef545..ff4641dae2be 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -7690,6 +7690,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.1.321.g9e740568ce-goog
> >
Powered by blists - more mailing lists