* [RFC PATCH bpf-next v2 5/6] selftests/bpf: test BPF_MAP_DUMP command on a bpf hashmap
From: Brian Vazquez @ 2019-06-27 20:24 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
In-Reply-To: <20190627202417.33370-1-brianvv@google.com>
This tests exercise the new command on a bpf hashmap and make sure it
works as expected.
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
tools/testing/selftests/bpf/test_maps.c | 70 ++++++++++++++++++++++++-
1 file changed, 68 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index a3fbc571280a9..3df72b46fd1d9 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -309,6 +309,73 @@ static void test_hashmap_walk(unsigned int task, void *data)
close(fd);
}
+static void test_hashmap_dump(void)
+{
+ int fd, i, max_entries = 3;
+ uint64_t keys[max_entries], values[max_entries];
+ uint64_t key, value, next_key;
+ bool next_key_valid = true;
+ void *buf, *elem, *prev_key;
+ u32 buf_len;
+ const int elem_size = sizeof(key) + sizeof(value);
+
+ fd = helper_fill_hashmap(max_entries);
+
+ // Get the elements in the hashmap, and store them in that order
+ assert(bpf_map_get_next_key(fd, NULL, &key) == 0);
+ i = 0;
+ keys[i] = key;
+ for (i = 1; next_key_valid; i++) {
+ next_key_valid = bpf_map_get_next_key(fd, &key, &next_key) == 0;
+ assert(bpf_map_lookup_elem(fd, &key, &values[i - 1]) == 0);
+ keys[i-1] = key;
+ key = next_key;
+ }
+
+ // Alloc memory for the whole table
+ buf = malloc(elem_size * max_entries);
+ assert(buf != NULL);
+
+ // Check that buf_len < elem_size returns EINVAL
+ buf_len = elem_size-1;
+ errno = 0;
+ assert(bpf_map_dump(fd, NULL, buf, &buf_len) == -1 && errno == EINVAL);
+
+ // Check that it returns the first two elements
+ errno = 0;
+ buf_len = elem_size * 2;
+ prev_key = NULL;
+ i = 0;
+ assert(bpf_map_dump(fd, prev_key, buf, &buf_len) == 0 &&
+ buf_len == 2*elem_size);
+ elem = buf;
+ assert((*(uint64_t *)elem) == keys[i] &&
+ (*(uint64_t *)(elem + sizeof(key))) == values[i]);
+ elem = buf + elem_size;
+ i++;
+ assert((*(uint64_t *)elem) == keys[i] &&
+ (*(uint64_t *)(elem + sizeof(key))) == values[i]);
+ i++;
+
+ /* Continue reading from map and verify buf_len only contains 1 element
+ * even though buf_len is 2 elem_size.
+ */
+ prev_key = elem;
+ assert(bpf_map_dump(fd, prev_key, buf, &buf_len) == 0 &&
+ buf_len == elem_size);
+ elem = buf;
+ assert((*(uint64_t *)elem) == keys[i] &&
+ (*(uint64_t *)(elem + sizeof(key))) == values[i]);
+
+ // Check that there are no more entries after last_key
+ prev_key = &keys[i];
+ assert(bpf_map_dump(fd, prev_key, buf, &buf_len) == -1 &&
+ errno == ENOENT);
+
+ free(buf);
+ close(fd);
+}
+
static void test_hashmap_zero_seed(void)
{
int i, first, second, old_flags;
@@ -1668,6 +1735,7 @@ static void run_all_tests(void)
test_hashmap_percpu(0, NULL);
test_hashmap_walk(0, NULL);
test_hashmap_zero_seed();
+ test_hashmap_dump();
test_arraymap(0, NULL);
test_arraymap_percpu(0, NULL);
@@ -1705,11 +1773,9 @@ int main(void)
map_flags = BPF_F_NO_PREALLOC;
run_all_tests();
-
#define CALL
#include <map_tests/tests.h>
#undef CALL
-
printf("test_maps: OK, %d SKIPPED\n", skips);
return 0;
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [RFC PATCH bpf-next v2 6/6] selftests/bpf: add test to measure performance of BPF_MAP_DUMP
From: Brian Vazquez @ 2019-06-27 20:24 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
In-Reply-To: <20190627202417.33370-1-brianvv@google.com>
This tests compares the amount of time that takes to read an entire
table of 100K elements on a bpf hashmap using both BPF_MAP_DUMP and
BPF_MAP_GET_NEXT_KEY + BPF_MAP_LOOKUP_ELEM.
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
tools/testing/selftests/bpf/test_maps.c | 71 +++++++++++++++++++++++++
1 file changed, 71 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
index 3df72b46fd1d9..61050272c20ee 100644
--- a/tools/testing/selftests/bpf/test_maps.c
+++ b/tools/testing/selftests/bpf/test_maps.c
@@ -18,6 +18,7 @@
#include <sys/socket.h>
#include <netinet/in.h>
#include <linux/bpf.h>
+#include <linux/time64.h>
#include <bpf/bpf.h>
#include <bpf/libbpf.h>
@@ -376,6 +377,75 @@ static void test_hashmap_dump(void)
close(fd);
}
+static void test_hashmap_dump_perf(void)
+{
+ int fd, i, max_entries = 100000;
+ uint64_t key, value, next_key;
+ bool next_key_valid = true;
+ void *buf;
+ u32 buf_len, entries;
+ int j, k = 0;
+ int num_ent, off;
+ int clk_id = CLOCK_MONOTONIC;
+ struct timespec begin, end;
+ long long time_spent, dump_time_spent;
+ double res;
+ int tests[] = {1, 2, 230, 5000, 73000, 100000, 234567};
+ int test_len = ARRAY_SIZE(tests);
+ const int elem_size = sizeof(key) + sizeof(value);
+
+ fd = helper_fill_hashmap(max_entries);
+ // Alloc memory considering the largest buffer
+ buf = malloc(elem_size * tests[test_len-1]);
+ assert(buf != NULL);
+
+test:
+ entries = tests[k];
+ buf_len = elem_size*tests[k];
+ k++;
+ clock_gettime(clk_id, &begin);
+ errno = 0;
+ i = 0;
+ while (errno == 0) {
+ bpf_map_dump(fd, !i ? NULL : &key,
+ buf, &buf_len);
+ if (errno)
+ break;
+ num_ent = buf_len / elem_size;
+ for (j = 0, off = 0; j < num_ent; j++) {
+ key = *((uint64_t *)(buf + off));
+ off += sizeof(key);
+ value = *((uint64_t *)(buf + off));
+ off += sizeof(value);
+ }
+ i += num_ent;
+ }
+ clock_gettime(clk_id, &end);
+ assert(i == max_entries);
+ dump_time_spent = NSEC_PER_SEC * (end.tv_sec - begin.tv_sec) +
+ end.tv_nsec - begin.tv_nsec;
+ next_key_valid = true;
+ clock_gettime(clk_id, &begin);
+ assert(bpf_map_get_next_key(fd, NULL, &key) == 0);
+ for (i = 0; next_key_valid; i++) {
+ next_key_valid = bpf_map_get_next_key(fd, &key, &next_key) == 0;
+ assert(bpf_map_lookup_elem(fd, &key, &value) == 0);
+ key = next_key;
+ }
+ clock_gettime(clk_id, &end);
+ time_spent = NSEC_PER_SEC * (end.tv_sec - begin.tv_sec) +
+ end.tv_nsec - begin.tv_nsec;
+ res = (1-((double)dump_time_spent/time_spent))*100;
+ printf("buf_len_%u:\t %llu entry-by-entry: %llu improvement %lf\n",
+ entries, dump_time_spent, time_spent, res);
+ assert(i == max_entries);
+
+ if (k < test_len)
+ goto test;
+ free(buf);
+ close(fd);
+}
+
static void test_hashmap_zero_seed(void)
{
int i, first, second, old_flags;
@@ -1736,6 +1806,7 @@ static void run_all_tests(void)
test_hashmap_walk(0, NULL);
test_hashmap_zero_seed();
test_hashmap_dump();
+ test_hashmap_dump_perf();
test_arraymap(0, NULL);
test_arraymap_percpu(0, NULL);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [RFC PATCH bpf-next v2 4/6] libbpf: support BPF_MAP_DUMP command
From: Brian Vazquez @ 2019-06-27 20:24 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
In-Reply-To: <20190627202417.33370-1-brianvv@google.com>
Make libbpf aware of new BPF_MAP_DUMP command and add bpf_map_dump and
bpf_map_dump_flags to use them from the library.
Suggested-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
tools/lib/bpf/bpf.c | 28 ++++++++++++++++++++++++++++
tools/lib/bpf/bpf.h | 4 ++++
tools/lib/bpf/libbpf.map | 2 ++
3 files changed, 34 insertions(+)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index c7d7993c44bb0..c1139b7db756a 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -368,6 +368,34 @@ int bpf_map_update_elem(int fd, const void *key, const void *value,
return sys_bpf(BPF_MAP_UPDATE_ELEM, &attr, sizeof(attr));
}
+int bpf_map_dump(int fd, const void *prev_key, void *buf, void *buf_len)
+{
+ union bpf_attr attr;
+
+ memset(&attr, 0, sizeof(attr));
+ attr.dump.map_fd = fd;
+ attr.dump.prev_key = ptr_to_u64(prev_key);
+ attr.dump.buf = ptr_to_u64(buf);
+ attr.dump.buf_len = ptr_to_u64(buf_len);
+
+ return sys_bpf(BPF_MAP_DUMP, &attr, sizeof(attr));
+}
+
+int bpf_map_dump_flags(int fd, const void *prev_key, void *buf, void *buf_len,
+ __u64 flags)
+{
+ union bpf_attr attr;
+
+ memset(&attr, 0, sizeof(attr));
+ attr.dump.map_fd = fd;
+ attr.dump.prev_key = ptr_to_u64(prev_key);
+ attr.dump.buf = ptr_to_u64(buf);
+ attr.dump.buf_len = ptr_to_u64(buf_len);
+ attr.dump.flags = flags;
+
+ return sys_bpf(BPF_MAP_DUMP, &attr, sizeof(attr));
+}
+
int bpf_map_lookup_elem(int fd, const void *key, void *value)
{
union bpf_attr attr;
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index ff42ca043dc8f..86496443440e9 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -112,6 +112,10 @@ LIBBPF_API int bpf_verify_program(enum bpf_prog_type type,
LIBBPF_API int bpf_map_update_elem(int fd, const void *key, const void *value,
__u64 flags);
+LIBBPF_API int bpf_map_dump(int fd, const void *prev_key, void *buf,
+ void *buf_len);
+LIBBPF_API int bpf_map_dump_flags(int fd, const void *prev_key, void *buf,
+ void *buf_len, __u64 flags);
LIBBPF_API int bpf_map_lookup_elem(int fd, const void *key, void *value);
LIBBPF_API int bpf_map_lookup_elem_flags(int fd, const void *key, void *value,
__u64 flags);
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2c6d835620d25..e7641773cfb0f 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -173,4 +173,6 @@ LIBBPF_0.0.4 {
btf__parse_elf;
bpf_object__load_xattr;
libbpf_num_possible_cpus;
+ bpf_map_dump;
+ bpf_map_dump_flags;
} LIBBPF_0.0.3;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [RFC PATCH bpf-next v2 2/6] bpf: add BPF_MAP_DUMP command to access more than one entry per call
From: Brian Vazquez @ 2019-06-27 20:24 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
In-Reply-To: <20190627202417.33370-1-brianvv@google.com>
This introduces a new command to retrieve a variable number of entries
from a bpf map wrapping the existing bpf methods:
map_get_next_key and map_lookup_elem
Note that map_dump doesn't guarantee that reading the entire table is
consistent since this function is always racing with kernel and user code
but the same behaviour is found when the entire table is walked using
the current interfaces: map_get_next_key + map_lookup_elem.
It is also important to note that when a locked map is provided it is
consistent only for 1 entry at the time, meaning that the buf returned
might or might not be consistent.
Suggested-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
include/uapi/linux/bpf.h | 9 ++++
kernel/bpf/syscall.c | 108 +++++++++++++++++++++++++++++++++++++++
2 files changed, 117 insertions(+)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b077507efa3f3..1d753958874df 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -106,6 +106,7 @@ enum bpf_cmd {
BPF_TASK_FD_QUERY,
BPF_MAP_LOOKUP_AND_DELETE_ELEM,
BPF_MAP_FREEZE,
+ BPF_MAP_DUMP,
};
enum bpf_map_type {
@@ -385,6 +386,14 @@ union bpf_attr {
__u64 flags;
};
+ struct { /* struct used by BPF_MAP_DUMP command */
+ __u32 map_fd;
+ __aligned_u64 prev_key;
+ __aligned_u64 buf;
+ __aligned_u64 buf_len; /* input/output: len of buf */
+ __u64 flags;
+ } dump;
+
struct { /* anonymous struct used by BPF_PROG_LOAD command */
__u32 prog_type; /* one of enum bpf_prog_type */
__u32 insn_cnt;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index a1823a50f9be0..7653346b5cfd1 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1097,6 +1097,111 @@ static int map_get_next_key(union bpf_attr *attr)
return err;
}
+/* last field in 'union bpf_attr' used by this command */
+#define BPF_MAP_DUMP_LAST_FIELD dump.buf_len
+
+static int map_dump(union bpf_attr *attr)
+{
+ void __user *ukey = u64_to_user_ptr(attr->dump.prev_key);
+ void __user *ubuf = u64_to_user_ptr(attr->dump.buf);
+ u32 __user *ubuf_len = u64_to_user_ptr(attr->dump.buf_len);
+ int ufd = attr->dump.map_fd;
+ struct bpf_map *map;
+ void *buf, *prev_key, *key, *value;
+ u32 value_size, elem_size, buf_len, cp_len;
+ struct fd f;
+ int err;
+
+ if (CHECK_ATTR(BPF_MAP_DUMP))
+ return -EINVAL;
+
+ attr->flags = 0;
+ if (attr->dump.flags & ~BPF_F_LOCK)
+ return -EINVAL;
+
+ f = fdget(ufd);
+ map = __bpf_map_get(f);
+ if (IS_ERR(map))
+ return PTR_ERR(map);
+ if (!(map_get_sys_perms(map, f) & FMODE_CAN_READ)) {
+ err = -EPERM;
+ goto err_put;
+ }
+
+ if ((attr->dump.flags & BPF_F_LOCK) &&
+ !map_value_has_spin_lock(map)) {
+ err = -EINVAL;
+ goto err_put;
+ }
+
+ if (map->map_type == BPF_MAP_TYPE_QUEUE ||
+ map->map_type == BPF_MAP_TYPE_STACK) {
+ err = -ENOTSUPP;
+ goto err_put;
+ }
+
+ value_size = bpf_map_value_size(map);
+
+ err = get_user(buf_len, ubuf_len);
+ if (err)
+ goto err_put;
+
+ elem_size = map->key_size + value_size;
+ if (buf_len < elem_size) {
+ err = -EINVAL;
+ goto err_put;
+ }
+
+ if (ukey) {
+ prev_key = __bpf_copy_key(ukey, map->key_size);
+ if (IS_ERR(prev_key)) {
+ err = PTR_ERR(prev_key);
+ goto err_put;
+ }
+ } else {
+ prev_key = NULL;
+ }
+
+ err = -ENOMEM;
+ buf = kmalloc(elem_size, GFP_USER | __GFP_NOWARN);
+ if (!buf)
+ goto err_put;
+
+ key = buf;
+ value = key + map->key_size;
+ for (cp_len = 0; cp_len + elem_size <= buf_len ; cp_len += elem_size) {
+next:
+ if (signal_pending(current)) {
+ err = -EINTR;
+ break;
+ }
+
+ rcu_read_lock();
+ err = map->ops->map_get_next_key(map, prev_key, key);
+ rcu_read_unlock();
+
+ if (err)
+ break;
+
+ if (bpf_map_copy_value(map, key, value, attr->dump.flags))
+ goto next;
+
+ if (copy_to_user(ubuf + cp_len, buf, elem_size))
+ break;
+
+ prev_key = key;
+ }
+
+ if (cp_len)
+ err = 0;
+ if (copy_to_user(ubuf_len, &cp_len, sizeof(cp_len)))
+ err = -EFAULT;
+ kfree(buf);
+err_put:
+ fdput(f);
+ return err;
+}
+
#define BPF_MAP_LOOKUP_AND_DELETE_ELEM_LAST_FIELD value
static int map_lookup_and_delete_elem(union bpf_attr *attr)
@@ -2891,6 +2996,9 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
case BPF_MAP_LOOKUP_AND_DELETE_ELEM:
err = map_lookup_and_delete_elem(&attr);
break;
+ case BPF_MAP_DUMP:
+ err = map_dump(&attr);
+ break;
default:
err = -EINVAL;
break;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [RFC PATCH bpf-next v2 1/6] bpf: add bpf_map_value_size and bp_map_copy_value helper functions
From: Brian Vazquez @ 2019-06-27 20:24 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
In-Reply-To: <20190627202417.33370-1-brianvv@google.com>
Move reusable code from map_lookup_elem to helper functions to avoid code
duplication in kernel/bpf/syscall.c
Suggested-by: Stanislav Fomichev <sdf@google.com>
Signed-off-by: Brian Vazquez <brianvv@google.com>
---
kernel/bpf/syscall.c | 134 +++++++++++++++++++++++--------------------
1 file changed, 73 insertions(+), 61 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 7713cf39795a4..a1823a50f9be0 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -126,6 +126,76 @@ static struct bpf_map *find_and_alloc_map(union bpf_attr *attr)
return map;
}
+static u32 bpf_map_value_size(struct bpf_map *map)
+{
+ if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
+ map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
+ map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY ||
+ map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
+ return round_up(map->value_size, 8) * num_possible_cpus();
+ else if (IS_FD_MAP(map))
+ return sizeof(u32);
+ else
+ return map->value_size;
+}
+
+static int bpf_map_copy_value(struct bpf_map *map, void *key, void *value,
+ __u64 flags)
+{
+ void *ptr;
+ int err;
+
+ if (bpf_map_is_dev_bound(map))
+ return bpf_map_offload_lookup_elem(map, key, value);
+
+ preempt_disable();
+ this_cpu_inc(bpf_prog_active);
+ if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
+ map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH) {
+ err = bpf_percpu_hash_copy(map, key, value);
+ } else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
+ err = bpf_percpu_array_copy(map, key, value);
+ } else if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
+ err = bpf_percpu_cgroup_storage_copy(map, key, value);
+ } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE) {
+ err = bpf_stackmap_copy(map, key, value);
+ } else if (IS_FD_ARRAY(map)) {
+ err = bpf_fd_array_map_lookup_elem(map, key, value);
+ } else if (IS_FD_HASH(map)) {
+ err = bpf_fd_htab_map_lookup_elem(map, key, value);
+ } else if (map->map_type == BPF_MAP_TYPE_REUSEPORT_SOCKARRAY) {
+ err = bpf_fd_reuseport_array_lookup_elem(map, key, value);
+ } else if (map->map_type == BPF_MAP_TYPE_QUEUE ||
+ map->map_type == BPF_MAP_TYPE_STACK) {
+ err = map->ops->map_peek_elem(map, value);
+ } else {
+ rcu_read_lock();
+ if (map->ops->map_lookup_elem_sys_only)
+ ptr = map->ops->map_lookup_elem_sys_only(map, key);
+ else
+ ptr = map->ops->map_lookup_elem(map, key);
+ if (IS_ERR(ptr)) {
+ err = PTR_ERR(ptr);
+ } else if (!ptr) {
+ err = -ENOENT;
+ } else {
+ err = 0;
+ if (flags & BPF_F_LOCK)
+ /* lock 'ptr' and copy everything but lock */
+ copy_map_value_locked(map, value, ptr, true);
+ else
+ copy_map_value(map, value, ptr);
+ /* mask lock, since value wasn't zero inited */
+ check_and_init_map_lock(map, value);
+ }
+ rcu_read_unlock();
+ }
+ this_cpu_dec(bpf_prog_active);
+ preempt_enable();
+
+ return err;
+}
+
void *bpf_map_area_alloc(size_t size, int numa_node)
{
/* We really just want to fail instead of triggering OOM killer
@@ -729,7 +799,7 @@ static int map_lookup_elem(union bpf_attr *attr)
void __user *uvalue = u64_to_user_ptr(attr->value);
int ufd = attr->map_fd;
struct bpf_map *map;
- void *key, *value, *ptr;
+ void *key, *value;
u32 value_size;
struct fd f;
int err;
@@ -761,72 +831,14 @@ static int map_lookup_elem(union bpf_attr *attr)
goto err_put;
}
- if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
- map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH ||
- map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY ||
- map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE)
- value_size = round_up(map->value_size, 8) * num_possible_cpus();
- else if (IS_FD_MAP(map))
- value_size = sizeof(u32);
- else
- value_size = map->value_size;
+ value_size = bpf_map_value_size(map);
err = -ENOMEM;
value = kmalloc(value_size, GFP_USER | __GFP_NOWARN);
if (!value)
goto free_key;
- if (bpf_map_is_dev_bound(map)) {
- err = bpf_map_offload_lookup_elem(map, key, value);
- goto done;
- }
-
- preempt_disable();
- this_cpu_inc(bpf_prog_active);
- if (map->map_type == BPF_MAP_TYPE_PERCPU_HASH ||
- map->map_type == BPF_MAP_TYPE_LRU_PERCPU_HASH) {
- err = bpf_percpu_hash_copy(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_PERCPU_ARRAY) {
- err = bpf_percpu_array_copy(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE) {
- err = bpf_percpu_cgroup_storage_copy(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_STACK_TRACE) {
- err = bpf_stackmap_copy(map, key, value);
- } else if (IS_FD_ARRAY(map)) {
- err = bpf_fd_array_map_lookup_elem(map, key, value);
- } else if (IS_FD_HASH(map)) {
- err = bpf_fd_htab_map_lookup_elem(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_REUSEPORT_SOCKARRAY) {
- err = bpf_fd_reuseport_array_lookup_elem(map, key, value);
- } else if (map->map_type == BPF_MAP_TYPE_QUEUE ||
- map->map_type == BPF_MAP_TYPE_STACK) {
- err = map->ops->map_peek_elem(map, value);
- } else {
- rcu_read_lock();
- if (map->ops->map_lookup_elem_sys_only)
- ptr = map->ops->map_lookup_elem_sys_only(map, key);
- else
- ptr = map->ops->map_lookup_elem(map, key);
- if (IS_ERR(ptr)) {
- err = PTR_ERR(ptr);
- } else if (!ptr) {
- err = -ENOENT;
- } else {
- err = 0;
- if (attr->flags & BPF_F_LOCK)
- /* lock 'ptr' and copy everything but lock */
- copy_map_value_locked(map, value, ptr, true);
- else
- copy_map_value(map, value, ptr);
- /* mask lock, since value wasn't zero inited */
- check_and_init_map_lock(map, value);
- }
- rcu_read_unlock();
- }
- this_cpu_dec(bpf_prog_active);
- preempt_enable();
-
-done:
+ err = bpf_map_copy_value(map, key, value, attr->flags);
if (err)
goto free_value;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [RFC PATCH bpf-next v2 0/6] bpf: add BPF_MAP_DUMP command to
From: Brian Vazquez @ 2019-06-27 20:24 UTC (permalink / raw)
To: Brian Vazquez, Alexei Starovoitov, Daniel Borkmann,
David S . Miller
Cc: Stanislav Fomichev, Willem de Bruijn, Petar Penkov, linux-kernel,
netdev, bpf, Brian Vazquez
This introduces a new command to retrieve a variable number of entries
from a bpf map.
This new command can be executed from the existing BPF syscall as
follows:
err = bpf(BPF_MAP_DUMP, union bpf_attr *attr, u32 size)
using attr->dump.map_fd, attr->dump.prev_key, attr->dump.buf,
attr->dump.buf_len
returns zero or negative error, and populates buf and buf_len on
succees
This implementation is wrapping the existing bpf methods:
map_get_next_key and map_lookup_elem
the results show that even with a 1-elem_size buffer, it runs ~40 faster
than the current implementation, improvements of ~85% are reported when
the buffer size is increased, although, after the buffer size is around
5% of the total number of entries there's no huge difference in
increasing
it.
Tested:
Tried different size buffers to handle case where the bulk is bigger, or
the elements to retrieve are less than the existing ones, all runs read
a map of 100K entries. Below are the results(in ns) from the different
runs:
buf_len_1: 55528939 entry-by-entry: 97244981 improvement
42.897887%
buf_len_2: 34425779 entry-by-entry: 88863122 improvement
61.259769%
buf_len_230: 11700316 entry-by-entry: 88753301 improvement
86.817036%
buf_len_5000: 11615290 entry-by-entry: 88362637 improvement
86.854976%
buf_len_73000: 12083976 entry-by-entry: 89956483 improvement
86.566865%
buf_len_100000: 12638913 entry-by-entry: 89642303 improvement
85.900727%
buf_len_234567: 11873964 entry-by-entry: 89080077 improvement
86.670461%
Changelog:
v2:
- use proper bpf-next tag
Brian Vazquez (6):
bpf: add bpf_map_value_size and bp_map_copy_value helper functions
bpf: add BPF_MAP_DUMP command to access more than one entry per call
bpf: keep bpf.h in sync with tools/
libbpf: support BPF_MAP_DUMP command
selftests/bpf: test BPF_MAP_DUMP command on a bpf hashmap
selftests/bpf: add test to measure performance of BPF_MAP_DUMP
include/uapi/linux/bpf.h | 9 +
kernel/bpf/syscall.c | 242 ++++++++++++++++++------
tools/include/uapi/linux/bpf.h | 9 +
tools/lib/bpf/bpf.c | 28 +++
tools/lib/bpf/bpf.h | 4 +
tools/lib/bpf/libbpf.map | 2 +
tools/testing/selftests/bpf/test_maps.c | 141 +++++++++++++-
7 files changed, 372 insertions(+), 63 deletions(-)
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [PATCH bpf-next 1/3] libbpf: capture value in BTF type info for BTF-defined map defs
From: Andrii Nakryiko @ 2019-06-27 20:19 UTC (permalink / raw)
To: Song Liu
Cc: Andrii Nakryiko, Alexei Starovoitov, daniel@iogearbox.net,
Kernel Team, bpf@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <079A7D73-697C-4CFD-97F3-7CFB741BE4C3@fb.com>
On Thu, Jun 27, 2019 at 10:56 AM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Jun 27, 2019, at 10:47 AM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Thu, Jun 27, 2019 at 10:27 AM Song Liu <songliubraving@fb.com> wrote:
> >>
> >>
> >>
> >>> On Jun 26, 2019, at 4:21 PM, Andrii Nakryiko <andriin@fb.com> wrote:
> >>>
> >>> Change BTF-defined map definitions to capture compile-time integer
> >>> values as part of BTF type definition, to avoid split of key/value type
> >>> information and actual type/size/flags initialization for maps.
> >>
> >> If I have an old bpf program and compiled it with new llvm, will it
> >> work with new libbpf?
> >
> > You mean BPF programs that used previous incarnation of BTF-defined
> > maps? No, they won't work. But we never released them, so I think it's
> > ok to change them. Nothing should be using that except for selftests,
> > which I fixed.
>
> I see. This makes sense.
>
> >
> >>
> >>
> >>>
> >>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> >>> ---
> >
> > <snip>
> >
> >>> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> >>> index 1a5b1accf091..aa5ddf58c088 100644
> >>> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> >>> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> >>> @@ -8,6 +8,9 @@
> >>> */
> >>> #define SEC(NAME) __attribute__((section(NAME), used))
> >>>
> >>> +#define __int(name, val) int (*name)[val]
> >>> +#define __type(name, val) val *name
> >>> +
> >>
> >> I think we need these two in libbpf.
> >
> > Yes, but it's another story for another set of patches. We'll need to
> > provide bpf_helpers as part of libbpf for inclusion into BPF programs,
> > but there are a bunch of problems right now with existing
> > bpf_heplers.h that prevents us from just copying it into libbpf. We'll
> > need to resolve those first.
> >
> > But then again, there is no use of __int and __type for user-space
> > programs, so for now it's ok.
>
> OK. How about we put these two lines in an separate patch?
Sure, no problem.
>
> Thanks,
> Song
>
^ permalink raw reply
* [PATCH v2 bpf-next 4/4] bpftool: use libbpf_[enable|disable]_sys_bpf()
From: Song Liu @ 2019-06-27 20:19 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, kernel-team, lmb, jannh, gregkh, Song Liu
In-Reply-To: <20190627201923.2589391-1-songliubraving@fb.com>
This patch calls libbpf_[enable|disable]_sys_bpf() from bpftool. This
allows users with access to /dev/bpf to perform operations like root.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/bpf/bpftool/feature.c | 2 +-
tools/bpf/bpftool/main.c | 5 +++++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/bpf/bpftool/feature.c b/tools/bpf/bpftool/feature.c
index d672d9086fff..772c9f445d34 100644
--- a/tools/bpf/bpftool/feature.c
+++ b/tools/bpf/bpftool/feature.c
@@ -583,7 +583,7 @@ static int do_probe(int argc, char **argv)
/* Detection assumes user has sufficient privileges (CAP_SYS_ADMIN).
* Let's approximate, and restrict usage to root user only.
*/
- if (geteuid()) {
+ if (!libbpf_enable_sys_bpf()) {
p_err("please run this command as root user");
return -1;
}
diff --git a/tools/bpf/bpftool/main.c b/tools/bpf/bpftool/main.c
index 4879f6395c7e..56959c5ac552 100644
--- a/tools/bpf/bpftool/main.c
+++ b/tools/bpf/bpftool/main.c
@@ -390,6 +390,10 @@ int main(int argc, char **argv)
if (argc < 0)
usage();
+ if (!libbpf_enable_sys_bpf()) {
+ p_err("cannot enable access to sys_bpf()");
+ usage();
+ }
ret = cmd_select(cmds, argc, argv, do_help);
if (json_output)
@@ -400,5 +404,6 @@ int main(int argc, char **argv)
delete_pinned_obj_table(&map_table);
}
+ libbpf_disable_sys_bpf();
return ret;
}
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 3/4] libbpf: add libbpf_[enable|disable]_sys_bpf()
From: Song Liu @ 2019-06-27 20:19 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, kernel-team, lmb, jannh, gregkh, Song Liu
In-Reply-To: <20190627201923.2589391-1-songliubraving@fb.com>
This patch adds two more API to libbpf: libbpf_enable_sys_bpf() and
libbpf_disable_sys_bpf().
For root, these two APIs are no-op.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/lib/bpf/libbpf.c | 54 ++++++++++++++++++++++++++++++++++++++++
tools/lib/bpf/libbpf.h | 7 ++++++
tools/lib/bpf/libbpf.map | 2 ++
3 files changed, 63 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 5186b7710430..449764840c5a 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -35,6 +35,7 @@
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/vfs.h>
+#include <sys/ioctl.h>
#include <tools/libc_compat.h>
#include <libelf.h>
#include <gelf.h>
@@ -4286,3 +4287,56 @@ int libbpf_num_possible_cpus(void)
}
return cpus;
}
+
+LIBBPF_API bool libbpf_enable_sys_bpf(void)
+{
+ char *cp, errmsg[STRERR_BUFSIZE];
+ int fd, ret;
+
+ if (geteuid() == 0)
+ return true;
+
+ fd = open(LIBBPF_DEV_BPF, O_WRONLY);
+ if (fd < 0) {
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("failed to open %s: %s\n", LIBBPF_DEV_BPF, cp);
+ return false;
+ }
+
+ ret = ioctl(fd, BPF_DEV_IOCTL_ENABLE_SYS_BPF);
+
+ if (ret) {
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("failed to enable access to sys_bpf(): %s\n", cp);
+ close(fd);
+ return false;
+ }
+ close(fd);
+ pr_debug("enabled access to sys_bpf() for non-privileged user\n");
+ return true;
+}
+
+LIBBPF_API void libbpf_disable_sys_bpf(void)
+{
+ char *cp, errmsg[STRERR_BUFSIZE];
+ int fd, ret;
+
+ if (geteuid() == 0)
+ return;
+
+ fd = open(LIBBPF_DEV_BPF, O_WRONLY);
+ if (fd < 0) {
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("failed to open %s: %s\n", LIBBPF_DEV_BPF, cp);
+ return;
+ }
+
+ ret = ioctl(fd, BPF_DEV_IOCTL_DISABLE_SYS_BPF);
+ if (ret) {
+ cp = libbpf_strerror_r(errno, errmsg, sizeof(errmsg));
+ pr_warning("failed to disable access to sys_bpf(): %s\n", cp);
+ close(fd);
+ }
+ close(fd);
+ pr_debug("disabled access to sys_bpf() for non-privileged user\n");
+}
diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
index d639f47e3110..0e3b9c0f1cdf 100644
--- a/tools/lib/bpf/libbpf.h
+++ b/tools/lib/bpf/libbpf.h
@@ -470,6 +470,13 @@ bpf_program__bpil_offs_to_addr(struct bpf_prog_info_linear *info_linear);
*/
LIBBPF_API int libbpf_num_possible_cpus(void);
+#define LIBBPF_DEV_BPF "/dev/bpf"
+
+/* (For non-root user) get permission to access bpf() syscall */
+LIBBPF_API bool libbpf_enable_sys_bpf(void);
+/* (For non-root user) put permission to access bpf() syscall */
+LIBBPF_API void libbpf_disable_sys_bpf(void);
+
#ifdef __cplusplus
} /* extern "C" */
#endif
diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
index 2c6d835620d2..c5951315a3a5 100644
--- a/tools/lib/bpf/libbpf.map
+++ b/tools/lib/bpf/libbpf.map
@@ -173,4 +173,6 @@ LIBBPF_0.0.4 {
btf__parse_elf;
bpf_object__load_xattr;
libbpf_num_possible_cpus;
+ libbpf_enable_sys_bpf;
+ libbpf_disable_sys_bpf;
} LIBBPF_0.0.3;
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 2/4] bpf: sync tools/include/uapi/linux/bpf.h
From: Song Liu @ 2019-06-27 20:19 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, kernel-team, lmb, jannh, gregkh, Song Liu
In-Reply-To: <20190627201923.2589391-1-songliubraving@fb.com>
Sync changes for bpf_dev_ioctl.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
tools/include/uapi/linux/bpf.h | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index b077507efa3f..13e148bd6c7c 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -3541,4 +3541,10 @@ struct bpf_sysctl {
*/
};
+#define BPF_IOCTL 0xBF
+
+/* enable/disable sys_bpf() for current */
+#define BPF_DEV_IOCTL_ENABLE_SYS_BPF _IO(BPF_IOCTL, 0x01)
+#define BPF_DEV_IOCTL_DISABLE_SYS_BPF _IO(BPF_IOCTL, 0x02)
+
#endif /* _UAPI__LINUX_BPF_H__ */
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 1/4] bpf: unprivileged BPF access via /dev/bpf
From: Song Liu @ 2019-06-27 20:19 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, kernel-team, lmb, jannh, gregkh, Song Liu
In-Reply-To: <20190627201923.2589391-1-songliubraving@fb.com>
This patch introduce unprivileged BPF access. The access control is
achieved via device /dev/bpf. Users with write access to /dev/bpf are able
to call sys_bpf().
Two ioctl command are added to /dev/bpf:
The two commands enable/disable permission to call sys_bpf() for current
task. This permission is noted by bpf_permitted in task_struct. This
permission is inherited during clone(CLONE_THREAD).
Helper function bpf_capable() is added to check whether the task has got
permission via /dev/bpf.
Signed-off-by: Song Liu <songliubraving@fb.com>
---
Documentation/ioctl/ioctl-number.txt | 1 +
include/linux/bpf.h | 11 +++++
include/linux/sched.h | 3 ++
include/uapi/linux/bpf.h | 6 +++
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/cgroup.c | 2 +-
kernel/bpf/core.c | 4 +-
kernel/bpf/cpumap.c | 2 +-
kernel/bpf/devmap.c | 2 +-
kernel/bpf/hashtab.c | 4 +-
kernel/bpf/lpm_trie.c | 2 +-
kernel/bpf/offload.c | 2 +-
kernel/bpf/queue_stack_maps.c | 2 +-
kernel/bpf/reuseport_array.c | 2 +-
kernel/bpf/stackmap.c | 2 +-
kernel/bpf/syscall.c | 71 +++++++++++++++++++++-------
kernel/bpf/verifier.c | 2 +-
kernel/bpf/xskmap.c | 2 +-
kernel/fork.c | 5 ++
net/core/filter.c | 6 +--
20 files changed, 99 insertions(+), 34 deletions(-)
diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index c9558146ac58..19998b99d603 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -327,6 +327,7 @@ Code Seq#(hex) Include File Comments
0xB4 00-0F linux/gpio.h <mailto:linux-gpio@vger.kernel.org>
0xB5 00-0F uapi/linux/rpmsg.h <mailto:linux-remoteproc@vger.kernel.org>
0xB6 all linux/fpga-dfl.h
+0xBP 01-02 uapi/linux/bpf.h <mailto:bpf@vger.kernel.org>
0xC0 00-0F linux/usb/iowarrior.h
0xCA 00-0F uapi/misc/cxl.h
0xCA 10-2F uapi/misc/ocxl.h
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a62e7889b0b6..d17d57dff467 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -14,6 +14,10 @@
#include <linux/numa.h>
#include <linux/wait.h>
#include <linux/u64_stats_sync.h>
+#include <linux/sched.h>
+#include <linux/capability.h>
+
+#include <asm/current.h>
struct bpf_verifier_env;
struct perf_event;
@@ -742,6 +746,11 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
const union bpf_attr *kattr,
union bpf_attr __user *uattr);
+
+static inline bool bpf_capable(int cap)
+{
+ return current->bpf_permitted || capable(cap);
+}
#else /* !CONFIG_BPF_SYSCALL */
static inline struct bpf_prog *bpf_prog_get(u32 ufd)
{
@@ -874,6 +883,8 @@ static inline int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
{
return -ENOTSUPP;
}
+
+#define bpf_capable(cap) capable((cap))
#endif /* CONFIG_BPF_SYSCALL */
static inline struct bpf_prog *bpf_prog_get_type(u32 ufd,
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 11837410690f..e03ee4779c9e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -732,6 +732,9 @@ struct task_struct {
/* to be used once the psi infrastructure lands upstream. */
unsigned use_memdelay:1;
#endif
+#ifdef CONFIG_BPF_SYSCALL
+ unsigned bpf_permitted:1;
+#endif
unsigned long atomic_flags; /* Flags requiring atomic access. */
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index b077507efa3f..13e148bd6c7c 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -3541,4 +3541,10 @@ struct bpf_sysctl {
*/
};
+#define BPF_IOCTL 0xBF
+
+/* enable/disable sys_bpf() for current */
+#define BPF_DEV_IOCTL_ENABLE_SYS_BPF _IO(BPF_IOCTL, 0x01)
+#define BPF_DEV_IOCTL_DISABLE_SYS_BPF _IO(BPF_IOCTL, 0x02)
+
#endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
index 1c65ce0098a9..9ae668fa9185 100644
--- a/kernel/bpf/arraymap.c
+++ b/kernel/bpf/arraymap.c
@@ -73,7 +73,7 @@ static struct bpf_map *array_map_alloc(union bpf_attr *attr)
bool percpu = attr->map_type == BPF_MAP_TYPE_PERCPU_ARRAY;
int ret, numa_node = bpf_map_attr_numa_node(attr);
u32 elem_size, index_mask, max_entries;
- bool unpriv = !capable(CAP_SYS_ADMIN);
+ bool unpriv = !bpf_capable(CAP_SYS_ADMIN);
u64 cost, array_size, mask64;
struct bpf_map_memory mem;
struct bpf_array *array;
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 077ed3a19848..cf9c6a95e4d2 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -794,7 +794,7 @@ cgroup_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
case BPF_FUNC_get_current_cgroup_id:
return &bpf_get_current_cgroup_id_proto;
case BPF_FUNC_trace_printk:
- if (capable(CAP_SYS_ADMIN))
+ if (bpf_capable(CAP_SYS_ADMIN))
return bpf_get_trace_printk_proto();
/* fall through */
default:
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 561ed07d3007..762a855404d2 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -646,7 +646,7 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
void bpf_prog_kallsyms_add(struct bpf_prog *fp)
{
if (!bpf_prog_kallsyms_candidate(fp) ||
- !capable(CAP_SYS_ADMIN))
+ !bpf_capable(CAP_SYS_ADMIN))
return;
spin_lock_bh(&bpf_lock);
@@ -768,7 +768,7 @@ static int bpf_jit_charge_modmem(u32 pages)
{
if (atomic_long_add_return(pages, &bpf_jit_current) >
(bpf_jit_limit >> PAGE_SHIFT)) {
- if (!capable(CAP_SYS_ADMIN)) {
+ if (!bpf_capable(CAP_SYS_ADMIN)) {
atomic_long_sub(pages, &bpf_jit_current);
return -EPERM;
}
diff --git a/kernel/bpf/cpumap.c b/kernel/bpf/cpumap.c
index 8dff08768087..4c6054626b4f 100644
--- a/kernel/bpf/cpumap.c
+++ b/kernel/bpf/cpumap.c
@@ -83,7 +83,7 @@ static struct bpf_map *cpu_map_alloc(union bpf_attr *attr)
u64 cost;
int ret;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return ERR_PTR(-EPERM);
/* check sanity of attributes */
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index 40e86a7e0ef0..b7c3785be289 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -83,7 +83,7 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
u64 cost;
int err;
- if (!capable(CAP_NET_ADMIN))
+ if (!bpf_capable(CAP_NET_ADMIN))
return ERR_PTR(-EPERM);
/* check sanity of attributes */
diff --git a/kernel/bpf/hashtab.c b/kernel/bpf/hashtab.c
index 22066a62c8c9..461a75c311a4 100644
--- a/kernel/bpf/hashtab.c
+++ b/kernel/bpf/hashtab.c
@@ -244,13 +244,13 @@ static int htab_map_alloc_check(union bpf_attr *attr)
BUILD_BUG_ON(offsetof(struct htab_elem, fnode.next) !=
offsetof(struct htab_elem, hash_node.pprev));
- if (lru && !capable(CAP_SYS_ADMIN))
+ if (lru && !bpf_capable(CAP_SYS_ADMIN))
/* LRU implementation is much complicated than other
* maps. Hence, limit to CAP_SYS_ADMIN for now.
*/
return -EPERM;
- if (zero_seed && !capable(CAP_SYS_ADMIN))
+ if (zero_seed && !bpf_capable(CAP_SYS_ADMIN))
/* Guard against local DoS, and discourage production use. */
return -EPERM;
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
index 56e6c75d354d..571962022fdf 100644
--- a/kernel/bpf/lpm_trie.c
+++ b/kernel/bpf/lpm_trie.c
@@ -543,7 +543,7 @@ static struct bpf_map *trie_alloc(union bpf_attr *attr)
u64 cost = sizeof(*trie), cost_per_node;
int ret;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return ERR_PTR(-EPERM);
/* check sanity of attributes */
diff --git a/kernel/bpf/offload.c b/kernel/bpf/offload.c
index ba635209ae9a..d3e5378c5a15 100644
--- a/kernel/bpf/offload.c
+++ b/kernel/bpf/offload.c
@@ -366,7 +366,7 @@ struct bpf_map *bpf_map_offload_map_alloc(union bpf_attr *attr)
struct bpf_offloaded_map *offmap;
int err;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return ERR_PTR(-EPERM);
if (attr->map_type != BPF_MAP_TYPE_ARRAY &&
attr->map_type != BPF_MAP_TYPE_HASH)
diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
index f697647ceb54..01d848f1a783 100644
--- a/kernel/bpf/queue_stack_maps.c
+++ b/kernel/bpf/queue_stack_maps.c
@@ -45,7 +45,7 @@ static bool queue_stack_map_is_full(struct bpf_queue_stack *qs)
/* Called from syscall */
static int queue_stack_map_alloc_check(union bpf_attr *attr)
{
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
/* check sanity of attributes */
diff --git a/kernel/bpf/reuseport_array.c b/kernel/bpf/reuseport_array.c
index 50c083ba978c..840f38a58c7d 100644
--- a/kernel/bpf/reuseport_array.c
+++ b/kernel/bpf/reuseport_array.c
@@ -154,7 +154,7 @@ static struct bpf_map *reuseport_array_alloc(union bpf_attr *attr)
struct bpf_map_memory mem;
u64 array_size;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return ERR_PTR(-EPERM);
array_size = sizeof(*array);
diff --git a/kernel/bpf/stackmap.c b/kernel/bpf/stackmap.c
index 052580c33d26..1eab27b0bc17 100644
--- a/kernel/bpf/stackmap.c
+++ b/kernel/bpf/stackmap.c
@@ -90,7 +90,7 @@ static struct bpf_map *stack_map_alloc(union bpf_attr *attr)
u64 cost, n_buckets;
int err;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return ERR_PTR(-EPERM);
if (attr->map_flags & ~STACK_CREATE_FLAG_MASK)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 7713cf39795a..13a3ba59d509 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -23,6 +23,8 @@
#include <linux/timekeeping.h>
#include <linux/ctype.h>
#include <linux/nospec.h>
+#include <linux/miscdevice.h>
+#include <linux/resource.h>
#define IS_FD_ARRAY(map) ((map)->map_type == BPF_MAP_TYPE_PROG_ARRAY || \
(map)->map_type == BPF_MAP_TYPE_PERF_EVENT_ARRAY || \
@@ -1166,7 +1168,7 @@ static int map_freeze(const union bpf_attr *attr)
err = -EBUSY;
goto err_put;
}
- if (!capable(CAP_SYS_ADMIN)) {
+ if (!bpf_capable(CAP_SYS_ADMIN)) {
err = -EPERM;
goto err_put;
}
@@ -1616,7 +1618,7 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
if (!IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) &&
(attr->prog_flags & BPF_F_ANY_ALIGNMENT) &&
- !capable(CAP_SYS_ADMIN))
+ !bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
/* copy eBPF program license from user space */
@@ -1629,11 +1631,12 @@ static int bpf_prog_load(union bpf_attr *attr, union bpf_attr __user *uattr)
is_gpl = license_is_gpl_compatible(license);
if (attr->insn_cnt == 0 ||
- attr->insn_cnt > (capable(CAP_SYS_ADMIN) ? BPF_COMPLEXITY_LIMIT_INSNS : BPF_MAXINSNS))
+ attr->insn_cnt > (bpf_capable(CAP_SYS_ADMIN) ?
+ BPF_COMPLEXITY_LIMIT_INSNS : BPF_MAXINSNS))
return -E2BIG;
if (type != BPF_PROG_TYPE_SOCKET_FILTER &&
type != BPF_PROG_TYPE_CGROUP_SKB &&
- !capable(CAP_SYS_ADMIN))
+ !bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
bpf_prog_load_fixup_attach_type(attr);
@@ -1861,7 +1864,7 @@ static int bpf_prog_attach(const union bpf_attr *attr)
struct bpf_prog *prog;
int ret;
- if (!capable(CAP_NET_ADMIN))
+ if (!bpf_capable(CAP_NET_ADMIN))
return -EPERM;
if (CHECK_ATTR(BPF_PROG_ATTACH))
@@ -1951,7 +1954,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
{
enum bpf_prog_type ptype;
- if (!capable(CAP_NET_ADMIN))
+ if (!bpf_capable(CAP_NET_ADMIN))
return -EPERM;
if (CHECK_ATTR(BPF_PROG_DETACH))
@@ -2007,7 +2010,7 @@ static int bpf_prog_detach(const union bpf_attr *attr)
static int bpf_prog_query(const union bpf_attr *attr,
union bpf_attr __user *uattr)
{
- if (!capable(CAP_NET_ADMIN))
+ if (!bpf_capable(CAP_NET_ADMIN))
return -EPERM;
if (CHECK_ATTR(BPF_PROG_QUERY))
return -EINVAL;
@@ -2051,7 +2054,7 @@ static int bpf_prog_test_run(const union bpf_attr *attr,
struct bpf_prog *prog;
int ret = -ENOTSUPP;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
if (CHECK_ATTR(BPF_PROG_TEST_RUN))
return -EINVAL;
@@ -2088,7 +2091,7 @@ static int bpf_obj_get_next_id(const union bpf_attr *attr,
if (CHECK_ATTR(BPF_OBJ_GET_NEXT_ID) || next_id >= INT_MAX)
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
next_id++;
@@ -2114,7 +2117,7 @@ static int bpf_prog_get_fd_by_id(const union bpf_attr *attr)
if (CHECK_ATTR(BPF_PROG_GET_FD_BY_ID))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
spin_lock_bh(&prog_idr_lock);
@@ -2148,7 +2151,7 @@ static int bpf_map_get_fd_by_id(const union bpf_attr *attr)
attr->open_flags & ~BPF_OBJ_FLAG_MASK)
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
f_flags = bpf_get_file_flag(attr->open_flags);
@@ -2323,7 +2326,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
info.run_time_ns = stats.nsecs;
info.run_cnt = stats.cnt;
- if (!capable(CAP_SYS_ADMIN)) {
+ if (!bpf_capable(CAP_SYS_ADMIN)) {
info.jited_prog_len = 0;
info.xlated_prog_len = 0;
info.nr_jited_ksyms = 0;
@@ -2641,7 +2644,7 @@ static int bpf_btf_load(const union bpf_attr *attr)
if (CHECK_ATTR(BPF_BTF_LOAD))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
return btf_new_fd(attr);
@@ -2654,7 +2657,7 @@ static int bpf_btf_get_fd_by_id(const union bpf_attr *attr)
if (CHECK_ATTR(BPF_BTF_GET_FD_BY_ID))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
return btf_get_fd_by_id(attr->btf_id);
@@ -2723,7 +2726,7 @@ static int bpf_task_fd_query(const union bpf_attr *attr,
if (CHECK_ATTR(BPF_TASK_FD_QUERY))
return -EINVAL;
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
if (attr->task_fd_query.flags != 0)
@@ -2791,7 +2794,7 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
union bpf_attr attr = {};
int err;
- if (sysctl_unprivileged_bpf_disabled && !capable(CAP_SYS_ADMIN))
+ if (sysctl_unprivileged_bpf_disabled && !bpf_capable(CAP_SYS_ADMIN))
return -EPERM;
err = bpf_check_uarg_tail_zero(uattr, sizeof(attr), size);
@@ -2886,3 +2889,39 @@ SYSCALL_DEFINE3(bpf, int, cmd, union bpf_attr __user *, uattr, unsigned int, siz
return err;
}
+
+static long bpf_dev_ioctl(struct file *filp,
+ unsigned int ioctl, unsigned long arg)
+{
+ switch (ioctl) {
+ case BPF_DEV_IOCTL_ENABLE_SYS_BPF:
+ current->bpf_permitted = 1;
+ break;
+ case BPF_DEV_IOCTL_DISABLE_SYS_BPF:
+ current->bpf_permitted = 0;
+ break;
+ default:
+ return -EINVAL;
+ }
+ return 0;
+}
+
+static const struct file_operations bpf_chardev_ops = {
+ .unlocked_ioctl = bpf_dev_ioctl,
+};
+
+static struct miscdevice bpf_dev = {
+ .minor = MISC_DYNAMIC_MINOR,
+ .name = "bpf",
+ .fops = &bpf_chardev_ops,
+ .mode = 0220,
+};
+
+static int __init bpf_dev_init(void)
+{
+ if (misc_register(&bpf_dev))
+ pr_warn("BPF: Failed to create /dev/bpf. Continue without it...\n");
+
+ return 0;
+}
+device_initcall(bpf_dev_init);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 0e079b2298f8..79dc4d641cf3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -9134,7 +9134,7 @@ int bpf_check(struct bpf_prog **prog, union bpf_attr *attr,
env->insn_aux_data[i].orig_idx = i;
env->prog = *prog;
env->ops = bpf_verifier_ops[env->prog->type];
- is_priv = capable(CAP_SYS_ADMIN);
+ is_priv = bpf_capable(CAP_SYS_ADMIN);
/* grab the mutex to protect few globals used by verifier */
if (!is_priv)
diff --git a/kernel/bpf/xskmap.c b/kernel/bpf/xskmap.c
index ef7338cebd18..06063679c27a 100644
--- a/kernel/bpf/xskmap.c
+++ b/kernel/bpf/xskmap.c
@@ -21,7 +21,7 @@ static struct bpf_map *xsk_map_alloc(union bpf_attr *attr)
int cpu, err;
u64 cost;
- if (!capable(CAP_NET_ADMIN))
+ if (!bpf_capable(CAP_NET_ADMIN))
return ERR_PTR(-EPERM);
if (attr->max_entries == 0 || attr->key_size != 4 ||
diff --git a/kernel/fork.c b/kernel/fork.c
index 75675b9bf6df..b312562c1523 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1998,6 +1998,11 @@ static __latent_entropy struct task_struct *copy_process(
p->sequential_io_avg = 0;
#endif
+#ifdef CONFIG_BPF_SYSCALL
+ if ((clone_flags & CLONE_THREAD) == 0)
+ p->bpf_permitted = 0;
+#endif
+
/* Perform scheduler related setup. Assign this task to a CPU. */
retval = sched_fork(clone_flags, p);
if (retval)
diff --git a/net/core/filter.c b/net/core/filter.c
index 2014d76e0d2a..01ccf031849c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5875,7 +5875,7 @@ bpf_base_func_proto(enum bpf_func_id func_id)
break;
}
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return NULL;
switch (func_id) {
@@ -6438,7 +6438,7 @@ static bool cg_skb_is_valid_access(int off, int size,
return false;
case bpf_ctx_range(struct __sk_buff, data):
case bpf_ctx_range(struct __sk_buff, data_end):
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return false;
break;
}
@@ -6450,7 +6450,7 @@ static bool cg_skb_is_valid_access(int off, int size,
case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
break;
case bpf_ctx_range(struct __sk_buff, tstamp):
- if (!capable(CAP_SYS_ADMIN))
+ if (!bpf_capable(CAP_SYS_ADMIN))
return false;
break;
default:
--
2.17.1
^ permalink raw reply related
* [PATCH v2 bpf-next 0/4] sys_bpf() access control via /dev/bpf
From: Song Liu @ 2019-06-27 20:19 UTC (permalink / raw)
To: netdev, bpf; +Cc: ast, daniel, kernel-team, lmb, jannh, gregkh, Song Liu
Changes v1 => v2:
1. Make default mode of /dev/bpf 0220 (Greg);
2. Rename ioctl commands as BPF_DEV_IOCTL_ENABLE_SYS_BPF and
BPF_DEV_IOCTL_DISABLE_SYS_BPF (Daniel);
3. Save space for task_struct by reusing free bit (Daniel);
4. Make the permission per process (Lorenz).
Currently, most access to sys_bpf() is limited to root. However, there are
use cases that would benefit from non-privileged use of sys_bpf(), e.g.
systemd.
This set introduces a new model to control the access to sys_bpf(). A
special device, /dev/bpf, is introduced to manage access to sys_bpf().
Users with access to open /dev/bpf will be able to access most of
sys_bpf() features. The use can get access to sys_bpf() by opening /dev/bpf
and use ioctl to enable/disable the access.
The permission to access sys_bpf() is marked by bit bpf_permitted in
task_struct. During clone(), child will inherit this bit if CLONE_THREAD
is set. Therefore, the permission is shared within same user process,
but not via fork().
libbpf APIs libbpf_[enable|disable]_sys_bpf() are added to help get and
put the permission. bpftool is updated to use these APIs.
Song Liu (4):
bpf: unprivileged BPF access via /dev/bpf
bpf: sync tools/include/uapi/linux/bpf.h
libbpf: add libbpf_[enable|disable]_sys_bpf()
bpftool: use libbpf_[enable|disable]_sys_bpf()
Documentation/ioctl/ioctl-number.txt | 1 +
include/linux/bpf.h | 11 +++++
include/linux/sched.h | 3 ++
include/uapi/linux/bpf.h | 6 +++
kernel/bpf/arraymap.c | 2 +-
kernel/bpf/cgroup.c | 2 +-
kernel/bpf/core.c | 4 +-
kernel/bpf/cpumap.c | 2 +-
kernel/bpf/devmap.c | 2 +-
kernel/bpf/hashtab.c | 4 +-
kernel/bpf/lpm_trie.c | 2 +-
kernel/bpf/offload.c | 2 +-
kernel/bpf/queue_stack_maps.c | 2 +-
kernel/bpf/reuseport_array.c | 2 +-
kernel/bpf/stackmap.c | 2 +-
kernel/bpf/syscall.c | 71 +++++++++++++++++++++-------
kernel/bpf/verifier.c | 2 +-
kernel/bpf/xskmap.c | 2 +-
kernel/fork.c | 5 ++
net/core/filter.c | 6 +--
tools/bpf/bpftool/feature.c | 2 +-
tools/bpf/bpftool/main.c | 5 ++
tools/include/uapi/linux/bpf.h | 6 +++
tools/lib/bpf/libbpf.c | 54 +++++++++++++++++++++
tools/lib/bpf/libbpf.h | 7 +++
tools/lib/bpf/libbpf.map | 2 +
26 files changed, 174 insertions(+), 35 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [PATCH V33 24/30] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Stephen Smalley @ 2019-06-27 20:16 UTC (permalink / raw)
To: James Morris
Cc: Andy Lutomirski, Andy Lutomirski, Matthew Garrett, linux-security,
LKML, Linux API, David Howells, Alexei Starovoitov,
Matthew Garrett, Network Development, Chun-Yi Lee,
Daniel Borkmann, linux-security-module
In-Reply-To: <alpine.LRH.2.21.1906280332500.17363@namei.org>
On 6/27/19 2:06 PM, James Morris wrote:
> On Thu, 27 Jun 2019, Stephen Smalley wrote:
>
>> There are two scenarios where finer-grained distinctions make sense:
>>
>> - Users may need to enable specific functionality that falls under the
>> umbrella of "confidentiality" or "integrity" lockdown. Finer-grained lockdown
>> reasons free them from having to make an all-or-nothing choice between lost
>> functionality or no lockdown at all.
>
> Agreed. This will be used for more than just UEFI secure boot on desktops,
> e.g. embedded systems using verified boot, where finer grained policy will
> be needed for what are sometimes very specific use-cases (which may be
> also covered by other mitigations).
>
>> This can be supported directly by the
>> lockdown module without any help from SELinux or other security modules; we
>> just need the ability to specify these finer-grained lockdown levels via the
>> boot parameters and securityfs nodes.
>
> If the lockdown LSM implements fine grained policy (rather than the simple
> coarse grained policy), I'd suggest adding a new lockdown level of
> 'custom' which by default enables all hooks but allows selective
> disablement via params/sysfs.
>
> This would be simpler than telling users to use a different lockdown LSM
> for this.
>
>> - Different processes/programs may need to use different sets of functionality
>> restricted via lockdown confidentiality or integrity categories. If we have
>> to allow all-or-none for the set of interfaces/functionality covered by the
>> generic confidentiality or integrity categories, then we'll end up having to
>> choose between lost functionality or overprivileged processes, neither of
>> which is optimal.
>>
>> Is it truly the case that everything under the "confidentiality" category
>> poses the same level of risk to kernel confidentiality, and similarly for
>> everything under the "integrity" category? If not, then being able to
>> distinguish them definitely has benefit.
>
> Good question. We can't know the answer to this unless we know how an
> attacker might leverage access.
>
> The value here IMHO is more in allowing tradeoffs to be made by system
> designers vs. disabling lockdown entirely.
>
>> I'm still not clear though on how/if this will compose with or be overridden
>> by other security modules. We would need some means for another security
>> module to take over lockdown decisions once it has initialized (including
>> policy load), and to be able to access state that is currently private to the
>> lockdown module, like the level.
>
> Why not utilize stacking (restrictively), similarly to capabilities?
That would only allow the LSM to further lock down the system above the
lockdown level set at boot, not grant exemptions for specific
functionality/interfaces required by the user or by a specific
process/program. You'd have to boot with lockdown=none (or your
lockdown=custom suggestion) in order for the LSM to allow anything
covered by the integrity or confidentiality levels. And then the kernel
would be unprotected prior to full initialization of the LSM, including
policy load.
It seems like one would want to be able to boot with lockdown=integrity
to protect the kernel initially, then switch over to allowing the LSM to
selectively override it.
^ permalink raw reply
* Re: [PATCH 80/87] net: hippi: remove memset after pci_alloc_consistent
From: Jes Sorensen @ 2019-06-27 20:06 UTC (permalink / raw)
To: Fuqian Huang; +Cc: David S. Miller, netdev, linux-kernel
In-Reply-To: <20190627174452.5677-1-huangfq.daxian@gmail.com>
On 6/27/19 1:44 PM, Fuqian Huang wrote:
> pci_alloc_consistent calls dma_alloc_coherent directly.
> In commit af7ddd8a627c
> ("Merge tag 'dma-mapping-4.21' of git://git.infradead.org/users/hch/dma-mapping"),
> dma_alloc_coherent has already zeroed the memory.
> So memset is not needed.
>
> Signed-off-by: Fuqian Huang <huangfq.daxian@gmail.com>
> ---
> drivers/net/hippi/rrunner.c | 2 --
> 1 file changed, 2 deletions(-)
Acked-by: Jes Sorensen <Jes.Sorensen@gmail.com>
> diff --git a/drivers/net/hippi/rrunner.c b/drivers/net/hippi/rrunner.c
> index 7b9350dbebdd..2a6ec5394966 100644
> --- a/drivers/net/hippi/rrunner.c
> +++ b/drivers/net/hippi/rrunner.c
> @@ -1196,7 +1196,6 @@ static int rr_open(struct net_device *dev)
> goto error;
> }
> rrpriv->rx_ctrl_dma = dma_addr;
> - memset(rrpriv->rx_ctrl, 0, 256*sizeof(struct ring_ctrl));
>
> rrpriv->info = pci_alloc_consistent(pdev, sizeof(struct rr_info),
> &dma_addr);
> @@ -1205,7 +1204,6 @@ static int rr_open(struct net_device *dev)
> goto error;
> }
> rrpriv->info_dma = dma_addr;
> - memset(rrpriv->info, 0, sizeof(struct rr_info));
> wmb();
>
> spin_lock_irqsave(&rrpriv->lock, flags);
>
^ permalink raw reply
* Re: [PATCH net-next v2 1/4] net/sched: Introduce action ct
From: David Miller @ 2019-06-27 19:53 UTC (permalink / raw)
To: paulb
Cc: jiri, roid, yossiku, ozsh, marcelo.leitner, netdev, aconole,
wangzhike, ronye, nst-kernel, john.hurley, simon.horman, jpettit
In-Reply-To: <1561038141-31370-2-git-send-email-paulb@mellanox.com>
From: Paul Blakey <paulb@mellanox.com>
Date: Thu, 20 Jun 2019 16:42:18 +0300
> +struct tcf_ct_params {
...
> + struct rcu_head rcu;
> +
> +};
Please get ride of that empty line after the 'rcu' member.
> + switch (skb->protocol) {
> + case htons(ETH_P_IP):
> + family = NFPROTO_IPV4;
> + break;
> + case htons(ETH_P_IPV6):
> + family = NFPROTO_IPV6;
> + break;
> + default:
> + break;
Break statement is not indented properly.
> +static __net_init int ct_init_net(struct net *net)
> +{
> + struct tc_ct_action_net *tn = net_generic(net, ct_net_id);
> + unsigned int n_bits = FIELD_SIZEOF(struct tcf_ct_params, labels) * 8;
Reverse christmas tree here please.
^ permalink raw reply
* Re: [PATCH v4 net-next 1/4] net: core: page_pool: add user cnt preventing pool deletion
From: Jesper Dangaard Brouer @ 2019-06-27 19:44 UTC (permalink / raw)
To: Ivan Khoronzhuk
Cc: davem, grygorii.strashko, saeedm, leon, ast, linux-kernel,
linux-omap, ilias.apalodimas, netdev, daniel, jakub.kicinski,
john.fastabend, brouer
In-Reply-To: <20190625175948.24771-2-ivan.khoronzhuk@linaro.org>
On Tue, 25 Jun 2019 20:59:45 +0300
Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org> wrote:
> Add user counter allowing to delete pool only when no users.
> It doesn't prevent pool from flush, only prevents freeing the
> pool instance. Helps when no need to delete the pool and now
> it's user responsibility to free it by calling page_pool_free()
> while destroying procedure. It also makes to use page_pool_free()
> explicitly, not fully hidden in xdp unreg, which looks more
> correct after page pool "create" routine.
I don't think that "create" and "free" routines paring looks "more
correct" together.
Maybe we can scale back your solution(?), via creating a page_pool_get()
and page_pool_put() API that can be used by your driver, to keep the
page_pool object after a xdp_rxq_info_unreg() call. Then you can use
it for two xdp_rxq_info structs, and call page_pool_put() after you
have unregistered both.
The API would basically be:
diff --git a/net/core/page_pool.c b/net/core/page_pool.c
index b366f59885c1..691ddacfb5a6 100644
--- a/net/core/page_pool.c
+++ b/net/core/page_pool.c
@@ -357,6 +357,10 @@ static void __warn_in_flight(struct page_pool *pool)
void __page_pool_free(struct page_pool *pool)
{
WARN(pool->alloc.count, "API usage violation");
+
+ if (atomic_read(&pool->user_cnt) != 0)
+ return;
+
WARN(!ptr_ring_empty(&pool->ring), "ptr_ring is not empty");
/* Can happen due to forced shutdown */
@@ -372,6 +376,19 @@ void __page_pool_free(struct page_pool *pool)
}
EXPORT_SYMBOL(__page_pool_free);
+void page_pool_put(struct page_pool *pool)
+{
+ if (!atomic_dec_and_test(&pool->user_cnt))
+ __page_pool_free(pool);
+}
+EXPORT_SYMBOL(page_pool_put);
+
+void page_pool_get(struct page_pool *pool)
+{
+ atomic_inc(&pool->user_cnt);
+}
+EXPORT_SYMBOL(page_pool_get);
+
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply related
* Re: [pull request][for-next V2 0/7] Generic DIM lib for netdev and RDMA
From: David Miller @ 2019-06-27 19:43 UTC (permalink / raw)
To: saeedm; +Cc: dledford, jgg, leonro, ogerlitz, sagi, talgi, netdev, linux-rdma
In-Reply-To: <20190625205701.17849-1-saeedm@mellanox.com>
From: Saeed Mahameed <saeedm@mellanox.com>
Date: Tue, 25 Jun 2019 20:57:27 +0000
> Once we are all happy with the series, please pull to net-next and
> rdma-next trees.
Pulled into net-next, thanks.
I'll push it back out once I am done build testing.
^ permalink raw reply
* [PATCHv2 next 3/3] blackhole_dev: add a selftest
From: Mahesh Bandewar @ 2019-06-27 19:43 UTC (permalink / raw)
To: Netdev
Cc: Eric Dumazet, David Miller, Michael Chan, Daniel Axtens,
Mahesh Bandewar, Mahesh Bandewar
Since this is not really a device with all capabilities, this test
ensures that it has *enough* to make it through the data path
without causing unwanted side-effects (read crash!).
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
v1 -> v2
fixed the conflict resolution in selftests Makefile
lib/Kconfig.debug | 9 ++
lib/Makefile | 1 +
lib/test_blackhole_dev.c | 100 ++++++++++++++++++
tools/testing/selftests/net/Makefile | 2 +-
tools/testing/selftests/net/config | 1 +
.../selftests/net/test_blackhole_dev.sh | 11 ++
6 files changed, 123 insertions(+), 1 deletion(-)
create mode 100644 lib/test_blackhole_dev.c
create mode 100755 tools/testing/selftests/net/test_blackhole_dev.sh
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index cbdfae379896..79d96e1614f5 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1909,6 +1909,15 @@ config TEST_BPF
If unsure, say N.
+config TEST_BLACKHOLE_DEV
+ tristate "Test BPF filter functionality"
+ depends on m && NET
+ help
+ This builds the "test_blackhole_dev" module that validates the
+ data path through this blackhole netdev.
+
+ If unsure, say N.
+
config FIND_BIT_BENCHMARK
tristate "Test find_bit functions"
help
diff --git a/lib/Makefile b/lib/Makefile
index fb7697031a79..c293624d3664 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -91,6 +91,7 @@ obj-$(CONFIG_TEST_DEBUG_VIRTUAL) += test_debug_virtual.o
obj-$(CONFIG_TEST_MEMCAT_P) += test_memcat_p.o
obj-$(CONFIG_TEST_OBJAGG) += test_objagg.o
obj-$(CONFIG_TEST_STACKINIT) += test_stackinit.o
+obj-$(CONFIG_TEST_BLACKHOLE_DEV) += test_blackhole_dev.o
obj-$(CONFIG_TEST_LIVEPATCH) += livepatch/
diff --git a/lib/test_blackhole_dev.c b/lib/test_blackhole_dev.c
new file mode 100644
index 000000000000..4c40580a99a3
--- /dev/null
+++ b/lib/test_blackhole_dev.c
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * This module tests the blackhole_dev that is created during the
+ * net subsystem initialization. The test this module performs is
+ * by injecting an skb into the stack with skb->dev as the
+ * blackhole_dev and expects kernel to behave in a sane manner
+ * (in other words, *not crash*)!
+ *
+ * Copyright (c) 2018, Mahesh Bandewar <maheshb@google.com>
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <linux/printk.h>
+#include <linux/skbuff.h>
+#include <linux/netdevice.h>
+#include <linux/udp.h>
+#include <linux/ipv6.h>
+
+#include <net/dst.h>
+
+#define SKB_SIZE 256
+#define HEAD_SIZE (14+40+8) /* Ether + IPv6 + UDP */
+#define TAIL_SIZE 32 /* random tail-room */
+
+#define UDP_PORT 1234
+
+static int __init test_blackholedev_init(void)
+{
+ struct ipv6hdr *ip6h;
+ struct sk_buff *skb;
+ struct ethhdr *ethh;
+ struct udphdr *uh;
+ int data_len;
+ int ret;
+
+ skb = alloc_skb(SKB_SIZE, GFP_KERNEL);
+ if (!skb)
+ return -ENOMEM;
+
+ /* Reserve head-room for the headers */
+ skb_reserve(skb, HEAD_SIZE);
+
+ /* Add data to the skb */
+ data_len = SKB_SIZE - (HEAD_SIZE + TAIL_SIZE);
+ memset(__skb_put(skb, data_len), 0xf, data_len);
+
+ /* Add protocol data */
+ /* (Transport) UDP */
+ uh = (struct udphdr *)skb_push(skb, sizeof(struct udphdr));
+ skb_set_transport_header(skb, 0);
+ uh->source = uh->dest = htons(UDP_PORT);
+ uh->len = htons(data_len);
+ uh->check = 0;
+ /* (Network) IPv6 */
+ ip6h = (struct ipv6hdr *)skb_push(skb, sizeof(struct ipv6hdr));
+ skb_set_network_header(skb, 0);
+ ip6h->hop_limit = 32;
+ ip6h->payload_len = data_len + sizeof(struct udphdr);
+ ip6h->nexthdr = IPPROTO_UDP;
+ ip6h->saddr = in6addr_loopback;
+ ip6h->daddr = in6addr_loopback;
+ /* Ether */
+ ethh = (struct ethhdr *)skb_push(skb, sizeof(struct ethhdr));
+ skb_set_mac_header(skb, 0);
+
+ skb->protocol = htons(ETH_P_IPV6);
+ skb->pkt_type = PACKET_HOST;
+ skb->dev = blackhole_netdev;
+
+ /* Now attempt to send the packet */
+ ret = dev_queue_xmit(skb);
+
+ switch (ret) {
+ case NET_XMIT_SUCCESS:
+ pr_warn("dev_queue_xmit() returned NET_XMIT_SUCCESS\n");
+ break;
+ case NET_XMIT_DROP:
+ pr_warn("dev_queue_xmit() returned NET_XMIT_DROP\n");
+ break;
+ case NET_XMIT_CN:
+ pr_warn("dev_queue_xmit() returned NET_XMIT_CN\n");
+ break;
+ default:
+ pr_err("dev_queue_xmit() returned UNKNOWN(%d)\n", ret);
+ }
+
+ return 0;
+}
+
+static void __exit test_blackholedev_exit(void)
+{
+ pr_warn("test_blackholedev module terminating.\n");
+}
+
+module_init(test_blackholedev_init);
+module_exit(test_blackholedev_exit);
+
+MODULE_AUTHOR("Mahesh Bandewar <maheshb@google.com>");
+MODULE_LICENSE("GPL");
diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 9a275d932fd5..1b24e36b4047 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -5,7 +5,7 @@ CFLAGS = -Wall -Wl,--no-as-needed -O2 -g
CFLAGS += -I../../../../usr/include/
TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh \
- rtnetlink.sh xfrm_policy.sh
+ rtnetlink.sh xfrm_policy.sh test_blackhole_dev.sh
TEST_PROGS += fib_tests.sh fib-onlink-tests.sh pmtu.sh udpgso.sh ip_defrag.sh
TEST_PROGS += udpgso_bench.sh fib_rule_tests.sh msg_zerocopy.sh psock_snd.sh
TEST_PROGS += udpgro_bench.sh udpgro.sh test_vxlan_under_vrf.sh reuseport_addr_any.sh
diff --git a/tools/testing/selftests/net/config b/tools/testing/selftests/net/config
index 89f84b5118bf..e4b878d95ba0 100644
--- a/tools/testing/selftests/net/config
+++ b/tools/testing/selftests/net/config
@@ -27,3 +27,4 @@ CONFIG_NFT_CHAIN_NAT_IPV6=m
CONFIG_NFT_CHAIN_NAT_IPV4=m
CONFIG_NET_SCH_FQ=m
CONFIG_NET_SCH_ETF=m
+CONFIG_TEST_BLACKHOLE_DEV=m
diff --git a/tools/testing/selftests/net/test_blackhole_dev.sh b/tools/testing/selftests/net/test_blackhole_dev.sh
new file mode 100755
index 000000000000..3119b80e711f
--- /dev/null
+++ b/tools/testing/selftests/net/test_blackhole_dev.sh
@@ -0,0 +1,11 @@
+#!/bin/sh
+# SPDX-License-Identifier: GPL-2.0
+# Runs blackhole-dev test using blackhole-dev kernel module
+
+if /sbin/modprobe -q test_blackhole_dev ; then
+ /sbin/modprobe -q -r test_blackhole_dev;
+ echo "test_blackhole_dev: ok";
+else
+ echo "test_blackhole_dev: [FAIL]";
+ exit 1;
+fi
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCHv2 next 2/3] blackhole_netdev: use blackhole_netdev to invalidate dst entries
From: Mahesh Bandewar @ 2019-06-27 19:43 UTC (permalink / raw)
To: Netdev
Cc: Eric Dumazet, David Miller, Michael Chan, Daniel Axtens,
Mahesh Bandewar, Mahesh Bandewar
Use blackhole_netdev instead of 'lo' device with lower MTU when marking
dst "dead".
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
v1 -> v2
no change
net/core/dst.c | 2 +-
net/ipv4/route.c | 3 +--
net/ipv6/route.c | 2 +-
3 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/net/core/dst.c b/net/core/dst.c
index e46366228eaf..1325316d9eab 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -160,7 +160,7 @@ void dst_dev_put(struct dst_entry *dst)
dst->ops->ifdown(dst, dev, true);
dst->input = dst_discard;
dst->output = dst_discard_out;
- dst->dev = dev_net(dst->dev)->loopback_dev;
+ dst->dev = blackhole_netdev;
dev_hold(dst->dev);
dev_put(dev);
}
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 59670fafcd26..d61f43feb7fa 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1532,7 +1532,6 @@ static void ipv4_dst_destroy(struct dst_entry *dst)
void rt_flush_dev(struct net_device *dev)
{
- struct net *net = dev_net(dev);
struct rtable *rt;
int cpu;
@@ -1543,7 +1542,7 @@ void rt_flush_dev(struct net_device *dev)
list_for_each_entry(rt, &ul->head, rt_uncached) {
if (rt->dst.dev != dev)
continue;
- rt->dst.dev = net->loopback_dev;
+ rt->dst.dev = blackhole_netdev;
dev_hold(rt->dst.dev);
dev_put(dev);
}
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index e7c2824435c6..ff44c6287633 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -176,7 +176,7 @@ static void rt6_uncached_list_flush_dev(struct net *net, struct net_device *dev)
}
if (rt_dev == dev) {
- rt->dst.dev = loopback_dev;
+ rt->dst.dev = blackhole_netdev;
dev_hold(rt->dst.dev);
dev_put(rt_dev);
}
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCHv2 next 1/3] loopback: create blackhole net device similar to loopack.
From: Mahesh Bandewar @ 2019-06-27 19:43 UTC (permalink / raw)
To: Netdev
Cc: Eric Dumazet, David Miller, Michael Chan, Daniel Axtens,
Mahesh Bandewar, Mahesh Bandewar
Create a blackhole net device that can be used for "dead"
dst entries instead of loopback device. This blackhole device differs
from loopback in few aspects: (a) It's not per-ns. (b) MTU on this
device is ETH_MIN_MTU (c) The xmit function is essentially kfree_skb().
and (d) since it's not registered it won't have ifindex.
Lower MTU effectively make the device not pass the MTU check during
the route check when a dst associated with the skb is dead.
Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
v1->v2
no change
drivers/net/loopback.c | 76 ++++++++++++++++++++++++++++++++++-----
include/linux/netdevice.h | 2 ++
2 files changed, 69 insertions(+), 9 deletions(-)
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 87d361666cdd..3b39def5471e 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -55,6 +55,13 @@
#include <net/net_namespace.h>
#include <linux/u64_stats_sync.h>
+/* blackhole_netdev - a device used for dsts that are marked expired!
+ * This is global device (instead of per-net-ns) since it's not needed
+ * to be per-ns and gets initialized at boot time.
+ */
+struct net_device *blackhole_netdev;
+EXPORT_SYMBOL(blackhole_netdev);
+
/* The higher levels take care of making this non-reentrant (it's
* called with bh's disabled).
*/
@@ -150,12 +157,14 @@ static const struct net_device_ops loopback_ops = {
.ndo_set_mac_address = eth_mac_addr,
};
-/* The loopback device is special. There is only one instance
- * per network namespace.
- */
-static void loopback_setup(struct net_device *dev)
+static void gen_lo_setup(struct net_device *dev,
+ unsigned int mtu,
+ const struct ethtool_ops *eth_ops,
+ const struct header_ops *hdr_ops,
+ const struct net_device_ops *dev_ops,
+ void (*dev_destructor)(struct net_device *dev))
{
- dev->mtu = 64 * 1024;
+ dev->mtu = mtu;
dev->hard_header_len = ETH_HLEN; /* 14 */
dev->min_header_len = ETH_HLEN; /* 14 */
dev->addr_len = ETH_ALEN; /* 6 */
@@ -174,11 +183,20 @@ static void loopback_setup(struct net_device *dev)
| NETIF_F_NETNS_LOCAL
| NETIF_F_VLAN_CHALLENGED
| NETIF_F_LOOPBACK;
- dev->ethtool_ops = &loopback_ethtool_ops;
- dev->header_ops = ð_header_ops;
- dev->netdev_ops = &loopback_ops;
+ dev->ethtool_ops = eth_ops;
+ dev->header_ops = hdr_ops;
+ dev->netdev_ops = dev_ops;
dev->needs_free_netdev = true;
- dev->priv_destructor = loopback_dev_free;
+ dev->priv_destructor = dev_destructor;
+}
+
+/* The loopback device is special. There is only one instance
+ * per network namespace.
+ */
+static void loopback_setup(struct net_device *dev)
+{
+ gen_lo_setup(dev, (64 * 1024), &loopback_ethtool_ops, ð_header_ops,
+ &loopback_ops, loopback_dev_free);
}
/* Setup and register the loopback device. */
@@ -213,3 +231,43 @@ static __net_init int loopback_net_init(struct net *net)
struct pernet_operations __net_initdata loopback_net_ops = {
.init = loopback_net_init,
};
+
+/* blackhole netdevice */
+static netdev_tx_t blackhole_netdev_xmit(struct sk_buff *skb,
+ struct net_device *dev)
+{
+ kfree_skb(skb);
+ net_warn_ratelimited("%s(): Dropping skb.\n", __func__);
+ return NETDEV_TX_OK;
+}
+
+static const struct net_device_ops blackhole_netdev_ops = {
+ .ndo_start_xmit = blackhole_netdev_xmit,
+};
+
+/* This is a dst-dummy device used specifically for invalidated
+ * DSTs and unlike loopback, this is not per-ns.
+ */
+static void blackhole_netdev_setup(struct net_device *dev)
+{
+ gen_lo_setup(dev, ETH_MIN_MTU, NULL, NULL, &blackhole_netdev_ops, NULL);
+}
+
+/* Setup and register the blackhole_netdev. */
+static int __init blackhole_netdev_init(void)
+{
+ blackhole_netdev = alloc_netdev(0, "blackhole_dev", NET_NAME_UNKNOWN,
+ blackhole_netdev_setup);
+ if (!blackhole_netdev)
+ return -ENOMEM;
+
+ dev_init_scheduler(blackhole_netdev);
+ dev_activate(blackhole_netdev);
+
+ blackhole_netdev->flags |= IFF_UP | IFF_RUNNING;
+ dev_net_set(blackhole_netdev, &init_net);
+
+ return 0;
+}
+
+device_initcall(blackhole_netdev_init);
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index eeacebd7debb..88292953aa6f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -4870,4 +4870,6 @@ do { \
#define PTYPE_HASH_SIZE (16)
#define PTYPE_HASH_MASK (PTYPE_HASH_SIZE - 1)
+extern struct net_device *blackhole_netdev;
+
#endif /* _LINUX_NETDEVICE_H */
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related
* [PATCHv2 next 0/3] blackhole device to invalidate dst
From: Mahesh Bandewar @ 2019-06-27 19:42 UTC (permalink / raw)
To: Netdev
Cc: Eric Dumazet, David Miller, Michael Chan, Daniel Axtens,
Mahesh Bandewar, Mahesh Bandewar
When we invalidate dst or mark it "dead", we assign 'lo' to
dst->dev. First of all this assignment is racy and more over,
it has MTU implications.
The standard dev MTU is 1500 while the Loopback MTU is 64k. TCP
code when dereferencing the dst don't check if the dst is valid
or not. TCP when dereferencing a dead-dst while negotiating a
new connection, may use dst device which is 'lo' instead of
using the correct device. Consider the following scenario:
A SYN arrives on an interface and tcp-layer while processing
SYNACK finds a dst and associates it with SYNACK skb. Now before
skb gets passed to L3 for processing, if that dst gets "dead"
(because of the virtual device getting disappeared & then reappeared),
the 'lo' gets assigned to that dst (lo MTU = 64k). Let's assume
the SYN has ADV_MSS set as 9k while the output device through
which this SYNACK is going to go out has standard MTU of 1500.
The MTU check during the route check passes since MIN(9K, 64K)
is 9k and TCP successfully negotiates 9k MSS. The subsequent
data packet; bigger in size gets passed to the device and it
won't be marked as GSO since the assumed MTU of the device is
9k.
This either crashes the NIC and we have seen fixes that went
into drivers to handle this scenario. 8914a595110a ('bnx2x:
disable GSO where gso_size is too big for hardware') and
2b16f048729b ('net: create skb_gso_validate_mac_len()') and
with those fixes TCP eventually recovers but not before
few dropped segments.
Well, I'm not a TCP expert and though we have experienced
these corner cases in our environment, I could not reproduce
this case reliably in my test setup to try this fix myself.
However, Michael Chan <michael.chan@broadcom.com> had a setup
where these fixes helped him mitigate the issue and not cause
the crash.
The idea here is to not alter the data-path with additional
locks or smb()/rmb() barriers to avoid racy assignments but
to create a new device that has really low MTU that has
.ndo_start_xmit essentially a kfree_skb(). Make use of this
device instead of 'lo' when marking the dst dead.
First patch implements the blackhole device and second
patch uses it in IPv4 and IPv6 stack while the third patch
is the self test that ensures the sanity of this device.
v1->v2
fixed the self-test patch to handle the conflict
Mahesh Bandewar (3):
loopback: create blackhole net device similar to loopack.
blackhole_netdev: use blackhole_netdev to invalidate dst entries
blackhole_dev: add a selftest
drivers/net/loopback.c | 76 +++++++++++--
include/linux/netdevice.h | 2 +
lib/Kconfig.debug | 9 ++
lib/Makefile | 1 +
lib/test_blackhole_dev.c | 100 ++++++++++++++++++
net/core/dst.c | 2 +-
net/ipv4/route.c | 3 +-
net/ipv6/route.c | 2 +-
tools/testing/selftests/net/Makefile | 2 +-
tools/testing/selftests/net/config | 1 +
.../selftests/net/test_blackhole_dev.sh | 11 ++
11 files changed, 195 insertions(+), 14 deletions(-)
create mode 100644 lib/test_blackhole_dev.c
create mode 100755 tools/testing/selftests/net/test_blackhole_dev.sh
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply
* Re: [RFC] longer netdev names proposal
From: Dan Williams @ 2019-06-27 19:35 UTC (permalink / raw)
To: Stephen Hemminger, Michal Kubecek
Cc: netdev, Andrew Lunn, David Ahern, Jiri Pirko, davem,
jakub.kicinski, mlxsw
In-Reply-To: <20190627122041.18c46daf@hermes.lan>
On Thu, 2019-06-27 at 12:20 -0700, Stephen Hemminger wrote:
> On Thu, 27 Jun 2019 20:39:48 +0200
> Michal Kubecek <mkubecek@suse.cz> wrote:
>
> > > $ ip li set dev enp3s0 alias "Onboard Ethernet"
> > > # ip link show "Onboard Ethernet"
> > > Device "Onboard Ethernet" does not exist.
> > >
> > > So it does not really appear to be an alias, it is a label. To be
> > > truly useful, it needs to be more than a label, it needs to be a
> > > real
> > > alias which you can use.
> >
> > That's exactly what I meant: to be really useful, one should be
> > able to
> > use the alias(es) for setting device options, for adding routes, in
> > netfilter rules etc.
> >
> > Michal
>
> The kernel doesn't enforce uniqueness of alias.
Can we even enforce unique aliases/labels? Given that the kernel hasn't
enforced that in the past there's a good possibility of breaking stuff
if it started. (unfortunately)
Dan
> Also current kernel RTM_GETLINK doesn't do filter by alias (easily
> fixed).
>
> If it did, then handling it in iproute would be something like:
>
> diff --git a/lib/ll_map.c b/lib/ll_map.c
> index e0ed54bf77c9..c798ba542224 100644
> --- a/lib/ll_map.c
> +++ b/lib/ll_map.c
> @@ -26,15 +26,18 @@
> struct ll_cache {
> struct hlist_node idx_hash;
> struct hlist_node name_hash;
> + struct hlist_node alias_hash;
> unsigned flags;
> unsigned index;
> unsigned short type;
> - char name[];
> + char *alias;
> + char name[IFNAMSIZ];
> };
>
> #define IDXMAP_SIZE 1024
> static struct hlist_head idx_head[IDXMAP_SIZE];
> static struct hlist_head name_head[IDXMAP_SIZE];
> +static struct hlist_head alias_head[IDXMAP_SIZE];
>
> static struct ll_cache *ll_get_by_index(unsigned index)
> {
> @@ -77,10 +80,26 @@ static struct ll_cache *ll_get_by_name(const char
> *name)
> return NULL;
> }
>
> +static struct ll_cache *ll_get_by_alias(const char *alias)
> +{
> + struct hlist_node *n;
> + unsigned h = namehash(alias) & (IDXMAP_SIZE - 1);
> +
> + hlist_for_each(n, &alias_head[h]) {
> + struct ll_cache *im
> + = container_of(n, struct ll_cache, alias_hash);
> +
> + if (strcmp(im->alias, alias) == 0)
> + return im;
> + }
> +
> + return NULL;
> +}
> +
> int ll_remember_index(struct nlmsghdr *n, void *arg)
> {
> unsigned int h;
> - const char *ifname;
> + const char *ifname, *ifalias;
> struct ifinfomsg *ifi = NLMSG_DATA(n);
> struct ll_cache *im;
> struct rtattr *tb[IFLA_MAX+1];
> @@ -96,6 +115,10 @@ int ll_remember_index(struct nlmsghdr *n, void
> *arg)
> if (im) {
> hlist_del(&im->name_hash);
> hlist_del(&im->idx_hash);
> + if (im->alias) {
> + hlist_del(&im->alias_hash);
> + free(im->alias);
> + }
> free(im);
> }
> return 0;
> @@ -106,6 +129,8 @@ int ll_remember_index(struct nlmsghdr *n, void
> *arg)
> if (ifname == NULL)
> return 0;
>
> + ifalias = tb[IFLA_IFALIAS] ? rta_getattr_str(tb[IFLA_IFALIAS])
> : NULL;
> +
> if (im) {
> /* change to existing entry */
> if (strcmp(im->name, ifname) != 0) {
> @@ -114,6 +139,14 @@ int ll_remember_index(struct nlmsghdr *n, void
> *arg)
> hlist_add_head(&im->name_hash, &name_head[h]);
> }
>
> + if (im->alias) {
> + hlist_del(&im->alias_hash);
> + if (ifalias) {
> + h = namehash(ifalias) & (IDXMAP_SIZE -
> 1);
> + hlist_add_head(&im->alias_hash,
> &alias_head[h]);
> + }
> + }
> +
> im->flags = ifi->ifi_flags;
> return 0;
> }
> @@ -132,6 +165,12 @@ int ll_remember_index(struct nlmsghdr *n, void
> *arg)
> h = namehash(ifname) & (IDXMAP_SIZE - 1);
> hlist_add_head(&im->name_hash, &name_head[h]);
>
> + if (ifalias) {
> + im->alias = strdup(ifalias);
> + h = namehash(ifalias) & (IDXMAP_SIZE - 1);
> + hlist_add_head(&im->alias_hash, &alias_head[h]);
> + }
> +
> return 0;
> }
>
> @@ -152,7 +191,7 @@ static unsigned int ll_idx_a2n(const char *name)
> return idx;
> }
>
> -static int ll_link_get(const char *name, int index)
> +static int ll_link_get(const char *name, const char *alias, int
> index)
> {
> struct {
> struct nlmsghdr n;
> @@ -176,6 +215,9 @@ static int ll_link_get(const char *name, int
> index)
> if (name)
> addattr_l(&req.n, sizeof(req), IFLA_IFNAME, name,
> strlen(name) + 1);
> + if (alias)
> + addattr_l(&req.n, sizeof(req), IFLA_IFALIAS, alias,
> + strlen(alias) + 1);
>
> if (rtnl_talk_suppress_rtnl_errmsg(&rth, &req.n, &answer) < 0)
> goto out;
> @@ -206,7 +248,7 @@ const char *ll_index_to_name(unsigned int idx)
> if (im)
> return im->name;
>
> - if (ll_link_get(NULL, idx) == idx) {
> + if (ll_link_get(NULL, NULL, idx) == idx) {
> im = ll_get_by_index(idx);
> if (im)
> return im->name;
> @@ -252,7 +294,13 @@ unsigned ll_name_to_index(const char *name)
> if (im)
> return im->index;
>
> - idx = ll_link_get(name, 0);
> + im = ll_get_by_alias(name);
> + if (im)
> + return im->index;
> +
> + idx = ll_link_get(name, NULL, 0);
> + if (idx == 0)
> + idx = ll_link_get(NULL, name, 0);
> if (idx == 0)
> idx = if_nametoindex(name);
> if (idx == 0)
> @@ -270,7 +318,10 @@ void ll_drop_by_index(unsigned index)
>
> hlist_del(&im->idx_hash);
> hlist_del(&im->name_hash);
> -
> + if (im->alias) {
> + hlist_del(&im->alias_hash);
> + free(im->alias);
> + }
> free(im);
> }
>
>
^ permalink raw reply
* Re: [PATCH RFC net-next 1/5] net: dsa: mt7530: Convert to PHYLINK API
From: Andrew Lunn @ 2019-06-27 19:28 UTC (permalink / raw)
To: Daniel Santos
Cc: Russell King - ARM Linux admin, René van Dorst, sean.wang,
f.fainelli, davem, matthias.bgg, vivien.didelot, frank-w, netdev,
linux-mediatek, linux-mips
In-Reply-To: <e469daa1-3e28-db9c-e29a-7f68cc676fda@pobox.com>
> > Looking at the data sheet page, you want FORCE_MODE_Pn set. You never
> > want the MAC directly talking to the PHY. Bad things will happen.
>
> So what exactly do you mean by the MAC directly talking to the PHY? Do
> you mean setting speed, duplex, etc. via the MAC registers instead of
> via MDIO to the MII registers of the PHY?
The data sheet implies the MAC hardware performs reads on the PHY to
get the status, and then uses that to configure the MAC. This is often
called PHY polling. The MAC hardware however has no idea what the PHY
driver is doing. The MAC does not take the PHY mutex. So the PHY
driver might be doing something at the same time the MAC hardware
polls the PHY, and it gets odd results. Some PHYs have multiple pages,
and for example reading the temperature sensor means swapping
pages. If the MAC hardware was to poll while the sensor is being read,
it would not get the link status, it would read some random
temperature register.
So you want the PHY driver to read the results of auto-neg and it then
tell the MAC the results, so the MAC can be configured correctly.
> > Then use FORCE_RX_FC_Pn and FORCE_TX_Pn to reflect phydev->pause and
> > phydev->asym_pause.
> >
> > The same idea applies when using phylink.
> >
> > Andrew
>
> You're help is greatly appreciated here. Admittedly, I'm also trying to
> get this working in the now deprecated swconfig for a 3.18 kernel that's
> in production.
I'm not very familiar with swconfig. Is there software driving the
PHY? If not, it is then safe for the MAC hardware to poll the PHY.
Andrew
^ permalink raw reply
* Re: [RFC] longer netdev names proposal
From: Stephen Hemminger @ 2019-06-27 19:20 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, Andrew Lunn, David Ahern, Jiri Pirko, davem,
jakub.kicinski, mlxsw
In-Reply-To: <20190627183948.GK27240@unicorn.suse.cz>
On Thu, 27 Jun 2019 20:39:48 +0200
Michal Kubecek <mkubecek@suse.cz> wrote:
> >
> > $ ip li set dev enp3s0 alias "Onboard Ethernet"
> > # ip link show "Onboard Ethernet"
> > Device "Onboard Ethernet" does not exist.
> >
> > So it does not really appear to be an alias, it is a label. To be
> > truly useful, it needs to be more than a label, it needs to be a real
> > alias which you can use.
>
> That's exactly what I meant: to be really useful, one should be able to
> use the alias(es) for setting device options, for adding routes, in
> netfilter rules etc.
>
> Michal
The kernel doesn't enforce uniqueness of alias.
Also current kernel RTM_GETLINK doesn't do filter by alias (easily fixed).
If it did, then handling it in iproute would be something like:
diff --git a/lib/ll_map.c b/lib/ll_map.c
index e0ed54bf77c9..c798ba542224 100644
--- a/lib/ll_map.c
+++ b/lib/ll_map.c
@@ -26,15 +26,18 @@
struct ll_cache {
struct hlist_node idx_hash;
struct hlist_node name_hash;
+ struct hlist_node alias_hash;
unsigned flags;
unsigned index;
unsigned short type;
- char name[];
+ char *alias;
+ char name[IFNAMSIZ];
};
#define IDXMAP_SIZE 1024
static struct hlist_head idx_head[IDXMAP_SIZE];
static struct hlist_head name_head[IDXMAP_SIZE];
+static struct hlist_head alias_head[IDXMAP_SIZE];
static struct ll_cache *ll_get_by_index(unsigned index)
{
@@ -77,10 +80,26 @@ static struct ll_cache *ll_get_by_name(const char *name)
return NULL;
}
+static struct ll_cache *ll_get_by_alias(const char *alias)
+{
+ struct hlist_node *n;
+ unsigned h = namehash(alias) & (IDXMAP_SIZE - 1);
+
+ hlist_for_each(n, &alias_head[h]) {
+ struct ll_cache *im
+ = container_of(n, struct ll_cache, alias_hash);
+
+ if (strcmp(im->alias, alias) == 0)
+ return im;
+ }
+
+ return NULL;
+}
+
int ll_remember_index(struct nlmsghdr *n, void *arg)
{
unsigned int h;
- const char *ifname;
+ const char *ifname, *ifalias;
struct ifinfomsg *ifi = NLMSG_DATA(n);
struct ll_cache *im;
struct rtattr *tb[IFLA_MAX+1];
@@ -96,6 +115,10 @@ int ll_remember_index(struct nlmsghdr *n, void *arg)
if (im) {
hlist_del(&im->name_hash);
hlist_del(&im->idx_hash);
+ if (im->alias) {
+ hlist_del(&im->alias_hash);
+ free(im->alias);
+ }
free(im);
}
return 0;
@@ -106,6 +129,8 @@ int ll_remember_index(struct nlmsghdr *n, void *arg)
if (ifname == NULL)
return 0;
+ ifalias = tb[IFLA_IFALIAS] ? rta_getattr_str(tb[IFLA_IFALIAS]) : NULL;
+
if (im) {
/* change to existing entry */
if (strcmp(im->name, ifname) != 0) {
@@ -114,6 +139,14 @@ int ll_remember_index(struct nlmsghdr *n, void *arg)
hlist_add_head(&im->name_hash, &name_head[h]);
}
+ if (im->alias) {
+ hlist_del(&im->alias_hash);
+ if (ifalias) {
+ h = namehash(ifalias) & (IDXMAP_SIZE - 1);
+ hlist_add_head(&im->alias_hash, &alias_head[h]);
+ }
+ }
+
im->flags = ifi->ifi_flags;
return 0;
}
@@ -132,6 +165,12 @@ int ll_remember_index(struct nlmsghdr *n, void *arg)
h = namehash(ifname) & (IDXMAP_SIZE - 1);
hlist_add_head(&im->name_hash, &name_head[h]);
+ if (ifalias) {
+ im->alias = strdup(ifalias);
+ h = namehash(ifalias) & (IDXMAP_SIZE - 1);
+ hlist_add_head(&im->alias_hash, &alias_head[h]);
+ }
+
return 0;
}
@@ -152,7 +191,7 @@ static unsigned int ll_idx_a2n(const char *name)
return idx;
}
-static int ll_link_get(const char *name, int index)
+static int ll_link_get(const char *name, const char *alias, int index)
{
struct {
struct nlmsghdr n;
@@ -176,6 +215,9 @@ static int ll_link_get(const char *name, int index)
if (name)
addattr_l(&req.n, sizeof(req), IFLA_IFNAME, name,
strlen(name) + 1);
+ if (alias)
+ addattr_l(&req.n, sizeof(req), IFLA_IFALIAS, alias,
+ strlen(alias) + 1);
if (rtnl_talk_suppress_rtnl_errmsg(&rth, &req.n, &answer) < 0)
goto out;
@@ -206,7 +248,7 @@ const char *ll_index_to_name(unsigned int idx)
if (im)
return im->name;
- if (ll_link_get(NULL, idx) == idx) {
+ if (ll_link_get(NULL, NULL, idx) == idx) {
im = ll_get_by_index(idx);
if (im)
return im->name;
@@ -252,7 +294,13 @@ unsigned ll_name_to_index(const char *name)
if (im)
return im->index;
- idx = ll_link_get(name, 0);
+ im = ll_get_by_alias(name);
+ if (im)
+ return im->index;
+
+ idx = ll_link_get(name, NULL, 0);
+ if (idx == 0)
+ idx = ll_link_get(NULL, name, 0);
if (idx == 0)
idx = if_nametoindex(name);
if (idx == 0)
@@ -270,7 +318,10 @@ void ll_drop_by_index(unsigned index)
hlist_del(&im->idx_hash);
hlist_del(&im->name_hash);
-
+ if (im->alias) {
+ hlist_del(&im->alias_hash);
+ free(im->alias);
+ }
free(im);
}
^ permalink raw reply related
* Re: [PATCH v4 00/13] net: Add generic and Allwinner YAML bindings
From: Rob Herring @ 2019-06-27 19:17 UTC (permalink / raw)
To: David Miller
Cc: Maxime Ripard, Mark Rutland, Frank Rowand, Chen-Yu Tsai,
Maxime Coquelin, Alexandre Torgue, netdev,
moderated list:ARM/FREESCALE IMX / MXC ARM ARCHITECTURE,
devicetree, linux-stm32, Maxime Chevallier, Antoine Tenart,
Andrew Lunn, Florian Fainelli, Heiner Kallweit
In-Reply-To: <20190627.102256.1839462093915893704.davem@davemloft.net>
On Thu, Jun 27, 2019 at 11:22 AM David Miller <davem@davemloft.net> wrote:
>
> From: Maxime Ripard <maxime.ripard@bootlin.com>
> Date: Thu, 27 Jun 2019 17:31:42 +0200
>
> > This is an attempt at getting the main generic DT bindings for the ethernet
> > (and related) devices, and convert some DT bindings for the Allwinner DTs
> > to YAML as well.
> >
> > This should provide some DT validation coverage.
>
> I don't think this should go via my tree as it's all DT stuff.
That's fine. I can take it. There's one conflict with commit
79b647a0c0d5 ("dt-bindings: net: document new usxgmii phy mode"), but
that's easy enough to handle. Any other changes to the binding docs
will need to go thru me this cycle.
Rob
^ 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