* [PATCH net-next 1/4] sctp: remove reconf_enable from asoc
From: Xin Long @ 2019-07-08 16:57 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
In-Reply-To: <cover.1562604972.git.lucien.xin@gmail.com>
asoc's reconf support is actually decided by the 4-shakehand negotiation,
not something that users can set by sockopt. asoc->peer.reconf_capable is
working for this. So remove it from asoc.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/net/sctp/structs.h | 3 +--
net/sctp/associola.c | 1 -
net/sctp/sm_make_chunk.c | 5 ++---
net/sctp/socket.c | 7 ++-----
4 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
index 0767701..d9e0e1a 100644
--- a/include/net/sctp/structs.h
+++ b/include/net/sctp/structs.h
@@ -2051,8 +2051,7 @@ struct sctp_association {
temp:1, /* Is it a temporary association? */
force_delay:1,
intl_enable:1,
- prsctp_enable:1,
- reconf_enable:1;
+ prsctp_enable:1;
__u8 strreset_enable;
__u8 strreset_outstanding; /* request param count on the fly */
diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 1999237..321c199 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -262,7 +262,6 @@ static struct sctp_association *sctp_association_init(
asoc->active_key_id = ep->active_key_id;
asoc->prsctp_enable = ep->prsctp_enable;
- asoc->reconf_enable = ep->reconf_enable;
asoc->strreset_enable = ep->strreset_enable;
/* Save the hmacs and chunks list into this association */
diff --git a/net/sctp/sm_make_chunk.c b/net/sctp/sm_make_chunk.c
index 9b0e5b0..d784dc1 100644
--- a/net/sctp/sm_make_chunk.c
+++ b/net/sctp/sm_make_chunk.c
@@ -261,7 +261,7 @@ struct sctp_chunk *sctp_make_init(const struct sctp_association *asoc,
num_ext += 2;
}
- if (asoc->reconf_enable) {
+ if (asoc->ep->reconf_enable) {
extensions[num_ext] = SCTP_CID_RECONF;
num_ext += 1;
}
@@ -2007,8 +2007,7 @@ static void sctp_process_ext_param(struct sctp_association *asoc,
for (i = 0; i < num_ext; i++) {
switch (param.ext->chunks[i]) {
case SCTP_CID_RECONF:
- if (asoc->reconf_enable &&
- !asoc->peer.reconf_capable)
+ if (asoc->ep->reconf_enable)
asoc->peer.reconf_capable = 1;
break;
case SCTP_CID_FWD_TSN:
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 39ea0a3..0424876 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4226,10 +4226,7 @@ static int sctp_setsockopt_reconfig_supported(struct sock *sk,
sctp_style(sk, UDP))
goto out;
- if (asoc)
- asoc->reconf_enable = !!params.assoc_value;
- else
- sctp_sk(sk)->ep->reconf_enable = !!params.assoc_value;
+ sctp_sk(sk)->ep->reconf_enable = !!params.assoc_value;
retval = 0;
@@ -7554,7 +7551,7 @@ static int sctp_getsockopt_reconfig_supported(struct sock *sk, int len,
goto out;
}
- params.assoc_value = asoc ? asoc->reconf_enable
+ params.assoc_value = asoc ? asoc->peer.reconf_capable
: sctp_sk(sk)->ep->reconf_enable;
if (put_user(len, optlen))
--
2.1.0
^ permalink raw reply related
* [PATCH net-next 0/4] sctp: tidy up some ep and asoc feature flags
From: Xin Long @ 2019-07-08 16:57 UTC (permalink / raw)
To: network dev, linux-sctp; +Cc: Marcelo Ricardo Leitner, Neil Horman, davem
This patchset is to remove some unnecessary feature flags from
sctp_assocation and move some others to the right places.
Xin Long (4):
sctp: remove reconf_enable from asoc
sctp: remove prsctp_enable from asoc
sctp: rename asoc intl_enable to asoc peer.intl_capable
sctp: rename sp strm_interleave to ep intl_enable
include/net/sctp/structs.h | 37 ++++++++++++++++++-------------------
net/sctp/associola.c | 2 --
net/sctp/sm_make_chunk.c | 21 ++++++++++-----------
net/sctp/socket.c | 19 ++++++++-----------
net/sctp/stream_interleave.c | 4 ++--
net/sctp/stream_sched.c | 2 +-
6 files changed, 39 insertions(+), 46 deletions(-)
--
2.1.0
^ permalink raw reply
* Re: [bpf-next v2 08/10] bpf: Implement bpf_prog_test_run for perf event programs
From: Krzesimir Nowak @ 2019-07-08 16:51 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: netdev, Alban Crequy, Iago López Galeiras,
Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
Yonghong Song, linux-kernel, bpf
In-Reply-To: <20190626161231.GA4866@mini-arch>
On Wed, Jun 26, 2019 at 6:12 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
>
> On 06/26, Krzesimir Nowak wrote:
> > On Tue, Jun 25, 2019 at 10:12 PM Stanislav Fomichev <sdf@fomichev.me> wrote:
> > >
> > > On 06/25, Krzesimir Nowak wrote:
> > > > As an input, test run for perf event program takes struct
> > > > bpf_perf_event_data as ctx_in and struct bpf_perf_event_value as
> > > > data_in. For an output, it basically ignores ctx_out and data_out.
> > > >
> > > > The implementation sets an instance of struct bpf_perf_event_data_kern
> > > > in such a way that the BPF program reading data from context will
> > > > receive what we passed to the bpf prog test run in ctx_in. Also BPF
> > > > program can call bpf_perf_prog_read_value to receive what was passed
> > > > in data_in.
> > > >
> > > > Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
> > > > ---
> > > > kernel/trace/bpf_trace.c | 107 ++++++++++++++++++
> > > > .../bpf/verifier/perf_event_sample_period.c | 8 ++
> > > > 2 files changed, 115 insertions(+)
> > > >
> > > > diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
> > > > index c102c240bb0b..2fa49ea8a475 100644
> > > > --- a/kernel/trace/bpf_trace.c
> > > > +++ b/kernel/trace/bpf_trace.c
> > > > @@ -16,6 +16,8 @@
> > > >
> > > > #include <asm/tlb.h>
> > > >
> > > > +#include <trace/events/bpf_test_run.h>
> > > > +
> > > > #include "trace_probe.h"
> > > > #include "trace.h"
> > > >
> > > > @@ -1160,7 +1162,112 @@ const struct bpf_verifier_ops perf_event_verifier_ops = {
> > > > .convert_ctx_access = pe_prog_convert_ctx_access,
> > > > };
> > > >
> > > > +static int pe_prog_test_run(struct bpf_prog *prog,
> > > > + const union bpf_attr *kattr,
> > > > + union bpf_attr __user *uattr)
> > > > +{
> > > > + void __user *ctx_in = u64_to_user_ptr(kattr->test.ctx_in);
> > > > + void __user *data_in = u64_to_user_ptr(kattr->test.data_in);
> > > > + u32 data_size_in = kattr->test.data_size_in;
> > > > + u32 ctx_size_in = kattr->test.ctx_size_in;
> > > > + u32 repeat = kattr->test.repeat;
> > > > + u32 retval = 0, duration = 0;
> > > > + int err = -EINVAL;
> > > > + u64 time_start, time_spent = 0;
> > > > + int i;
> > > > + struct perf_sample_data sample_data = {0, };
> > > > + struct perf_event event = {0, };
> > > > + struct bpf_perf_event_data_kern real_ctx = {0, };
> > > > + struct bpf_perf_event_data fake_ctx = {0, };
> > > > + struct bpf_perf_event_value value = {0, };
> > > > +
> > > > + if (ctx_size_in != sizeof(fake_ctx))
> > > > + goto out;
> > > > + if (data_size_in != sizeof(value))
> > > > + goto out;
> > > > +
> > > > + if (copy_from_user(&fake_ctx, ctx_in, ctx_size_in)) {
> > > > + err = -EFAULT;
> > > > + goto out;
> > > > + }
> > > Move this to net/bpf/test_run.c? I have a bpf_ctx_init helper to deal
> > > with ctx input, might save you some code above wrt ctx size/etc.
> >
> > My impression about net/bpf/test_run.c was that it was a collection of
> > helpers for test runs of the network-related BPF programs, because
> > they are so similar to each other. So kernel/trace/bpf_trace.c looked
> > like an obvious place for the test_run implementation since other perf
> > trace BPF stuff was already there.
> Maybe net/bpf/test_run.c should be renamed to kernel/bpf/test_run.c?
Just sent another version of this patch series. I went with slightly
different approach - moved some functions to kernel/bpf/test_run.c and
left the network specific stuff in net/bpf/test_run.c.
>
> > And about bpf_ctx_init - looks useful as it seems to me that it
> > handles the scenario where the size of the ctx struct grows, but still
> > allows passing older version of the struct (thus smaller) from
> > userspace for compatibility. Maybe that checking and copying part of
> > the function could be moved into some non-static helper function, so I
> > could use it and still skip the need for allocating memory for the
> > context?
> You can always make bpf_ctx_init non-static and export it.
> But, again, consider adding your stuff to the net/bpf/test_run.c
> and exporting only pe_prog_test_run. That way you can reuse
> bpf_ctx_init and bpf_test_run.
>
> Why do you care about memory allocation though? It's a one time
> operation and doesn't affect the performance measurements.
>
> > > > + if (copy_from_user(&value, data_in, data_size_in)) {
> > > > + err = -EFAULT;
> > > > + goto out;
> > > > + }
> > > > +
> > > > + real_ctx.regs = &fake_ctx.regs;
> > > > + real_ctx.data = &sample_data;
> > > > + real_ctx.event = &event;
> > > > + perf_sample_data_init(&sample_data, fake_ctx.addr,
> > > > + fake_ctx.sample_period);
> > > > + event.cpu = smp_processor_id();
> > > > + event.oncpu = -1;
> > > > + event.state = PERF_EVENT_STATE_OFF;
> > > > + local64_set(&event.count, value.counter);
> > > > + event.total_time_enabled = value.enabled;
> > > > + event.total_time_running = value.running;
> > > > + /* make self as a leader - it is used only for checking the
> > > > + * state field
> > > > + */
> > > > + event.group_leader = &event;
> > > > +
> > > > + /* slightly changed copy pasta from bpf_test_run() in
> > > > + * net/bpf/test_run.c
> > > > + */
> > > > + if (!repeat)
> > > > + repeat = 1;
> > > > +
> > > > + rcu_read_lock();
> > > > + preempt_disable();
> > > > + time_start = ktime_get_ns();
> > > > + for (i = 0; i < repeat; i++) {
> > > Any reason for not using bpf_test_run?
> >
> > Two, mostly. One was that it is a static function and my code was
> > elsewhere. Second was that it does some cgroup storage setup and I'm
> > not sure if the perf event BPF program needs that.
> You can always make it non-static.
>
> Regarding cgroup storage: do we care? If you can see it affecting
> your performance numbers, then yes, but you can try to measure to see
> if it gives you any noticeable overhead. Maybe add an argument to
> bpf_test_run to skip cgroup storage stuff?
>
> > > > + retval = BPF_PROG_RUN(prog, &real_ctx);
> > > > +
> > > > + if (signal_pending(current)) {
> > > > + err = -EINTR;
> > > > + preempt_enable();
> > > > + rcu_read_unlock();
> > > > + goto out;
> > > > + }
> > > > +
> > > > + if (need_resched()) {
> > > > + time_spent += ktime_get_ns() - time_start;
> > > > + preempt_enable();
> > > > + rcu_read_unlock();
> > > > +
> > > > + cond_resched();
> > > > +
> > > > + rcu_read_lock();
> > > > + preempt_disable();
> > > > + time_start = ktime_get_ns();
> > > > + }
> > > > + }
> > > > + time_spent += ktime_get_ns() - time_start;
> > > > + preempt_enable();
> > > > + rcu_read_unlock();
> > > > +
> > > > + do_div(time_spent, repeat);
> > > > + duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> > > > + /* end of slightly changed copy pasta from bpf_test_run() in
> > > > + * net/bpf/test_run.c
> > > > + */
> > > > +
> > > > + if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval))) {
> > > > + err = -EFAULT;
> > > > + goto out;
> > > > + }
> > > > + if (copy_to_user(&uattr->test.duration, &duration, sizeof(duration))) {
> > > > + err = -EFAULT;
> > > > + goto out;
> > > > + }
> > > Can BPF program modify fake_ctx? Do we need/want to copy it back?
> >
> > Reading the pe_prog_is_valid_access function tells me that it's not
> > possible - the only type of valid access is read. So maybe I should be
> > stricter about the requirements for the data_out and ctx_out sizes
> > (should be zero or return -EINVAL).
> Yes, better to explicitly prohibit anything that we don't support.
>
> > > > + err = 0;
> > > > +out:
> > > > + trace_bpf_test_finish(&err);
> > > > + return err;
> > > > +}
> > > > +
> > > > const struct bpf_prog_ops perf_event_prog_ops = {
> > > > + .test_run = pe_prog_test_run,
> > > > };
> > > >
> > > > static DEFINE_MUTEX(bpf_event_mutex);
> > > > diff --git a/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c b/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
> > > > index 471c1a5950d8..16e9e5824d14 100644
> > > > --- a/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
> > > > +++ b/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
> > > This should probably go in another patch.
> >
> > Yeah, I was wondering about it. These changes are here to avoid
> > breaking the tests, since perf event program can actually be run now
> > and the test_run for perf event required certain sizes for ctx and
> > data.
> You need to make sure the context is optional, that way you don't break
> any existing tests out in the wild and can move those changes to
> another patch.
>
> > So, I will either move them to a separate patch or rework the test_run
> > for perf event to accept the size between 0 and sizeof(struct
> > something), so the changes in tests maybe will not be necessary.
> >
> > >
> > > > @@ -13,6 +13,8 @@
> > > > },
> > > > .result = ACCEPT,
> > > > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > > > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > > > + .data_len = sizeof(struct bpf_perf_event_value),
> > > > },
> > > > {
> > > > "check bpf_perf_event_data->sample_period half load permitted",
> > > > @@ -29,6 +31,8 @@
> > > > },
> > > > .result = ACCEPT,
> > > > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > > > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > > > + .data_len = sizeof(struct bpf_perf_event_value),
> > > > },
> > > > {
> > > > "check bpf_perf_event_data->sample_period word load permitted",
> > > > @@ -45,6 +49,8 @@
> > > > },
> > > > .result = ACCEPT,
> > > > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > > > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > > > + .data_len = sizeof(struct bpf_perf_event_value),
> > > > },
> > > > {
> > > > "check bpf_perf_event_data->sample_period dword load permitted",
> > > > @@ -56,4 +62,6 @@
> > > > },
> > > > .result = ACCEPT,
> > > > .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> > > > + .ctx_len = sizeof(struct bpf_perf_event_data),
> > > > + .data_len = sizeof(struct bpf_perf_event_value),
> > > > },
> > > > --
> > > > 2.20.1
> > > >
> >
> >
> >
> > --
> > Kinvolk GmbH | Adalbertstr.6a, 10999 Berlin | tel: +491755589364
> > Geschäftsführer/Directors: Alban Crequy, Chris Kühl, Iago López Galeiras
> > Registergericht/Court of registration: Amtsgericht Charlottenburg
> > Registernummer/Registration number: HRB 171414 B
> > Ust-ID-Nummer/VAT ID number: DE302207000
--
Kinvolk GmbH | Adalbertstr.6a, 10999 Berlin | tel: +491755589364
Geschäftsführer/Directors: Alban Crequy, Chris Kühl, Iago López Galeiras
Registergericht/Court of registration: Amtsgericht Charlottenburg
Registernummer/Registration number: HRB 171414 B
Ust-ID-Nummer/VAT ID number: DE302207000
^ permalink raw reply
* Re: [PATCH bpf-next 0/3] xdp: Add devmap_hash map type
From: Jonathan Lemon @ 2019-07-08 16:50 UTC (permalink / raw)
To: Toke Høiland-Jørgensen
Cc: Daniel Borkmann, Alexei Starovoitov, netdev, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <87bly4zg8n.fsf@toke.dk>
On 8 Jul 2019, at 8:40, Toke Høiland-Jørgensen wrote:
> "Jonathan Lemon" <jlemon@flugsvamp.com> writes:
>
>> On 5 Jul 2019, at 10:56, Toke Høiland-Jørgensen wrote:
>>
>>> This series adds a new map type, devmap_hash, that works like the
>>> existing
>>> devmap type, but using a hash-based indexing scheme. This is useful
>>> for the use
>>> case where a devmap is indexed by ifindex (for instance for use with
>>> the routing
>>> table lookup helper). For this use case, the regular devmap needs to
>>> be sized
>>> after the maximum ifindex number, not the number of devices in it. A
>>> hash-based
>>> indexing scheme makes it possible to size the map after the number of
>>> devices it
>>> should contain instead.
>>
>> This device hash map is sized at NETDEV_HASHENTRIES == 2^8 == 256. Is
>> this actually smaller than an array? What ifindex values are you
>> seeing?
>
> Well, not in all cases, certainly. But machines with lots of virtual
> interfaces (e.g., container hosts) can easily exceed that. Also, for a
> devmap we charge the full size of max_entries * struct bpf_dtab_netdev
> towards the locked memory cost on map creation. And since sizeof(struct
> bpf_dtab_netdev) is 64, the size of the hashmap only corresponds to 32
> entries...
>
> But more importantly, it's a UI issue: Say you want to create a simple
> program that uses the fib_lookup helper (something like the xdp_fwd
> example under samples/bpf/). You know that you only want to route
> between a couple of interfaces, so you naturally create a devmap that
> can hold, say, 8 entries (just to be sure). This works fine on your
> initial test, where the machine only has a couple of physical interfaces
> brought up at boot. But then you try to run the same program on your
> production server, where the interfaces you need to use just happen to
> have ifindexes higher than 8, and now it breaks for no discernible
> reason. Or even worse, if you remove and re-add an interface, you may no
> longer be able to insert it into your map because the ifindex changed...
Thanks for the explanation, that makes sense.
--
Jonathan
^ permalink raw reply
* Re: [PATCH 1/1] tools/dtrace: initial implementation of DTrace
From: Kris Van Hees @ 2019-07-08 16:48 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Kris Van Hees, netdev, bpf, dtrace-devel, linux-kernel, rostedt,
mhiramat, acme, ast, daniel, Chris Mason
In-Reply-To: <20190704130336.GN3402@hirez.programming.kicks-ass.net>
On Thu, Jul 04, 2019 at 03:03:36PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 03, 2019 at 08:14:30PM -0700, Kris Van Hees wrote:
> > +/*
> > + * Read the data_head offset from the header page of the ring buffer. The
> > + * argument is declared 'volatile' because it references a memory mapped page
> > + * that the kernel may be writing to while we access it here.
> > + */
> > +static u64 read_rb_head(volatile struct perf_event_mmap_page *rb_page)
> > +{
> > + u64 head = rb_page->data_head;
> > +
> > + asm volatile("" ::: "memory");
> > +
> > + return head;
> > +}
> > +
> > +/*
> > + * Write the data_tail offset in the header page of the ring buffer. The
> > + * argument is declared 'volatile' because it references a memory mapped page
> > + * that the kernel may be writing to while we access it here.
>
> s/writing/reading/
Thanks!
> > + */
> > +static void write_rb_tail(volatile struct perf_event_mmap_page *rb_page,
> > + u64 tail)
> > +{
> > + asm volatile("" ::: "memory");
> > +
> > + rb_page->data_tail = tail;
> > +}
>
> That volatile usage is atrocious (kernel style would have you use
> {READ,WRITE}_ONCE()). Also your comments fail to mark these as
> load_acquire and store_release. And by only using a compiler barrier
> you're hard assuming TSO, which is somewhat fragile at best.
>
> Alternatively, you can use the C11 bits and write:
>
> return __atomic_load_n(&rb_page->data_head, __ATOMIC_ACQUIRE);
>
> __atomic_store_n(&rb_page->data_tail, tail, __ATOMIC_RELEASE);
Perhaps I should just use ring_buffer_read_head() and ring_buffer_write_tail()
since they are provided in tools/include/linux/ring_buffer.h? I expect that
would be even more preferable over __atomic_load_n() and __atomic_store_n()?
> > +/*
> > + * Process and output the probe data at the supplied address.
> > + */
> > +static int output_event(int cpu, u64 *buf)
> > +{
> > + u8 *data = (u8 *)buf;
> > + struct perf_event_header *hdr;
> > +
> > + hdr = (struct perf_event_header *)data;
> > + data += sizeof(struct perf_event_header);
> > +
> > + if (hdr->type == PERF_RECORD_SAMPLE) {
> > + u8 *ptr = data;
> > + u32 i, size, probe_id;
> > +
> > + /*
> > + * struct {
> > + * struct perf_event_header header;
> > + * u32 size;
> > + * u32 probe_id;
> > + * u32 gap;
> > + * u64 data[n];
> > + * }
> > + * and data points to the 'size' member at this point.
> > + */
> > + if (ptr > (u8 *)buf + hdr->size) {
> > + fprintf(stderr, "BAD: corrupted sample header\n");
> > + goto out;
> > + }
> > +
> > + size = *(u32 *)data;
> > + data += sizeof(size);
> > + ptr += sizeof(size) + size;
> > + if (ptr != (u8 *)buf + hdr->size) {
> > + fprintf(stderr, "BAD: invalid sample size\n");
> > + goto out;
> > + }
> > +
> > + probe_id = *(u32 *)data;
> > + data += sizeof(probe_id);
> > + size -= sizeof(probe_id);
> > + data += sizeof(u32); /* skip 32-bit gap */
> > + size -= sizeof(u32);
> > + buf = (u64 *)data;
> > +
> > + printf("%3d %6d ", cpu, probe_id);
> > + for (i = 0, size /= sizeof(u64); i < size; i++)
> > + printf("%#016lx ", buf[i]);
> > + printf("\n");
> > + } else if (hdr->type == PERF_RECORD_LOST) {
> > + u64 lost;
> > +
> > + /*
> > + * struct {
> > + * struct perf_event_header header;
> > + * u64 id;
> > + * u64 lost;
> > + * }
> > + * and data points to the 'id' member at this point.
> > + */
> > + lost = *(u64 *)(data + sizeof(u64));
> > +
> > + printf("[%ld probes dropped]\n", lost);
> > + } else
> > + fprintf(stderr, "UNKNOWN: record type %d\n", hdr->type);
> > +
> > +out:
> > + return hdr->size;
> > +}
>
> I see a distinct lack of wrapping support. AFAICT when buf+hdr->size
> wraps you're doing out-of-bounds accesses.
Yes, that is correct. I'm actually trying to figure out why it didn't actually
cause a SEGV when I tested this because I'm clearly reading past the end of
the mmap'd memory. Thank you for noticing this - I was trying to be too
minimal in the code I was putting out and really didn't pay attention to this.
Fixed in the V2 I am preparing.
> > +/*
> > + * Process the available probe data in the given buffer.
> > + */
> > +static void process_data(struct dtrace_buffer *buf)
> > +{
> > + /* This is volatile because the kernel may be updating the content. */
> > + volatile struct perf_event_mmap_page *rb_page = buf->base;
> > + u8 *base = (u8 *)buf->base +
> > + buf->page_size;
> > + u64 head = read_rb_head(rb_page);
> > +
> > + while (rb_page->data_tail != head) {
> > + u64 tail = rb_page->data_tail;
> > + u64 *ptr = (u64 *)(base + tail % buf->data_size);
> > + int len;
> > +
> > + len = output_event(buf->cpu, ptr);
> > +
> > + write_rb_tail(rb_page, tail + len);
> > + head = read_rb_head(rb_page);
> > + }
> > +}
>
> more volatile yuck.
>
> Also:
>
> for (;;) {
> head = __atomic_load_n(&rb_page->data_head, __ATOMIC_ACQUIRE);
> tail = __atomic_load_n(&rb_page->data_tail, __ATOMIC_RELAXED);
>
> if (head == tail)
> break;
>
> do {
> hdr = buf->base + (tail & ((1UL << buf->data_shift) - 1));
> if ((tail >> buf->data_shift) !=
> ((tail + hdr->size) >> buf->data_shift))
> /* handle wrap case */
> else
> /* normal case */
>
> tail += hdr->size;
> } while (tail != head);
>
> __atomic_store_n(&rb_page->data_tail, tail, __ATOMIC_RELEASE);
> }
>
> Or something.
Thank you for this suggestion. As mentioned above, I lean towards using the
provided ring_buffer_(read_head,write_tail) implementations since that is the
'other end' of the ring buffer head/tail mechanism that is going to be kept
in sync with any changes that might happen on the kernel side, right?
> > +/*
> > + * Wait for data to become available in any of the buffers.
> > + */
> > +int dt_buffer_poll(int epoll_fd, int timeout)
> > +{
> > + struct epoll_event events[dt_numcpus];
> > + int i, cnt;
> > +
> > + cnt = epoll_wait(epoll_fd, events, dt_numcpus, timeout);
> > + if (cnt < 0)
> > + return -errno;
> > +
> > + for (i = 0; i < cnt; i++)
> > + process_data((struct dtrace_buffer *)events[i].data.ptr);
> > +
> > + return cnt;
> > +}
>
> Or make sure to read on the CPU by having a poll thread per CPU, then
> you can do away with the memory barriers.
That is definitely something for the todo list for future optimizations.
Thanks for your review and code suggestions.
Kris
^ permalink raw reply
* Re: [PATCH bpf-next v3 5/6] tools/libbpf_probes: Add new devmap_hash type
From: Yonghong Song @ 2019-07-08 16:48 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, Daniel Borkmann
Cc: Alexei Starovoitov, netdev@vger.kernel.org, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156258334745.1664.1686759894096070590.stgit@alrua-x1>
On 7/8/19 3:55 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> This adds the definition for BPF_MAP_TYPE_DEVMAP_HASH to libbpf_probes.c in
> tools/lib/bpf.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> tools/lib/bpf/libbpf_probes.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> index ace1a0708d99..4b0b0364f5fc 100644
> --- a/tools/lib/bpf/libbpf_probes.c
> +++ b/tools/lib/bpf/libbpf_probes.c
> @@ -244,6 +244,7 @@ bool bpf_probe_map_type(enum bpf_map_type map_type, __u32 ifindex)
> case BPF_MAP_TYPE_ARRAY_OF_MAPS:
> case BPF_MAP_TYPE_HASH_OF_MAPS:
> case BPF_MAP_TYPE_DEVMAP:
> + case BPF_MAP_TYPE_DEVMAP_HASH:
> case BPF_MAP_TYPE_SOCKMAP:
> case BPF_MAP_TYPE_CPUMAP:
> case BPF_MAP_TYPE_XSKMAP:
>
^ permalink raw reply
* Re: [PATCH bpf-next v3 4/6] tools/include/uapi: Add devmap_hash BPF map type
From: Yonghong Song @ 2019-07-08 16:47 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, Daniel Borkmann
Cc: Alexei Starovoitov, netdev@vger.kernel.org, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156258334740.1664.18295003114988159871.stgit@alrua-x1>
On 7/8/19 3:55 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> This adds the devmap_hash BPF map type to the uapi headers in tools/.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> tools/include/uapi/linux/bpf.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index cecf42c871d4..8afaa0a19c67 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -134,6 +134,7 @@ enum bpf_map_type {
> BPF_MAP_TYPE_QUEUE,
> BPF_MAP_TYPE_STACK,
> BPF_MAP_TYPE_SK_STORAGE,
> + BPF_MAP_TYPE_DEVMAP_HASH,
> };
>
> /* Note that tracing related programs such as
>
^ permalink raw reply
* Re: [PATCH bpf-next v3 3/6] xdp: Add devmap_hash map type for looking up devices by hashed index
From: Yonghong Song @ 2019-07-08 16:47 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, Daniel Borkmann
Cc: Alexei Starovoitov, netdev@vger.kernel.org, David Miller,
Jesper Dangaard Brouer, Jakub Kicinski, Björn Töpel
In-Reply-To: <156258334732.1664.10214955962271992722.stgit@alrua-x1>
On 7/8/19 3:55 AM, Toke Høiland-Jørgensen wrote:
> From: Toke Høiland-Jørgensen <toke@redhat.com>
>
> A common pattern when using xdp_redirect_map() is to create a device map
> where the lookup key is simply ifindex. Because device maps are arrays,
> this leaves holes in the map, and the map has to be sized to fit the
> largest ifindex, regardless of how many devices actually are actually
> needed in the map.
>
> This patch adds a second type of device map where the key is looked up
> using a hashmap, instead of being used as an array index. This allows maps
> to be densely packed, so they can be smaller.
>
> Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>
Acked-by: Yonghong Song <yhs@fb.com>
> ---
> include/linux/bpf.h | 7 ++
> include/linux/bpf_types.h | 1
> include/trace/events/xdp.h | 3 -
> include/uapi/linux/bpf.h | 1
> kernel/bpf/devmap.c | 194 ++++++++++++++++++++++++++++++++++++++++++++
> kernel/bpf/verifier.c | 2
> net/core/filter.c | 9 ++
> 7 files changed, 214 insertions(+), 3 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 net-next 1/3] tc-testing: Add JSON verification to tdc
From: Lucas Bates @ 2019-07-08 16:48 UTC (permalink / raw)
To: Alexander Aring
Cc: David Miller, Linux Kernel Network Developers, Jamal Hadi Salim,
Cong Wang, Jiri Pirko, Marcelo Ricardo Leitner, Vlad Buslov,
Davide Caratti, kernel
In-Reply-To: <20190704202130.tv2ivy5tjj7pjasj@x220t>
On Thu, Jul 4, 2019 at 4:21 PM Alexander Aring <aring@mojatatu.com> wrote:
> why you just use eval() as pattern matching operation and let the user
> define how to declare a matching mechanism instead you introduce another
> static matching scheme based on a json description?
>
> Whereas in eval() you could directly use the python bool expression
> parser to make whatever you want.
>
> I don't know, I see at some points you will hit limitations what you can
> express with this matchFOO and we need to introduce another matchBAR,
> whereas in providing the code it should be no problem expression
> anything. If you want smaller shortcuts writing matching patterns you
> can implement them and using in your eval() operation.
Regarding hitting limitations: quite possibly, yes.
Using eval() to provide code for matching is going to put more of a
dependency on the test writer knowing Python. I know it's not a
terribly difficult language to pick up, but it's still setting a
higher barrier to entry. This is the primary reason I scrapped the
work I had presented at Netdev 1.2 in Tokyo, where all the tests were
coded using Python's unittest framework - I want to be sure it's as
easy as possible for people to use tdc and write tests for it.
Unless I'm off-base here?
Lucas
^ permalink raw reply
* Re: [bpf-next v3 09/12] bpf: Split out some helper functions
From: Krzesimir Nowak @ 2019-07-08 16:40 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies
In-Reply-To: <20190708163121.18477-10-krzesimir@kinvolk.io>
On Mon, Jul 8, 2019 at 6:31 PM Krzesimir Nowak <krzesimir@kinvolk.io> wrote:
>
> The moved functions are generally useful for implementing
> bpf_prog_test_run for other types of BPF programs - they don't have
> any network-specific stuff in them, so I can use them in a test run
> implementation for perf event BPF program too.
>
> Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
This patch has some whitespace issues (indentation with spaces instead
of tabs in some places), will fix them for v4. Sorry about that.
> ---
> include/linux/bpf.h | 28 +++++
> kernel/bpf/Makefile | 1 +
> kernel/bpf/test_run.c | 212 ++++++++++++++++++++++++++++++++++
> net/bpf/test_run.c | 263 +++++++++++-------------------------------
> 4 files changed, 308 insertions(+), 196 deletions(-)
> create mode 100644 kernel/bpf/test_run.c
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 18f4cc2c6acd..28db8ba57bc3 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1143,4 +1143,32 @@ static inline u32 bpf_xdp_sock_convert_ctx_access(enum bpf_access_type type,
> }
> #endif /* CONFIG_INET */
>
> +/* Helper functions for bpf_prog_test_run implementations */
> +typedef u32 bpf_prog_run_helper_t(struct bpf_prog *prog, void *ctx,
> + void *private_data);
> +
> +enum bpf_test_run_flags {
> + BPF_TEST_RUN_PLAIN = 0,
> + BPF_TEST_RUN_SETUP_CGROUP_STORAGE = 1 << 0,
> +};
> +
> +int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
> + u32 *retval, u32 *duration);
> +
> +int bpf_test_run_cb(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
> + bpf_prog_run_helper_t run_prog, void *private_data,
> + u32 *retval, u32 *duration);
> +
> +int bpf_test_finish(union bpf_attr __user *uattr, u32 retval, u32 duration);
> +
> +void *bpf_receive_ctx(const union bpf_attr *kattr, u32 max_size);
> +
> +int bpf_send_ctx(const union bpf_attr *kattr, union bpf_attr __user *uattr,
> + const void *data, u32 size);
> +
> +void *bpf_receive_data(const union bpf_attr *kattr, u32 max_size);
> +
> +int bpf_send_data(const union bpf_attr *kattr, union bpf_attr __user *uattr,
> + const void *data, u32 size);
> +
> #endif /* _LINUX_BPF_H */
> diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
> index 29d781061cd5..570fd40288f4 100644
> --- a/kernel/bpf/Makefile
> +++ b/kernel/bpf/Makefile
> @@ -22,3 +22,4 @@ obj-$(CONFIG_CGROUP_BPF) += cgroup.o
> ifeq ($(CONFIG_INET),y)
> obj-$(CONFIG_BPF_SYSCALL) += reuseport_array.o
> endif
> +obj-$(CONFIG_BPF_SYSCALL) += test_run.o
> diff --git a/kernel/bpf/test_run.c b/kernel/bpf/test_run.c
> new file mode 100644
> index 000000000000..0481373da8be
> --- /dev/null
> +++ b/kernel/bpf/test_run.c
> @@ -0,0 +1,212 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/* Copyright (c) 2017 Facebook
> + * Copyright (c) 2019 Tigera, Inc
> + */
> +
> +#include <asm/div64.h>
> +
> +#include <linux/bpf-cgroup.h>
> +#include <linux/bpf.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/filter.h>
> +#include <linux/gfp.h>
> +#include <linux/kernel.h>
> +#include <linux/limits.h>
> +#include <linux/preempt.h>
> +#include <linux/rcupdate.h>
> +#include <linux/sched.h>
> +#include <linux/sched/signal.h>
> +#include <linux/slab.h>
> +#include <linux/timekeeping.h>
> +#include <linux/uaccess.h>
> +
> +static void teardown_cgroup_storage(struct bpf_cgroup_storage **storage)
> +{
> + enum bpf_cgroup_storage_type stype;
> +
> + if (!storage)
> + return;
> + for_each_cgroup_storage_type(stype)
> + bpf_cgroup_storage_free(storage[stype]);
> + kfree(storage);
> +}
> +
> +static struct bpf_cgroup_storage **setup_cgroup_storage(struct bpf_prog *prog)
> +{
> + enum bpf_cgroup_storage_type stype;
> + struct bpf_cgroup_storage **storage;
> + size_t size = MAX_BPF_CGROUP_STORAGE_TYPE;
> +
> + size *= sizeof(struct bpf_cgroup_storage *);
> + storage = kzalloc(size, GFP_KERNEL);
> + for_each_cgroup_storage_type(stype) {
> + storage[stype] = bpf_cgroup_storage_alloc(prog, stype);
> + if (IS_ERR(storage[stype])) {
> + storage[stype] = NULL;
> + teardown_cgroup_storage(storage);
> + return ERR_PTR(-ENOMEM);
> + }
> + }
> + return storage;
> +}
> +
> +static u32 run_bpf_prog(struct bpf_prog *prog, void *ctx, void *private_data)
> +{
> + return BPF_PROG_RUN(prog, ctx);
> +}
> +
> +int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
> + u32 *retval, u32 *duration)
> +{
> + return bpf_test_run_cb(prog, ctx, repeat, flags, run_bpf_prog, NULL,
> + retval, duration);
> +}
> +
> +int bpf_test_run_cb(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
> + bpf_prog_run_helper_t run_prog, void *private_data,
> + u32 *retval, u32 *duration)
> +{
> + struct bpf_cgroup_storage **storage = NULL;
> + u64 time_start, time_spent = 0;
> + int ret = 0;
> + u32 i;
> +
> + if (flags & BPF_TEST_RUN_SETUP_CGROUP_STORAGE) {
> + storage = setup_cgroup_storage(prog);
> + if (IS_ERR(storage))
> + return PTR_ERR(storage);
> + }
> +
> + if (!repeat)
> + repeat = 1;
> +
> + rcu_read_lock();
> + preempt_disable();
> + time_start = ktime_get_ns();
> + for (i = 0; i < repeat; i++) {
> + if (storage)
> + bpf_cgroup_storage_set(storage);
> + *retval = run_prog(prog, ctx, private_data);
> +
> + if (signal_pending(current)) {
> + preempt_enable();
> + rcu_read_unlock();
> + teardown_cgroup_storage(storage);
> + return -EINTR;
> + }
> +
> + if (need_resched()) {
> + time_spent += ktime_get_ns() - time_start;
> + preempt_enable();
> + rcu_read_unlock();
> +
> + cond_resched();
> +
> + rcu_read_lock();
> + preempt_disable();
> + time_start = ktime_get_ns();
> + }
> + }
> + time_spent += ktime_get_ns() - time_start;
> + preempt_enable();
> + rcu_read_unlock();
> +
> + do_div(time_spent, repeat);
> + *duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> +
> + teardown_cgroup_storage(storage);
> +
> + return ret;
> +}
> +
> +int bpf_test_finish(union bpf_attr __user *uattr, u32 retval, u32 duration)
> +{
> + if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval)))
> + return -EFAULT;
> + if (copy_to_user(&uattr->test.duration, &duration, sizeof(duration)))
> + return -EFAULT;
> + return 0;
> +}
> +
> +static void *bpf_receive_mem(u64 in, u32 in_size, u32 max_size)
> +{
> + void __user *data_in = u64_to_user_ptr(in);
> + void *data;
> + int err;
> +
> + if (!data_in && in_size)
> + return ERR_PTR(-EINVAL);
> + data = kzalloc(max_size, GFP_USER);
> + if (!data)
> + return ERR_PTR(-ENOMEM);
> +
> + if (data_in) {
> + err = bpf_check_uarg_tail_zero(data_in, max_size, in_size);
> + if (err) {
> + kfree(data);
> + return ERR_PTR(err);
> + }
> +
> + in_size = min_t(u32, max_size, in_size);
> + if (copy_from_user(data, data_in, in_size)) {
> + kfree(data);
> + return ERR_PTR(-EFAULT);
> + }
> + }
> + return data;
> +}
> +
> +static int bpf_send_mem(u64 out, u32 out_size, u32 *out_size_write,
> + const void *data, u32 data_size)
> +{
> + void __user *data_out = u64_to_user_ptr(out);
> + int err = -EFAULT;
> + u32 copy_size = data_size;
> +
> + if (!data_out && out_size)
> + return -EINVAL;
> +
> + if (!data || !data_out)
> + return 0;
> +
> + if (copy_size > out_size) {
> + copy_size = out_size;
> + err = -ENOSPC;
> + }
> +
> + if (copy_to_user(data_out, data, copy_size))
> + goto out;
> + if (copy_to_user(out_size_write, &data_size, sizeof(data_size)))
> + goto out;
> + if (err != -ENOSPC)
> + err = 0;
> +out:
> + return err;
> +}
> +
> +void *bpf_receive_data(const union bpf_attr *kattr, u32 max_size)
> +{
> + return bpf_receive_mem(kattr->test.data_in, kattr->test.data_size_in,
> + max_size);
> +}
> +
> +int bpf_send_data(const union bpf_attr *kattr, union bpf_attr __user *uattr,
> + const void *data, u32 size)
> +{
> + return bpf_send_mem(kattr->test.data_out, kattr->test.data_size_out,
> + &uattr->test.data_size_out, data, size);
> +}
> +
> +void *bpf_receive_ctx(const union bpf_attr *kattr, u32 max_size)
> +{
> + return bpf_receive_mem(kattr->test.ctx_in, kattr->test.ctx_size_in,
> + max_size);
> +}
> +
> +int bpf_send_ctx(const union bpf_attr *kattr, union bpf_attr __user *uattr,
> + const void *data, u32 size)
> +{
> + return bpf_send_mem(kattr->test.ctx_out, kattr->test.ctx_size_out,
> + &uattr->test.ctx_size_out, data, size);
> +}
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index 80e6f3a6864d..fe6b7b1af0cc 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -14,97 +14,6 @@
> #define CREATE_TRACE_POINTS
> #include <trace/events/bpf_test_run.h>
>
> -static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
> - u32 *retval, u32 *time)
> -{
> - struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { NULL };
> - enum bpf_cgroup_storage_type stype;
> - u64 time_start, time_spent = 0;
> - int ret = 0;
> - u32 i;
> -
> - for_each_cgroup_storage_type(stype) {
> - storage[stype] = bpf_cgroup_storage_alloc(prog, stype);
> - if (IS_ERR(storage[stype])) {
> - storage[stype] = NULL;
> - for_each_cgroup_storage_type(stype)
> - bpf_cgroup_storage_free(storage[stype]);
> - return -ENOMEM;
> - }
> - }
> -
> - if (!repeat)
> - repeat = 1;
> -
> - rcu_read_lock();
> - preempt_disable();
> - time_start = ktime_get_ns();
> - for (i = 0; i < repeat; i++) {
> - bpf_cgroup_storage_set(storage);
> - *retval = BPF_PROG_RUN(prog, ctx);
> -
> - if (signal_pending(current)) {
> - ret = -EINTR;
> - break;
> - }
> -
> - if (need_resched()) {
> - time_spent += ktime_get_ns() - time_start;
> - preempt_enable();
> - rcu_read_unlock();
> -
> - cond_resched();
> -
> - rcu_read_lock();
> - preempt_disable();
> - time_start = ktime_get_ns();
> - }
> - }
> - time_spent += ktime_get_ns() - time_start;
> - preempt_enable();
> - rcu_read_unlock();
> -
> - do_div(time_spent, repeat);
> - *time = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> -
> - for_each_cgroup_storage_type(stype)
> - bpf_cgroup_storage_free(storage[stype]);
> -
> - return ret;
> -}
> -
> -static int bpf_test_finish(const union bpf_attr *kattr,
> - union bpf_attr __user *uattr, const void *data,
> - u32 size, u32 retval, u32 duration)
> -{
> - void __user *data_out = u64_to_user_ptr(kattr->test.data_out);
> - int err = -EFAULT;
> - u32 copy_size = size;
> -
> - /* Clamp copy if the user has provided a size hint, but copy the full
> - * buffer if not to retain old behaviour.
> - */
> - if (kattr->test.data_size_out &&
> - copy_size > kattr->test.data_size_out) {
> - copy_size = kattr->test.data_size_out;
> - err = -ENOSPC;
> - }
> -
> - if (data_out && copy_to_user(data_out, data, copy_size))
> - goto out;
> - if (copy_to_user(&uattr->test.data_size_out, &size, sizeof(size)))
> - goto out;
> - if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval)))
> - goto out;
> - if (copy_to_user(&uattr->test.duration, &duration, sizeof(duration)))
> - goto out;
> - if (err != -ENOSPC)
> - err = 0;
> -out:
> - trace_bpf_test_finish(&err);
> - return err;
> -}
> -
> static void *bpf_test_init(const union bpf_attr *kattr, u32 size,
> u32 headroom, u32 tailroom)
> {
> @@ -125,63 +34,6 @@ static void *bpf_test_init(const union bpf_attr *kattr, u32 size,
> return data;
> }
>
> -static void *bpf_ctx_init(const union bpf_attr *kattr, u32 max_size)
> -{
> - void __user *data_in = u64_to_user_ptr(kattr->test.ctx_in);
> - void __user *data_out = u64_to_user_ptr(kattr->test.ctx_out);
> - u32 size = kattr->test.ctx_size_in;
> - void *data;
> - int err;
> -
> - if (!data_in && !data_out)
> - return NULL;
> -
> - data = kzalloc(max_size, GFP_USER);
> - if (!data)
> - return ERR_PTR(-ENOMEM);
> -
> - if (data_in) {
> - err = bpf_check_uarg_tail_zero(data_in, max_size, size);
> - if (err) {
> - kfree(data);
> - return ERR_PTR(err);
> - }
> -
> - size = min_t(u32, max_size, size);
> - if (copy_from_user(data, data_in, size)) {
> - kfree(data);
> - return ERR_PTR(-EFAULT);
> - }
> - }
> - return data;
> -}
> -
> -static int bpf_ctx_finish(const union bpf_attr *kattr,
> - union bpf_attr __user *uattr, const void *data,
> - u32 size)
> -{
> - void __user *data_out = u64_to_user_ptr(kattr->test.ctx_out);
> - int err = -EFAULT;
> - u32 copy_size = size;
> -
> - if (!data || !data_out)
> - return 0;
> -
> - if (copy_size > kattr->test.ctx_size_out) {
> - copy_size = kattr->test.ctx_size_out;
> - err = -ENOSPC;
> - }
> -
> - if (copy_to_user(data_out, data, copy_size))
> - goto out;
> - if (copy_to_user(&uattr->test.ctx_size_out, &size, sizeof(size)))
> - goto out;
> - if (err != -ENOSPC)
> - err = 0;
> -out:
> - return err;
> -}
> -
> /**
> * range_is_zero - test whether buffer is initialized
> * @buf: buffer to check
> @@ -238,6 +90,36 @@ static void convert_skb_to___skb(struct sk_buff *skb, struct __sk_buff *__skb)
> memcpy(__skb->cb, &cb->data, QDISC_CB_PRIV_LEN);
> }
>
> +static int bpf_net_prog_test_run_finish(const union bpf_attr *kattr,
> + union bpf_attr __user *uattr,
> + const void *data, u32 data_size,
> + const void *ctx, u32 ctx_size,
> + u32 retval, u32 duration)
> +{
> + int ret;
> + union bpf_attr fixed_kattr;
> + const union bpf_attr *kattr_ptr = kattr;
> +
> + /* Clamp copy (in bpf_send_mem) if the user has provided a
> + * size hint, but copy the full buffer if not to retain old
> + * behaviour.
> + */
> + if (!kattr->test.data_size_out && kattr->test.data_out) {
> + fixed_kattr = *kattr;
> + fixed_kattr.test.data_size_out = U32_MAX;
> + kattr_ptr = &fixed_kattr;
> + }
> +
> + ret = bpf_send_data(kattr_ptr, uattr, data, data_size);
> + if (!ret) {
> + ret = bpf_test_finish(uattr, retval, duration);
> + if (!ret && ctx)
> + ret = bpf_send_ctx(kattr_ptr, uattr, ctx, ctx_size);
> + }
> + trace_bpf_test_finish(&ret);
> + return ret;
> +}
> +
> int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> union bpf_attr __user *uattr)
> {
> @@ -257,7 +139,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> if (IS_ERR(data))
> return PTR_ERR(data);
>
> - ctx = bpf_ctx_init(kattr, sizeof(struct __sk_buff));
> + ctx = bpf_receive_ctx(kattr, sizeof(struct __sk_buff));
> if (IS_ERR(ctx)) {
> kfree(data);
> return PTR_ERR(ctx);
> @@ -307,7 +189,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> ret = convert___skb_to_skb(skb, ctx);
> if (ret)
> goto out;
> - ret = bpf_test_run(prog, skb, repeat, &retval, &duration);
> + ret = bpf_test_run(prog, skb, repeat, BPF_TEST_RUN_SETUP_CGROUP_STORAGE,
> + &retval, &duration);
> if (ret)
> goto out;
> if (!is_l2) {
> @@ -327,10 +210,9 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> /* bpf program can never convert linear skb to non-linear */
> if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
> size = skb_headlen(skb);
> - ret = bpf_test_finish(kattr, uattr, skb->data, size, retval, duration);
> - if (!ret)
> - ret = bpf_ctx_finish(kattr, uattr, ctx,
> - sizeof(struct __sk_buff));
> + ret = bpf_net_prog_test_run_finish(kattr, uattr, skb->data, size,
> + ctx, sizeof(struct __sk_buff),
> + retval, duration);
> out:
> kfree_skb(skb);
> bpf_sk_storage_free(sk);
> @@ -365,32 +247,48 @@ 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;
>
> - ret = bpf_test_run(prog, &xdp, repeat, &retval, &duration);
> + ret = bpf_test_run(prog, &xdp, repeat,
> + BPF_TEST_RUN_SETUP_CGROUP_STORAGE,
> + &retval, &duration);
> if (ret)
> goto out;
> if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN ||
> xdp.data_end != xdp.data + size)
> size = xdp.data_end - xdp.data;
> - ret = bpf_test_finish(kattr, uattr, xdp.data, size, retval, duration);
> + ret = bpf_net_prog_test_run_finish(kattr, uattr, xdp.data, size,
> + NULL, 0, retval, duration);
> out:
> kfree(data);
> return ret;
> }
>
> +struct bpf_flow_dissect_run_data {
> + __be16 proto;
> + int nhoff;
> + int hlen;
> +};
> +
> +static u32 bpf_flow_dissect_run(struct bpf_prog *prog, void *ctx,
> + void *private_data)
> +{
> + struct bpf_flow_dissect_run_data *data = private_data;
> +
> + return bpf_flow_dissect(prog, ctx, data->proto, data->nhoff, data->hlen);
> +}
> +
> int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> const union bpf_attr *kattr,
> union bpf_attr __user *uattr)
> {
> + struct bpf_flow_dissect_run_data run_data = {};
> u32 size = kattr->test.data_size_in;
> struct bpf_flow_dissector ctx = {};
> u32 repeat = kattr->test.repeat;
> struct bpf_flow_keys flow_keys;
> - u64 time_start, time_spent = 0;
> const struct ethhdr *eth;
> u32 retval, duration;
> void *data;
> int ret;
> - u32 i;
>
> if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR)
> return -EINVAL;
> @@ -407,49 +305,22 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
>
> eth = (struct ethhdr *)data;
>
> - if (!repeat)
> - repeat = 1;
> -
> ctx.flow_keys = &flow_keys;
> ctx.data = data;
> ctx.data_end = (__u8 *)data + size;
>
> - rcu_read_lock();
> - preempt_disable();
> - time_start = ktime_get_ns();
> - for (i = 0; i < repeat; i++) {
> - retval = bpf_flow_dissect(prog, &ctx, eth->h_proto, ETH_HLEN,
> - size);
> -
> - if (signal_pending(current)) {
> - preempt_enable();
> - rcu_read_unlock();
> -
> - ret = -EINTR;
> - goto out;
> - }
> -
> - if (need_resched()) {
> - time_spent += ktime_get_ns() - time_start;
> - preempt_enable();
> - rcu_read_unlock();
> -
> - cond_resched();
> -
> - rcu_read_lock();
> - preempt_disable();
> - time_start = ktime_get_ns();
> - }
> - }
> - time_spent += ktime_get_ns() - time_start;
> - preempt_enable();
> - rcu_read_unlock();
> -
> - do_div(time_spent, repeat);
> - duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
> + run_data.proto = eth->h_proto;
> + run_data.nhoff = ETH_HLEN;
> + run_data.hlen = size;
> + ret = bpf_test_run_cb(prog, &ctx, repeat, BPF_TEST_RUN_PLAIN,
> + bpf_flow_dissect_run, &run_data,
> + &retval, &duration);
> + if (!ret)
> + goto out;
>
> - ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys),
> - retval, duration);
> + ret = bpf_net_prog_test_run_finish(kattr, uattr, &flow_keys,
> + sizeof(flow_keys), NULL, 0,
> + retval, duration);
>
> out:
> kfree(data);
> --
> 2.20.1
>
--
Kinvolk GmbH | Adalbertstr.6a, 10999 Berlin | tel: +491755589364
Geschäftsführer/Directors: Alban Crequy, Chris Kühl, Iago López Galeiras
Registergericht/Court of registration: Amtsgericht Charlottenburg
Registernummer/Registration number: HRB 171414 B
Ust-ID-Nummer/VAT ID number: DE302207000
^ permalink raw reply
* Re: [PATCH 1/1] tools/dtrace: initial implementation of DTrace
From: Kris Van Hees @ 2019-07-08 16:38 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Kris Van Hees, netdev, bpf, dtrace-devel, linux-kernel, rostedt,
mhiramat, acme, ast, daniel, Chris Mason
In-Reply-To: <20190704130509.GO3402@hirez.programming.kicks-ass.net>
On Thu, Jul 04, 2019 at 03:05:09PM +0200, Peter Zijlstra wrote:
> On Wed, Jul 03, 2019 at 08:14:30PM -0700, Kris Van Hees wrote:
> > +int dt_bpf_attach(int event_id, int bpf_fd)
> > +{
> > + int event_fd;
> > + int rc;
> > + struct perf_event_attr attr = {};
> > +
> > + attr.type = PERF_TYPE_TRACEPOINT;
> > + attr.sample_type = PERF_SAMPLE_RAW;
> > + attr.sample_period = 1;
> > + attr.wakeup_events = 1;
> > + attr.config = event_id;
> > +
> > + /* Register the event (based on its id), and obtain a fd. */
> > + event_fd = perf_event_open(&attr, -1, 0, -1, 0);
> > + if (event_fd < 0) {
> > + perror("sys_perf_event_open");
> > + return -1;
> > + }
> > +
> > + /* Enable the probe. */
> > + rc = ioctl(event_fd, PERF_EVENT_IOC_ENABLE, 0);
>
> AFAICT you didn't use attr.disabled = 1, so this IOC_ENABLE is
> completely superfluous.
Oh yes, good point (and the same applies to the dt_buffer.c code where I set
up the events that own each buffer - no point in doing an explicit enable there
eiteher).
Thanks for catching this!
> > + if (rc < 0) {
> > + perror("PERF_EVENT_IOC_ENABLE");
> > + return -1;
> > + }
> > +
> > + /* Associate the BPF program with the event. */
> > + rc = ioctl(event_fd, PERF_EVENT_IOC_SET_BPF, bpf_fd);
> > + if (rc < 0) {
> > + perror("PERF_EVENT_IOC_SET_BPF");
> > + return -1;
> > + }
> > +
> > + return 0;
> > +}
^ permalink raw reply
* [bpf-next v3 00/12] Test the 32bit narrow reads
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
These patches try to test the fix made in commit e2f7fc0ac695 ("bpf:
fix undefined behavior in narrow load handling"). The problem existed
in the generated BPF bytecode that was doing a 32bit narrow read of a
64bit field, so to test it the code would need to be executed.
Currently the only such field exists in BPF_PROG_TYPE_PERF_EVENT,
which was not supported by bpf_prog_test_run().
I'm sending these patches to bpf-next now as they introduce a new
feature. But maybe some of those patches could go to the bpf branch?
There is a bit of yak shaving to do for the test to be run:
1. Print why the program could not be run (patch 1).
2. Some fixes for errno clobbering (patches 2 and 3).
3. Using bpf_prog_test_run_xattr, so I can pass ctx_in stuff too
(patch 4).
4. Adding ctx stuff and data out size override to struct bpf_test, and
use them for the perf event tests (patches 5 and 6).
5. Some tools headers syncing (patches 7 and 8).
6. Split out some useful functions for bpf_prog_test_run
implementation out of the net/bpf/test_run.c (patch 9)
7. Implement bpf_prog_test_run for perf event programs and test it
(patches 10 and 11).
The last point is where I'm least sure how things should be done
properly:
1. There is a bunch of stuff to prepare for the
bpf_perf_prog_read_value to work, and that stuff is very hacky. I
would welcome some hints about how to set up the perf_event and
perf_sample_data structs in a way that is a bit more future-proof
than just setting some fields in a specific way, so some other code
won't use some other fields (like setting event.oncpu to -1 to
avoid event.pmu to be used for reading the value of the event).
2. The tests try to see if the test run for perf event sets up the
context properly, so they verify the struct pt_regs contents. They
way it is currently written Works For Me, but surely it won't work
on other arch. So what would be the way forward? Just put the test
case inside #ifdef __x86_64__?
3. Another thing in tests - I'm trying to make sure that the
bpf_perf_prog_read_value helper works as it seems to be the only
bpf perf helper that takes the ctx as a parameter. Is that enough
or I should test other helpers too?
About the test itself - I'm not sure if it will work on a big endian
machine. I think it should, but I don't have anything handy here to
verify it.
Krzesimir Nowak (12):
selftests/bpf: Print a message when tester could not run a program
selftests/bpf: Avoid a clobbering of errno
selftests/bpf: Avoid another case of errno clobbering
selftests/bpf: Use bpf_prog_test_run_xattr
selftests/bpf: Allow passing more information to BPF prog test run
selftests/bpf: Make sure that preexisting tests for perf event work
tools headers: Adopt compiletime_assert from kernel sources
tools headers: Sync struct bpf_perf_event_data
bpf: Split out some helper functions
bpf: Implement bpf_prog_test_run for perf event programs
selftests/bpf: Add tests for bpf_prog_test_run for perf events progs
selftests/bpf: Test correctness of narrow 32bit read on 64bit field
include/linux/bpf.h | 28 ++
kernel/bpf/Makefile | 1 +
kernel/bpf/test_run.c | 212 ++++++++++++++
kernel/trace/bpf_trace.c | 60 ++++
net/bpf/test_run.c | 263 +++++-------------
tools/include/linux/compiler.h | 28 ++
tools/include/uapi/linux/bpf_perf_event.h | 1 +
tools/testing/selftests/bpf/test_verifier.c | 197 ++++++++++++-
.../selftests/bpf/verifier/perf_event_run.c | 96 +++++++
.../bpf/verifier/perf_event_sample_period.c | 4 +
.../testing/selftests/bpf/verifier/var_off.c | 21 ++
11 files changed, 700 insertions(+), 211 deletions(-)
create mode 100644 kernel/bpf/test_run.c
create mode 100644 tools/testing/selftests/bpf/verifier/perf_event_run.c
--
2.20.1
^ permalink raw reply
* [bpf-next v3 02/12] selftests/bpf: Avoid a clobbering of errno
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
Save errno right after bpf_prog_test_run returns, so we later check
the error code actually set by bpf_prog_test_run, not by some libcap
function.
Changes since v1:
- Fix the "Fixes:" tag to mention actual commit that introduced the
bug
Changes since v2:
- Move the declaration so it fits the reverse christmas tree style.
Cc: Daniel Borkmann <daniel@iogearbox.net>
Fixes: 832c6f2c29ec ("bpf: test make sure to run unpriv test cases in test_verifier")
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
tools/testing/selftests/bpf/test_verifier.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index b8d065623ead..3fe126e0083b 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -823,16 +823,18 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
__u8 tmp[TEST_DATA_LEN << 2];
__u32 size_tmp = sizeof(tmp);
uint32_t retval;
+ int saved_errno;
int err;
if (unpriv)
set_admin(true);
err = bpf_prog_test_run(fd_prog, 1, data, size_data,
tmp, &size_tmp, &retval, NULL);
+ saved_errno = errno;
if (unpriv)
set_admin(false);
if (err) {
- switch (errno) {
+ switch (saved_errno) {
case 524/*ENOTSUPP*/:
printf("Did not run the program (not supported) ");
return 0;
--
2.20.1
^ permalink raw reply related
* [bpf-next v3 01/12] selftests/bpf: Print a message when tester could not run a program
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
This prints a message when the error is about program type being not
supported by the test runner or because of permissions problem. This
is to see if the program we expected to run was actually executed.
The messages are open-coded because strerror(ENOTSUPP) returns
"Unknown error 524".
Changes since v2:
- Also print "FAIL" on an unexpected bpf_prog_test_run error, so there
is a corresponding "FAIL" message for each failed test.
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
tools/testing/selftests/bpf/test_verifier.c | 17 +++++++++++++----
1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index c5514daf8865..b8d065623ead 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -831,11 +831,20 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
tmp, &size_tmp, &retval, NULL);
if (unpriv)
set_admin(false);
- if (err && errno != 524/*ENOTSUPP*/ && errno != EPERM) {
- printf("Unexpected bpf_prog_test_run error ");
- return err;
+ if (err) {
+ switch (errno) {
+ case 524/*ENOTSUPP*/:
+ printf("Did not run the program (not supported) ");
+ return 0;
+ case EPERM:
+ printf("Did not run the program (no permission) ");
+ return 0;
+ default:
+ printf("FAIL: Unexpected bpf_prog_test_run error (%s) ", strerror(saved_errno));
+ return err;
+ }
}
- if (!err && retval != expected_val &&
+ if (retval != expected_val &&
expected_val != POINTER_VALUE) {
printf("FAIL retval %d != %d ", retval, expected_val);
return 1;
--
2.20.1
^ permalink raw reply related
* [bpf-next v3 03/12] selftests/bpf: Avoid another case of errno clobbering
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
Commit 8184d44c9a57 ("selftests/bpf: skip verifier tests for
unsupported program types") added a check for an unsupported program
type. The function doing it changes errno, so test_verifier should
save it before calling it if test_verifier wants to print a reason why
verifying a BPF program of a supported type failed.
Changes since v2:
- Move the declaration to fit the reverse christmas tree style.
Fixes: 8184d44c9a57 ("selftests/bpf: skip verifier tests for unsupported program types")
Cc: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
tools/testing/selftests/bpf/test_verifier.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 3fe126e0083b..c7541f572932 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -864,6 +864,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
int run_errs, run_successes;
int map_fds[MAX_NR_MAPS];
const char *expected_err;
+ int saved_errno;
int fixup_skips;
__u32 pflags;
int i, err;
@@ -894,6 +895,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
pflags |= BPF_F_ANY_ALIGNMENT;
fd_prog = bpf_verify_program(prog_type, prog, prog_len, pflags,
"GPL", 0, bpf_vlog, sizeof(bpf_vlog), 4);
+ saved_errno = errno;
if (fd_prog < 0 && !bpf_probe_prog_type(prog_type, 0)) {
printf("SKIP (unsupported program type %d)\n", prog_type);
skips++;
@@ -910,7 +912,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
if (expected_ret == ACCEPT) {
if (fd_prog < 0) {
printf("FAIL\nFailed to load prog '%s'!\n",
- strerror(errno));
+ strerror(saved_errno));
goto fail_log;
}
#ifndef CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
--
2.20.1
^ permalink raw reply related
* [bpf-next v3 04/12] selftests/bpf: Use bpf_prog_test_run_xattr
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
The bpf_prog_test_run_xattr function gives more options to set up a
test run of a BPF program than the bpf_prog_test_run function.
We will need this extra flexibility to pass ctx data later.
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
tools/testing/selftests/bpf/test_verifier.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index c7541f572932..1640ba9f12c1 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -822,14 +822,20 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
{
__u8 tmp[TEST_DATA_LEN << 2];
__u32 size_tmp = sizeof(tmp);
- uint32_t retval;
int saved_errno;
int err;
+ struct bpf_prog_test_run_attr attr = {
+ .prog_fd = fd_prog,
+ .repeat = 1,
+ .data_in = data,
+ .data_size_in = size_data,
+ .data_out = tmp,
+ .data_size_out = size_tmp,
+ };
if (unpriv)
set_admin(true);
- err = bpf_prog_test_run(fd_prog, 1, data, size_data,
- tmp, &size_tmp, &retval, NULL);
+ err = bpf_prog_test_run_xattr(&attr);
saved_errno = errno;
if (unpriv)
set_admin(false);
@@ -846,9 +852,9 @@ static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
return err;
}
}
- if (retval != expected_val &&
+ if (attr.retval != expected_val &&
expected_val != POINTER_VALUE) {
- printf("FAIL retval %d != %d ", retval, expected_val);
+ printf("FAIL retval %d != %d ", attr.retval, expected_val);
return 1;
}
--
2.20.1
^ permalink raw reply related
* [bpf-next v3 06/12] selftests/bpf: Make sure that preexisting tests for perf event work
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
We are going to introduce a test run implementation for perf event in
a later commit and it will not allow passing any data out or ctx out
to it, and requires their sizes to be specified to zero. To avoid test
failures when the feature is introduced, override the data out size to
zero. That will also cause NULL buffer to be sent to the kernel.
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
.../testing/selftests/bpf/verifier/perf_event_sample_period.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c b/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
index 471c1a5950d8..19f5d824b275 100644
--- a/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
+++ b/tools/testing/selftests/bpf/verifier/perf_event_sample_period.c
@@ -13,6 +13,7 @@
},
.result = ACCEPT,
.prog_type = BPF_PROG_TYPE_PERF_EVENT,
+ .override_data_out_len = true,
},
{
"check bpf_perf_event_data->sample_period half load permitted",
@@ -29,6 +30,7 @@
},
.result = ACCEPT,
.prog_type = BPF_PROG_TYPE_PERF_EVENT,
+ .override_data_out_len = true,
},
{
"check bpf_perf_event_data->sample_period word load permitted",
@@ -45,6 +47,7 @@
},
.result = ACCEPT,
.prog_type = BPF_PROG_TYPE_PERF_EVENT,
+ .override_data_out_len = true,
},
{
"check bpf_perf_event_data->sample_period dword load permitted",
@@ -56,4 +59,5 @@
},
.result = ACCEPT,
.prog_type = BPF_PROG_TYPE_PERF_EVENT,
+ .override_data_out_len = true,
},
--
2.20.1
^ permalink raw reply related
* [bpf-next v3 05/12] selftests/bpf: Allow passing more information to BPF prog test run
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
The test case can now specify a custom length of the data member,
context data and its length, which will be passed to
bpf_prog_test_run_xattr. For backward compatilibity, if the data
length is 0 (which is what will happen when the field is left
unspecified in the designated initializer of a struct), then the
length passed to the bpf_prog_test_run_xattr is TEST_DATA_LEN.
Also for backward compatilibity, if context data length is 0, NULL is
passed as a context to bpf_prog_test_run_xattr. This is to avoid
breaking other tests, where context data being NULL and context data
length being 0 is handled differently from the case where context data
is not NULL and context data length is 0.
Custom lengths still can't be greater than hardcoded 64 bytes for data
and 192 for context data.
192 for context data was picked to allow passing struct
bpf_perf_event_data as a context for perf event programs. The struct
is quite large, because it contains struct pt_regs.
Test runs for perf event programs will not allow the copying the data
back to data_out buffer, so they require data_out_size to be zero and
data_out to be NULL. Since test_verifier hardcodes it, make it
possible to override the size. Overriding the size to zero will cause
the buffer to be NULL.
Changes since v2:
- Allow overriding the data out size and buffer.
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
tools/testing/selftests/bpf/test_verifier.c | 105 +++++++++++++++++---
1 file changed, 93 insertions(+), 12 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 1640ba9f12c1..6f124cc4ee34 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -54,6 +54,7 @@
#define MAX_TEST_RUNS 8
#define POINTER_VALUE 0xcafe4all
#define TEST_DATA_LEN 64
+#define TEST_CTX_LEN 192
#define F_NEEDS_EFFICIENT_UNALIGNED_ACCESS (1 << 0)
#define F_LOAD_WITH_STRICT_ALIGNMENT (1 << 1)
@@ -96,7 +97,12 @@ struct bpf_test {
enum bpf_prog_type prog_type;
uint8_t flags;
__u8 data[TEST_DATA_LEN];
+ __u32 data_len;
+ __u8 ctx[TEST_CTX_LEN];
+ __u32 ctx_len;
void (*fill_helper)(struct bpf_test *self);
+ bool override_data_out_len;
+ __u32 overridden_data_out_len;
uint8_t runs;
struct {
uint32_t retval, retval_unpriv;
@@ -104,6 +110,9 @@ struct bpf_test {
__u8 data[TEST_DATA_LEN];
__u64 data64[TEST_DATA_LEN / 8];
};
+ __u32 data_len;
+ __u8 ctx[TEST_CTX_LEN];
+ __u32 ctx_len;
} retvals[MAX_TEST_RUNS];
};
@@ -818,21 +827,35 @@ static int set_admin(bool admin)
}
static int do_prog_test_run(int fd_prog, bool unpriv, uint32_t expected_val,
- void *data, size_t size_data)
+ void *data, size_t size_data, void *ctx,
+ size_t size_ctx, u32 *overridden_data_out_size)
{
- __u8 tmp[TEST_DATA_LEN << 2];
- __u32 size_tmp = sizeof(tmp);
- int saved_errno;
- int err;
struct bpf_prog_test_run_attr attr = {
.prog_fd = fd_prog,
.repeat = 1,
.data_in = data,
.data_size_in = size_data,
- .data_out = tmp,
- .data_size_out = size_tmp,
+ .ctx_in = ctx,
+ .ctx_size_in = size_ctx,
};
+ __u8 tmp[TEST_DATA_LEN << 2];
+ __u32 size_tmp = sizeof(tmp);
+ __u32 size_buf = size_tmp;
+ __u8 *buf = tmp;
+ int saved_errno;
+ int err;
+ if (overridden_data_out_size)
+ size_buf = *overridden_data_out_size;
+ if (size_buf > size_tmp) {
+ printf("FAIL: out data size (%d) greater than a buffer size (%d) ",
+ size_buf, size_tmp);
+ return -EINVAL;
+ }
+ if (!size_buf)
+ buf = NULL;
+ attr.data_size_out = size_buf;
+ attr.data_out = buf;
if (unpriv)
set_admin(true);
err = bpf_prog_test_run_xattr(&attr);
@@ -956,13 +979,45 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
if (!alignment_prevented_execution && fd_prog >= 0) {
uint32_t expected_val;
int i;
+ __u32 size_data;
+ __u32 size_ctx;
+ bool bad_size;
+ void *ctx;
+ __u32 *overridden_data_out_size;
if (!test->runs) {
+ if (test->data_len > 0)
+ size_data = test->data_len;
+ else
+ size_data = sizeof(test->data);
+ if (test->override_data_out_len)
+ overridden_data_out_size = &test->overridden_data_out_len;
+ else
+ overridden_data_out_size = NULL;
+ size_ctx = test->ctx_len;
+ bad_size = false;
expected_val = unpriv && test->retval_unpriv ?
test->retval_unpriv : test->retval;
- err = do_prog_test_run(fd_prog, unpriv, expected_val,
- test->data, sizeof(test->data));
+ if (size_data > sizeof(test->data)) {
+ printf("FAIL: data size (%u) greater than TEST_DATA_LEN (%lu) ", size_data, sizeof(test->data));
+ bad_size = true;
+ }
+ if (size_ctx > sizeof(test->ctx)) {
+ printf("FAIL: ctx size (%u) greater than TEST_CTX_LEN (%lu) ", size_ctx, sizeof(test->ctx));
+ bad_size = true;
+ }
+ if (size_ctx)
+ ctx = test->ctx;
+ else
+ ctx = NULL;
+ if (bad_size)
+ err = 1;
+ else
+ err = do_prog_test_run(fd_prog, unpriv, expected_val,
+ test->data, size_data,
+ ctx, size_ctx,
+ overridden_data_out_size);
if (err)
run_errs++;
else
@@ -970,14 +1025,40 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
}
for (i = 0; i < test->runs; i++) {
+ if (test->retvals[i].data_len > 0)
+ size_data = test->retvals[i].data_len;
+ else
+ size_data = sizeof(test->retvals[i].data);
+ if (test->override_data_out_len)
+ overridden_data_out_size = &test->overridden_data_out_len;
+ else
+ overridden_data_out_size = NULL;
+ size_ctx = test->retvals[i].ctx_len;
+ bad_size = false;
if (unpriv && test->retvals[i].retval_unpriv)
expected_val = test->retvals[i].retval_unpriv;
else
expected_val = test->retvals[i].retval;
- err = do_prog_test_run(fd_prog, unpriv, expected_val,
- test->retvals[i].data,
- sizeof(test->retvals[i].data));
+ if (size_data > sizeof(test->retvals[i].data)) {
+ printf("FAIL: data size (%u) at run %i greater than TEST_DATA_LEN (%lu) ", size_data, i + 1, sizeof(test->retvals[i].data));
+ bad_size = true;
+ }
+ if (size_ctx > sizeof(test->retvals[i].ctx)) {
+ printf("FAIL: ctx size (%u) at run %i greater than TEST_CTX_LEN (%lu) ", size_ctx, i + 1, sizeof(test->retvals[i].ctx));
+ bad_size = true;
+ }
+ if (size_ctx)
+ ctx = test->retvals[i].ctx;
+ else
+ ctx = NULL;
+ if (bad_size)
+ err = 1;
+ else
+ err = do_prog_test_run(fd_prog, unpriv, expected_val,
+ test->retvals[i].data, size_data,
+ ctx, size_ctx,
+ overridden_data_out_size);
if (err) {
printf("(run %d/%d) ", i + 1, test->runs);
run_errs++;
--
2.20.1
^ permalink raw reply related
* [bpf-next v3 07/12] tools headers: Adopt compiletime_assert from kernel sources
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
This will come in handy to verify that the hardcoded size of the
context data in bpf_test struct is high enough to hold some struct.
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
tools/include/linux/compiler.h | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)
diff --git a/tools/include/linux/compiler.h b/tools/include/linux/compiler.h
index 1827c2f973f9..b4e97751000a 100644
--- a/tools/include/linux/compiler.h
+++ b/tools/include/linux/compiler.h
@@ -172,4 +172,32 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
# define __fallthrough
#endif
+
+#ifdef __OPTIMIZE__
+# define __compiletime_assert(condition, msg, prefix, suffix) \
+ do { \
+ extern void prefix ## suffix(void) __compiletime_error(msg); \
+ if (!(condition)) \
+ prefix ## suffix(); \
+ } while (0)
+#else
+# define __compiletime_assert(condition, msg, prefix, suffix) do { } while (0)
+#endif
+
+#define _compiletime_assert(condition, msg, prefix, suffix) \
+ __compiletime_assert(condition, msg, prefix, suffix)
+
+/**
+ * compiletime_assert - break build and emit msg if condition is false
+ * @condition: a compile-time constant condition to check
+ * @msg: a message to emit if condition is false
+ *
+ * In tradition of POSIX assert, this macro will break the build if the
+ * supplied condition is *false*, emitting the supplied error message if the
+ * compiler has support to do so.
+ */
+#define compiletime_assert(condition, msg) \
+ _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
+
+
#endif /* _TOOLS_LINUX_COMPILER_H */
--
2.20.1
^ permalink raw reply related
* [bpf-next v3 08/12] tools headers: Sync struct bpf_perf_event_data
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
struct bpf_perf_event_data in kernel headers has the addr field, which
is missing in the tools version of the struct. This will be important
for the bpf prog test run implementation for perf events as it will
expect data to be an instance of struct bpf_perf_event_data, so the
size of the data needs to match sizeof(bpf_perf_event_data).
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
tools/include/uapi/linux/bpf_perf_event.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/include/uapi/linux/bpf_perf_event.h b/tools/include/uapi/linux/bpf_perf_event.h
index 8f95303f9d80..eb1b9d21250c 100644
--- a/tools/include/uapi/linux/bpf_perf_event.h
+++ b/tools/include/uapi/linux/bpf_perf_event.h
@@ -13,6 +13,7 @@
struct bpf_perf_event_data {
bpf_user_pt_regs_t regs;
__u64 sample_period;
+ __u64 addr;
};
#endif /* _UAPI__LINUX_BPF_PERF_EVENT_H__ */
--
2.20.1
^ permalink raw reply related
* [bpf-next v3 12/12] selftests/bpf: Test correctness of narrow 32bit read on 64bit field
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
Test the correctness of the 32bit narrow reads by reading both halves
of the 64 bit field and doing a binary or on them to see if we get the
original value.
It succeeds as it should, but with the commit e2f7fc0ac695 ("bpf: fix
undefined behavior in narrow load handling") reverted, the test fails
with a following message:
> $ sudo ./test_verifier
> ...
> #967/p 32bit loads of a 64bit field (both least and most significant words) FAIL retval -1985229329 != 0
> verification time 17 usec
> stack depth 0
> processed 8 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0
> ...
> Summary: 1519 PASSED, 0 SKIPPED, 1 FAILED
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
tools/testing/selftests/bpf/test_verifier.c | 19 +++++++++++++++++
.../testing/selftests/bpf/verifier/var_off.c | 21 +++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 484ea8842b06..2a20280a4a44 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -24,6 +24,7 @@
#include <sys/capability.h>
+#include <linux/compiler.h>
#include <linux/unistd.h>
#include <linux/filter.h>
#include <linux/bpf_perf_event.h>
@@ -343,6 +344,24 @@ static void bpf_fill_perf_event_test_run_check(struct bpf_test *self)
self->fill_insns = NULL;
}
+static void bpf_fill_32bit_loads(struct bpf_test *self)
+{
+ compiletime_assert(
+ sizeof(struct bpf_perf_event_data) <= TEST_CTX_LEN,
+ "buffer for ctx is too short to fit struct bpf_perf_event_data");
+ compiletime_assert(
+ sizeof(struct bpf_perf_event_value) <= TEST_DATA_LEN,
+ "buffer for data is too short to fit struct bpf_perf_event_value");
+
+ struct bpf_perf_event_data ctx = {
+ .sample_period = 0x0123456789abcdef,
+ };
+
+ memcpy(self->ctx, &ctx, sizeof(ctx));
+ free(self->fill_insns);
+ self->fill_insns = NULL;
+}
+
/* BPF_SK_LOOKUP contains 13 instructions, if you need to fix up maps */
#define BPF_SK_LOOKUP(func) \
/* struct bpf_sock_tuple tuple = {} */ \
diff --git a/tools/testing/selftests/bpf/verifier/var_off.c b/tools/testing/selftests/bpf/verifier/var_off.c
index 8504ac937809..3f8bee0a50ef 100644
--- a/tools/testing/selftests/bpf/verifier/var_off.c
+++ b/tools/testing/selftests/bpf/verifier/var_off.c
@@ -246,3 +246,24 @@
.result = ACCEPT,
.prog_type = BPF_PROG_TYPE_LWT_IN,
},
+{
+ "32bit loads of a 64bit field (both least and most significant words)",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1, offsetof(struct bpf_perf_event_data, sample_period)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1, offsetof(struct bpf_perf_event_data, sample_period) + 4),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_6, BPF_REG_1, offsetof(struct bpf_perf_event_data, sample_period)),
+ BPF_ALU64_IMM(BPF_LSH, BPF_REG_5, 32),
+ BPF_ALU64_REG(BPF_OR, BPF_REG_4, BPF_REG_5),
+ BPF_ALU64_REG(BPF_XOR, BPF_REG_4, BPF_REG_6),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_4),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_PERF_EVENT,
+ .ctx = { 0, },
+ .ctx_len = sizeof(struct bpf_perf_event_data),
+ .data = { 0, },
+ .data_len = sizeof(struct bpf_perf_event_value),
+ .fill_helper = bpf_fill_32bit_loads,
+ .override_data_out_len = true,
+},
--
2.20.1
^ permalink raw reply related
* [bpf-next v3 11/12] selftests/bpf: Add tests for bpf_prog_test_run for perf events progs
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
The tests check if ctx and data are correctly prepared from ctx_in and
data_in, so accessing the ctx and using the bpf_perf_prog_read_value
work as expected.
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
tools/testing/selftests/bpf/test_verifier.c | 48 ++++++++++
.../selftests/bpf/verifier/perf_event_run.c | 96 +++++++++++++++++++
2 files changed, 144 insertions(+)
create mode 100644 tools/testing/selftests/bpf/verifier/perf_event_run.c
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 6f124cc4ee34..484ea8842b06 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -295,6 +295,54 @@ static void bpf_fill_scale(struct bpf_test *self)
}
}
+static void bpf_fill_perf_event_test_run_check(struct bpf_test *self)
+{
+ compiletime_assert(
+ sizeof(struct bpf_perf_event_data) <= TEST_CTX_LEN,
+ "buffer for ctx is too short to fit struct bpf_perf_event_data");
+ compiletime_assert(
+ sizeof(struct bpf_perf_event_value) <= TEST_DATA_LEN,
+ "buffer for data is too short to fit struct bpf_perf_event_value");
+
+ struct bpf_perf_event_data ctx = {
+ .regs = (bpf_user_pt_regs_t) {
+ .r15 = 1,
+ .r14 = 2,
+ .r13 = 3,
+ .r12 = 4,
+ .rbp = 5,
+ .rbx = 6,
+ .r11 = 7,
+ .r10 = 8,
+ .r9 = 9,
+ .r8 = 10,
+ .rax = 11,
+ .rcx = 12,
+ .rdx = 13,
+ .rsi = 14,
+ .rdi = 15,
+ .orig_rax = 16,
+ .rip = 17,
+ .cs = 18,
+ .eflags = 19,
+ .rsp = 20,
+ .ss = 21,
+ },
+ .sample_period = 1,
+ .addr = 2,
+ };
+ struct bpf_perf_event_value data = {
+ .counter = 1,
+ .enabled = 2,
+ .running = 3,
+ };
+
+ memcpy(self->ctx, &ctx, sizeof(ctx));
+ memcpy(self->data, &data, sizeof(data));
+ free(self->fill_insns);
+ self->fill_insns = NULL;
+}
+
/* BPF_SK_LOOKUP contains 13 instructions, if you need to fix up maps */
#define BPF_SK_LOOKUP(func) \
/* struct bpf_sock_tuple tuple = {} */ \
diff --git a/tools/testing/selftests/bpf/verifier/perf_event_run.c b/tools/testing/selftests/bpf/verifier/perf_event_run.c
new file mode 100644
index 000000000000..3f877458a7f8
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/perf_event_run.c
@@ -0,0 +1,96 @@
+#define PER_LOAD_AND_CHECK_PTREG(PT_REG_FIELD, VALUE) \
+ PER_LOAD_AND_CHECK_CTX(offsetof(bpf_user_pt_regs_t, PT_REG_FIELD), VALUE)
+#define PER_LOAD_AND_CHECK_EVENT(PED_FIELD, VALUE) \
+ PER_LOAD_AND_CHECK_CTX(offsetof(struct bpf_perf_event_data, PED_FIELD), VALUE)
+#define PER_LOAD_AND_CHECK_CTX(OFFSET, VALUE) \
+ PER_LOAD_AND_CHECK_64(BPF_REG_4, BPF_REG_1, OFFSET, VALUE)
+#define PER_LOAD_AND_CHECK_VALUE(PEV_FIELD, VALUE) \
+ PER_LOAD_AND_CHECK_64(BPF_REG_7, BPF_REG_6, offsetof(struct bpf_perf_event_value, PEV_FIELD), VALUE)
+#define PER_LOAD_AND_CHECK_64(DST, SRC, OFFSET, VALUE) \
+ BPF_LDX_MEM(BPF_DW, DST, SRC, OFFSET), \
+ BPF_JMP_IMM(BPF_JEQ, DST, VALUE, 2), \
+ BPF_MOV64_IMM(BPF_REG_0, VALUE), \
+ BPF_EXIT_INSN()
+
+{
+ "check if regs contain expected values",
+ .insns = {
+ PER_LOAD_AND_CHECK_PTREG(r15, 1),
+ PER_LOAD_AND_CHECK_PTREG(r14, 2),
+ PER_LOAD_AND_CHECK_PTREG(r13, 3),
+ PER_LOAD_AND_CHECK_PTREG(r12, 4),
+ PER_LOAD_AND_CHECK_PTREG(rbp, 5),
+ PER_LOAD_AND_CHECK_PTREG(rbx, 6),
+ PER_LOAD_AND_CHECK_PTREG(r11, 7),
+ PER_LOAD_AND_CHECK_PTREG(r10, 8),
+ PER_LOAD_AND_CHECK_PTREG(r9, 9),
+ PER_LOAD_AND_CHECK_PTREG(r8, 10),
+ PER_LOAD_AND_CHECK_PTREG(rax, 11),
+ PER_LOAD_AND_CHECK_PTREG(rcx, 12),
+ PER_LOAD_AND_CHECK_PTREG(rdx, 13),
+ PER_LOAD_AND_CHECK_PTREG(rsi, 14),
+ PER_LOAD_AND_CHECK_PTREG(rdi, 15),
+ PER_LOAD_AND_CHECK_PTREG(orig_rax, 16),
+ PER_LOAD_AND_CHECK_PTREG(rip, 17),
+ PER_LOAD_AND_CHECK_PTREG(cs, 18),
+ PER_LOAD_AND_CHECK_PTREG(eflags, 19),
+ PER_LOAD_AND_CHECK_PTREG(rsp, 20),
+ PER_LOAD_AND_CHECK_PTREG(ss, 21),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_PERF_EVENT,
+ .ctx_len = sizeof(struct bpf_perf_event_data),
+ .data_len = sizeof(struct bpf_perf_event_value),
+ .fill_helper = bpf_fill_perf_event_test_run_check,
+ .override_data_out_len = true,
+},
+{
+ "check if sample period and addr contain expected values",
+ .insns = {
+ PER_LOAD_AND_CHECK_EVENT(sample_period, 1),
+ PER_LOAD_AND_CHECK_EVENT(addr, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_PERF_EVENT,
+ .ctx_len = sizeof(struct bpf_perf_event_data),
+ .data_len = sizeof(struct bpf_perf_event_value),
+ .fill_helper = bpf_fill_perf_event_test_run_check,
+ .override_data_out_len = true,
+},
+{
+ "check if bpf_perf_prog_read_value returns expected data",
+ .insns = {
+ // allocate space for a struct bpf_perf_event_value
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_6, -(int)sizeof(struct bpf_perf_event_value)),
+ // prepare parameters for bpf_perf_prog_read_value(ctx, struct bpf_perf_event_value*, u32)
+ // BPF_REG_1 already contains the context
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_6),
+ BPF_MOV64_IMM(BPF_REG_3, sizeof(struct bpf_perf_event_value)),
+ BPF_EMIT_CALL(BPF_FUNC_perf_prog_read_value),
+ // check the return value
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 1),
+ BPF_EXIT_INSN(),
+ // check if the fields match the expected values
+ PER_LOAD_AND_CHECK_VALUE(counter, 1),
+ PER_LOAD_AND_CHECK_VALUE(enabled, 2),
+ PER_LOAD_AND_CHECK_VALUE(running, 3),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_PERF_EVENT,
+ .ctx_len = sizeof(struct bpf_perf_event_data),
+ .data_len = sizeof(struct bpf_perf_event_value),
+ .fill_helper = bpf_fill_perf_event_test_run_check,
+ .override_data_out_len = true,
+},
+#undef PER_LOAD_AND_CHECK_64
+#undef PER_LOAD_AND_CHECK_VALUE
+#undef PER_LOAD_AND_CHECK_CTX
+#undef PER_LOAD_AND_CHECK_EVENT
+#undef PER_LOAD_AND_CHECK_PTREG
--
2.20.1
^ permalink raw reply related
* [bpf-next v3 09/12] bpf: Split out some helper functions
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
The moved functions are generally useful for implementing
bpf_prog_test_run for other types of BPF programs - they don't have
any network-specific stuff in them, so I can use them in a test run
implementation for perf event BPF program too.
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
include/linux/bpf.h | 28 +++++
kernel/bpf/Makefile | 1 +
kernel/bpf/test_run.c | 212 ++++++++++++++++++++++++++++++++++
net/bpf/test_run.c | 263 +++++++++++-------------------------------
4 files changed, 308 insertions(+), 196 deletions(-)
create mode 100644 kernel/bpf/test_run.c
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 18f4cc2c6acd..28db8ba57bc3 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1143,4 +1143,32 @@ static inline u32 bpf_xdp_sock_convert_ctx_access(enum bpf_access_type type,
}
#endif /* CONFIG_INET */
+/* Helper functions for bpf_prog_test_run implementations */
+typedef u32 bpf_prog_run_helper_t(struct bpf_prog *prog, void *ctx,
+ void *private_data);
+
+enum bpf_test_run_flags {
+ BPF_TEST_RUN_PLAIN = 0,
+ BPF_TEST_RUN_SETUP_CGROUP_STORAGE = 1 << 0,
+};
+
+int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
+ u32 *retval, u32 *duration);
+
+int bpf_test_run_cb(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
+ bpf_prog_run_helper_t run_prog, void *private_data,
+ u32 *retval, u32 *duration);
+
+int bpf_test_finish(union bpf_attr __user *uattr, u32 retval, u32 duration);
+
+void *bpf_receive_ctx(const union bpf_attr *kattr, u32 max_size);
+
+int bpf_send_ctx(const union bpf_attr *kattr, union bpf_attr __user *uattr,
+ const void *data, u32 size);
+
+void *bpf_receive_data(const union bpf_attr *kattr, u32 max_size);
+
+int bpf_send_data(const union bpf_attr *kattr, union bpf_attr __user *uattr,
+ const void *data, u32 size);
+
#endif /* _LINUX_BPF_H */
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 29d781061cd5..570fd40288f4 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -22,3 +22,4 @@ obj-$(CONFIG_CGROUP_BPF) += cgroup.o
ifeq ($(CONFIG_INET),y)
obj-$(CONFIG_BPF_SYSCALL) += reuseport_array.o
endif
+obj-$(CONFIG_BPF_SYSCALL) += test_run.o
diff --git a/kernel/bpf/test_run.c b/kernel/bpf/test_run.c
new file mode 100644
index 000000000000..0481373da8be
--- /dev/null
+++ b/kernel/bpf/test_run.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright (c) 2017 Facebook
+ * Copyright (c) 2019 Tigera, Inc
+ */
+
+#include <asm/div64.h>
+
+#include <linux/bpf-cgroup.h>
+#include <linux/bpf.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/filter.h>
+#include <linux/gfp.h>
+#include <linux/kernel.h>
+#include <linux/limits.h>
+#include <linux/preempt.h>
+#include <linux/rcupdate.h>
+#include <linux/sched.h>
+#include <linux/sched/signal.h>
+#include <linux/slab.h>
+#include <linux/timekeeping.h>
+#include <linux/uaccess.h>
+
+static void teardown_cgroup_storage(struct bpf_cgroup_storage **storage)
+{
+ enum bpf_cgroup_storage_type stype;
+
+ if (!storage)
+ return;
+ for_each_cgroup_storage_type(stype)
+ bpf_cgroup_storage_free(storage[stype]);
+ kfree(storage);
+}
+
+static struct bpf_cgroup_storage **setup_cgroup_storage(struct bpf_prog *prog)
+{
+ enum bpf_cgroup_storage_type stype;
+ struct bpf_cgroup_storage **storage;
+ size_t size = MAX_BPF_CGROUP_STORAGE_TYPE;
+
+ size *= sizeof(struct bpf_cgroup_storage *);
+ storage = kzalloc(size, GFP_KERNEL);
+ for_each_cgroup_storage_type(stype) {
+ storage[stype] = bpf_cgroup_storage_alloc(prog, stype);
+ if (IS_ERR(storage[stype])) {
+ storage[stype] = NULL;
+ teardown_cgroup_storage(storage);
+ return ERR_PTR(-ENOMEM);
+ }
+ }
+ return storage;
+}
+
+static u32 run_bpf_prog(struct bpf_prog *prog, void *ctx, void *private_data)
+{
+ return BPF_PROG_RUN(prog, ctx);
+}
+
+int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
+ u32 *retval, u32 *duration)
+{
+ return bpf_test_run_cb(prog, ctx, repeat, flags, run_bpf_prog, NULL,
+ retval, duration);
+}
+
+int bpf_test_run_cb(struct bpf_prog *prog, void *ctx, u32 repeat, u32 flags,
+ bpf_prog_run_helper_t run_prog, void *private_data,
+ u32 *retval, u32 *duration)
+{
+ struct bpf_cgroup_storage **storage = NULL;
+ u64 time_start, time_spent = 0;
+ int ret = 0;
+ u32 i;
+
+ if (flags & BPF_TEST_RUN_SETUP_CGROUP_STORAGE) {
+ storage = setup_cgroup_storage(prog);
+ if (IS_ERR(storage))
+ return PTR_ERR(storage);
+ }
+
+ if (!repeat)
+ repeat = 1;
+
+ rcu_read_lock();
+ preempt_disable();
+ time_start = ktime_get_ns();
+ for (i = 0; i < repeat; i++) {
+ if (storage)
+ bpf_cgroup_storage_set(storage);
+ *retval = run_prog(prog, ctx, private_data);
+
+ if (signal_pending(current)) {
+ preempt_enable();
+ rcu_read_unlock();
+ teardown_cgroup_storage(storage);
+ return -EINTR;
+ }
+
+ if (need_resched()) {
+ time_spent += ktime_get_ns() - time_start;
+ preempt_enable();
+ rcu_read_unlock();
+
+ cond_resched();
+
+ rcu_read_lock();
+ preempt_disable();
+ time_start = ktime_get_ns();
+ }
+ }
+ time_spent += ktime_get_ns() - time_start;
+ preempt_enable();
+ rcu_read_unlock();
+
+ do_div(time_spent, repeat);
+ *duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
+
+ teardown_cgroup_storage(storage);
+
+ return ret;
+}
+
+int bpf_test_finish(union bpf_attr __user *uattr, u32 retval, u32 duration)
+{
+ if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval)))
+ return -EFAULT;
+ if (copy_to_user(&uattr->test.duration, &duration, sizeof(duration)))
+ return -EFAULT;
+ return 0;
+}
+
+static void *bpf_receive_mem(u64 in, u32 in_size, u32 max_size)
+{
+ void __user *data_in = u64_to_user_ptr(in);
+ void *data;
+ int err;
+
+ if (!data_in && in_size)
+ return ERR_PTR(-EINVAL);
+ data = kzalloc(max_size, GFP_USER);
+ if (!data)
+ return ERR_PTR(-ENOMEM);
+
+ if (data_in) {
+ err = bpf_check_uarg_tail_zero(data_in, max_size, in_size);
+ if (err) {
+ kfree(data);
+ return ERR_PTR(err);
+ }
+
+ in_size = min_t(u32, max_size, in_size);
+ if (copy_from_user(data, data_in, in_size)) {
+ kfree(data);
+ return ERR_PTR(-EFAULT);
+ }
+ }
+ return data;
+}
+
+static int bpf_send_mem(u64 out, u32 out_size, u32 *out_size_write,
+ const void *data, u32 data_size)
+{
+ void __user *data_out = u64_to_user_ptr(out);
+ int err = -EFAULT;
+ u32 copy_size = data_size;
+
+ if (!data_out && out_size)
+ return -EINVAL;
+
+ if (!data || !data_out)
+ return 0;
+
+ if (copy_size > out_size) {
+ copy_size = out_size;
+ err = -ENOSPC;
+ }
+
+ if (copy_to_user(data_out, data, copy_size))
+ goto out;
+ if (copy_to_user(out_size_write, &data_size, sizeof(data_size)))
+ goto out;
+ if (err != -ENOSPC)
+ err = 0;
+out:
+ return err;
+}
+
+void *bpf_receive_data(const union bpf_attr *kattr, u32 max_size)
+{
+ return bpf_receive_mem(kattr->test.data_in, kattr->test.data_size_in,
+ max_size);
+}
+
+int bpf_send_data(const union bpf_attr *kattr, union bpf_attr __user *uattr,
+ const void *data, u32 size)
+{
+ return bpf_send_mem(kattr->test.data_out, kattr->test.data_size_out,
+ &uattr->test.data_size_out, data, size);
+}
+
+void *bpf_receive_ctx(const union bpf_attr *kattr, u32 max_size)
+{
+ return bpf_receive_mem(kattr->test.ctx_in, kattr->test.ctx_size_in,
+ max_size);
+}
+
+int bpf_send_ctx(const union bpf_attr *kattr, union bpf_attr __user *uattr,
+ const void *data, u32 size)
+{
+ return bpf_send_mem(kattr->test.ctx_out, kattr->test.ctx_size_out,
+ &uattr->test.ctx_size_out, data, size);
+}
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 80e6f3a6864d..fe6b7b1af0cc 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -14,97 +14,6 @@
#define CREATE_TRACE_POINTS
#include <trace/events/bpf_test_run.h>
-static int bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat,
- u32 *retval, u32 *time)
-{
- struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { NULL };
- enum bpf_cgroup_storage_type stype;
- u64 time_start, time_spent = 0;
- int ret = 0;
- u32 i;
-
- for_each_cgroup_storage_type(stype) {
- storage[stype] = bpf_cgroup_storage_alloc(prog, stype);
- if (IS_ERR(storage[stype])) {
- storage[stype] = NULL;
- for_each_cgroup_storage_type(stype)
- bpf_cgroup_storage_free(storage[stype]);
- return -ENOMEM;
- }
- }
-
- if (!repeat)
- repeat = 1;
-
- rcu_read_lock();
- preempt_disable();
- time_start = ktime_get_ns();
- for (i = 0; i < repeat; i++) {
- bpf_cgroup_storage_set(storage);
- *retval = BPF_PROG_RUN(prog, ctx);
-
- if (signal_pending(current)) {
- ret = -EINTR;
- break;
- }
-
- if (need_resched()) {
- time_spent += ktime_get_ns() - time_start;
- preempt_enable();
- rcu_read_unlock();
-
- cond_resched();
-
- rcu_read_lock();
- preempt_disable();
- time_start = ktime_get_ns();
- }
- }
- time_spent += ktime_get_ns() - time_start;
- preempt_enable();
- rcu_read_unlock();
-
- do_div(time_spent, repeat);
- *time = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
-
- for_each_cgroup_storage_type(stype)
- bpf_cgroup_storage_free(storage[stype]);
-
- return ret;
-}
-
-static int bpf_test_finish(const union bpf_attr *kattr,
- union bpf_attr __user *uattr, const void *data,
- u32 size, u32 retval, u32 duration)
-{
- void __user *data_out = u64_to_user_ptr(kattr->test.data_out);
- int err = -EFAULT;
- u32 copy_size = size;
-
- /* Clamp copy if the user has provided a size hint, but copy the full
- * buffer if not to retain old behaviour.
- */
- if (kattr->test.data_size_out &&
- copy_size > kattr->test.data_size_out) {
- copy_size = kattr->test.data_size_out;
- err = -ENOSPC;
- }
-
- if (data_out && copy_to_user(data_out, data, copy_size))
- goto out;
- if (copy_to_user(&uattr->test.data_size_out, &size, sizeof(size)))
- goto out;
- if (copy_to_user(&uattr->test.retval, &retval, sizeof(retval)))
- goto out;
- if (copy_to_user(&uattr->test.duration, &duration, sizeof(duration)))
- goto out;
- if (err != -ENOSPC)
- err = 0;
-out:
- trace_bpf_test_finish(&err);
- return err;
-}
-
static void *bpf_test_init(const union bpf_attr *kattr, u32 size,
u32 headroom, u32 tailroom)
{
@@ -125,63 +34,6 @@ static void *bpf_test_init(const union bpf_attr *kattr, u32 size,
return data;
}
-static void *bpf_ctx_init(const union bpf_attr *kattr, u32 max_size)
-{
- void __user *data_in = u64_to_user_ptr(kattr->test.ctx_in);
- void __user *data_out = u64_to_user_ptr(kattr->test.ctx_out);
- u32 size = kattr->test.ctx_size_in;
- void *data;
- int err;
-
- if (!data_in && !data_out)
- return NULL;
-
- data = kzalloc(max_size, GFP_USER);
- if (!data)
- return ERR_PTR(-ENOMEM);
-
- if (data_in) {
- err = bpf_check_uarg_tail_zero(data_in, max_size, size);
- if (err) {
- kfree(data);
- return ERR_PTR(err);
- }
-
- size = min_t(u32, max_size, size);
- if (copy_from_user(data, data_in, size)) {
- kfree(data);
- return ERR_PTR(-EFAULT);
- }
- }
- return data;
-}
-
-static int bpf_ctx_finish(const union bpf_attr *kattr,
- union bpf_attr __user *uattr, const void *data,
- u32 size)
-{
- void __user *data_out = u64_to_user_ptr(kattr->test.ctx_out);
- int err = -EFAULT;
- u32 copy_size = size;
-
- if (!data || !data_out)
- return 0;
-
- if (copy_size > kattr->test.ctx_size_out) {
- copy_size = kattr->test.ctx_size_out;
- err = -ENOSPC;
- }
-
- if (copy_to_user(data_out, data, copy_size))
- goto out;
- if (copy_to_user(&uattr->test.ctx_size_out, &size, sizeof(size)))
- goto out;
- if (err != -ENOSPC)
- err = 0;
-out:
- return err;
-}
-
/**
* range_is_zero - test whether buffer is initialized
* @buf: buffer to check
@@ -238,6 +90,36 @@ static void convert_skb_to___skb(struct sk_buff *skb, struct __sk_buff *__skb)
memcpy(__skb->cb, &cb->data, QDISC_CB_PRIV_LEN);
}
+static int bpf_net_prog_test_run_finish(const union bpf_attr *kattr,
+ union bpf_attr __user *uattr,
+ const void *data, u32 data_size,
+ const void *ctx, u32 ctx_size,
+ u32 retval, u32 duration)
+{
+ int ret;
+ union bpf_attr fixed_kattr;
+ const union bpf_attr *kattr_ptr = kattr;
+
+ /* Clamp copy (in bpf_send_mem) if the user has provided a
+ * size hint, but copy the full buffer if not to retain old
+ * behaviour.
+ */
+ if (!kattr->test.data_size_out && kattr->test.data_out) {
+ fixed_kattr = *kattr;
+ fixed_kattr.test.data_size_out = U32_MAX;
+ kattr_ptr = &fixed_kattr;
+ }
+
+ ret = bpf_send_data(kattr_ptr, uattr, data, data_size);
+ if (!ret) {
+ ret = bpf_test_finish(uattr, retval, duration);
+ if (!ret && ctx)
+ ret = bpf_send_ctx(kattr_ptr, uattr, ctx, ctx_size);
+ }
+ trace_bpf_test_finish(&ret);
+ return ret;
+}
+
int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
union bpf_attr __user *uattr)
{
@@ -257,7 +139,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
if (IS_ERR(data))
return PTR_ERR(data);
- ctx = bpf_ctx_init(kattr, sizeof(struct __sk_buff));
+ ctx = bpf_receive_ctx(kattr, sizeof(struct __sk_buff));
if (IS_ERR(ctx)) {
kfree(data);
return PTR_ERR(ctx);
@@ -307,7 +189,8 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
ret = convert___skb_to_skb(skb, ctx);
if (ret)
goto out;
- ret = bpf_test_run(prog, skb, repeat, &retval, &duration);
+ ret = bpf_test_run(prog, skb, repeat, BPF_TEST_RUN_SETUP_CGROUP_STORAGE,
+ &retval, &duration);
if (ret)
goto out;
if (!is_l2) {
@@ -327,10 +210,9 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
/* bpf program can never convert linear skb to non-linear */
if (WARN_ON_ONCE(skb_is_nonlinear(skb)))
size = skb_headlen(skb);
- ret = bpf_test_finish(kattr, uattr, skb->data, size, retval, duration);
- if (!ret)
- ret = bpf_ctx_finish(kattr, uattr, ctx,
- sizeof(struct __sk_buff));
+ ret = bpf_net_prog_test_run_finish(kattr, uattr, skb->data, size,
+ ctx, sizeof(struct __sk_buff),
+ retval, duration);
out:
kfree_skb(skb);
bpf_sk_storage_free(sk);
@@ -365,32 +247,48 @@ 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;
- ret = bpf_test_run(prog, &xdp, repeat, &retval, &duration);
+ ret = bpf_test_run(prog, &xdp, repeat,
+ BPF_TEST_RUN_SETUP_CGROUP_STORAGE,
+ &retval, &duration);
if (ret)
goto out;
if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN ||
xdp.data_end != xdp.data + size)
size = xdp.data_end - xdp.data;
- ret = bpf_test_finish(kattr, uattr, xdp.data, size, retval, duration);
+ ret = bpf_net_prog_test_run_finish(kattr, uattr, xdp.data, size,
+ NULL, 0, retval, duration);
out:
kfree(data);
return ret;
}
+struct bpf_flow_dissect_run_data {
+ __be16 proto;
+ int nhoff;
+ int hlen;
+};
+
+static u32 bpf_flow_dissect_run(struct bpf_prog *prog, void *ctx,
+ void *private_data)
+{
+ struct bpf_flow_dissect_run_data *data = private_data;
+
+ return bpf_flow_dissect(prog, ctx, data->proto, data->nhoff, data->hlen);
+}
+
int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
const union bpf_attr *kattr,
union bpf_attr __user *uattr)
{
+ struct bpf_flow_dissect_run_data run_data = {};
u32 size = kattr->test.data_size_in;
struct bpf_flow_dissector ctx = {};
u32 repeat = kattr->test.repeat;
struct bpf_flow_keys flow_keys;
- u64 time_start, time_spent = 0;
const struct ethhdr *eth;
u32 retval, duration;
void *data;
int ret;
- u32 i;
if (prog->type != BPF_PROG_TYPE_FLOW_DISSECTOR)
return -EINVAL;
@@ -407,49 +305,22 @@ int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
eth = (struct ethhdr *)data;
- if (!repeat)
- repeat = 1;
-
ctx.flow_keys = &flow_keys;
ctx.data = data;
ctx.data_end = (__u8 *)data + size;
- rcu_read_lock();
- preempt_disable();
- time_start = ktime_get_ns();
- for (i = 0; i < repeat; i++) {
- retval = bpf_flow_dissect(prog, &ctx, eth->h_proto, ETH_HLEN,
- size);
-
- if (signal_pending(current)) {
- preempt_enable();
- rcu_read_unlock();
-
- ret = -EINTR;
- goto out;
- }
-
- if (need_resched()) {
- time_spent += ktime_get_ns() - time_start;
- preempt_enable();
- rcu_read_unlock();
-
- cond_resched();
-
- rcu_read_lock();
- preempt_disable();
- time_start = ktime_get_ns();
- }
- }
- time_spent += ktime_get_ns() - time_start;
- preempt_enable();
- rcu_read_unlock();
-
- do_div(time_spent, repeat);
- duration = time_spent > U32_MAX ? U32_MAX : (u32)time_spent;
+ run_data.proto = eth->h_proto;
+ run_data.nhoff = ETH_HLEN;
+ run_data.hlen = size;
+ ret = bpf_test_run_cb(prog, &ctx, repeat, BPF_TEST_RUN_PLAIN,
+ bpf_flow_dissect_run, &run_data,
+ &retval, &duration);
+ if (!ret)
+ goto out;
- ret = bpf_test_finish(kattr, uattr, &flow_keys, sizeof(flow_keys),
- retval, duration);
+ ret = bpf_net_prog_test_run_finish(kattr, uattr, &flow_keys,
+ sizeof(flow_keys), NULL, 0,
+ retval, duration);
out:
kfree(data);
--
2.20.1
^ permalink raw reply related
* [bpf-next v3 10/12] bpf: Implement bpf_prog_test_run for perf event programs
From: Krzesimir Nowak @ 2019-07-08 16:31 UTC (permalink / raw)
To: linux-kernel
Cc: Alban Crequy, Iago López Galeiras, Alexei Starovoitov,
Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
David S. Miller, Jakub Kicinski, Jesper Dangaard Brouer,
John Fastabend, Stanislav Fomichev, netdev, bpf, xdp-newbies,
Krzesimir Nowak
In-Reply-To: <20190708163121.18477-1-krzesimir@kinvolk.io>
As an input, test run for perf event program takes struct
bpf_perf_event_data as ctx_in and struct bpf_perf_event_value as
data_in. For an output, it basically ignores ctx_out and data_out.
The implementation sets an instance of struct bpf_perf_event_data_kern
in such a way that the BPF program reading data from context will
receive what we passed to the bpf prog test run in ctx_in. Also BPF
program can call bpf_perf_prog_read_value to receive what was passed
in data_in.
Changes since v2:
- drop the changes in perf event verifier test - they are not needed
anymore after reworked ctx size handling
Signed-off-by: Krzesimir Nowak <krzesimir@kinvolk.io>
---
kernel/trace/bpf_trace.c | 60 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index ca1255d14576..b870fc2314d0 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -19,6 +19,8 @@
#include "trace_probe.h"
#include "trace.h"
+#include <trace/events/bpf_test_run.h>
+
#define bpf_event_rcu_dereference(p) \
rcu_dereference_protected(p, lockdep_is_held(&bpf_event_mutex))
@@ -1160,7 +1162,65 @@ const struct bpf_verifier_ops perf_event_verifier_ops = {
.convert_ctx_access = pe_prog_convert_ctx_access,
};
+static int pe_prog_test_run(struct bpf_prog *prog,
+ const union bpf_attr *kattr,
+ union bpf_attr __user *uattr)
+{
+ struct bpf_perf_event_data_kern real_ctx = {0, };
+ struct perf_sample_data sample_data = {0, };
+ struct bpf_perf_event_data *fake_ctx;
+ struct bpf_perf_event_value *value;
+ struct perf_event event = {0, };
+ u32 retval = 0, duration = 0;
+ int err;
+
+ if (kattr->test.data_size_out || kattr->test.data_out)
+ return -EINVAL;
+ if (kattr->test.ctx_size_out || kattr->test.ctx_out)
+ return -EINVAL;
+
+ fake_ctx = bpf_receive_ctx(kattr, sizeof(struct bpf_perf_event_data));
+ if (IS_ERR(fake_ctx))
+ return PTR_ERR(fake_ctx);
+
+ value = bpf_receive_data(kattr, sizeof(struct bpf_perf_event_value));
+ if (IS_ERR(value)) {
+ kfree(fake_ctx);
+ return PTR_ERR(value);
+ }
+
+ real_ctx.regs = &fake_ctx->regs;
+ real_ctx.data = &sample_data;
+ real_ctx.event = &event;
+ perf_sample_data_init(&sample_data, fake_ctx->addr,
+ fake_ctx->sample_period);
+ event.cpu = smp_processor_id();
+ event.oncpu = -1;
+ event.state = PERF_EVENT_STATE_OFF;
+ local64_set(&event.count, value->counter);
+ event.total_time_enabled = value->enabled;
+ event.total_time_running = value->running;
+ /* make self as a leader - it is used only for checking the
+ * state field
+ */
+ event.group_leader = &event;
+ err = bpf_test_run(prog, &real_ctx, kattr->test.repeat,
+ BPF_TEST_RUN_PLAIN, &retval, &duration);
+ if (err) {
+ kfree(value);
+ kfree(fake_ctx);
+ return err;
+ }
+
+ err = bpf_test_finish(uattr, retval, duration);
+ trace_bpf_test_finish(&err);
+ kfree(value);
+ kfree(fake_ctx);
+ return err;
+}
+
const struct bpf_prog_ops perf_event_prog_ops = {
+ .test_run = pe_prog_test_run,
};
static DEFINE_MUTEX(bpf_event_mutex);
--
2.20.1
^ permalink raw reply related
* [PATCH] sunrpc/cache: remove the exporting of cache_seq_next
From: Denis Efremov @ 2019-07-08 16:14 UTC (permalink / raw)
To: Trond Myklebust
Cc: Denis Efremov, J. Bruce Fields, Chuck Lever, Anna Schumaker,
David S. Miller, linux-nfs, netdev, linux-kernel
The function cache_seq_next is declared static and marked
EXPORT_SYMBOL_GPL, which is at best an odd combination. Because the
function is not used outside of the net/sunrpc/cache.c file it is
defined in, this commit removes the EXPORT_SYMBOL_GPL() marking.
Fixes: d48cf356a130 ("SUNRPC: Remove non-RCU protected lookup")
Signed-off-by: Denis Efremov <efremov@linux.com>
---
net/sunrpc/cache.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 66fbb9d2fba7..6f1528f271ee 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -1375,7 +1375,6 @@ static void *cache_seq_next(struct seq_file *m, void *p, loff_t *pos)
hlist_first_rcu(&cd->hash_table[hash])),
struct cache_head, cache_list);
}
-EXPORT_SYMBOL_GPL(cache_seq_next);
void *cache_seq_start_rcu(struct seq_file *m, loff_t *pos)
__acquires(RCU)
--
2.21.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox