* Re: [RFC PATCH bpf-next v3 7/7] selftests/bpf: add test cases for queue and stack maps
From: Alexei Starovoitov @ 2018-09-18 23:32 UTC (permalink / raw)
To: Mauricio Vasquez B
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Yonghong Song
In-Reply-To: <153724638737.7866.4029986291571667976.stgit@kernel>
On Tue, Sep 18, 2018 at 06:53:07AM +0200, Mauricio Vasquez B wrote:
> Two types of tests are done:
> - test_maps: only userspace api.
> - test_progs: userspace api and ebpf helpers.
>
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
> ---
> kernel/bpf/helpers.c | 2
> tools/lib/bpf/bpf.c | 12 ++
> tools/lib/bpf/bpf.h | 1
> tools/testing/selftests/bpf/Makefile | 5 +
> tools/testing/selftests/bpf/bpf_helpers.h | 7 +
> tools/testing/selftests/bpf/test_maps.c | 130 ++++++++++++++++++++
> tools/testing/selftests/bpf/test_progs.c | 99 +++++++++++++++
> tools/testing/selftests/bpf/test_queue_map.c | 4 +
> tools/testing/selftests/bpf/test_queue_stack_map.h | 59 +++++++++
> tools/testing/selftests/bpf/test_stack_map.c | 4 +
> 10 files changed, 321 insertions(+), 2 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/test_queue_map.c
> create mode 100644 tools/testing/selftests/bpf/test_queue_stack_map.h
> create mode 100644 tools/testing/selftests/bpf/test_stack_map.c
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 5f364e6acaf1..1293cd5240e3 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -76,7 +76,7 @@ const struct bpf_func_proto bpf_map_delete_elem_proto = {
> .arg2_type = ARG_PTR_TO_MAP_KEY,
> };
>
> -BPF_CALL_4(bpf_map_push_elem, struct bpf_map *, map, void *, value, u32 size,
> +BPF_CALL_4(bpf_map_push_elem, struct bpf_map *, map, void *, value, u32, size,
> u64, flags)
part of earlier patch?
> {
> if (map->value_size != size)
> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> index 60aa4ca8b2c5..7056b2eb554d 100644
> --- a/tools/lib/bpf/bpf.c
> +++ b/tools/lib/bpf/bpf.c
> @@ -286,6 +286,18 @@ int bpf_map_lookup_elem(int fd, const void *key, void *value)
> return sys_bpf(BPF_MAP_LOOKUP_ELEM, &attr, sizeof(attr));
> }
>
> +int bpf_map_lookup_and_delete_elem(int fd, const void *key, const void *value)
> +{
> + union bpf_attr attr;
> +
> + bzero(&attr, sizeof(attr));
> + attr.map_fd = fd;
> + attr.key = ptr_to_u64(key);
> + attr.value = ptr_to_u64(value);
> +
> + return sys_bpf(BPF_MAP_LOOKUP_AND_DELETE_ELEM, &attr, sizeof(attr));
> +}
> +
> int bpf_map_delete_elem(int fd, const void *key)
> {
> union bpf_attr attr;
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 6f38164b2618..6134ed9517d3 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -86,6 +86,7 @@ int bpf_map_update_elem(int fd, const void *key, const void *value,
> __u64 flags);
>
> int bpf_map_lookup_elem(int fd, const void *key, void *value);
> +int bpf_map_lookup_and_delete_elem(int fd, const void *key, const void *value);
> int bpf_map_delete_elem(int fd, const void *key);
> int bpf_map_get_next_key(int fd, const void *key, void *next_key);
> int bpf_obj_pin(int fd, const char *pathname);
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index fff7fb1285fc..ad8a2b8fb738 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -35,7 +35,7 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o test_tcp_estats.o test
> test_get_stack_rawtp.o test_sockmap_kern.o test_sockhash_kern.o \
> test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o test_lirc_mode2_kern.o \
> get_cgroup_id_kern.o socket_cookie_prog.o test_select_reuseport_kern.o \
> - test_skb_cgroup_id_kern.o
> + test_skb_cgroup_id_kern.o test_queue_map.o test_stack_map.o
>
> # Order correspond to 'make run_tests' order
> TEST_PROGS := test_kmod.sh \
> @@ -110,6 +110,9 @@ CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> $(OUTPUT)/test_l4lb_noinline.o: CLANG_FLAGS += -fno-inline
> $(OUTPUT)/test_xdp_noinline.o: CLANG_FLAGS += -fno-inline
>
> +$(OUTPUT)/test_queue_map.o: test_queue_stack_map.h
> +$(OUTPUT)/test_stack_map.o: test_queue_stack_map.h
> +
> BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris)
> BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF)
> BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i 'usage.*llvm')
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index e4be7730222d..bdbe8f84023e 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -16,6 +16,13 @@ static int (*bpf_map_update_elem)(void *map, void *key, void *value,
> (void *) BPF_FUNC_map_update_elem;
> static int (*bpf_map_delete_elem)(void *map, void *key) =
> (void *) BPF_FUNC_map_delete_elem;
> +static int (*bpf_map_push_elem)(void *map, const void *value, int len,
> + unsigned long long flags) =
> + (void *) BPF_FUNC_map_push_elem;
> +static int (*bpf_map_pop_elem)(void *map, void *value, int len) =
> + (void *) BPF_FUNC_map_pop_elem;
> +static int (*bpf_map_peek_elem)(void *map, void *value, int len) =
> + (void *) BPF_FUNC_map_peek_elem;
> static int (*bpf_probe_read)(void *dst, int size, void *unsafe_ptr) =
> (void *) BPF_FUNC_probe_read;
> static unsigned long long (*bpf_ktime_get_ns)(void) =
> diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c
> index 6f54f84144a0..abb21ea731fa 100644
> --- a/tools/testing/selftests/bpf/test_maps.c
> +++ b/tools/testing/selftests/bpf/test_maps.c
> @@ -15,6 +15,7 @@
> #include <string.h>
> #include <assert.h>
> #include <stdlib.h>
> +#include <time.h>
>
> #include <sys/wait.h>
> #include <sys/socket.h>
> @@ -471,6 +472,130 @@ static void test_devmap(int task, void *data)
> close(fd);
> }
>
> +static void test_queuemap(int task, void *data)
> +{
> + const int MAP_SIZE = 32;
> + __u32 vals[MAP_SIZE + MAP_SIZE/2], val;
> + int fd, i;
> +
> + /* Fill test values to be used */
> + for (i = 0; i < MAP_SIZE + MAP_SIZE/2; i++)
> + vals[i] = rand();
> +
> + /* Invalid key size */
> + fd = bpf_create_map(BPF_MAP_TYPE_QUEUE, 4, sizeof(val), MAP_SIZE,
> + map_flags);
> + assert(fd < 0 && errno == EINVAL);
> +
> + /* Invalid value size */
> + fd = bpf_create_map(BPF_MAP_TYPE_QUEUE, 0, 3, MAP_SIZE,map_flags);
> + assert(fd < 0 && errno == EINVAL);
> +
> + fd = bpf_create_map(BPF_MAP_TYPE_QUEUE, 0, sizeof(val), MAP_SIZE,
> + map_flags);
> + /* Queue map does not support BPF_F_NO_PREALLOC */
> + if (map_flags & BPF_F_NO_PREALLOC) {
> + assert(fd < 0 && errno == EINVAL);
> + return;
> + }
> + if (fd < 0) {
> + printf("Failed to create queuemap '%s'!\n", strerror(errno));
> + exit(1);
> + }
> +
> + /* Push MAP_SIZE elements */
> + for (i = 0; i < MAP_SIZE; i++)
> + assert(bpf_map_update_elem(fd, NULL, &vals[i], 0) == 0);
> +
> + /* Check that element cannot be pushed due to max_entries limit */
> + assert(bpf_map_update_elem(fd, NULL, &val, 0) == -1 &&
> + errno == E2BIG);
> +
> + /* Peek element */
> + assert(bpf_map_lookup_elem(fd, NULL, &val) == 0 && val == vals[0]);
> +
> + /* Replace half elements */
> + for (i = MAP_SIZE; i < MAP_SIZE + MAP_SIZE/2; i++)
> + assert(bpf_map_update_elem(fd, NULL, &vals[i], BPF_EXIST) == 0);
> +
> + /* Pop all elements */
> + for (i = MAP_SIZE/2; i < MAP_SIZE + MAP_SIZE/2; i++)
> + assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == 0 &&
> + val == vals[i]);
> +
> + /* Check that there are not elements left */
> + assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == -1 &&
> + errno == ENOENT);
> +
> + /* Check that non supported functions set errno to EINVAL */
> + assert(bpf_map_delete_elem(fd, NULL) == -1 && errno == EINVAL);
> + assert(bpf_map_get_next_key(fd, NULL, NULL) == -1 && errno == EINVAL);
> +
> + close(fd);
> +}
> +
> +static void test_stackmap(int task, void *data)
> +{
> + const int MAP_SIZE = 32;
> + __u32 vals[MAP_SIZE + MAP_SIZE/2], val;
> + int fd, i;
> +
> + /* Fill test values to be used */
> + for (i = 0; i < MAP_SIZE + MAP_SIZE/2; i++)
> + vals[i] = rand();
> +
> + /* Invalid key size */
> + fd = bpf_create_map(BPF_MAP_TYPE_STACK, 4, sizeof(val), MAP_SIZE,
> + map_flags);
> + assert(fd < 0 && errno == EINVAL);
> +
> + /* Invalid value size */
> + fd = bpf_create_map(BPF_MAP_TYPE_STACK, 0, 3, MAP_SIZE,map_flags);
> + assert(fd < 0 && errno == EINVAL);
> +
> + fd = bpf_create_map(BPF_MAP_TYPE_STACK, 0, sizeof(val), MAP_SIZE,
> + map_flags);
> + /* Stack map does not support BPF_F_NO_PREALLOC */
> + if (map_flags & BPF_F_NO_PREALLOC) {
> + assert(fd < 0 && errno == EINVAL);
> + return;
> + }
> + if (fd < 0) {
> + printf("Failed to create stackmap '%s'!\n", strerror(errno));
> + exit(1);
> + }
> +
> + /* Push MAP_SIZE elements */
> + for (i = 0; i < MAP_SIZE; i++)
> + assert(bpf_map_update_elem(fd, NULL, &vals[i], 0) == 0);
> +
> + /* Check that element cannot be pushed due to max_entries limit */
> + assert(bpf_map_update_elem(fd, NULL, &val, 0) == -1 &&
> + errno == E2BIG);
> +
> + /* Peek element */
> + assert(bpf_map_lookup_elem(fd, NULL, &val) == 0 && val == vals[i - 1]);
> +
> + /* Replace half elements */
> + for (i = MAP_SIZE; i < MAP_SIZE + MAP_SIZE/2; i++)
> + assert(bpf_map_update_elem(fd, NULL, &vals[i], BPF_EXIST) == 0);
> +
> + /* Pop all elements */
> + for (i = MAP_SIZE + MAP_SIZE/2 - 1; i >= MAP_SIZE/2; i--)
> + assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == 0 &&
> + val == vals[i]);
> +
> + /* Check that there are not elements left */
> + assert(bpf_map_lookup_and_delete_elem(fd, NULL, &val) == -1 &&
> + errno == ENOENT);
> +
> + /* Check that non supported functions set errno to EINVAL */
> + assert(bpf_map_delete_elem(fd, NULL) == -1 && errno == EINVAL);
> + assert(bpf_map_get_next_key(fd, NULL, NULL) == -1 && errno == EINVAL);
> +
> + close(fd);
> +}
> +
> #include <sys/socket.h>
> #include <sys/ioctl.h>
> #include <arpa/inet.h>
> @@ -1430,10 +1555,15 @@ static void run_all_tests(void)
> test_map_wronly();
>
> test_reuseport_array();
> +
> + test_queuemap(0, NULL);
> + test_stackmap(0, NULL);
> }
>
> int main(void)
> {
> + srand(time(NULL));
> +
> map_flags = 0;
> run_all_tests();
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index 63a671803ed6..a69b9e1e668d 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -1698,8 +1698,105 @@ static void test_task_fd_query_tp(void)
> "sys_enter_read");
> }
>
> +enum {
> + QUEUE,
> + STACK,
> +};
> +
> +static void test_queue_stack_map(int type)
> +{
> + const int MAP_SIZE = 32;
> + __u32 vals[MAP_SIZE], duration, retval, size, val;
> + int i, err, prog_fd, map_in_fd, map_out_fd;
> + char file[32], buf[128];
> + struct bpf_object *obj;
> + struct iphdr *iph = (void *)buf + sizeof(struct ethhdr);
> +
> + /* Fill test values to be used */
> + for (i = 0; i < MAP_SIZE; i++)
> + vals[i] = rand();
> +
> + if (type == QUEUE)
> + strncpy(file, "./test_queue_map.o", sizeof(file));
> + else if (type == STACK)
> + strncpy(file, "./test_stack_map.o", sizeof(file));
> + else
> + return;
> +
> + err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, &obj, &prog_fd);
> + if (err) {
> + error_cnt++;
> + return;
> + }
> +
> + map_in_fd = bpf_find_map(__func__, obj, "map_in");
> + if (map_in_fd < 0)
> + goto out;
> +
> + map_out_fd = bpf_find_map(__func__, obj, "map_out");
> + if (map_out_fd < 0)
> + goto out;
> +
> + /* Push 32 elements to the input map */
> + for (i = 0; i < MAP_SIZE; i++) {
> + err = bpf_map_update_elem(map_in_fd, NULL, &vals[i], 0);
> + if (err) {
> + error_cnt++;
> + goto out;
> + }
> + }
> +
> + /* The eBPF program pushes iph.saddr in the output map,
> + * pops the input map and saves this value in iph.daddr
> + */
> + for (i = 0; i < MAP_SIZE; i++) {
> + if (type == QUEUE) {
> + val = vals[i];
> + pkt_v4.iph.saddr = vals[i] * 5;
> + } else if (type == STACK) {
> + val = vals[MAP_SIZE - 1 - i];
> + pkt_v4.iph.saddr = vals[MAP_SIZE - 1 - i] * 5;
> + }
> +
> + err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
> + buf, &size, &retval, &duration);
> + if (err || retval || size != sizeof(pkt_v4) ||
> + iph->daddr != val)
> + break;
> + }
> +
> + CHECK(err || retval || size != sizeof(pkt_v4) || iph->daddr != val,
> + "bpf_map_pop_elem",
> + "err %d errno %d retval %d size %d iph->daddr %u\n",
> + err, errno, retval, size, iph->daddr);
> +
> + /* Queue is empty, program should return TC_ACT_SHOT */
> + err = bpf_prog_test_run(prog_fd, 1, &pkt_v4, sizeof(pkt_v4),
> + buf, &size, &retval, &duration);
> + CHECK(err || retval != 2 /* TC_ACT_SHOT */|| size != sizeof(pkt_v4),
> + "check-queue-stack-map-empty",
> + "err %d errno %d retval %d size %d\n",
> + err, errno, retval, size);
> +
> + /* Check that the program pushed elements correctly */
> + for (i = 0; i < MAP_SIZE; i++) {
> + err = bpf_map_lookup_and_delete_elem(map_out_fd, NULL, &val);
> + if (err || val != vals[i] * 5)
> + break;
> + }
> +
> + CHECK(i != MAP_SIZE && (err || val != vals[i] * 5),
> + "bpf_map_push_elem", "err %d value %u\n", err, val);
> +
> +out:
> + pkt_v4.iph.saddr = 0;
> + bpf_object__close(obj);
> +}
> +
> int main(void)
> {
> + srand(time(NULL));
> +
> jit_enabled = is_jit_enabled();
>
> test_pkt_access();
> @@ -1719,6 +1816,8 @@ int main(void)
> test_get_stack_raw_tp();
> test_task_fd_query_rawtp();
> test_task_fd_query_tp();
> + test_queue_stack_map(QUEUE);
> + test_queue_stack_map(STACK);
>
> printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
> return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
> diff --git a/tools/testing/selftests/bpf/test_queue_map.c b/tools/testing/selftests/bpf/test_queue_map.c
> new file mode 100644
> index 000000000000..3fdb3b9cb038
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_queue_map.c
> @@ -0,0 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (c) 2017 Politecnico di Torino
> +#define MAP_TYPE BPF_MAP_TYPE_QUEUE
> +#include "test_queue_stack_map.h"
> diff --git a/tools/testing/selftests/bpf/test_queue_stack_map.h b/tools/testing/selftests/bpf/test_queue_stack_map.h
> new file mode 100644
> index 000000000000..23a5b5d60069
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_queue_stack_map.h
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +// Copyright (c) 2017 Politecnico di Torino
> +#include <stddef.h>
> +#include <string.h>
> +#include <linux/bpf.h>
> +#include <linux/if_ether.h>
> +#include <linux/ip.h>
> +#include <linux/pkt_cls.h>
> +#include "bpf_helpers.h"
> +
> +int _version SEC("version") = 1;
> +
> +struct bpf_map_def __attribute__ ((section("maps"), used)) map_in = {
> + .type = MAP_TYPE,
> + .key_size = 0,
> + .value_size = sizeof(__u32),
> + .max_entries = 32,
> + .map_flags = 0,
> +};
> +
> +struct bpf_map_def __attribute__ ((section("maps"), used)) map_out = {
> + .type = MAP_TYPE,
> + .key_size = 0,
> + .value_size = sizeof(__u32),
> + .max_entries = 32,
> + .map_flags = 0,
> +};
> +
> +SEC("test")
> +int _test(struct __sk_buff *skb)
> +{
> + void *data_end = (void *)(long)skb->data_end;
> + void *data = (void *)(long)skb->data;
> + struct ethhdr *eth = (struct ethhdr *)(data);
> + __u32 value;
> + int err;
> +
> + if (eth + 1 > data_end)
> + return TC_ACT_SHOT;
> +
> + struct iphdr *iph = (struct iphdr *)(eth + 1);
> +
> + if (iph + 1 > data_end)
> + return TC_ACT_SHOT;
> +
> + err = bpf_map_pop_elem(&map_in, &value, sizeof(value));
> + if (err)
> + return TC_ACT_SHOT;
> +
> + iph->daddr = value;
> +
> + err = bpf_map_push_elem(&map_out, &iph->saddr, sizeof(iph->saddr), 0);
> + if (err)
> + return TC_ACT_SHOT;
is it possible to add a test for 'peek' helper in reliable way?
it seems it will alwasy be racy. may be we shouldn't implement peek at all?
^ permalink raw reply
* Re: [PATCH bpf-next] flow_dissector: fix build failure without CONFIG_NET
From: Y Song @ 2018-09-18 23:59 UTC (permalink / raw)
To: willemdebruijn.kernel
Cc: netdev, Alexei Starovoitov, Daniel Borkmann, rdunlap, willemb
In-Reply-To: <20180918202018.204099-1-willemdebruijn.kernel@gmail.com>
On Tue, Sep 18, 2018 at 1:20 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> From: Willem de Bruijn <willemb@google.com>
>
> If boolean CONFIG_BPF_SYSCALL is enabled, kernel/bpf/syscall.c will
> call flow_dissector functions from net/core/flow_dissector.c.
>
> This causes this build failure if CONFIG_NET is disabled:
>
> kernel/bpf/syscall.o: In function `__x64_sys_bpf':
> syscall.c:(.text+0x3278): undefined reference to
> `skb_flow_dissector_bpf_prog_attach'
> syscall.c:(.text+0x3310): undefined reference to
> `skb_flow_dissector_bpf_prog_detach'
> kernel/bpf/syscall.o:(.rodata+0x3f0): undefined reference to
> `flow_dissector_prog_ops'
> kernel/bpf/verifier.o:(.rodata+0x250): undefined reference to
> `flow_dissector_verifier_ops'
>
> Analogous to other optional BPF program types in syscall.c, add stubs
> if the relevant functions are not compiled and move the BPF_PROG_TYPE
> definition in the #ifdef CONFIG_NET block.
>
> Fixes: d58e468b1112 ("flow_dissector: implements flow dissector BPF hook")
> Reported-by: Randy Dunlap <rdunlap@infradead.org>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply
* [PATCH 0/1] macsec: reflect the master interface state
From: Radu Rendec @ 2018-09-19 0:16 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Sabrina Dubroca, Radu Rendec
Hi everyone,
I came across an issue with macsec interfaces where they don't reflect
the master interface state. On one hand you cannot set the macsec
interface link state to up unless the master interface is already up,
but on the other hand, if the master interface goes down while the
macsec interface is already up, the macsec interface stays up.
This may also be problematic if macsec interfaces are used as bridge
ports (probably not very usual, but that is my use case anyway). In that
case the bridge has no idea of the link state of the real device, since
it only sees the macsec interface, which is always up.
I'm proposing the attached patch, which is heavily inspired from the
vlan (802.1q) driver. However, in the case of macsec, there is no
concept of "loose binding" and the state is always reflected.
I tested the patch on x86_64, but I guess it's pretty much architecture
independent.
Any comments or suggestions are welcome. Thanks!
Radu Rendec (1):
macsec: reflect the master interface state
drivers/net/macsec.c | 57 +++++++++++++++++++++++++++++++++++---------
1 file changed, 46 insertions(+), 11 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH 1/1] macsec: reflect the master interface state
From: Radu Rendec @ 2018-09-19 0:16 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Sabrina Dubroca, Radu Rendec
In-Reply-To: <20180919001612.2636-1-radu.rendec@gmail.com>
This patch makes macsec interfaces reflect the state of the underlying
interface: if the master interface changes state to up/down, the macsec
interface changes its state accordingly.
This closes a loophole in the macsec interface state logic: the macsec
interface cannot be brought up if the master interface is down (the
operation is rejected with ENETDOWN); however, if the macsec interface
is brought up while the master interface is up and then the master
interface goes down, the macsec interface stays up.
Reflecting the master interface state can also be useful if the macsec
interface is used as a bridge port: if the master (physical) interface
goes down, the state propagates through the macsec interface to the
bridge module, which can react to the state change (for example if it
runs STP).
The patch does nothing original. The same logic is implemented for vlan
interfaces in vlan_device_event() in net/8021q/vlan.c. In fact, the code
was copied and adapted from there.
Signed-off-by: Radu Rendec <radu.rendec@gmail.com>
---
drivers/net/macsec.c | 57 +++++++++++++++++++++++++++++++++++---------
1 file changed, 46 insertions(+), 11 deletions(-)
diff --git a/drivers/net/macsec.c b/drivers/net/macsec.c
index 7de88b33d5b9..cb93a1290f85 100644
--- a/drivers/net/macsec.c
+++ b/drivers/net/macsec.c
@@ -3486,20 +3486,21 @@ static int macsec_notify(struct notifier_block *this, unsigned long event,
void *ptr)
{
struct net_device *real_dev = netdev_notifier_info_to_dev(ptr);
- LIST_HEAD(head);
+ struct macsec_dev *m;
+ struct macsec_rxh_data *rxd;
if (!is_macsec_master(real_dev))
return NOTIFY_DONE;
+ rxd = macsec_data_rtnl(real_dev);
+
switch (event) {
case NETDEV_UNREGISTER: {
- struct macsec_dev *m, *n;
- struct macsec_rxh_data *rxd;
+ struct macsec_dev *n;
+ LIST_HEAD(head);
- rxd = macsec_data_rtnl(real_dev);
- list_for_each_entry_safe(m, n, &rxd->secys, secys) {
+ list_for_each_entry_safe(m, n, &rxd->secys, secys)
macsec_common_dellink(m->secy.netdev, &head);
- }
netdev_rx_handler_unregister(real_dev);
kfree(rxd);
@@ -3507,11 +3508,7 @@ static int macsec_notify(struct notifier_block *this, unsigned long event,
unregister_netdevice_many(&head);
break;
}
- case NETDEV_CHANGEMTU: {
- struct macsec_dev *m;
- struct macsec_rxh_data *rxd;
-
- rxd = macsec_data_rtnl(real_dev);
+ case NETDEV_CHANGEMTU:
list_for_each_entry(m, &rxd->secys, secys) {
struct net_device *dev = m->secy.netdev;
unsigned int mtu = real_dev->mtu - (m->secy.icv_len +
@@ -3520,7 +3517,45 @@ static int macsec_notify(struct notifier_block *this, unsigned long event,
if (dev->mtu > mtu)
dev_set_mtu(dev, mtu);
}
+ break;
+ case NETDEV_CHANGE:
+ list_for_each_entry(m, &rxd->secys, secys) {
+ struct net_device *dev = m->secy.netdev;
+
+ netif_stacked_transfer_operstate(real_dev, dev);
+ }
+ break;
+ case NETDEV_DOWN: {
+ struct net_device *dev, *tmp;
+ LIST_HEAD(close_list);
+
+ list_for_each_entry(m, &rxd->secys, secys) {
+ dev = m->secy.netdev;
+
+ if (dev->flags & IFF_UP)
+ list_add(&dev->close_list, &close_list);
+ }
+
+ dev_close_many(&close_list, false);
+
+ list_for_each_entry_safe(dev, tmp, &close_list, close_list) {
+ netif_stacked_transfer_operstate(real_dev, dev);
+ list_del_init(&dev->close_list);
+ }
+ list_del(&close_list);
+ break;
}
+ case NETDEV_UP:
+ list_for_each_entry(m, &rxd->secys, secys) {
+ struct net_device *dev = m->secy.netdev;
+ int flags = dev_get_flags(dev);
+
+ if (!(flags & IFF_UP)) {
+ dev_change_flags(dev, flags | IFF_UP);
+ netif_stacked_transfer_operstate(real_dev, dev);
+ }
+ }
+ break;
}
return NOTIFY_OK;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next v5 04/20] zinc: ChaCha20 x86_64 implementation
From: Thomas Gleixner @ 2018-09-19 6:13 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson, Andy Polyakov, mingo, X86 ML,
Kate Stewart
In-Reply-To: <CAHmME9oARODJsrDbcNK7A22KrTRJP945hj9+oCJtQM6OjGF4Ag@mail.gmail.com>
Jason,
On Wed, 19 Sep 2018, Jason A. Donenfeld wrote:
> On Wed, Sep 19, 2018 at 12:30 AM Thomas Gleixner <tglx@linutronix.de> wrote:
> >
> > > +++ b/lib/zinc/chacha20/chacha20-x86_64-glue.h
> > > @@ -0,0 +1,100 @@
> > > +/* SPDX-License-Identifier: MIT
> >
> > Please put that into a separate one liner comment. Also this should be
> > 'GPL-2.0[+] or MIT' I think.
>
> I had that originally, but changed it to just MIT, since MIT is a
> subset of GPL-2.0. And looking at tree repo, it appears this is what
> others do too.
Subset? Not really. Both MIT and BSD3-Clause are GPL2.0 compatible
licenses. And if your intention is to have those files MIT/BSD only, yes
then the single license identifier is the right thing. If you want it dual
licensed then it should be expressed there clearly.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH net-next v5 12/20] zinc: BLAKE2s generic C implementation and selftest
From: Eric Biggers @ 2018-09-19 0:41 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: linux-kernel, netdev, linux-crypto, davem, gregkh, Samuel Neves,
Andy Lutomirski, Jean-Philippe Aumasson
In-Reply-To: <20180918161646.19105-13-Jason@zx2c4.com>
On Tue, Sep 18, 2018 at 06:16:38PM +0200, Jason A. Donenfeld wrote:
> The C implementation was originally based on Samuel Neves' public
> domain reference implementation but has since been heavily modified
> for the kernel. We're able to do compile-time optimizations by moving
> some scaffolding around the final function into the header file.
>
> Information: https://blake2.net/
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
> Signed-off-by: Samuel Neves <sneves@dei.uc.pt>
> Cc: Andy Lutomirski <luto@kernel.org>
> Cc: Greg KH <gregkh@linuxfoundation.org>
> Cc: Jean-Philippe Aumasson <jeanphilippe.aumasson@gmail.com>
> ---
> include/zinc/blake2s.h | 95 ++
> lib/zinc/Kconfig | 3 +
> lib/zinc/Makefile | 3 +
> lib/zinc/blake2s/blake2s.c | 301 +++++
> lib/zinc/selftest/blake2s.h | 2095 +++++++++++++++++++++++++++++++++++
> 5 files changed, 2497 insertions(+)
> create mode 100644 include/zinc/blake2s.h
> create mode 100644 lib/zinc/blake2s/blake2s.c
> create mode 100644 lib/zinc/selftest/blake2s.h
>
> diff --git a/include/zinc/blake2s.h b/include/zinc/blake2s.h
> new file mode 100644
> index 000000000000..951281596274
> --- /dev/null
> +++ b/include/zinc/blake2s.h
> @@ -0,0 +1,95 @@
> +/* SPDX-License-Identifier: MIT
> + *
> + * Copyright (C) 2015-2018 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
> + */
> +
> +#ifndef _ZINC_BLAKE2S_H
> +#define _ZINC_BLAKE2S_H
> +
> +#include <linux/types.h>
> +#include <linux/kernel.h>
> +#include <crypto/algapi.h>
> +
> +enum blake2s_lengths {
> + BLAKE2S_BLOCKBYTES = 64,
> + BLAKE2S_OUTBYTES = 32,
> + BLAKE2S_KEYBYTES = 32
> +};
> +
> +struct blake2s_state {
> + u32 h[8];
> + u32 t[2];
> + u32 f[2];
> + u8 buf[BLAKE2S_BLOCKBYTES];
> + size_t buflen;
> + u8 last_node;
> +};
> +
> +void blake2s_init(struct blake2s_state *state, const size_t outlen);
> +void blake2s_init_key(struct blake2s_state *state, const size_t outlen,
> + const void *key, const size_t keylen);
> +void blake2s_update(struct blake2s_state *state, const u8 *in, size_t inlen);
> +void __blake2s_final(struct blake2s_state *state);
> +static inline void blake2s_final(struct blake2s_state *state, u8 *out,
> + const size_t outlen)
> +{
> + int i;
> +
> +#ifdef DEBUG
> + BUG_ON(!out || !outlen || outlen > BLAKE2S_OUTBYTES);
> +#endif
> + __blake2s_final(state);
> +
> + if (__builtin_constant_p(outlen) && !(outlen % sizeof(u32))) {
> + if (IS_ENABLED(CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS) ||
> + IS_ALIGNED((unsigned long)out, __alignof__(u32))) {
> + __le32 *outwords = (__le32 *)out;
> +
> + for (i = 0; i < outlen / sizeof(u32); ++i)
> + outwords[i] = cpu_to_le32(state->h[i]);
> + } else {
> + __le32 buffer[BLAKE2S_OUTBYTES];
This buffer is 4 times too long.
> +
> + for (i = 0; i < outlen / sizeof(u32); ++i)
> + buffer[i] = cpu_to_le32(state->h[i]);
> + memcpy(out, buffer, outlen);
> + memzero_explicit(buffer, sizeof(buffer));
> + }
> + } else {
> + u8 buffer[BLAKE2S_OUTBYTES] __aligned(__alignof__(u32));
> + __le32 *outwords = (__le32 *)buffer;
> +
> + for (i = 0; i < 8; ++i)
> + outwords[i] = cpu_to_le32(state->h[i]);
> + memcpy(out, buffer, outlen);
> + memzero_explicit(buffer, sizeof(buffer));
> + }
> +
> + memzero_explicit(state, sizeof(*state));
> +}
Or how about something much simpler:
static inline void blake2s_final(struct blake2s_state *state, u8 *out,
const size_t outlen)
{
#ifdef DEBUG
BUG_ON(!out || !outlen || outlen > BLAKE2S_OUTBYTES);
#endif
__blake2s_final(state);
cpu_to_le32_array(state->h, ARRAY_SIZE(state->h));
memcpy(out, state->h, outlen);
memzero_explicit(state, sizeof(*state));
}
^ permalink raw reply
* Re: [PATCH net 2/2] ipv6: fix memory leak on dst->_metrics
From: Wei Wang @ 2018-09-19 1:02 UTC (permalink / raw)
To: dsahern
Cc: Wei Wang, David S . Miller, Linux Kernel Network Developers,
Eric Dumazet, Cong Wang, sd
In-Reply-To: <15a5dad7-544e-37d7-1a04-fb3e2fad599f@gmail.com>
On Tue, Sep 18, 2018 at 4:25 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 9/18/18 1:45 PM, Wei Wang wrote:
> > From: Wei Wang <weiwan@google.com>
> >
> > When dst->_metrics and f6i->fib6_metrics share the same memory, both
> > take reference count on the dst_metrics structure. However, when dst is
> > destroyed, ip6_dst_destroy() only invokes dst_destroy_metrics_generic()
> > which does not take care of READONLY metrics and does not release refcnt.
> > This causes memory leak.
> > Similar to ipv4 logic, the fix is to properly release refcnt and free
> > the memory space pointed by dst->_metrics if refcnt becomes 0.
> >
> > Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
> > Reported-by: Sabrina Dubroca <sd@queasysnail.net>
> > Signed-off-by: Wei Wang <weiwan@google.com>
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > ---
> > net/ipv6/route.c | 5 ++++-
> > 1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index b5d3e6b294ab..826b14de7dbb 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -364,11 +364,14 @@ EXPORT_SYMBOL(ip6_dst_alloc);
> >
> > static void ip6_dst_destroy(struct dst_entry *dst)
> > {
> > + struct dst_metrics *p = (struct dst_metrics *)DST_METRICS_PTR(dst);
> > struct rt6_info *rt = (struct rt6_info *)dst;
> > struct fib6_info *from;
> > struct inet6_dev *idev;
> >
> > - dst_destroy_metrics_generic(dst);
> > + if (p != &dst_default_metrics && refcount_dec_and_test(&p->refcnt))
> > + kfree(p);
> > +
> > rt6_uncached_list_del(rt);
> >
> > idev = rt->rt6i_idev;
> >
>
> Reviewed-by: David Ahern <dsahern@gmail.com>
>
> With the revert in patch 1 we are back to my original code after
> 93531c67431 ("net/ipv6: separate handling of FIB entries from dst based
> routes").
>
> My intention with that series was to make IPv6 handling of metrics as
> identical to IPv4 as possible (v6 does have differences for example due
> to autoconf and changing metrics after installing a route). The change
> in this patch is what I missed back in April.
>
> Comparing IPv4 and IPv6 code for memory allocation and freeing for FIB
> entries, transferring metrics to dst_entry and cleanup of dst_entry all
> look nearly identical - to the point that net-next could have common
> helpers to manage the refcnt'ing. I can submit those after this change
> hits net-next.
Yes. Agree. Since both v4 and v6 use the same logic on handling
metrics, helpers can be useful.
>
> Thanks for your time getting to the bottom of the leak.
^ permalink raw reply
* [PATCH RFT net-next 0/2] net: phy: Eliminate unnecessary soft
From: Florian Fainelli @ 2018-09-19 1:35 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, David S. Miller, Andrew Lunn, nbd, cphealy,
harini.katakam, afleming, agust, arnd, asmirnov, avi.kp.137,
avorontsov, baijiaju1990, benh, charles-antoine.couret,
clemens.gruber, colin.king, cyril, david.thomson, ddaney,
dongsheng.wang, dwmw2, eha, houjingj, jeff, Jingju.Hou,
Jisheng.Zhang, johan, Kapil.Juneja, kim.phillips, linyunsheng,
madalin.
Hi all,
This patch series eliminates unnecessary software resets of the PHY.
This should hopefully not break anybody's hardware; but I would
appreciate testing to make sure this is is the case.
Sorry for this long email list, I wanted to make sure I reached out to
all people who made changes to the Marvell PHY driver.
Thank you!
Florian Fainelli (2):
net: phy: Stop with excessive soft reset
net: phy: marvell: Avoid unnecessary soft reset
drivers/net/phy/marvell.c | 63 ++++++++++++------------------------
drivers/net/phy/phy_device.c | 2 --
2 files changed, 21 insertions(+), 44 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH RFT net-next 1/2] net: phy: Stop with excessive soft reset
From: Florian Fainelli @ 2018-09-19 1:35 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, David S. Miller, Andrew Lunn, nbd, cphealy,
harini.katakam, afleming, agust, arnd, asmirnov, avi.kp.137,
avorontsov, baijiaju1990, benh, charles-antoine.couret,
clemens.gruber, colin.king, cyril, david.thomson, ddaney,
dongsheng.wang, dwmw2, eha, houjingj, jeff, Jingju.Hou,
Jisheng.Zhang, johan, Kapil.Juneja, kim.phillips, linyunsheng,
madalin.
In-Reply-To: <20180919013505.11347-1-f.fainelli@gmail.com>
While consolidating the PHY reset in phy_init_hw() an unconditionaly
BMCR soft-reset I became quite trigger happy with those. This was later
on deactivated for the Generic PHY driver on the premise that a prior
software entity (e.g: bootloader) might have applied workarounds in
commit 0878fff1f42c ("net: phy: Do not perform software reset for
Generic PHY").
Since we have a hook to wire-up a soft_reset callback, just use that and
get rid of the call to genphy_soft_reset() entirely. This speeds up
initialization and link establishment for most PHYs out there that do
not require a reset.
Fixes: 87aa9f9c61ad ("net: phy: consolidate PHY reset in phy_init_hw()")
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/phy_device.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index db1172db1e7c..9216d2f8e41e 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -880,8 +880,6 @@ int phy_init_hw(struct phy_device *phydev)
if (phydev->drv->soft_reset)
ret = phydev->drv->soft_reset(phydev);
- else
- ret = genphy_soft_reset(phydev);
if (ret < 0)
return ret;
--
2.17.1
^ permalink raw reply related
* [PATCH RFT net-next 2/2] net: phy: marvell: Avoid unnecessary soft reset
From: Florian Fainelli @ 2018-09-19 1:35 UTC (permalink / raw)
To: netdev
Cc: Florian Fainelli, David S. Miller, Andrew Lunn, nbd, cphealy,
harini.katakam, afleming, agust, arnd, asmirnov, avi.kp.137,
avorontsov, baijiaju1990, benh, charles-antoine.couret,
clemens.gruber, colin.king, cyril, david.thomson, ddaney,
dongsheng.wang, dwmw2, eha, houjingj, jeff, Jingju.Hou,
Jisheng.Zhang, johan, Kapil.Juneja, kim.phillips, linyunsheng,
madalin.
In-Reply-To: <20180919013505.11347-1-f.fainelli@gmail.com>
The BMCR.RESET bit on Marvell PHYs appears to be acting as a commit
operation, so we try to hit that bit which disrupts the link, only when
an actual register programming required a change.
Determine from marvell_set_polarity()'s return code whether the register value
was changed and if it was, propagate that to the logic that hits the software
reset bit. This avoids doing unnecessary soft reset if the PHY is configured in
the same state it was previously.
Since m88e1111_config_aneg() likely just hit the software reset bit
blindly without checking whether this is necessary, eliminate that
function and make it use the generic marvell_config_aneg() function
instead.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/marvell.c | 63 +++++++++++++--------------------------
1 file changed, 21 insertions(+), 42 deletions(-)
diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
index f7c69ca34056..24fc4a73c300 100644
--- a/drivers/net/phy/marvell.c
+++ b/drivers/net/phy/marvell.c
@@ -265,7 +265,7 @@ static int marvell_set_polarity(struct phy_device *phydev, int polarity)
return err;
}
- return 0;
+ return val != reg;
}
static int marvell_set_downshift(struct phy_device *phydev, bool enable,
@@ -287,12 +287,15 @@ static int marvell_set_downshift(struct phy_device *phydev, bool enable,
static int marvell_config_aneg(struct phy_device *phydev)
{
+ int changed = 0;
int err;
err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
if (err < 0)
return err;
+ changed = err;
+
err = phy_write(phydev, MII_M1111_PHY_LED_CONTROL,
MII_M1111_PHY_LED_DIRECT);
if (err < 0)
@@ -302,7 +305,7 @@ static int marvell_config_aneg(struct phy_device *phydev)
if (err < 0)
return err;
- if (phydev->autoneg != AUTONEG_ENABLE) {
+ if (phydev->autoneg != AUTONEG_ENABLE || changed) {
/* A write to speed/duplex bits (that is performed by
* genphy_config_aneg() call above) must be followed by
* a software reset. Otherwise, the write has no effect.
@@ -350,42 +353,6 @@ static int m88e1101_config_aneg(struct phy_device *phydev)
return marvell_config_aneg(phydev);
}
-static int m88e1111_config_aneg(struct phy_device *phydev)
-{
- int err;
-
- /* The Marvell PHY has an errata which requires
- * that certain registers get written in order
- * to restart autonegotiation
- */
- err = genphy_soft_reset(phydev);
-
- err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
- if (err < 0)
- return err;
-
- err = phy_write(phydev, MII_M1111_PHY_LED_CONTROL,
- MII_M1111_PHY_LED_DIRECT);
- if (err < 0)
- return err;
-
- err = genphy_config_aneg(phydev);
- if (err < 0)
- return err;
-
- if (phydev->autoneg != AUTONEG_ENABLE) {
- /* A write to speed/duplex bits (that is performed by
- * genphy_config_aneg() call above) must be followed by
- * a software reset. Otherwise, the write has no effect.
- */
- err = genphy_soft_reset(phydev);
- if (err < 0)
- return err;
- }
-
- return 0;
-}
-
#ifdef CONFIG_OF_MDIO
/* Set and/or override some configuration registers based on the
* marvell,reg-init property stored in the of_node for the phydev.
@@ -479,6 +446,7 @@ static int m88e1121_config_aneg_rgmii_delays(struct phy_device *phydev)
static int m88e1121_config_aneg(struct phy_device *phydev)
{
+ int changed = 0;
int err = 0;
if (phy_interface_is_rgmii(phydev)) {
@@ -487,15 +455,26 @@ static int m88e1121_config_aneg(struct phy_device *phydev)
return err;
}
- err = genphy_soft_reset(phydev);
+ err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
if (err < 0)
return err;
- err = marvell_set_polarity(phydev, phydev->mdix_ctrl);
+ changed = err;
+
+ err = genphy_config_aneg(phydev);
if (err < 0)
return err;
- return genphy_config_aneg(phydev);
+ if (phydev->autoneg != AUTONEG_ENABLE || changed) {
+ /* A software reset is used to ensure a "commit" of the
+ * changes is done.
+ */
+ err = genphy_soft_reset(phydev);
+ if (err < 0)
+ return err;
+ }
+
+ return 0;
}
static int m88e1318_config_aneg(struct phy_device *phydev)
@@ -2067,7 +2046,7 @@ static struct phy_driver marvell_drivers[] = {
.flags = PHY_HAS_INTERRUPT,
.probe = marvell_probe,
.config_init = &m88e1111_config_init,
- .config_aneg = &m88e1111_config_aneg,
+ .config_aneg = &marvell_config_aneg,
.read_status = &marvell_read_status,
.ack_interrupt = &marvell_ack_interrupt,
.config_intr = &marvell_config_intr,
--
2.17.1
^ permalink raw reply related
* Re: bpfilter breaks IPT_SO_GET_INFO
From: Michal Kubecek @ 2018-09-19 7:18 UTC (permalink / raw)
To: Dmitry Vyukov
Cc: Alexei Starovoitov, David Miller, Daniel Borkmann, netdev, LKML,
syzkaller, netfilter-devel, Fabian Vogt
In-Reply-To: <CACT4Y+YS55tk5xoPmscBXcgSbLrgWFSLAz1MUEjby0XT8eanmQ@mail.gmail.com>
On Mon, Sep 17, 2018 at 03:36:21PM +0200, Dmitry Vyukov wrote:
> Hi,
>
> I am having some problem with upstream kernel and bpfilter. The
> manifestation is that IPT_SO_GET_INFO on an ipv4 socket works, then
> something (that I can't fully localize but can reproduce) happens and
> then IPT_SO_GET_INFO starts permanently returning 256.
...
> Now the litmus program always fails with:
>
> getsockopt(3, SOL_IP, 0x40 /* IP_??? */,
> "filter\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., [84])
> = 256
>
> I am currently on upstream commit
> 28619527b8a712590c93d0a9e24b4425b9376a8c, my .config is attached. I
> don't know what is bpfilter, I see it mentions some umh, if it
> requires some additional setup I don't it, i.e. I don't install any
> userspace modules/helpers.
This looks similar to the fallback issue described here:
https://bugzilla.suse.com/show_bug.cgi?id=1106751#c1
Unfortunately I didn't have time to look into it more closely yet.
Michal Kubecek
^ permalink raw reply
* Re: [PATCH bpf-next 2/2] xsk: fix bug when trying to use both copy and zero-copy on one queue id
From: Jakub Kicinski @ 2018-09-19 1:55 UTC (permalink / raw)
To: Y Song
Cc: magnus.karlsson, Björn Töpel, Alexei Starovoitov,
Daniel Borkmann, netdev
In-Reply-To: <CAH3MdRU8jKBdpNK6Uq3NRyeb2ih6tgWJOAKW_pQFm0QQ0kuvpw@mail.gmail.com>
On Tue, 18 Sep 2018 10:22:11 -0700, Y Song wrote:
> > +/* The umem is stored both in the _rx struct and the _tx struct as we do
> > + * not know if the device has more tx queues than rx, or the opposite.
> > + * This might also change during run time.
> > + */
> > +static void xdp_reg_umem_at_qid(struct net_device *dev, struct xdp_umem *umem,
> > + u16 queue_id)
> > +{
> > + if (queue_id < dev->real_num_rx_queues)
> > + dev->_rx[queue_id].umem = umem;
> > + if (queue_id < dev->real_num_tx_queues)
> > + dev->_tx[queue_id].umem = umem;
> > +}
> > +
> > +static struct xdp_umem *xdp_get_umem_from_qid(struct net_device *dev,
> > + u16 queue_id)
> > +{
> > + if (queue_id < dev->real_num_rx_queues)
> > + return dev->_rx[queue_id].umem;
> > + if (queue_id < dev->real_num_tx_queues)
> > + return dev->_tx[queue_id].umem;
> > +
> > + return NULL;
> > +}
> > +
> > +static void xdp_clear_umem_at_qid(struct net_device *dev, u16 queue_id)
> > +{
> > + /* Zero out the entry independent on how many queues are configured
> > + * at this point in time, as it might be used in the future.
> > + */
> > + if (queue_id < dev->num_rx_queues)
> > + dev->_rx[queue_id].umem = NULL;
> > + if (queue_id < dev->num_tx_queues)
> > + dev->_tx[queue_id].umem = NULL;
> > +}
> > +
>
> I am sure whether the following scenario can happen or not.
> Could you clarify?
> 1. suppose initially we have num_rx_queues = num_tx_queues = 10
> xdp_reg_umem_at_qid() set umem1 to queue_id = 8
> 2. num_tx_queues is changed to 5
> 3. xdp_clear_umem_at_qid() is called for queue_id = 8,
> and dev->_rx[8].umum = 0.
> 4. xdp_reg_umem_at_qid() is called gain to set for queue_id = 8
> dev->_rx[8].umem = umem2
> 5. num_tx_queues is changed to 10
> Now dev->_rx[8].umem != dev->_tx[8].umem, is this possible and is it
> a problem?
Plus IIRC the check of qid vs real_num_[rt]x_queues in xsk_bind() is
not under rtnl_lock so it doesn't count for much. Why not do all the
checks against num_[rt]x_queues here, instead of real_..?
^ permalink raw reply
* Re: [PATCH net-next v5 03/20] zinc: ChaCha20 generic C implementation and selftest
From: Jason A. Donenfeld @ 2018-09-19 2:02 UTC (permalink / raw)
To: Eric Biggers
Cc: LKML, Netdev, Linux Crypto Mailing List, David Miller,
Greg Kroah-Hartman, Samuel Neves, Andrew Lutomirski,
Jean-Philippe Aumasson
In-Reply-To: <20180919010816.GD74746@gmail.com>
On Wed, Sep 19, 2018 at 3:08 AM Eric Biggers <ebiggers@kernel.org> wrote:
> Does this consistently perform as well as an implementation that organizes the
> operations such that the quarterrounds for all columns/diagonals are
> interleaved? As-is, there are tight dependencies in QUARTER_ROUND() (as well as
> in the existing chacha20_block() in lib/chacha20.c, for that matter), so we're
> heavily depending on the compiler to do the needed interleaving so as to not get
> potentially disastrous performance. Making it explicit could be a good idea.
It does perform as well, and the compiler outputs good code, even on
older compilers. Notably that's all a single statement (via the comma
operator).
> > +}
> > +
> > +static void chacha20_generic(u8 *out, const u8 *in, u32 len, const u32 key[8],
> > + const u32 counter[4])
> > +{
> > + __le32 buf[CHACHA20_BLOCK_WORDS];
> > + u32 x[] = {
> > + EXPAND_32_BYTE_K,
> > + key[0], key[1], key[2], key[3],
> > + key[4], key[5], key[6], key[7],
> > + counter[0], counter[1], counter[2], counter[3]
> > + };
> > +
> > + if (out != in)
> > + memmove(out, in, len);
> > +
> > + while (len >= CHACHA20_BLOCK_SIZE) {
> > + chacha20_block_generic(buf, x);
> > + crypto_xor(out, (u8 *)buf, CHACHA20_BLOCK_SIZE);
> > + len -= CHACHA20_BLOCK_SIZE;
> > + out += CHACHA20_BLOCK_SIZE;
> > + }
> > + if (len) {
> > + chacha20_block_generic(buf, x);
> > + crypto_xor(out, (u8 *)buf, len);
> > + }
> > +}
>
> If crypto_xor_cpy() is used instead of crypto_xor(), and 'in' is incremented
> along with 'out', then the memmove() is not needed.
Nice idea, thanks. Implemented.
Jason
^ permalink raw reply
* 答复: [PATCH][net-next] netlink: avoid to allocate full skb when sending to many devices
From: Li,Rongqing @ 2018-09-19 1:19 UTC (permalink / raw)
To: Eric Dumazet, netdev@vger.kernel.org
In-Reply-To: <3f01dd84-32c5-f674-75ac-a93a90f32bf7@gmail.com>
> On 09/17/2018 10:26 PM, Li RongQing wrote:
> > if skb->head is vmalloc address, when this skb is delivered, full
> > allocation for this skb is required, if there are many devices, the
> > ---
> > net/netlink/af_netlink.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> >
>
> This looks very broken to me.
>
> Only the original skb (given as an argument to __netlink_deliver_tap()) is
> guaranteed to not disappear while the loop is performed.
>
> (There is no skb_clone() after the first netlink_to_full_skb())
>
Thank you;
I will rework it
-RongQing
^ permalink raw reply
* Re: [PATCH] net: apm: xgene: force XGene enet driver to re-balance IRQ usage
From: Eric Dumazet @ 2018-09-19 2:32 UTC (permalink / raw)
To: Florian Fainelli, ahs3, Lendacky, Thomas, David Miller
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
isubramanian@apm.com, kchudgar@apm.com, qnguyen@apm.com
In-Reply-To: <0e0f3aa6-9b6e-edd5-226f-2e23de9b5a0f@gmail.com>
On 09/18/2018 05:03 PM, Florian Fainelli wrote:
> On 09/18/2018 04:56 PM, Eric Dumazet wrote:
>>
>>
>> On 09/18/2018 04:27 PM, Eric Dumazet wrote:
>>>
>>
>>> I remember one of the napi_complete_done() change had to be reverted,
>>> for some obscure reason.
>>
>>
>>
>> That was not exactly a revert, :
>
> This is what I have so far for the drivers that both use
> napi_complete_done() without checking the return value and implement a
> ndo_poll_controller() callback:
>
> https://github.com/ffainelli/linux/commits/napi-check
In fact, there is still to explain what the bug is.
napi_complete_done() return value can be ignored, unless drivers
have to disable IRQ in their interrupt handler, using the following
construct :
if (napi_schedule_prep(napi)) {
.... disable interrupt ....
__napi_schedule_irqoff(napi);
}
It _can_ be used by other drivers to not rearm interrupts needlessly.
The bug discussed in this thread (re-balance IRQ usage) is of the same kind than
the one fixed in commit d7aba644ffdebf756e51e26a2229055211838e89 ("amd-xgbe: Enable IRQs only if napi_complete_done() is true")
^ permalink raw reply
* Re: [net-next 00/14][pull request] 40GbE Intel Wired LAN Driver Updates 2018-09-18
From: David Miller @ 2018-09-19 2:42 UTC (permalink / raw)
To: jeffrey.t.kirsher; +Cc: netdev, nhorman, sassmann, jogreene
In-Reply-To: <20180918223731.31876-1-jeffrey.t.kirsher@intel.com>
From: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Date: Tue, 18 Sep 2018 15:37:17 -0700
> This series contains changes to i40evf so that it becomes a more
> generic virtual function driver for current and future silicon.
>
> While doing the rename of i40evf to a more generic name of iavf,
> we also put the driver on a severe diet due to how much of the
> code was unneeded or was unused. The outcome is a lean and mean
> virtual function driver that continues to work on existing 40GbE
> (i40e) virtual devices and prepped for future supported devices,
> like the 100GbE (ice) virtual devices.
>
> This solves 2 issues we saw coming or were already present, the
> first was constant code duplication happening with i40e/i40evf,
> when much of the duplicate code in the i40evf was not used or was
> not needed. The second was to remove the future confusion of why
> future VF devices that were not considered "40GbE" only devices
> were supported by i40evf.
>
> The thought is that iavf will be the virtual function driver for
> all future devices, so it should have a "generic" name to properly
> represent that it is the VF driver for multiple generations of
> devices.
>
> The last patch in this series is unreleated to the iavf conversion
> and just has to do with a MODULE_LICENSE correction.
>
> Known Caveats:
> Existing user space configurations may have to change, but the module
> alias in patch 1 helps a bit here.
Ok, let's see how this goes.
Pulled, thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 03/10] net: sched: extend Qdisc with rcu
From: David Miller @ 2018-09-19 2:45 UTC (permalink / raw)
To: vladbu
Cc: netdev, jhs, xiyou.wangcong, jiri, stephen, ktkhai,
nicolas.dichtel, gregkh, mark.rutland, leon, paulmck, fw, dsahern,
christian, lucien.xin, jakub.kicinski, jbenc
In-Reply-To: <1537168660-24032-4-git-send-email-vladbu@mellanox.com>
From: Vlad Buslov <vladbu@mellanox.com>
Date: Mon, 17 Sep 2018 10:17:33 +0300
> +struct Qdisc *qdisc_lookup_rcu(struct net_device *dev, u32 handle)
> +{
> + struct Qdisc *q;
> + struct netdev_queue *nq;
Reverse christmas tree for the local variables, please.
^ permalink raw reply
* Re: [PATCH net 0/2] net: stmmac: Coalesce and tail addr fixes
From: David Miller @ 2018-09-19 2:48 UTC (permalink / raw)
To: Jose.Abreu
Cc: netdev, f.fainelli, narmstrong, jbrunet, martin.blumenstingl,
Joao.Pinto, peppe.cavallaro, alexandre.torgue
In-Reply-To: <cover.1537171752.git.joabreu@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Mon, 17 Sep 2018 09:22:55 +0100
> The fix for coalesce timer and a fix in tail address setting that impacts
> XGMAC2 operation.
>
> The series is:
> Tested-by: Jerome Brunet <jbrunet@baylibre.com>
> on a113 s400 board (single queue)
Series applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH v2 1/2] netlink: add NLA_REJECT policy type
From: David Miller @ 2018-09-19 2:51 UTC (permalink / raw)
To: johannes; +Cc: netdev, marcelo.leitner, mkubecek, johannes.berg
In-Reply-To: <20180917095729.11185-1-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Mon, 17 Sep 2018 11:57:28 +0200
> From: Johannes Berg <johannes.berg@intel.com>
>
> In some situations some netlink attributes may be used for output
> only (kernel->userspace) or may be reserved for future use. It's
> then helpful to be able to prevent userspace from using them in
> messages sent to the kernel, since they'd otherwise be ignored and
> any future will become impossible if this happens.
>
> Add NLA_REJECT to the policy which does nothing but reject (with
> EINVAL) validation of any messages containing this attribute.
> Allow for returning a specific extended ACK error message in the
> validation_data pointer.
>
> While at it clear up the documentation a bit - the NLA_BITFIELD32
> documentation was added to the list of len field descriptions.
>
> Also, use NL_SET_BAD_ATTR() in one place where it's open-coded.
>
> The specific case I have in mind now is a shared nested attribute
> containing request/response data, and it would be pointless and
> potentially confusing to have userspace include response data in
> the messages that actually contain a request.
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
> v2: preserve behaviour of overwriting the extack message, with
> either the generic or the specific one now
Applied.
^ permalink raw reply
* Re: [PATCH v2 2/2] netlink: add ethernet address policy types
From: David Miller @ 2018-09-19 2:51 UTC (permalink / raw)
To: johannes; +Cc: netdev, marcelo.leitner, mkubecek, johannes.berg
In-Reply-To: <20180917095729.11185-2-johannes@sipsolutions.net>
From: Johannes Berg <johannes@sipsolutions.net>
Date: Mon, 17 Sep 2018 11:57:29 +0200
> From: Johannes Berg <johannes.berg@intel.com>
>
> Commonly, ethernet addresses are just using a policy of
> { .len = ETH_ALEN }
> which leaves userspace free to send more data than it should,
> which may hide bugs.
>
> Introduce NLA_EXACT_LEN which checks for exact size, rejecting
> the attribute if it's not exactly that length. Also add
> NLA_EXACT_LEN_WARN which requires the minimum length and will
> warn on longer attributes, for backward compatibility.
>
> Use these to define NLA_POLICY_ETH_ADDR (new strict policy) and
> NLA_POLICY_ETH_ADDR_COMPAT (compatible policy with warning);
> these are used like this:
>
> static const struct nla_policy <name>[...] = {
> [NL_ATTR_NAME] = NLA_POLICY_ETH_ADDR,
> ...
> };
>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
> v2: add only NLA_EXACT_LEN/NLA_EXACT_LEN_WARN and build on top
> of that for ethernet address validation, so it can be extended
> for other types (e.g. IPv6 addresses)
Applied.
^ permalink raw reply
* Re: [PATCH][net-next] veth: rename pcpu_vstats as pcpu_lstats
From: David Miller @ 2018-09-19 2:52 UTC (permalink / raw)
To: lirongqing; +Cc: netdev
In-Reply-To: <1537181215-13213-1-git-send-email-lirongqing@baidu.com>
From: Li RongQing <lirongqing@baidu.com>
Date: Mon, 17 Sep 2018 18:46:55 +0800
> struct pcpu_vstats and pcpu_lstats have same members and
> usage, and pcpu_lstats is used in many files, so rename
> pcpu_vstats as pcpu_lstats to reduce duplicate definition
>
> Signed-off-by: Zhang Yu <zhangyu31@baidu.com>
> Signed-off-by: Li RongQing <lirongqing@baidu.com>
Applied.
^ permalink raw reply
* Re: [RFC PATCH bpf-next v3 1/7] bpf: rename stack trace map
From: Mauricio Vasquez @ 2018-09-19 2:53 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, Daniel Borkmann, netdev, Yonghong Song
In-Reply-To: <20180918230551.7rh3wytacry7i3wl@ast-mbp>
On 09/18/2018 06:05 PM, Alexei Starovoitov wrote:
> On Tue, Sep 18, 2018 at 06:52:34AM +0200, Mauricio Vasquez B wrote:
>> In the following patches queue and stack maps (FIFO and LIFO
>> datastructures) will be implemented. In order to avoid confusion and
>> a possible name clash rename stackmap.c to stacktracemap.c and
>> stack_map_ops to stack_trace_map_ops
>>
>> Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
>> ---
>> include/linux/bpf_types.h | 2
>> kernel/bpf/Makefile | 2
>> kernel/bpf/stackmap.c | 624 --------------------------------------------
>> kernel/bpf/stacktracemap.c | 624 ++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 626 insertions(+), 626 deletions(-)
>> delete mode 100644 kernel/bpf/stackmap.c
>> create mode 100644 kernel/bpf/stacktracemap.c
> Just new file for stack/queue will be enough. I understand that unfortunate name confusion,
> but this is too late. It will cause all sorts of headaches in fixes that go to net/stable.
>
>
Ok, will only rename stack_map_ops to stack_trace_map_ops then.
^ permalink raw reply
* Re: bpfilter breaks IPT_SO_GET_INFO
From: Dmitry Vyukov @ 2018-09-19 8:29 UTC (permalink / raw)
To: Michal Kubecek
Cc: Alexei Starovoitov, David Miller, Daniel Borkmann, netdev, LKML,
syzkaller, NetFilter, Fabian Vogt, Takashi Iwai
In-Reply-To: <20180919071847.GE3876@unicorn.suse.cz>
On Wed, Sep 19, 2018 at 9:18 AM, Michal Kubecek <mkubecek@suse.cz> wrote:
> On Mon, Sep 17, 2018 at 03:36:21PM +0200, Dmitry Vyukov wrote:
>> Hi,
>>
>> I am having some problem with upstream kernel and bpfilter. The
>> manifestation is that IPT_SO_GET_INFO on an ipv4 socket works, then
>> something (that I can't fully localize but can reproduce) happens and
>> then IPT_SO_GET_INFO starts permanently returning 256.
> ...
>> Now the litmus program always fails with:
>>
>> getsockopt(3, SOL_IP, 0x40 /* IP_??? */,
>> "filter\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., [84])
>> = 256
>>
>> I am currently on upstream commit
>> 28619527b8a712590c93d0a9e24b4425b9376a8c, my .config is attached. I
>> don't know what is bpfilter, I see it mentions some umh, if it
>> requires some additional setup I don't it, i.e. I don't install any
>> userspace modules/helpers.
>
> This looks similar to the fallback issue described here:
>
> https://bugzilla.suse.com/show_bug.cgi?id=1106751#c1
>
> Unfortunately I didn't have time to look into it more closely yet.
+Takashi
But I already have CONFIG_BPFILTER_UMH=y in my config, so it does not
help completely.
Also in my case it is working initially, but breaks after I run the
second program.
^ permalink raw reply
* Re: [PATCH net] selftests: pmtu: properly redirect stderr to /dev/null
From: David Miller @ 2018-09-19 2:54 UTC (permalink / raw)
To: sd; +Cc: netdev, sbrivio
In-Reply-To: <132b14cab49cd959d9cf6e6607b01c383db2d984.1537190905.git.sd@queasysnail.net>
From: Sabrina Dubroca <sd@queasysnail.net>
Date: Mon, 17 Sep 2018 15:30:06 +0200
> The cleanup function uses "$CMD 2 > /dev/null", which doesn't actually
> send stderr to /dev/null, so when the netns doesn't exist, the error
> message is shown. Use "2> /dev/null" instead, so that those messages
> disappear, as was intended.
>
> Fixes: d1f1b9cbf34c ("selftests: net: Introduce first PMTU test")
> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Applied.
^ permalink raw reply
* Re: [PATCH] net: emac: fix fixed-link setup for the RTL8363SB switch
From: David Miller @ 2018-09-19 2:56 UTC (permalink / raw)
To: chunkeey; +Cc: netdev, ivan
In-Reply-To: <20180917152240.18177-1-chunkeey@gmail.com>
From: Christian Lamparter <chunkeey@gmail.com>
Date: Mon, 17 Sep 2018 17:22:40 +0200
> On the Netgear WNDAP620, the emac ethernet isn't receiving nor
> xmitting any frames from/to the RTL8363SB (identifies itself
> as a RTL8367RB).
>
> This is caused by the emac hardware not knowing the forced link
> parameters for speed, duplex, pause, etc.
>
> This begs the question, how this was working on the original
> driver code, when it was necessary to set the phy_address and
> phy_map to 0xffffffff. But I guess without access to the old
> PPC405/440/460 hardware, it's not possible to know.
>
> Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
Applied.
^ 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