* Re: [PATCH 3/5] virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.
From: Rusty Russell @ 2015-02-11 0:06 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: lkml, netdev@vger.kernel.org
In-Reply-To: <87fvag3qv3.fsf@rustcorp.com.au>
Rusty Russell <rusty@rustcorp.com.au> writes:
> "Michael S. Tsirkin" <mst@redhat.com> writes:
>> On Fri, Feb 06, 2015 at 03:36:54PM +1030, Rusty Russell wrote:
>>> In particular, the virtio header always has the u16 num_buffers field.
>>> We define a new 'struct virtio_net_modern_hdr' for this (rather than
>>> simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
>>> if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
>>>
>>> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>>> ---
>>> include/uapi/linux/virtio_net.h | 30 ++++++++++++++++++++++++++++--
>>> 1 file changed, 28 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
>>> index b5f1677b291c..32754f3000e8 100644
>>> --- a/include/uapi/linux/virtio_net.h
>>> +++ b/include/uapi/linux/virtio_net.h
>>> @@ -35,7 +35,6 @@
>>> #define VIRTIO_NET_F_CSUM 0 /* Host handles pkts w/ partial csum */
>>> #define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */
>>> #define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */
>>> -#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
>>> #define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */
>>> #define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */
>>> #define VIRTIO_NET_F_GUEST_ECN 9 /* Guest can handle TSO[6] w/ ECN in. */
>>> @@ -56,6 +55,10 @@
>>> * Steering */
>>> #define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
>>>
>>> +#ifndef VIRTIO_NET_NO_LEGACY
>>> +#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
>>> +#endif /* VIRTIO_NET_NO_LEGACY */
>>> +
>>> #define VIRTIO_NET_S_LINK_UP 1 /* Link is up */
>>> #define VIRTIO_NET_S_ANNOUNCE 2 /* Announcement is needed */
>>>
>>> @@ -71,8 +74,9 @@ struct virtio_net_config {
>>> __u16 max_virtqueue_pairs;
>>> } __attribute__((packed));
>>>
>>> +#ifndef VIRTIO_NET_NO_LEGACY
>>> /* This header comes first in the scatter-gather list.
>>> - * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
>>> + * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
>>> * be the first element of the scatter-gather list. If you don't
>>> * specify GSO or CSUM features, you can simply ignore the header. */
>>> struct virtio_net_hdr {
>>> @@ -97,6 +101,28 @@ struct virtio_net_hdr_mrg_rxbuf {
>>> struct virtio_net_hdr hdr;
>>> __virtio16 num_buffers; /* Number of merged rx buffers */
>>> };
>>> +#else /* ... VIRTIO_NET_NO_LEGACY */
>>> +/*
>>> + * This header comes first in the scatter-gather list. If you don't
>>> + * specify GSO or CSUM features, you can simply ignore the header.
>>> + */
>>> +struct virtio_net_modern_hdr {
>>> +#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */
>>> +#define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */
>>> + __u8 flags;
>>> +#define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */
>>> +#define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */
>>> +#define VIRTIO_NET_HDR_GSO_UDP 3 /* GSO frame, IPv4 UDP (UFO) */
>>> +#define VIRTIO_NET_HDR_GSO_TCPV6 4 /* GSO frame, IPv6 TCP */
>>> +#define VIRTIO_NET_HDR_GSO_ECN 0x80 /* TCP has ECN set */
>>> + __u8 gso_type;
>>> + __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
>>> + __virtio16 gso_size; /* Bytes to append to hdr_len per frame */
>>> + __virtio16 csum_start; /* Position to start checksumming from */
>>> + __virtio16 csum_offset; /* Offset after that to place checksum */
>>> + __virtio16 num_buffers; /* Number of merged rx buffers */
>>> +};
>>> +#endif /* ...VIRTIO_NET_NO_LEGACY */
>>
>> This kind of masks the fact that it's the same as
>> virtio_net_hdr_mrg_rxbuf. So it's forcing people to duplicate
>> code for transitional devices.
>
> It's a hard problem to solve; someone suffers. My preference is to make
> non-legacy implementations as "clean" as possible, even if transitional
> devices suffer (since their suffering should be shorter-lived).
>
> Transitional devices can't define VIRTIO_NET_NO_LEGACY, so they'll
> have to use virtio_net_hdr_mrg_rxbuf anyway.
Here's what I ended up with. In effect, it's just the name change
and a comment that it's the same as the legacy virtio_net_hdr_mrg_rxbuf:
Subject: virtio: Don't expose legacy net features when VIRTIO_NET_NO_LEGACY defined.
In particular, the virtio header always has the u16 num_buffers field.
We define a new 'struct virtio_net_hdr_v1' for this (rather than
simply calling it 'struct virtio_net_hdr', to avoid nasty type errors
if some parts of a project define VIRTIO_NET_NO_LEGACY and some don't.
Transitional devices (which can't define VIRTIO_NET_NO_LEGACY) will
have to keep using struct virtio_net_hdr_mrg_rxbuf, which has the same
byte layout as struct virtio_net_hdr_v1.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index b5f1677b291c..4a9b58113d6e 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -35,7 +35,6 @@
#define VIRTIO_NET_F_CSUM 0 /* Host handles pkts w/ partial csum */
#define VIRTIO_NET_F_GUEST_CSUM 1 /* Guest handles pkts w/ partial csum */
#define VIRTIO_NET_F_MAC 5 /* Host has given MAC address. */
-#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
#define VIRTIO_NET_F_GUEST_TSO4 7 /* Guest can handle TSOv4 in. */
#define VIRTIO_NET_F_GUEST_TSO6 8 /* Guest can handle TSOv6 in. */
#define VIRTIO_NET_F_GUEST_ECN 9 /* Guest can handle TSO[6] w/ ECN in. */
@@ -56,6 +55,10 @@
* Steering */
#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
+#ifndef VIRTIO_NET_NO_LEGACY
+#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
+#endif /* VIRTIO_NET_NO_LEGACY */
+
#define VIRTIO_NET_S_LINK_UP 1 /* Link is up */
#define VIRTIO_NET_S_ANNOUNCE 2 /* Announcement is needed */
@@ -71,8 +74,9 @@ struct virtio_net_config {
__u16 max_virtqueue_pairs;
} __attribute__((packed));
+#ifndef VIRTIO_NET_NO_LEGACY
/* This header comes first in the scatter-gather list.
- * If VIRTIO_F_ANY_LAYOUT is not negotiated, it must
+ * For legacy virtio, if VIRTIO_F_ANY_LAYOUT is not negotiated, it must
* be the first element of the scatter-gather list. If you don't
* specify GSO or CSUM features, you can simply ignore the header. */
struct virtio_net_hdr {
@@ -97,6 +101,30 @@ struct virtio_net_hdr_mrg_rxbuf {
struct virtio_net_hdr hdr;
__virtio16 num_buffers; /* Number of merged rx buffers */
};
+#else /* ... VIRTIO_NET_NO_LEGACY */
+/*
+ * This header comes first in the scatter-gather list. If you don't
+ * specify GSO or CSUM features, you can simply ignore the header.
+ *
+ * This is bitwise-equivalent to the legacy struct virtio_net_hdr_mrg_rxbuf.
+ */
+struct virtio_net_hdr_v1 {
+#define VIRTIO_NET_HDR_F_NEEDS_CSUM 1 /* Use csum_start, csum_offset */
+#define VIRTIO_NET_HDR_F_DATA_VALID 2 /* Csum is valid */
+ __u8 flags;
+#define VIRTIO_NET_HDR_GSO_NONE 0 /* Not a GSO frame */
+#define VIRTIO_NET_HDR_GSO_TCPV4 1 /* GSO frame, IPv4 TCP (TSO) */
+#define VIRTIO_NET_HDR_GSO_UDP 3 /* GSO frame, IPv4 UDP (UFO) */
+#define VIRTIO_NET_HDR_GSO_TCPV6 4 /* GSO frame, IPv6 TCP */
+#define VIRTIO_NET_HDR_GSO_ECN 0x80 /* TCP has ECN set */
+ __u8 gso_type;
+ __virtio16 hdr_len; /* Ethernet + IP + tcp/udp hdrs */
+ __virtio16 gso_size; /* Bytes to append to hdr_len per frame */
+ __virtio16 csum_start; /* Position to start checksumming from */
+ __virtio16 csum_offset; /* Offset after that to place checksum */
+ __virtio16 num_buffers; /* Number of merged rx buffers */
+};
+#endif /* ...VIRTIO_NET_NO_LEGACY */
/*
* Control virtqueue data structures
^ permalink raw reply related
* [RFC PATCH net-next 0/7] eBPF support for cls_bpf
From: Daniel Borkmann @ 2015-02-11 0:15 UTC (permalink / raw)
To: jiri; +Cc: ast, netdev, Daniel Borkmann
I'm sending this out only as RFC as the merge window is open
anyway and Dave's pull request pending.
My plan would be to get this work fully refined and remove
some rough edges until net-next opens up again, submitting
it as non-RFC.
So for the time being similarly as the recent OVS eBPF action
patchset this may serve as a discussion ground, also wrt to
the BPF tutorial at netdev01 [1].
I presume on top of this e.g. Jiri might want to follow-up
with act_bpf, I'm also open/happy to contribute to it.
The series starts with a couple of cleanups and making the
prog type infrastructure pluggable in eBPF. Most interesting
is probably the last patch that adds actual support and the
iproute2 bits from the link below.
With regards to accessing fields from the context (here: skb),
I leave that for future work just as we currently do in socket
filters, also with regard to the currently ongoing ABI discussion
from tracing side. Nevertheless, the state with these patches
still allows for interesting functionality to be implemented as
complex classifiers, e.g. such as a fully fledged C-like flow
dissector as shown in bpf samples directory and the like.
iproute2 part:
http://git.breakpoint.cc/cgit/dborkman/iproute2.git/log/?h=ebpf
I have configured and built LLVM with: --enable-experimental-targets=BPF
[1] https://www.netdev01.org/sessions/15
Thanks !
Daniel Borkmann (7):
ebpf: remove kernel test stubs
ebpf: constify various function pointer structs
ebpf: check first for MAXINSNS in bpf_prog_load
ebpf: extend program type/subsystem registration
ebpf: export BPF_PSEUDO_MAP_FD to uapi
ebpf: remove CONFIG_BPF_SYSCALL ifdefs in socket filter code
cls_bpf: add initial eBPF support for programmable classifiers
include/linux/bpf.h | 46 +++++++---
include/linux/filter.h | 2 -
include/uapi/linux/bpf.h | 3 +
include/uapi/linux/pkt_cls.h | 1 +
kernel/bpf/Makefile | 3 -
kernel/bpf/arraymap.c | 6 +-
kernel/bpf/hashtab.c | 6 +-
kernel/bpf/helpers.c | 9 +-
kernel/bpf/syscall.c | 96 +++++++++++++++------
kernel/bpf/test_stub.c | 78 -----------------
kernel/bpf/verifier.c | 28 ++++--
net/core/filter.c | 169 +++++++++++++++++-------------------
net/sched/cls_bpf.c | 200 ++++++++++++++++++++++++++++++++-----------
samples/bpf/libbpf.h | 4 +-
samples/bpf/test_verifier.c | 5 +-
15 files changed, 374 insertions(+), 282 deletions(-)
delete mode 100644 kernel/bpf/test_stub.c
--
1.9.3
^ permalink raw reply
* [PATCH net-next 1/7] ebpf: remove kernel test stubs
From: Daniel Borkmann @ 2015-02-11 0:15 UTC (permalink / raw)
To: jiri; +Cc: ast, netdev, Daniel Borkmann
In-Reply-To: <cover.1423610452.git.daniel@iogearbox.net>
Now that we have BPF_PROG_TYPE_SOCKET_FILTER up and running,
we can remove the test stubs which were added to get the
verifier suite up. We can just let the test cases probe under
socket filter type instead. In the fill/spill test case, we
cannot (yet) access fields from the context (skb), but we may
adapt that test case in future.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
kernel/bpf/Makefile | 3 --
kernel/bpf/test_stub.c | 78 ---------------------------------------------
samples/bpf/test_verifier.c | 5 +--
3 files changed, 3 insertions(+), 83 deletions(-)
delete mode 100644 kernel/bpf/test_stub.c
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index a5ae60f..e6983be 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -1,5 +1,2 @@
obj-y := core.o
obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o hashtab.o arraymap.o helpers.o
-ifdef CONFIG_TEST_BPF
-obj-$(CONFIG_BPF_SYSCALL) += test_stub.o
-endif
diff --git a/kernel/bpf/test_stub.c b/kernel/bpf/test_stub.c
deleted file mode 100644
index 0ceae1e..0000000
--- a/kernel/bpf/test_stub.c
+++ /dev/null
@@ -1,78 +0,0 @@
-/* Copyright (c) 2011-2014 PLUMgrid, http://plumgrid.com
- *
- * This program is free software; you can redistribute it and/or
- * modify it under the terms of version 2 of the GNU General Public
- * License as published by the Free Software Foundation.
- */
-#include <linux/kernel.h>
-#include <linux/types.h>
-#include <linux/slab.h>
-#include <linux/err.h>
-#include <linux/bpf.h>
-
-/* test stubs for BPF_MAP_TYPE_UNSPEC and for BPF_PROG_TYPE_UNSPEC
- * to be used by user space verifier testsuite
- */
-struct bpf_context {
- u64 arg1;
- u64 arg2;
-};
-
-static const struct bpf_func_proto *test_func_proto(enum bpf_func_id func_id)
-{
- switch (func_id) {
- case BPF_FUNC_map_lookup_elem:
- return &bpf_map_lookup_elem_proto;
- case BPF_FUNC_map_update_elem:
- return &bpf_map_update_elem_proto;
- case BPF_FUNC_map_delete_elem:
- return &bpf_map_delete_elem_proto;
- default:
- return NULL;
- }
-}
-
-static const struct bpf_context_access {
- int size;
- enum bpf_access_type type;
-} test_ctx_access[] = {
- [offsetof(struct bpf_context, arg1)] = {
- FIELD_SIZEOF(struct bpf_context, arg1),
- BPF_READ
- },
- [offsetof(struct bpf_context, arg2)] = {
- FIELD_SIZEOF(struct bpf_context, arg2),
- BPF_READ
- },
-};
-
-static bool test_is_valid_access(int off, int size, enum bpf_access_type type)
-{
- const struct bpf_context_access *access;
-
- if (off < 0 || off >= ARRAY_SIZE(test_ctx_access))
- return false;
-
- access = &test_ctx_access[off];
- if (access->size == size && (access->type & type))
- return true;
-
- return false;
-}
-
-static struct bpf_verifier_ops test_ops = {
- .get_func_proto = test_func_proto,
- .is_valid_access = test_is_valid_access,
-};
-
-static struct bpf_prog_type_list tl_prog = {
- .ops = &test_ops,
- .type = BPF_PROG_TYPE_UNSPEC,
-};
-
-static int __init register_test_ops(void)
-{
- bpf_register_prog_type(&tl_prog);
- return 0;
-}
-late_initcall(register_test_ops);
diff --git a/samples/bpf/test_verifier.c b/samples/bpf/test_verifier.c
index b96175e..7b56b59 100644
--- a/samples/bpf/test_verifier.c
+++ b/samples/bpf/test_verifier.c
@@ -288,7 +288,8 @@ static struct bpf_test tests[] = {
BPF_LDX_MEM(BPF_DW, BPF_REG_2, BPF_REG_10, -8),
/* should be able to access R0 = *(R2 + 8) */
- BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_2, 8),
+ /* BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_2, 8), */
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
BPF_EXIT_INSN(),
},
.result = ACCEPT,
@@ -687,7 +688,7 @@ static int test(void)
}
printf("#%d %s ", i, tests[i].descr);
- prog_fd = bpf_prog_load(BPF_PROG_TYPE_UNSPEC, prog,
+ prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, prog,
prog_len * sizeof(struct bpf_insn),
"GPL");
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 2/7] ebpf: constify various function pointer structs
From: Daniel Borkmann @ 2015-02-11 0:15 UTC (permalink / raw)
To: jiri; +Cc: ast, netdev, Daniel Borkmann
In-Reply-To: <cover.1423610452.git.daniel@iogearbox.net>
We can move bpf_map_ops and bpf_verifier_ops and other structs
into RO section, bpf_map_type_list and bpf_prog_type_list into
read mostly.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/bpf.h | 14 +++++++-------
kernel/bpf/arraymap.c | 6 +++---
kernel/bpf/hashtab.c | 6 +++---
kernel/bpf/helpers.c | 6 +++---
net/core/filter.c | 6 +++---
5 files changed, 19 insertions(+), 19 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index bbfceb7..7844686 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -32,13 +32,13 @@ struct bpf_map {
u32 key_size;
u32 value_size;
u32 max_entries;
- struct bpf_map_ops *ops;
+ const struct bpf_map_ops *ops;
struct work_struct work;
};
struct bpf_map_type_list {
struct list_head list_node;
- struct bpf_map_ops *ops;
+ const struct bpf_map_ops *ops;
enum bpf_map_type type;
};
@@ -109,7 +109,7 @@ struct bpf_verifier_ops {
struct bpf_prog_type_list {
struct list_head list_node;
- struct bpf_verifier_ops *ops;
+ const struct bpf_verifier_ops *ops;
enum bpf_prog_type type;
};
@@ -121,7 +121,7 @@ struct bpf_prog_aux {
atomic_t refcnt;
bool is_gpl_compatible;
enum bpf_prog_type prog_type;
- struct bpf_verifier_ops *ops;
+ const struct bpf_verifier_ops *ops;
struct bpf_map **used_maps;
u32 used_map_cnt;
struct bpf_prog *prog;
@@ -138,8 +138,8 @@ struct bpf_prog *bpf_prog_get(u32 ufd);
int bpf_check(struct bpf_prog *fp, union bpf_attr *attr);
/* verifier prototypes for helper functions called from eBPF programs */
-extern struct bpf_func_proto bpf_map_lookup_elem_proto;
-extern struct bpf_func_proto bpf_map_update_elem_proto;
-extern struct bpf_func_proto bpf_map_delete_elem_proto;
+extern const struct bpf_func_proto bpf_map_lookup_elem_proto;
+extern const struct bpf_func_proto bpf_map_update_elem_proto;
+extern const struct bpf_func_proto bpf_map_delete_elem_proto;
#endif /* _LINUX_BPF_H */
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 9eb4d8a..8a66165 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -134,7 +134,7 @@ static void array_map_free(struct bpf_map *map)
kvfree(array);
}
-static struct bpf_map_ops array_ops = {
+static const struct bpf_map_ops array_ops = {
.map_alloc = array_map_alloc,
.map_free = array_map_free,
.map_get_next_key = array_map_get_next_key,
@@ -143,14 +143,14 @@ static struct bpf_map_ops array_ops = {
.map_delete_elem = array_map_delete_elem,
};
-static struct bpf_map_type_list tl = {
+static struct bpf_map_type_list array_type __read_mostly = {
.ops = &array_ops,
.type = BPF_MAP_TYPE_ARRAY,
};
static int __init register_array_map(void)
{
- bpf_register_map_type(&tl);
+ bpf_register_map_type(&array_type);
return 0;
}
late_initcall(register_array_map);
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index b3ba436..83c209d 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -345,7 +345,7 @@ static void htab_map_free(struct bpf_map *map)
kfree(htab);
}
-static struct bpf_map_ops htab_ops = {
+static const struct bpf_map_ops htab_ops = {
.map_alloc = htab_map_alloc,
.map_free = htab_map_free,
.map_get_next_key = htab_map_get_next_key,
@@ -354,14 +354,14 @@ static struct bpf_map_ops htab_ops = {
.map_delete_elem = htab_map_delete_elem,
};
-static struct bpf_map_type_list tl = {
+static struct bpf_map_type_list htab_type __read_mostly = {
.ops = &htab_ops,
.type = BPF_MAP_TYPE_HASH,
};
static int __init register_htab_map(void)
{
- bpf_register_map_type(&tl);
+ bpf_register_map_type(&htab_type);
return 0;
}
late_initcall(register_htab_map);
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 9e3414d..a3c7701 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -41,7 +41,7 @@ static u64 bpf_map_lookup_elem(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
return (unsigned long) value;
}
-struct bpf_func_proto bpf_map_lookup_elem_proto = {
+const struct bpf_func_proto bpf_map_lookup_elem_proto = {
.func = bpf_map_lookup_elem,
.gpl_only = false,
.ret_type = RET_PTR_TO_MAP_VALUE_OR_NULL,
@@ -60,7 +60,7 @@ static u64 bpf_map_update_elem(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
return map->ops->map_update_elem(map, key, value, r4);
}
-struct bpf_func_proto bpf_map_update_elem_proto = {
+const struct bpf_func_proto bpf_map_update_elem_proto = {
.func = bpf_map_update_elem,
.gpl_only = false,
.ret_type = RET_INTEGER,
@@ -80,7 +80,7 @@ static u64 bpf_map_delete_elem(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
return map->ops->map_delete_elem(map, key);
}
-struct bpf_func_proto bpf_map_delete_elem_proto = {
+const struct bpf_func_proto bpf_map_delete_elem_proto = {
.func = bpf_map_delete_elem,
.gpl_only = false,
.ret_type = RET_INTEGER,
diff --git a/net/core/filter.c b/net/core/filter.c
index ec9baea..e823da5 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1159,19 +1159,19 @@ static bool sock_filter_is_valid_access(int off, int size, enum bpf_access_type
return false;
}
-static struct bpf_verifier_ops sock_filter_ops = {
+static const struct bpf_verifier_ops sock_filter_ops = {
.get_func_proto = sock_filter_func_proto,
.is_valid_access = sock_filter_is_valid_access,
};
-static struct bpf_prog_type_list tl = {
+static struct bpf_prog_type_list sock_filter_type __read_mostly = {
.ops = &sock_filter_ops,
.type = BPF_PROG_TYPE_SOCKET_FILTER,
};
static int __init register_sock_filter_ops(void)
{
- bpf_register_prog_type(&tl);
+ bpf_register_prog_type(&sock_filter_type);
return 0;
}
late_initcall(register_sock_filter_ops);
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 3/7] ebpf: check first for MAXINSNS in bpf_prog_load
From: Daniel Borkmann @ 2015-02-11 0:15 UTC (permalink / raw)
To: jiri; +Cc: ast, netdev, Daniel Borkmann
In-Reply-To: <cover.1423610452.git.daniel@iogearbox.net>
Just minor ... before doing all the copying work, we may want
to check for instruction count earlier. Also, we may want to
warn the user in case we would otherwise need to truncate the
license information.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
kernel/bpf/syscall.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 536edc2..73b105c 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -473,25 +473,26 @@ static int bpf_prog_load(union bpf_attr *attr)
{
enum bpf_prog_type type = attr->prog_type;
struct bpf_prog *prog;
- int err;
char license[128];
bool is_gpl;
+ int err;
if (CHECK_ATTR(BPF_PROG_LOAD))
return -EINVAL;
+ if (attr->insn_cnt >= BPF_MAXINSNS)
+ return -EINVAL;
/* copy eBPF program license from user space */
- if (strncpy_from_user(license, u64_to_ptr(attr->license),
- sizeof(license) - 1) < 0)
- return -EFAULT;
- license[sizeof(license) - 1] = 0;
+ err = strncpy_from_user(license, u64_to_ptr(attr->license),
+ sizeof(license));
+ if (err == sizeof(license))
+ err = -ERANGE;
+ if (err < 0)
+ return err;
/* eBPF programs must be GPL compatible to use GPL-ed functions */
is_gpl = license_is_gpl_compatible(license);
- if (attr->insn_cnt >= BPF_MAXINSNS)
- return -EINVAL;
-
/* plain bpf_prog allocation */
prog = bpf_prog_alloc(bpf_prog_size(attr->insn_cnt), GFP_USER);
if (!prog)
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 4/7] ebpf: extend program type/subsystem registration
From: Daniel Borkmann @ 2015-02-11 0:15 UTC (permalink / raw)
To: jiri; +Cc: ast, netdev, Daniel Borkmann
In-Reply-To: <cover.1423610452.git.daniel@iogearbox.net>
When various subsystems/modules start to make use of ebpf
e.g. cls_bpf, act_bpf, ovs, ... we need to make sure, they
can register their program types only once.
Moreover, we also need to serialize various registrations,
currently program type registration is being done without
locks. (We should make sure to not race in future when we
allow registration from modules.)
Last but not least, we need to be able to register subsystems
from module context as it's not sufficient to have them only
as built-in at all time. Built-in subsystems don't need to
provide an owner though.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/bpf.h | 11 +++----
kernel/bpf/helpers.c | 3 ++
kernel/bpf/syscall.c | 79 +++++++++++++++++++++++++++++++++++++++------------
kernel/bpf/verifier.c | 15 +++++-----
net/core/filter.c | 9 +++---
5 files changed, 83 insertions(+), 34 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 7844686..4fe1bd3 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -110,21 +110,22 @@ struct bpf_verifier_ops {
struct bpf_prog_type_list {
struct list_head list_node;
const struct bpf_verifier_ops *ops;
+ struct module *owner;
enum bpf_prog_type type;
};
-void bpf_register_prog_type(struct bpf_prog_type_list *tl);
+int bpf_register_prog_type(struct bpf_prog_type_list *tl);
+void bpf_unregister_prog_type(struct bpf_prog_type_list *tl);
struct bpf_prog;
struct bpf_prog_aux {
atomic_t refcnt;
- bool is_gpl_compatible;
- enum bpf_prog_type prog_type;
- const struct bpf_verifier_ops *ops;
+ bool gpl_compatible;
+ const struct bpf_prog_type_list *tl;
+ struct bpf_prog *prog;
struct bpf_map **used_maps;
u32 used_map_cnt;
- struct bpf_prog *prog;
struct work_struct work;
};
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index a3c7701..58efb27 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -48,6 +48,7 @@ const struct bpf_func_proto bpf_map_lookup_elem_proto = {
.arg1_type = ARG_CONST_MAP_PTR,
.arg2_type = ARG_PTR_TO_MAP_KEY,
};
+EXPORT_SYMBOL_GPL(bpf_map_lookup_elem_proto);
static u64 bpf_map_update_elem(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
{
@@ -69,6 +70,7 @@ const struct bpf_func_proto bpf_map_update_elem_proto = {
.arg3_type = ARG_PTR_TO_MAP_VALUE,
.arg4_type = ARG_ANYTHING,
};
+EXPORT_SYMBOL_GPL(bpf_map_update_elem_proto);
static u64 bpf_map_delete_elem(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5)
{
@@ -87,3 +89,4 @@ const struct bpf_func_proto bpf_map_delete_elem_proto = {
.arg1_type = ARG_CONST_MAP_PTR,
.arg2_type = ARG_PTR_TO_MAP_KEY,
};
+EXPORT_SYMBOL_GPL(bpf_map_delete_elem_proto);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 73b105c..bacec89 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -15,6 +15,7 @@
#include <linux/anon_inodes.h>
#include <linux/file.h>
#include <linux/license.h>
+#include <linux/module.h>
#include <linux/filter.h>
static LIST_HEAD(bpf_map_types);
@@ -102,7 +103,6 @@ static int map_create(union bpf_attr *attr)
atomic_set(&map->refcnt, 1);
err = anon_inode_getfd("bpf-map", &bpf_map_fops, map, O_RDWR | O_CLOEXEC);
-
if (err < 0)
/* failed to allocate fd */
goto free_map;
@@ -345,26 +345,61 @@ err_put:
return err;
}
+static DEFINE_SPINLOCK(bpf_prog_types_lock);
static LIST_HEAD(bpf_prog_types);
-static int find_prog_type(enum bpf_prog_type type, struct bpf_prog *prog)
+static void bpf_put_prog_type(const struct bpf_prog_type_list *tl)
+{
+ module_put(tl->owner);
+}
+
+static const struct bpf_prog_type_list *find_prog_type(enum bpf_prog_type type,
+ bool get_ref)
{
- struct bpf_prog_type_list *tl;
+ struct bpf_prog_type_list *tl, *ret = NULL;
- list_for_each_entry(tl, &bpf_prog_types, list_node) {
+ rcu_read_lock();
+ list_for_each_entry_rcu(tl, &bpf_prog_types, list_node) {
if (tl->type == type) {
- prog->aux->ops = tl->ops;
- prog->aux->prog_type = type;
- return 0;
+ if (!get_ref || try_module_get(tl->owner))
+ ret = tl;
+ break;
}
}
- return -EINVAL;
+ rcu_read_unlock();
+
+ return ret;
+}
+
+int bpf_register_prog_type(struct bpf_prog_type_list *tl)
+{
+ if (find_prog_type(tl->type, false))
+ return -EBUSY;
+
+ spin_lock(&bpf_prog_types_lock);
+ list_add_tail_rcu(&tl->list_node, &bpf_prog_types);
+ spin_unlock(&bpf_prog_types_lock);
+
+ return 0;
}
+EXPORT_SYMBOL_GPL(bpf_register_prog_type);
-void bpf_register_prog_type(struct bpf_prog_type_list *tl)
+void bpf_unregister_prog_type(struct bpf_prog_type_list *tl)
{
- list_add(&tl->list_node, &bpf_prog_types);
+ spin_lock(&bpf_prog_types_lock);
+ list_del_rcu(&tl->list_node);
+ spin_unlock(&bpf_prog_types_lock);
+
+ /* Wait for outstanding readers to complete before a prog
+ * type from a module gets removed entirely.
+ *
+ * A try_module_get() should fail by now as our module is
+ * in "going" state since no refs are held anymore and
+ * module_exit() handler being called.
+ */
+ synchronize_rcu();
}
+EXPORT_SYMBOL_GPL(bpf_unregister_prog_type);
/* fixup insn->imm field of bpf_call instructions:
* if (insn->imm == BPF_FUNC_map_lookup_elem)
@@ -384,13 +419,15 @@ static void fixup_bpf_calls(struct bpf_prog *prog)
struct bpf_insn *insn = &prog->insnsi[i];
if (insn->code == (BPF_JMP | BPF_CALL)) {
+ const struct bpf_verifier_ops *ops = prog->aux->tl->ops;
+
/* we reach here when program has bpf_call instructions
* and it passed bpf_check(), means that
* ops->get_func_proto must have been supplied, check it
*/
- BUG_ON(!prog->aux->ops->get_func_proto);
+ BUG_ON(!ops->get_func_proto);
- fn = prog->aux->ops->get_func_proto(insn->imm);
+ fn = ops->get_func_proto(insn->imm);
/* all functions that have prototype and verifier allowed
* programs to call them, must be real in-kernel functions
*/
@@ -414,10 +451,12 @@ static void free_used_maps(struct bpf_prog_aux *aux)
void bpf_prog_put(struct bpf_prog *prog)
{
if (atomic_dec_and_test(&prog->aux->refcnt)) {
+ bpf_put_prog_type(prog->aux->tl);
free_used_maps(prog->aux);
bpf_prog_free(prog);
}
}
+EXPORT_SYMBOL_GPL(bpf_prog_put);
static int bpf_prog_release(struct inode *inode, struct file *filp)
{
@@ -457,7 +496,6 @@ struct bpf_prog *bpf_prog_get(u32 ufd)
struct bpf_prog *prog;
prog = get_prog(f);
-
if (IS_ERR(prog))
return prog;
@@ -465,6 +503,7 @@ struct bpf_prog *bpf_prog_get(u32 ufd)
fdput(f);
return prog;
}
+EXPORT_SYMBOL_GPL(bpf_prog_get);
/* last field in 'union bpf_attr' used by this command */
#define BPF_PROG_LOAD_LAST_FIELD log_buf
@@ -472,6 +511,7 @@ struct bpf_prog *bpf_prog_get(u32 ufd)
static int bpf_prog_load(union bpf_attr *attr)
{
enum bpf_prog_type type = attr->prog_type;
+ const struct bpf_prog_type_list *tl;
struct bpf_prog *prog;
char license[128];
bool is_gpl;
@@ -509,16 +549,19 @@ static int bpf_prog_load(union bpf_attr *attr)
prog->jited = false;
atomic_set(&prog->aux->refcnt, 1);
- prog->aux->is_gpl_compatible = is_gpl;
+ prog->aux->gpl_compatible = is_gpl;
/* find program type: socket_filter vs tracing_filter */
- err = find_prog_type(type, prog);
- if (err < 0)
+ tl = find_prog_type(type, true);
+ if (!tl) {
+ err = -EINVAL;
goto free_prog;
+ }
+
+ prog->aux->tl = tl;
/* run eBPF verifier */
err = bpf_check(prog, attr);
-
if (err < 0)
goto free_used_maps;
@@ -529,7 +572,6 @@ static int bpf_prog_load(union bpf_attr *attr)
bpf_prog_select_runtime(prog);
err = anon_inode_getfd("bpf-prog", &bpf_prog_fops, prog, O_RDWR | O_CLOEXEC);
-
if (err < 0)
/* failed to allocate fd */
goto free_used_maps;
@@ -537,6 +579,7 @@ static int bpf_prog_load(union bpf_attr *attr)
return err;
free_used_maps:
+ bpf_put_prog_type(prog->aux->tl);
free_used_maps(prog->aux);
free_prog:
bpf_prog_free(prog);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index a28e09c..857e2fc 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -627,8 +627,9 @@ static int check_map_access(struct verifier_env *env, u32 regno, int off,
static int check_ctx_access(struct verifier_env *env, int off, int size,
enum bpf_access_type t)
{
- if (env->prog->aux->ops->is_valid_access &&
- env->prog->aux->ops->is_valid_access(off, size, t))
+ const struct bpf_verifier_ops *ops = env->prog->aux->tl->ops;
+
+ if (ops->is_valid_access && ops->is_valid_access(off, size, t))
return 0;
verbose("invalid bpf_context access off=%d size=%d\n", off, size);
@@ -831,6 +832,7 @@ static int check_func_arg(struct verifier_env *env, u32 regno,
static int check_call(struct verifier_env *env, int func_id)
{
struct verifier_state *state = &env->cur_state;
+ const struct bpf_verifier_ops *ops = env->prog->aux->tl->ops;
const struct bpf_func_proto *fn = NULL;
struct reg_state *regs = state->regs;
struct bpf_map *map = NULL;
@@ -843,16 +845,15 @@ static int check_call(struct verifier_env *env, int func_id)
return -EINVAL;
}
- if (env->prog->aux->ops->get_func_proto)
- fn = env->prog->aux->ops->get_func_proto(func_id);
-
+ if (ops->get_func_proto)
+ fn = ops->get_func_proto(func_id);
if (!fn) {
verbose("unknown func %d\n", func_id);
return -EINVAL;
}
/* eBPF programs must be GPL compatible to use GPL-ed functions */
- if (!env->prog->aux->is_gpl_compatible && fn->gpl_only) {
+ if (!env->prog->aux->gpl_compatible && fn->gpl_only) {
verbose("cannot call GPL only function from proprietary program\n");
return -EINVAL;
}
@@ -1194,7 +1195,7 @@ static int check_ld_abs(struct verifier_env *env, struct bpf_insn *insn)
struct reg_state *reg;
int i, err;
- if (env->prog->aux->prog_type != BPF_PROG_TYPE_SOCKET_FILTER) {
+ if (env->prog->aux->tl->type != BPF_PROG_TYPE_SOCKET_FILTER) {
verbose("BPF_LD_ABS|IND instructions are only allowed in socket filters\n");
return -EINVAL;
}
diff --git a/net/core/filter.c b/net/core/filter.c
index e823da5..d76560f 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -814,7 +814,8 @@ static void bpf_release_orig_filter(struct bpf_prog *fp)
static void __bpf_prog_release(struct bpf_prog *prog)
{
- if (prog->aux->prog_type == BPF_PROG_TYPE_SOCKET_FILTER) {
+ if (prog->aux->tl &&
+ prog->aux->tl->type == BPF_PROG_TYPE_SOCKET_FILTER) {
bpf_prog_put(prog);
} else {
bpf_release_orig_filter(prog);
@@ -1106,7 +1107,7 @@ int sk_attach_bpf(u32 ufd, struct sock *sk)
if (IS_ERR(prog))
return PTR_ERR(prog);
- if (prog->aux->prog_type != BPF_PROG_TYPE_SOCKET_FILTER) {
+ if (prog->aux->tl->type != BPF_PROG_TYPE_SOCKET_FILTER) {
/* valid fd, but invalid program type */
bpf_prog_put(prog);
return -EINVAL;
@@ -1171,8 +1172,7 @@ static struct bpf_prog_type_list sock_filter_type __read_mostly = {
static int __init register_sock_filter_ops(void)
{
- bpf_register_prog_type(&sock_filter_type);
- return 0;
+ return bpf_register_prog_type(&sock_filter_type);
}
late_initcall(register_sock_filter_ops);
#else
@@ -1181,6 +1181,7 @@ int sk_attach_bpf(u32 ufd, struct sock *sk)
return -EOPNOTSUPP;
}
#endif
+
int sk_detach_filter(struct sock *sk)
{
int ret = -ENOENT;
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 5/7] ebpf: export BPF_PSEUDO_MAP_FD to uapi
From: Daniel Borkmann @ 2015-02-11 0:15 UTC (permalink / raw)
To: jiri; +Cc: ast, netdev, Daniel Borkmann
In-Reply-To: <cover.1423610452.git.daniel@iogearbox.net>
We need to export BPF_PSEUDO_MAP_FD to user space, as it's used in
the ELF BPF loader where instructions are being loaded that need
map fixups (relocations). An initial stage loads all maps into the
kernel, and later on replaces related instructions in the eBPF blob
with BPF_PSEUDO_MAP_FD as source register and the actual fd as
immediate value. The kernel verifier recognizes this keyword and
replaces the map fd with a real pointer internally.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/filter.h | 2 --
include/uapi/linux/bpf.h | 2 ++
samples/bpf/libbpf.h | 4 +++-
3 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index caac208..5e3863d 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -145,8 +145,6 @@ struct bpf_prog_aux;
.off = 0, \
.imm = ((__u64) (IMM)) >> 32 })
-#define BPF_PSEUDO_MAP_FD 1
-
/* pseudo BPF_LD_IMM64 insn used to refer to process-local map_fd */
#define BPF_LD_MAP_FD(DST, MAP_FD) \
BPF_LD_IMM64_RAW(DST, BPF_PSEUDO_MAP_FD, MAP_FD)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 45da7ec..0248180 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -120,6 +120,8 @@ enum bpf_prog_type {
BPF_PROG_TYPE_SOCKET_FILTER,
};
+#define BPF_PSEUDO_MAP_FD 1
+
/* flags for BPF_MAP_UPDATE_ELEM command */
#define BPF_ANY 0 /* create new element or update existing */
#define BPF_NOEXIST 1 /* create new element if it didn't exist */
diff --git a/samples/bpf/libbpf.h b/samples/bpf/libbpf.h
index 58c5fe1..a6bb7e9 100644
--- a/samples/bpf/libbpf.h
+++ b/samples/bpf/libbpf.h
@@ -92,7 +92,9 @@ extern char bpf_log_buf[LOG_BUF_SIZE];
.off = 0, \
.imm = ((__u64) (IMM)) >> 32 })
-#define BPF_PSEUDO_MAP_FD 1
+#ifndef BPF_PSEUDO_MAP_FD
+# define BPF_PSEUDO_MAP_FD 1
+#endif
/* pseudo BPF_LD_IMM64 insn used to refer to process-local map_fd */
#define BPF_LD_MAP_FD(DST, MAP_FD) \
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 6/7] ebpf: remove CONFIG_BPF_SYSCALL ifdefs in socket filter code
From: Daniel Borkmann @ 2015-02-11 0:15 UTC (permalink / raw)
To: jiri; +Cc: ast, netdev, Daniel Borkmann
In-Reply-To: <cover.1423610452.git.daniel@iogearbox.net>
Socket filter code and other subsystems with upcoming eBPF support
should not need to deal with the fact that we have CONFIG_BPF_SYSCALL
defined or not. Having the bpf syscall as a config option is a nice
thing and I'd expect it to stay that way for expert users (I presume
one day the default setting of it might change, though), but code
making use of it should not care if it's actually enabled or not.
Instead, hide this via header files and let the rest deal with it.
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/linux/bpf.h | 27 +++++++--
net/core/filter.c | 166 ++++++++++++++++++++++++----------------------------
2 files changed, 100 insertions(+), 93 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 4fe1bd3..def0103 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -114,9 +114,6 @@ struct bpf_prog_type_list {
enum bpf_prog_type type;
};
-int bpf_register_prog_type(struct bpf_prog_type_list *tl);
-void bpf_unregister_prog_type(struct bpf_prog_type_list *tl);
-
struct bpf_prog;
struct bpf_prog_aux {
@@ -130,11 +127,31 @@ struct bpf_prog_aux {
};
#ifdef CONFIG_BPF_SYSCALL
+int bpf_register_prog_type(struct bpf_prog_type_list *tl);
+void bpf_unregister_prog_type(struct bpf_prog_type_list *tl);
+
void bpf_prog_put(struct bpf_prog *prog);
+struct bpf_prog *bpf_prog_get(u32 ufd);
#else
-static inline void bpf_prog_put(struct bpf_prog *prog) {}
+static inline int bpf_register_prog_type(struct bpf_prog_type_list *tl)
+{
+ return 0;
+}
+
+static inline void bpf_unregister_prog_type(struct bpf_prog_type_list *tl)
+{
+}
+
+static inline struct bpf_prog *bpf_prog_get(u32 ufd)
+{
+ return ERR_PTR(-EOPNOTSUPP);
+}
+
+static inline void bpf_prog_put(struct bpf_prog *prog)
+{
+}
#endif
-struct bpf_prog *bpf_prog_get(u32 ufd);
+
/* verify correctness of eBPF program */
int bpf_check(struct bpf_prog *fp, union bpf_attr *attr);
diff --git a/net/core/filter.c b/net/core/filter.c
index d76560f..306b860 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -1020,6 +1020,46 @@ void bpf_prog_destroy(struct bpf_prog *fp)
}
EXPORT_SYMBOL_GPL(bpf_prog_destroy);
+int sk_attach_bpf(u32 ufd, struct sock *sk)
+{
+ struct sk_filter *fp, *old_fp;
+ struct bpf_prog *prog;
+
+ if (sock_flag(sk, SOCK_FILTER_LOCKED))
+ return -EPERM;
+
+ prog = bpf_prog_get(ufd);
+ if (IS_ERR(prog))
+ return PTR_ERR(prog);
+
+ if (prog->aux->tl->type != BPF_PROG_TYPE_SOCKET_FILTER) {
+ bpf_prog_put(prog);
+ return -EINVAL;
+ }
+
+ fp = kmalloc(sizeof(*fp), GFP_KERNEL);
+ if (!fp) {
+ bpf_prog_put(prog);
+ return -ENOMEM;
+ }
+
+ fp->prog = prog;
+ atomic_set(&fp->refcnt, 0);
+
+ if (!sk_filter_charge(sk, fp)) {
+ __sk_filter_release(fp);
+ return -ENOMEM;
+ }
+
+ old_fp = rcu_dereference_protected(sk->sk_filter,
+ sock_owned_by_user(sk));
+ rcu_assign_pointer(sk->sk_filter, fp);
+ if (old_fp)
+ sk_filter_uncharge(sk, old_fp);
+
+ return 0;
+}
+
/**
* sk_attach_filter - attach a socket filter
* @fprog: the filter program
@@ -1094,94 +1134,6 @@ int sk_attach_filter(struct sock_fprog *fprog, struct sock *sk)
}
EXPORT_SYMBOL_GPL(sk_attach_filter);
-#ifdef CONFIG_BPF_SYSCALL
-int sk_attach_bpf(u32 ufd, struct sock *sk)
-{
- struct sk_filter *fp, *old_fp;
- struct bpf_prog *prog;
-
- if (sock_flag(sk, SOCK_FILTER_LOCKED))
- return -EPERM;
-
- prog = bpf_prog_get(ufd);
- if (IS_ERR(prog))
- return PTR_ERR(prog);
-
- if (prog->aux->tl->type != BPF_PROG_TYPE_SOCKET_FILTER) {
- /* valid fd, but invalid program type */
- bpf_prog_put(prog);
- return -EINVAL;
- }
-
- fp = kmalloc(sizeof(*fp), GFP_KERNEL);
- if (!fp) {
- bpf_prog_put(prog);
- return -ENOMEM;
- }
- fp->prog = prog;
-
- atomic_set(&fp->refcnt, 0);
-
- if (!sk_filter_charge(sk, fp)) {
- __sk_filter_release(fp);
- return -ENOMEM;
- }
-
- old_fp = rcu_dereference_protected(sk->sk_filter,
- sock_owned_by_user(sk));
- rcu_assign_pointer(sk->sk_filter, fp);
-
- if (old_fp)
- sk_filter_uncharge(sk, old_fp);
-
- return 0;
-}
-
-/* allow socket filters to call
- * bpf_map_lookup_elem(), bpf_map_update_elem(), bpf_map_delete_elem()
- */
-static const struct bpf_func_proto *sock_filter_func_proto(enum bpf_func_id func_id)
-{
- switch (func_id) {
- case BPF_FUNC_map_lookup_elem:
- return &bpf_map_lookup_elem_proto;
- case BPF_FUNC_map_update_elem:
- return &bpf_map_update_elem_proto;
- case BPF_FUNC_map_delete_elem:
- return &bpf_map_delete_elem_proto;
- default:
- return NULL;
- }
-}
-
-static bool sock_filter_is_valid_access(int off, int size, enum bpf_access_type type)
-{
- /* skb fields cannot be accessed yet */
- return false;
-}
-
-static const struct bpf_verifier_ops sock_filter_ops = {
- .get_func_proto = sock_filter_func_proto,
- .is_valid_access = sock_filter_is_valid_access,
-};
-
-static struct bpf_prog_type_list sock_filter_type __read_mostly = {
- .ops = &sock_filter_ops,
- .type = BPF_PROG_TYPE_SOCKET_FILTER,
-};
-
-static int __init register_sock_filter_ops(void)
-{
- return bpf_register_prog_type(&sock_filter_type);
-}
-late_initcall(register_sock_filter_ops);
-#else
-int sk_attach_bpf(u32 ufd, struct sock *sk)
-{
- return -EOPNOTSUPP;
-}
-#endif
-
int sk_detach_filter(struct sock *sk)
{
int ret = -ENOENT;
@@ -1241,3 +1193,41 @@ out:
release_sock(sk);
return ret;
}
+
+static const struct bpf_func_proto *
+sock_filter_func_proto(enum bpf_func_id func_id)
+{
+ switch (func_id) {
+ case BPF_FUNC_map_lookup_elem:
+ return &bpf_map_lookup_elem_proto;
+ case BPF_FUNC_map_update_elem:
+ return &bpf_map_update_elem_proto;
+ case BPF_FUNC_map_delete_elem:
+ return &bpf_map_delete_elem_proto;
+ default:
+ return NULL;
+ }
+}
+
+static bool sock_filter_is_valid_access(int off, int size,
+ enum bpf_access_type type)
+{
+ /* skb fields cannot be accessed yet */
+ return false;
+}
+
+static const struct bpf_verifier_ops sock_filter_ops = {
+ .get_func_proto = sock_filter_func_proto,
+ .is_valid_access = sock_filter_is_valid_access,
+};
+
+static struct bpf_prog_type_list sock_filter_type __read_mostly = {
+ .ops = &sock_filter_ops,
+ .type = BPF_PROG_TYPE_SOCKET_FILTER,
+};
+
+static int __init register_sock_filter_ops(void)
+{
+ return bpf_register_prog_type(&sock_filter_type);
+}
+late_initcall(register_sock_filter_ops);
--
1.9.3
^ permalink raw reply related
* [PATCH net-next 7/7] cls_bpf: add initial eBPF support for programmable classifiers
From: Daniel Borkmann @ 2015-02-11 0:15 UTC (permalink / raw)
To: jiri; +Cc: ast, netdev, Daniel Borkmann
In-Reply-To: <cover.1423610452.git.daniel@iogearbox.net>
This work extends the classic BPF programmable classifier by extending
its scope also to native eBPF code. This allows for implementing
custom C-like classifiers, compiling them with the LLVM eBPF backend
and loading the resulting object file via tc into the kernel.
Simple, minimal toy example:
#include <linux/ip.h>
#include <linux/if_ether.h>
#include <linux/bpf.h>
#include "tc_bpf_api.h"
__section("classify")
int cls_main(struct sk_buff *skb)
{
return (0x800 << 16) | load_byte(skb, ETH_HLEN + __builtin_offsetof(struct iphdr, tos));
}
char __license[] __section("license") = "GPL";
The classifier can then be compiled into eBPF opcodes and loaded via
tc, f.e.:
clang -O2 -emit-llvm -c cls.c -o - | llc -march=bpf -filetype=obj -o cls.o
tc filter add dev em1 parent 1: bpf run object-file cls.o [...]
As it has been demonstrated, the scope can even reach up to a fully
fledged flow dissector (similarly as in samples/bpf/sockex2_kern.c).
For tc, maps are allowed to be used, but from kernel context only,
in other words eBPF code can keep state across filter invocations.
Similarly as in socket filters, we may extend functionality for eBPF
classifiers over time depending on the use cases. For that purpose,
I have added the BPF_PROG_TYPE_SCHED_CLS program type for the cls_bpf
classifier module, so we can allow additional functions/accessors.
I was wondering whether cls_bpf and act_bpf may share C programs, I
can imagine that at some point, we may introduce i) some common
handlers for both (or even beyond their scope), and/or ii) some
restricted function space for each of them. Both can be abstracted
through struct bpf_verifier_ops in future. The context of a cls_bpf
versus act_bpf is slightly different though: a cls_bpf program will
return a specific classid whereas act_bpf a drop/non-drop return
code. That said, we can surely have a "classify" and "action" section
in a single object file, or considered mentioned constraint add a
possibility of a shared section.
The workflow for getting native eBPF running from tc [1] is as
follows: for f_bpf, I've added a slightly modified ELF parser code
from Alexei's kernel sample, which reads out the LLVM compiled
object, sets up maps (and dynamically fixes up map fds) if any,
and loads the eBPF instructions all centrally through the bpf
syscall. The resulting fd from the loaded program itself is being
passed down to cls_bpf, which looks up struct bpf_prog from the
fd store, and holds reference, so that it stays available also
after tc program lifetime. On tc filter destruction, it will then
drop its reference.
[1] http://git.breakpoint.cc/cgit/dborkman/iproute2.git/log/?h=ebpf
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
---
include/uapi/linux/bpf.h | 1 +
include/uapi/linux/pkt_cls.h | 1 +
kernel/bpf/verifier.c | 15 +++-
net/sched/cls_bpf.c | 200 ++++++++++++++++++++++++++++++++-----------
4 files changed, 165 insertions(+), 52 deletions(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 0248180..3fa1af8 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -118,6 +118,7 @@ enum bpf_map_type {
enum bpf_prog_type {
BPF_PROG_TYPE_UNSPEC,
BPF_PROG_TYPE_SOCKET_FILTER,
+ BPF_PROG_TYPE_SCHED_CLS,
};
#define BPF_PSEUDO_MAP_FD 1
diff --git a/include/uapi/linux/pkt_cls.h b/include/uapi/linux/pkt_cls.h
index 25731df..1f192cb 100644
--- a/include/uapi/linux/pkt_cls.h
+++ b/include/uapi/linux/pkt_cls.h
@@ -397,6 +397,7 @@ enum {
TCA_BPF_CLASSID,
TCA_BPF_OPS_LEN,
TCA_BPF_OPS,
+ TCA_BPF_EFD,
__TCA_BPF_MAX,
};
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 857e2fc..9aa4747 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1173,6 +1173,17 @@ static int check_ld_imm(struct verifier_env *env, struct bpf_insn *insn)
return 0;
}
+static bool may_access_skb(enum bpf_prog_type type)
+{
+ switch (type) {
+ case BPF_PROG_TYPE_SOCKET_FILTER:
+ case BPF_PROG_TYPE_SCHED_CLS:
+ return true;
+ default:
+ return false;
+ }
+}
+
/* verify safety of LD_ABS|LD_IND instructions:
* - they can only appear in the programs where ctx == skb
* - since they are wrappers of function calls, they scratch R1-R5 registers,
@@ -1195,8 +1206,8 @@ static int check_ld_abs(struct verifier_env *env, struct bpf_insn *insn)
struct reg_state *reg;
int i, err;
- if (env->prog->aux->tl->type != BPF_PROG_TYPE_SOCKET_FILTER) {
- verbose("BPF_LD_ABS|IND instructions are only allowed in socket filters\n");
+ if (!may_access_skb(env->prog->aux->tl->type)) {
+ verbose("BPF_LD_ABS|IND instructions not allowed for this program type\n");
return -EINVAL;
}
diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 5f3ee9e..c6e1328 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -16,6 +16,8 @@
#include <linux/types.h>
#include <linux/skbuff.h>
#include <linux/filter.h>
+#include <linux/bpf.h>
+
#include <net/rtnetlink.h>
#include <net/pkt_cls.h>
#include <net/sock.h>
@@ -37,18 +39,27 @@ struct cls_bpf_prog {
struct tcf_result res;
struct list_head link;
u32 handle;
- u16 bpf_num_ops;
+ union {
+ u32 bpf_fd;
+ u16 bpf_num_ops;
+ };
struct tcf_proto *tp;
struct rcu_head rcu;
};
static const struct nla_policy bpf_policy[TCA_BPF_MAX + 1] = {
[TCA_BPF_CLASSID] = { .type = NLA_U32 },
+ [TCA_BPF_EFD] = { .type = NLA_U32 },
[TCA_BPF_OPS_LEN] = { .type = NLA_U16 },
[TCA_BPF_OPS] = { .type = NLA_BINARY,
.len = sizeof(struct sock_filter) * BPF_MAXINSNS },
};
+static bool cls_bpf_is_ebpf(const struct cls_bpf_prog *prog)
+{
+ return prog->bpf_ops == NULL;
+}
+
static int cls_bpf_classify(struct sk_buff *skb, const struct tcf_proto *tp,
struct tcf_result *res)
{
@@ -94,7 +105,10 @@ static void cls_bpf_delete_prog(struct tcf_proto *tp, struct cls_bpf_prog *prog)
{
tcf_exts_destroy(&prog->exts);
- bpf_prog_destroy(prog->filter);
+ if (cls_bpf_is_ebpf(prog))
+ bpf_prog_put(prog->filter);
+ else
+ bpf_prog_destroy(prog->filter);
kfree(prog->bpf_ops);
kfree(prog);
@@ -114,6 +128,7 @@ static int cls_bpf_delete(struct tcf_proto *tp, unsigned long arg)
list_del_rcu(&prog->link);
tcf_unbind_filter(tp, &prog->res);
call_rcu(&prog->rcu, __cls_bpf_delete_prog);
+
return 0;
}
@@ -151,69 +166,104 @@ static unsigned long cls_bpf_get(struct tcf_proto *tp, u32 handle)
return ret;
}
-static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,
- struct cls_bpf_prog *prog,
- unsigned long base, struct nlattr **tb,
- struct nlattr *est, bool ovr)
+static int cls_bpf_prog_from_ops(struct nlattr **tb,
+ struct cls_bpf_prog *prog, u32 classid)
{
struct sock_filter *bpf_ops;
- struct tcf_exts exts;
- struct sock_fprog_kern tmp;
+ struct sock_fprog_kern fprog_tmp;
struct bpf_prog *fp;
u16 bpf_size, bpf_num_ops;
- u32 classid;
int ret;
- if (!tb[TCA_BPF_OPS_LEN] || !tb[TCA_BPF_OPS] || !tb[TCA_BPF_CLASSID])
- return -EINVAL;
-
- tcf_exts_init(&exts, TCA_BPF_ACT, TCA_BPF_POLICE);
- ret = tcf_exts_validate(net, tp, tb, est, &exts, ovr);
- if (ret < 0)
- return ret;
-
- classid = nla_get_u32(tb[TCA_BPF_CLASSID]);
bpf_num_ops = nla_get_u16(tb[TCA_BPF_OPS_LEN]);
- if (bpf_num_ops > BPF_MAXINSNS || bpf_num_ops == 0) {
- ret = -EINVAL;
- goto errout;
- }
+ if (bpf_num_ops > BPF_MAXINSNS || bpf_num_ops == 0)
+ return -EINVAL;
bpf_size = bpf_num_ops * sizeof(*bpf_ops);
- if (bpf_size != nla_len(tb[TCA_BPF_OPS])) {
- ret = -EINVAL;
- goto errout;
- }
+ if (bpf_size != nla_len(tb[TCA_BPF_OPS]))
+ return -EINVAL;
bpf_ops = kzalloc(bpf_size, GFP_KERNEL);
- if (bpf_ops == NULL) {
- ret = -ENOMEM;
- goto errout;
- }
+ if (bpf_ops == NULL)
+ return -ENOMEM;
memcpy(bpf_ops, nla_data(tb[TCA_BPF_OPS]), bpf_size);
- tmp.len = bpf_num_ops;
- tmp.filter = bpf_ops;
+ fprog_tmp.len = bpf_num_ops;
+ fprog_tmp.filter = bpf_ops;
- ret = bpf_prog_create(&fp, &tmp);
- if (ret)
- goto errout_free;
+ ret = bpf_prog_create(&fp, &fprog_tmp);
+ if (ret < 0) {
+ kfree(bpf_ops);
+ return ret;
+ }
prog->bpf_num_ops = bpf_num_ops;
prog->bpf_ops = bpf_ops;
prog->filter = fp;
prog->res.classid = classid;
+ return 0;
+}
+
+static int cls_bpf_prog_from_efd(struct nlattr **tb,
+ struct cls_bpf_prog *prog, u32 classid)
+{
+ struct bpf_prog *fp;
+ u32 bpf_fd;
+
+ bpf_fd = nla_get_u32(tb[TCA_BPF_EFD]);
+
+ fp = bpf_prog_get(bpf_fd);
+ if (IS_ERR(fp))
+ return PTR_ERR(fp);
+
+ if (fp->aux->tl->type != BPF_PROG_TYPE_SCHED_CLS) {
+ bpf_prog_put(fp);
+ return -EINVAL;
+ }
+
+ prog->bpf_ops = NULL;
+ prog->bpf_fd = bpf_fd;
+ prog->filter = fp;
+ prog->res.classid = classid;
+
+ return 0;
+}
+
+static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,
+ struct cls_bpf_prog *prog,
+ unsigned long base, struct nlattr **tb,
+ struct nlattr *est, bool ovr)
+{
+ struct tcf_exts exts;
+ bool is_bpf, is_ebpf;
+ u32 classid;
+ int ret;
+
+ is_bpf = tb[TCA_BPF_OPS_LEN] && tb[TCA_BPF_OPS];
+ is_ebpf = tb[TCA_BPF_EFD];
+ if ((!is_bpf && !is_ebpf) || !tb[TCA_BPF_CLASSID])
+ return -EINVAL;
+
+ tcf_exts_init(&exts, TCA_BPF_ACT, TCA_BPF_POLICE);
+ ret = tcf_exts_validate(net, tp, tb, est, &exts, ovr);
+ if (ret < 0)
+ return ret;
+
+ classid = nla_get_u32(tb[TCA_BPF_CLASSID]);
+
+ ret = is_bpf ? cls_bpf_prog_from_ops(tb, prog, classid) :
+ cls_bpf_prog_from_efd(tb, prog, classid);
+ if (ret < 0) {
+ tcf_exts_destroy(&exts);
+ return ret;
+ }
+
tcf_bind_filter(tp, &prog->res, base);
tcf_exts_change(tp, &prog->exts, &exts);
return 0;
-errout_free:
- kfree(bpf_ops);
-errout:
- tcf_exts_destroy(&exts);
- return ret;
}
static u32 cls_bpf_grab_new_handle(struct tcf_proto *tp,
@@ -290,10 +340,10 @@ static int cls_bpf_change(struct net *net, struct sk_buff *in_skb,
}
*arg = (unsigned long) prog;
+
return 0;
errout:
kfree(prog);
-
return ret;
}
@@ -301,7 +351,7 @@ static int cls_bpf_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
struct sk_buff *skb, struct tcmsg *tm)
{
struct cls_bpf_prog *prog = (struct cls_bpf_prog *) fh;
- struct nlattr *nest, *nla;
+ struct nlattr *nest;
if (prog == NULL)
return skb->len;
@@ -314,15 +364,23 @@ static int cls_bpf_dump(struct net *net, struct tcf_proto *tp, unsigned long fh,
if (nla_put_u32(skb, TCA_BPF_CLASSID, prog->res.classid))
goto nla_put_failure;
- if (nla_put_u16(skb, TCA_BPF_OPS_LEN, prog->bpf_num_ops))
- goto nla_put_failure;
- nla = nla_reserve(skb, TCA_BPF_OPS, prog->bpf_num_ops *
- sizeof(struct sock_filter));
- if (nla == NULL)
- goto nla_put_failure;
+ if (cls_bpf_is_ebpf(prog)) {
+ if (nla_put_u32(skb, TCA_BPF_EFD, prog->bpf_fd))
+ goto nla_put_failure;
+ } else {
+ struct nlattr *nla;
+
+ if (nla_put_u16(skb, TCA_BPF_OPS_LEN, prog->bpf_num_ops))
+ goto nla_put_failure;
- memcpy(nla_data(nla), prog->bpf_ops, nla_len(nla));
+ nla = nla_reserve(skb, TCA_BPF_OPS, prog->bpf_num_ops *
+ sizeof(struct sock_filter));
+ if (nla == NULL)
+ goto nla_put_failure;
+
+ memcpy(nla_data(nla), prog->bpf_ops, nla_len(nla));
+ }
if (tcf_exts_dump(skb, &prog->exts) < 0)
goto nla_put_failure;
@@ -356,6 +414,37 @@ skip:
}
}
+static const struct bpf_func_proto *bpf_cls_func_proto(enum bpf_func_id func_id)
+{
+ switch (func_id) {
+ default:
+ return NULL;
+ case BPF_FUNC_map_lookup_elem:
+ return &bpf_map_lookup_elem_proto;
+ case BPF_FUNC_map_update_elem:
+ return &bpf_map_update_elem_proto;
+ case BPF_FUNC_map_delete_elem:
+ return &bpf_map_delete_elem_proto;
+ }
+}
+
+static bool bpf_cls_valid_access(int off, int size, enum bpf_access_type type)
+{
+ /* TODO: skb fields cannot be accessed yet */
+ return false;
+}
+
+static const struct bpf_verifier_ops bpf_cls_vops = {
+ .get_func_proto = bpf_cls_func_proto,
+ .is_valid_access = bpf_cls_valid_access,
+};
+
+static struct bpf_prog_type_list bpf_cls_type = {
+ .ops = &bpf_cls_vops,
+ .type = BPF_PROG_TYPE_SCHED_CLS,
+ .owner = THIS_MODULE,
+};
+
static struct tcf_proto_ops cls_bpf_ops __read_mostly = {
.kind = "bpf",
.owner = THIS_MODULE,
@@ -371,12 +460,23 @@ static struct tcf_proto_ops cls_bpf_ops __read_mostly = {
static int __init cls_bpf_init_mod(void)
{
- return register_tcf_proto_ops(&cls_bpf_ops);
+ int ret;
+
+ ret = bpf_register_prog_type(&bpf_cls_type);
+ if (ret)
+ return ret;
+
+ ret = register_tcf_proto_ops(&cls_bpf_ops);
+ if (ret)
+ bpf_unregister_prog_type(&bpf_cls_type);
+
+ return ret;
}
static void __exit cls_bpf_exit_mod(void)
{
unregister_tcf_proto_ops(&cls_bpf_ops);
+ bpf_unregister_prog_type(&bpf_cls_type);
}
module_init(cls_bpf_init_mod);
--
1.9.3
^ permalink raw reply related
* Re: [PATCH v3 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Alexei Starovoitov @ 2015-02-11 0:22 UTC (permalink / raw)
To: Steven Rostedt
Cc: Ingo Molnar, Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa,
Masami Hiramatsu, Linux API, Network Development, LKML,
Linus Torvalds, Peter Zijlstra, Eric W. Biederman
On Tue, Feb 10, 2015 at 1:53 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Tue, 10 Feb 2015 11:53:22 -0800
> Alexei Starovoitov <ast@plumgrid.com> wrote:
>
>> On Tue, Feb 10, 2015 at 5:05 AM, Steven Rostedt <rostedt@goodmis.org> wrote:
>> > On Mon, 9 Feb 2015 22:10:45 -0800
>> > Alexei Starovoitov <ast@plumgrid.com> wrote:
>> >
>> >> One can argue that current TP_printk format is already an ABI,
>> >> because somebody might be parsing the text output.
>> >
>> > If somebody does, then it is an ABI. Luckily, it's not that useful to
>> > parse, thus it hasn't been an issue. As Linus has stated in the past,
>> > it's not that we can't change ABI interfaces, its just that we can not
>> > change them if there's a user space application that depends on it.
>>
>> there are already tools that parse trace_pipe:
>> https://github.com/brendangregg/perf-tools
>
> Yep, and if this becomes a standard, then any change that makes
> trace_pipe different will be reverted.
I think reading of trace_pipe is widespread.
>> > and expect some events to have specific fields. Now we can add new
>> > fields, or even remove fields that no user space tool is using. This is
>> > because today, tools use libtraceevent to parse the event data.
>>
>> not all tools use libtraceevent.
>> gdb calls perf_event_open directly:
>> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;a=blob;f=gdb/nat/linux-btrace.c
>> and parses PERF_RECORD_SAMPLE as a binary.
>> In this case it's branch records, but I think we never said anywhere
>> that PERF_SAMPLE_IP | PERF_SAMPLE_ADDR should come
>> in this particular order.
>
> What particular order? Note, that's a hardware event, not a software
> one.
yes, but gdb assumes that 'u64 ip' precedes, 'u64 addr'
when attr.sample_type = IP | ADDR whereas this is an
internal order of 'if' statements inside perf_output_sample()...
>> But some maintainers think of them as ABI, whereas others
>> are using them freely. imo it's time to remove ambiguity.
>
> I would love to, and have brought this up at Kernel Summit more than
> once with no solution out of it.
let's try it again at plumbers in august?
For now I'm going to drop bpf+tracepoints, since it's so controversial
and go with bpf+syscall and bpf+kprobe only.
Hopefully by august it will be clear what bpf+kprobes can do
and why I'm excited about bpf+tracepoints in the future.
>> The idea for new style of tracepoints is the following:
>> introduce new macro: DEFINE_EVENT_USER
>> and let programs attach to them.
>
> We actually have that today. But it's TRACE_EVENT_FLAGS(), although
> that should be cleaned up a bit. Frederic added it to label events that
> are safe for perf non root. It seems to be used currently only for
> syscalls.
I didn't mean to let unprivileged apps to use.
Better name probably would be: DEFINE_EVENT_BPF
and only let bpf programs use it.
>> These tracepoint will receive one or two pointers to important
>> structs only. They will not have TP_printk, assign and fields.
>> The placement and arguments to these new tracepoints
>> will be an ABI.
>> All existing tracepoints are not.
>
> TP_printk() is not really an issue.
I think it is. The way things are printed is the most
visible part of tracepoints and I suspect maintainers don't
want to add new ones, because internal fields are printed
and users do parse trace_pipe.
Recent discussion about tcp instrumentation was
about adding new tracepoints and a module to print them.
As soon as something like this is in, the next question
'what we're going to do when more arguments need
to be printed'...
imo the solution is DEFINE_EVENT_BPF that doesn't
print anything and a bpf program to process it.
Then programs decide what they like to pass to
user space and in what format.
The kernel/user ABI is the position of this new tracepoint
and arguments that are passed in.
bpf program takes care of walking pointers, extracting
interesting fields, aggregating and passing them to
user in the format specific to this particular program.
Then when user wants to collect more fields it just
changes the program and corresponding userland side.
>> The position of new tracepoints and their arguments
>> will be an ABI and the programs can be both.
>
> You means "special tracepoints" one that does export the arguments?
>
> Question is, how many maintainers will add these, knowing that they
> will have to be forever maintained as is.
Obviously we would need to prove usefulness of
tracepoint_bpf before they're accepted.
Hopefully bpf+kprobe will make an impression on its own
and users would want similar but without debug info.
>> If program is using bpf_fetch*() helpers it obviously
>> wants to access internal data structures, so
>> it's really nothing more, but 'safe kernel module'
>> and kernel layout specific.
>> Both old and new tracepoints + programs will be used
>> for live kernel debugging.
>>
>> If program is accessing user-ized data structures then
>
> Technically, the TP_struct__entry is a user-ized structure.
>
>> it is portable and will run on any kernel.
>> In uapi header we can define:
>> struct task_struct_user {
>> int pid;
>> int prio;
>
> Here's a perfect example of something that looks stable to show to
> user space, but is really a pimple that is hiding cancer.
>
> Lets start with pid. We have name spaces. What pid will be put there?
> We have to show the pid of the name space it is under.
>
> Then we have prio. What is prio in the DEADLINE scheduler. It is rather
> meaningless. Also, it is meaningless in SCHED_OTHER.
>
> Also note that even for SCHED_FIFO, the prio is used differently in the
> kernel than it is in userspace. For the kernel, lower is higher.
well, ->prio and ->pid are already printed by sched tracepoints
and their meaning depends on scheduler. So users taking that
into account.
I'm not suggesting to preserve the meaning of 'pid' semantically
in all cases. That's not what users would want anyway.
I want to allow programs to access important fields and print
them in more generic way than current TP_printk does.
Then exposed ABI of such tracepoint_bpf is smaller than
with current tracepoints.
>> };
>> and let bpf programs access it via real 'struct task_struct *'
>> pointer passed into tracepoint.
>> bpf loader will translate offsets and sizes used inside
>> the program into real task_struct's offsets and loads.
>
> It would need to do more that that. It may have to calculate the value
> that it returns, as the internal value may be different with different
> kernels.
back to 'prio'... the 'prio' accessible from the program
should be the same 'prio' that we're storing inside task_struct.
No extra conversions.
The bpf-script for sched analytics would want to see
the actual value to make sense of it.
> But what if the userspace tool depends on that value returning
> something meaningful. If it was meaningful in the past, it will have to
> be meaningful in the future, even if the internals of the kernel make
> it otherwise.
in some cases... yes. if we make a poor choice of
selecting fields for this 'task_struct_user' then some fields
may disappear from real task_struct, and placeholders
will be left in _user version.
so exposed fields need to be thought through.
In case of networking some of these choices were
already made by classic bpf. Fields:
protocol, pkttype, ifindex, hatype, mark, hash, queue_mapping
have to be reference-able from 'skb' pointer.
They can move from skb to some other struct, change
their sizes, location within sk_buff, etc
They can even disappear, but user visible
struct sk_buff_user will still contain the above.
> eBPF is very flexible, which means it is bound to have someone use it
> in a way you never dreamed of, and that will be what bites you in the
> end (pun intended).
understood :)
let's start slow then with bpf+syscall and bpf+kprobe only.
>> also not all bpf programs are equal.
>> bpf+existing tracepoint is not ABI
>
> Why not?
well, because we want to see more tracepoints in the kernel.
We're already struggling to add more.
>> bpf+new tracepoint is ABI if programs are not using bpf_fetch
>
> How is this different?
the new ones will be explicit by definition.
>> bpf+syscall is ABI if programs are not using bpf_fetch
>
> Well, this is easy. As syscalls are ABI, and the tracepoints for them
> match the ABI, it by default becomes an ABI.
>
>> bpf+kprobe is not ABI
>
> Right.
>
>> bpf+sockets is ABI
>
> Right, because sockets themselves are ABI.
>
>> At the end we want most of the programs to be written
>> without assuming anything about kernel internals.
>> But for live kernel debugging we will write programs
>> very specific to given kernel layout.
>
> And here lies the trick. How do we differentiate applications for
> everyday use from debugging tools? There's been times when debugging
> tools have shown themselves as being so useful they become everyday
> use tools.
>
>>
>> We can categorize the above in non-ambigous via
>> bpf program type.
>> Programs with:
>> BPF_PROG_TYPE_TRACEPOINT - not ABI
>> BPF_PROG_TYPE_KPROBE - not ABI
>> BPF_PROG_TYPE_SOCKET_FILTER - ABI
>
> Again, what enforces this? (hint, it's Linus)
>
>
>>
>> for my proposed 'new tracepoints' we can add type:
>> BPF_PROG_TYPE_TRACEPOINT_USER - ABI
>> and disallow calls to bpf_fetch*() for them.
>> To make it more strict we can do kernel version check
>> for all prog types that are 'not ABI', but is it really necessary?
>
> If we have something that makes it difficult for a tool to work from
> one kernel to the next, or ever with different configs, where that tool
> will never become a standard, then that should be good enough to keep
> it from dictating user ABI.
>
> To give you an example, we thought about scrambling the trace event
> field locations from boot to boot to keep tools from hard coding the
> event layout. This may sound crazy, but developers out there are crazy.
> And if you want to keep them from abusing interfaces, you just need to
> be a bit more crazy than they are.
that is indeed crazy. the point is understood.
right now I cannot think of a solid way to prevent abuse
of bpf+tracepoint, so just going to drop it for now.
Cool things can be done with bpf+kprobe/syscall already.
>> To summarize and consolidate other threads:
>> - I will remove reading of PERF_SAMPLE_RAW in tracex1 example.
>> it's really orthogonal to this whole discussion.
>
> Or yous libtraceevent ;-) We really need to finish that and package it
> up for distros.
sure. some sophisticated example can use it too.
>> - will add more comments through out that just because
>> programs can read tracepoint arguments, they shouldn't
>> make any assumptions that args stays as-is from version to version
>
> We may need to find a way to actually keep it from being as is from
> version to version even if the users do not change.
>
>> - will work on a patch to demonstrate how few in-kernel
>> structures can be user-ized and how programs can access
>> them in version-indepedent way
>
> It will be interesting to see what kernel structures can be user-ized
> that are not already used by system calls.
well, for networking, few fields I mentioned above would
be enough for most of the programs.
>> btw the concept of user-ized data structures already exists
>> with classic bpf, since 'A = load -0x1000' is translated into
>> 'A = skb->protocol'. I'm thinking of something similar
>> but more generic and less obscure.
>
> I have to try to wrap my head around understanding the classic bpf, and
> how "load -0x1000" translates to "skb->protocol". Is that documented
> somewhere?
well, the magic constants are in uapi/linux/filter.h
load -0x1000 = skb->protocol
load -0x1000 + 4 = skb->pkt_type
load -0x1000 + 8 = skb->dev->ifindex
for eBPF I want to clean it up in a way that user program
will see:
struct sk_buff_user {
int protocol;
int pkt_type;
int ifindex;
...
};
and C code of bpf program will look normal: skb->ifindex.
bpf loader will do verification and translation of
'load skb_ptr+12' into sequence of loads with correct
internal offsets.
So struct sk_buff_user is only an interface. It won't exist
in such layout in memory.
^ permalink raw reply
* [PATCH net-next 0/7] net: Fixes to remote checksum offload and CHECKSUM_PARTIAL
From: Tom Herbert @ 2015-02-11 0:30 UTC (permalink / raw)
To: davem, netdev
This patch set fixes a correctness problem with remote checksum
offload, clarifies the meaning of CHECKSUM_PARTIAL, and allows
remote checksum offload to set CHECKSUM_PARTIAL instead of
calling csum_partial and modifying the checksum.
Specifically:
- In the GRO remote checksum path, restore the checksum after
calling lower layer GRO functions. This is needed if the
packet is forwarded off host with the Remote Checksum Offload
option still present.
- Clarify meaning of CHECKSUM PARTIAL in the receive path. Only
the checksums referred to by checksum partial and any preceding
checksums can be considered verified.
- Fixes to UDP tunnel GRO complete. Need to set SKB_GSO_UDP_TUNNEL_*,
SKB_GSO_TUNNEL_REMCSUM, and skb->encapsulation for forwarding
case.
- Infrastructure to allow setting of CHECKSUM_PARTIAL in remote
checksum offload. This a potential performance benefit instead
of calling csum_partial (potentially twice, once in GRO path
and once in normal path). The downside of using CHECKSUM_PARTIAL
and not actually writing the checksum is that we aren't verifying
that the sender correctly wrote the pseudo checksum into the
checksum field, or that the start/offset values actually point
to a checksum. If the sender did not set up these fields correctly,
a packet might be accepted locally, but not accepted by a peer
when the packet is forwarded off host. Verifying these fields
seems non-trivial, and because the fields can only be incorrect
due to sender error and not corruption (outer checksum protects
against that) we'll make use of CHECKSUM_PARTIAL the default. This
behavior can be reverted as an netlink option on the encapsulation
socket.
- Change VXLAN and GUE to set CHECKSUM_PARTIAL in remote checksum
offload by default, configuration hooks can revert to using
csum_partial.
Testing:
I ran performance numbers using netperf TCP_STREAM and TCP_RR with 200
streams for GRE/GUE and for VXLAN. This compares before the fixes,
the fixes with not setting checksum partial in remote checksum offload,
and with the fixes setting checksum partial. The overall effect seems
be that using checksum partial is a slight performance win, perf
definitely shows a significant reduction of time in csum_partial on
the receive CPUs.
GRE/GUE
TCP_STREAM
Before fixes
9.22% TX CPU utilization
13.57% RX CPU utilization
9133 Mbps
Not using checksum partial
9.59% TX CPU utilization
14.95% RX CPU utilization
9132 Mbps
Using checksum partial
9.37% TX CPU utilization
13.89% RX CPU utilization
9132 Mbps
TCP_RR
Before fixes
CPU utilization
159/251/447 90/95/99% latencies
1.1462e+06 tps
Not using checksum partial
92.94% CPU utilization
158/253/445 90/95/99% latencies
1.12988e+06 tps
Using checksum partial
92.78% CPU utilization
158/250/450 90/95/99% latencies
1.15343e+06 tps
VXLAN
TCP_STREAM
Before fixes
9.24% TX CPU utilization
13.74% RX CPU utilization
9093 Mbps
Not using checksum partial
9.95% TX CPU utilization
14.66% RX CPU utilization
9094 Mbps
Using checksum partial
10.24% TX CPU utilization
13.32% RX CPU utilization
9093 Mbps
TCP_RR
Before fixes
92.91% CPU utilization
151/241/437 90/95/99% latencies
1.15939e+06 tps
Not using checksum partial
93.07% CPU utilization
156/246/425 90/95/99% latencies
1.1451e+06 tps
Using checksum partial
95.51% CPU utilization
156/249/459 90/95/99% latencies
1.17004e+06 tps
Tom Herbert (7):
net: Fix remcsum in GRO path to not change packet
net: Clarify meaning of CHECKSUM_PARTIAL for receive path
udp: Set SKB_GSO_UDP_TUNNEL* in UDP GRO path
net: Use more bit fields in napi_gro_cb
net: Infrastructure for CHECKSUM_PARTIAL with remote checsum offload
vxlan: Use checksum partial with remote checksum offload
gue: Use checksum partial with remote checksum offload
drivers/net/vxlan.c | 38 +++++++++++++++++----------
include/linux/netdevice.h | 62 +++++++++++++++++++++++++++++++++++++-------
include/linux/skbuff.h | 32 ++++++++++++++++++-----
include/net/checksum.h | 5 ++++
include/net/vxlan.h | 4 ++-
include/uapi/linux/fou.h | 1 +
include/uapi/linux/if_link.h | 1 +
net/core/dev.c | 1 +
net/ipv4/fou.c | 42 ++++++++++++++++++++----------
net/ipv4/udp_offload.c | 13 +++++++++-
net/ipv6/udp_offload.c | 6 ++++-
11 files changed, 160 insertions(+), 45 deletions(-)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply
* [PATCH net-next 1/7] net: Fix remcsum in GRO path to not change packet
From: Tom Herbert @ 2015-02-11 0:30 UTC (permalink / raw)
To: davem, netdev
In-Reply-To: <1423614633-25042-1-git-send-email-therbert@google.com>
Remote checksum offload processing is currently the same for both
the GRO and non-GRO path. When the remote checksum offload option
is encountered, the checksum field referred to is modified in
the packet. So in the GRO case, the packet is modified in the
GRO path and then the operation is skipped when the packet goes
through the normal path based on skb->remcsum_offload. There is
a problem in that the packet may be modified in the GRO path, but
then forwarded off host still containing the remote checksum option.
A remote host will again perform RCO but now the checksum verification
will fail since GRO RCO already modified the checksum.
To fix this, we ensure that GRO restores a packet to it's original
state before returning. In this model, when GRO processes a remote
checksum option it still changes the checksum per the algorithm
but on return from lower layer processing the checksum is restored
to its original value.
In this patch we add define gro_remcsum structure which is passed
to skb_gro_remcsum_process to save offset and delta for the checksum
being changed. After lower layer processing, skb_gro_remcsum_cleanup
is called to restore the checksum before returning from GRO.
Signed-off-by: Tom Herbert <therbert@google.com>
---
drivers/net/vxlan.c | 19 +++++++++----------
include/linux/netdevice.h | 25 +++++++++++++++++++++++--
include/net/checksum.h | 5 +++++
net/ipv4/fou.c | 20 ++++++++++----------
4 files changed, 47 insertions(+), 22 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 0e57e86..30310a6 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -555,12 +555,12 @@ static int vxlan_fdb_append(struct vxlan_fdb *f,
static struct vxlanhdr *vxlan_gro_remcsum(struct sk_buff *skb,
unsigned int off,
struct vxlanhdr *vh, size_t hdrlen,
- u32 data)
+ u32 data, struct gro_remcsum *grc)
{
size_t start, offset, plen;
if (skb->remcsum_offload)
- return vh;
+ return NULL;
if (!NAPI_GRO_CB(skb)->csum_valid)
return NULL;
@@ -579,7 +579,8 @@ static struct vxlanhdr *vxlan_gro_remcsum(struct sk_buff *skb,
return NULL;
}
- skb_gro_remcsum_process(skb, (void *)vh + hdrlen, start, offset);
+ skb_gro_remcsum_process(skb, (void *)vh + hdrlen,
+ start, offset, grc);
skb->remcsum_offload = 1;
@@ -597,6 +598,9 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head,
struct vxlan_sock *vs = container_of(uoff, struct vxlan_sock,
udp_offloads);
u32 flags;
+ struct gro_remcsum grc;
+
+ skb_gro_remcsum_init(&grc);
off_vx = skb_gro_offset(skb);
hlen = off_vx + sizeof(*vh);
@@ -614,7 +618,7 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head,
if ((flags & VXLAN_HF_RCO) && (vs->flags & VXLAN_F_REMCSUM_RX)) {
vh = vxlan_gro_remcsum(skb, off_vx, vh, sizeof(struct vxlanhdr),
- ntohl(vh->vx_vni));
+ ntohl(vh->vx_vni), &grc);
if (!vh)
goto out;
@@ -637,6 +641,7 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head,
pp = eth_gro_receive(head, skb);
out:
+ skb_gro_remcsum_cleanup(skb, &grc);
NAPI_GRO_CB(skb)->flush |= flush;
return pp;
@@ -1154,12 +1159,6 @@ static struct vxlanhdr *vxlan_remcsum(struct sk_buff *skb, struct vxlanhdr *vh,
{
size_t start, offset, plen;
- if (skb->remcsum_offload) {
- /* Already processed in GRO path */
- skb->remcsum_offload = 0;
- return vh;
- }
-
start = (data & VXLAN_RCO_MASK) << VXLAN_RCO_SHIFT;
offset = start + ((data & VXLAN_RCO_UDP) ?
offsetof(struct udphdr, check) :
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d115256..3aa0245 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2321,8 +2321,19 @@ do { \
compute_pseudo(skb, proto)); \
} while (0)
+struct gro_remcsum {
+ int offset;
+ __wsum delta;
+};
+
+static inline void skb_gro_remcsum_init(struct gro_remcsum *grc)
+{
+ grc->delta = 0;
+}
+
static inline void skb_gro_remcsum_process(struct sk_buff *skb, void *ptr,
- int start, int offset)
+ int start, int offset,
+ struct gro_remcsum *grc)
{
__wsum delta;
@@ -2331,10 +2342,20 @@ static inline void skb_gro_remcsum_process(struct sk_buff *skb, void *ptr,
delta = remcsum_adjust(ptr, NAPI_GRO_CB(skb)->csum, start, offset);
/* Adjust skb->csum since we changed the packet */
- skb->csum = csum_add(skb->csum, delta);
NAPI_GRO_CB(skb)->csum = csum_add(NAPI_GRO_CB(skb)->csum, delta);
+
+ grc->offset = (ptr + offset) - (void *)skb->head;
+ grc->delta = delta;
}
+static inline void skb_gro_remcsum_cleanup(struct sk_buff *skb,
+ struct gro_remcsum *grc)
+{
+ if (!grc->delta)
+ return;
+
+ remcsum_unadjust((__sum16 *)(skb->head + grc->offset), grc->delta);
+}
static inline int dev_hard_header(struct sk_buff *skb, struct net_device *dev,
unsigned short type,
diff --git a/include/net/checksum.h b/include/net/checksum.h
index e339a95..0a55ac7 100644
--- a/include/net/checksum.h
+++ b/include/net/checksum.h
@@ -167,4 +167,9 @@ static inline __wsum remcsum_adjust(void *ptr, __wsum csum,
return delta;
}
+static inline void remcsum_unadjust(__sum16 *psum, __wsum delta)
+{
+ *psum = csum_fold(csum_sub(delta, *psum));
+}
+
#endif
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 92ddea1..7fa8d36 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -71,12 +71,6 @@ static struct guehdr *gue_remcsum(struct sk_buff *skb, struct guehdr *guehdr,
size_t offset = ntohs(pd[1]);
size_t plen = hdrlen + max_t(size_t, offset + sizeof(u16), start);
- if (skb->remcsum_offload) {
- /* Already processed in GRO path */
- skb->remcsum_offload = 0;
- return guehdr;
- }
-
if (!pskb_may_pull(skb, plen))
return NULL;
guehdr = (struct guehdr *)&udp_hdr(skb)[1];
@@ -214,7 +208,8 @@ out_unlock:
static struct guehdr *gue_gro_remcsum(struct sk_buff *skb, unsigned int off,
struct guehdr *guehdr, void *data,
- size_t hdrlen, u8 ipproto)
+ size_t hdrlen, u8 ipproto,
+ struct gro_remcsum *grc)
{
__be16 *pd = data;
size_t start = ntohs(pd[0]);
@@ -222,7 +217,7 @@ static struct guehdr *gue_gro_remcsum(struct sk_buff *skb, unsigned int off,
size_t plen = hdrlen + max_t(size_t, offset + sizeof(u16), start);
if (skb->remcsum_offload)
- return guehdr;
+ return NULL;
if (!NAPI_GRO_CB(skb)->csum_valid)
return NULL;
@@ -234,7 +229,8 @@ static struct guehdr *gue_gro_remcsum(struct sk_buff *skb, unsigned int off,
return NULL;
}
- skb_gro_remcsum_process(skb, (void *)guehdr + hdrlen, start, offset);
+ skb_gro_remcsum_process(skb, (void *)guehdr + hdrlen,
+ start, offset, grc);
skb->remcsum_offload = 1;
@@ -254,6 +250,9 @@ static struct sk_buff **gue_gro_receive(struct sk_buff **head,
void *data;
u16 doffset = 0;
int flush = 1;
+ struct gro_remcsum grc;
+
+ skb_gro_remcsum_init(&grc);
off = skb_gro_offset(skb);
len = off + sizeof(*guehdr);
@@ -295,7 +294,7 @@ static struct sk_buff **gue_gro_receive(struct sk_buff **head,
if (flags & GUE_PFLAG_REMCSUM) {
guehdr = gue_gro_remcsum(skb, off, guehdr,
data + doffset, hdrlen,
- guehdr->proto_ctype);
+ guehdr->proto_ctype, &grc);
if (!guehdr)
goto out;
@@ -345,6 +344,7 @@ out_unlock:
rcu_read_unlock();
out:
NAPI_GRO_CB(skb)->flush |= flush;
+ skb_gro_remcsum_cleanup(skb, &grc);
return pp;
}
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 2/7] net: Clarify meaning of CHECKSUM_PARTIAL for receive path
From: Tom Herbert @ 2015-02-11 0:30 UTC (permalink / raw)
To: davem, netdev
In-Reply-To: <1423614633-25042-1-git-send-email-therbert@google.com>
The current meaning of CHECKSUM_PARTIAL for validating checksums
is that _all_ checksums in the packet are considered valid.
However, in the manner that CHECKSUM_PARTIAL is set only the checksum
at csum_start+csum_offset and any preceding checksums may
be considered valid. If there are checksums in the packet after
csum_offset it is possible they have not been verfied.
This patch changes CHECKSUM_PARTIAL logic in skb_csum_unnecessary and
__skb_gro_checksum_validate_needed to only considered checksums
referring to csum_start and any preceding checksums (with starting
offset before csum_start) to be verified.
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/linux/netdevice.h | 4 +++-
include/linux/skbuff.h | 17 ++++++++++++-----
2 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 3aa0245..9e9be22 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2246,7 +2246,9 @@ static inline bool __skb_gro_checksum_validate_needed(struct sk_buff *skb,
bool zero_okay,
__sum16 check)
{
- return (skb->ip_summed != CHECKSUM_PARTIAL &&
+ return ((skb->ip_summed != CHECKSUM_PARTIAL ||
+ skb_checksum_start_offset(skb) <
+ skb_gro_offset(skb)) &&
NAPI_GRO_CB(skb)->csum_cnt == 0 &&
(!zero_okay || check));
}
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 1bb36ed..da6028a 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -83,11 +83,15 @@
*
* CHECKSUM_PARTIAL:
*
- * This is identical to the case for output below. This may occur on a packet
+ * A checksum is set up to be offloaded to a device as described in the
+ * output description for CHECKSUM_PARTIAL. This may occur on a packet
* received directly from another Linux OS, e.g., a virtualized Linux kernel
- * on the same host. The packet can be treated in the same way as
- * CHECKSUM_UNNECESSARY, except that on output (i.e., forwarding) the
- * checksum must be filled in by the OS or the hardware.
+ * on the same host, or it may be set in the input path in GRO or remote
+ * checksum offload. For the purposes of checksum verification, the checksum
+ * referred to by skb->csum_start + skb->csum_offset and any preceding
+ * checksums in the packet are considered verified. Any checksums in the
+ * packet that are after the checksum being offloaded are not considered to
+ * be verified.
*
* B. Checksumming on output.
*
@@ -2915,7 +2919,10 @@ __sum16 __skb_checksum_complete(struct sk_buff *skb);
static inline int skb_csum_unnecessary(const struct sk_buff *skb)
{
- return ((skb->ip_summed & CHECKSUM_UNNECESSARY) || skb->csum_valid);
+ return ((skb->ip_summed == CHECKSUM_UNNECESSARY) ||
+ skb->csum_valid ||
+ (skb->ip_summed == CHECKSUM_PARTIAL &&
+ skb_checksum_start_offset(skb) >= 0));
}
/**
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 3/7] udp: Set SKB_GSO_UDP_TUNNEL* in UDP GRO path
From: Tom Herbert @ 2015-02-11 0:30 UTC (permalink / raw)
To: davem, netdev
In-Reply-To: <1423614633-25042-1-git-send-email-therbert@google.com>
Properly set GSO types and skb->encapsulation in the UDP tunnel GRO
complete so that packets are properly represented for GSO. This sets
SKB_GSO_UDP_TUNNEL or SKB_GSO_UDP_TUNNEL_CSUM depending on whether
non-zero checksums were received, and sets SKB_GSO_TUNNEL_REMCSUM if
the remote checksum option was processed.
Signed-off-by: Tom Herbert <therbert@google.com>
---
net/ipv4/udp_offload.c | 13 ++++++++++++-
net/ipv6/udp_offload.c | 6 +++++-
2 files changed, 17 insertions(+), 2 deletions(-)
diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
index d10f6f4..4915d82 100644
--- a/net/ipv4/udp_offload.c
+++ b/net/ipv4/udp_offload.c
@@ -402,6 +402,13 @@ int udp_gro_complete(struct sk_buff *skb, int nhoff)
}
rcu_read_unlock();
+
+ if (skb->remcsum_offload)
+ skb_shinfo(skb)->gso_type |= SKB_GSO_TUNNEL_REMCSUM;
+
+ skb->encapsulation = 1;
+ skb_set_inner_mac_header(skb, nhoff + sizeof(struct udphdr));
+
return err;
}
@@ -410,9 +417,13 @@ static int udp4_gro_complete(struct sk_buff *skb, int nhoff)
const struct iphdr *iph = ip_hdr(skb);
struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
- if (uh->check)
+ if (uh->check) {
+ skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL_CSUM;
uh->check = ~udp_v4_check(skb->len - nhoff, iph->saddr,
iph->daddr, 0);
+ } else {
+ skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL;
+ }
return udp_gro_complete(skb, nhoff);
}
diff --git a/net/ipv6/udp_offload.c b/net/ipv6/udp_offload.c
index a562769..ab889bb 100644
--- a/net/ipv6/udp_offload.c
+++ b/net/ipv6/udp_offload.c
@@ -161,9 +161,13 @@ static int udp6_gro_complete(struct sk_buff *skb, int nhoff)
const struct ipv6hdr *ipv6h = ipv6_hdr(skb);
struct udphdr *uh = (struct udphdr *)(skb->data + nhoff);
- if (uh->check)
+ if (uh->check) {
+ skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL_CSUM;
uh->check = ~udp_v6_check(skb->len - nhoff, &ipv6h->saddr,
&ipv6h->daddr, 0);
+ } else {
+ skb_shinfo(skb)->gso_type |= SKB_GSO_UDP_TUNNEL;
+ }
return udp_gro_complete(skb, nhoff);
}
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 4/7] net: Use more bit fields in napi_gro_cb
From: Tom Herbert @ 2015-02-11 0:30 UTC (permalink / raw)
To: davem, netdev
In-Reply-To: <1423614633-25042-1-git-send-email-therbert@google.com>
This patch moves the free and same_flow fields to be bit fields
(2 and 1 bit sized respectively). This frees up some space for u16's.
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/linux/netdevice.h | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 9e9be22..43fd0a4 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1923,20 +1923,15 @@ struct napi_gro_cb {
/* Number of segments aggregated. */
u16 count;
- /* This is non-zero if the packet may be of the same flow. */
- u8 same_flow;
-
- /* Free the skb? */
- u8 free;
-#define NAPI_GRO_FREE 1
-#define NAPI_GRO_FREE_STOLEN_HEAD 2
-
/* jiffies when first packet was created/queued */
unsigned long age;
/* Used in ipv6_gro_receive() and foo-over-udp */
u16 proto;
+ /* This is non-zero if the packet may be of the same flow. */
+ u8 same_flow:1;
+
/* Used in udp_gro_receive */
u8 udp_mark:1;
@@ -1946,9 +1941,16 @@ struct napi_gro_cb {
/* Number of checksums via CHECKSUM_UNNECESSARY */
u8 csum_cnt:3;
+ /* Free the skb? */
+ u8 free:2;
+#define NAPI_GRO_FREE 1
+#define NAPI_GRO_FREE_STOLEN_HEAD 2
+
/* Used in foo-over-udp, set in udp[46]_gro_receive */
u8 is_ipv6:1;
+ /* 7 bit hole */
+
/* used to support CHECKSUM_COMPLETE for tunneling protocols */
__wsum csum;
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 5/7] net: Infrastructure for CHECKSUM_PARTIAL with remote checsum offload
From: Tom Herbert @ 2015-02-11 0:30 UTC (permalink / raw)
To: davem, netdev
In-Reply-To: <1423614633-25042-1-git-send-email-therbert@google.com>
This patch adds infrastructure so that remote checksum offload can
set CHECKSUM_PARTIAL instead of calling csum_partial and writing
the modfied checksum field.
Add skb_remcsum_adjust_partial function to set an skb for using
CHECKSUM_PARTIAL with remote checksum offload. Changed
skb_remcsum_process and skb_gro_remcsum_process to take a boolean
argument to indicate if checksum partial can be set or the
checksum needs to be modified using the normal algorithm.
Signed-off-by: Tom Herbert <therbert@google.com>
---
drivers/net/vxlan.c | 4 ++--
include/linux/netdevice.h | 19 ++++++++++++++++++-
include/linux/skbuff.h | 15 ++++++++++++++-
net/core/dev.c | 1 +
net/ipv4/fou.c | 4 ++--
5 files changed, 37 insertions(+), 6 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 30310a6..4f04443 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -580,7 +580,7 @@ static struct vxlanhdr *vxlan_gro_remcsum(struct sk_buff *skb,
}
skb_gro_remcsum_process(skb, (void *)vh + hdrlen,
- start, offset, grc);
+ start, offset, grc, true);
skb->remcsum_offload = 1;
@@ -1171,7 +1171,7 @@ static struct vxlanhdr *vxlan_remcsum(struct sk_buff *skb, struct vxlanhdr *vh,
vh = (struct vxlanhdr *)(udp_hdr(skb) + 1);
- skb_remcsum_process(skb, (void *)vh + hdrlen, start, offset);
+ skb_remcsum_process(skb, (void *)vh + hdrlen, start, offset, true);
return vh;
}
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 43fd0a4..5897b4e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1923,6 +1923,9 @@ struct napi_gro_cb {
/* Number of segments aggregated. */
u16 count;
+ /* Start offset for remote checksum offload */
+ u16 gro_remcsum_start;
+
/* jiffies when first packet was created/queued */
unsigned long age;
@@ -2244,6 +2247,12 @@ static inline void skb_gro_postpull_rcsum(struct sk_buff *skb,
__sum16 __skb_gro_checksum_complete(struct sk_buff *skb);
+static inline bool skb_at_gro_remcsum_start(struct sk_buff *skb)
+{
+ return (NAPI_GRO_CB(skb)->gro_remcsum_start - skb_headroom(skb) ==
+ skb_gro_offset(skb));
+}
+
static inline bool __skb_gro_checksum_validate_needed(struct sk_buff *skb,
bool zero_okay,
__sum16 check)
@@ -2251,6 +2260,7 @@ static inline bool __skb_gro_checksum_validate_needed(struct sk_buff *skb,
return ((skb->ip_summed != CHECKSUM_PARTIAL ||
skb_checksum_start_offset(skb) <
skb_gro_offset(skb)) &&
+ !skb_at_gro_remcsum_start(skb) &&
NAPI_GRO_CB(skb)->csum_cnt == 0 &&
(!zero_okay || check));
}
@@ -2337,12 +2347,19 @@ static inline void skb_gro_remcsum_init(struct gro_remcsum *grc)
static inline void skb_gro_remcsum_process(struct sk_buff *skb, void *ptr,
int start, int offset,
- struct gro_remcsum *grc)
+ struct gro_remcsum *grc,
+ bool nopartial)
{
__wsum delta;
BUG_ON(!NAPI_GRO_CB(skb)->csum_valid);
+ if (!nopartial) {
+ NAPI_GRO_CB(skb)->gro_remcsum_start =
+ ((unsigned char *)ptr + start) - skb->head;
+ return;
+ }
+
delta = remcsum_adjust(ptr, NAPI_GRO_CB(skb)->csum, start, offset);
/* Adjust skb->csum since we changed the packet */
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index da6028a..30007af 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -3104,16 +3104,29 @@ do { \
compute_pseudo(skb, proto)); \
} while (0)
+static inline void skb_remcsum_adjust_partial(struct sk_buff *skb, void *ptr,
+ u16 start, u16 offset)
+{
+ skb->ip_summed = CHECKSUM_PARTIAL;
+ skb->csum_start = ((unsigned char *)ptr + start) - skb->head;
+ skb->csum_offset = offset - start;
+}
+
/* Update skbuf and packet to reflect the remote checksum offload operation.
* When called, ptr indicates the starting point for skb->csum when
* ip_summed is CHECKSUM_COMPLETE. If we need create checksum complete
* here, skb_postpull_rcsum is done so skb->csum start is ptr.
*/
static inline void skb_remcsum_process(struct sk_buff *skb, void *ptr,
- int start, int offset)
+ int start, int offset, bool nopartial)
{
__wsum delta;
+ if (!nopartial) {
+ skb_remcsum_adjust_partial(skb, ptr, start, offset);
+ return;
+ }
+
if (unlikely(skb->ip_summed != CHECKSUM_COMPLETE)) {
__skb_checksum_complete(skb);
skb_postpull_rcsum(skb, skb->data, ptr - (void *)skb->data);
diff --git a/net/core/dev.c b/net/core/dev.c
index d030575..48c6ecb 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4024,6 +4024,7 @@ static enum gro_result dev_gro_receive(struct napi_struct *napi, struct sk_buff
NAPI_GRO_CB(skb)->flush = 0;
NAPI_GRO_CB(skb)->free = 0;
NAPI_GRO_CB(skb)->udp_mark = 0;
+ NAPI_GRO_CB(skb)->gro_remcsum_start = 0;
/* Setup for GRO checksum validation */
switch (skb->ip_summed) {
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index 7fa8d36..d320f57 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -75,7 +75,7 @@ static struct guehdr *gue_remcsum(struct sk_buff *skb, struct guehdr *guehdr,
return NULL;
guehdr = (struct guehdr *)&udp_hdr(skb)[1];
- skb_remcsum_process(skb, (void *)guehdr + hdrlen, start, offset);
+ skb_remcsum_process(skb, (void *)guehdr + hdrlen, start, offset, true);
return guehdr;
}
@@ -230,7 +230,7 @@ static struct guehdr *gue_gro_remcsum(struct sk_buff *skb, unsigned int off,
}
skb_gro_remcsum_process(skb, (void *)guehdr + hdrlen,
- start, offset, grc);
+ start, offset, grc, true);
skb->remcsum_offload = 1;
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 6/7] vxlan: Use checksum partial with remote checksum offload
From: Tom Herbert @ 2015-02-11 0:30 UTC (permalink / raw)
To: davem, netdev
In-Reply-To: <1423614633-25042-1-git-send-email-therbert@google.com>
Change remote checksum handling to set checksum partial as default
behavior. Added an iflink parameter to configure not using
checksum partial (calling csum_partial to update checksum).
Signed-off-by: Tom Herbert <therbert@google.com>
---
drivers/net/vxlan.c | 25 +++++++++++++++++++------
include/net/vxlan.h | 4 +++-
include/uapi/linux/if_link.h | 1 +
3 files changed, 23 insertions(+), 7 deletions(-)
diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 4f04443..1e0a775 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -555,7 +555,8 @@ static int vxlan_fdb_append(struct vxlan_fdb *f,
static struct vxlanhdr *vxlan_gro_remcsum(struct sk_buff *skb,
unsigned int off,
struct vxlanhdr *vh, size_t hdrlen,
- u32 data, struct gro_remcsum *grc)
+ u32 data, struct gro_remcsum *grc,
+ bool nopartial)
{
size_t start, offset, plen;
@@ -580,7 +581,7 @@ static struct vxlanhdr *vxlan_gro_remcsum(struct sk_buff *skb,
}
skb_gro_remcsum_process(skb, (void *)vh + hdrlen,
- start, offset, grc, true);
+ start, offset, grc, nopartial);
skb->remcsum_offload = 1;
@@ -618,7 +619,9 @@ static struct sk_buff **vxlan_gro_receive(struct sk_buff **head,
if ((flags & VXLAN_HF_RCO) && (vs->flags & VXLAN_F_REMCSUM_RX)) {
vh = vxlan_gro_remcsum(skb, off_vx, vh, sizeof(struct vxlanhdr),
- ntohl(vh->vx_vni), &grc);
+ ntohl(vh->vx_vni), &grc,
+ !!(vs->flags &
+ VXLAN_F_REMCSUM_NOPARTIAL));
if (!vh)
goto out;
@@ -1155,7 +1158,7 @@ static void vxlan_igmp_leave(struct work_struct *work)
}
static struct vxlanhdr *vxlan_remcsum(struct sk_buff *skb, struct vxlanhdr *vh,
- size_t hdrlen, u32 data)
+ size_t hdrlen, u32 data, bool nopartial)
{
size_t start, offset, plen;
@@ -1171,7 +1174,8 @@ static struct vxlanhdr *vxlan_remcsum(struct sk_buff *skb, struct vxlanhdr *vh,
vh = (struct vxlanhdr *)(udp_hdr(skb) + 1);
- skb_remcsum_process(skb, (void *)vh + hdrlen, start, offset, true);
+ skb_remcsum_process(skb, (void *)vh + hdrlen, start, offset,
+ nopartial);
return vh;
}
@@ -1208,7 +1212,8 @@ static int vxlan_udp_encap_recv(struct sock *sk, struct sk_buff *skb)
goto drop;
if ((flags & VXLAN_HF_RCO) && (vs->flags & VXLAN_F_REMCSUM_RX)) {
- vxh = vxlan_remcsum(skb, vxh, sizeof(struct vxlanhdr), vni);
+ vxh = vxlan_remcsum(skb, vxh, sizeof(struct vxlanhdr), vni,
+ !!(vs->flags & VXLAN_F_REMCSUM_NOPARTIAL));
if (!vxh)
goto drop;
@@ -2437,6 +2442,7 @@ static const struct nla_policy vxlan_policy[IFLA_VXLAN_MAX + 1] = {
[IFLA_VXLAN_REMCSUM_TX] = { .type = NLA_U8 },
[IFLA_VXLAN_REMCSUM_RX] = { .type = NLA_U8 },
[IFLA_VXLAN_GBP] = { .type = NLA_FLAG, },
+ [IFLA_VXLAN_REMCSUM_NOPARTIAL] = { .type = NLA_FLAG },
};
static int vxlan_validate(struct nlattr *tb[], struct nlattr *data[])
@@ -2760,6 +2766,9 @@ static int vxlan_newlink(struct net *src_net, struct net_device *dev,
if (data[IFLA_VXLAN_GBP])
vxlan->flags |= VXLAN_F_GBP;
+ if (data[IFLA_VXLAN_REMCSUM_NOPARTIAL])
+ vxlan->flags |= VXLAN_F_REMCSUM_NOPARTIAL;
+
if (vxlan_find_vni(src_net, vni, use_ipv6 ? AF_INET6 : AF_INET,
vxlan->dst_port, vxlan->flags)) {
pr_info("duplicate VNI %u\n", vni);
@@ -2909,6 +2918,10 @@ static int vxlan_fill_info(struct sk_buff *skb, const struct net_device *dev)
nla_put_flag(skb, IFLA_VXLAN_GBP))
goto nla_put_failure;
+ if (vxlan->flags & VXLAN_F_REMCSUM_NOPARTIAL &&
+ nla_put_flag(skb, IFLA_VXLAN_REMCSUM_NOPARTIAL))
+ goto nla_put_failure;
+
return 0;
nla_put_failure:
diff --git a/include/net/vxlan.h b/include/net/vxlan.h
index 2927d62..eabd3a0 100644
--- a/include/net/vxlan.h
+++ b/include/net/vxlan.h
@@ -128,13 +128,15 @@ struct vxlan_sock {
#define VXLAN_F_REMCSUM_TX 0x200
#define VXLAN_F_REMCSUM_RX 0x400
#define VXLAN_F_GBP 0x800
+#define VXLAN_F_REMCSUM_NOPARTIAL 0x1000
/* Flags that are used in the receive patch. These flags must match in
* order for a socket to be shareable
*/
#define VXLAN_F_RCV_FLAGS (VXLAN_F_GBP | \
VXLAN_F_UDP_ZERO_CSUM6_RX | \
- VXLAN_F_REMCSUM_RX)
+ VXLAN_F_REMCSUM_RX | \
+ VXLAN_F_REMCSUM_NOPARTIAL)
struct vxlan_sock *vxlan_sock_add(struct net *net, __be16 port,
vxlan_rcv_t *rcv, void *data,
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 0deee3e..dfd0bb2 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -374,6 +374,7 @@ enum {
IFLA_VXLAN_REMCSUM_TX,
IFLA_VXLAN_REMCSUM_RX,
IFLA_VXLAN_GBP,
+ IFLA_VXLAN_REMCSUM_NOPARTIAL,
__IFLA_VXLAN_MAX
};
#define IFLA_VXLAN_MAX (__IFLA_VXLAN_MAX - 1)
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* [PATCH net-next 7/7] gue: Use checksum partial with remote checksum offload
From: Tom Herbert @ 2015-02-11 0:30 UTC (permalink / raw)
To: davem, netdev
In-Reply-To: <1423614633-25042-1-git-send-email-therbert@google.com>
Change remote checksum handling to set checksum partial as default
behavior. Added an iflink parameter to configure not using
checksum partial (calling csum_partial to update checksum).
Signed-off-by: Tom Herbert <therbert@google.com>
---
include/uapi/linux/fou.h | 1 +
net/ipv4/fou.c | 28 ++++++++++++++++++++++------
2 files changed, 23 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/fou.h b/include/uapi/linux/fou.h
index 8df0689..c303588 100644
--- a/include/uapi/linux/fou.h
+++ b/include/uapi/linux/fou.h
@@ -14,6 +14,7 @@ enum {
FOU_ATTR_AF, /* u8 */
FOU_ATTR_IPPROTO, /* u8 */
FOU_ATTR_TYPE, /* u8 */
+ FOU_ATTR_REMCSUM_NOPARTIAL, /* flag */
__FOU_ATTR_MAX,
};
diff --git a/net/ipv4/fou.c b/net/ipv4/fou.c
index d320f57..ff069f6 100644
--- a/net/ipv4/fou.c
+++ b/net/ipv4/fou.c
@@ -22,14 +22,18 @@ static LIST_HEAD(fou_list);
struct fou {
struct socket *sock;
u8 protocol;
+ u8 flags;
u16 port;
struct udp_offload udp_offloads;
struct list_head list;
};
+#define FOU_F_REMCSUM_NOPARTIAL BIT(0)
+
struct fou_cfg {
u16 type;
u8 protocol;
+ u8 flags;
struct udp_port_cfg udp_config;
};
@@ -64,7 +68,8 @@ static int fou_udp_recv(struct sock *sk, struct sk_buff *skb)
}
static struct guehdr *gue_remcsum(struct sk_buff *skb, struct guehdr *guehdr,
- void *data, size_t hdrlen, u8 ipproto)
+ void *data, size_t hdrlen, u8 ipproto,
+ bool nopartial)
{
__be16 *pd = data;
size_t start = ntohs(pd[0]);
@@ -75,7 +80,8 @@ static struct guehdr *gue_remcsum(struct sk_buff *skb, struct guehdr *guehdr,
return NULL;
guehdr = (struct guehdr *)&udp_hdr(skb)[1];
- skb_remcsum_process(skb, (void *)guehdr + hdrlen, start, offset, true);
+ skb_remcsum_process(skb, (void *)guehdr + hdrlen,
+ start, offset, nopartial);
return guehdr;
}
@@ -136,7 +142,9 @@ static int gue_udp_recv(struct sock *sk, struct sk_buff *skb)
if (flags & GUE_PFLAG_REMCSUM) {
guehdr = gue_remcsum(skb, guehdr, data + doffset,
- hdrlen, guehdr->proto_ctype);
+ hdrlen, guehdr->proto_ctype,
+ !!(fou->flags &
+ FOU_F_REMCSUM_NOPARTIAL));
if (!guehdr)
goto drop;
@@ -209,7 +217,7 @@ out_unlock:
static struct guehdr *gue_gro_remcsum(struct sk_buff *skb, unsigned int off,
struct guehdr *guehdr, void *data,
size_t hdrlen, u8 ipproto,
- struct gro_remcsum *grc)
+ struct gro_remcsum *grc, bool nopartial)
{
__be16 *pd = data;
size_t start = ntohs(pd[0]);
@@ -230,7 +238,7 @@ static struct guehdr *gue_gro_remcsum(struct sk_buff *skb, unsigned int off,
}
skb_gro_remcsum_process(skb, (void *)guehdr + hdrlen,
- start, offset, grc, true);
+ start, offset, grc, nopartial);
skb->remcsum_offload = 1;
@@ -250,6 +258,7 @@ static struct sk_buff **gue_gro_receive(struct sk_buff **head,
void *data;
u16 doffset = 0;
int flush = 1;
+ struct fou *fou = container_of(uoff, struct fou, udp_offloads);
struct gro_remcsum grc;
skb_gro_remcsum_init(&grc);
@@ -294,7 +303,9 @@ static struct sk_buff **gue_gro_receive(struct sk_buff **head,
if (flags & GUE_PFLAG_REMCSUM) {
guehdr = gue_gro_remcsum(skb, off, guehdr,
data + doffset, hdrlen,
- guehdr->proto_ctype, &grc);
+ guehdr->proto_ctype, &grc,
+ !!(fou->flags &
+ FOU_F_REMCSUM_NOPARTIAL));
if (!guehdr)
goto out;
@@ -455,6 +466,7 @@ static int fou_create(struct net *net, struct fou_cfg *cfg,
sk = sock->sk;
+ fou->flags = cfg->flags;
fou->port = cfg->udp_config.local_udp_port;
/* Initial for fou type */
@@ -541,6 +553,7 @@ static struct nla_policy fou_nl_policy[FOU_ATTR_MAX + 1] = {
[FOU_ATTR_AF] = { .type = NLA_U8, },
[FOU_ATTR_IPPROTO] = { .type = NLA_U8, },
[FOU_ATTR_TYPE] = { .type = NLA_U8, },
+ [FOU_ATTR_REMCSUM_NOPARTIAL] = { .type = NLA_FLAG, },
};
static int parse_nl_config(struct genl_info *info,
@@ -571,6 +584,9 @@ static int parse_nl_config(struct genl_info *info,
if (info->attrs[FOU_ATTR_TYPE])
cfg->type = nla_get_u8(info->attrs[FOU_ATTR_TYPE]);
+ if (info->attrs[FOU_ATTR_REMCSUM_NOPARTIAL])
+ cfg->flags |= FOU_F_REMCSUM_NOPARTIAL;
+
return 0;
}
--
2.2.0.rc0.207.ga3a616c
^ permalink raw reply related
* Re: [PATCH net-next 1/7] ebpf: remove kernel test stubs
From: Alexei Starovoitov @ 2015-02-11 0:42 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Jiří Pírko, Network Development
In-Reply-To: <0772f0348f40e517e19a838877f417b8932b95a9.1423610452.git.daniel@iogearbox.net>
On Tue, Feb 10, 2015 at 4:15 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> Now that we have BPF_PROG_TYPE_SOCKET_FILTER up and running,
> we can remove the test stubs which were added to get the
> verifier suite up. We can just let the test cases probe under
> socket filter type instead. In the fill/spill test case, we
> cannot (yet) access fields from the context (skb), but we may
> adapt that test case in future.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Alexei Starovoitov <ast@plumgrid.com>
has been on my todo list for a while. Thanks a bunch!
^ permalink raw reply
* Re: [PATCH net-next 2/7] ebpf: constify various function pointer structs
From: Alexei Starovoitov @ 2015-02-11 0:43 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Jiří Pírko, Network Development
In-Reply-To: <1be48c6154312f8c3fd239c26fae856fa3416bd8.1423610452.git.daniel@iogearbox.net>
On Tue, Feb 10, 2015 at 4:15 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> We can move bpf_map_ops and bpf_verifier_ops and other structs
> into RO section, bpf_map_type_list and bpf_prog_type_list into
> read mostly.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
sure. makes sense.
Acked-by: Alexei Starovoitov <ast@plumgrid.com>
^ permalink raw reply
* Re: [PATCH v3 linux-trace 1/8] tracing: attach eBPF programs to tracepoints and syscalls
From: Steven Rostedt @ 2015-02-11 0:50 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Ingo Molnar, Namhyung Kim, Arnaldo Carvalho de Melo, Jiri Olsa,
Masami Hiramatsu, Linux API, Network Development, LKML,
Linus Torvalds, Peter Zijlstra, Eric W. Biederman
In-Reply-To: <CAMEtUuzY_Po=WtFEFg1aqzJ8dEF4rHGcWDsaS44KYgACMNPPgA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Tue, 10 Feb 2015 16:22:50 -0800
Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> > Yep, and if this becomes a standard, then any change that makes
> > trace_pipe different will be reverted.
>
> I think reading of trace_pipe is widespread.
I've heard of a few, but nothing that has broken when something changed.
Is it scripts or actual C code?
Again, it matters if people complain about the change.
> >> But some maintainers think of them as ABI, whereas others
> >> are using them freely. imo it's time to remove ambiguity.
> >
> > I would love to, and have brought this up at Kernel Summit more than
> > once with no solution out of it.
>
> let's try it again at plumbers in august?
Well, we need a statement from Linus. And it would be nice if we could
also get Ingo involved in the discussion, but he seldom comes to
anything but Kernel Summit.
>
> For now I'm going to drop bpf+tracepoints, since it's so controversial
> and go with bpf+syscall and bpf+kprobe only.
Probably the safest bet.
>
> Hopefully by august it will be clear what bpf+kprobes can do
> and why I'm excited about bpf+tracepoints in the future.
BTW, I wonder if I could make a simple compiler in the kernel that
would translate the current ftrace filters into a BPF program, where it
could use the program and not use the current filter logic.
> >> These tracepoint will receive one or two pointers to important
> >> structs only. They will not have TP_printk, assign and fields.
> >> The placement and arguments to these new tracepoints
> >> will be an ABI.
> >> All existing tracepoints are not.
> >
> > TP_printk() is not really an issue.
>
> I think it is. The way things are printed is the most
> visible part of tracepoints and I suspect maintainers don't
> want to add new ones, because internal fields are printed
> and users do parse trace_pipe.
> Recent discussion about tcp instrumentation was
> about adding new tracepoints and a module to print them.
> As soon as something like this is in, the next question
> 'what we're going to do when more arguments need
> to be printed'...
I should rephrase that. It's not that it's not an issue, it's just that
it hasn't been an issue. the trace_pipe code is slow. The
raw_trace_pipe is much faster. Any tool would benefit from using it.
I really need to get a library out to help users do such a thing.
>
> imo the solution is DEFINE_EVENT_BPF that doesn't
> print anything and a bpf program to process it.
You mean to be completely invisible to ftrace? And the debugfs/tracefs
directory?
> >
> >> it is portable and will run on any kernel.
> >> In uapi header we can define:
> >> struct task_struct_user {
> >> int pid;
> >> int prio;
> >
> > Here's a perfect example of something that looks stable to show to
> > user space, but is really a pimple that is hiding cancer.
> >
> > Lets start with pid. We have name spaces. What pid will be put there?
> > We have to show the pid of the name space it is under.
> >
> > Then we have prio. What is prio in the DEADLINE scheduler. It is rather
> > meaningless. Also, it is meaningless in SCHED_OTHER.
> >
> > Also note that even for SCHED_FIFO, the prio is used differently in the
> > kernel than it is in userspace. For the kernel, lower is higher.
>
> well, ->prio and ->pid are already printed by sched tracepoints
> and their meaning depends on scheduler. So users taking that
> into account.
I know, and Peter hates this.
> I'm not suggesting to preserve the meaning of 'pid' semantically
> in all cases. That's not what users would want anyway.
> I want to allow programs to access important fields and print
> them in more generic way than current TP_printk does.
> Then exposed ABI of such tracepoint_bpf is smaller than
> with current tracepoints.
Again, this would mean they become invisible to ftrace, and even
ftrace_dump_on_oops.
I'm not fully understanding what is to be exported by this new ABI. If
the fields available, will always be available, then why can't the
appear in a TP_printk()?
> > eBPF is very flexible, which means it is bound to have someone use it
> > in a way you never dreamed of, and that will be what bites you in the
> > end (pun intended).
>
> understood :)
> let's start slow then with bpf+syscall and bpf+kprobe only.
I'm fine with that.
>
> >> also not all bpf programs are equal.
> >> bpf+existing tracepoint is not ABI
> >
> > Why not?
>
> well, because we want to see more tracepoints in the kernel.
> We're already struggling to add more.
Still, the question is, even with a new "tracepoint" that limits what
it shows, there still needs to be something that is guaranteed to
always be there. I still don't see how this is different than the
current tracepoints.
>
> >> bpf+new tracepoint is ABI if programs are not using bpf_fetch
> >
> > How is this different?
>
> the new ones will be explicit by definition.
Who monitors this?
> > To give you an example, we thought about scrambling the trace event
> > field locations from boot to boot to keep tools from hard coding the
> > event layout. This may sound crazy, but developers out there are crazy.
> > And if you want to keep them from abusing interfaces, you just need to
> > be a bit more crazy than they are.
>
> that is indeed crazy. the point is understood.
>
> right now I cannot think of a solid way to prevent abuse
> of bpf+tracepoint, so just going to drop it for now.
Welcome to our world ;-)
> Cool things can be done with bpf+kprobe/syscall already.
True.
-- Steve
^ permalink raw reply
* Re: [PATCH net-next 3/7] ebpf: check first for MAXINSNS in bpf_prog_load
From: Alexei Starovoitov @ 2015-02-11 1:21 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Jiří Pírko, Network Development
In-Reply-To: <a1a9810494aebf0c0be8660e996b84c1cdb06444.1423610452.git.daniel@iogearbox.net>
On Tue, Feb 10, 2015 at 4:15 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> Just minor ... before doing all the copying work, we may want
> to check for instruction count earlier. Also, we may want to
> warn the user in case we would otherwise need to truncate the
> license information.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
> kernel/bpf/syscall.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 536edc2..73b105c 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -473,25 +473,26 @@ static int bpf_prog_load(union bpf_attr *attr)
> {
> enum bpf_prog_type type = attr->prog_type;
> struct bpf_prog *prog;
> - int err;
> char license[128];
> bool is_gpl;
> + int err;
>
> if (CHECK_ATTR(BPF_PROG_LOAD))
> return -EINVAL;
> + if (attr->insn_cnt >= BPF_MAXINSNS)
> + return -EINVAL;
>
> /* copy eBPF program license from user space */
> - if (strncpy_from_user(license, u64_to_ptr(attr->license),
> - sizeof(license) - 1) < 0)
> - return -EFAULT;
> - license[sizeof(license) - 1] = 0;
> + err = strncpy_from_user(license, u64_to_ptr(attr->license),
> + sizeof(license));
> + if (err == sizeof(license))
> + err = -ERANGE;
I think this error is misleading.
In case of license we care whether it's gpl or not.
This boolean indicator we remember for the life of the program.
We don't keep the license.
So if user specified 'my_ultra_long_proprietery_license'
that should be fine. The program should still be accepted
and marked as non-gpl.
> + if (err < 0)
> + return err;
>
> /* eBPF programs must be GPL compatible to use GPL-ed functions */
> is_gpl = license_is_gpl_compatible(license);
>
> - if (attr->insn_cnt >= BPF_MAXINSNS)
> - return -EINVAL;
moving this check, I guess, is fine. No one should depend
on order of errors.
^ permalink raw reply
* Re: [PATCH net-next 4/7] ebpf: extend program type/subsystem registration
From: Alexei Starovoitov @ 2015-02-11 1:37 UTC (permalink / raw)
To: Daniel Borkmann, Thomas Graf
Cc: Jiří Pírko, Network Development
On Tue, Feb 10, 2015 at 4:15 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> When various subsystems/modules start to make use of ebpf
> e.g. cls_bpf, act_bpf, ovs, ... we need to make sure, they
> can register their program types only once.
>
> Moreover, we also need to serialize various registrations,
> currently program type registration is being done without
> locks. (We should make sure to not race in future when we
> allow registration from modules.)
>
> Last but not least, we need to be able to register subsystems
> from module context as it's not sufficient to have them only
> as built-in at all time.
imo that is scary. there is no unregister_type by design,
since I didn't want modules to use bpf.
My concern that if we allow modules to register new program
types we allow bypass of gpl and all safety checks we've
put in place.
Modules will define whatever helper functions they like.
We won't be able to control or even code review this stuff.
I think all types and all helper functions should be built-in.
This way we know what bpf is used for and that it's not abused.
Yes, having all types and helpers built-in creates
some challenges for ovs and tc, but I think it's much
safer long term.
I think for networking one or two prog types will be enough
and hopefully ovs/tc/... will settle on common set of helpers.
'bpf for modules' was one of the topics I wanted to
discuss at netdev01. Looks like we started it early...
^ permalink raw reply
* Re: [PATCH net-next 5/7] ebpf: export BPF_PSEUDO_MAP_FD to uapi
From: Alexei Starovoitov @ 2015-02-11 1:39 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Jiří Pírko, Network Development
In-Reply-To: <2b58ccd3ab577142297a7995fda7a065e51abef8.1423610452.git.daniel@iogearbox.net>
On Tue, Feb 10, 2015 at 4:15 PM, Daniel Borkmann <daniel@iogearbox.net> wrote:
> We need to export BPF_PSEUDO_MAP_FD to user space, as it's used in
> the ELF BPF loader where instructions are being loaded that need
> map fixups (relocations). An initial stage loads all maps into the
> kernel, and later on replaces related instructions in the eBPF blob
> with BPF_PSEUDO_MAP_FD as source register and the actual fd as
> immediate value. The kernel verifier recognizes this keyword and
> replaces the map fd with a real pointer internally.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
oops. thanks. didn't realize that I forgot to add it to uapi.
Acked-by: Alexei Starovoitov <ast@plumgrid.com>
^ permalink raw reply
* Re: [GIT] Networking
From: Al Viro @ 2015-02-11 1:45 UTC (permalink / raw)
To: Linus Torvalds
Cc: David Miller, Andrew Morton, Network Development,
Linux Kernel Mailing List
In-Reply-To: <CA+55aFyqFrf9cRQ7wDSDLNraR+meZ-=dwWdDThtpPtrFFqrcGw@mail.gmail.com>
On Tue, Feb 10, 2015 at 01:50:16PM -0800, Linus Torvalds wrote:
> On Tue, Feb 10, 2015 at 1:26 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > Just to confirm that yes, it's that particular commit 1d10eb2f156f.
> >
> > I reverted it and things work again. So it's not the miscalculation of
> > "used" , but it's certainly *something* in that commit.
>
> How about this?
>
> - npages = (off + len + PAGE_SIZE - 1) >> PAGE_SHIFT;
> + npages = PAGE_ALIGN(off + n);
>
> The two are not even *remotely* the same thing.
The latter isn't a replacement for the former, but yes, it is bogus - bytes
instead of pages.
Could you check if
diff --git a/crypto/af_alg.c b/crypto/af_alg.c
index eb78fe8..5b11d64 100644
--- a/crypto/af_alg.c
+++ b/crypto/af_alg.c
@@ -348,7 +348,7 @@ int af_alg_make_sg(struct af_alg_sgl *sgl, struct iov_iter *iter, int len)
if (n < 0)
return n;
- npages = PAGE_ALIGN(off + n);
+ npages = DIV_ROUND_UP(off + n, PAGE_SIZE);
if (WARN_ON(npages == 0))
return -EINVAL;
on top of what went into Dave's tree is enough to fix what you are seeing?
^ 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