* [PATCH v11 net-next 01/12] bpf: introduce BPF syscall and maps
From: Alexei Starovoitov @ 2014-09-10 5:09 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin,
Andrew Morton, Kees Cook, linux-api, netdev, linux-kernel
In-Reply-To: <1410325808-3657-1-git-send-email-ast@plumgrid.com>
BPF syscall is a multiplexor for a range of different operations on eBPF.
This patch introduces syscall with single command to create a map.
Next patch adds commands to access maps.
'maps' is a generic storage of different types for sharing data between kernel
and userspace.
Userspace example:
/* this syscall wrapper creates a map with given type and attributes
* and returns map_fd on success.
* use close(map_fd) to delete the map
*/
int bpf_create_map(enum bpf_map_type map_type, int key_size,
int value_size, int max_entries)
{
union bpf_attr attr = {
.map_type = map_type,
.key_size = key_size,
.value_size = value_size,
.max_entries = max_entries
};
return bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
}
syscall is using 'union bpf_attr' to be backwards compatible with future
extensions. Different syscall commands will use different attributes.
More details in Documentation/networking/filter.txt and in manpage
Signed-off-by: Alexei Starovoitov <ast@plumgrid.com>
---
Documentation/networking/filter.txt | 39 +++++++++
include/linux/bpf.h | 41 ++++++++++
include/uapi/linux/bpf.h | 24 ++++++
kernel/bpf/Makefile | 2 +-
kernel/bpf/syscall.c | 149 +++++++++++++++++++++++++++++++++++
5 files changed, 254 insertions(+), 1 deletion(-)
create mode 100644 include/linux/bpf.h
create mode 100644 kernel/bpf/syscall.c
diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index 81916ab5d96f..1900d29518f1 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -1001,6 +1001,45 @@ instruction that loads 64-bit immediate value into a dst_reg.
Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM which loads
32-bit immediate value into a register.
+eBPF maps
+---------
+'maps' is a generic storage of different types for sharing data between kernel
+and userspace.
+
+The maps are accessed from user space via BPF syscall, which has commands:
+- create a map with given type and attributes
+ map_fd = bpf(BPF_MAP_CREATE, union bpf_attr *attr, u32 size)
+ using attr->map_type, attr->key_size, attr->value_size, attr->max_entries
+ returns process-local file descriptor or negative error
+
+- lookup key in a given map
+ err = bpf(BPF_MAP_LOOKUP_ELEM, union bpf_attr *attr, u32 size)
+ using attr->map_fd, attr->key, attr->value
+ returns zero and stores found elem into value or negative error
+
+- create or update key/value pair in a given map
+ err = bpf(BPF_MAP_UPDATE_ELEM, union bpf_attr *attr, u32 size)
+ using attr->map_fd, attr->key, attr->value
+ returns zero or negative error
+
+- find and delete element by key in a given map
+ err = bpf(BPF_MAP_DELETE_ELEM, union bpf_attr *attr, u32 size)
+ using attr->map_fd, attr->key
+
+- to delete map: close(fd)
+ Exiting process will delete maps automatically
+
+userspace programs use this syscall to create/access maps that eBPF programs
+are concurrently updating.
+
+maps can have different types: hash, array, bloom filter, radix-tree, etc.
+
+The map is defined by:
+ . type
+ . max number of elements
+ . key size in bytes
+ . value size in bytes
+
Testing
-------
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
new file mode 100644
index 000000000000..48014a71f0fe
--- /dev/null
+++ b/include/linux/bpf.h
@@ -0,0 +1,41 @@
+/* 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.
+ */
+#ifndef _LINUX_BPF_H
+#define _LINUX_BPF_H 1
+
+#include <uapi/linux/bpf.h>
+#include <linux/workqueue.h>
+
+struct bpf_map;
+
+/* map is generic key/value storage optionally accesible by eBPF programs */
+struct bpf_map_ops {
+ /* funcs callable from userspace (via syscall) */
+ struct bpf_map *(*map_alloc)(union bpf_attr *attr);
+ void (*map_free)(struct bpf_map *);
+};
+
+struct bpf_map {
+ atomic_t refcnt;
+ enum bpf_map_type map_type;
+ u32 key_size;
+ u32 value_size;
+ u32 max_entries;
+ struct bpf_map_ops *ops;
+ struct work_struct work;
+};
+
+struct bpf_map_type_list {
+ struct list_head list_node;
+ struct bpf_map_ops *ops;
+ enum bpf_map_type type;
+};
+
+void bpf_register_map_type(struct bpf_map_type_list *tl);
+void bpf_map_put(struct bpf_map *map);
+
+#endif /* _LINUX_BPF_H */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 479ed0b6be16..7d83ef63849d 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -62,4 +62,28 @@ struct bpf_insn {
__s32 imm; /* signed immediate constant */
};
+/* BPF syscall commands */
+enum bpf_cmd {
+ /* create a map with given type and attributes
+ * fd = bpf(BPF_MAP_CREATE, union bpf_attr *, u32 size)
+ * returns fd or negative error
+ * map is deleted when fd is closed
+ */
+ BPF_MAP_CREATE,
+};
+
+enum bpf_map_type {
+ BPF_MAP_TYPE_UNSPEC,
+};
+
+union bpf_attr {
+ struct { /* anonymous struct used by BPF_MAP_CREATE command */
+ enum bpf_map_type map_type;
+ __u32 key_size; /* size of key in bytes */
+ __u32 value_size; /* size of value in bytes */
+ __u32 max_entries; /* max number of entries in a map */
+#define BPF_MAP_CREATE_LAST_FIELD max_entries
+ };
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
index 6a71145e2769..e9f7334ed07a 100644
--- a/kernel/bpf/Makefile
+++ b/kernel/bpf/Makefile
@@ -1 +1 @@
-obj-y := core.o
+obj-y := core.o syscall.o
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
new file mode 100644
index 000000000000..e353eaf3ac59
--- /dev/null
+++ b/kernel/bpf/syscall.c
@@ -0,0 +1,149 @@
+/* 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.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * General Public License for more details.
+ */
+#include <linux/bpf.h>
+#include <linux/syscalls.h>
+#include <linux/slab.h>
+#include <linux/anon_inodes.h>
+
+static LIST_HEAD(bpf_map_types);
+
+static struct bpf_map *find_and_alloc_map(union bpf_attr *attr)
+{
+ struct bpf_map_type_list *tl;
+ struct bpf_map *map;
+
+ list_for_each_entry(tl, &bpf_map_types, list_node) {
+ if (tl->type == attr->map_type) {
+ map = tl->ops->map_alloc(attr);
+ if (IS_ERR(map))
+ return map;
+ map->ops = tl->ops;
+ map->map_type = attr->map_type;
+ return map;
+ }
+ }
+ return ERR_PTR(-EINVAL);
+}
+
+/* boot time registration of different map implementations */
+void bpf_register_map_type(struct bpf_map_type_list *tl)
+{
+ list_add(&tl->list_node, &bpf_map_types);
+}
+
+/* called from workqueue */
+static void bpf_map_free_deferred(struct work_struct *work)
+{
+ struct bpf_map *map = container_of(work, struct bpf_map, work);
+
+ /* implementation dependent freeing */
+ map->ops->map_free(map);
+}
+
+/* decrement map refcnt and schedule it for freeing via workqueue
+ * (unrelying map implementation ops->map_free() might sleep)
+ */
+void bpf_map_put(struct bpf_map *map)
+{
+ if (atomic_dec_and_test(&map->refcnt)) {
+ INIT_WORK(&map->work, bpf_map_free_deferred);
+ schedule_work(&map->work);
+ }
+}
+
+static int bpf_map_release(struct inode *inode, struct file *filp)
+{
+ struct bpf_map *map = filp->private_data;
+
+ bpf_map_put(map);
+ return 0;
+}
+
+static const struct file_operations bpf_map_fops = {
+ .release = bpf_map_release,
+};
+
+/* helper macro to check that unused fields 'union bpf_attr' are zero */
+#define CHECK_ATTR(CMD) \
+ memchr_inv((void *) &attr->CMD##_LAST_FIELD + \
+ sizeof(attr->CMD##_LAST_FIELD), 0, \
+ sizeof(*attr) - \
+ offsetof(union bpf_attr, CMD##_LAST_FIELD)) != NULL
+
+/* called via syscall */
+static int map_create(union bpf_attr *attr)
+{
+ struct bpf_map *map;
+ int err;
+
+ err = CHECK_ATTR(BPF_MAP_CREATE);
+ if (err)
+ return -EINVAL;
+
+ /* find map type and init map: hashtable vs rbtree vs bloom vs ... */
+ map = find_and_alloc_map(attr);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+
+ 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;
+
+ return err;
+
+free_map:
+ map->ops->map_free(map);
+ return err;
+}
+
+SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, size)
+{
+ union bpf_attr *attr;
+ int err;
+
+ /* the syscall is limited to root temporarily. This restriction will be
+ * lifted when security audit is clean. Note that eBPF+tracing must have
+ * this restriction, since it may pass kernel data to user space
+ */
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ /* newer userspace cannot run with older kernel */
+ if (size > sizeof(*attr))
+ return -EINVAL;
+
+ attr = kzalloc(sizeof(*attr), GFP_USER);
+ if (!attr)
+ return -ENOMEM;
+
+ /* copy attributes from user space, may be less than sizeof(bpf_attr) */
+ err = -EFAULT;
+ if (copy_from_user(attr, uattr, size) != 0)
+ goto free_attr;
+
+ switch (cmd) {
+ case BPF_MAP_CREATE:
+ err = map_create(attr);
+ break;
+ default:
+ err = -EINVAL;
+ break;
+ }
+
+free_attr:
+ kfree(attr);
+ return err;
+}
--
1.7.9.5
^ permalink raw reply related
* [PATCH v11 net-next 00/12] eBPF syscall, verifier, testsuite
From: Alexei Starovoitov @ 2014-09-10 5:09 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, Pablo Neira Ayuso, H. Peter Anvin,
Andrew Morton, Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Hi David,
I've managed to reduce this set to 12:
Patches 1-4 establish BPF syscall shell for maps and programs.
Patches 5-10 add verifier step by step
Patch 11 exposes existing instruction macros to user space
Patch 12 adds test stubs and verifier testsuite from user space
I don't know how to reduce it further. Drop verifier and
have programs loaded without verification? Sounds wrong.
If anyone has other ideas, I'll gladly reduce it further.
Note that patches 1,3,4,7 add commands and attributes to the syscall
while being backwards compatible from each other, which should demonstrate
how other commands can be added in the future.
Daniel,
bpf_common.h patch (that we discussed earlier) I didn't include here
to reduce the number of patches. It can come next.
For those who have looked at the last set of 28 patches, the difference is:
- moved attaching to tracing and sockets to future patches
- moved hash table map type implementation to future
- split verifier further and moved LD_ABS checks and state prunning to future
- instead of running verifier testsuite on real tracing programs added
test_stub.c with fake maps, context and helper functions to test verifier only
- rebased
Note, after this set the programs can be loaded for testing only. They cannot
be attached to any events. This will come in the next set.
As requested by Andy and others, here is the man page:
BPF(2) Linux Programmer's Manual BPF(2)
NAME
bpf - perform a command on eBPF map or program
SYNOPSIS
#include <linux/bpf.h>
int bpf(int cmd, union bpf_attr *attr, unsigned int size);
DESCRIPTION
bpf() syscall is a multiplexor for a range of different operations on
eBPF which can be characterized as "universal in-kernel virtual
machine". eBPF is similar to original Berkeley Packet Filter (or "clas-
sic BPF") used to filter network packets. Both statically analyze the
programs before loading them into the kernel to ensure that programs
cannot harm the running system.
eBPF extends classic BPF in multiple ways including ability to call in-
kernel helper functions and access shared data structures like eBPF
maps. The programs can be written in a restricted C that is compiled
into eBPF bytecode and executed on the eBPF virtual machine or JITed
into native instruction set.
eBPF Design/Architecture
eBPF maps is a generic storage of different types. User process can
create multiple maps (with key/value being opaque bytes of data) and
access them via file descriptor. In parallel eBPF programs can access
maps from inside the kernel. It's up to user process and eBPF program
to decide what they store inside maps.
eBPF programs are similar to kernel modules. They are loaded by the
user process and automatically unloaded when process exits. Each eBPF
program is a safe run-to-completion set of instructions. eBPF verifier
statically determines that the program terminates and is safe to exe-
cute. During verification the program takes a hold of maps that it
intends to use, so selected maps cannot be removed until the program is
unloaded. The program can be attached to different events. These events
can be packets, tracepoint events and other types in the future. A new
event triggers execution of the program which may store information
about the event in the maps. Beyond storing data the programs may call
into in-kernel helper functions which may, for example, dump stack, do
trace_printk or other forms of live kernel debugging. The same program
can be attached to multiple events. Different programs can access the
same map:
tracepoint tracepoint tracepoint sk_buff sk_buff
event A event B event C on eth0 on eth1
| | | | |
| | | | |
--> tracing <-- tracing socket socket
prog_1 prog_2 prog_3 prog_4
| | | |
|--- -----| |-------| map_3
map_1 map_2
Syscall Arguments
bpf() syscall operation is determined by cmd which can be one of the
following:
BPF_MAP_CREATE
Create a map with given type and attributes and return map FD
BPF_MAP_LOOKUP_ELEM
Lookup element by key in a given map and return its value
BPF_MAP_UPDATE_ELEM
Create or update element (key/value pair) in a given map
BPF_MAP_DELETE_ELEM
Lookup and delete element by key in a given map
BPF_MAP_GET_NEXT_KEY
Lookup element by key in a given map and return key of next ele-
ment
BPF_PROG_LOAD
Verify and load eBPF program
attr is a pointer to a union of type bpf_attr as defined below.
size is the size of the union.
union bpf_attr {
struct { /* anonymous struct used by BPF_MAP_CREATE command */
enum bpf_map_type map_type;
__u32 key_size; /* size of key in bytes */
__u32 value_size; /* size of value in bytes */
__u32 max_entries; /* max number of entries in a map */
};
struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */
int map_fd;
void *key;
union {
void *value;
void *next_key;
};
};
struct { /* anonymous struct used by BPF_PROG_LOAD command */
enum bpf_prog_type prog_type;
__u32 insn_cnt;
const struct bpf_insn *insns;
const char *license;
__u32 log_level; /* verbosity level of eBPF verifier */
__u32 log_size; /* size of user buffer */
void *log_buf; /* user supplied buffer */
};
};
eBPF maps
maps is a generic storage of different types for sharing data between
kernel and userspace.
Any map type has the following attributes:
. type
. max number of elements
. key size in bytes
. value size in bytes
The following wrapper functions demonstrate how this syscall can be
used to access the maps. The functions use the cmd argument to invoke
different operations.
BPF_MAP_CREATE
int bpf_create_map(enum bpf_map_type map_type, int key_size,
int value_size, int max_entries)
{
union bpf_attr attr = {
.map_type = map_type,
.key_size = key_size,
.value_size = value_size,
.max_entries = max_entries
};
return bpf(BPF_MAP_CREATE, &attr, sizeof(attr));
}
bpf() syscall creates a map of map_type type and given
attributes key_size, value_size, max_entries. On success it
returns process-local file descriptor or negative error other-
wise.
BPF_MAP_LOOKUP_ELEM
int bpf_lookup_elem(int fd, void *key, void *value)
{
union bpf_attr attr = {
.map_fd = fd,
.key = key,
.value = value,
};
return bpf(BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr));
}
bpf() syscall looks up an element with given key in a map fd.
If element is found it returns zero and stores element's value
into value. Otherwise negative error is returned.
BPF_MAP_UPDATE_ELEM
int bpf_update_elem(int fd, void *key, void *value)
{
union bpf_attr attr = {
.map_fd = fd,
.key = key,
.value = value,
};
return bpf(BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
}
The call creates or updates element with given key/value in a
map fd. On success it returns zero or negative error otherwise.
BPF_MAP_DELETE_ELEM
int bpf_delete_elem(int fd, void *key)
{
union bpf_attr attr = {
.map_fd = fd,
.key = key,
};
return bpf(BPF_MAP_DELETE_ELEM, &attr, sizeof(attr));
}
The call deletes an element in a map fd with given key.
BPF_MAP_GET_NEXT_KEY
int bpf_get_next_key(int fd, void *key, void *next_key)
{
union bpf_attr attr = {
.map_fd = fd,
.key = key,
.next_key = next_key,
};
return bpf(BPF_MAP_GET_NEXT_KEY, &attr, sizeof(attr));
}
The call looks up an element by key in a given map fd and
returns key of next element into next_key pointer. On success it
returns zero or negative error otherwise. This method can be
used to iterate over all elements of the map.
close(map_fd)
will delete the map map_fd. Exiting process will delete all
maps automatically.
In the future maps can have different types: hash, array, bloom filter,
radix-tree, but currently only hash type is supported:
enum bpf_map_type {
BPF_MAP_TYPE_UNSPEC,
BPF_MAP_TYPE_HASH,
};
eBPF programs
BPF_PROG_LOAD
This cmd is used to load eBPF program into the kernel.
char bpf_log_buf[LOG_BUF_SIZE];
int bpf_prog_load(enum bpf_prog_type prog_type,
const struct bpf_insn *insns, int insn_cnt,
const char *license)
{
union bpf_attr attr = {
.prog_type = prog_type,
.insns = insns,
.insn_cnt = insn_cnt,
.license = license,
.log_buf = bpf_log_buf,
.log_size = LOG_BUF_SIZE,
.log_level = 1,
};
return bpf(BPF_PROG_LOAD, &attr, sizeof(attr));
}
prog_type one of the available program types:
enum bpf_prog_type {
BPF_PROG_TYPE_UNSPEC,
BPF_PROG_TYPE_SOCKET_FILTER,
BPF_PROG_TYPE_TRACING_FILTER,
};
insns array of "struct bpf_insn" instructions
insn_cnt number of instructions in the program
license license string, which must be GPL compatible to call
helper functions marked gpl_only
log_buf user supplied buffer that in-kernel verifier is using to
store verification log
log_size size of user buffer
log_level verbosity level of eBPF verifier, where zero means no
logs provided
close(prog_fd)
will unload eBPF program
The maps are accesible from programs and generally tie the two
together. Programs process various events (like tracepoint, kprobe,
packets) and store the data into maps. User space fetches data from
maps. Either the same or a different map may be used by user space as
configuration space to alter program behavior on the fly.
Events
Once an eBPF program is loaded, it can be attached to an event. Various
kernel subsystems have different ways to do so. For example:
setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd, sizeof(prog_fd));
will attach the program prog_fd to socket sock which was received by
prior call to socket().
ioctl(event_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
will attach the program prog_fd to perf event event_fd which was
received by prior call to perf_event_open().
Another way to attach the program to a tracing event is:
event_fd = open("/sys/kernel/debug/tracing/events/skb/kfree_skb/filter");
write(event_fd, "bpf-123"); /* where 123 is eBPF program FD */
/* here program is attached and will be triggered by events */
close(event_fd); /* to detach from event */
EXAMPLES
/* eBPF+sockets example:
* 1. create map with maximum of 2 elements
* 2. set map[6] = 0 and map[17] = 0
* 3. load eBPF program that counts number of TCP and UDP packets received
* via map[skb->ip->proto]++
* 4. attach prog_fd to raw socket via setsockopt()
* 5. print number of received TCP/UDP packets every second
*/
int main(int ac, char **av)
{
static struct bpf_insn prog[] = {
BPF_MOV64_REG(BPF_REG_6, BPF_REG_1),
BPF_LD_ABS(BPF_B, 14 + 9 /* R0 = ip->proto */),
BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_0, -4), /* *(u32 *)(fp - 4) = r0 */
BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4), /* r2 = fp - 4 */
BPF_LD_MAP_FD(BPF_REG_1, 0),
BPF_CALL_FUNC(BPF_FUNC_map_lookup_elem),
BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 0, 2),
BPF_MOV64_IMM(BPF_REG_1, 1), /* r1 = 1 */
BPF_XADD(BPF_DW, BPF_REG_0, BPF_REG_1, 0, 0), /* xadd r0 += r1 */
BPF_MOV64_IMM(BPF_REG_0, 0), /* r0 = 0 */
BPF_EXIT_INSN(),
};
int sock, map_fd, prog_fd, key;
long long value = 0, tcp_cnt, udp_cnt;
map_fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(key), sizeof(value), 2);
if (map_fd < 0) {
printf("failed to create map '%s'\n", strerror(errno));
/* likely not run as root */
return 1;
}
key = 6; /* tcp */
assert(bpf_update_elem(map_fd, &key, &value) == 0);
key = 17; /* udp */
assert(bpf_update_elem(map_fd, &key, &value) == 0);
prog[5].imm = map_fd;
prog_fd = bpf_prog_load(BPF_PROG_TYPE_SOCKET_FILTER, prog, sizeof(prog),
"GPL");
assert(prog_fd >= 0);
sock = open_raw_sock("lo");
assert(setsockopt(sock, SOL_SOCKET, SO_ATTACH_BPF, &prog_fd,
sizeof(prog_fd)) == 0);
for (;;) {
key = 6;
assert(bpf_lookup_elem(map_fd, &key, &tcp_cnt) == 0);
key = 17;
assert(bpf_lookup_elem(map_fd, &key, &udp_cnt) == 0);
printf("TCP %lld UDP %lld packets0, tcp_cnt, udp_cnt);
sleep(1);
}
return 0;
}
RETURN VALUE
For a successful call, the return value depends on the operation:
BPF_MAP_CREATE
The new file descriptor associated with eBPF map.
BPF_PROG_LOAD
The new file descriptor associated with eBPF program.
All other commands
Zero.
On error, -1 is returned, and errno is set appropriately.
ERRORS
EPERM bpf() syscall was made without sufficient privilege (without the
CAP_SYS_ADMIN capability).
ENOMEM Cannot allocate sufficient memory.
EBADF fd is not an open file descriptor
EFAULT One of the pointers ( key or value or log_buf or insns ) is out-
side accessible address space.
EINVAL The value specified in cmd is not recognized by this kernel.
EINVAL For BPF_MAP_CREATE, either map_type or attributes are invalid.
EINVAL For BPF_MAP_*_ELEM commands, some of the fields of "union
bpf_attr" unused by this command are not set to zero.
EINVAL For BPF_PROG_LOAD, attempt to load invalid program (unrecognized
instruction or uses reserved fields or jumps out of range or
loop detected or calls unknown function).
EACCES For BPF_PROG_LOAD, though program has valid instructions, it was
rejected, since it was deemed unsafe (may access disallowed mem-
ory region or uninitialized stack/register or function con-
straints don't match actual types or misaligned access). In such
case it is recommended to call bpf() again with log_level = 1
and examine log_buf for specific reason provided by verifier.
ENOENT For BPF_MAP_LOOKUP_ELEM or BPF_MAP_DELETE_ELEM, indicates that
element with given key was not found.
E2BIG program is too large.
NOTES
These commands may be used only by a privileged process (one having the
CAP_SYS_ADMIN capability).
SEE ALSO
eBPF architecture and instruction set is explained in Documenta-
tion/networking/filter.txt
Linux 2014-09-01 BPF(2)
^ permalink raw reply
* Re: [PATCH v10 net-next 0/2] load imm64 insn and uapi/linux/bpf.h
From: Alexei Starovoitov @ 2014-09-10 1:42 UTC (permalink / raw)
To: David Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <20140909.103059.1330265085939107782.davem@davemloft.net>
On Tue, Sep 9, 2014 at 10:30 AM, David Miller <davem@davemloft.net> wrote:
>
> You kept saying "LLVM is the user" but that's a bullshit argument
> because you aren't including the patches necessary to actually
> propagate native eBPF programs into the kernel.
>
> That's what, 1 or 2 patches, right? Which is not an unreasonable
> request.
I just don't see how I could do that in 2 patches.
In V8 series the first user was appearing in patch 22 out of 28.
Last two days I spent solely hacking the series to move it sooner.
Today the first user is still in patch 15.
Will try to shorten them even more.
So blame it on my skills and not my attitude.
> Anyways, I'm just extremely frustrated with how you operate and work,
> you push things way too hard. I hate to say this, but you are the
> kind of submitter who gets his way by being persistent rather than
> making well formed pleasant submissions that are easy to integrate.
Understood. Will try to make patches shorter and hopefully
more pleasant. I think 'being persistent' is mandatory here :)
^ permalink raw reply
* Re: [PATCH v10 net-next 0/2] load imm64 insn and uapi/linux/bpf.h
From: David Miller @ 2014-09-09 17:30 UTC (permalink / raw)
To: ast-uqk4Ao+rVK5Wk0Htik3J/w
Cc: mingo-DgEjT+Ai2ygdnm+yROfE0A,
torvalds-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
luto-kltTT9wpgjJwATOyAt5JVQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
dborkman-H+wXaHxf7aLQT0dZR+AlfA,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, hpa-YMNOUZJC4hwAvxtiuMwx3w,
akpm-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409894238-9055-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Date: Thu, 4 Sep 2014 22:17:16 -0700
> V9->V10
> - no changes, added Daniel's ack
>
> Note they're on top of Hannes's patch in the same area [1]
>
> V8 thread with 'why' reasoning and end goal [2]
>
> Original set [3] of ~28 patches I'm planning to present in 4 stages:
>
> I. this 2 patches to fork off llvm upstreaming
> II. bpf syscall with manpage and map implementation
> III. bpf program load/unload with verifier testsuite (1st user of
> instruction macros from bpf.h and 1st user of load imm64 insn)
> IV. tracing, etc
>
> [1] http://patchwork.ozlabs.org/patch/385266/
> [2] https://lkml.org/lkml/2014/8/27/628
> [3] https://lkml.org/lkml/2014/8/26/859
Begrudgingly, I've applied this series.
Although I really wish you had included the mechanism for userland to
use the eBPF instructions alongside exporting them to userspace.
You kept saying "LLVM is the user" but that's a bullshit argument
because you aren't including the patches necessary to actually
propagate native eBPF programs into the kernel.
That's what, 1 or 2 patches, right? Which is not an unreasonable
request.
Anyways, I'm just extremely frustrated with how you operate and work,
you push things way too hard. I hate to say this, but you are the
kind of submitter who gets his way by being persistent rather than
making well formed pleasant submissions that are easy to integrate.
^ permalink raw reply
* Re: [PATCH] locks: Ability to test for flock presence on fd
From: J. Bruce Fields @ 2014-09-09 16:18 UTC (permalink / raw)
To: Pavel Emelyanov
Cc: Jeff Layton, Alexander Viro, linux-fsdevel,
Linux Kernel Mailing List, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <54061562.4080306-bzQdu9zFT3WakBO8gow8eQ@public.gmane.org>
On Tue, Sep 02, 2014 at 11:07:14PM +0400, Pavel Emelyanov wrote:
> > Would it make sense to return the lock type held instead, so you could
> > do one flock(fd, LOCK_TEST) instead of flock(fd, LOCK_TEST|LOCK_SH) and
> > flock(fd, LOCK_TEST|LOCK_EX) ?
>
> Well, in our case we parse /proc/locks anyway to see what
> files at least to test for being locked. But what you propose
> looks even better. I'll look what can be done here.
Actually I think I prefer your version. It seems cleaner to define
LOCK_TEST as returning the same result as you'd get if you actually
tried the lock, just without applying the lock. It avoids having a
different return-value convention for this one command. It might avoid
some ambiguity in cases where the flock might be denied for reasons
other than a conflicting flock (e.g. on NFS where flocks and fcntl locks
conflict). It's closer to what GETLK does in the fcntl case.
--b.
^ permalink raw reply
* Re: Request to add linux-kselftest to linux-next
From: Stephen Rothwell @ 2014-09-08 23:14 UTC (permalink / raw)
To: Shuah Khan
Cc: Andrew Morton, linux-next-u79uwXL29TY76Z2rM5mHXA,
greg Kroah-Hartman, fengguang.wu-ral2JQCrhuEAvxtiuMwx3w,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <540E36BD.4060607-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]
Hi Shuah,
On Mon, 08 Sep 2014 17:07:41 -0600 Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
>
> On 09/08/2014 04:58 PM, Stephen Rothwell wrote:
> > Hi Shuah,
> >
> > On Mon, 08 Sep 2014 08:52:45 -0600 Shuah Khan
> > <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
> >>
> >> Could you please add linux-kselftest tree to linux-next. I plan
> >> to maintain two branches:
> >>
> >> linux-kselftest for Kselftest Framework
> >>
> >> fixes - Contains changes intended for merging before the next
> >> major release, including fixes for regressions introduced during
> >> merge window. next - Contains changes intended for merging
> >> during the next merge window between a major release and -rc1.
> >
> > Umm, a git URL would be helpful :-)
> >
>
> Sorry: here it is:
>
> https://git.kernel.org/cgit/linux/kernel/git/shuah/linux-kselftest.git/
Not a git URL :-(
I assume hat you mean is
git://git.kernel.org/pub/scm/linux/kernel/git/shuah/linux-kselftest.git .
I will use that.
--
Cheers,
Stephen Rothwell sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: Request to add linux-kselftest to linux-next
From: Shuah Khan @ 2014-09-08 23:07 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Andrew Morton, linux-next, greg Kroah-Hartman, fengguang.wu,
linux-api, Shuah Khan
In-Reply-To: <20140909085841.2ec2871e@canb.auug.org.au>
On 09/08/2014 04:58 PM, Stephen Rothwell wrote:
> Hi Shuah,
>
> On Mon, 08 Sep 2014 08:52:45 -0600 Shuah Khan
> <shuahkh@osg.samsung.com> wrote:
>>
>> Could you please add linux-kselftest tree to linux-next. I plan
>> to maintain two branches:
>>
>> linux-kselftest for Kselftest Framework
>>
>> fixes - Contains changes intended for merging before the next
>> major release, including fixes for regressions introduced during
>> merge window. next - Contains changes intended for merging
>> during the next merge window between a major release and -rc1.
>
> Umm, a git URL would be helpful :-)
>
Sorry: here it is:
https://git.kernel.org/cgit/linux/kernel/git/shuah/linux-kselftest.git/
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Ingo Molnar @ 2014-09-08 6:37 UTC (permalink / raw)
To: David Miller
Cc: ast-uqk4Ao+rVK5Wk0Htik3J/w, pablo-Cap9r6Oaw4JrovVCs/uTlw,
torvalds-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
luto-kltTT9wpgjJwATOyAt5JVQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
dborkman-H+wXaHxf7aLQT0dZR+AlfA,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, hpa-YMNOUZJC4hwAvxtiuMwx3w,
akpm-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140907.222841.1933004265241590374.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
* David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Date: Mon, 8 Sep 2014 07:23:29 +0200
>
> >
> > * Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
> >
> >> > I don't think the speed up of the llvm submission is a
> >> > good argument, this sounds to me similar to the "please
> >> > apply this patch that reserves this new netlink family in
> >> > include/linux/netlink.h, I promise this new subsystem will
> >> > be submitted soon though. Meanwhile this will speed up
> >> > submission of my userspace software to distributions for
> >> > packaging" argument.
> >>
> >> You're not correct here. I'm not saying 'I promise it will
> >> be submitted'. There _were_ already submitted. [...]
> >
> > And this split-up smaller submissions was requested by David
> > Miller, the networking maintainer, so if Pablo wants another
> > submission format, he needs to take it up with David - we
> > can't do both at once obviously.
>
> I think that just because I asked the submission size to be
> smaller, it does not mean that you can submit things before you
> provide the initial user as well.
>
> And how to work that out and keep the submission size
> reasonable is the submitter's problem, not mine.
That's a pretty harsh requirement but might be doable
technically: Alexei, please submit a series that is large enough
to provide self-sufficient functionality as per Pablo's request,
but is also small and minimal, as per David's request.
If that fails then another route would be to decouple from
networking initially and create something new and stand-alone in
kernel/ebpf/ (or any other name really), with tracing and perf
usecases, with networking integration and code deduplication to
be done at the end, when there can be no legitimate argument
about its utility.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-09-08 6:34 UTC (permalink / raw)
To: David Miller
Cc: Ingo Molnar, Pablo Neira Ayuso, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Daniel Borkmann, Hannes Frederic Sowa,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra, H. Peter Anvin,
Andrew Morton, Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <20140907.222841.1933004265241590374.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Sun, Sep 7, 2014 at 10:28 PM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Date: Mon, 8 Sep 2014 07:23:29 +0200
>
>>
>> * Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>>
>>> > I don't think the speed up of the llvm submission is a good
>>> > argument, this sounds to me similar to the "please apply this
>>> > patch that reserves this new netlink family in
>>> > include/linux/netlink.h, I promise this new subsystem will be
>>> > submitted soon though. Meanwhile this will speed up
>>> > submission of my userspace software to distributions for
>>> > packaging" argument.
>>>
>>> You're not correct here. I'm not saying 'I promise it will be
>>> submitted'. There _were_ already submitted. [...]
>>
>> And this split-up smaller submissions was requested by David
>> Miller, the networking maintainer, so if Pablo wants another
>> submission format, he needs to take it up with David - we can't
>> do both at once obviously.
>
> I think that just because I asked the submission size to be smaller,
> it does not mean that you can submit things before you provide the
> initial user as well.
>
> And how to work that out and keep the submission size reasonable is
> the submitter's problem, not mine.
imo llvm is more than enough for the first user, no?
I think I've explained that it's practically impossible to have
first in-tree user in the first patch. What do you suggest?
I'm listening, but currently I see no way out.
The patch was large. I broke it down. The first patch is as tiny
as it can get, but Pablo is stalling it, like he did for the last year.
Honestly it doesn't feel fair.
When there were technical arguments (like global vs fd) or
(union attr vs long for syscall) I've listened and rewrote things.
Now it doesn't sound technical.
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: David Miller @ 2014-09-08 5:28 UTC (permalink / raw)
To: mingo-DgEjT+Ai2ygdnm+yROfE0A
Cc: ast-uqk4Ao+rVK5Wk0Htik3J/w, pablo-Cap9r6Oaw4JrovVCs/uTlw,
torvalds-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
luto-kltTT9wpgjJwATOyAt5JVQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
dborkman-H+wXaHxf7aLQT0dZR+AlfA,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, hpa-YMNOUZJC4hwAvxtiuMwx3w,
akpm-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20140908052329.GA30461-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
From: Ingo Molnar <mingo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Date: Mon, 8 Sep 2014 07:23:29 +0200
>
> * Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org> wrote:
>
>> > I don't think the speed up of the llvm submission is a good
>> > argument, this sounds to me similar to the "please apply this
>> > patch that reserves this new netlink family in
>> > include/linux/netlink.h, I promise this new subsystem will be
>> > submitted soon though. Meanwhile this will speed up
>> > submission of my userspace software to distributions for
>> > packaging" argument.
>>
>> You're not correct here. I'm not saying 'I promise it will be
>> submitted'. There _were_ already submitted. [...]
>
> And this split-up smaller submissions was requested by David
> Miller, the networking maintainer, so if Pablo wants another
> submission format, he needs to take it up with David - we can't
> do both at once obviously.
I think that just because I asked the submission size to be smaller,
it does not mean that you can submit things before you provide the
initial user as well.
And how to work that out and keep the submission size reasonable is
the submitter's problem, not mine.
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Ingo Molnar @ 2014-09-08 5:23 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Pablo Neira Ayuso, David S. Miller, Linus Torvalds,
Andy Lutomirski, Steven Rostedt, Daniel Borkmann,
Hannes Frederic Sowa, Chema Gonzalez, Eric Dumazet,
Peter Zijlstra, H. Peter Anvin, Andrew Morton, Kees Cook,
Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuxJLDu8AAsZQdtG781vwC8y60Ygdt4ji5tbeyTE8YbWbQ@mail.gmail.com>
* Alexei Starovoitov <ast@plumgrid.com> wrote:
> > I don't think the speed up of the llvm submission is a good
> > argument, this sounds to me similar to the "please apply this
> > patch that reserves this new netlink family in
> > include/linux/netlink.h, I promise this new subsystem will be
> > submitted soon though. Meanwhile this will speed up
> > submission of my userspace software to distributions for
> > packaging" argument.
>
> You're not correct here. I'm not saying 'I promise it will be
> submitted'. There _were_ already submitted. [...]
And this split-up smaller submissions was requested by David
Miller, the networking maintainer, so if Pablo wants another
submission format, he needs to take it up with David - we can't
do both at once obviously.
Thanks,
Ingo
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-09-07 20:13 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Daniel Borkmann, Hannes Frederic Sowa,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra, H. Peter Anvin,
Andrew Morton, Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <20140907180737.GA5057@salvia>
On Sun, Sep 7, 2014 at 11:07 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
>
> If the patches that provide the very first user interface don't get in
> time to this merge window round for whatever reason, we'll have the
> layout of this exposed to userspace in the next kernel version with no
> clients at all, that doesn't make sense to me.
eBPF cannot have the first user without verifier and tracing
fully reviewed, so first user cannot be in the first
patch no matter what.
In particular this patch only exposed eBPF as an _instruction set_
to user space. llvm and gcc are only two users.
llvm patches _were_ submitted to the list.
Compilers are not some fictitious users.
eBPF ISA is solid. Two backends is a proof.
For eBPF to be loaded, verifier, syscall and other pieces need to
come in gradually. So I logically split them in series:
stage I - expose instruction set
stage II - bpf syscall for maps and manpage
stage III - programs, verifier and user space testsuite
stage IV - ebpf+tracing (the first user of ebpf isa and syscall)
stage V - ebpf+sockets
stage VI - ebpf+ovs
all of the patches _were_ submitted in the past.
Split is done to make review and integration easier.
After stage III user space will be able to load eBPF programs,
but they will still be useless, because they cannot be attached
to anything. Realistically only stage IV makes first real use of
them in tracing.
You know this, yet, you're saying the first user must be
in the first series. Really, what this 'feedback' is about?
> I don't think the speed up of the llvm submission is a good argument,
> this sounds to me similar to the "please apply this patch that
> reserves this new netlink family in include/linux/netlink.h, I promise
> this new subsystem will be submitted soon though. Meanwhile this will
> speed up submission of my userspace software to distributions for
> packaging" argument.
You're not correct here. I'm not saying 'I promise it will be submitted'.
There _were_ already submitted. I split them in chunks to make
review easier and to follow standard linux philosophy of making
small decisions that can be reverted.
We still have a month until merge window, so if stages II and III
don't make it in time, Dave can revert these small patches just
as easily. You're advocating first_user_must_be_in_first_patch
approach, which is against the linux methodology.
> I think you have to find the way to send a small batch with the very
> essencial stuff that, if merged mainstream, will provide just one new
> feature while leaving the repository in consistent state. Then, send
> follow up patches that enhance your thing and that add new clients of
> it.
As I said above the _minimum_ useful program needs verifier
which is more than Dave's cutoff of 10 patches. Therefore I
split all very_essential_stuff into these stages.
Note I'm not sending radix-tree type of eBPF maps as part
of these stages, neither I send array type of eBPF maps,
though they're needed for my last stage (ebpf+ovs).
I'm not sending pointer leak detector for verifier either,
it will be needed before syscall can be exposed to unprivileged
users, etc. There is a lot of stuff that I need for ebpf+ovs that
is _not_ part of these stages. What you see is really
the minimum to make ebpf+tracing useful.
btw, all of ebpf+ovs was submitted to the list back in Sep 2013.
So there is no practical way to do first set of < 10 patches
that will be usable from user space on its own.
Even if I remove verifier and maps altogether, bpf programs
need to be attached to something like tracing and
that is again >10 patches.
These stages with small patches is only sensible approach.
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Pablo Neira Ayuso @ 2014-09-07 18:07 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Daniel Borkmann, Hannes Frederic Sowa,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra, H. Peter Anvin,
Andrew Morton, Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <CAMEtUuxPynQ4wdWDgCU+9_3_GRYn8oA4f-emcF+mqiQdLsqEWg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Sat, Sep 06, 2014 at 09:04:23AM -0700, Alexei Starovoitov wrote:
> On Sat, Sep 6, 2014 at 7:10 AM, Pablo Neira Ayuso <pablo-Cap9r6Oaw4JrovVCs/uTlw@public.gmane.org> wrote:
> > On Thu, Sep 04, 2014 at 10:17:18PM -0700, Alexei Starovoitov wrote:
> >> allow user space to generate eBPF programs
> >>
> >> uapi/linux/bpf.h: eBPF instruction set definition
> >>
> >> linux/filter.h: the rest
> >>
> >> This patch only moves macro definitions, but practically it freezes existing
> >> eBPF instruction set, though new instructions can still be added in the future.
> >>
> >> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
> >> may conflict with existing applications.
> >>
> >> Full eBPF ISA description is in Documentation/networking/filter.txt
> >
> > I think you need to have at least one single interface using this
> > before you can expose it to userspace. So this should come in the
> > small batch that introduces the first interface of your ebpf code in
> > userspace. AFAIK, this has been the policy so far.
>
> That's what I've been doing over the last year.
> My first eBPF patch was in Sep of 2013!
> since then I've been only tweaking and massaging it.
> Nothing fundamentally changed.
> Last few month I've been posting these series with not only
> first user, but with multiple. Many examples, test cases and so on.
> The series became big and Dave asked to split them.
> Please see the cover letter. These two patches is stage I.
> More examples and use cases in stage II, stage III, stage IV, etc
If the patches that provide the very first user interface don't get in
time to this merge window round for whatever reason, we'll have the
layout of this exposed to userspace in the next kernel version with no
clients at all, that doesn't make sense to me.
I don't think the speed up of the llvm submission is a good argument,
this sounds to me similar to the "please apply this patch that
reserves this new netlink family in include/linux/netlink.h, I promise
this new subsystem will be submitted soon though. Meanwhile this will
speed up submission of my userspace software to distributions for
packaging" argument.
I think you have to find the way to send a small batch with the very
essencial stuff that, if merged mainstream, will provide just one new
feature while leaving the repository in consistent state. Then, send
follow up patches that enhance your thing and that add new clients of
it.
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-09-06 16:04 UTC (permalink / raw)
To: Pablo Neira Ayuso
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Daniel Borkmann, Hannes Frederic Sowa,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra, H. Peter Anvin,
Andrew Morton, Kees Cook, Linux API, Network Development, LKML
In-Reply-To: <20140906141051.GA4245@salvia>
On Sat, Sep 6, 2014 at 7:10 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Thu, Sep 04, 2014 at 10:17:18PM -0700, Alexei Starovoitov wrote:
>> allow user space to generate eBPF programs
>>
>> uapi/linux/bpf.h: eBPF instruction set definition
>>
>> linux/filter.h: the rest
>>
>> This patch only moves macro definitions, but practically it freezes existing
>> eBPF instruction set, though new instructions can still be added in the future.
>>
>> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
>> may conflict with existing applications.
>>
>> Full eBPF ISA description is in Documentation/networking/filter.txt
>
> I think you need to have at least one single interface using this
> before you can expose it to userspace. So this should come in the
> small batch that introduces the first interface of your ebpf code in
> userspace. AFAIK, this has been the policy so far.
That's what I've been doing over the last year.
My first eBPF patch was in Sep of 2013!
since then I've been only tweaking and massaging it.
Nothing fundamentally changed.
Last few month I've been posting these series with not only
first user, but with multiple. Many examples, test cases and so on.
The series became big and Dave asked to split them.
Please see the cover letter. These two patches is stage I.
More examples and use cases in stage II, stage III, stage IV, etc
All these patches are ready. I'm only submitting them one at
a time to make review easier.
Please see them in my tree if you interested.
Thanks
^ permalink raw reply
* Re: [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Pablo Neira Ayuso @ 2014-09-06 14:10 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Ingo Molnar, Linus Torvalds, Andy Lutomirski,
Steven Rostedt, Daniel Borkmann, Hannes Frederic Sowa,
Chema Gonzalez, Eric Dumazet, Peter Zijlstra, H. Peter Anvin,
Andrew Morton, Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409894238-9055-3-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
On Thu, Sep 04, 2014 at 10:17:18PM -0700, Alexei Starovoitov wrote:
> allow user space to generate eBPF programs
>
> uapi/linux/bpf.h: eBPF instruction set definition
>
> linux/filter.h: the rest
>
> This patch only moves macro definitions, but practically it freezes existing
> eBPF instruction set, though new instructions can still be added in the future.
>
> These eBPF definitions cannot go into uapi/linux/filter.h, since the names
> may conflict with existing applications.
>
> Full eBPF ISA description is in Documentation/networking/filter.txt
I think you need to have at least one single interface using this
before you can expose it to userspace. So this should come in the
small batch that introduces the first interface of your ebpf code in
userspace. AFAIK, this has been the policy so far.
^ permalink raw reply
* [PATCH v10 net-next 2/2] net: filter: split filter.h and expose eBPF to user space
From: Alexei Starovoitov @ 2014-09-05 5:17 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409894238-9055-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
allow user space to generate eBPF programs
uapi/linux/bpf.h: eBPF instruction set definition
linux/filter.h: the rest
This patch only moves macro definitions, but practically it freezes existing
eBPF instruction set, though new instructions can still be added in the future.
These eBPF definitions cannot go into uapi/linux/filter.h, since the names
may conflict with existing applications.
Full eBPF ISA description is in Documentation/networking/filter.txt
Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Acked-by: Daniel Borkmann <dborkman-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
include/linux/filter.h | 56 +-------------------------------------
include/uapi/linux/Kbuild | 1 +
include/uapi/linux/bpf.h | 65 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 67 insertions(+), 55 deletions(-)
create mode 100644 include/uapi/linux/bpf.h
diff --git a/include/linux/filter.h b/include/linux/filter.h
index bf323da77950..8f82ef3f1cdd 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -10,58 +10,12 @@
#include <linux/workqueue.h>
#include <uapi/linux/filter.h>
#include <asm/cacheflush.h>
+#include <uapi/linux/bpf.h>
struct sk_buff;
struct sock;
struct seccomp_data;
-/* Internally used and optimized filter representation with extended
- * instruction set based on top of classic BPF.
- */
-
-/* instruction classes */
-#define BPF_ALU64 0x07 /* alu mode in double word width */
-
-/* ld/ldx fields */
-#define BPF_DW 0x18 /* double word */
-#define BPF_XADD 0xc0 /* exclusive add */
-
-/* alu/jmp fields */
-#define BPF_MOV 0xb0 /* mov reg to reg */
-#define BPF_ARSH 0xc0 /* sign extending arithmetic shift right */
-
-/* change endianness of a register */
-#define BPF_END 0xd0 /* flags for endianness conversion: */
-#define BPF_TO_LE 0x00 /* convert to little-endian */
-#define BPF_TO_BE 0x08 /* convert to big-endian */
-#define BPF_FROM_LE BPF_TO_LE
-#define BPF_FROM_BE BPF_TO_BE
-
-#define BPF_JNE 0x50 /* jump != */
-#define BPF_JSGT 0x60 /* SGT is signed '>', GT in x86 */
-#define BPF_JSGE 0x70 /* SGE is signed '>=', GE in x86 */
-#define BPF_CALL 0x80 /* function call */
-#define BPF_EXIT 0x90 /* function return */
-
-/* Register numbers */
-enum {
- BPF_REG_0 = 0,
- BPF_REG_1,
- BPF_REG_2,
- BPF_REG_3,
- BPF_REG_4,
- BPF_REG_5,
- BPF_REG_6,
- BPF_REG_7,
- BPF_REG_8,
- BPF_REG_9,
- BPF_REG_10,
- __MAX_BPF_REG,
-};
-
-/* BPF has 10 general purpose 64-bit registers and stack frame. */
-#define MAX_BPF_REG __MAX_BPF_REG
-
/* ArgX, context and stack frame pointer register positions. Note,
* Arg1, Arg2, Arg3, etc are used as argument mappings of function
* calls in BPF_CALL instruction.
@@ -322,14 +276,6 @@ enum {
#define SK_RUN_FILTER(filter, ctx) \
(*filter->prog->bpf_func)(ctx, filter->prog->insnsi)
-struct bpf_insn {
- __u8 code; /* opcode */
- __u8 dst_reg:4; /* dest register */
- __u8 src_reg:4; /* source register */
- __s16 off; /* signed offset */
- __s32 imm; /* signed immediate constant */
-};
-
#ifdef CONFIG_COMPAT
/* A struct sock_filter is architecture independent. */
struct compat_sock_fprog {
diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
index 24e9033f8b3f..fb3f7b675229 100644
--- a/include/uapi/linux/Kbuild
+++ b/include/uapi/linux/Kbuild
@@ -67,6 +67,7 @@ header-y += bfs_fs.h
header-y += binfmts.h
header-y += blkpg.h
header-y += blktrace_api.h
+header-y += bpf.h
header-y += bpqether.h
header-y += bsg.h
header-y += btrfs.h
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
new file mode 100644
index 000000000000..479ed0b6be16
--- /dev/null
+++ b/include/uapi/linux/bpf.h
@@ -0,0 +1,65 @@
+/* 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.
+ */
+#ifndef _UAPI__LINUX_BPF_H__
+#define _UAPI__LINUX_BPF_H__
+
+#include <linux/types.h>
+
+/* Extended instruction set based on top of classic BPF */
+
+/* instruction classes */
+#define BPF_ALU64 0x07 /* alu mode in double word width */
+
+/* ld/ldx fields */
+#define BPF_DW 0x18 /* double word */
+#define BPF_XADD 0xc0 /* exclusive add */
+
+/* alu/jmp fields */
+#define BPF_MOV 0xb0 /* mov reg to reg */
+#define BPF_ARSH 0xc0 /* sign extending arithmetic shift right */
+
+/* change endianness of a register */
+#define BPF_END 0xd0 /* flags for endianness conversion: */
+#define BPF_TO_LE 0x00 /* convert to little-endian */
+#define BPF_TO_BE 0x08 /* convert to big-endian */
+#define BPF_FROM_LE BPF_TO_LE
+#define BPF_FROM_BE BPF_TO_BE
+
+#define BPF_JNE 0x50 /* jump != */
+#define BPF_JSGT 0x60 /* SGT is signed '>', GT in x86 */
+#define BPF_JSGE 0x70 /* SGE is signed '>=', GE in x86 */
+#define BPF_CALL 0x80 /* function call */
+#define BPF_EXIT 0x90 /* function return */
+
+/* Register numbers */
+enum {
+ BPF_REG_0 = 0,
+ BPF_REG_1,
+ BPF_REG_2,
+ BPF_REG_3,
+ BPF_REG_4,
+ BPF_REG_5,
+ BPF_REG_6,
+ BPF_REG_7,
+ BPF_REG_8,
+ BPF_REG_9,
+ BPF_REG_10,
+ __MAX_BPF_REG,
+};
+
+/* BPF has 10 general purpose 64-bit registers and stack frame. */
+#define MAX_BPF_REG __MAX_BPF_REG
+
+struct bpf_insn {
+ __u8 code; /* opcode */
+ __u8 dst_reg:4; /* dest register */
+ __u8 src_reg:4; /* source register */
+ __s16 off; /* signed offset */
+ __s32 imm; /* signed immediate constant */
+};
+
+#endif /* _UAPI__LINUX_BPF_H__ */
--
1.7.9.5
^ permalink raw reply related
* [PATCH v10 net-next 1/2] net: filter: add "load 64-bit immediate" eBPF instruction
From: Alexei Starovoitov @ 2014-09-05 5:17 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1409894238-9055-1-git-send-email-ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
add BPF_LD_IMM64 instruction to load 64-bit immediate value into a register.
All previous instructions were 8-byte. This is first 16-byte instruction.
Two consecutive 'struct bpf_insn' blocks are interpreted as single instruction:
insn[0].code = BPF_LD | BPF_DW | BPF_IMM
insn[0].dst_reg = destination register
insn[0].imm = lower 32-bit
insn[1].code = 0
insn[1].imm = upper 32-bit
All unused fields must be zero.
Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM
which loads 32-bit immediate value into a register.
x64 JITs it as single 'movabsq %rax, imm64'
arm64 may JIT as sequence of four 'movk x0, #imm16, lsl #shift' insn
Note that old eBPF programs are binary compatible with new interpreter.
It helps eBPF programs load 64-bit constant into a register with one
instruction instead of using two registers and 4 instructions:
BPF_MOV32_IMM(R1, imm32)
BPF_ALU64_IMM(BPF_LSH, R1, 32)
BPF_MOV32_IMM(R2, imm32)
BPF_ALU64_REG(BPF_OR, R1, R2)
User space generated programs will use this instruction to load constants only.
To tell kernel that user space needs a pointer the _pseudo_ variant of
this instruction may be added later, which will use extra bits of encoding
to indicate what type of pointer user space is asking kernel to provide.
For example 'off' or 'src_reg' fields can be used for such purpose.
src_reg = 1 could mean that user space is asking kernel to validate and
load in-kernel map pointer.
src_reg = 2 could mean that user space needs readonly data section pointer
src_reg = 3 could mean that user space needs a pointer to per-cpu local data
All such future pseudo instructions will not be carrying the actual pointer
as part of the instruction, but rather will be treated as a request to kernel
to provide one. The kernel will verify the request_for_a_pointer, then
will drop _pseudo_ marking and will store actual internal pointer inside
the instruction, so the end result is the interpreter and JITs never
see pseudo BPF_LD_IMM64 insns and only operate on generic BPF_LD_IMM64 that
loads 64-bit immediate into a register. User space never operates on direct
pointers and verifier can easily recognize request_for_pointer vs other
instructions.
Signed-off-by: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
---
Documentation/networking/filter.txt | 8 +++++++-
arch/x86/net/bpf_jit_comp.c | 17 +++++++++++++++++
include/linux/filter.h | 18 ++++++++++++++++++
kernel/bpf/core.c | 5 +++++
lib/test_bpf.c | 21 +++++++++++++++++++++
5 files changed, 68 insertions(+), 1 deletion(-)
diff --git a/Documentation/networking/filter.txt b/Documentation/networking/filter.txt
index c48a9704bda8..81916ab5d96f 100644
--- a/Documentation/networking/filter.txt
+++ b/Documentation/networking/filter.txt
@@ -951,7 +951,7 @@ Size modifier is one of ...
Mode modifier is one of:
- BPF_IMM 0x00 /* classic BPF only, reserved in eBPF */
+ BPF_IMM 0x00 /* used for 32-bit mov in classic BPF and 64-bit in eBPF */
BPF_ABS 0x20
BPF_IND 0x40
BPF_MEM 0x60
@@ -995,6 +995,12 @@ BPF_XADD | BPF_DW | BPF_STX: lock xadd *(u64 *)(dst_reg + off16) += src_reg
Where size is one of: BPF_B or BPF_H or BPF_W or BPF_DW. Note that 1 and
2 byte atomic increments are not supported.
+eBPF has one 16-byte instruction: BPF_LD | BPF_DW | BPF_IMM which consists
+of two consecutive 'struct bpf_insn' 8-byte blocks and interpreted as single
+instruction that loads 64-bit immediate value into a dst_reg.
+Classic BPF has similar instruction: BPF_LD | BPF_W | BPF_IMM which loads
+32-bit immediate value into a register.
+
Testing
-------
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 39ccfbb4a723..06f8c17f5484 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -393,6 +393,23 @@ static int do_jit(struct bpf_prog *bpf_prog, int *addrs, u8 *image,
EMIT1_off32(add_1reg(0xB8, dst_reg), imm32);
break;
+ case BPF_LD | BPF_IMM | BPF_DW:
+ if (insn[1].code != 0 || insn[1].src_reg != 0 ||
+ insn[1].dst_reg != 0 || insn[1].off != 0) {
+ /* verifier must catch invalid insns */
+ pr_err("invalid BPF_LD_IMM64 insn\n");
+ return -EINVAL;
+ }
+
+ /* movabsq %rax, imm64 */
+ EMIT2(add_1mod(0x48, dst_reg), add_1reg(0xB8, dst_reg));
+ EMIT(insn[0].imm, 4);
+ EMIT(insn[1].imm, 4);
+
+ insn++;
+ i++;
+ break;
+
/* dst %= src, dst /= src, dst %= imm32, dst /= imm32 */
case BPF_ALU | BPF_MOD | BPF_X:
case BPF_ALU | BPF_DIV | BPF_X:
diff --git a/include/linux/filter.h b/include/linux/filter.h
index c78994593355..bf323da77950 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -166,6 +166,24 @@ enum {
.off = 0, \
.imm = IMM })
+/* BPF_LD_IMM64 macro encodes single 'load 64-bit immediate' insn */
+#define BPF_LD_IMM64(DST, IMM) \
+ BPF_LD_IMM64_RAW(DST, 0, IMM)
+
+#define BPF_LD_IMM64_RAW(DST, SRC, IMM) \
+ ((struct bpf_insn) { \
+ .code = BPF_LD | BPF_DW | BPF_IMM, \
+ .dst_reg = DST, \
+ .src_reg = SRC, \
+ .off = 0, \
+ .imm = (__u32) (IMM) }), \
+ ((struct bpf_insn) { \
+ .code = 0, /* zero is reserved opcode */ \
+ .dst_reg = 0, \
+ .src_reg = 0, \
+ .off = 0, \
+ .imm = ((__u64) (IMM)) >> 32 })
+
/* Short form of mov based on type, BPF_X: dst_reg = src_reg, BPF_K: dst_reg = imm32 */
#define BPF_MOV64_RAW(TYPE, DST, SRC, IMM) \
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index b54bb2c2e494..2c2bfaacce66 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -242,6 +242,7 @@ static unsigned int __bpf_prog_run(void *ctx, const struct bpf_insn *insn)
[BPF_LD | BPF_IND | BPF_W] = &&LD_IND_W,
[BPF_LD | BPF_IND | BPF_H] = &&LD_IND_H,
[BPF_LD | BPF_IND | BPF_B] = &&LD_IND_B,
+ [BPF_LD | BPF_IMM | BPF_DW] = &&LD_IMM_DW,
};
void *ptr;
int off;
@@ -301,6 +302,10 @@ select_insn:
ALU64_MOV_K:
DST = IMM;
CONT;
+ LD_IMM_DW:
+ DST = (u64) (u32) insn[0].imm | ((u64) (u32) insn[1].imm) << 32;
+ insn++;
+ CONT;
ALU64_ARSH_X:
(*(s64 *) &DST) >>= SRC;
CONT;
diff --git a/lib/test_bpf.c b/lib/test_bpf.c
index 9a67456ba29a..413890815d3e 100644
--- a/lib/test_bpf.c
+++ b/lib/test_bpf.c
@@ -1735,6 +1735,27 @@ static struct bpf_test tests[] = {
{ },
{ { 1, 0 } },
},
+ {
+ "load 64-bit immediate",
+ .u.insns_int = {
+ BPF_LD_IMM64(R1, 0x567800001234L),
+ BPF_MOV64_REG(R2, R1),
+ BPF_MOV64_REG(R3, R2),
+ BPF_ALU64_IMM(BPF_RSH, R2, 32),
+ BPF_ALU64_IMM(BPF_LSH, R3, 32),
+ BPF_ALU64_IMM(BPF_RSH, R3, 32),
+ BPF_ALU64_IMM(BPF_MOV, R0, 0),
+ BPF_JMP_IMM(BPF_JEQ, R2, 0x5678, 1),
+ BPF_EXIT_INSN(),
+ BPF_JMP_IMM(BPF_JEQ, R3, 0x1234, 1),
+ BPF_EXIT_INSN(),
+ BPF_ALU64_IMM(BPF_MOV, R0, 1),
+ BPF_EXIT_INSN(),
+ },
+ INTERNAL,
+ { },
+ { { 0, 1 } }
+ },
};
static struct net_device dev;
--
1.7.9.5
^ permalink raw reply related
* [PATCH v10 net-next 0/2] load imm64 insn and uapi/linux/bpf.h
From: Alexei Starovoitov @ 2014-09-05 5:17 UTC (permalink / raw)
To: David S. Miller
Cc: Ingo Molnar, Linus Torvalds, Andy Lutomirski, Steven Rostedt,
Daniel Borkmann, Hannes Frederic Sowa, Chema Gonzalez,
Eric Dumazet, Peter Zijlstra, H. Peter Anvin, Andrew Morton,
Kees Cook, linux-api, netdev, linux-kernel
Hi,
V9->V10
- no changes, added Daniel's ack
Note they're on top of Hannes's patch in the same area [1]
V8 thread with 'why' reasoning and end goal [2]
Original set [3] of ~28 patches I'm planning to present in 4 stages:
I. this 2 patches to fork off llvm upstreaming
II. bpf syscall with manpage and map implementation
III. bpf program load/unload with verifier testsuite (1st user of
instruction macros from bpf.h and 1st user of load imm64 insn)
IV. tracing, etc
[1] http://patchwork.ozlabs.org/patch/385266/
[2] https://lkml.org/lkml/2014/8/27/628
[3] https://lkml.org/lkml/2014/8/26/859
^ permalink raw reply
* Re: [PATCH 1/1] add selftest for virtio-net v1.0
From: Jason Wang @ 2014-09-05 3:31 UTC (permalink / raw)
To: Hengjinxiao, virtualization, netdev, linux-kernel, linux-api; +Cc: famz, mst
In-Reply-To: <1409881866-14780-1-git-send-email-hjxiaohust@gmail.com>
On 09/05/2014 09:51 AM, Hengjinxiao wrote:
> Selftest is an important part of network driver, this patch adds selftest for
> virtio-net, including loopback test, negotiate test and reset test. Loopback
> test checks whether virtio-net can send and receive packets normally. Negotiate test
> executes feature negotiation between virtio-net driver in Guest OS and virtio-net
> device in Host OS. Reset test resets virtio-net.
> Following last patch, this version has deleted some useless codes and fixed bugs
> as you suggest.
> Any corrections are welcome.
>
> Signed-off-by: Hengjinxiao <hjxiaohust@gmail.com>
Some of the lines were indented correctly, some others exceeded 80
characters per line. Please see Documentation/SubmittingPatches for more
guide lines. More important, some of the comments in V1 were still not
addressed.
I suggest split this patch into series:
- patch that introduces virtio core helpers
- patch that introduces new virtio-net helpers
- patch that implements a skeleton of the selftest
- patch that implements loopback test
- patch that implements feature negotation test
Thanks
>
> ---
> drivers/net/virtio_net.c | 241 ++++++++++++++++++++++++++++++++++++++--
> drivers/virtio/virtio.c | 20 +++-
> include/linux/virtio.h | 2 +
> include/uapi/linux/virtio_net.h | 9 ++
> 4 files changed, 256 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 59caa06..22d8228 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -28,6 +28,7 @@
> #include <linux/cpu.h>
> #include <linux/average.h>
> #include <net/busy_poll.h>
> +#include <linux/pci.h>
Why pci.h is needed?
>
> static int napi_weight = NAPI_POLL_WEIGHT;
> module_param(napi_weight, int, 0444);
> @@ -51,6 +52,17 @@ module_param(gso, bool, 0444);
> #define MERGEABLE_BUFFER_ALIGN max(L1_CACHE_BYTES, 256)
>
> #define VIRTNET_DRIVER_VERSION "1.0.0"
> +#define __VIRTNET_TESTING 0
> +
> +static const struct {
> + const char string[ETH_GSTRING_LEN];
> +} virtnet_gstrings_test[] = {
> + { "loopback test (offline)" },
> + { "negotiate test (offline)" },
> + { "reset test (offline)" },
> +};
> +
> +#define VIRTNET_NUM_TEST ARRAY_SIZE(virtnet_gstrings_test)
>
> struct virtnet_stats {
> struct u64_stats_sync tx_syncp;
> @@ -104,6 +116,8 @@ struct virtnet_info {
> struct send_queue *sq;
> struct receive_queue *rq;
> unsigned int status;
> + unsigned long flags;
> + atomic_t lb_count;
>
> /* Max # of queue pairs supported by the device */
> u16 max_queue_pairs;
> @@ -436,6 +450,19 @@ err_buf:
> return NULL;
> }
>
> +void virtnet_check_lb_frame(struct virtnet_info *vi,
> + struct sk_buff *skb)
> +{
> + unsigned int frame_size = skb->len;
> +
> + if (*(skb->data + 3) == 0xFF) {
> + if ((*(skb->data + frame_size / 2 + 10) == 0xBE) &&
> + (*(skb->data + frame_size / 2 + 12) == 0xAF)) {
> + atomic_dec(&vi->lb_count);
> + }
> + }
> +}
> +
> static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> {
> struct virtnet_info *vi = rq->vq->vdev->priv;
> @@ -485,7 +512,12 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
> } else if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID) {
> skb->ip_summed = CHECKSUM_UNNECESSARY;
> }
> -
> + /* loopback self test for ethtool */
> + if (test_bit(__VIRTNET_TESTING, &vi->flags)) {
> + virtnet_check_lb_frame(vi, skb);
> + dev_kfree_skb_any(skb);
> + return;
> + }
Again, we really need a selftest specific handler and don't put anything
in the ordinary fast path.
> skb->protocol = eth_type_trans(skb, dev);
> pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
> ntohs(skb->protocol), skb->len, skb->pkt_type);
> @@ -813,6 +845,9 @@ static int virtnet_open(struct net_device *dev)
> {
> struct virtnet_info *vi = netdev_priv(dev);
> int i;
> + /* disallow open during test */
> + if (test_bit(__VIRTNET_TESTING, &vi->flags))
> + return -EBUSY;
>
> for (i = 0; i < vi->max_queue_pairs; i++) {
> if (i < vi->curr_queue_pairs)
> @@ -1363,12 +1398,166 @@ static void virtnet_get_channels(struct net_device *dev,
> channels->other_count = 0;
> }
>
> +static int virtnet_reset(struct virtnet_info *vi, u64 *data);
> +
> +static void virtnet_create_lb_frame(struct sk_buff *skb,
> + unsigned int frame_size)
> +{
> + memset(skb->data, 0xFF, frame_size);
> + frame_size &= ~1;
> + memset(&skb->data[frame_size / 2], 0xAA, frame_size / 2 - 1);
> + memset(&skb->data[frame_size / 2 + 10], 0xBE, 1);
> + memset(&skb->data[frame_size / 2 + 12], 0xAF, 1);
> +}
> +
> +static int virtnet_start_loopback(struct virtnet_info *vi)
> +{
> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_LOOPBACK,
> + VIRTIO_NET_CTRL_LOOPBACK_SET, NULL, NULL)) {
> + dev_warn(&vi->dev->dev, "Failed to set loopback.\n");
> + return -EINVAL;
> + }
> + for (i = 0; i < vi->curr_queue_pairs; i++)
> + napi_disable(&vi->rq[i].napi);
> + return 0;
> +}
> +
> +static int virtnet_run_loopback_test(struct virtnet_info *vi)
> +{
> + int i;
> + struct sk_buff *skb;
> + unsigned int size = GOOD_COPY_LEN;
> +
> + for (i = 0; i < 100; i++) {
> + skb = netdev_alloc_skb(vi->dev, size);
> + if (!skb)
> + return -ENOMEM;
> +
> + skb->queue_mapping = 0;
> + skb_put(skb, size);
> + virtnet_create_lb_frame(skb, size);
> + start_xmit(skb, vi->dev);
> + atomic_inc(&vi->lb_count);
> + }
> + free_old_xmit_skbs(&vi->sq[skb->queue_mapping]);
You could not assume the packets were all sent at this time. It may be
delayed by several reasons e.g host load or zerocopy enabled.
> + /* Give queue time to settle before testing results. */
> + msleep(20);
> + for (i = 0; i < vi->curr_queue_pairs; i++) {
> + void *buf;
> + unsigned int len, received = 0;
> +
> + while ((received < 100) &&
> + (buf = virtqueue_get_buf(vi->rq[i].vq, &len)) != NULL) {
Please use a test specific rx handler for this. And the code here
duplicates the code of virtnet_receive(), you can just pass 100 as
budget to that function.
> + receive_buf(&vi->rq[i], buf, len);
> + --vi->rq[i].num;
> + received++;
> + }
> + if ((vi->rq[i].vq)->num_free <
> + virtqueue_get_vring_size(vi->rq[i].vq) / 2)
> + if (!try_fill_recv(&vi->rq[i], GFP_ATOMIC))
> + schedule_delayed_work(&vi->refill, 0);
You disable NAPI but schedule the refill work, it won't be work until
you enable the NAPI. What's more serious is that if there's a refill
work who want to run at the same time of the beginning of the loopback
test, the virtqueue will never get refilled and the test may fail.
Mixing selftest with real workload brings a lot of complexity. Lots of
thing get easier when you using a dedicated vq handler for selftest:
- Don't bother fast path.
- The rx virtqueue will be empty and completely refilled, so if you're
testing 100 packets which is smaller than 256, no need to care about the
refill, (if you detect a fill need, it was probably a bug)
- You can use polling for rx by just disabling the rx notifiy.
- The tx virtqueue will be empty and you can just detach all skbs at the
end of test.
> + }
> + return atomic_read(&vi->lb_count) ? -EIO : 0;
> +}
> +
> +static int virtnet_stop_loopback(struct virtnet_info *vi)
> +{
> + int i;
> +
> + for (i = 0; i < vi->curr_queue_pairs; i++)
> + virtnet_napi_enable(&vi->rq[i]);
> + if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_LOOPBACK,
> + VIRTIO_NET_CTRL_LOOPBACK_UNSET, NULL, NULL)) {
> + dev_warn(&vi->dev->dev, "Failed to unset loopback.\n");
> + return -EINVAL;
> + }
> + return 0;
> +}
> +
> +static int virtnet_loopback_test(struct virtnet_info *vi, u64 *data)
> +{
> + *data = virtnet_start_loopback(vi);
> + if (*data)
> + goto out;
> + *data = virtnet_run_loopback_test(vi);
> + if (*data)
> + goto out;
Again, you may also need to stop the loopback test in this case?
> + *data = virtnet_stop_loopback(vi);
> +out:
> + return *data;
> +}
> +
> +static int virtnet_feature_neg_test(struct virtnet_info *vi, u64 *data)
> +{
> + struct virtio_device *dev = vi->vdev;
> + u8 status;
> +
> + status = dev->config->get_status(dev);
> + if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
> + u8 test_status = status & ~VIRTIO_CONFIG_S_DRIVER_OK;
> +
> + dev->config->set_status(dev, test_status);
> + }
> + *data = virtio_feature_negotiate(dev);
> + dev->config->set_status(dev, status);
> + return *data;
> +}
> +
> +static int virtnet_get_sset_count(struct net_device *netdev, int sset)
> +{
> + switch (sset) {
> + case ETH_SS_TEST:
> + return VIRTNET_NUM_TEST;
> + default:
> + return -EOPNOTSUPP;
> + }
> +}
> +
> +static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
> +{
> + switch (stringset) {
> + case ETH_SS_TEST:
> + memcpy(buf, &virtnet_gstrings_test,
> + sizeof(virtnet_gstrings_test));
> + break;
> + default:
> + break;
> + }
> +}
> +
> +static void virtnet_self_test(struct net_device *netdev,
> + struct ethtool_test *eth_test, u64 *data)
> +{
> + struct virtnet_info *vi = netdev_priv(netdev);
> +
> + memset(data, 0, sizeof(u64) * VIRTNET_NUM_TEST);
> + if (netif_running(netdev)) {
> + set_bit(__VIRTNET_TESTING, &vi->flags);
> + if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
> + if (virtnet_loopback_test(vi, &data[0]))
> + eth_test->flags |= ETH_TEST_FL_FAILED;
> + if (virtnet_feature_neg_test(vi, &data[1]))
> + eth_test->flags |= ETH_TEST_FL_FAILED;
> + if (virtnet_reset(vi, &data[2]))
> + eth_test->flags |= ETH_TEST_FL_FAILED;
virtnet_reset() sounds like a generic helper, better not use a ehttool
specific parameter.
> + }
> + clear_bit(__VIRTNET_TESTING, &vi->flags);
> + } else {
> + dev_warn(&vi->dev->dev,
> + "%s is down, Loopback test will fail.\n", netdev->name);
> + eth_test->flags |= ETH_TEST_FL_FAILED;
> + }
> +}
> +
> static const struct ethtool_ops virtnet_ethtool_ops = {
> .get_drvinfo = virtnet_get_drvinfo,
> .get_link = ethtool_op_get_link,
> .get_ringparam = virtnet_get_ringparam,
> .set_channels = virtnet_set_channels,
> .get_channels = virtnet_get_channels,
> + .self_test = virtnet_self_test,
> + .get_strings = virtnet_get_strings,
> + .get_sset_count = virtnet_get_sset_count,
> };
>
> #define MIN_MTU 68
> @@ -1890,14 +2079,10 @@ static void virtnet_remove(struct virtio_device *vdev)
> free_netdev(vi->dev);
> }
>
> -#ifdef CONFIG_PM_SLEEP
> -static int virtnet_freeze(struct virtio_device *vdev)
> +static void virtnet_stop(struct virtnet_info *vi)
Please split those introduction of new helpers into separate patches.
> {
> - struct virtnet_info *vi = vdev->priv;
> int i;
>
> - unregister_hotcpu_notifier(&vi->nb);
> -
> /* Prevent config work handler from accessing the device */
> mutex_lock(&vi->config_lock);
> vi->config_enable = false;
> @@ -1906,24 +2091,20 @@ static int virtnet_freeze(struct virtio_device *vdev)
> netif_device_detach(vi->dev);
> cancel_delayed_work_sync(&vi->refill);
>
> - if (netif_running(vi->dev)) {
> + if (netif_running(vi->dev))
> for (i = 0; i < vi->max_queue_pairs; i++) {
> napi_disable(&vi->rq[i].napi);
> napi_hash_del(&vi->rq[i].napi);
> netif_napi_del(&vi->rq[i].napi);
> }
> - }
Those removing of braces seems unrelated to the topic, please drop them.
>
> remove_vq_common(vi);
>
> flush_work(&vi->config_work);
> -
> - return 0;
> }
>
> -static int virtnet_restore(struct virtio_device *vdev)
> +static int virtnet_start(struct virtnet_info *vi)
> {
> - struct virtnet_info *vi = vdev->priv;
> int err, i;
>
> err = init_vqs(vi);
> @@ -1944,7 +2125,27 @@ static int virtnet_restore(struct virtio_device *vdev)
> mutex_lock(&vi->config_lock);
> vi->config_enable = true;
> mutex_unlock(&vi->config_lock);
> + return 0;
> +}
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int virtnet_freeze(struct virtio_device *vdev)
> +{
> + struct virtnet_info *vi = vdev->priv;
>
> + unregister_hotcpu_notifier(&vi->nb);
> + virtnet_stop(vi);
> + return 0;
> +}
> +
> +static int virtnet_restore(struct virtio_device *vdev)
> +{
> + struct virtnet_info *vi = vdev->priv;
> + int err;
> +
> + err = virtnet_start(vi);
> + if (err)
> + return err;
> rtnl_lock();
> virtnet_set_queues(vi, vi->curr_queue_pairs);
> rtnl_unlock();
> @@ -1957,6 +2158,22 @@ static int virtnet_restore(struct virtio_device *vdev)
> }
> #endif
>
> +static int virtnet_reset(struct virtnet_info *vi, u64 *data)
> +{
> + struct virtio_device *vdev = vi->vdev;
> + u8 status;
> +
> + virtnet_stop(vi);
> + virtio_feature_negotiate(vdev);
> + *data = virtnet_start(vi);
> + if (*data)
> + return *data;
> + virtnet_set_queues(vi, vi->curr_queue_pairs);
> + status = vdev->config->get_status(vdev);
> + vdev->config->set_status(vdev, status | VIRTIO_CONFIG_S_DRIVER_OK);
> + return 0;
> +}
> +
> static struct virtio_device_id id_table[] = {
> { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
> { 0 },
> diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
> index fed0ce1..2fc396c 100644
> --- a/drivers/virtio/virtio.c
> +++ b/drivers/virtio/virtio.c
> @@ -117,11 +117,10 @@ void virtio_check_driver_offered_feature(const struct virtio_device *vdev,
> }
> EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature);
>
> -static int virtio_dev_probe(struct device *_d)
> +int virtio_feature_negotiate(struct virtio_device *dev)
> {
> - int err, i;
> - struct virtio_device *dev = dev_to_virtio(_d);
> struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
> + int i;
> u32 device_features;
>
> /* We have a driver! */
> @@ -134,7 +133,8 @@ static int virtio_dev_probe(struct device *_d)
> memset(dev->features, 0, sizeof(dev->features));
> for (i = 0; i < drv->feature_table_size; i++) {
> unsigned int f = drv->feature_table[i];
> - BUG_ON(f >= 32);
> + if (f >= 32)
> + return -EINVAL;
> if (device_features & (1 << f))
> set_bit(f, dev->features);
> }
> @@ -145,7 +145,19 @@ static int virtio_dev_probe(struct device *_d)
> set_bit(i, dev->features);
>
> dev->config->finalize_features(dev);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(virtio_feature_negotiate);
>
> +static int virtio_dev_probe(struct device *_d)
Need another patch for this.
> +{
> + int err;
> + struct virtio_device *dev = dev_to_virtio(_d);
> + struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
> +
> + err = virtio_feature_negotiate(dev);
> + if (err)
> + return err;
> err = drv->probe(dev);
> if (err)
> add_status(dev, VIRTIO_CONFIG_S_FAILED);
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index b46671e..49d8ab4 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -98,6 +98,8 @@ struct virtio_device {
> void *priv;
> };
>
> +int virtio_feature_negotiate(struct virtio_device *dev);
> +
> static inline struct virtio_device *dev_to_virtio(struct device *_dev)
> {
> return container_of(_dev, struct virtio_device, dev);
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index 172a7f0..1f31f90 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -201,4 +201,13 @@ struct virtio_net_ctrl_mq {
> #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN 1
> #define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX 0x8000
>
> + /*
> + * Control Loopback(5 is used by VIRTIO_NET_CTRL_GUEST_OFFLOADS in latest qemu)
> + *
> + * The command VIRTIO_NET_CTRL_LOOPBACK_SET is used to require the device come
> + * into loopback state.
> + */
> +#define VIRTIO_NET_CTRL_LOOPBACK 6
> + #define VIRTIO_NET_CTRL_LOOPBACK_SET 0
> + #define VIRTIO_NET_CTRL_LOOPBACK_UNSET 1
> #endif /* _LINUX_VIRTIO_NET_H */
^ permalink raw reply
* [PATCH 1/1] add selftest for virtio-net v1.0
From: Hengjinxiao @ 2014-09-05 1:51 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, linux-api; +Cc: famz, mst
Selftest is an important part of network driver, this patch adds selftest for
virtio-net, including loopback test, negotiate test and reset test. Loopback
test checks whether virtio-net can send and receive packets normally. Negotiate test
executes feature negotiation between virtio-net driver in Guest OS and virtio-net
device in Host OS. Reset test resets virtio-net.
Following last patch, this version has deleted some useless codes and fixed bugs
as you suggest.
Any corrections are welcome.
Signed-off-by: Hengjinxiao <hjxiaohust@gmail.com>
---
drivers/net/virtio_net.c | 241 ++++++++++++++++++++++++++++++++++++++--
drivers/virtio/virtio.c | 20 +++-
include/linux/virtio.h | 2 +
include/uapi/linux/virtio_net.h | 9 ++
4 files changed, 256 insertions(+), 16 deletions(-)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index 59caa06..22d8228 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -28,6 +28,7 @@
#include <linux/cpu.h>
#include <linux/average.h>
#include <net/busy_poll.h>
+#include <linux/pci.h>
static int napi_weight = NAPI_POLL_WEIGHT;
module_param(napi_weight, int, 0444);
@@ -51,6 +52,17 @@ module_param(gso, bool, 0444);
#define MERGEABLE_BUFFER_ALIGN max(L1_CACHE_BYTES, 256)
#define VIRTNET_DRIVER_VERSION "1.0.0"
+#define __VIRTNET_TESTING 0
+
+static const struct {
+ const char string[ETH_GSTRING_LEN];
+} virtnet_gstrings_test[] = {
+ { "loopback test (offline)" },
+ { "negotiate test (offline)" },
+ { "reset test (offline)" },
+};
+
+#define VIRTNET_NUM_TEST ARRAY_SIZE(virtnet_gstrings_test)
struct virtnet_stats {
struct u64_stats_sync tx_syncp;
@@ -104,6 +116,8 @@ struct virtnet_info {
struct send_queue *sq;
struct receive_queue *rq;
unsigned int status;
+ unsigned long flags;
+ atomic_t lb_count;
/* Max # of queue pairs supported by the device */
u16 max_queue_pairs;
@@ -436,6 +450,19 @@ err_buf:
return NULL;
}
+void virtnet_check_lb_frame(struct virtnet_info *vi,
+ struct sk_buff *skb)
+{
+ unsigned int frame_size = skb->len;
+
+ if (*(skb->data + 3) == 0xFF) {
+ if ((*(skb->data + frame_size / 2 + 10) == 0xBE) &&
+ (*(skb->data + frame_size / 2 + 12) == 0xAF)) {
+ atomic_dec(&vi->lb_count);
+ }
+ }
+}
+
static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
{
struct virtnet_info *vi = rq->vq->vdev->priv;
@@ -485,7 +512,12 @@ static void receive_buf(struct receive_queue *rq, void *buf, unsigned int len)
} else if (hdr->hdr.flags & VIRTIO_NET_HDR_F_DATA_VALID) {
skb->ip_summed = CHECKSUM_UNNECESSARY;
}
-
+ /* loopback self test for ethtool */
+ if (test_bit(__VIRTNET_TESTING, &vi->flags)) {
+ virtnet_check_lb_frame(vi, skb);
+ dev_kfree_skb_any(skb);
+ return;
+ }
skb->protocol = eth_type_trans(skb, dev);
pr_debug("Receiving skb proto 0x%04x len %i type %i\n",
ntohs(skb->protocol), skb->len, skb->pkt_type);
@@ -813,6 +845,9 @@ static int virtnet_open(struct net_device *dev)
{
struct virtnet_info *vi = netdev_priv(dev);
int i;
+ /* disallow open during test */
+ if (test_bit(__VIRTNET_TESTING, &vi->flags))
+ return -EBUSY;
for (i = 0; i < vi->max_queue_pairs; i++) {
if (i < vi->curr_queue_pairs)
@@ -1363,12 +1398,166 @@ static void virtnet_get_channels(struct net_device *dev,
channels->other_count = 0;
}
+static int virtnet_reset(struct virtnet_info *vi, u64 *data);
+
+static void virtnet_create_lb_frame(struct sk_buff *skb,
+ unsigned int frame_size)
+{
+ memset(skb->data, 0xFF, frame_size);
+ frame_size &= ~1;
+ memset(&skb->data[frame_size / 2], 0xAA, frame_size / 2 - 1);
+ memset(&skb->data[frame_size / 2 + 10], 0xBE, 1);
+ memset(&skb->data[frame_size / 2 + 12], 0xAF, 1);
+}
+
+static int virtnet_start_loopback(struct virtnet_info *vi)
+{
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_LOOPBACK,
+ VIRTIO_NET_CTRL_LOOPBACK_SET, NULL, NULL)) {
+ dev_warn(&vi->dev->dev, "Failed to set loopback.\n");
+ return -EINVAL;
+ }
+ for (i = 0; i < vi->curr_queue_pairs; i++)
+ napi_disable(&vi->rq[i].napi);
+ return 0;
+}
+
+static int virtnet_run_loopback_test(struct virtnet_info *vi)
+{
+ int i;
+ struct sk_buff *skb;
+ unsigned int size = GOOD_COPY_LEN;
+
+ for (i = 0; i < 100; i++) {
+ skb = netdev_alloc_skb(vi->dev, size);
+ if (!skb)
+ return -ENOMEM;
+
+ skb->queue_mapping = 0;
+ skb_put(skb, size);
+ virtnet_create_lb_frame(skb, size);
+ start_xmit(skb, vi->dev);
+ atomic_inc(&vi->lb_count);
+ }
+ free_old_xmit_skbs(&vi->sq[skb->queue_mapping]);
+ /* Give queue time to settle before testing results. */
+ msleep(20);
+ for (i = 0; i < vi->curr_queue_pairs; i++) {
+ void *buf;
+ unsigned int len, received = 0;
+
+ while ((received < 100) &&
+ (buf = virtqueue_get_buf(vi->rq[i].vq, &len)) != NULL) {
+ receive_buf(&vi->rq[i], buf, len);
+ --vi->rq[i].num;
+ received++;
+ }
+ if ((vi->rq[i].vq)->num_free <
+ virtqueue_get_vring_size(vi->rq[i].vq) / 2)
+ if (!try_fill_recv(&vi->rq[i], GFP_ATOMIC))
+ schedule_delayed_work(&vi->refill, 0);
+ }
+ return atomic_read(&vi->lb_count) ? -EIO : 0;
+}
+
+static int virtnet_stop_loopback(struct virtnet_info *vi)
+{
+ int i;
+
+ for (i = 0; i < vi->curr_queue_pairs; i++)
+ virtnet_napi_enable(&vi->rq[i]);
+ if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_LOOPBACK,
+ VIRTIO_NET_CTRL_LOOPBACK_UNSET, NULL, NULL)) {
+ dev_warn(&vi->dev->dev, "Failed to unset loopback.\n");
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static int virtnet_loopback_test(struct virtnet_info *vi, u64 *data)
+{
+ *data = virtnet_start_loopback(vi);
+ if (*data)
+ goto out;
+ *data = virtnet_run_loopback_test(vi);
+ if (*data)
+ goto out;
+ *data = virtnet_stop_loopback(vi);
+out:
+ return *data;
+}
+
+static int virtnet_feature_neg_test(struct virtnet_info *vi, u64 *data)
+{
+ struct virtio_device *dev = vi->vdev;
+ u8 status;
+
+ status = dev->config->get_status(dev);
+ if (status & VIRTIO_CONFIG_S_DRIVER_OK) {
+ u8 test_status = status & ~VIRTIO_CONFIG_S_DRIVER_OK;
+
+ dev->config->set_status(dev, test_status);
+ }
+ *data = virtio_feature_negotiate(dev);
+ dev->config->set_status(dev, status);
+ return *data;
+}
+
+static int virtnet_get_sset_count(struct net_device *netdev, int sset)
+{
+ switch (sset) {
+ case ETH_SS_TEST:
+ return VIRTNET_NUM_TEST;
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
+static void virtnet_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
+{
+ switch (stringset) {
+ case ETH_SS_TEST:
+ memcpy(buf, &virtnet_gstrings_test,
+ sizeof(virtnet_gstrings_test));
+ break;
+ default:
+ break;
+ }
+}
+
+static void virtnet_self_test(struct net_device *netdev,
+ struct ethtool_test *eth_test, u64 *data)
+{
+ struct virtnet_info *vi = netdev_priv(netdev);
+
+ memset(data, 0, sizeof(u64) * VIRTNET_NUM_TEST);
+ if (netif_running(netdev)) {
+ set_bit(__VIRTNET_TESTING, &vi->flags);
+ if (eth_test->flags == ETH_TEST_FL_OFFLINE) {
+ if (virtnet_loopback_test(vi, &data[0]))
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+ if (virtnet_feature_neg_test(vi, &data[1]))
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+ if (virtnet_reset(vi, &data[2]))
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+ }
+ clear_bit(__VIRTNET_TESTING, &vi->flags);
+ } else {
+ dev_warn(&vi->dev->dev,
+ "%s is down, Loopback test will fail.\n", netdev->name);
+ eth_test->flags |= ETH_TEST_FL_FAILED;
+ }
+}
+
static const struct ethtool_ops virtnet_ethtool_ops = {
.get_drvinfo = virtnet_get_drvinfo,
.get_link = ethtool_op_get_link,
.get_ringparam = virtnet_get_ringparam,
.set_channels = virtnet_set_channels,
.get_channels = virtnet_get_channels,
+ .self_test = virtnet_self_test,
+ .get_strings = virtnet_get_strings,
+ .get_sset_count = virtnet_get_sset_count,
};
#define MIN_MTU 68
@@ -1890,14 +2079,10 @@ static void virtnet_remove(struct virtio_device *vdev)
free_netdev(vi->dev);
}
-#ifdef CONFIG_PM_SLEEP
-static int virtnet_freeze(struct virtio_device *vdev)
+static void virtnet_stop(struct virtnet_info *vi)
{
- struct virtnet_info *vi = vdev->priv;
int i;
- unregister_hotcpu_notifier(&vi->nb);
-
/* Prevent config work handler from accessing the device */
mutex_lock(&vi->config_lock);
vi->config_enable = false;
@@ -1906,24 +2091,20 @@ static int virtnet_freeze(struct virtio_device *vdev)
netif_device_detach(vi->dev);
cancel_delayed_work_sync(&vi->refill);
- if (netif_running(vi->dev)) {
+ if (netif_running(vi->dev))
for (i = 0; i < vi->max_queue_pairs; i++) {
napi_disable(&vi->rq[i].napi);
napi_hash_del(&vi->rq[i].napi);
netif_napi_del(&vi->rq[i].napi);
}
- }
remove_vq_common(vi);
flush_work(&vi->config_work);
-
- return 0;
}
-static int virtnet_restore(struct virtio_device *vdev)
+static int virtnet_start(struct virtnet_info *vi)
{
- struct virtnet_info *vi = vdev->priv;
int err, i;
err = init_vqs(vi);
@@ -1944,7 +2125,27 @@ static int virtnet_restore(struct virtio_device *vdev)
mutex_lock(&vi->config_lock);
vi->config_enable = true;
mutex_unlock(&vi->config_lock);
+ return 0;
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int virtnet_freeze(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+ unregister_hotcpu_notifier(&vi->nb);
+ virtnet_stop(vi);
+ return 0;
+}
+
+static int virtnet_restore(struct virtio_device *vdev)
+{
+ struct virtnet_info *vi = vdev->priv;
+ int err;
+
+ err = virtnet_start(vi);
+ if (err)
+ return err;
rtnl_lock();
virtnet_set_queues(vi, vi->curr_queue_pairs);
rtnl_unlock();
@@ -1957,6 +2158,22 @@ static int virtnet_restore(struct virtio_device *vdev)
}
#endif
+static int virtnet_reset(struct virtnet_info *vi, u64 *data)
+{
+ struct virtio_device *vdev = vi->vdev;
+ u8 status;
+
+ virtnet_stop(vi);
+ virtio_feature_negotiate(vdev);
+ *data = virtnet_start(vi);
+ if (*data)
+ return *data;
+ virtnet_set_queues(vi, vi->curr_queue_pairs);
+ status = vdev->config->get_status(vdev);
+ vdev->config->set_status(vdev, status | VIRTIO_CONFIG_S_DRIVER_OK);
+ return 0;
+}
+
static struct virtio_device_id id_table[] = {
{ VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
{ 0 },
diff --git a/drivers/virtio/virtio.c b/drivers/virtio/virtio.c
index fed0ce1..2fc396c 100644
--- a/drivers/virtio/virtio.c
+++ b/drivers/virtio/virtio.c
@@ -117,11 +117,10 @@ void virtio_check_driver_offered_feature(const struct virtio_device *vdev,
}
EXPORT_SYMBOL_GPL(virtio_check_driver_offered_feature);
-static int virtio_dev_probe(struct device *_d)
+int virtio_feature_negotiate(struct virtio_device *dev)
{
- int err, i;
- struct virtio_device *dev = dev_to_virtio(_d);
struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+ int i;
u32 device_features;
/* We have a driver! */
@@ -134,7 +133,8 @@ static int virtio_dev_probe(struct device *_d)
memset(dev->features, 0, sizeof(dev->features));
for (i = 0; i < drv->feature_table_size; i++) {
unsigned int f = drv->feature_table[i];
- BUG_ON(f >= 32);
+ if (f >= 32)
+ return -EINVAL;
if (device_features & (1 << f))
set_bit(f, dev->features);
}
@@ -145,7 +145,19 @@ static int virtio_dev_probe(struct device *_d)
set_bit(i, dev->features);
dev->config->finalize_features(dev);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(virtio_feature_negotiate);
+static int virtio_dev_probe(struct device *_d)
+{
+ int err;
+ struct virtio_device *dev = dev_to_virtio(_d);
+ struct virtio_driver *drv = drv_to_virtio(dev->dev.driver);
+
+ err = virtio_feature_negotiate(dev);
+ if (err)
+ return err;
err = drv->probe(dev);
if (err)
add_status(dev, VIRTIO_CONFIG_S_FAILED);
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index b46671e..49d8ab4 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -98,6 +98,8 @@ struct virtio_device {
void *priv;
};
+int virtio_feature_negotiate(struct virtio_device *dev);
+
static inline struct virtio_device *dev_to_virtio(struct device *_dev)
{
return container_of(_dev, struct virtio_device, dev);
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index 172a7f0..1f31f90 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -201,4 +201,13 @@ struct virtio_net_ctrl_mq {
#define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MIN 1
#define VIRTIO_NET_CTRL_MQ_VQ_PAIRS_MAX 0x8000
+ /*
+ * Control Loopback(5 is used by VIRTIO_NET_CTRL_GUEST_OFFLOADS in latest qemu)
+ *
+ * The command VIRTIO_NET_CTRL_LOOPBACK_SET is used to require the device come
+ * into loopback state.
+ */
+#define VIRTIO_NET_CTRL_LOOPBACK 6
+ #define VIRTIO_NET_CTRL_LOOPBACK_SET 0
+ #define VIRTIO_NET_CTRL_LOOPBACK_UNSET 1
#endif /* _LINUX_VIRTIO_NET_H */
--
1.8.3.2
^ permalink raw reply related
* Re: [PATCH v9 net-next 2/4] net: filter: split filter.h and expose eBPF to user space
From: David Miller @ 2014-09-04 21:49 UTC (permalink / raw)
To: ast-uqk4Ao+rVK5Wk0Htik3J/w
Cc: mingo-DgEjT+Ai2ygdnm+yROfE0A,
torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
luto-kltTT9wpgjJwATOyAt5JVQ, rostedt-nx8X9YLhiw1AfugRpC6u6w,
hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
chema-hpIqsD4AKlfQT0dZR+AlfA, edumazet-hpIqsD4AKlfQT0dZR+AlfA,
a.p.zijlstra-/NLkJaSkS4VmR6Xm/wNWPw, hpa-YMNOUZJC4hwAvxtiuMwx3w,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b,
keescook-F7+t8E8rja9g9hUCZPvPmw, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
dborkman-H+wXaHxf7aLQT0dZR+AlfA
In-Reply-To: <CAMEtUuz6zraph_8xXW43bNbh5uPRki0yOufMfV5pKMRLhDrQQA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
From: Alexei Starovoitov <ast-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
Date: Wed, 3 Sep 2014 22:41:48 -0700
> Do you want me to resubmit just first two?
Yes, you can't submit hodge-podge path series, either it's all to
be applied or it's all RFC material.
^ permalink raw reply
* Re: [PATCH Resend] memfd_test: Add missing argument to printf()
From: Shuah Khan @ 2014-09-04 18:31 UTC (permalink / raw)
To: Pranith Kumar, Andrew Morton, Hugh Dickins, David Herrmann,
open list:KERNEL SELFTEST F..., open list, Shuah Khan
In-Reply-To: <1409846302-3513-1-git-send-email-bobby.prani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 09/04/2014 09:58 AM, Pranith Kumar wrote:
> Add a missing path argument buf to printf()
>
> Signed-off-by: Pranith Kumar <bobby.prani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reviewed-by: David Herrmann <dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> tools/testing/selftests/memfd/memfd_test.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c
> index 3634c90..c343df8 100644
> --- a/tools/testing/selftests/memfd/memfd_test.c
> +++ b/tools/testing/selftests/memfd/memfd_test.c
> @@ -205,7 +205,7 @@ static void mfd_fail_open(int fd, int flags, mode_t mode)
> sprintf(buf, "/proc/self/fd/%d", fd);
> r = open(buf, flags, mode);
> if (r >= 0) {
> - printf("open(%s) didn't fail as expected\n");
> + printf("open(%s) didn't fail as expected\n", buf);
> abort();
> }
> }
>
Thanks. I queued it for 3.17 fixes.
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* [PATCH Resend] memfd_test: Add missing argument to printf()
From: Pranith Kumar @ 2014-09-04 15:58 UTC (permalink / raw)
To: Shuah Khan, Andrew Morton, Hugh Dickins, David Herrmann,
open list:KERNEL SELFTEST F..., open list
Add a missing path argument buf to printf()
Signed-off-by: Pranith Kumar <bobby.prani-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reviewed-by: David Herrmann <dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
tools/testing/selftests/memfd/memfd_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/memfd/memfd_test.c b/tools/testing/selftests/memfd/memfd_test.c
index 3634c90..c343df8 100644
--- a/tools/testing/selftests/memfd/memfd_test.c
+++ b/tools/testing/selftests/memfd/memfd_test.c
@@ -205,7 +205,7 @@ static void mfd_fail_open(int fd, int flags, mode_t mode)
sprintf(buf, "/proc/self/fd/%d", fd);
r = open(buf, flags, mode);
if (r >= 0) {
- printf("open(%s) didn't fail as expected\n");
+ printf("open(%s) didn't fail as expected\n", buf);
abort();
}
}
--
2.1.0
^ permalink raw reply related
* Re: [PATCH] selftests/memfd: fix mfd_fail_open() to pass in buf to printf
From: Shuah Khan @ 2014-09-04 15:21 UTC (permalink / raw)
To: Pranith Kumar
Cc: David Herrmann, Andrew Morton, Hugh Dickins,
open list:KERNEL SELFTEST F..., LKML
In-Reply-To: <CAJhHMCCuyC3HUfUktTpkQcxnqJDpA2O0Gqc8wZMq0TwB5PeY6w-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 09/04/2014 09:16 AM, Pranith Kumar wrote:
> On Thu, Sep 4, 2014 at 11:07 AM, Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
>> mfd_fail_open() doesn't pass in the buffer to printf resulting
>> in the following warning:
>>
>> memfd_test.c: In function ‘mfd_fail_open’:
>> memfd_test.c:208:3: warning: format ‘%s’ expects a matching ‘char *’ argument [-Wformat=]
>> printf("open(%s) didn't fail as expected\n");
>> ^
>>
>> This change fixes the problem.
>>
>> Signed-off-by: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
>
> Hi Shuah,
>
> I already sent in a patch to fix this:
>
> https://lkml.org/lkml/2014/8/31/85
>
> Can you please pick that up?
Hi Pranith,
Great. Could you please resend it to me cc'ing linux-api as well.
thanks,
-- Shuah
--
Shuah Khan
Sr. Linux Kernel Developer
Samsung Research America (Silicon Valley)
shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org | (970) 217-8978
^ permalink raw reply
* Re: [PATCH] selftests/memfd: fix mfd_fail_open() to pass in buf to printf
From: Pranith Kumar @ 2014-09-04 15:16 UTC (permalink / raw)
To: Shuah Khan
Cc: David Herrmann, Andrew Morton, Hugh Dickins,
open list:KERNEL SELFTEST F..., LKML
In-Reply-To: <1409843242-6596-1-git-send-email-shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On Thu, Sep 4, 2014 at 11:07 AM, Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org> wrote:
> mfd_fail_open() doesn't pass in the buffer to printf resulting
> in the following warning:
>
> memfd_test.c: In function ‘mfd_fail_open’:
> memfd_test.c:208:3: warning: format ‘%s’ expects a matching ‘char *’ argument [-Wformat=]
> printf("open(%s) didn't fail as expected\n");
> ^
>
> This change fixes the problem.
>
> Signed-off-by: Shuah Khan <shuahkh-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
Hi Shuah,
I already sent in a patch to fix this:
https://lkml.org/lkml/2014/8/31/85
Can you please pick that up?
Thanks!
--
Pranith
^ permalink raw reply
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;
as well as URLs for NNTP newsgroup(s).