* Re: [PATCH 3/3] tools bpftool: Display license GPL compatible in prog show/list
From: Jakub Kicinski @ 2018-04-25 21:03 UTC (permalink / raw)
To: Jiri Olsa
Cc: Alexei Starovoitov, Daniel Borkmann, lkml, netdev, Quentin Monnet
In-Reply-To: <20180425174108.6586-4-jolsa@kernel.org>
On Wed, 25 Apr 2018 19:41:08 +0200, Jiri Olsa wrote:
> @@ -295,6 +297,7 @@ static void print_prog_plain(struct bpf_prog_info *info, int fd)
> printf("tag ");
> fprint_hex(stdout, info->tag, BPF_TAG_SIZE, "");
> print_dev_plain(info->ifindex, info->netns_dev, info->netns_ino);
> + printf(" license GPL %scompatible", info->gpl_compatible ? "" : "NON ");
3 nit picks:
Other "fields" are separated by two spaces between each other:
4: kprobe name func_begin tag 57cd311f2e27366b license GPL compatible
^^ ^^ X
loaded_at Apr 25/11:20 uid 0
^^
xlated 16B not jited memlock 4096B
^^ ^^
Could you also update the example outputs in the man page:
tools/bpf/bpftool/Documentation/bpftool-prog.rst
Sorry about the bike shedding but I would also vote for:
"[not] GPL compatible"
rather than
"license GPL [NON] compatible"
for brevity..
^ permalink raw reply
* Re: [PATCH net-next 02/10] udp: add gso
From: Alexander Duyck @ 2018-04-25 21:02 UTC (permalink / raw)
To: Willem de Bruijn
Cc: Netdev, David Miller, Dimitris Michailidis, Willem de Bruijn
In-Reply-To: <CAF=yD-K2Oqf14oC-iH2sj+0cWdimdoAuY4HOYgF=BXVFbQMhEQ@mail.gmail.com>
On Wed, Apr 25, 2018 at 1:51 PM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>> It might be nice if you could break this into two patches. One for
>> actually doing the GSO in software, and another enabling the stack to
>> request it.
>
> Okay.
>
>>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>>> index d274059529eb..a4a5c0c5cba8 100644
>>> --- a/include/linux/skbuff.h
>>> +++ b/include/linux/skbuff.h
>>> @@ -573,6 +573,8 @@ enum {
>>> SKB_GSO_ESP = 1 << 15,
>>>
>>> SKB_GSO_UDP = 1 << 16,
>>> +
>>> + SKB_GSO_UDP_L4 = 1 << 17,
>>> };
>>>
>>
>> Part of me really wishes we could just rename SKB_GSO_UDP to something
>> like SKB_GFO_UDP since GSO implies "segmentation" but what we really
>> mean is "fragmentation" in the case of the existing UDP offload.
>
> I have been itching to rename the UFO flag, though I then find
> SKB_GSO_UFO easier to differentiate from SKB_GSO_UDP
> than SKB_GFO_UDP.
>
> But, the uapi headers will continue to have
> VIRTIO_NET_HDR_GSO_UDP for UFO, so changing this
> might only further complicate things.
Agreed. I was just venting frustration that we didn't do something
about it sooner.
>>> +struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
>>> + netdev_features_t features,
>>> + unsigned int mss, __sum16 check)
>>> +{
>>> + struct udphdr *uh = udp_hdr(gso_skb);
>>> + struct sk_buff *segs;
>>> + unsigned int hdrlen;
>>> +
>>> + if (gso_skb->len <= sizeof(*uh) + mss)
>>> + return ERR_PTR(-EINVAL);
>>> +
>>> + uh->len = htons(sizeof(*uh) + mss);
>>> + uh->check = check;
>>> + skb_pull(gso_skb, sizeof(*uh));
>>> + hdrlen = gso_skb->data - skb_mac_header(gso_skb);
>>
>> Normally I don't believe we modify the headers before calling
>> skb_segment. Normally that happens after via a while (skb->next) {}
>> type loop.
>
> Setting the header for all but the last segment before segmentation
> avoided the need for a while loop, at least before the wmem_alloc
> patch.
>
> With that patch, the argument no longer really holds, so I can convert
> if that is needed for other features, like GSO_PARTIAL.
Thanks.
>> That way for things like GSO_PARTIAL we can update after segmentation
>> since there are only going to be 2 segments most likely instead of
>> multiple MSS sized segments.
>
> I don't quite follow. Which two segments?
When we do GSO partial we end up with 2 segments. One really big one
that is a multiple of MSS and the remainder assuming the frame is odd
sized. The idea is we can just replicate all of the headers from the
outer IP header to the inner transport header in hardware so we do all
the updates based on that assumption and then we do the standard
segmentation update on the tail skb.
>>> +
>>> + segs = skb_segment(gso_skb, features);
>>> + if (unlikely(IS_ERR_OR_NULL(segs)))
>>> + return segs;
>>> +
>>> + /* If last packet is not full, fix up its header */
>>> + if (segs->prev->len != hdrlen + mss) {
>>> + unsigned int mss_last = segs->prev->len - hdrlen;
>>> +
>>> + uh = udp_hdr(segs->prev);
>>> + uh->len = htons(sizeof(*uh) + mss_last);
>>> + csum_replace2(&uh->check, htons(mss), htons(mss_last));
>>> + }
>>> +
>>
>> You could probably just assume that this last segment is always going
>> to need to be updated regardless. If you are doing any sort of
>> segmentation the last segment will always need the length and checksum
>> updated anyway, and since you probably need to move over to the "while
>> (skb->next)" style loop then you could just assume the last segment
>> needs updating.
>
> Ack. If we have to do a loop and set everything in there, I'll just write
> all headers unconditionally.
^ permalink raw reply
* Business Proposal,
From: Mrs Zeliha Faruk @ 2018-04-25 21:00 UTC (permalink / raw)
To: Recipients
Hello Dear
Greetings to you, please I have a very important business proposal for our mutual benefit, please let me know if you are interested.
Best Regards,
Miss. Zeliha ömer Faruk
Caddesi Kristal Kule Binasi
No:215
^ permalink raw reply
* [PATCH v5] fault-injection: introduce kvmalloc fallback options
From: Mikulas Patocka @ 2018-04-25 20:57 UTC (permalink / raw)
To: Randy Dunlap
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, Matthew Wilcox,
Michal Hocko, linux-mm, edumazet, Andrew Morton, virtualization,
David Miller, Vlastimil Babka
In-Reply-To: <1114eda5-9b1f-4db8-2090-556b4a37c532@infradead.org>
On Wed, 25 Apr 2018, Randy Dunlap wrote:
> On 04/25/2018 01:02 PM, Mikulas Patocka wrote:
> >
> >
> > From: Mikulas Patocka <mpatocka@redhat.com>
> > Subject: [PATCH v4] fault-injection: introduce kvmalloc fallback options
> >
> > This patch introduces a fault-injection option "kvmalloc_fallback". This
> > option makes kvmalloc randomly fall back to vmalloc.
> >
> > Unfortunatelly, some kernel code has bugs - it uses kvmalloc and then
>
> Unfortunately,
OK - here I fixed the typos:
From: Mikulas Patocka <mpatocka@redhat.com>
Subject: [PATCH] fault-injection: introduce kvmalloc fallback options
This patch introduces a fault-injection option "kvmalloc_fallback". This
option makes kvmalloc randomly fall back to vmalloc.
Unfortunately, some kernel code has bugs - it uses kvmalloc and then
uses DMA-API on the returned memory or frees it with kfree. Such bugs were
found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific
code. This options helps to test for these bugs.
The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY.
It can be enabled in distribution debug kernels, so that kvmalloc abuse
can be tested by the users. The default can be overridden with
"kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
Documentation/fault-injection/fault-injection.txt | 7 +++++
include/linux/fault-inject.h | 9 +++---
kernel/futex.c | 2 -
lib/Kconfig.debug | 15 +++++++++++
mm/failslab.c | 2 -
mm/page_alloc.c | 2 -
mm/util.c | 30 ++++++++++++++++++++++
7 files changed, 60 insertions(+), 7 deletions(-)
Index: linux-2.6/Documentation/fault-injection/fault-injection.txt
===================================================================
--- linux-2.6.orig/Documentation/fault-injection/fault-injection.txt 2018-04-16 21:08:34.000000000 +0200
+++ linux-2.6/Documentation/fault-injection/fault-injection.txt 2018-04-25 21:36:36.000000000 +0200
@@ -15,6 +15,12 @@ o fail_page_alloc
injects page allocation failures. (alloc_pages(), get_free_pages(), ...)
+o kvmalloc_fallback
+
+ makes the function kvmalloc randomly fall back to vmalloc. This could be used
+ to detects bugs such as using DMA-API on the result of kvmalloc or freeing
+ the result of kvmalloc with free.
+
o fail_futex
injects futex deadlock and uaddr fault errors.
@@ -167,6 +173,7 @@ use the boot option:
failslab=
fail_page_alloc=
+ kvmalloc_fallback=
fail_make_request=
fail_futex=
mmc_core.fail_request=<interval>,<probability>,<space>,<times>
Index: linux-2.6/include/linux/fault-inject.h
===================================================================
--- linux-2.6.orig/include/linux/fault-inject.h 2018-04-16 21:08:36.000000000 +0200
+++ linux-2.6/include/linux/fault-inject.h 2018-04-25 21:38:22.000000000 +0200
@@ -31,17 +31,18 @@ struct fault_attr {
struct dentry *dname;
};
-#define FAULT_ATTR_INITIALIZER { \
+#define FAULT_ATTR_INITIALIZER(p) { \
+ .probability = (p), \
.interval = 1, \
- .times = ATOMIC_INIT(1), \
+ .times = ATOMIC_INIT((p) ? -1 : 1), \
+ .verbose = (p) ? 0 : 2, \
.require_end = ULONG_MAX, \
.stacktrace_depth = 32, \
.ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \
- .verbose = 2, \
.dname = NULL, \
}
-#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER
+#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER(0)
int setup_fault_attr(struct fault_attr *attr, char *str);
bool should_fail(struct fault_attr *attr, ssize_t size);
Index: linux-2.6/lib/Kconfig.debug
===================================================================
--- linux-2.6.orig/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200
+++ linux-2.6/lib/Kconfig.debug 2018-04-25 21:39:45.000000000 +0200
@@ -1527,6 +1527,21 @@ config FAIL_PAGE_ALLOC
help
Provide fault-injection capability for alloc_pages().
+config FAIL_KVMALLOC_FALLBACK_PROBABILITY
+ int "Default kvmalloc fallback probability"
+ depends on FAULT_INJECTION
+ range 0 100
+ default "0"
+ help
+ This option will make kvmalloc randomly fall back to vmalloc.
+ Normally, kvmalloc falls back to vmalloc only rarely, if memory
+ is fragmented.
+
+ This option helps to detect hard-to-reproduce driver bugs, for
+ example using DMA API on the result of kvmalloc.
+
+ The default may be overridden with the kvmalloc_fallback parameter.
+
config FAIL_MAKE_REQUEST
bool "Fault-injection capability for disk IO"
depends on FAULT_INJECTION && BLOCK
Index: linux-2.6/mm/util.c
===================================================================
--- linux-2.6.orig/mm/util.c 2018-04-25 15:48:39.000000000 +0200
+++ linux-2.6/mm/util.c 2018-04-25 21:43:31.000000000 +0200
@@ -14,6 +14,7 @@
#include <linux/hugetlb.h>
#include <linux/vmalloc.h>
#include <linux/userfaultfd_k.h>
+#include <linux/fault-inject.h>
#include <asm/sections.h>
#include <linux/uaccess.h>
@@ -377,6 +378,29 @@ unsigned long vm_mmap(struct file *file,
}
EXPORT_SYMBOL(vm_mmap);
+#ifdef CONFIG_FAULT_INJECTION
+
+static struct fault_attr kvmalloc_fallback =
+ FAULT_ATTR_INITIALIZER(CONFIG_FAIL_KVMALLOC_FALLBACK_PROBABILITY);
+
+static int __init setup_kvmalloc_fallback(char *str)
+{
+ return setup_fault_attr(&kvmalloc_fallback, str);
+}
+
+__setup("kvmalloc_fallback=", setup_kvmalloc_fallback);
+
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+static int __init kvmalloc_fallback_debugfs_init(void)
+{
+ fault_create_debugfs_attr("kvmalloc_fallback", NULL, &kvmalloc_fallback);
+ return 0;
+}
+late_initcall(kvmalloc_fallback_debugfs_init);
+#endif
+
+#endif
+
/**
* kvmalloc_node - attempt to allocate physically contiguous memory, but upon
* failure, fall back to non-contiguous (vmalloc) allocation.
@@ -404,6 +428,11 @@ void *kvmalloc_node(size_t size, gfp_t f
*/
WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL);
+#ifdef CONFIG_FAULT_INJECTION
+ if (should_fail(&kvmalloc_fallback, size))
+ goto do_vmalloc;
+#endif
+
/*
* We want to attempt a large physically contiguous block first because
* it is less likely to fragment multiple larger blocks and therefore
@@ -427,6 +456,7 @@ void *kvmalloc_node(size_t size, gfp_t f
if (ret || size <= PAGE_SIZE)
return ret;
+do_vmalloc: __maybe_unused
return __vmalloc_node_flags_caller(size, node, flags,
__builtin_return_address(0));
}
Index: linux-2.6/kernel/futex.c
===================================================================
--- linux-2.6.orig/kernel/futex.c 2018-02-14 20:24:42.000000000 +0100
+++ linux-2.6/kernel/futex.c 2018-04-25 21:11:33.000000000 +0200
@@ -288,7 +288,7 @@ static struct {
bool ignore_private;
} fail_futex = {
- .attr = FAULT_ATTR_INITIALIZER,
+ .attr = FAULT_ATTR_INITIALIZER(0),
.ignore_private = false,
};
Index: linux-2.6/mm/failslab.c
===================================================================
--- linux-2.6.orig/mm/failslab.c 2018-04-16 21:08:36.000000000 +0200
+++ linux-2.6/mm/failslab.c 2018-04-25 21:11:40.000000000 +0200
@@ -9,7 +9,7 @@ static struct {
bool ignore_gfp_reclaim;
bool cache_filter;
} failslab = {
- .attr = FAULT_ATTR_INITIALIZER,
+ .attr = FAULT_ATTR_INITIALIZER(0),
.ignore_gfp_reclaim = true,
.cache_filter = false,
};
Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c 2018-04-16 21:08:36.000000000 +0200
+++ linux-2.6/mm/page_alloc.c 2018-04-25 21:11:47.000000000 +0200
@@ -3055,7 +3055,7 @@ static struct {
bool ignore_gfp_reclaim;
u32 min_order;
} fail_page_alloc = {
- .attr = FAULT_ATTR_INITIALIZER,
+ .attr = FAULT_ATTR_INITIALIZER(0),
.ignore_gfp_reclaim = true,
.ignore_gfp_highmem = true,
.min_order = 1,
^ permalink raw reply
* Re: [PATCH net-next 02/10] udp: add gso
From: David Miller @ 2018-04-25 20:54 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: alexander.duyck, netdev, dmichail, willemb
In-Reply-To: <CAF=yD-K2Oqf14oC-iH2sj+0cWdimdoAuY4HOYgF=BXVFbQMhEQ@mail.gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Wed, 25 Apr 2018 16:51:41 -0400
> I have been itching to rename the UFO flag, though I then find
> SKB_GSO_UFO easier to differentiate from SKB_GSO_UDP
> than SKB_GFO_UDP.
>
> But, the uapi headers will continue to have
> VIRTIO_NET_HDR_GSO_UDP for UFO, so changing this
> might only further complicate things.
I think changing the name makes things more complicated, exactly
because of all of this other legacy stuff.
^ permalink raw reply
* Re: [PATCH net-next 02/10] udp: add gso
From: Willem de Bruijn @ 2018-04-25 20:51 UTC (permalink / raw)
To: Alexander Duyck
Cc: Netdev, David Miller, Dimitris Michailidis, Willem de Bruijn
In-Reply-To: <CAKgT0UfP7ztrtV7smQviAXZyaiPAwCSARuKnbKnc3SP_R1ogsQ@mail.gmail.com>
> It might be nice if you could break this into two patches. One for
> actually doing the GSO in software, and another enabling the stack to
> request it.
Okay.
>> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
>> index d274059529eb..a4a5c0c5cba8 100644
>> --- a/include/linux/skbuff.h
>> +++ b/include/linux/skbuff.h
>> @@ -573,6 +573,8 @@ enum {
>> SKB_GSO_ESP = 1 << 15,
>>
>> SKB_GSO_UDP = 1 << 16,
>> +
>> + SKB_GSO_UDP_L4 = 1 << 17,
>> };
>>
>
> Part of me really wishes we could just rename SKB_GSO_UDP to something
> like SKB_GFO_UDP since GSO implies "segmentation" but what we really
> mean is "fragmentation" in the case of the existing UDP offload.
I have been itching to rename the UFO flag, though I then find
SKB_GSO_UFO easier to differentiate from SKB_GSO_UDP
than SKB_GFO_UDP.
But, the uapi headers will continue to have
VIRTIO_NET_HDR_GSO_UDP for UFO, so changing this
might only further complicate things.
>> +struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
>> + netdev_features_t features,
>> + unsigned int mss, __sum16 check)
>> +{
>> + struct udphdr *uh = udp_hdr(gso_skb);
>> + struct sk_buff *segs;
>> + unsigned int hdrlen;
>> +
>> + if (gso_skb->len <= sizeof(*uh) + mss)
>> + return ERR_PTR(-EINVAL);
>> +
>> + uh->len = htons(sizeof(*uh) + mss);
>> + uh->check = check;
>> + skb_pull(gso_skb, sizeof(*uh));
>> + hdrlen = gso_skb->data - skb_mac_header(gso_skb);
>
> Normally I don't believe we modify the headers before calling
> skb_segment. Normally that happens after via a while (skb->next) {}
> type loop.
Setting the header for all but the last segment before segmentation
avoided the need for a while loop, at least before the wmem_alloc
patch.
With that patch, the argument no longer really holds, so I can convert
if that is needed for other features, like GSO_PARTIAL.
> That way for things like GSO_PARTIAL we can update after segmentation
> since there are only going to be 2 segments most likely instead of
> multiple MSS sized segments.
I don't quite follow. Which two segments?
>> +
>> + segs = skb_segment(gso_skb, features);
>> + if (unlikely(IS_ERR_OR_NULL(segs)))
>> + return segs;
>> +
>> + /* If last packet is not full, fix up its header */
>> + if (segs->prev->len != hdrlen + mss) {
>> + unsigned int mss_last = segs->prev->len - hdrlen;
>> +
>> + uh = udp_hdr(segs->prev);
>> + uh->len = htons(sizeof(*uh) + mss_last);
>> + csum_replace2(&uh->check, htons(mss), htons(mss_last));
>> + }
>> +
>
> You could probably just assume that this last segment is always going
> to need to be updated regardless. If you are doing any sort of
> segmentation the last segment will always need the length and checksum
> updated anyway, and since you probably need to move over to the "while
> (skb->next)" style loop then you could just assume the last segment
> needs updating.
Ack. If we have to do a loop and set everything in there, I'll just write
all headers unconditionally.
^ permalink raw reply
* Re: [PATCH RFC 2/9] veth: Add driver XDP
From: Jesper Dangaard Brouer @ 2018-04-25 20:39 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: brouer, netdev, Toshiaki Makita
In-Reply-To: <20180424143923.26519-3-toshiaki.makita1@gmail.com>
On Tue, 24 Apr 2018 23:39:16 +0900
Toshiaki Makita <toshiaki.makita1@gmail.com> wrote:
> This is basic implementation of veth driver XDP.
>
> Incoming packets are sent from the peer veth device in the form of skb,
> so this is generally doing the same thing as generic XDP.
I'm unsure that context you are calling veth_xdp_rcv_skb() from. The
XDP (RX side) depend heavily on the protection provided by NAPI context.
It looks like you are adding NAPI handler later.
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH net-next 02/10] udp: add gso
From: Alexander Duyck @ 2018-04-25 20:38 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: Netdev, David Miller, dmichail, Willem de Bruijn
In-Reply-To: <20180425185601.55511-3-willemdebruijn.kernel@gmail.com>
On Wed, Apr 25, 2018 at 11:55 AM, Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> Implement generic segmentation offload for udp datagrams. Callers can
> concatenate and send at once the payload of multiple datagrams with
> the same destination.
>
> To set segment size, the caller sets socket option UDP_SEGMENT to the
> length of each discrete payload. This value must be smaller than or
> equal to the relevant MTU.
>
> A follow-up patch adds cmsg UDP_SEGMENT to specify segment size on a
> per send call basis.
>
> Total byte length may then exceed MTU. If not an exact multiple of
> segment size, the last segment will be shorter.
>
> UDP GSO is not UFO. UFO fragments a single large datagram. GSO splits
> a large payload into a number of discrete UDP datagrams.
>
> The implementation adds a GSO type SKB_UDP_GSO_L4 to differentiate it
> from UFO (SKB_UDP_GSO). It adds a gso_size field to the udp socket,
> ip(v6) cmsg cookie and inet_cork structure to be able to set the value
> at setsockopt or cmsg time and to work with both lockless and corked
> paths.
>
> IPPROTO_UDPLITE is excluded, as that protocol has no gso handler
> registered.
>
> Initial benchmark numbers show UDP GSO about as expensive as TCP GSO.
>
> tcp tso
> 3197 MB/s 54232 msg/s 54232 calls/s
> 6,457,754,262 cycles
>
> tcp gso
> 1765 MB/s 29939 msg/s 29939 calls/s
> 11,203,021,806 cycles
>
> tcp without tso/gso *
> 739 MB/s 12548 msg/s 12548 calls/s
> 11,205,483,630 cycles
>
> udp
> 876 MB/s 14873 msg/s 624666 calls/s
> 11,205,777,429 cycles
>
> udp gso
> 2139 MB/s 36282 msg/s 36282 calls/s
> 11,204,374,561 cycles
>
> [*] after reverting commit 0a6b2a1dc2a2
> ("tcp: switch to GSO being always on")
>
> Measured total system cycles ('-a') for one core while pinning both
> the network receive path and benchmark process to that core:
>
> perf stat -a -C 12 -e cycles \
> ./udpgso_bench_tx -C 12 -4 -D "$DST" -l 4
>
> Note the reduction in calls/s with GSO. Bytes per syscall drops
> increases from 1470 to 61818.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
It might be nice if you could break this into two patches. One for
actually doing the GSO in software, and another enabling the stack to
request it.
> ---
> include/linux/skbuff.h | 2 ++
> include/linux/udp.h | 3 +++
> include/net/inet_sock.h | 1 +
> include/net/ip.h | 1 +
> include/net/ipv6.h | 1 +
> include/net/udp.h | 4 ++++
> include/uapi/linux/udp.h | 1 +
> net/core/skbuff.c | 2 ++
> net/ipv4/ip_output.c | 9 ++++---
> net/ipv4/udp.c | 33 ++++++++++++++++++++++---
> net/ipv4/udp_offload.c | 52 +++++++++++++++++++++++++++++++++++++++-
> net/ipv6/ip6_offload.c | 6 +++--
> net/ipv6/ip6_output.c | 6 +++--
> net/ipv6/udp.c | 23 +++++++++++++++---
> net/ipv6/udp_offload.c | 19 ++++++++++++++-
> 15 files changed, 148 insertions(+), 15 deletions(-)
>
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index d274059529eb..a4a5c0c5cba8 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -573,6 +573,8 @@ enum {
> SKB_GSO_ESP = 1 << 15,
>
> SKB_GSO_UDP = 1 << 16,
> +
> + SKB_GSO_UDP_L4 = 1 << 17,
> };
>
Part of me really wishes we could just rename SKB_GSO_UDP to something
like SKB_GFO_UDP since GSO implies "segmentation" but what we really
mean is "fragmentation" in the case of the existing UDP offload.
> #if BITS_PER_LONG > 32
> diff --git a/include/linux/udp.h b/include/linux/udp.h
> index eaea63bc79bb..ca840345571b 100644
> --- a/include/linux/udp.h
> +++ b/include/linux/udp.h
> @@ -55,6 +55,7 @@ struct udp_sock {
> * when the socket is uncorked.
> */
> __u16 len; /* total length of pending frames */
> + __u16 gso_size;
> /*
> * Fields specific to UDP-Lite.
> */
> @@ -87,6 +88,8 @@ struct udp_sock {
> int forward_deficit;
> };
>
> +#define UDP_MAX_SEGMENTS (1 << 6UL)
> +
> static inline struct udp_sock *udp_sk(const struct sock *sk)
> {
> return (struct udp_sock *)sk;
> diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
> index 0a671c32d6b9..83d5b3c2ac42 100644
> --- a/include/net/inet_sock.h
> +++ b/include/net/inet_sock.h
> @@ -147,6 +147,7 @@ struct inet_cork {
> __u8 ttl;
> __s16 tos;
> char priority;
> + __u16 gso_size;
> };
>
> struct inet_cork_full {
> diff --git a/include/net/ip.h b/include/net/ip.h
> index 7ec543a64bbc..bada1f1f871e 100644
> --- a/include/net/ip.h
> +++ b/include/net/ip.h
> @@ -76,6 +76,7 @@ struct ipcm_cookie {
> __u8 ttl;
> __s16 tos;
> char priority;
> + __u16 gso_size;
> };
>
> #define IPCB(skb) ((struct inet_skb_parm*)((skb)->cb))
> diff --git a/include/net/ipv6.h b/include/net/ipv6.h
> index 0dd722cab037..0a872a7c33c8 100644
> --- a/include/net/ipv6.h
> +++ b/include/net/ipv6.h
> @@ -298,6 +298,7 @@ struct ipcm6_cookie {
> __s16 tclass;
> __s8 dontfrag;
> struct ipv6_txoptions *opt;
> + __u16 gso_size;
> };
>
> static inline struct ipv6_txoptions *txopt_get(const struct ipv6_pinfo *np)
> diff --git a/include/net/udp.h b/include/net/udp.h
> index 0676b272f6ac..741d888d0fdb 100644
> --- a/include/net/udp.h
> +++ b/include/net/udp.h
> @@ -174,6 +174,10 @@ struct sk_buff **udp_gro_receive(struct sk_buff **head, struct sk_buff *skb,
> struct udphdr *uh, udp_lookup_t lookup);
> int udp_gro_complete(struct sk_buff *skb, int nhoff, udp_lookup_t lookup);
>
> +struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> + netdev_features_t features,
> + unsigned int mss, __sum16 check);
> +
> static inline struct udphdr *udp_gro_udphdr(struct sk_buff *skb)
> {
> struct udphdr *uh;
> diff --git a/include/uapi/linux/udp.h b/include/uapi/linux/udp.h
> index efb7b5991c2f..09d00f8c442b 100644
> --- a/include/uapi/linux/udp.h
> +++ b/include/uapi/linux/udp.h
> @@ -32,6 +32,7 @@ struct udphdr {
> #define UDP_ENCAP 100 /* Set the socket to accept encapsulated packets */
> #define UDP_NO_CHECK6_TX 101 /* Disable sending checksum for UDP6X */
> #define UDP_NO_CHECK6_RX 102 /* Disable accpeting checksum for UDP6 */
> +#define UDP_SEGMENT 103 /* Set GSO segmentation size */
>
> /* UDP encapsulation types */
> #define UDP_ENCAP_ESPINUDP_NON_IKE 1 /* draft-ietf-ipsec-nat-t-ike-00/01 */
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index ff49e352deea..c647cfe114e0 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -4940,6 +4940,8 @@ static unsigned int skb_gso_transport_seglen(const struct sk_buff *skb)
> thlen = tcp_hdrlen(skb);
> } else if (unlikely(skb_is_gso_sctp(skb))) {
> thlen = sizeof(struct sctphdr);
> + } else if (shinfo->gso_type & SKB_GSO_UDP_L4) {
> + thlen = sizeof(struct udphdr);
> }
> /* UFO sets gso_size to the size of the fragmentation
> * payload, i.e. the size of the L4 (UDP) header is already
It might make more sense to look at converting this over to a switch
statement based off of shinfo(skb)->gso_type & GSO_TRANSPORT_MASK,
where the transport mask consists of the 4 bits that are supported.
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index 2883ff1e909c..da4abbee10f7 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -882,7 +882,8 @@ static int __ip_append_data(struct sock *sk,
> skb = skb_peek_tail(queue);
>
> exthdrlen = !skb ? rt->dst.header_len : 0;
> - mtu = cork->fragsize;
> + mtu = cork->gso_size ? IP_MAX_MTU : cork->fragsize;
> +
> if (cork->tx_flags & SKBTX_ANY_SW_TSTAMP &&
> sk->sk_tsflags & SOF_TIMESTAMPING_OPT_ID)
> tskey = sk->sk_tskey++;
> @@ -906,7 +907,7 @@ static int __ip_append_data(struct sock *sk,
> if (transhdrlen &&
> length + fragheaderlen <= mtu &&
> rt->dst.dev->features & (NETIF_F_HW_CSUM | NETIF_F_IP_CSUM) &&
> - !(flags & MSG_MORE) &&
> + (!(flags & MSG_MORE) || cork->gso_size) &&
> !exthdrlen)
> csummode = CHECKSUM_PARTIAL;
>
> @@ -1135,6 +1136,8 @@ static int ip_setup_cork(struct sock *sk, struct inet_cork *cork,
> *rtp = NULL;
> cork->fragsize = ip_sk_use_pmtu(sk) ?
> dst_mtu(&rt->dst) : rt->dst.dev->mtu;
> +
> + cork->gso_size = sk->sk_type == SOCK_DGRAM ? ipc->gso_size : 0;
> cork->dst = &rt->dst;
> cork->length = 0;
> cork->ttl = ipc->ttl;
> @@ -1214,7 +1217,7 @@ ssize_t ip_append_page(struct sock *sk, struct flowi4 *fl4, struct page *page,
> return -EOPNOTSUPP;
>
> hh_len = LL_RESERVED_SPACE(rt->dst.dev);
> - mtu = cork->fragsize;
> + mtu = cork->gso_size ? IP_MAX_MTU : cork->fragsize;
>
> fragheaderlen = sizeof(struct iphdr) + (opt ? opt->optlen : 0);
> maxfraglen = ((mtu - fragheaderlen) & ~7) + fragheaderlen;
> diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> index 6b9d8017b319..bda022c5480b 100644
> --- a/net/ipv4/udp.c
> +++ b/net/ipv4/udp.c
> @@ -757,7 +757,8 @@ void udp_set_csum(bool nocheck, struct sk_buff *skb,
> }
> EXPORT_SYMBOL(udp_set_csum);
>
> -static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4)
> +static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4,
> + struct inet_cork *cork)
> {
> struct sock *sk = skb->sk;
> struct inet_sock *inet = inet_sk(sk);
> @@ -777,6 +778,21 @@ static int udp_send_skb(struct sk_buff *skb, struct flowi4 *fl4)
> uh->len = htons(len);
> uh->check = 0;
>
> + if (cork->gso_size) {
> + const int hlen = skb_network_header_len(skb) +
> + sizeof(struct udphdr);
> +
> + if (hlen + cork->gso_size > cork->fragsize)
> + return -EINVAL;
> + if (skb->len > cork->gso_size * UDP_MAX_SEGMENTS)
> + return -EINVAL;
> + if (skb->ip_summed != CHECKSUM_PARTIAL || is_udplite)
> + return -EIO;
> +
> + skb_shinfo(skb)->gso_size = cork->gso_size;
> + skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
> + }
> +
> if (is_udplite) /* UDP-Lite */
> csum = udplite_csum(skb);
>
> @@ -828,7 +844,7 @@ int udp_push_pending_frames(struct sock *sk)
> if (!skb)
> goto out;
>
> - err = udp_send_skb(skb, fl4);
> + err = udp_send_skb(skb, fl4, &inet->cork.base);
>
> out:
> up->len = 0;
> @@ -922,6 +938,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> ipc.sockc.tsflags = sk->sk_tsflags;
> ipc.addr = inet->inet_saddr;
> ipc.oif = sk->sk_bound_dev_if;
> + ipc.gso_size = up->gso_size;
>
> if (msg->msg_controllen) {
> err = ip_cmsg_send(sk, msg, &ipc, sk->sk_family == AF_INET6);
> @@ -1037,7 +1054,7 @@ int udp_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> &cork, msg->msg_flags);
> err = PTR_ERR(skb);
> if (!IS_ERR_OR_NULL(skb))
> - err = udp_send_skb(skb, fl4);
> + err = udp_send_skb(skb, fl4, &cork);
> goto out;
> }
>
> @@ -2367,6 +2384,12 @@ int udp_lib_setsockopt(struct sock *sk, int level, int optname,
> up->no_check6_rx = valbool;
> break;
>
> + case UDP_SEGMENT:
> + if (val < 0 || val > USHRT_MAX)
> + return -EINVAL;
> + up->gso_size = val;
> + break;
> +
> /*
> * UDP-Lite's partial checksum coverage (RFC 3828).
> */
> @@ -2457,6 +2480,10 @@ int udp_lib_getsockopt(struct sock *sk, int level, int optname,
> val = up->no_check6_rx;
> break;
>
> + case UDP_SEGMENT:
> + val = up->gso_size;
> + break;
> +
> /* The following two cannot be changed on UDP sockets, the return is
> * always 0 (which corresponds to the full checksum coverage of UDP). */
> case UDPLITE_SEND_CSCOV:
> diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> index ea6e6e7df0ee..a3244768e45f 100644
> --- a/net/ipv4/udp_offload.c
> +++ b/net/ipv4/udp_offload.c
> @@ -187,6 +187,53 @@ struct sk_buff *skb_udp_tunnel_segment(struct sk_buff *skb,
> }
> EXPORT_SYMBOL(skb_udp_tunnel_segment);
>
> +struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb,
> + netdev_features_t features,
> + unsigned int mss, __sum16 check)
> +{
> + struct udphdr *uh = udp_hdr(gso_skb);
> + struct sk_buff *segs;
> + unsigned int hdrlen;
> +
> + if (gso_skb->len <= sizeof(*uh) + mss)
> + return ERR_PTR(-EINVAL);
> +
> + uh->len = htons(sizeof(*uh) + mss);
> + uh->check = check;
> + skb_pull(gso_skb, sizeof(*uh));
> + hdrlen = gso_skb->data - skb_mac_header(gso_skb);
Normally I don't believe we modify the headers before calling
skb_segment. Normally that happens after via a while (skb->next) {}
type loop.
That way for things like GSO_PARTIAL we can update after segmentation
since there are only going to be 2 segments most likely instead of
multiple MSS sized segments.
> +
> + segs = skb_segment(gso_skb, features);
> + if (unlikely(IS_ERR_OR_NULL(segs)))
> + return segs;
> +
> + /* If last packet is not full, fix up its header */
> + if (segs->prev->len != hdrlen + mss) {
> + unsigned int mss_last = segs->prev->len - hdrlen;
> +
> + uh = udp_hdr(segs->prev);
> + uh->len = htons(sizeof(*uh) + mss_last);
> + csum_replace2(&uh->check, htons(mss), htons(mss_last));
> + }
> +
You could probably just assume that this last segment is always going
to need to be updated regardless. If you are doing any sort of
segmentation the last segment will always need the length and checksum
updated anyway, and since you probably need to move over to the "while
(skb->next)" style loop then you could just assume the last segment
needs updating.
> + return segs;
> +}
> +
> +static struct sk_buff *__udp4_gso_segment(struct sk_buff *gso_skb,
> + netdev_features_t features)
> +{
> + const struct iphdr *iph = ip_hdr(gso_skb);
> + unsigned int mss = skb_shinfo(gso_skb)->gso_size;
> +
> + if (!can_checksum_protocol(features, htons(ETH_P_IP)))
> + return ERR_PTR(-EIO);
> +
> + return __udp_gso_segment(gso_skb, features, mss,
> + udp_v4_check(sizeof(struct udphdr) + mss,
> + iph->saddr, iph->daddr, 0));
> +}
> +EXPORT_SYMBOL_GPL(__udp4_gso_segment);
> +
> static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
> netdev_features_t features)
> {
> @@ -203,12 +250,15 @@ static struct sk_buff *udp4_ufo_fragment(struct sk_buff *skb,
> goto out;
> }
>
> - if (!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP))
> + if (!(skb_shinfo(skb)->gso_type & (SKB_GSO_UDP | SKB_GSO_UDP_L4)))
> goto out;
>
> if (!pskb_may_pull(skb, sizeof(struct udphdr)))
> goto out;
>
> + if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)
> + return __udp4_gso_segment(skb, features);
> +
> mss = skb_shinfo(skb)->gso_size;
> if (unlikely(skb->len <= mss))
> goto out;
> diff --git a/net/ipv6/ip6_offload.c b/net/ipv6/ip6_offload.c
> index 4a87f9428ca5..5b3f2f89ef41 100644
> --- a/net/ipv6/ip6_offload.c
> +++ b/net/ipv6/ip6_offload.c
> @@ -88,9 +88,11 @@ static struct sk_buff *ipv6_gso_segment(struct sk_buff *skb,
>
> if (skb->encapsulation &&
> skb_shinfo(skb)->gso_type & (SKB_GSO_IPXIP4 | SKB_GSO_IPXIP6))
> - udpfrag = proto == IPPROTO_UDP && encap;
> + udpfrag = proto == IPPROTO_UDP && encap &&
> + (skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
> else
> - udpfrag = proto == IPPROTO_UDP && !skb->encapsulation;
> + udpfrag = proto == IPPROTO_UDP && !skb->encapsulation &&
> + (skb_shinfo(skb)->gso_type & SKB_GSO_UDP);
>
> ops = rcu_dereference(inet6_offloads[proto]);
> if (likely(ops && ops->callbacks.gso_segment)) {
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index 7fa1db447405..a1c4a78132d2 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -1240,6 +1240,8 @@ static int ip6_setup_cork(struct sock *sk, struct inet_cork_full *cork,
> if (mtu < IPV6_MIN_MTU)
> return -EINVAL;
> cork->base.fragsize = mtu;
> + cork->base.gso_size = sk->sk_type == SOCK_DGRAM ? ipc6->gso_size : 0;
> +
> if (dst_allfrag(xfrm_dst_path(&rt->dst)))
> cork->base.flags |= IPCORK_ALLFRAG;
> cork->base.length = 0;
> @@ -1281,7 +1283,7 @@ static int __ip6_append_data(struct sock *sk,
> dst_exthdrlen = rt->dst.header_len - rt->rt6i_nfheader_len;
> }
>
> - mtu = cork->fragsize;
> + mtu = cork->gso_size ? IP6_MAX_MTU : cork->fragsize;
> orig_mtu = mtu;
>
> hh_len = LL_RESERVED_SPACE(rt->dst.dev);
> @@ -1329,7 +1331,7 @@ static int __ip6_append_data(struct sock *sk,
> if (transhdrlen && sk->sk_protocol == IPPROTO_UDP &&
> headersize == sizeof(struct ipv6hdr) &&
> length <= mtu - headersize &&
> - !(flags & MSG_MORE) &&
> + (!(flags & MSG_MORE) || cork->gso_size) &&
> rt->dst.dev->features & (NETIF_F_IPV6_CSUM | NETIF_F_HW_CSUM))
> csummode = CHECKSUM_PARTIAL;
>
> diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
> index 824797f8d1ab..86b7dd58d4b4 100644
> --- a/net/ipv6/udp.c
> +++ b/net/ipv6/udp.c
> @@ -1023,7 +1023,8 @@ static void udp6_hwcsum_outgoing(struct sock *sk, struct sk_buff *skb,
> * Sending
> */
>
> -static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6)
> +static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6,
> + struct inet_cork *cork)
> {
> struct sock *sk = skb->sk;
> struct udphdr *uh;
> @@ -1042,6 +1043,21 @@ static int udp_v6_send_skb(struct sk_buff *skb, struct flowi6 *fl6)
> uh->len = htons(len);
> uh->check = 0;
>
> + if (cork->gso_size) {
> + const int hlen = skb_network_header_len(skb) +
> + sizeof(struct udphdr);
> +
> + if (hlen + cork->gso_size > cork->fragsize)
> + return -EINVAL;
> + if (skb->len > cork->gso_size * UDP_MAX_SEGMENTS)
> + return -EINVAL;
> + if (skb->ip_summed != CHECKSUM_PARTIAL || is_udplite)
> + return -EIO;
> +
> + skb_shinfo(skb)->gso_size = cork->gso_size;
> + skb_shinfo(skb)->gso_type = SKB_GSO_UDP_L4;
> + }
> +
> if (is_udplite)
> csum = udplite_csum(skb);
> else if (udp_sk(sk)->no_check6_tx) { /* UDP csum disabled */
> @@ -1093,7 +1109,7 @@ static int udp_v6_push_pending_frames(struct sock *sk)
> if (!skb)
> goto out;
>
> - err = udp_v6_send_skb(skb, &fl6);
> + err = udp_v6_send_skb(skb, &fl6, &inet_sk(sk)->cork.base);
>
> out:
> up->len = 0;
> @@ -1127,6 +1143,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> ipc6.hlimit = -1;
> ipc6.tclass = -1;
> ipc6.dontfrag = -1;
> + ipc6.gso_size = up->gso_size;
> sockc.tsflags = sk->sk_tsflags;
>
> /* destination address check */
> @@ -1333,7 +1350,7 @@ int udpv6_sendmsg(struct sock *sk, struct msghdr *msg, size_t len)
> msg->msg_flags, &cork, &sockc);
> err = PTR_ERR(skb);
> if (!IS_ERR_OR_NULL(skb))
> - err = udp_v6_send_skb(skb, &fl6);
> + err = udp_v6_send_skb(skb, &fl6, &cork.base);
> goto out;
> }
>
> diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
> index 2a04dc9c781b..f7b85b1e6b3e 100644
> --- a/net/ipv6/udp_offload.c
> +++ b/net/ipv6/udp_offload.c
> @@ -17,6 +17,20 @@
> #include <net/ip6_checksum.h>
> #include "ip6_offload.h"
>
> +static struct sk_buff *__udp6_gso_segment(struct sk_buff *gso_skb,
> + netdev_features_t features)
> +{
> + const struct ipv6hdr *ip6h = ipv6_hdr(gso_skb);
> + unsigned int mss = skb_shinfo(gso_skb)->gso_size;
> +
> + if (!can_checksum_protocol(features, htons(ETH_P_IPV6)))
> + return ERR_PTR(-EIO);
> +
> + return __udp_gso_segment(gso_skb, features, mss,
> + udp_v6_check(sizeof(struct udphdr) + mss,
> + &ip6h->saddr, &ip6h->daddr, 0));
> +}
> +
> static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
> netdev_features_t features)
> {
> @@ -42,12 +56,15 @@ static struct sk_buff *udp6_ufo_fragment(struct sk_buff *skb,
> const struct ipv6hdr *ipv6h;
> struct udphdr *uh;
>
> - if (!(skb_shinfo(skb)->gso_type & SKB_GSO_UDP))
> + if (!(skb_shinfo(skb)->gso_type & (SKB_GSO_UDP | SKB_GSO_UDP_L4)))
> goto out;
>
> if (!pskb_may_pull(skb, sizeof(struct udphdr)))
> goto out;
>
> + if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_L4)
> + return __udp6_gso_segment(skb, features);
> +
> /* Do software UFO. Complete and fill in the UDP checksum as HW cannot
> * do checksum of UDP packets sent as multiple IP fragments.
> */
> --
> 2.17.0.441.gb46fe60e1d-goog
>
^ permalink raw reply
* Re: [PATCH RFC 6/9] veth: Add ndo_xdp_xmit
From: Jesper Dangaard Brouer @ 2018-04-25 20:24 UTC (permalink / raw)
To: Toshiaki Makita; +Cc: brouer, netdev, Toshiaki Makita
In-Reply-To: <20180424143923.26519-7-toshiaki.makita1@gmail.com>
On Tue, 24 Apr 2018 23:39:20 +0900
Toshiaki Makita <toshiaki.makita1@gmail.com> wrote:
> +static int veth_xdp_xmit(struct net_device *dev, struct xdp_frame *frame)
> +{
> + struct veth_priv *rcv_priv, *priv = netdev_priv(dev);
> + int headroom = frame->data - (void *)frame;
> + struct net_device *rcv;
> + int err = 0;
> +
> + rcv = rcu_dereference(priv->peer);
> + if (unlikely(!rcv))
> + return -ENXIO;
> +
> + rcv_priv = netdev_priv(rcv);
> + /* xdp_ring is initialized on receive side? */
> + if (rcu_access_pointer(rcv_priv->xdp_prog)) {
> + err = xdp_ok_fwd_dev(rcv, frame->len);
> + if (unlikely(err))
> + return err;
> +
> + err = veth_xdp_enqueue(rcv_priv, veth_xdp_to_ptr(frame));
> + } else {
> + struct sk_buff *skb;
> +
> + skb = veth_build_skb(frame, headroom, frame->len, 0);
> + if (unlikely(!skb))
> + return -ENOMEM;
> +
> + /* Get page ref in case skb is dropped in netif_rx.
> + * The caller is responsible for freeing the page on error.
> + */
> + get_page(virt_to_page(frame->data));
I'm not sure you can make this assumption, that xdp_frames coming from
another device driver uses a refcnt based memory model. But maybe I'm
confused, as this looks like an SKB receive path, but in the
ndo_xdp_xmit().
> + if (unlikely(veth_forward_skb(rcv, skb, false) != NET_RX_SUCCESS))
> + return -ENXIO;
> +
> + /* Put page ref on success */
> + page_frag_free(frame->data);
> + }
> +
> + return err;
> +}
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [PATCH v4] fault-injection: introduce kvmalloc fallback options
From: Randy Dunlap @ 2018-04-25 20:20 UTC (permalink / raw)
To: Mikulas Patocka, Michal Hocko
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, Matthew Wilcox,
virtualization, linux-mm, edumazet, Andrew Morton, David Miller,
Vlastimil Babka
In-Reply-To: <alpine.LRH.2.02.1804251556060.30569@file01.intranet.prod.int.rdu2.redhat.com>
On 04/25/2018 01:02 PM, Mikulas Patocka wrote:
>
>
> From: Mikulas Patocka <mpatocka@redhat.com>
> Subject: [PATCH v4] fault-injection: introduce kvmalloc fallback options
>
> This patch introduces a fault-injection option "kvmalloc_fallback". This
> option makes kvmalloc randomly fall back to vmalloc.
>
> Unfortunatelly, some kernel code has bugs - it uses kvmalloc and then
Unfortunately,
> uses DMA-API on the returned memory or frees it with kfree. Such bugs were
> found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific
> code. This options helps to test for these bugs.
>
> The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY.
> It can be enabled in distribution debug kernels, so that kvmalloc abuse
> can be tested by the users. The default can be overriden with
overridden
> "kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/.
>
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
>
> ---
> Documentation/fault-injection/fault-injection.txt | 7 +++++
> include/linux/fault-inject.h | 9 +++---
> kernel/futex.c | 2 -
> lib/Kconfig.debug | 15 +++++++++++
> mm/failslab.c | 2 -
> mm/page_alloc.c | 2 -
> mm/util.c | 30 ++++++++++++++++++++++
> 7 files changed, 60 insertions(+), 7 deletions(-)
>
> Index: linux-2.6/Documentation/fault-injection/fault-injection.txt
> ===================================================================
> --- linux-2.6.orig/Documentation/fault-injection/fault-injection.txt 2018-04-16 21:08:34.000000000 +0200
> +++ linux-2.6/Documentation/fault-injection/fault-injection.txt 2018-04-25 21:36:36.000000000 +0200
> @@ -15,6 +15,12 @@ o fail_page_alloc
>
> injects page allocation failures. (alloc_pages(), get_free_pages(), ...)
>
> +o kvmalloc_faillback
kvmalloc_fallback
> +
> + makes the function kvmalloc randonly fall back to vmalloc. This could be used
randomly
> + to detects bugs such as using DMA-API on the result of kvmalloc or freeing
> + the result of kvmalloc with free.
> +
> o fail_futex
>
> injects futex deadlock and uaddr fault errors.
> @@ -167,6 +173,7 @@ use the boot option:
>
> failslab=
> fail_page_alloc=
> + kvmalloc_faillback=
kvmalloc_fallback=
> fail_make_request=
> fail_futex=
> mmc_core.fail_request=<interval>,<probability>,<space>,<times>
> Index: linux-2.6/lib/Kconfig.debug
> ===================================================================
> --- linux-2.6.orig/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200
> +++ linux-2.6/lib/Kconfig.debug 2018-04-25 21:39:45.000000000 +0200
> @@ -1527,6 +1527,21 @@ config FAIL_PAGE_ALLOC
> help
> Provide fault-injection capability for alloc_pages().
>
> +config FAIL_KVMALLOC_FALLBACK_PROBABILITY
> + int "Default kvmalloc fallback probability"
> + depends on FAULT_INJECTION
> + range 0 100
> + default "0"
> + help
> + This option will make kvmalloc randomly fall back to vmalloc.
> + Normally, kvmalloc falls back to vmalloc only rarely, if memory
> + is fragmented.
> +
> + This option helps to detect hard-to-reproduce driver bugs, for
> + example using DMA API on the result of kvmalloc.
> +
> + The default may be overriden with the kvmalloc_faillback parameter.
overridden kvmalloc_fallback
> +
> config FAIL_MAKE_REQUEST
> bool "Fault-injection capability for disk IO"
> depends on FAULT_INJECTION && BLOCK
--
~Randy
^ permalink raw reply
* [PATCH v4] fault-injection: introduce kvmalloc fallback options
From: Mikulas Patocka @ 2018-04-25 20:02 UTC (permalink / raw)
To: Michal Hocko
Cc: Matthew Wilcox, David Miller, Andrew Morton, linux-mm,
eric.dumazet, edumazet, netdev, linux-kernel, mst, jasowang,
virtualization, dm-devel, Vlastimil Babka
In-Reply-To: <20180424173836.GR17484@dhcp22.suse.cz>
On Tue, 24 Apr 2018, Michal Hocko wrote:
> > > Wouldn't it be equally trivial to simply enable the fault injection? You
> > > would get additional failure paths testing as a bonus.
> >
> > The RHEL and Fedora debugging kernels are compiled with fault injection.
> > But the fault-injection framework will do nothing unless it is enabled by
> > a kernel parameter or debugfs write.
> >
> > Most users don't know about the fault injection kernel parameters or
> > debugfs files and won't enabled it. We need a CONFIG_ option to enable it
> > by default in the debugging kernels (and we could add a kernel parameter
> > to override the default, fine-tune the fallback probability etc.)
>
> If it is a real issue to install the debugging kernel with the required
> kernel parameter then I a config option for the default on makes sense
> to me.
Yes - the debug kernels use the same default kernel parameters as
non-debug kernels and it is expected that all debug features are enabled
by default.
Here I'm sending the patch using the fault-injection framework and the new
option CONFIG_FAIL_KVMALLOC_FALLBACK_PROBABILITY.
Mikulas
From: Mikulas Patocka <mpatocka@redhat.com>
Subject: [PATCH v4] fault-injection: introduce kvmalloc fallback options
This patch introduces a fault-injection option "kvmalloc_fallback". This
option makes kvmalloc randomly fall back to vmalloc.
Unfortunatelly, some kernel code has bugs - it uses kvmalloc and then
uses DMA-API on the returned memory or frees it with kfree. Such bugs were
found in the virtio-net driver, dm-integrity or RHEL7 powerpc-specific
code. This options helps to test for these bugs.
The patch introduces a config option FAIL_KVMALLOC_FALLBACK_PROBABILITY.
It can be enabled in distribution debug kernels, so that kvmalloc abuse
can be tested by the users. The default can be overriden with
"kvmalloc_fallback" parameter or in /sys/kernel/debug/kvmalloc_fallback/.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
Documentation/fault-injection/fault-injection.txt | 7 +++++
include/linux/fault-inject.h | 9 +++---
kernel/futex.c | 2 -
lib/Kconfig.debug | 15 +++++++++++
mm/failslab.c | 2 -
mm/page_alloc.c | 2 -
mm/util.c | 30 ++++++++++++++++++++++
7 files changed, 60 insertions(+), 7 deletions(-)
Index: linux-2.6/Documentation/fault-injection/fault-injection.txt
===================================================================
--- linux-2.6.orig/Documentation/fault-injection/fault-injection.txt 2018-04-16 21:08:34.000000000 +0200
+++ linux-2.6/Documentation/fault-injection/fault-injection.txt 2018-04-25 21:36:36.000000000 +0200
@@ -15,6 +15,12 @@ o fail_page_alloc
injects page allocation failures. (alloc_pages(), get_free_pages(), ...)
+o kvmalloc_faillback
+
+ makes the function kvmalloc randonly fall back to vmalloc. This could be used
+ to detects bugs such as using DMA-API on the result of kvmalloc or freeing
+ the result of kvmalloc with free.
+
o fail_futex
injects futex deadlock and uaddr fault errors.
@@ -167,6 +173,7 @@ use the boot option:
failslab=
fail_page_alloc=
+ kvmalloc_faillback=
fail_make_request=
fail_futex=
mmc_core.fail_request=<interval>,<probability>,<space>,<times>
Index: linux-2.6/include/linux/fault-inject.h
===================================================================
--- linux-2.6.orig/include/linux/fault-inject.h 2018-04-16 21:08:36.000000000 +0200
+++ linux-2.6/include/linux/fault-inject.h 2018-04-25 21:38:22.000000000 +0200
@@ -31,17 +31,18 @@ struct fault_attr {
struct dentry *dname;
};
-#define FAULT_ATTR_INITIALIZER { \
+#define FAULT_ATTR_INITIALIZER(p) { \
+ .probability = (p), \
.interval = 1, \
- .times = ATOMIC_INIT(1), \
+ .times = ATOMIC_INIT((p) ? -1 : 1), \
+ .verbose = (p) ? 0 : 2, \
.require_end = ULONG_MAX, \
.stacktrace_depth = 32, \
.ratelimit_state = RATELIMIT_STATE_INIT_DISABLED, \
- .verbose = 2, \
.dname = NULL, \
}
-#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER
+#define DECLARE_FAULT_ATTR(name) struct fault_attr name = FAULT_ATTR_INITIALIZER(0)
int setup_fault_attr(struct fault_attr *attr, char *str);
bool should_fail(struct fault_attr *attr, ssize_t size);
Index: linux-2.6/lib/Kconfig.debug
===================================================================
--- linux-2.6.orig/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200
+++ linux-2.6/lib/Kconfig.debug 2018-04-25 21:39:45.000000000 +0200
@@ -1527,6 +1527,21 @@ config FAIL_PAGE_ALLOC
help
Provide fault-injection capability for alloc_pages().
+config FAIL_KVMALLOC_FALLBACK_PROBABILITY
+ int "Default kvmalloc fallback probability"
+ depends on FAULT_INJECTION
+ range 0 100
+ default "0"
+ help
+ This option will make kvmalloc randomly fall back to vmalloc.
+ Normally, kvmalloc falls back to vmalloc only rarely, if memory
+ is fragmented.
+
+ This option helps to detect hard-to-reproduce driver bugs, for
+ example using DMA API on the result of kvmalloc.
+
+ The default may be overriden with the kvmalloc_faillback parameter.
+
config FAIL_MAKE_REQUEST
bool "Fault-injection capability for disk IO"
depends on FAULT_INJECTION && BLOCK
Index: linux-2.6/mm/util.c
===================================================================
--- linux-2.6.orig/mm/util.c 2018-04-25 15:48:39.000000000 +0200
+++ linux-2.6/mm/util.c 2018-04-25 21:43:31.000000000 +0200
@@ -14,6 +14,7 @@
#include <linux/hugetlb.h>
#include <linux/vmalloc.h>
#include <linux/userfaultfd_k.h>
+#include <linux/fault-inject.h>
#include <asm/sections.h>
#include <linux/uaccess.h>
@@ -377,6 +378,29 @@ unsigned long vm_mmap(struct file *file,
}
EXPORT_SYMBOL(vm_mmap);
+#ifdef CONFIG_FAULT_INJECTION
+
+static struct fault_attr kvmalloc_fallback =
+ FAULT_ATTR_INITIALIZER(CONFIG_FAIL_KVMALLOC_FALLBACK_PROBABILITY);
+
+static int __init setup_kvmalloc_fallback(char *str)
+{
+ return setup_fault_attr(&kvmalloc_fallback, str);
+}
+
+__setup("kvmalloc_fallback=", setup_kvmalloc_fallback);
+
+#ifdef CONFIG_FAULT_INJECTION_DEBUG_FS
+static int __init kvmalloc_fallback_debugfs_init(void)
+{
+ fault_create_debugfs_attr("kvmalloc_fallback", NULL, &kvmalloc_fallback);
+ return 0;
+}
+late_initcall(kvmalloc_fallback_debugfs_init);
+#endif
+
+#endif
+
/**
* kvmalloc_node - attempt to allocate physically contiguous memory, but upon
* failure, fall back to non-contiguous (vmalloc) allocation.
@@ -404,6 +428,11 @@ void *kvmalloc_node(size_t size, gfp_t f
*/
WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL);
+#ifdef CONFIG_FAULT_INJECTION
+ if (should_fail(&kvmalloc_fallback, size))
+ goto do_vmalloc;
+#endif
+
/*
* We want to attempt a large physically contiguous block first because
* it is less likely to fragment multiple larger blocks and therefore
@@ -427,6 +456,7 @@ void *kvmalloc_node(size_t size, gfp_t f
if (ret || size <= PAGE_SIZE)
return ret;
+do_vmalloc: __maybe_unused
return __vmalloc_node_flags_caller(size, node, flags,
__builtin_return_address(0));
}
Index: linux-2.6/kernel/futex.c
===================================================================
--- linux-2.6.orig/kernel/futex.c 2018-02-14 20:24:42.000000000 +0100
+++ linux-2.6/kernel/futex.c 2018-04-25 21:11:33.000000000 +0200
@@ -288,7 +288,7 @@ static struct {
bool ignore_private;
} fail_futex = {
- .attr = FAULT_ATTR_INITIALIZER,
+ .attr = FAULT_ATTR_INITIALIZER(0),
.ignore_private = false,
};
Index: linux-2.6/mm/failslab.c
===================================================================
--- linux-2.6.orig/mm/failslab.c 2018-04-16 21:08:36.000000000 +0200
+++ linux-2.6/mm/failslab.c 2018-04-25 21:11:40.000000000 +0200
@@ -9,7 +9,7 @@ static struct {
bool ignore_gfp_reclaim;
bool cache_filter;
} failslab = {
- .attr = FAULT_ATTR_INITIALIZER,
+ .attr = FAULT_ATTR_INITIALIZER(0),
.ignore_gfp_reclaim = true,
.cache_filter = false,
};
Index: linux-2.6/mm/page_alloc.c
===================================================================
--- linux-2.6.orig/mm/page_alloc.c 2018-04-16 21:08:36.000000000 +0200
+++ linux-2.6/mm/page_alloc.c 2018-04-25 21:11:47.000000000 +0200
@@ -3055,7 +3055,7 @@ static struct {
bool ignore_gfp_reclaim;
u32 min_order;
} fail_page_alloc = {
- .attr = FAULT_ATTR_INITIALIZER,
+ .attr = FAULT_ATTR_INITIALIZER(0),
.ignore_gfp_reclaim = true,
.ignore_gfp_highmem = true,
.min_order = 1,
^ permalink raw reply
* [PATCH] fault-injection: reorder config entries
From: Mikulas Patocka @ 2018-04-25 20:02 UTC (permalink / raw)
To: Michal Hocko
Cc: dm-devel, eric.dumazet, mst, netdev, linux-kernel, Matthew Wilcox,
virtualization, linux-mm, edumazet, Andrew Morton, David Miller,
Vlastimil Babka
In-Reply-To: <20180424173836.GR17484@dhcp22.suse.cz>
This patch reorders Kconfig entries, so that menuconfig displays proper
indentation.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
lib/Kconfig.debug | 36 ++++++++++++++++++------------------
1 file changed, 18 insertions(+), 18 deletions(-)
Index: linux-2.6/lib/Kconfig.debug
===================================================================
--- linux-2.6.orig/lib/Kconfig.debug 2018-04-16 21:08:36.000000000 +0200
+++ linux-2.6/lib/Kconfig.debug 2018-04-25 15:56:16.000000000 +0200
@@ -1503,6 +1503,10 @@ config NETDEV_NOTIFIER_ERROR_INJECT
If unsure, say N.
+config FUNCTION_ERROR_INJECTION
+ def_bool y
+ depends on HAVE_FUNCTION_ERROR_INJECTION && KPROBES
+
config FAULT_INJECTION
bool "Fault-injection framework"
depends on DEBUG_KERNEL
@@ -1510,10 +1514,6 @@ config FAULT_INJECTION
Provide fault-injection framework.
For more details, see Documentation/fault-injection/.
-config FUNCTION_ERROR_INJECTION
- def_bool y
- depends on HAVE_FUNCTION_ERROR_INJECTION && KPROBES
-
config FAILSLAB
bool "Fault-injection capability for kmalloc"
depends on FAULT_INJECTION
@@ -1544,16 +1544,6 @@ config FAIL_IO_TIMEOUT
Only works with drivers that use the generic timeout handling,
for others it wont do anything.
-config FAIL_MMC_REQUEST
- bool "Fault-injection capability for MMC IO"
- depends on FAULT_INJECTION_DEBUG_FS && MMC
- help
- Provide fault-injection capability for MMC IO.
- This will make the mmc core return data errors. This is
- useful to test the error handling in the mmc block device
- and to test how the mmc host driver handles retries from
- the block device.
-
config FAIL_FUTEX
bool "Fault-injection capability for futexes"
select DEBUG_FS
@@ -1561,6 +1551,12 @@ config FAIL_FUTEX
help
Provide fault-injection capability for futexes.
+config FAULT_INJECTION_DEBUG_FS
+ bool "Debugfs entries for fault-injection capabilities"
+ depends on FAULT_INJECTION && SYSFS && DEBUG_FS
+ help
+ Enable configuration of fault-injection capabilities via debugfs.
+
config FAIL_FUNCTION
bool "Fault-injection capability for functions"
depends on FAULT_INJECTION_DEBUG_FS && FUNCTION_ERROR_INJECTION
@@ -1571,11 +1567,15 @@ config FAIL_FUNCTION
an error value and have to handle it. This is useful to test the
error handling in various subsystems.
-config FAULT_INJECTION_DEBUG_FS
- bool "Debugfs entries for fault-injection capabilities"
- depends on FAULT_INJECTION && SYSFS && DEBUG_FS
+config FAIL_MMC_REQUEST
+ bool "Fault-injection capability for MMC IO"
+ depends on FAULT_INJECTION_DEBUG_FS && MMC
help
- Enable configuration of fault-injection capabilities via debugfs.
+ Provide fault-injection capability for MMC IO.
+ This will make the mmc core return data errors. This is
+ useful to test the error handling in the mmc block device
+ and to test how the mmc host driver handles retries from
+ the block device.
config FAULT_INJECTION_STACKTRACE_FILTER
bool "stacktrace filter for fault-injection capabilities"
^ permalink raw reply
* Re: [PATCH bpf-next] bpf: clear the ip_tunnel_info.
From: William Tu @ 2018-04-25 20:01 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Linux Kernel Network Developers
In-Reply-To: <cf175e85-07f2-6c05-d9be-35b725bab9ef@iogearbox.net>
On Wed, Apr 25, 2018 at 12:54 AM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> On 04/25/2018 08:46 AM, William Tu wrote:
>> The percpu metadata_dst might carry the stale ip_tunnel_info
>> and cause incorrect behavior. When mixing tests using ipv4/ipv6
>> bpf vxlan and geneve tunnel, the ipv6 tunnel info incorrectly uses
>> ipv4's src ip addr as its ipv6 src address, because the previous
>> tunnel info does not clean up. The patch zeros the fields in
>> ip_tunnel_info.
>>
>> Signed-off-by: William Tu <u9012063@gmail.com>
>> Reported-by: Yifeng Sun <pkusunyifeng@gmail.com>
>
> Since this is a fix, I've applied this to bpf, thanks William!
Thanks.
Just to add some context about this issue.
This happens when doing in sequence
1) start ipv4 vxlan bpf tunnel
2) delete all related devices
3) start ipv6 vxlan bpf tunnel
The first ipv4 vxlan tunnel sets the ipv4 src ip in the ip_tunnel_key
and does not clear. So the 3) ipv6 vxlan bpf tunnel, uses the ipv4's
address as its ipv6 address. As a result, vxlan driver reports
[81227.576732] ip6vxlan00: add 7a:2c:d7:fe:a9:43 ->
0000:0000:ac10:0164:0000:0000:0000:0000
[81237.614330] ip6vxlan00: no route to 0000:0000:ac10:0164:0000:0000:0000:0000
where "ac10:0164" is 172.16.1.200.
Similar issue when testing ipv4 geneve followed by ipv6 geneve.
Regards,
William
^ permalink raw reply
* RE: e1000e I219 timestamping oops related to TSYNCRXCTL read
From: Keller, Jacob E @ 2018-04-25 19:59 UTC (permalink / raw)
To: Benjamin Poirier, Allan, Bruce W, Yanir Lubetkin, Neftin, Sasha
Cc: Alexander Duyck, Kirsher, Jeffrey T, Achim Mildenberger,
olouvignes@gmail.com, jayanth@goubiq.com, ehabkost@redhat.com,
postmodern.mod3@gmail.com, Bart.VanAssche@wdc.com,
intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org
In-Reply-To: <20180425065243.g5mqewg5irkwgwgv@f2>
Hi Benjamin,
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
> On Behalf Of Benjamin Poirier
> Sent: Tuesday, April 24, 2018 11:53 PM
> To: Allan, Bruce W <bruce.w.allan@intel.com>; Yanir Lubetkin
> <yanirx.lubetkin@intel.com>; Keller, Jacob E <jacob.e.keller@intel.com>; Neftin,
> Sasha <sasha.neftin@intel.com>
> Cc: Alexander Duyck <alexander.duyck@gmail.com>; Kirsher, Jeffrey T
> <jeffrey.t.kirsher@intel.com>; Achim Mildenberger <admin@fph.physik.uni-
> karlsruhe.de>; olouvignes@gmail.com; jayanth@goubiq.com;
> ehabkost@redhat.com; postmodern.mod3@gmail.com;
> Bart.VanAssche@wdc.com; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org
> Subject: e1000e I219 timestamping oops related to TSYNCRXCTL read
>
> In the following openSUSE bug report
> https://bugzilla.suse.com/show_bug.cgi?id=1075876
> Achim reported an oops related to e1000e timestamping:
> kernel: RIP: 0010:[<ffffffff8110303f>] timecounter_read+0xf/0x50
> [...]
> kernel: Call Trace:
> kernel: [<ffffffffa0806b0f>] e1000e_phc_gettime+0x2f/0x60 [e1000e]
> kernel: [<ffffffffa0806c5d>] e1000e_systim_overflow_work+0x1d/0x80 [e1000e]
> kernel: [<ffffffff810992c5>] process_one_work+0x155/0x440
> kernel: [<ffffffff81099e16>] worker_thread+0x116/0x4b0
> kernel: [<ffffffff8109f422>] kthread+0xd2/0xf0
> kernel: [<ffffffff8163184f>] ret_from_fork+0x3f/0x70
>
> It always occurs 4 hours after boot but not on every boot. It is most
> likely the same problem reported here:
> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1668356
> http://lkml.iu.edu/hypermail/linux/kernel/1506.2/index.html#02530
> https://bugzilla.redhat.com/show_bug.cgi?id=1463882
> https://bugzilla.redhat.com/show_bug.cgi?id=1431863
>
It probably occurs due to the systim overflow check, yes.
> This occurs with MAC: 12, e1000_pch_spt/I219. The reporter has
> reproduced it on a v4.16 derivative.
>
> We've traced it to the fact that e1000e_systim_reset() skips the
> timecounter_init() call if e1000e_get_base_timinca() returns -EINVAL,
> which leads to a null deref in timecounter_read() (see comment 8 of the
> suse bugzilla for more details.)
>
> In commit 83129b37ef35 ("e1000e: fix systim issues", v4.2-rc1) Yanir
> reworked e1000e_get_base_timinca() in such a way that it can return
> -EINVAL for e1000_pch_spt if the SYSCFI bit is not set in TSYNCRXCTL.
> This is also the commit that was identified by bisection in the second
> link above (lkml).
>
> What we've observed (in comment 14) is that TSYNCRXCTL reads sometimes
> don't have the SYSCFI bit set. Retrying the read shortly after finds the
> bit to be set. This was observed at boot (probe) but also link up and
> link down.
>
I don't know offhand what the SYSCFI bit is for yet still digging into it.
> I have a few questions:
>
> What's the purpose of the SYSCFI bit in TSYNCRXCTL ("Reserved" in the
> datasheet)?
>
> Why does it look like subsequent reads of TSYNCRXCTL sometimes have the
> SYSCFI bit set/not set on I219?
>
> Is it right to check the SYSCFI bit in e1000e_get_base_timinca() for
> _spt and return -EINVAL if it's not set? Could we just remove that
> check?
>
I think the right approach might be proper cleanup when we fail to reset. I think the problem is that when e1000e_systim_reset is called and fails, we don't properly cleanup the work items. I think we need to actually stop and kill the work task so that it won't run.
> The patch in comment 13 of the suse bugzilla works around the problem by
> retrying TSYNCRXCTL reads, maybe we could instead remove that read
> altogether or move the timecounter_init() call to at least avoid the
> oops. The best approach to take seems to depend on the behavior expected
> of TSYNCRXCTL reads on I219 so I'm hoping that you could provide more
> info on that.
>
Yea, we need to do something here, I'm still investigating why we need the SYSCFI check, but at a minimum, we should disable the overflow check task if we fail here, I think.
It looks like the SYSCFI is the System Clock Frequency Indicator bit, and it should be used to tell which of two clock frequencies to choose. I do not understand why that would be changing at different reads. We do need to make sure the clock is already enabled, but we do that prior to the switch case... Something is really weird here...
Thanks,
Jake
> Thanks,
> -Benjamin
^ permalink raw reply
* Re: [RFC bpf-next 8/9] bpf: Provide helper to do lookups in kernel FIB table
From: Daniel Borkmann @ 2018-04-25 19:55 UTC (permalink / raw)
To: David Ahern, netdev, borkmann, ast
Cc: shm, roopa, brouer, toke, john.fastabend
In-Reply-To: <20180425183449.25134-9-dsahern@gmail.com>
On 04/25/2018 08:34 PM, David Ahern wrote:
> Provide a helper for doing a FIB and neighbor lookup in the kernel
> tables from an XDP program. The helper provides a fastpath for forwarding
> packets. If the packet is a local delivery or for any reason is not a
> simple lookup and forward, the packet continues up the stack.
>
> If it is to be forwarded, the forwarding can be done directly if the
> neighbor is already known. If the neighbor does not exist, the first
> few packets go up the stack for neighbor resolution. Once resolved, the
> xdp program provides the fast path.
>
> On successful lookup the nexthop dmac, current device smac and egress
> device index are returned.
>
> The API supports IPv4, IPv6 and MPLS protocols, but only IPv4 and IPv6
> are implemented in this patch. The API includes layer 4 parameters if
> the XDP program chooses to do deep packet inspection to allow compare
> against ACLs implemented as FIB rules.
>
> Header rewrite is left to the XDP program.
>
> The lookup takes 2 flags:
> - BPF_FIB_LOOKUP_DIRECT to do a lookup that bypasses FIB rules and goes
> straight to the table associated with the device (expert setting for
> those looking to maximize throughput)
>
> - BPF_FIB_LOOKUP_OUTPUT to do a lookup from the egress perspective.
> Default is an ingress lookup.
>
> Initial performance numbers collected by Jesper, forwarded packets/sec:
>
> Full stack XDP FIB lookup XDP Direct lookup
> IPv4 1,947,969 7,074,156 7,415,333
> IPv6 1,728,000 6,165,504 7,262,720
>
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> include/uapi/linux/bpf.h | 68 +++++++++++++-
> net/core/filter.c | 233 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 300 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index e6679393b687..82601c132b9f 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -10,6 +10,8 @@
>
> #include <linux/types.h>
> #include <linux/bpf_common.h>
> +#include <linux/if_ether.h>
> +#include <linux/in6.h>
>
> /* Extended instruction set based on top of classic BPF */
>
> @@ -783,6 +785,17 @@ union bpf_attr {
> * @size: size of 'struct bpf_xfrm_state'
> * @flags: room for future extensions
> * Return: 0 on success or negative error
> + *
> + * int bpf_fib_lookup(ctx, params, plen, flags)
> + * Do a FIB lookup based on given parameters
> + * @ctx: pointer to context of type xdp_md
Nit: would just say pointer to context here since used with xdp/skb
> + * @params: pointer to bpf_fib_lookup
> + * @plen: size of params argument
> + * @flags: u32 bitmask of BPF_FIB_LOOKUP_* flags
> + * Return: egress device index if packet is to be forwarded,
> + * 0 for local delivery (anything that needs to be handled
> + * by the full stack), or negative on error.
> + * If index is > 0, output data in bpf_fib_lookup is set
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -851,7 +864,9 @@ union bpf_attr {
> FN(msg_pull_data), \
> FN(bind), \
> FN(xdp_adjust_tail), \
> - FN(skb_get_xfrm_state),
> + FN(skb_get_xfrm_state), \
> + FN(fib_lookup), \
> +
>
Nit: trailing '\' resp. double newline
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
[...]
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 8e45c6c7ab08..37602b2fb94a 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -59,6 +59,10 @@
> #include <net/tcp.h>
> #include <net/xfrm.h>
> #include <linux/bpf_trace.h>
> +#include <linux/inetdevice.h>
> +#include <net/ip_fib.h>
> +#include <net/flow.h>
> +#include <net/arp.h>
>
> /**
> * sk_filter_trim_cap - run a packet through a socket filter
> @@ -3787,6 +3791,231 @@ static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = {
> };
> #endif
>
> +#if IS_ENABLED(CONFIG_INET) || IS_ENABLED(CONFIG_IPV6)
> +static int bpf_fib_set_fwd_params(struct bpf_fib_lookup *params,
> + const struct neighbour *neigh,
> + const struct net_device *dev)
> +{
> + memcpy(params->dmac, neigh->ha, ETH_ALEN);
> + memcpy(params->smac, dev->dev_addr, ETH_ALEN);
> + params->h_vlan_TCI = 0;
> + params->h_vlan_proto = 0;
> +
> + return dev->ifindex;
> +}
> +#endif
> +
> +#if IS_ENABLED(CONFIG_INET)
> +static int bpf_ipv4_fib_lookup(struct xdp_buff *ctx,
Instead of passing xdp_buff here, just pass the netdev pointer. More below
why it's needed.
> + struct bpf_fib_lookup *params, u32 flags)
> +{
> + struct net *net = dev_net(ctx->rxq->dev);
> + struct in_device *in_dev;
> + struct neighbour *neigh;
> + struct net_device *dev;
> + struct fib_result res;
> + struct fib_nh *nh;
> + struct flowi4 fl4;
> + int err;
> +
> + dev = dev_get_by_index_rcu(net, params->ifindex);
> + if (unlikely(!dev))
> + return -ENODEV;
> +
> + /* verify forwarding is enabled on this interface */
> + in_dev = __in_dev_get_rcu(dev);
> + if (unlikely(!in_dev || !IN_DEV_FORWARD(in_dev)))
> + return 0;
> +
> + if (flags & BPF_FIB_LOOKUP_OUTPUT) {
> + fl4.flowi4_iif = 1;
> + fl4.flowi4_oif = params->ifindex;
> + } else {
> + fl4.flowi4_iif = params->ifindex;
> + fl4.flowi4_oif = 0;
> + }
> + fl4.flowi4_tos = params->tos & IPTOS_RT_MASK;
> + fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
> + fl4.flowi4_flags = 0;
> +
> + fl4.flowi4_proto = params->l4_protocol;
> + fl4.daddr = params->ipv4_dst;
> + fl4.saddr = params->ipv4_src;
> + fl4.fl4_sport = params->sport;
> + fl4.fl4_dport = params->dport;
> +
> + if (flags & BPF_FIB_LOOKUP_DIRECT) {
> + u32 tbid = l3mdev_fib_table_rcu(dev) ? : RT_TABLE_MAIN;
> + struct fib_table *tb;
> +
> + tb = fib_get_table(net, tbid);
> + if (unlikely(!tb))
> + return 0;
> +
> + err = fib_table_lookup(tb, &fl4, &res, FIB_LOOKUP_NOREF);
> + } else {
> + fl4.flowi4_mark = 0;
> + fl4.flowi4_secid = 0;
> + fl4.flowi4_tun_key.tun_id = 0;
> + fl4.flowi4_uid = sock_net_uid(net, NULL);
> +
> + err = fib_lookup(net, &fl4, &res, FIB_LOOKUP_NOREF);
> + }
> +
> + if (err || res.type != RTN_UNICAST)
> + return 0;
> +
> + if (res.fi->fib_nhs > 1)
> + fib_select_path(net, &res, &fl4, NULL);
> +
> + nh = &res.fi->fib_nh[res.nh_sel];
> +
> + /* do not handle lwt encaps right now */
> + if (nh->nh_lwtstate)
> + return 0;
> +
> + dev = nh->nh_dev;
> + if (unlikely(!dev))
> + return 0;
> +
> + if (nh->nh_gw)
> + params->ipv4_dst = nh->nh_gw;
> +
> + params->rt_metric = res.fi->fib_priority;
> +
> + /* xdp and cls_bpf programs are run in RCU-bh so
> + * rcu_read_lock_bh is not needed here
> + */
> + neigh = __ipv4_neigh_lookup_noref(dev, (__force u32)params->ipv4_dst);
> + if (neigh)
> + return bpf_fib_set_fwd_params(params, neigh, dev);
> +
> + return 0;
> +}
> +#endif
> +
> +#if IS_ENABLED(CONFIG_IPV6)
> +static int bpf_ipv6_fib_lookup(struct xdp_buff *ctx,
Same here.
> + struct bpf_fib_lookup *params, u32 flags)
> +{
> + struct net *net = dev_net(ctx->rxq->dev);
> + struct neighbour *neigh;
> + struct net_device *dev;
> + struct fib6_info *f6i;
> + struct flowi6 fl6;
> + int strict = 0;
> + int oif;
> +
> + /* link local addresses are never forwarded */
> + if (rt6_need_strict(¶ms->ipv6_dst) ||
> + rt6_need_strict(¶ms->ipv6_src))
> + return 0;
> +
> + dev = dev_get_by_index_rcu(net, params->ifindex);
> + if (unlikely(!dev))
> + return -ENODEV;
> +
> + if (flags & BPF_FIB_LOOKUP_OUTPUT) {
> + fl6.flowi6_iif = 1;
> + oif = fl6.flowi6_oif = params->ifindex;
> + } else {
> + oif = fl6.flowi6_iif = params->ifindex;
> + fl6.flowi6_oif = 0;
> + strict = RT6_LOOKUP_F_HAS_SADDR;
> + }
> + fl6.flowlabel = params->flowlabel;
> + fl6.flowi6_scope = 0;
> + fl6.flowi6_flags = 0;
> + fl6.mp_hash = 0;
> +
> + fl6.flowi6_proto = params->l4_protocol;
> + fl6.daddr = params->ipv6_dst;
> + fl6.saddr = params->ipv6_src;
> + fl6.fl6_sport = params->sport;
> + fl6.fl6_dport = params->dport;
> +
> + if (flags & BPF_FIB_LOOKUP_DIRECT) {
> + u32 tbid = l3mdev_fib_table_rcu(dev) ? : RT_TABLE_MAIN;
> + struct fib6_table *tb;
> +
> + tb = ipv6_stub->fib6_get_table(net, tbid);
> + if (unlikely(!tb))
> + return 0;
> +
> + f6i = ipv6_stub->fib6_table_lookup(net, tb, oif, &fl6, strict);
> + } else {
> + fl6.flowi6_mark = 0;
> + fl6.flowi6_secid = 0;
> + fl6.flowi6_tun_key.tun_id = 0;
> + fl6.flowi6_uid = sock_net_uid(net, NULL);
> +
> + f6i = ipv6_stub->fib6_lookup(net, oif, &fl6, strict);
> + }
> +
> + if (unlikely(IS_ERR_OR_NULL(f6i) || f6i == net->ipv6.fib6_null_entry))
> + return 0;
> +
> + if (unlikely(f6i->fib6_flags & RTF_REJECT ||
> + f6i->fib6_type != RTN_UNICAST))
> + return 0;
> +
> + if (f6i->fib6_nsiblings && fl6.flowi6_oif == 0)
> + f6i = ipv6_stub->fib6_multipath_select(net, f6i, &fl6,
> + fl6.flowi6_oif, NULL,
> + strict);
> +
> + if (f6i->fib6_nh.nh_lwtstate)
> + return 0;
> +
> + if (f6i->fib6_flags & RTF_GATEWAY)
> + params->ipv6_dst = f6i->fib6_nh.nh_gw;
> +
> + dev = f6i->fib6_nh.nh_dev;
> + params->rt_metric = f6i->fib6_metric;
> +
> + /* xdp and cls_bpf programs are run in RCU-bh so rcu_read_lock_bh is
> + * not needed here. Can not use __ipv6_neigh_lookup_noref here
> + * because we need to get nd_tbl via the stub
> + */
> + neigh = ___neigh_lookup_noref(ipv6_stub->nd_tbl, neigh_key_eq128,
> + ndisc_hashfn, ¶ms->ipv6_dst, dev);
> + if (neigh)
> + return bpf_fib_set_fwd_params(params, neigh, dev);
> +
> + return 0;
> +}
> +#endif
> +
> +BPF_CALL_4(bpf_fib_lookup, struct xdp_buff *, ctx,
> + struct bpf_fib_lookup *, params, int, plen, u32, flags)
> +{
> + if (plen < sizeof(*params))
> + return -EINVAL;
> +
> + switch (params->family) {
> +#if IS_ENABLED(CONFIG_INET)
> + case AF_INET:
> + return bpf_ipv4_fib_lookup(ctx, params, flags);
> +#endif
> +#if IS_ENABLED(CONFIG_IPV6)
> + case AF_INET6:
> + return bpf_ipv6_fib_lookup(ctx, params, flags);
> +#endif
> + }
> + return -ENOTSUPP;
> +}
> +
> +static const struct bpf_func_proto bpf_fib_lookup_proto = {
> + .func = bpf_fib_lookup,
> + .gpl_only = true,
> + .pkt_access = true,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_PTR_TO_MEM,
> + .arg3_type = ARG_CONST_SIZE,
> + .arg4_type = ARG_ANYTHING,
> +};
> +
> static const struct bpf_func_proto *
> bpf_base_func_proto(enum bpf_func_id func_id)
> {
> @@ -3861,6 +4090,8 @@ sk_filter_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> return &bpf_get_socket_cookie_proto;
> case BPF_FUNC_get_socket_uid:
> return &bpf_get_socket_uid_proto;
> + case BPF_FUNC_fib_lookup:
> + return &bpf_fib_lookup_proto;
This part doesn't belong to sk_filter_func_proto(), but to the
tc_cls_act_func_proto() instead.
> default:
> return bpf_base_func_proto(func_id);
> }
> @@ -3957,6 +4188,8 @@ xdp_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> return &bpf_xdp_redirect_map_proto;
> case BPF_FUNC_xdp_adjust_tail:
> return &bpf_xdp_adjust_tail_proto;
> + case BPF_FUNC_fib_lookup:
> + return &bpf_fib_lookup_proto;
Basically, you're using the very same bpf_fib_lookup_proto for
both XDP and skb. In the skb case, you're reusing the two functions
bpf_ipv{4,6}_fib_lookup(), so when you get the netdev pointer for
retrieving the netns, you'll crash at dev_net(ctx->rxq->dev) since
this is XDP only and not skb meta data.
Therefore, as mentioned, pass the netdev to bpf_ipv{4,6}_fib_lookup()
to have it generic and have bpf_xdp_fib_lookup_proto and
bpf_skb_fib_lookup_proto where both are under the case BPF_FUNC_fib_lookup
in the respective *func_proto(), but using the proper prototypes according
to their correct context. Meaning, both reuse bpf_ipv{4,6}_fib_lookup()
from each of their BPF_CALL_4() helper implementation.
> default:
> return bpf_base_func_proto(func_id);
> }
>
^ permalink raw reply
* [PATCH bpf-next v7 05/10] bpf/verifier: improve register value range tracking with ARSH
From: Yonghong Song @ 2018-04-25 19:29 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20180425192910.556352-1-yhs@fb.com>
When helpers like bpf_get_stack returns an int value
and later on used for arithmetic computation, the LSH and ARSH
operations are often required to get proper sign extension into
64-bit. For example, without this patch:
54: R0=inv(id=0,umax_value=800)
54: (bf) r8 = r0
55: R0=inv(id=0,umax_value=800) R8_w=inv(id=0,umax_value=800)
55: (67) r8 <<= 32
56: R8_w=inv(id=0,umax_value=3435973836800,var_off=(0x0; 0x3ff00000000))
56: (c7) r8 s>>= 32
57: R8=inv(id=0)
With this patch:
54: R0=inv(id=0,umax_value=800)
54: (bf) r8 = r0
55: R0=inv(id=0,umax_value=800) R8_w=inv(id=0,umax_value=800)
55: (67) r8 <<= 32
56: R8_w=inv(id=0,umax_value=3435973836800,var_off=(0x0; 0x3ff00000000))
56: (c7) r8 s>>= 32
57: R8=inv(id=0, umax_value=800,var_off=(0x0; 0x3ff))
With better range of "R8", later on when "R8" is added to other register,
e.g., a map pointer or scalar-value register, the better register
range can be derived and verifier failure may be avoided.
In our later example,
......
usize = bpf_get_stack(ctx, raw_data, max_len, BPF_F_USER_STACK);
if (usize < 0)
return 0;
ksize = bpf_get_stack(ctx, raw_data + usize, max_len - usize, 0);
......
Without improving ARSH value range tracking, the register representing
"max_len - usize" will have smin_value equal to S64_MIN and will be
rejected by verifier.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/tnum.h | 4 +++-
kernel/bpf/tnum.c | 10 ++++++++++
kernel/bpf/verifier.c | 41 +++++++++++++++++++++++++++++++++++++++++
3 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/include/linux/tnum.h b/include/linux/tnum.h
index 0d2d3da..c7dc2b5 100644
--- a/include/linux/tnum.h
+++ b/include/linux/tnum.h
@@ -23,8 +23,10 @@ struct tnum tnum_range(u64 min, u64 max);
/* Arithmetic and logical ops */
/* Shift a tnum left (by a fixed shift) */
struct tnum tnum_lshift(struct tnum a, u8 shift);
-/* Shift a tnum right (by a fixed shift) */
+/* Shift (rsh) a tnum right (by a fixed shift) */
struct tnum tnum_rshift(struct tnum a, u8 shift);
+/* Shift (arsh) a tnum right (by a fixed min_shift) */
+struct tnum tnum_arshift(struct tnum a, u8 min_shift);
/* Add two tnums, return @a + @b */
struct tnum tnum_add(struct tnum a, struct tnum b);
/* Subtract two tnums, return @a - @b */
diff --git a/kernel/bpf/tnum.c b/kernel/bpf/tnum.c
index 1f4bf68..938d412 100644
--- a/kernel/bpf/tnum.c
+++ b/kernel/bpf/tnum.c
@@ -43,6 +43,16 @@ struct tnum tnum_rshift(struct tnum a, u8 shift)
return TNUM(a.value >> shift, a.mask >> shift);
}
+struct tnum tnum_arshift(struct tnum a, u8 min_shift)
+{
+ /* if a.value is negative, arithmetic shifting by minimum shift
+ * will have larger negative offset compared to more shifting.
+ * If a.value is nonnegative, arithmetic shifting by minimum shift
+ * will have larger positive offset compare to more shifting.
+ */
+ return TNUM((s64)a.value >> min_shift, (s64)a.mask >> min_shift);
+}
+
struct tnum tnum_add(struct tnum a, struct tnum b)
{
u64 sm, sv, sigma, chi, mu;
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6e3f859..573807f 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2974,6 +2974,47 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
/* We may learn something more from the var_off */
__update_reg_bounds(dst_reg);
break;
+ case BPF_ARSH:
+ if (umax_val >= insn_bitness) {
+ /* Shifts greater than 31 or 63 are undefined.
+ * This includes shifts by a negative number.
+ */
+ mark_reg_unknown(env, regs, insn->dst_reg);
+ break;
+ }
+
+ /* BPF_ARSH is an arithmetic shift. The new range of
+ * smin_value and smax_value should take the sign
+ * into consideration.
+ *
+ * For example, if smin_value = -16, umin_val = 0
+ * and umax_val = 2, the new smin_value should be
+ * -16 >> 0 = -16 since -16 >> 2 = -4.
+ * If smin_value = 16, umin_val = 0 and umax_val = 2,
+ * the new smin_value should be 16 >> 2 = 4.
+ *
+ * Now suppose smax_value = -4, umin_val = 0 and
+ * umax_val = 2, the new smax_value should be
+ * -4 >> 2 = -1. If smax_value = 32 with the same
+ * umin_val/umax_val, the new smax_value should remain 32.
+ */
+ if (dst_reg->smin_value < 0)
+ dst_reg->smin_value >>= umin_val;
+ else
+ dst_reg->smin_value >>= umax_val;
+ if (dst_reg->smax_value < 0)
+ dst_reg->smax_value >>= umax_val;
+ else
+ dst_reg->smax_value >>= umin_val;
+ dst_reg->var_off = tnum_arshift(dst_reg->var_off, umin_val);
+
+ /* blow away the dst_reg umin_value/umax_value and rely on
+ * dst_reg var_off to refine the result.
+ */
+ dst_reg->umin_value = 0;
+ dst_reg->umax_value = U64_MAX;
+ __update_reg_bounds(dst_reg);
+ break;
default:
mark_reg_unknown(env, regs, insn->dst_reg);
break;
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next v7 00/10] bpf: add bpf_get_stack helper
From: Yonghong Song @ 2018-04-25 19:29 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
Currently, stackmap and bpf_get_stackid helper are provided
for bpf program to get the stack trace. This approach has
a limitation though. If two stack traces have the same hash,
only one will get stored in the stackmap table regardless of
whether BPF_F_REUSE_STACKID is specified or not,
so some stack traces may be missing from user perspective.
This patch implements a new helper, bpf_get_stack, will
send stack traces directly to bpf program. The bpf program
is able to see all stack traces, and then can do in-kernel
processing or send stack traces to user space through
shared map or bpf_perf_event_output.
Patches #1 and #2 implemented the core kernel support.
Patch #3 removes two never-hit branches in verifier.
Patches #4 and #5 are two verifier improves to make
bpf programming easier. Patch #6 synced the new helper
to tools headers. Patch #7 moved perf_event polling code
and ksym lookup code from samples/bpf to
tools/testing/selftests/bpf. Patch #8 added a verifier
test in tools/bpf for new verifier change.
Patches #9 and #10 added tests for raw tracepoint prog
and tracepoint prog respectively.
Changelogs:
v6 -> v7:
. do perf callchain buffer allocation inside the
verifier. so if the prog->has_callchain_buf is set,
it is guaranteed that the buffer has been allocated.
. change condition "trace_nr <= skip" to "trace_nr < skip"
so that for zero size buffer, return 0 instead of -EFAULT
v5 -> v6:
. after refining return register smax_value and umax_value
for helpers bpf_get_stack and bpf_probe_read_str,
bounds and var_off of the return register are further refined.
. added missing commit message for tools header sync commit.
. removed one unnecessary empty line.
v4 -> v5:
. relied on dst_reg->var_off to refine umin_val/umax_val
in verifier handling BPF_ARSH value range tracking,
suggested by Edward.
v3 -> v4:
. fixed a bug when meta ptr is set to NULL in check_func_arg.
. introduced tnum_arshift and added detailed comments for
the underlying implementation
. avoided using VLA in tools/bpf test_progs.
v2 -> v3:
. used meta to track helper memory size argument
. implemented range checking for ARSH in verifier
. moved perf event polling and ksym related functions
from samples/bpf to tools/bpf
. added test to compare build id's between bpf_get_stackid
and bpf_get_stack
v1 -> v2:
. fixed compilation error when CONFIG_PERF_EVENTS is not enabled
Yonghong Song (10):
bpf: change prototype for stack_map_get_build_id_offset
bpf: add bpf_get_stack helper
bpf/verifier: refine retval R0 state for bpf_get_stack helper
bpf: remove never-hit branches in verifier adjust_scalar_min_max_vals
bpf/verifier: improve register value range tracking with ARSH
tools/bpf: add bpf_get_stack helper to tools headers
samples/bpf: move common-purpose trace functions to selftests
tools/bpf: add a verifier test case for bpf_get_stack helper and ARSH
tools/bpf: add a test for bpf_get_stack with raw tracepoint prog
tools/bpf: add a test for bpf_get_stack with tracepoint prog
include/linux/bpf.h | 1 +
include/linux/filter.h | 3 +-
include/linux/tnum.h | 4 +-
include/uapi/linux/bpf.h | 19 +-
kernel/bpf/core.c | 5 +
kernel/bpf/stackmap.c | 80 ++++++++-
kernel/bpf/tnum.c | 10 ++
kernel/bpf/verifier.c | 98 ++++++++++-
kernel/trace/bpf_trace.c | 50 +++++-
samples/bpf/Makefile | 11 +-
samples/bpf/bpf_load.c | 63 -------
samples/bpf/bpf_load.h | 7 -
samples/bpf/offwaketime_user.c | 1 +
samples/bpf/sampleip_user.c | 1 +
samples/bpf/spintest_user.c | 1 +
samples/bpf/trace_event_user.c | 1 +
samples/bpf/trace_output_user.c | 125 ++------------
tools/include/uapi/linux/bpf.h | 19 +-
tools/testing/selftests/bpf/Makefile | 3 +-
tools/testing/selftests/bpf/bpf_helpers.h | 2 +
tools/testing/selftests/bpf/test_get_stack_rawtp.c | 102 +++++++++++
tools/testing/selftests/bpf/test_progs.c | 192 ++++++++++++++++++++-
.../selftests/bpf/test_stacktrace_build_id.c | 20 ++-
tools/testing/selftests/bpf/test_stacktrace_map.c | 19 +-
tools/testing/selftests/bpf/test_verifier.c | 45 +++++
tools/testing/selftests/bpf/trace_helpers.c | 186 ++++++++++++++++++++
tools/testing/selftests/bpf/trace_helpers.h | 24 +++
27 files changed, 874 insertions(+), 218 deletions(-)
create mode 100644 tools/testing/selftests/bpf/test_get_stack_rawtp.c
create mode 100644 tools/testing/selftests/bpf/trace_helpers.c
create mode 100644 tools/testing/selftests/bpf/trace_helpers.h
--
2.9.5
^ permalink raw reply
* [PATCH bpf-next v7 09/10] tools/bpf: add a test for bpf_get_stack with raw tracepoint prog
From: Yonghong Song @ 2018-04-25 19:29 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20180425192910.556352-1-yhs@fb.com>
The test attached a raw_tracepoint program to sched/sched_switch.
It tested to get stack for user space, kernel space and user
space with build_id request. It also tested to get user
and kernel stack into the same buffer with back-to-back
bpf_get_stack helper calls.
Whenever the kernel stack is available, the user space
application will check to ensure that the kernel function
for raw_tracepoint ___bpf_prog_run is part of the stack.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/Makefile | 3 +-
tools/testing/selftests/bpf/test_get_stack_rawtp.c | 102 +++++++++++++++++
tools/testing/selftests/bpf/test_progs.c | 122 +++++++++++++++++++++
3 files changed, 226 insertions(+), 1 deletion(-)
create mode 100644 tools/testing/selftests/bpf/test_get_stack_rawtp.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 0c19d5e..5a199af 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -32,7 +32,7 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
test_l4lb_noinline.o test_xdp_noinline.o test_stacktrace_map.o \
sample_map_ret0.o test_tcpbpf_kern.o test_stacktrace_build_id.o \
sockmap_tcp_msg_prog.o connect4_prog.o connect6_prog.o test_adjust_tail.o \
- test_btf_haskv.o test_btf_nokv.o test_sockmap_kern.o
+ test_btf_haskv.o test_btf_nokv.o test_sockmap_kern.o test_get_stack_rawtp.o
# Order correspond to 'make run_tests' order
TEST_PROGS := test_kmod.sh \
@@ -57,6 +57,7 @@ $(OUTPUT)/test_dev_cgroup: cgroup_helpers.c
$(OUTPUT)/test_sock: cgroup_helpers.c
$(OUTPUT)/test_sock_addr: cgroup_helpers.c
$(OUTPUT)/test_sockmap: cgroup_helpers.c
+$(OUTPUT)/test_progs: trace_helpers.c
.PHONY: force
diff --git a/tools/testing/selftests/bpf/test_get_stack_rawtp.c b/tools/testing/selftests/bpf/test_get_stack_rawtp.c
new file mode 100644
index 0000000..ba1dcf9
--- /dev/null
+++ b/tools/testing/selftests/bpf/test_get_stack_rawtp.c
@@ -0,0 +1,102 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/bpf.h>
+#include "bpf_helpers.h"
+
+/* Permit pretty deep stack traces */
+#define MAX_STACK_RAWTP 100
+struct stack_trace_t {
+ int pid;
+ int kern_stack_size;
+ int user_stack_size;
+ int user_stack_buildid_size;
+ __u64 kern_stack[MAX_STACK_RAWTP];
+ __u64 user_stack[MAX_STACK_RAWTP];
+ struct bpf_stack_build_id user_stack_buildid[MAX_STACK_RAWTP];
+};
+
+struct bpf_map_def SEC("maps") perfmap = {
+ .type = BPF_MAP_TYPE_PERF_EVENT_ARRAY,
+ .key_size = sizeof(int),
+ .value_size = sizeof(__u32),
+ .max_entries = 2,
+};
+
+struct bpf_map_def SEC("maps") stackdata_map = {
+ .type = BPF_MAP_TYPE_PERCPU_ARRAY,
+ .key_size = sizeof(__u32),
+ .value_size = sizeof(struct stack_trace_t),
+ .max_entries = 1,
+};
+
+/* Allocate per-cpu space twice the needed. For the code below
+ * usize = bpf_get_stack(ctx, raw_data, max_len, BPF_F_USER_STACK);
+ * if (usize < 0)
+ * return 0;
+ * ksize = bpf_get_stack(ctx, raw_data + usize, max_len - usize, 0);
+ *
+ * If we have value_size = MAX_STACK_RAWTP * sizeof(__u64),
+ * verifier will complain that access "raw_data + usize"
+ * with size "max_len - usize" may be out of bound.
+ * The maximum "raw_data + usize" is "raw_data + max_len"
+ * and the maximum "max_len - usize" is "max_len", verifier
+ * concludes that the maximum buffer access range is
+ * "raw_data[0...max_len * 2 - 1]" and hence reject the program.
+ *
+ * Doubling the to-be-used max buffer size can fix this verifier
+ * issue and avoid complicated C programming massaging.
+ * This is an acceptable workaround since there is one entry here.
+ */
+struct bpf_map_def SEC("maps") rawdata_map = {
+ .type = BPF_MAP_TYPE_PERCPU_ARRAY,
+ .key_size = sizeof(__u32),
+ .value_size = MAX_STACK_RAWTP * sizeof(__u64) * 2,
+ .max_entries = 1,
+};
+
+SEC("tracepoint/sched/sched_switch")
+int bpf_prog1(void *ctx)
+{
+ int max_len, max_buildid_len, usize, ksize, total_size;
+ struct stack_trace_t *data;
+ void *raw_data;
+ __u32 key = 0;
+
+ data = bpf_map_lookup_elem(&stackdata_map, &key);
+ if (!data)
+ return 0;
+
+ max_len = MAX_STACK_RAWTP * sizeof(__u64);
+ max_buildid_len = MAX_STACK_RAWTP * sizeof(struct bpf_stack_build_id);
+ data->pid = bpf_get_current_pid_tgid();
+ data->kern_stack_size = bpf_get_stack(ctx, data->kern_stack,
+ max_len, 0);
+ data->user_stack_size = bpf_get_stack(ctx, data->user_stack, max_len,
+ BPF_F_USER_STACK);
+ data->user_stack_buildid_size = bpf_get_stack(
+ ctx, data->user_stack_buildid, max_buildid_len,
+ BPF_F_USER_STACK | BPF_F_USER_BUILD_ID);
+ bpf_perf_event_output(ctx, &perfmap, 0, data, sizeof(*data));
+
+ /* write both kernel and user stacks to the same buffer */
+ raw_data = bpf_map_lookup_elem(&rawdata_map, &key);
+ if (!raw_data)
+ return 0;
+
+ usize = bpf_get_stack(ctx, raw_data, max_len, BPF_F_USER_STACK);
+ if (usize < 0)
+ return 0;
+
+ ksize = bpf_get_stack(ctx, raw_data + usize, max_len - usize, 0);
+ if (ksize < 0)
+ return 0;
+
+ total_size = usize + ksize;
+ if (total_size > 0 && total_size <= max_len)
+ bpf_perf_event_output(ctx, &perfmap, 0, raw_data, total_size);
+
+ return 0;
+}
+
+char _license[] SEC("license") = "GPL";
+__u32 _version SEC("version") = 1; /* ignored by tracepoints, required by libbpf.a */
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index eedda98..c148a55 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -38,6 +38,7 @@ typedef __u16 __sum16;
#include "bpf_util.h"
#include "bpf_endian.h"
#include "bpf_rlimit.h"
+#include "trace_helpers.h"
static int error_cnt, pass_cnt;
@@ -1204,6 +1205,126 @@ static void test_stacktrace_build_id(void)
return;
}
+#define MAX_CNT_RAWTP 10ull
+#define MAX_STACK_RAWTP 100
+struct get_stack_trace_t {
+ int pid;
+ int kern_stack_size;
+ int user_stack_size;
+ int user_stack_buildid_size;
+ __u64 kern_stack[MAX_STACK_RAWTP];
+ __u64 user_stack[MAX_STACK_RAWTP];
+ struct bpf_stack_build_id user_stack_buildid[MAX_STACK_RAWTP];
+};
+
+static void get_stack_raw_tp_action(void)
+{
+ FILE *f;
+
+ f = popen("taskset 1 dd if=/dev/zero of=/dev/null", "r");
+ (void) f;
+}
+
+static int get_stack_print_output(void *data, int size)
+{
+ bool good_kern_stack = false, good_user_stack = false;
+ const char *expected_func = "___bpf_prog_run";
+ struct get_stack_trace_t *e = data;
+ int i, num_stack;
+ static __u64 cnt;
+ struct ksym *ks;
+
+ cnt++;
+
+ if (size < sizeof(struct get_stack_trace_t)) {
+ __u64 *raw_data = data;
+
+ num_stack = size / sizeof(__u64);
+ for (i = 0; i < num_stack; i++) {
+ ks = ksym_search(raw_data[i]);
+ if (ks && (strcmp(ks->name, expected_func) == 0)) {
+ good_kern_stack = true;
+ good_user_stack = (i > 0);
+ }
+ }
+ } else {
+ if (e->kern_stack_size > 0) {
+ num_stack = e->kern_stack_size / sizeof(__u64);
+ for (i = 0; i < num_stack; i++) {
+ ks = ksym_search(e->kern_stack[i]);
+ if (ks && (strcmp(ks->name, expected_func) == 0))
+ good_kern_stack = true;
+ }
+ }
+ if (e->user_stack_size > 0 && e->user_stack_buildid_size > 0)
+ good_user_stack = true;
+ }
+ if (!good_kern_stack || !good_user_stack)
+ return PERF_EVENT_ERROR;
+
+ if (cnt == MAX_CNT_RAWTP)
+ return PERF_EVENT_DONE;
+
+ return PERF_EVENT_CONT;
+}
+
+static void test_get_stack_raw_tp(void)
+{
+ const char *file = "./test_get_stack_rawtp.o";
+ int efd, err, prog_fd, pmu_fd, perfmap_fd;
+ struct perf_event_attr attr = {};
+ __u32 key = 0, duration = 0;
+ struct bpf_object *obj;
+
+ err = bpf_prog_load(file, BPF_PROG_TYPE_RAW_TRACEPOINT, &obj, &prog_fd);
+ if (CHECK(err, "prog_load raw tp", "err %d errno %d\n", err, errno))
+ return;
+
+ efd = bpf_raw_tracepoint_open("sched_switch", prog_fd);
+ if (CHECK(efd < 0, "raw_tp_open", "err %d errno %d\n", efd, errno))
+ goto close_prog;
+
+ perfmap_fd = bpf_find_map(__func__, obj, "perfmap");
+ if (CHECK(perfmap_fd < 0, "bpf_find_map", "err %d errno %d\n",
+ perfmap_fd, errno))
+ goto close_prog;
+
+ err = load_kallsyms();
+ if (CHECK(err < 0, "load_kallsyms", "err %d errno %d\n", err, errno))
+ goto close_prog;
+
+ attr.sample_type = PERF_SAMPLE_RAW;
+ attr.type = PERF_TYPE_SOFTWARE;
+ attr.config = PERF_COUNT_SW_BPF_OUTPUT;
+ pmu_fd = syscall(__NR_perf_event_open, &attr, -1/*pid*/, 0/*cpu*/,
+ -1/*group_fd*/, 0);
+ if (CHECK(pmu_fd < 0, "perf_event_open", "err %d errno %d\n", pmu_fd,
+ errno))
+ goto close_prog;
+
+ err = bpf_map_update_elem(perfmap_fd, &key, &pmu_fd, BPF_ANY);
+ if (CHECK(err < 0, "bpf_map_update_elem", "err %d errno %d\n", err,
+ errno))
+ goto close_prog;
+
+ err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
+ if (CHECK(err < 0, "ioctl PERF_EVENT_IOC_ENABLE", "err %d errno %d\n",
+ err, errno))
+ goto close_prog;
+
+ err = perf_event_poller(pmu_fd, get_stack_raw_tp_action,
+ get_stack_print_output);
+ if (CHECK(err < 0, "perf_event_poller", "err %d errno %d\n", err,
+ errno))
+ goto close_prog;
+
+ goto close_prog_noerr;
+close_prog:
+ error_cnt++;
+close_prog_noerr:
+ bpf_object__close(obj);
+}
+
int main(void)
{
test_pkt_access();
@@ -1219,6 +1340,7 @@ int main(void)
test_stacktrace_map();
test_stacktrace_build_id();
test_stacktrace_map_raw_tp();
+ test_get_stack_raw_tp();
printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next v7 08/10] tools/bpf: add a verifier test case for bpf_get_stack helper and ARSH
From: Yonghong Song @ 2018-04-25 19:29 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20180425192910.556352-1-yhs@fb.com>
The test_verifier already has a few ARSH test cases.
This patch adds a new test case which takes advantage of newly
improved verifier behavior for bpf_get_stack and ARSH.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/test_verifier.c | 45 +++++++++++++++++++++++++++++
1 file changed, 45 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index 165e9dd..1acafe26 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -11680,6 +11680,51 @@ static struct bpf_test tests[] = {
.errstr = "BPF_XADD stores into R2 packet",
.prog_type = BPF_PROG_TYPE_XDP,
},
+ {
+ "bpf_get_stack return R0 within range",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_LD_MAP_FD(BPF_REG_1, 0),
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
+ BPF_FUNC_map_lookup_elem),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 28),
+ BPF_MOV64_REG(BPF_REG_7, BPF_REG_0),
+ BPF_MOV64_IMM(BPF_REG_9, sizeof(struct test_val)),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_7),
+ BPF_MOV64_IMM(BPF_REG_3, sizeof(struct test_val)),
+ BPF_MOV64_IMM(BPF_REG_4, 256),
+ BPF_EMIT_CALL(BPF_FUNC_get_stack),
+ BPF_MOV64_IMM(BPF_REG_1, 0),
+ BPF_MOV64_REG(BPF_REG_8, BPF_REG_0),
+ BPF_ALU64_IMM(BPF_LSH, BPF_REG_8, 32),
+ BPF_ALU64_IMM(BPF_ARSH, BPF_REG_8, 32),
+ BPF_JMP_REG(BPF_JSLT, BPF_REG_1, BPF_REG_8, 16),
+ BPF_ALU64_REG(BPF_SUB, BPF_REG_9, BPF_REG_8),
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_7),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_2, BPF_REG_8),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_9),
+ BPF_ALU64_IMM(BPF_LSH, BPF_REG_1, 32),
+ BPF_ALU64_IMM(BPF_ARSH, BPF_REG_1, 32),
+ BPF_MOV64_REG(BPF_REG_3, BPF_REG_2),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_3, BPF_REG_1),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_7),
+ BPF_MOV64_IMM(BPF_REG_5, sizeof(struct test_val)),
+ BPF_ALU64_REG(BPF_ADD, BPF_REG_1, BPF_REG_5),
+ BPF_JMP_REG(BPF_JGE, BPF_REG_3, BPF_REG_1, 4),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_6),
+ BPF_MOV64_REG(BPF_REG_3, BPF_REG_9),
+ BPF_MOV64_IMM(BPF_REG_4, 0),
+ BPF_EMIT_CALL(BPF_FUNC_get_stack),
+ BPF_EXIT_INSN(),
+ },
+ .fixup_map2 = { 4 },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_TRACEPOINT,
+ },
};
static int probe_filter_length(const struct bpf_insn *fp)
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next v7 01/10] bpf: change prototype for stack_map_get_build_id_offset
From: Yonghong Song @ 2018-04-25 19:29 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20180425192910.556352-1-yhs@fb.com>
This patch didn't incur functionality change. The function prototype
got changed so that the same function can be reused later.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/stackmap.c | 13 +++++--------
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 57eeb12..04f6ec1 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -262,16 +262,11 @@ static int stack_map_get_build_id(struct vm_area_struct *vma,
return ret;
}
-static void stack_map_get_build_id_offset(struct bpf_map *map,
- struct stack_map_bucket *bucket,
+static void stack_map_get_build_id_offset(struct bpf_stack_build_id *id_offs,
u64 *ips, u32 trace_nr, bool user)
{
int i;
struct vm_area_struct *vma;
- struct bpf_stack_build_id *id_offs;
-
- bucket->nr = trace_nr;
- id_offs = (struct bpf_stack_build_id *)bucket->data;
/*
* We cannot do up_read() in nmi context, so build_id lookup is
@@ -361,8 +356,10 @@ BPF_CALL_3(bpf_get_stackid, struct pt_regs *, regs, struct bpf_map *, map,
pcpu_freelist_pop(&smap->freelist);
if (unlikely(!new_bucket))
return -ENOMEM;
- stack_map_get_build_id_offset(map, new_bucket, ips,
- trace_nr, user);
+ new_bucket->nr = trace_nr;
+ stack_map_get_build_id_offset(
+ (struct bpf_stack_build_id *)new_bucket->data,
+ ips, trace_nr, user);
trace_len = trace_nr * sizeof(struct bpf_stack_build_id);
if (hash_matches && bucket->nr == trace_nr &&
memcmp(bucket->data, new_bucket->data, trace_len) == 0) {
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next v7 10/10] tools/bpf: add a test for bpf_get_stack with tracepoint prog
From: Yonghong Song @ 2018-04-25 19:29 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20180425192910.556352-1-yhs@fb.com>
The test_stacktrace_map and test_stacktrace_build_id are
enhanced to call bpf_get_stack in the helper to get the
stack trace as well. The stack traces from bpf_get_stack
and bpf_get_stackid are compared to ensure that for the
same stack as represented as the same hash, their ip addresses
or build id's must be the same.
Acked-by: Alexei Starovoitov <ast@kernel.org>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/testing/selftests/bpf/test_progs.c | 70 ++++++++++++++++++++--
.../selftests/bpf/test_stacktrace_build_id.c | 20 ++++++-
tools/testing/selftests/bpf/test_stacktrace_map.c | 19 +++++-
3 files changed, 98 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index c148a55..664db67 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -897,11 +897,47 @@ static int compare_map_keys(int map1_fd, int map2_fd)
return 0;
}
+static int compare_stack_ips(int smap_fd, int amap_fd, int stack_trace_len)
+{
+ __u32 key, next_key, *cur_key_p, *next_key_p;
+ char *val_buf1, *val_buf2;
+ int i, err = 0;
+
+ val_buf1 = malloc(stack_trace_len);
+ val_buf2 = malloc(stack_trace_len);
+ cur_key_p = NULL;
+ next_key_p = &key;
+ while (bpf_map_get_next_key(smap_fd, cur_key_p, next_key_p) == 0) {
+ err = bpf_map_lookup_elem(smap_fd, next_key_p, val_buf1);
+ if (err)
+ goto out;
+ err = bpf_map_lookup_elem(amap_fd, next_key_p, val_buf2);
+ if (err)
+ goto out;
+ for (i = 0; i < stack_trace_len; i++) {
+ if (val_buf1[i] != val_buf2[i]) {
+ err = -1;
+ goto out;
+ }
+ }
+ key = *next_key_p;
+ cur_key_p = &key;
+ next_key_p = &next_key;
+ }
+ if (errno != ENOENT)
+ err = -1;
+
+out:
+ free(val_buf1);
+ free(val_buf2);
+ return err;
+}
+
static void test_stacktrace_map()
{
- int control_map_fd, stackid_hmap_fd, stackmap_fd;
+ int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
const char *file = "./test_stacktrace_map.o";
- int bytes, efd, err, pmu_fd, prog_fd;
+ int bytes, efd, err, pmu_fd, prog_fd, stack_trace_len;
struct perf_event_attr attr = {};
__u32 key, val, duration = 0;
struct bpf_object *obj;
@@ -957,6 +993,10 @@ static void test_stacktrace_map()
if (stackmap_fd < 0)
goto disable_pmu;
+ stack_amap_fd = bpf_find_map(__func__, obj, "stack_amap");
+ if (stack_amap_fd < 0)
+ goto disable_pmu;
+
/* give some time for bpf program run */
sleep(1);
@@ -978,6 +1018,12 @@ static void test_stacktrace_map()
"err %d errno %d\n", err, errno))
goto disable_pmu_noerr;
+ stack_trace_len = PERF_MAX_STACK_DEPTH * sizeof(__u64);
+ err = compare_stack_ips(stackmap_fd, stack_amap_fd, stack_trace_len);
+ if (CHECK(err, "compare_stack_ips stackmap vs. stack_amap",
+ "err %d errno %d\n", err, errno))
+ goto disable_pmu_noerr;
+
goto disable_pmu_noerr;
disable_pmu:
error_cnt++;
@@ -1071,9 +1117,9 @@ static int extract_build_id(char *build_id, size_t size)
static void test_stacktrace_build_id(void)
{
- int control_map_fd, stackid_hmap_fd, stackmap_fd;
+ int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
const char *file = "./test_stacktrace_build_id.o";
- int bytes, efd, err, pmu_fd, prog_fd;
+ int bytes, efd, err, pmu_fd, prog_fd, stack_trace_len;
struct perf_event_attr attr = {};
__u32 key, previous_key, val, duration = 0;
struct bpf_object *obj;
@@ -1138,6 +1184,11 @@ static void test_stacktrace_build_id(void)
err, errno))
goto disable_pmu;
+ stack_amap_fd = bpf_find_map(__func__, obj, "stack_amap");
+ if (CHECK(stack_amap_fd < 0, "bpf_find_map stack_amap",
+ "err %d errno %d\n", err, errno))
+ goto disable_pmu;
+
assert(system("dd if=/dev/urandom of=/dev/zero count=4 2> /dev/null")
== 0);
assert(system("./urandom_read if=/dev/urandom of=/dev/zero count=4 2> /dev/null") == 0);
@@ -1189,8 +1240,15 @@ static void test_stacktrace_build_id(void)
previous_key = key;
} while (bpf_map_get_next_key(stackmap_fd, &previous_key, &key) == 0);
- CHECK(build_id_matches < 1, "build id match",
- "Didn't find expected build ID from the map");
+ if (CHECK(build_id_matches < 1, "build id match",
+ "Didn't find expected build ID from the map"))
+ goto disable_pmu;
+
+ stack_trace_len = PERF_MAX_STACK_DEPTH
+ * sizeof(struct bpf_stack_build_id);
+ err = compare_stack_ips(stackmap_fd, stack_amap_fd, stack_trace_len);
+ CHECK(err, "compare_stack_ips stackmap vs. stack_amap",
+ "err %d errno %d\n", err, errno);
disable_pmu:
ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
diff --git a/tools/testing/selftests/bpf/test_stacktrace_build_id.c b/tools/testing/selftests/bpf/test_stacktrace_build_id.c
index b755bd7..d86c281 100644
--- a/tools/testing/selftests/bpf/test_stacktrace_build_id.c
+++ b/tools/testing/selftests/bpf/test_stacktrace_build_id.c
@@ -19,7 +19,7 @@ struct bpf_map_def SEC("maps") stackid_hmap = {
.type = BPF_MAP_TYPE_HASH,
.key_size = sizeof(__u32),
.value_size = sizeof(__u32),
- .max_entries = 10000,
+ .max_entries = 16384,
};
struct bpf_map_def SEC("maps") stackmap = {
@@ -31,6 +31,14 @@ struct bpf_map_def SEC("maps") stackmap = {
.map_flags = BPF_F_STACK_BUILD_ID,
};
+struct bpf_map_def SEC("maps") stack_amap = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(__u32),
+ .value_size = sizeof(struct bpf_stack_build_id)
+ * PERF_MAX_STACK_DEPTH,
+ .max_entries = 128,
+};
+
/* taken from /sys/kernel/debug/tracing/events/random/urandom_read/format */
struct random_urandom_args {
unsigned long long pad;
@@ -42,7 +50,10 @@ struct random_urandom_args {
SEC("tracepoint/random/urandom_read")
int oncpu(struct random_urandom_args *args)
{
+ __u32 max_len = sizeof(struct bpf_stack_build_id)
+ * PERF_MAX_STACK_DEPTH;
__u32 key = 0, val = 0, *value_p;
+ void *stack_p;
value_p = bpf_map_lookup_elem(&control_map, &key);
if (value_p && *value_p)
@@ -50,8 +61,13 @@ int oncpu(struct random_urandom_args *args)
/* The size of stackmap and stackid_hmap should be the same */
key = bpf_get_stackid(args, &stackmap, BPF_F_USER_STACK);
- if ((int)key >= 0)
+ if ((int)key >= 0) {
bpf_map_update_elem(&stackid_hmap, &key, &val, 0);
+ stack_p = bpf_map_lookup_elem(&stack_amap, &key);
+ if (stack_p)
+ bpf_get_stack(args, stack_p, max_len,
+ BPF_F_USER_STACK | BPF_F_USER_BUILD_ID);
+ }
return 0;
}
diff --git a/tools/testing/selftests/bpf/test_stacktrace_map.c b/tools/testing/selftests/bpf/test_stacktrace_map.c
index 76d85c5d..af111af 100644
--- a/tools/testing/selftests/bpf/test_stacktrace_map.c
+++ b/tools/testing/selftests/bpf/test_stacktrace_map.c
@@ -19,14 +19,21 @@ struct bpf_map_def SEC("maps") stackid_hmap = {
.type = BPF_MAP_TYPE_HASH,
.key_size = sizeof(__u32),
.value_size = sizeof(__u32),
- .max_entries = 10000,
+ .max_entries = 16384,
};
struct bpf_map_def SEC("maps") stackmap = {
.type = BPF_MAP_TYPE_STACK_TRACE,
.key_size = sizeof(__u32),
.value_size = sizeof(__u64) * PERF_MAX_STACK_DEPTH,
- .max_entries = 10000,
+ .max_entries = 16384,
+};
+
+struct bpf_map_def SEC("maps") stack_amap = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(__u32),
+ .value_size = sizeof(__u64) * PERF_MAX_STACK_DEPTH,
+ .max_entries = 16384,
};
/* taken from /sys/kernel/debug/tracing/events/sched/sched_switch/format */
@@ -44,7 +51,9 @@ struct sched_switch_args {
SEC("tracepoint/sched/sched_switch")
int oncpu(struct sched_switch_args *ctx)
{
+ __u32 max_len = PERF_MAX_STACK_DEPTH * sizeof(__u64);
__u32 key = 0, val = 0, *value_p;
+ void *stack_p;
value_p = bpf_map_lookup_elem(&control_map, &key);
if (value_p && *value_p)
@@ -52,8 +61,12 @@ int oncpu(struct sched_switch_args *ctx)
/* The size of stackmap and stackid_hmap should be the same */
key = bpf_get_stackid(ctx, &stackmap, 0);
- if ((int)key >= 0)
+ if ((int)key >= 0) {
bpf_map_update_elem(&stackid_hmap, &key, &val, 0);
+ stack_p = bpf_map_lookup_elem(&stack_amap, &key);
+ if (stack_p)
+ bpf_get_stack(ctx, stack_p, max_len, 0);
+ }
return 0;
}
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next v7 06/10] tools/bpf: add bpf_get_stack helper to tools headers
From: Yonghong Song @ 2018-04-25 19:29 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20180425192910.556352-1-yhs@fb.com>
The tools header file bpf.h is synced with kernel uapi bpf.h.
The new helper is also added to bpf_helpers.h.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
tools/include/uapi/linux/bpf.h | 19 +++++++++++++++++--
tools/testing/selftests/bpf/bpf_helpers.h | 2 ++
2 files changed, 19 insertions(+), 2 deletions(-)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index e667939..663900f 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -529,6 +529,17 @@ union bpf_attr {
* other bits - reserved
* Return: >= 0 stackid on success or negative error
*
+ * int bpf_get_stack(ctx, buf, size, flags)
+ * walk user or kernel stack and store the ips in buf
+ * @ctx: struct pt_regs*
+ * @buf: user buffer to fill stack
+ * @size: the buf size
+ * @flags: bits 0-7 - numer of stack frames to skip
+ * bit 8 - collect user stack instead of kernel
+ * bit 11 - get build-id as well if user stack
+ * other bits - reserved
+ * Return: >= 0 size copied on success or negative error
+ *
* s64 bpf_csum_diff(from, from_size, to, to_size, seed)
* calculate csum diff
* @from: raw from buffer
@@ -851,7 +862,8 @@ union bpf_attr {
FN(msg_pull_data), \
FN(bind), \
FN(xdp_adjust_tail), \
- FN(skb_get_xfrm_state),
+ FN(skb_get_xfrm_state), \
+ FN(get_stack),
/* integer value in 'imm' field of BPF_CALL instruction selects which helper
* function eBPF program intends to call
@@ -885,11 +897,14 @@ enum bpf_func_id {
/* BPF_FUNC_skb_set_tunnel_key and BPF_FUNC_skb_get_tunnel_key flags. */
#define BPF_F_TUNINFO_IPV6 (1ULL << 0)
-/* BPF_FUNC_get_stackid flags. */
+/* flags for both BPF_FUNC_get_stackid and BPF_FUNC_get_stack. */
#define BPF_F_SKIP_FIELD_MASK 0xffULL
#define BPF_F_USER_STACK (1ULL << 8)
+/* flags used by BPF_FUNC_get_stackid only. */
#define BPF_F_FAST_STACK_CMP (1ULL << 9)
#define BPF_F_REUSE_STACKID (1ULL << 10)
+/* flags used by BPF_FUNC_get_stack only. */
+#define BPF_F_USER_BUILD_ID (1ULL << 11)
/* BPF_FUNC_skb_set_tunnel_key flags. */
#define BPF_F_ZERO_CSUM_TX (1ULL << 1)
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 69d7b91..265f8e0 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -101,6 +101,8 @@ static int (*bpf_xdp_adjust_tail)(void *ctx, int offset) =
static int (*bpf_skb_get_xfrm_state)(void *ctx, int index, void *state,
int size, int flags) =
(void *) BPF_FUNC_skb_get_xfrm_state;
+static int (*bpf_get_stack)(void *ctx, void *buf, int size, int flags) =
+ (void *) BPF_FUNC_get_stack;
/* llvm builtin functions that eBPF C program may use to
* emit BPF_LD_ABS and BPF_LD_IND instructions
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next v7 07/10] samples/bpf: move common-purpose trace functions to selftests
From: Yonghong Song @ 2018-04-25 19:29 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20180425192910.556352-1-yhs@fb.com>
There is no functionality change in this patch. The common-purpose
trace functions, including perf_event polling and ksym lookup,
are moved from trace_output_user.c and bpf_load.c to
selftests/bpf/trace_helpers.c so that these function can
be reused later in selftests.
Acked-by: Alexei Starovoitov <ast@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
samples/bpf/Makefile | 11 +-
samples/bpf/bpf_load.c | 63 ----------
samples/bpf/bpf_load.h | 7 --
samples/bpf/offwaketime_user.c | 1 +
samples/bpf/sampleip_user.c | 1 +
samples/bpf/spintest_user.c | 1 +
samples/bpf/trace_event_user.c | 1 +
samples/bpf/trace_output_user.c | 125 +++----------------
tools/testing/selftests/bpf/trace_helpers.c | 186 ++++++++++++++++++++++++++++
tools/testing/selftests/bpf/trace_helpers.h | 24 ++++
10 files changed, 238 insertions(+), 182 deletions(-)
create mode 100644 tools/testing/selftests/bpf/trace_helpers.c
create mode 100644 tools/testing/selftests/bpf/trace_helpers.h
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index aa8c392..d36444c 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -49,6 +49,7 @@ hostprogs-y += xdp_adjust_tail
# Libbpf dependencies
LIBBPF := ../../tools/lib/bpf/bpf.o ../../tools/lib/bpf/nlattr.o
CGROUP_HELPERS := ../../tools/testing/selftests/bpf/cgroup_helpers.o
+TRACE_HELPERS := ../../tools/testing/selftests/bpf/trace_helpers.o
test_lru_dist-objs := test_lru_dist.o $(LIBBPF)
sock_example-objs := sock_example.o $(LIBBPF)
@@ -65,10 +66,10 @@ tracex6-objs := bpf_load.o $(LIBBPF) tracex6_user.o
tracex7-objs := bpf_load.o $(LIBBPF) tracex7_user.o
load_sock_ops-objs := bpf_load.o $(LIBBPF) load_sock_ops.o
test_probe_write_user-objs := bpf_load.o $(LIBBPF) test_probe_write_user_user.o
-trace_output-objs := bpf_load.o $(LIBBPF) trace_output_user.o
+trace_output-objs := bpf_load.o $(LIBBPF) trace_output_user.o $(TRACE_HELPERS)
lathist-objs := bpf_load.o $(LIBBPF) lathist_user.o
-offwaketime-objs := bpf_load.o $(LIBBPF) offwaketime_user.o
-spintest-objs := bpf_load.o $(LIBBPF) spintest_user.o
+offwaketime-objs := bpf_load.o $(LIBBPF) offwaketime_user.o $(TRACE_HELPERS)
+spintest-objs := bpf_load.o $(LIBBPF) spintest_user.o $(TRACE_HELPERS)
map_perf_test-objs := bpf_load.o $(LIBBPF) map_perf_test_user.o
test_overhead-objs := bpf_load.o $(LIBBPF) test_overhead_user.o
test_cgrp2_array_pin-objs := $(LIBBPF) test_cgrp2_array_pin.o
@@ -82,8 +83,8 @@ xdp2-objs := bpf_load.o $(LIBBPF) xdp1_user.o
xdp_router_ipv4-objs := bpf_load.o $(LIBBPF) xdp_router_ipv4_user.o
test_current_task_under_cgroup-objs := bpf_load.o $(LIBBPF) $(CGROUP_HELPERS) \
test_current_task_under_cgroup_user.o
-trace_event-objs := bpf_load.o $(LIBBPF) trace_event_user.o
-sampleip-objs := bpf_load.o $(LIBBPF) sampleip_user.o
+trace_event-objs := bpf_load.o $(LIBBPF) trace_event_user.o $(TRACE_HELPERS)
+sampleip-objs := bpf_load.o $(LIBBPF) sampleip_user.o $(TRACE_HELPERS)
tc_l2_redirect-objs := bpf_load.o $(LIBBPF) tc_l2_redirect_user.o
lwt_len_hist-objs := bpf_load.o $(LIBBPF) lwt_len_hist_user.o
xdp_tx_iptunnel-objs := bpf_load.o $(LIBBPF) xdp_tx_iptunnel_user.o
diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
index bebe418..529972e 100644
--- a/samples/bpf/bpf_load.c
+++ b/samples/bpf/bpf_load.c
@@ -650,66 +650,3 @@ void read_trace_pipe(void)
}
}
}
-
-#define MAX_SYMS 300000
-static struct ksym syms[MAX_SYMS];
-static int sym_cnt;
-
-static int ksym_cmp(const void *p1, const void *p2)
-{
- return ((struct ksym *)p1)->addr - ((struct ksym *)p2)->addr;
-}
-
-int load_kallsyms(void)
-{
- FILE *f = fopen("/proc/kallsyms", "r");
- char func[256], buf[256];
- char symbol;
- void *addr;
- int i = 0;
-
- if (!f)
- return -ENOENT;
-
- while (!feof(f)) {
- if (!fgets(buf, sizeof(buf), f))
- break;
- if (sscanf(buf, "%p %c %s", &addr, &symbol, func) != 3)
- break;
- if (!addr)
- continue;
- syms[i].addr = (long) addr;
- syms[i].name = strdup(func);
- i++;
- }
- sym_cnt = i;
- qsort(syms, sym_cnt, sizeof(struct ksym), ksym_cmp);
- return 0;
-}
-
-struct ksym *ksym_search(long key)
-{
- int start = 0, end = sym_cnt;
- int result;
-
- while (start < end) {
- size_t mid = start + (end - start) / 2;
-
- result = key - syms[mid].addr;
- if (result < 0)
- end = mid;
- else if (result > 0)
- start = mid + 1;
- else
- return &syms[mid];
- }
-
- if (start >= 1 && syms[start - 1].addr < key &&
- key < syms[start].addr)
- /* valid ksym */
- return &syms[start - 1];
-
- /* out of range. return _stext */
- return &syms[0];
-}
-
diff --git a/samples/bpf/bpf_load.h b/samples/bpf/bpf_load.h
index 453c200..2c3d0b4 100644
--- a/samples/bpf/bpf_load.h
+++ b/samples/bpf/bpf_load.h
@@ -54,12 +54,5 @@ int load_bpf_file(char *path);
int load_bpf_file_fixup_map(const char *path, fixup_map_cb fixup_map);
void read_trace_pipe(void);
-struct ksym {
- long addr;
- char *name;
-};
-
-int load_kallsyms(void);
-struct ksym *ksym_search(long key);
int bpf_set_link_xdp_fd(int ifindex, int fd, __u32 flags);
#endif
diff --git a/samples/bpf/offwaketime_user.c b/samples/bpf/offwaketime_user.c
index 512f87a..f06063a 100644
--- a/samples/bpf/offwaketime_user.c
+++ b/samples/bpf/offwaketime_user.c
@@ -17,6 +17,7 @@
#include <sys/resource.h>
#include "libbpf.h"
#include "bpf_load.h"
+#include "trace_helpers.h"
#define PRINT_RAW_ADDR 0
diff --git a/samples/bpf/sampleip_user.c b/samples/bpf/sampleip_user.c
index 4ed690b..60c2b73 100644
--- a/samples/bpf/sampleip_user.c
+++ b/samples/bpf/sampleip_user.c
@@ -22,6 +22,7 @@
#include "libbpf.h"
#include "bpf_load.h"
#include "perf-sys.h"
+#include "trace_helpers.h"
#define DEFAULT_FREQ 99
#define DEFAULT_SECS 5
diff --git a/samples/bpf/spintest_user.c b/samples/bpf/spintest_user.c
index 3d73621..8d3e9cf 100644
--- a/samples/bpf/spintest_user.c
+++ b/samples/bpf/spintest_user.c
@@ -7,6 +7,7 @@
#include <sys/resource.h>
#include "libbpf.h"
#include "bpf_load.h"
+#include "trace_helpers.h"
int main(int ac, char **argv)
{
diff --git a/samples/bpf/trace_event_user.c b/samples/bpf/trace_event_user.c
index 56f7a25..1fa1bec 100644
--- a/samples/bpf/trace_event_user.c
+++ b/samples/bpf/trace_event_user.c
@@ -21,6 +21,7 @@
#include "libbpf.h"
#include "bpf_load.h"
#include "perf-sys.h"
+#include "trace_helpers.h"
#define SAMPLE_FREQ 50
diff --git a/samples/bpf/trace_output_user.c b/samples/bpf/trace_output_user.c
index ccca1e3..cc4b383 100644
--- a/samples/bpf/trace_output_user.c
+++ b/samples/bpf/trace_output_user.c
@@ -21,100 +21,10 @@
#include "libbpf.h"
#include "bpf_load.h"
#include "perf-sys.h"
+#include "trace_helpers.h"
static int pmu_fd;
-int page_size;
-int page_cnt = 8;
-volatile struct perf_event_mmap_page *header;
-
-typedef void (*print_fn)(void *data, int size);
-
-static int perf_event_mmap(int fd)
-{
- void *base;
- int mmap_size;
-
- page_size = getpagesize();
- mmap_size = page_size * (page_cnt + 1);
-
- base = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
- if (base == MAP_FAILED) {
- printf("mmap err\n");
- return -1;
- }
-
- header = base;
- return 0;
-}
-
-static int perf_event_poll(int fd)
-{
- struct pollfd pfd = { .fd = fd, .events = POLLIN };
-
- return poll(&pfd, 1, 1000);
-}
-
-struct perf_event_sample {
- struct perf_event_header header;
- __u32 size;
- char data[];
-};
-
-static void perf_event_read(print_fn fn)
-{
- __u64 data_tail = header->data_tail;
- __u64 data_head = header->data_head;
- __u64 buffer_size = page_cnt * page_size;
- void *base, *begin, *end;
- char buf[256];
-
- asm volatile("" ::: "memory"); /* in real code it should be smp_rmb() */
- if (data_head == data_tail)
- return;
-
- base = ((char *)header) + page_size;
-
- begin = base + data_tail % buffer_size;
- end = base + data_head % buffer_size;
-
- while (begin != end) {
- struct perf_event_sample *e;
-
- e = begin;
- if (begin + e->header.size > base + buffer_size) {
- long len = base + buffer_size - begin;
-
- assert(len < e->header.size);
- memcpy(buf, begin, len);
- memcpy(buf + len, base, e->header.size - len);
- e = (void *) buf;
- begin = base + e->header.size - len;
- } else if (begin + e->header.size == base + buffer_size) {
- begin = base;
- } else {
- begin += e->header.size;
- }
-
- if (e->header.type == PERF_RECORD_SAMPLE) {
- fn(e->data, e->size);
- } else if (e->header.type == PERF_RECORD_LOST) {
- struct {
- struct perf_event_header header;
- __u64 id;
- __u64 lost;
- } *lost = (void *) e;
- printf("lost %lld events\n", lost->lost);
- } else {
- printf("unknown event type=%d size=%d\n",
- e->header.type, e->header.size);
- }
- }
-
- __sync_synchronize(); /* smp_mb() */
- header->data_tail = data_head;
-}
-
static __u64 time_get_ns(void)
{
struct timespec ts;
@@ -127,7 +37,7 @@ static __u64 start_time;
#define MAX_CNT 100000ll
-static void print_bpf_output(void *data, int size)
+static int print_bpf_output(void *data, int size)
{
static __u64 cnt;
struct {
@@ -138,7 +48,7 @@ static void print_bpf_output(void *data, int size)
if (e->cookie != 0x12345678) {
printf("BUG pid %llx cookie %llx sized %d\n",
e->pid, e->cookie, size);
- kill(0, SIGINT);
+ return PERF_EVENT_ERROR;
}
cnt++;
@@ -146,8 +56,10 @@ static void print_bpf_output(void *data, int size)
if (cnt == MAX_CNT) {
printf("recv %lld events per sec\n",
MAX_CNT * 1000000000ll / (time_get_ns() - start_time));
- kill(0, SIGINT);
+ return PERF_EVENT_DONE;
}
+
+ return PERF_EVENT_CONT;
}
static void test_bpf_perf_event(void)
@@ -166,10 +78,18 @@ static void test_bpf_perf_event(void)
ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
}
+static void exec_action(void)
+{
+ FILE *f;
+
+ f = popen("taskset 1 dd if=/dev/zero of=/dev/null", "r");
+ (void) f;
+}
+
int main(int argc, char **argv)
{
char filename[256];
- FILE *f;
+ int ret;
snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
@@ -180,17 +100,8 @@ int main(int argc, char **argv)
test_bpf_perf_event();
- if (perf_event_mmap(pmu_fd) < 0)
- return 1;
-
- f = popen("taskset 1 dd if=/dev/zero of=/dev/null", "r");
- (void) f;
-
start_time = time_get_ns();
- for (;;) {
- perf_event_poll(pmu_fd);
- perf_event_read(print_bpf_output);
- }
-
- return 0;
+ ret = perf_event_poller(pmu_fd, exec_action, print_bpf_output);
+ kill(0, SIGINT);
+ return ret;
}
diff --git a/tools/testing/selftests/bpf/trace_helpers.c b/tools/testing/selftests/bpf/trace_helpers.c
new file mode 100644
index 0000000..00954e3
--- /dev/null
+++ b/tools/testing/selftests/bpf/trace_helpers.c
@@ -0,0 +1,186 @@
+// SPDX-License-Identifier: GPL-2.0
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <assert.h>
+#include <errno.h>
+#include <poll.h>
+#include <unistd.h>
+#include <linux/perf_event.h>
+#include <sys/mman.h>
+#include "trace_helpers.h"
+
+#define MAX_SYMS 300000
+static struct ksym syms[MAX_SYMS];
+static int sym_cnt;
+
+static int ksym_cmp(const void *p1, const void *p2)
+{
+ return ((struct ksym *)p1)->addr - ((struct ksym *)p2)->addr;
+}
+
+int load_kallsyms(void)
+{
+ FILE *f = fopen("/proc/kallsyms", "r");
+ char func[256], buf[256];
+ char symbol;
+ void *addr;
+ int i = 0;
+
+ if (!f)
+ return -ENOENT;
+
+ while (!feof(f)) {
+ if (!fgets(buf, sizeof(buf), f))
+ break;
+ if (sscanf(buf, "%p %c %s", &addr, &symbol, func) != 3)
+ break;
+ if (!addr)
+ continue;
+ syms[i].addr = (long) addr;
+ syms[i].name = strdup(func);
+ i++;
+ }
+ sym_cnt = i;
+ qsort(syms, sym_cnt, sizeof(struct ksym), ksym_cmp);
+ return 0;
+}
+
+struct ksym *ksym_search(long key)
+{
+ int start = 0, end = sym_cnt;
+ int result;
+
+ while (start < end) {
+ size_t mid = start + (end - start) / 2;
+
+ result = key - syms[mid].addr;
+ if (result < 0)
+ end = mid;
+ else if (result > 0)
+ start = mid + 1;
+ else
+ return &syms[mid];
+ }
+
+ if (start >= 1 && syms[start - 1].addr < key &&
+ key < syms[start].addr)
+ /* valid ksym */
+ return &syms[start - 1];
+
+ /* out of range. return _stext */
+ return &syms[0];
+}
+
+static int page_size;
+static int page_cnt = 8;
+static volatile struct perf_event_mmap_page *header;
+
+static int perf_event_mmap(int fd)
+{
+ void *base;
+ int mmap_size;
+
+ page_size = getpagesize();
+ mmap_size = page_size * (page_cnt + 1);
+
+ base = mmap(NULL, mmap_size, PROT_READ | PROT_WRITE, MAP_SHARED, fd, 0);
+ if (base == MAP_FAILED) {
+ printf("mmap err\n");
+ return -1;
+ }
+
+ header = base;
+ return 0;
+}
+
+static int perf_event_poll(int fd)
+{
+ struct pollfd pfd = { .fd = fd, .events = POLLIN };
+
+ return poll(&pfd, 1, 1000);
+}
+
+struct perf_event_sample {
+ struct perf_event_header header;
+ __u32 size;
+ char data[];
+};
+
+static int perf_event_read(perf_event_print_fn fn)
+{
+ __u64 data_tail = header->data_tail;
+ __u64 data_head = header->data_head;
+ __u64 buffer_size = page_cnt * page_size;
+ void *base, *begin, *end;
+ char buf[256];
+ int ret;
+
+ asm volatile("" ::: "memory"); /* in real code it should be smp_rmb() */
+ if (data_head == data_tail)
+ return PERF_EVENT_CONT;
+
+ base = ((char *)header) + page_size;
+
+ begin = base + data_tail % buffer_size;
+ end = base + data_head % buffer_size;
+
+ while (begin != end) {
+ struct perf_event_sample *e;
+
+ e = begin;
+ if (begin + e->header.size > base + buffer_size) {
+ long len = base + buffer_size - begin;
+
+ assert(len < e->header.size);
+ memcpy(buf, begin, len);
+ memcpy(buf + len, base, e->header.size - len);
+ e = (void *) buf;
+ begin = base + e->header.size - len;
+ } else if (begin + e->header.size == base + buffer_size) {
+ begin = base;
+ } else {
+ begin += e->header.size;
+ }
+
+ if (e->header.type == PERF_RECORD_SAMPLE) {
+ ret = fn(e->data, e->size);
+ if (ret != PERF_EVENT_CONT)
+ return ret;
+ } else if (e->header.type == PERF_RECORD_LOST) {
+ struct {
+ struct perf_event_header header;
+ __u64 id;
+ __u64 lost;
+ } *lost = (void *) e;
+ printf("lost %lld events\n", lost->lost);
+ } else {
+ printf("unknown event type=%d size=%d\n",
+ e->header.type, e->header.size);
+ }
+ }
+
+ __sync_synchronize(); /* smp_mb() */
+ header->data_tail = data_head;
+ return PERF_EVENT_CONT;
+}
+
+int perf_event_poller(int fd, perf_event_exec_fn exec_fn,
+ perf_event_print_fn output_fn)
+{
+ int ret;
+
+ if (perf_event_mmap(fd) < 0)
+ return PERF_EVENT_ERROR;
+
+ exec_fn();
+
+ for (;;) {
+ perf_event_poll(fd);
+ ret = perf_event_read(output_fn);
+ if (ret != PERF_EVENT_CONT)
+ return ret;
+ }
+
+ return PERF_EVENT_DONE;
+}
diff --git a/tools/testing/selftests/bpf/trace_helpers.h b/tools/testing/selftests/bpf/trace_helpers.h
new file mode 100644
index 0000000..8750778
--- /dev/null
+++ b/tools/testing/selftests/bpf/trace_helpers.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __TRACE_HELPER_H
+#define __TRACE_HELPER_H
+
+struct ksym {
+ long addr;
+ char *name;
+};
+
+int load_kallsyms(void);
+struct ksym *ksym_search(long key);
+
+typedef void (*perf_event_exec_fn)(void);
+typedef int (*perf_event_print_fn)(void *data, int size);
+
+/* return code for perf_event_print_fn */
+#define PERF_EVENT_DONE 0
+#define PERF_EVENT_ERROR 1
+#define PERF_EVENT_CONT 2
+
+/* return PERF_EVENT_DONE or PERF_EVENT_ERROR */
+int perf_event_poller(int fd, perf_event_exec_fn exec_fn,
+ perf_event_print_fn output_fn);
+#endif
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next v7 03/10] bpf/verifier: refine retval R0 state for bpf_get_stack helper
From: Yonghong Song @ 2018-04-25 19:29 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20180425192910.556352-1-yhs@fb.com>
The special property of return values for helpers bpf_get_stack
and bpf_probe_read_str are captured in verifier.
Both helpers return a negative error code or
a length, which is equal to or smaller than the buffer
size argument. This additional information in the
verifier can avoid the condition such as "retval > bufsize"
in the bpf program. For example, for the code blow,
usize = bpf_get_stack(ctx, raw_data, max_len, BPF_F_USER_STACK);
if (usize < 0 || usize > max_len)
return 0;
The verifier may have the following errors:
52: (85) call bpf_get_stack#65
R0=map_value(id=0,off=0,ks=4,vs=1600,imm=0) R1_w=ctx(id=0,off=0,imm=0)
R2_w=map_value(id=0,off=0,ks=4,vs=1600,imm=0) R3_w=inv800 R4_w=inv256
R6=ctx(id=0,off=0,imm=0) R7=map_value(id=0,off=0,ks=4,vs=1600,imm=0)
R9_w=inv800 R10=fp0,call_-1
53: (bf) r8 = r0
54: (bf) r1 = r8
55: (67) r1 <<= 32
56: (bf) r2 = r1
57: (77) r2 >>= 32
58: (25) if r2 > 0x31f goto pc+33
R0=inv(id=0) R1=inv(id=0,smax_value=9223372032559808512,
umax_value=18446744069414584320,
var_off=(0x0; 0xffffffff00000000))
R2=inv(id=0,umax_value=799,var_off=(0x0; 0x3ff))
R6=ctx(id=0,off=0,imm=0) R7=map_value(id=0,off=0,ks=4,vs=1600,imm=0)
R8=inv(id=0) R9=inv800 R10=fp0,call_-1
59: (1f) r9 -= r8
60: (c7) r1 s>>= 32
61: (bf) r2 = r7
62: (0f) r2 += r1
math between map_value pointer and register with unbounded
min value is not allowed
The failure is due to llvm compiler optimization where register "r2",
which is a copy of "r1", is tested for condition while later on "r1"
is used for map_ptr operation. The verifier is not able to track such
inst sequence effectively.
Without the "usize > max_len" condition, there is no llvm optimization
and the below generated code passed verifier:
52: (85) call bpf_get_stack#65
R0=map_value(id=0,off=0,ks=4,vs=1600,imm=0) R1_w=ctx(id=0,off=0,imm=0)
R2_w=map_value(id=0,off=0,ks=4,vs=1600,imm=0) R3_w=inv800 R4_w=inv256
R6=ctx(id=0,off=0,imm=0) R7=map_value(id=0,off=0,ks=4,vs=1600,imm=0)
R9_w=inv800 R10=fp0,call_-1
53: (b7) r1 = 0
54: (bf) r8 = r0
55: (67) r8 <<= 32
56: (c7) r8 s>>= 32
57: (6d) if r1 s> r8 goto pc+24
R0=inv(id=0,umax_value=800,var_off=(0x0; 0x3ff))
R1=inv0 R6=ctx(id=0,off=0,imm=0)
R7=map_value(id=0,off=0,ks=4,vs=1600,imm=0)
R8=inv(id=0,umax_value=800,var_off=(0x0; 0x3ff)) R9=inv800
R10=fp0,call_-1
58: (bf) r2 = r7
59: (0f) r2 += r8
60: (1f) r9 -= r8
61: (bf) r1 = r6
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/verifier.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 253f6bd..988400e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -165,6 +165,8 @@ struct bpf_call_arg_meta {
bool pkt_access;
int regno;
int access_size;
+ s64 msize_smax_value;
+ u64 msize_umax_value;
};
static DEFINE_MUTEX(bpf_verifier_lock);
@@ -1985,6 +1987,12 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
} else if (arg_type_is_mem_size(arg_type)) {
bool zero_size_allowed = (arg_type == ARG_CONST_SIZE_OR_ZERO);
+ /* remember the mem_size which may be used later
+ * to refine return values.
+ */
+ meta->msize_smax_value = reg->smax_value;
+ meta->msize_umax_value = reg->umax_value;
+
/* The register is SCALAR_VALUE; the access check
* happens using its boundaries.
*/
@@ -2324,6 +2332,23 @@ static int prepare_func_exit(struct bpf_verifier_env *env, int *insn_idx)
return 0;
}
+static void do_refine_retval_range(struct bpf_reg_state *regs, int ret_type,
+ int func_id,
+ struct bpf_call_arg_meta *meta)
+{
+ struct bpf_reg_state *ret_reg = ®s[BPF_REG_0];
+
+ if (ret_type != RET_INTEGER ||
+ (func_id != BPF_FUNC_get_stack &&
+ func_id != BPF_FUNC_probe_read_str))
+ return;
+
+ ret_reg->smax_value = meta->msize_smax_value;
+ ret_reg->umax_value = meta->msize_umax_value;
+ __reg_deduce_bounds(ret_reg);
+ __reg_bound_offset(ret_reg);
+}
+
static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn_idx)
{
const struct bpf_func_proto *fn = NULL;
@@ -2447,6 +2472,8 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
return -EINVAL;
}
+ do_refine_retval_range(regs, fn->ret_type, func_id, &meta);
+
err = check_map_func_compatibility(env, meta.map_ptr, func_id);
if (err)
return err;
--
2.9.5
^ permalink raw reply related
* [PATCH bpf-next v7 04/10] bpf: remove never-hit branches in verifier adjust_scalar_min_max_vals
From: Yonghong Song @ 2018-04-25 19:29 UTC (permalink / raw)
To: ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20180425192910.556352-1-yhs@fb.com>
In verifier function adjust_scalar_min_max_vals,
when src_known is false and the opcode is BPF_LSH/BPF_RSH,
early return will happen in the function. So remove
the branch in handling BPF_LSH/BPF_RSH when src_known is false.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
kernel/bpf/verifier.c | 11 ++---------
1 file changed, 2 insertions(+), 9 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 988400e..6e3f859 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -2940,10 +2940,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
dst_reg->umin_value <<= umin_val;
dst_reg->umax_value <<= umax_val;
}
- if (src_known)
- dst_reg->var_off = tnum_lshift(dst_reg->var_off, umin_val);
- else
- dst_reg->var_off = tnum_lshift(tnum_unknown, umin_val);
+ dst_reg->var_off = tnum_lshift(dst_reg->var_off, umin_val);
/* We may learn something more from the var_off */
__update_reg_bounds(dst_reg);
break;
@@ -2971,11 +2968,7 @@ static int adjust_scalar_min_max_vals(struct bpf_verifier_env *env,
*/
dst_reg->smin_value = S64_MIN;
dst_reg->smax_value = S64_MAX;
- if (src_known)
- dst_reg->var_off = tnum_rshift(dst_reg->var_off,
- umin_val);
- else
- dst_reg->var_off = tnum_rshift(tnum_unknown, umin_val);
+ dst_reg->var_off = tnum_rshift(dst_reg->var_off, umin_val);
dst_reg->umin_value >>= umax_val;
dst_reg->umax_value >>= umin_val;
/* We may learn something more from the var_off */
--
2.9.5
^ 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