* Re: [PATCH net-next] udp: Add tracepoints to monitor skbs going in and out of a UDP socket
From: David Howells @ 2018-10-05 6:46 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: dhowells, David Ahern, netdev, linux-afs, linux-kernel
In-Reply-To: <20181005020316.7cqlcfkedhamwhtt@ast-mbp.dhcp.thefacebook.com>
Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> - kprobes at the top of the function don't use traps and they've been
> optimized over the years to have very low overhead
To quote kprobes.txt:
How Does a Kprobe Work?
-----------------------
When a kprobe is registered, Kprobes makes a copy of the probed
instruction and replaces the first byte(s) of the probed instruction
with a breakpoint instruction (e.g., int3 on i386 and x86_64).
Perhaps the docs need updating.
However, for my purposes, tracepoints are easier.
Anyway, as I said, I don't feel that strongly about the patch. It was useful
for me, and I thought it might be useful for other people.
David
^ permalink raw reply
* Re: [RFC PATCH bpf-next v4 4/7] bpf: add bpf queue and stack maps
From: Alexei Starovoitov @ 2018-10-04 23:57 UTC (permalink / raw)
To: Mauricio Vasquez B; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev
In-Reply-To: <153867316459.10087.2913745359364075968.stgit@kernel>
On Thu, Oct 04, 2018 at 07:12:44PM +0200, Mauricio Vasquez B wrote:
> Implement two new kind of maps that support the peek, push and pop
> operations.
>
> A use case for this is to keep track of a pool of elements, like
> network ports in a SNAT.
>
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
> ---
> include/linux/bpf.h | 7 +
> include/linux/bpf_types.h | 2
> include/uapi/linux/bpf.h | 35 ++++-
> kernel/bpf/Makefile | 2
> kernel/bpf/core.c | 3
> kernel/bpf/helpers.c | 43 ++++++
> kernel/bpf/queue_stack_maps.c | 300 +++++++++++++++++++++++++++++++++++++++++
> kernel/bpf/syscall.c | 31 +++-
> kernel/bpf/verifier.c | 14 +-
> net/core/filter.c | 6 +
> 10 files changed, 424 insertions(+), 19 deletions(-)
> create mode 100644 kernel/bpf/queue_stack_maps.c
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 98c7eeb6d138..cad3bc5cffd1 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -40,6 +40,9 @@ struct bpf_map_ops {
> int (*map_update_elem)(struct bpf_map *map, void *key, void *value, u64 flags);
> int (*map_delete_elem)(struct bpf_map *map, void *key);
> void *(*map_lookup_and_delete_elem)(struct bpf_map *map, void *key);
> + int (*map_push_elem)(struct bpf_map *map, void *value, u64 flags);
> + int (*map_pop_elem)(struct bpf_map *map, void *value);
> + int (*map_peek_elem)(struct bpf_map *map, void *value);
>
> /* funcs called by prog_array and perf_event_array map */
> void *(*map_fd_get_ptr)(struct bpf_map *map, struct file *map_file,
> @@ -139,6 +142,7 @@ enum bpf_arg_type {
> ARG_CONST_MAP_PTR, /* const argument used as pointer to bpf_map */
> ARG_PTR_TO_MAP_KEY, /* pointer to stack used as map key */
> ARG_PTR_TO_MAP_VALUE, /* pointer to stack used as map value */
> + ARG_PTR_TO_UNINIT_MAP_VALUE, /* pointer to valid memory used to store a map value */
>
> /* the following constraints used to prototype bpf_memcmp() and other
> * functions that access data on eBPF program stack
> @@ -825,6 +829,9 @@ static inline int bpf_fd_reuseport_array_update_elem(struct bpf_map *map,
> extern const struct bpf_func_proto bpf_map_lookup_elem_proto;
> extern const struct bpf_func_proto bpf_map_update_elem_proto;
> extern const struct bpf_func_proto bpf_map_delete_elem_proto;
> +extern const struct bpf_func_proto bpf_map_push_elem_proto;
> +extern const struct bpf_func_proto bpf_map_pop_elem_proto;
> +extern const struct bpf_func_proto bpf_map_peek_elem_proto;
>
> extern const struct bpf_func_proto bpf_get_prandom_u32_proto;
> extern const struct bpf_func_proto bpf_get_smp_processor_id_proto;
> diff --git a/include/linux/bpf_types.h b/include/linux/bpf_types.h
> index 658509daacd4..a2ec73aa1ec7 100644
> --- a/include/linux/bpf_types.h
> +++ b/include/linux/bpf_types.h
> @@ -69,3 +69,5 @@ BPF_MAP_TYPE(BPF_MAP_TYPE_XSKMAP, xsk_map_ops)
> BPF_MAP_TYPE(BPF_MAP_TYPE_REUSEPORT_SOCKARRAY, reuseport_array_ops)
> #endif
> #endif
> +BPF_MAP_TYPE(BPF_MAP_TYPE_QUEUE, queue_map_ops)
> +BPF_MAP_TYPE(BPF_MAP_TYPE_STACK, stack_map_ops)
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 3bb94aa2d408..bfa042273fad 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -129,6 +129,8 @@ enum bpf_map_type {
> BPF_MAP_TYPE_CGROUP_STORAGE,
> BPF_MAP_TYPE_REUSEPORT_SOCKARRAY,
> BPF_MAP_TYPE_PERCPU_CGROUP_STORAGE,
> + BPF_MAP_TYPE_QUEUE,
> + BPF_MAP_TYPE_STACK,
> };
>
> enum bpf_prog_type {
> @@ -463,6 +465,28 @@ union bpf_attr {
> * Return
> * 0 on success, or a negative error in case of failure.
> *
> + * int bpf_map_push_elem(struct bpf_map *map, const void *value, u64 flags)
> + * Description
> + * Push an element *value* in *map*. *flags* is one of:
> + *
> + * **BPF_EXIST**
> + * If the queue/stack is full, the oldest element is removed to
> + * make room for this.
> + * Return
> + * 0 on success, or a negative error in case of failure.
> + *
> + * int bpf_map_pop_elem(struct bpf_map *map, void *value)
> + * Description
> + * Pop an element from *map*.
> + * Return
> + * 0 on success, or a negative error in case of failure.
> + *
> + * int bpf_map_peek_elem(struct bpf_map *map, void *value)
> + * Description
> + * Get an element from *map* without removing it.
> + * Return
> + * 0 on success, or a negative error in case of failure.
> + *
> * int bpf_probe_read(void *dst, u32 size, const void *src)
> * Description
> * For tracing programs, safely attempt to read *size* bytes from
> @@ -790,14 +814,14 @@ union bpf_attr {
> *
> * int ret;
> * struct bpf_tunnel_key key = {};
> - *
> + *
> * ret = bpf_skb_get_tunnel_key(skb, &key, sizeof(key), 0);
> * if (ret < 0)
> * return TC_ACT_SHOT; // drop packet
> - *
> + *
> * if (key.remote_ipv4 != 0x0a000001)
> * return TC_ACT_SHOT; // drop packet
> - *
> + *
> * return TC_ACT_OK; // accept packet
> *
> * This interface can also be used with all encapsulation devices
> @@ -2304,7 +2328,10 @@ union bpf_attr {
> FN(skb_ancestor_cgroup_id), \
> FN(sk_lookup_tcp), \
> FN(sk_lookup_udp), \
> - FN(sk_release),
> + FN(sk_release), \
> + FN(map_push_elem), \
> + FN(map_pop_elem), \
> + FN(map_peek_elem),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> diff --git a/kernel/bpf/Makefile b/kernel/bpf/Makefile
> index 0488b8258321..17afae9e65f3 100644
> --- a/kernel/bpf/Makefile
> +++ b/kernel/bpf/Makefile
> @@ -3,7 +3,7 @@ obj-y := core.o
>
> obj-$(CONFIG_BPF_SYSCALL) += syscall.o verifier.o inode.o helpers.o tnum.o
> obj-$(CONFIG_BPF_SYSCALL) += hashtab.o arraymap.o percpu_freelist.o bpf_lru_list.o lpm_trie.o map_in_map.o
> -obj-$(CONFIG_BPF_SYSCALL) += local_storage.o
> +obj-$(CONFIG_BPF_SYSCALL) += local_storage.o queue_stack_maps.o
> obj-$(CONFIG_BPF_SYSCALL) += disasm.o
> obj-$(CONFIG_BPF_SYSCALL) += btf.o
> ifeq ($(CONFIG_NET),y)
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 3f5bf1af0826..8d2db076d123 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -1783,6 +1783,9 @@ BPF_CALL_0(bpf_user_rnd_u32)
> const struct bpf_func_proto bpf_map_lookup_elem_proto __weak;
> const struct bpf_func_proto bpf_map_update_elem_proto __weak;
> const struct bpf_func_proto bpf_map_delete_elem_proto __weak;
> +const struct bpf_func_proto bpf_map_push_elem_proto __weak;
> +const struct bpf_func_proto bpf_map_pop_elem_proto __weak;
> +const struct bpf_func_proto bpf_map_peek_elem_proto __weak;
>
> const struct bpf_func_proto bpf_get_prandom_u32_proto __weak;
> const struct bpf_func_proto bpf_get_smp_processor_id_proto __weak;
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index 6502115e8f55..ab0d5e3f9892 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -76,6 +76,49 @@ const struct bpf_func_proto bpf_map_delete_elem_proto = {
> .arg2_type = ARG_PTR_TO_MAP_KEY,
> };
>
> +BPF_CALL_3(bpf_map_push_elem, struct bpf_map *, map, void *, value, u64, flags)
> +{
> + return map->ops->map_push_elem(map, value, flags);
> +}
> +
> +const struct bpf_func_proto bpf_map_push_elem_proto = {
> + .func = bpf_map_push_elem,
> + .gpl_only = false,
> + .pkt_access = true,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_CONST_MAP_PTR,
> + .arg2_type = ARG_PTR_TO_MAP_VALUE,
> + .arg3_type = ARG_ANYTHING,
> +};
> +
> +BPF_CALL_2(bpf_map_pop_elem, struct bpf_map *, map, void *, value)
> +{
> + return map->ops->map_pop_elem(map, value);
> +}
> +
> +const struct bpf_func_proto bpf_map_pop_elem_proto = {
> + .func = bpf_map_pop_elem,
> + .gpl_only = false,
> + .pkt_access = true,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_CONST_MAP_PTR,
> + .arg2_type = ARG_PTR_TO_UNINIT_MAP_VALUE,
> +};
> +
> +BPF_CALL_2(bpf_map_peek_elem, struct bpf_map *, map, void *, value)
> +{
> + return map->ops->map_peek_elem(map, value);
> +}
> +
> +const struct bpf_func_proto bpf_map_peek_elem_proto = {
> + .func = bpf_map_pop_elem,
> + .gpl_only = false,
> + .pkt_access = true,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_CONST_MAP_PTR,
> + .arg2_type = ARG_PTR_TO_UNINIT_MAP_VALUE,
> +};
> +
> const struct bpf_func_proto bpf_get_prandom_u32_proto = {
> .func = bpf_user_rnd_u32,
> .gpl_only = false,
> diff --git a/kernel/bpf/queue_stack_maps.c b/kernel/bpf/queue_stack_maps.c
> new file mode 100644
> index 000000000000..a597c5ba68f6
> --- /dev/null
> +++ b/kernel/bpf/queue_stack_maps.c
> @@ -0,0 +1,300 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * queue_stack_maps.c: BPF queue and stack maps
> + *
> + * Copyright (c) 2018 Politecnico di Torino
> + */
> +#include <linux/bpf.h>
> +#include <linux/list.h>
> +#include <linux/slab.h>
> +#include "percpu_freelist.h"
> +
> +#define QUEUE_STACK_CREATE_FLAG_MASK \
> + (BPF_F_NUMA_NODE | BPF_F_RDONLY | BPF_F_WRONLY)
> +
> +
> +struct bpf_queue_stack {
> + struct bpf_map map;
> + raw_spinlock_t lock;
> + u32 head, tail;
> + u32 index_mask;
> + u32 count;
> +
> + char elements[0] __aligned(8);
> +};
> +
> +static struct bpf_queue_stack *bpf_queue_stack(struct bpf_map *map)
> +{
> + return container_of(map, struct bpf_queue_stack, map);
> +}
> +
> +static bool queue_stack_map_is_empty(struct bpf_queue_stack *qs)
> +{
> + return qs->count == 0;
> +}
> +
> +static bool queue_stack_map_is_full(struct bpf_queue_stack *qs)
> +{
> + return qs->count == qs->map.max_entries;
> +}
> +
> +/* Called from syscall */
> +static int queue_stack_map_alloc_check(union bpf_attr *attr)
> +{
> + /* check sanity of attributes */
> + if (attr->max_entries == 0 || attr->key_size != 0 ||
> + attr->map_flags & ~QUEUE_STACK_CREATE_FLAG_MASK)
> + return -EINVAL;
> +
> + if (attr->value_size > KMALLOC_MAX_SIZE)
> + /* if value_size is bigger, the user space won't be able to
> + * access the elements.
> + */
> + return -E2BIG;
> +
> + return 0;
> +}
> +
> +static struct bpf_map *queue_stack_map_alloc(union bpf_attr *attr)
> +{
> + int ret, numa_node = bpf_map_attr_numa_node(attr);
> + u32 max_entries, value_size, index_mask;
> + u64 queue_size, cost, mask64;
> + struct bpf_queue_stack *qs;
> +
> + max_entries = attr->max_entries;
> + value_size = attr->value_size;
> +
> + /* From arraymap.c:
> + * On 32 bit archs roundup_pow_of_two() with max_entries that has
> + * upper most bit set in u32 space is undefined behavior due to
> + * resulting 1U << 32, so do it manually here in u64 space.
> + */
> + mask64 = fls_long(max_entries - 1);
> + mask64 = 1ULL << mask64;
> + mask64 -= 1;
> +
> + index_mask = mask64;
> +
> + /* Round up queue size to nearest power of 2 */
> + max_entries = index_mask + 1;
what's the point of roundup ?
The memory waste becomes quite large when max_entries are high.
If queue/stack is sized to exact max_entries,
then 'count' can be removed too, right?
> + /* Check for overflows. */
> + if (max_entries < attr->max_entries)
> + return ERR_PTR(-E2BIG);
> +
> + queue_size = sizeof(*qs) + (u64) value_size * max_entries;
> +
> + cost = queue_size;
> + if (cost >= U32_MAX - PAGE_SIZE)
> + return ERR_PTR(-E2BIG);
> +
> + cost = round_up(cost, PAGE_SIZE) >> PAGE_SHIFT;
> +
> + ret = bpf_map_precharge_memlock(cost);
> + if (ret < 0)
> + return ERR_PTR(ret);
> +
> + qs = bpf_map_area_alloc(queue_size, numa_node);
> + if (!qs)
> + return ERR_PTR(-ENOMEM);
> +
> + memset(qs, 0, sizeof(*qs));
> +
> + bpf_map_init_from_attr(&qs->map, attr);
> +
> + qs->map.pages = cost;
> + qs->index_mask = index_mask;
> +
> + raw_spin_lock_init(&qs->lock);
> +
> + return &qs->map;
> +}
> +
> +/* Called when map->refcnt goes to zero, either from workqueue or from syscall */
> +static void queue_stack_map_free(struct bpf_map *map)
> +{
> + struct bpf_queue_stack *qs = bpf_queue_stack(map);
> +
> + /* at this point bpf_prog->aux->refcnt == 0 and this map->refcnt == 0,
> + * so the programs (can be more than one that used this map) were
> + * disconnected from events. Wait for outstanding critical sections in
> + * these programs to complete
> + */
> + synchronize_rcu();
> +
> + bpf_map_area_free(qs);
> +}
> +
> +static int __queue_map_get(struct bpf_map *map, void *value, bool delete)
> +{
> + struct bpf_queue_stack *qs = bpf_queue_stack(map);
> + unsigned long flags;
> + int err = 0;
> + void *ptr;
> +
> + raw_spin_lock_irqsave(&qs->lock, flags);
> +
> + if (queue_stack_map_is_empty(qs)) {
> + err = -ENOENT;
> + goto out;
> + }
> +
> + ptr = &qs->elements[qs->tail * qs->map.value_size];
> + memcpy(value, ptr, qs->map.value_size);
> +
> + if (delete) {
> + qs->tail = (qs->tail + 1) & qs->index_mask;
> + qs->count--;
> + }
> +
> +out:
> + raw_spin_unlock_irqrestore(&qs->lock, flags);
> + return err;
> +}
> +
> +
> +static int __stack_map_get(struct bpf_map *map, void *value, bool delete)
> +{
> + struct bpf_queue_stack *qs = bpf_queue_stack(map);
> + unsigned long flags;
> + int err = 0;
> + void *ptr;
> + u32 index;
> +
> + raw_spin_lock_irqsave(&qs->lock, flags);
> +
> + if (queue_stack_map_is_empty(qs)) {
> + err = -ENOENT;
> + goto out;
> + }
> +
> + index = (qs->head - 1) & qs->index_mask;
> + ptr = &qs->elements[index * qs->map.value_size];
> + memcpy(value, ptr, qs->map.value_size);
> +
> + if (delete) {
> + qs->head = (qs->head - 1) & qs->index_mask;
> + qs->count--;
> + }
> +
> +out:
> + raw_spin_unlock_irqrestore(&qs->lock, flags);
> + return err;
> +}
> +
> +/* Called from syscall or from eBPF program */
> +static int queue_map_peek_elem(struct bpf_map *map, void *value)
> +{
> + return __queue_map_get(map, value, false);
> +}
> +
> +/* Called from syscall or from eBPF program */
> +static int stack_map_peek_elem(struct bpf_map *map, void *value)
> +{
> + return __stack_map_get(map, value, false);
> +}
> +
> +/* Called from syscall or from eBPF program */
> +static int queue_map_pop_elem(struct bpf_map *map, void *value)
> +{
> + return __queue_map_get(map, value, true);
> +}
> +
> +/* Called from syscall or from eBPF program */
> +static int stack_map_pop_elem(struct bpf_map *map, void *value)
> +{
> + return __stack_map_get(map, value, true);
> +}
> +
> +/* Called from syscall or from eBPF program */
> +static int queue_stack_map_push_elem(struct bpf_map *map, void *value,
> + u64 flags)
> +{
> + struct bpf_queue_stack *qs = bpf_queue_stack(map);
> + unsigned long irq_flags;
> + int err = 0;
> + void *dst;
> +
> + /* BPF_EXIST is used to force making room for a new element in case the
> + * map is full
> + */
> + bool replace = (flags & BPF_EXIST);
> +
> + /* Check supported flags for queue and stack maps */
> + if (flags & BPF_NOEXIST || flags > BPF_EXIST)
> + return -EINVAL;
> +
> + raw_spin_lock_irqsave(&qs->lock, irq_flags);
> +
> + if (queue_stack_map_is_full(qs)) {
> + if (!replace) {
> + err = -E2BIG;
ENOSPC is probably more accurate ?
> + goto out;
> + }
> + /* advance tail pointer to overwrite oldest element */
'oldest' is ambiguous here.
For queue it's true, but for stack it's the last element.
Since stack is poping from the head, push w/exist flag will keep
overwriting the last element.
Pls explain it more clearly in helper description in bpf.h
> + qs->tail = (qs->tail + 1) & qs->index_mask;
> + qs->count--;
> + }
> +
> + dst = &qs->elements[qs->head * qs->map.value_size];
> + memcpy(dst, value, qs->map.value_size);
> +
> + qs->head = (qs->head + 1) & qs->index_mask;
> + qs->count++;
> +
> +out:
> + raw_spin_unlock_irqrestore(&qs->lock, irq_flags);
> + return err;
> +}
> +
> +/* Called from syscall or from eBPF program */
> +static void *queue_stack_map_lookup_elem(struct bpf_map *map, void *key)
> +{
> + return NULL;
> +}
> +
> +/* Called from syscall or from eBPF program */
> +static int queue_stack_map_update_elem(struct bpf_map *map, void *key,
> + void *value, u64 flags)
> +{
> + return -EINVAL;
> +}
> +
> +/* Called from syscall or from eBPF program */
> +static int queue_stack_map_delete_elem(struct bpf_map *map, void *key)
> +{
> + return -EINVAL;
> +}
> +
> +/* Called from syscall */
> +static int queue_stack_map_get_next_key(struct bpf_map *map, void *key,
> + void *next_key)
> +{
> + return -EINVAL;
> +}
> +
> +const struct bpf_map_ops queue_map_ops = {
> + .map_alloc_check = queue_stack_map_alloc_check,
> + .map_alloc = queue_stack_map_alloc,
> + .map_free = queue_stack_map_free,
> + .map_lookup_elem = queue_stack_map_lookup_elem,
> + .map_update_elem = queue_stack_map_update_elem,
> + .map_delete_elem = queue_stack_map_delete_elem,
> + .map_push_elem = queue_stack_map_push_elem,
> + .map_pop_elem = queue_map_pop_elem,
> + .map_peek_elem = queue_map_peek_elem,
> + .map_get_next_key = queue_stack_map_get_next_key,
> +};
> +
> +const struct bpf_map_ops stack_map_ops = {
> + .map_alloc_check = queue_stack_map_alloc_check,
> + .map_alloc = queue_stack_map_alloc,
> + .map_free = queue_stack_map_free,
> + .map_lookup_elem = queue_stack_map_lookup_elem,
> + .map_update_elem = queue_stack_map_update_elem,
> + .map_delete_elem = queue_stack_map_delete_elem,
> + .map_push_elem = queue_stack_map_push_elem,
> + .map_pop_elem = stack_map_pop_elem,
> + .map_peek_elem = stack_map_peek_elem,
> + .map_get_next_key = queue_stack_map_get_next_key,
> +};
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 50957e243bfb..c46bf2d38be3 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -727,6 +727,9 @@ static int map_lookup_elem(union bpf_attr *attr)
> 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();
> ptr = map->ops->map_lookup_elem(map, key);
> @@ -841,6 +844,9 @@ static int map_update_elem(union bpf_attr *attr)
> /* rcu_read_lock() is not needed */
> err = bpf_fd_reuseport_array_update_elem(map, key, value,
> attr->flags);
> + } else if (map->map_type == BPF_MAP_TYPE_QUEUE ||
> + map->map_type == BPF_MAP_TYPE_STACK) {
> + err = map->ops->map_push_elem(map, value, attr->flags);
> } else {
> rcu_read_lock();
> err = map->ops->map_update_elem(map, key, value, attr->flags);
> @@ -1001,11 +1007,6 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
> goto err_put;
> }
>
> - if (!map->ops->map_lookup_and_delete_elem) {
> - err = -ENOTSUPP;
> - goto err_put;
> - }
> -
> key = __bpf_copy_key(ukey, map->key_size);
> if (IS_ERR(key)) {
> err = PTR_ERR(key);
> @@ -1028,12 +1029,22 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
> */
> preempt_disable();
> __this_cpu_inc(bpf_prog_active);
> - rcu_read_lock();
> - ptr = map->ops->map_lookup_and_delete_elem(map, key);
> - if (ptr)
> - memcpy(value, ptr, value_size);
> - rcu_read_unlock();
> + if (map->map_type == BPF_MAP_TYPE_QUEUE ||
> + map->map_type == BPF_MAP_TYPE_STACK) {
> + err = map->ops->map_pop_elem(map, value);
please clean up the paches, so that patch 4 doesn't immediately
deletes the lines that were introduced in patch 3.
Otherwise what was the point of them in patch 3?
> + } else {
> + if (!map->ops->map_lookup_and_delete_elem) {
> + err = -ENOTSUPP;
> + goto free_value;
> + }
> + rcu_read_lock();
> + ptr = map->ops->map_lookup_and_delete_elem(map, key);
> + if (ptr)
> + memcpy(value, ptr, value_size);
> + rcu_read_unlock();
> err = ptr ? 0 : -ENOENT;
> + }
> +
> __this_cpu_dec(bpf_prog_active);
> preempt_enable();
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 73c81bef6ae8..489667f93061 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2121,7 +2121,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
> }
>
> if (arg_type == ARG_PTR_TO_MAP_KEY ||
> - arg_type == ARG_PTR_TO_MAP_VALUE) {
> + arg_type == ARG_PTR_TO_MAP_VALUE ||
> + arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE) {
> expected_type = PTR_TO_STACK;
> if (!type_is_pkt_pointer(type) && type != PTR_TO_MAP_VALUE &&
> type != expected_type)
> @@ -2191,7 +2192,8 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
> err = check_helper_mem_access(env, regno,
> meta->map_ptr->key_size, false,
> NULL);
> - } else if (arg_type == ARG_PTR_TO_MAP_VALUE) {
> + } else if (arg_type == ARG_PTR_TO_MAP_VALUE ||
> + arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE) {
> /* bpf_map_xxx(..., map_ptr, ..., value) call:
> * check [value, value + map->value_size) validity
> */
> @@ -2200,9 +2202,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
> verbose(env, "invalid map_ptr to access map->value\n");
> return -EACCES;
> }
> + meta->raw_mode = (arg_type == ARG_PTR_TO_UNINIT_MAP_VALUE);
> err = check_helper_mem_access(env, regno,
> meta->map_ptr->value_size, false,
> - NULL);
> + meta);
> } else if (arg_type_is_mem_size(arg_type)) {
> bool zero_size_allowed = (arg_type == ARG_CONST_SIZE_OR_ZERO);
>
> @@ -2676,7 +2679,10 @@ record_func_map(struct bpf_verifier_env *env, struct bpf_call_arg_meta *meta,
> if (func_id != BPF_FUNC_tail_call &&
> func_id != BPF_FUNC_map_lookup_elem &&
> func_id != BPF_FUNC_map_update_elem &&
> - func_id != BPF_FUNC_map_delete_elem)
> + func_id != BPF_FUNC_map_delete_elem &&
> + func_id != BPF_FUNC_map_push_elem &&
> + func_id != BPF_FUNC_map_pop_elem &&
> + func_id != BPF_FUNC_map_peek_elem)
> return 0;
>
> if (meta->map_ptr == NULL) {
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 591c698bc517..40736e0d9cff 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4993,6 +4993,12 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> return &bpf_map_update_elem_proto;
> case BPF_FUNC_map_delete_elem:
> return &bpf_map_delete_elem_proto;
> + case BPF_FUNC_map_push_elem:
> + return &bpf_map_push_elem_proto;
> + case BPF_FUNC_map_pop_elem:
> + return &bpf_map_pop_elem_proto;
> + case BPF_FUNC_map_peek_elem:
> + return &bpf_map_peek_elem_proto;
> case BPF_FUNC_get_prandom_u32:
> return &bpf_get_prandom_u32_proto;
> case BPF_FUNC_get_smp_processor_id:
>
^ permalink raw reply
* Re: [RFC PATCH bpf-next v4 5/7] bpf: restrict use of peek/push/pop
From: Alexei Starovoitov @ 2018-10-04 23:57 UTC (permalink / raw)
To: Mauricio Vasquez B; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev
In-Reply-To: <153867316984.10087.3212168623519526172.stgit@kernel>
On Thu, Oct 04, 2018 at 07:12:49PM +0200, Mauricio Vasquez B wrote:
> Restrict the use of peek, push and pop helpers only to queue and stack
> maps.
>
> Signed-off-by: Mauricio Vasquez B <mauricio.vasquez@polito.it>
> ---
> kernel/bpf/verifier.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 489667f93061..8b1f1b348782 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -2328,6 +2328,13 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
> if (func_id != BPF_FUNC_sk_select_reuseport)
> goto error;
> break;
> + case BPF_MAP_TYPE_QUEUE:
> + case BPF_MAP_TYPE_STACK:
> + if (func_id != BPF_FUNC_map_peek_elem &&
> + func_id != BPF_FUNC_map_pop_elem &&
> + func_id != BPF_FUNC_map_push_elem)
> + goto error;
why this is separate patch?
I think it should be part of previous patch.
> + break;
> default:
> break;
> }
> @@ -2384,6 +2391,13 @@ static int check_map_func_compatibility(struct bpf_verifier_env *env,
> if (map->map_type != BPF_MAP_TYPE_REUSEPORT_SOCKARRAY)
> goto error;
> break;
> + case BPF_FUNC_map_peek_elem:
> + case BPF_FUNC_map_pop_elem:
> + case BPF_FUNC_map_push_elem:
> + if (map->map_type != BPF_MAP_TYPE_QUEUE &&
> + map->map_type != BPF_MAP_TYPE_STACK)
> + goto error;
> + break;
> default:
> break;
> }
>
^ permalink raw reply
* Re: [RFC PATCH bpf-next v4 7/7] selftests/bpf: add test cases for queue and stack maps
From: Alexei Starovoitov @ 2018-10-04 23:59 UTC (permalink / raw)
To: Mauricio Vasquez B; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev
In-Reply-To: <153867318092.10087.8115445755907117246.stgit@kernel>
On Thu, Oct 04, 2018 at 07:13:00PM +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>
overall looks very close. thank for you working on it.
please fix the nits and resubmit without rfc tag.
Thx!
^ permalink raw reply
* [PATCH net-next] net: sched: remove unused helpers
From: Jakub Kicinski @ 2018-10-05 0:07 UTC (permalink / raw)
To: davem; +Cc: jiri, netdev, oss-drivers, Jakub Kicinski
tcf_block_dev() doesn't seem to be used anywhere in the tree.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
include/net/pkt_cls.h | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index 313fe840742d..338ef054bf16 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -65,11 +65,6 @@ static inline struct Qdisc *tcf_block_q(struct tcf_block *block)
return block->q;
}
-static inline struct net_device *tcf_block_dev(struct tcf_block *block)
-{
- return tcf_block_q(block)->dev_queue->dev;
-}
-
void *tcf_block_cb_priv(struct tcf_block_cb *block_cb);
struct tcf_block_cb *tcf_block_cb_lookup(struct tcf_block *block,
tc_setup_cb_t *cb, void *cb_ident);
@@ -122,11 +117,6 @@ static inline struct Qdisc *tcf_block_q(struct tcf_block *block)
return NULL;
}
-static inline struct net_device *tcf_block_dev(struct tcf_block *block)
-{
- return NULL;
-}
-
static inline
int tc_setup_cb_block_register(struct tcf_block *block, tc_setup_cb_t *cb,
void *cb_priv)
--
2.17.1
^ permalink raw reply related
* [PATCH net v2] net: mvpp2: Extract the correct ethtype from the skb for tx csum offload
From: Maxime Chevallier @ 2018-10-05 7:04 UTC (permalink / raw)
To: davem
Cc: Maxime Chevallier, netdev, linux-kernel, Antoine Tenart,
thomas.petazzoni, gregory.clement, miquel.raynal, nadavh, stefanc,
ymarkman, mw
When offloading the L3 and L4 csum computation on TX, we need to extract
the l3_proto from the ethtype, independently of the presence of a vlan
tag.
The actual driver uses skb->protocol as-is, resulting in packets with
the wrong L4 checksum being sent when there's a vlan tag in the packet
header and checksum offloading is enabled.
This commit makes use of vlan_protocol_get() to get the correct ethtype
regardless the presence of a vlan tag.
Fixes: 3f518509dedc ("ethernet: Add new driver for Marvell Armada 375 network unit")
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V2: Use htons on values that can be swapped at compile-time, following
Yan's comment. Fix the "Fixes" tag and a typo, following Sergei's comment.
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 38cc01beea79..a74002b43b51 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -1725,7 +1725,7 @@ static void mvpp2_txq_desc_put(struct mvpp2_tx_queue *txq)
}
/* Set Tx descriptors fields relevant for CSUM calculation */
-static u32 mvpp2_txq_desc_csum(int l3_offs, int l3_proto,
+static u32 mvpp2_txq_desc_csum(int l3_offs, __be16 l3_proto,
int ip_hdr_len, int l4_proto)
{
u32 command;
@@ -2600,14 +2600,15 @@ static u32 mvpp2_skb_tx_csum(struct mvpp2_port *port, struct sk_buff *skb)
if (skb->ip_summed == CHECKSUM_PARTIAL) {
int ip_hdr_len = 0;
u8 l4_proto;
+ __be16 l3_proto = vlan_get_protocol(skb);
- if (skb->protocol == htons(ETH_P_IP)) {
+ if (l3_proto == htons(ETH_P_IP)) {
struct iphdr *ip4h = ip_hdr(skb);
/* Calculate IPv4 checksum and L4 checksum */
ip_hdr_len = ip4h->ihl;
l4_proto = ip4h->protocol;
- } else if (skb->protocol == htons(ETH_P_IPV6)) {
+ } else if (l3_proto == htons(ETH_P_IPV6)) {
struct ipv6hdr *ip6h = ipv6_hdr(skb);
/* Read l4_protocol from one of IPv6 extra headers */
@@ -2619,7 +2620,7 @@ static u32 mvpp2_skb_tx_csum(struct mvpp2_port *port, struct sk_buff *skb)
}
return mvpp2_txq_desc_csum(skb_network_offset(skb),
- skb->protocol, ip_hdr_len, l4_proto);
+ l3_proto, ip_hdr_len, l4_proto);
}
return MVPP2_TXD_L4_CSUM_NOT | MVPP2_TXD_IP_CSUM_DISABLE;
--
2.11.0
^ permalink raw reply related
* [PATCH iproute2-next] tc: jsonify output of q_fifo
From: Jakub Kicinski @ 2018-10-05 0:08 UTC (permalink / raw)
To: dsahern; +Cc: netdev, oss-drivers, Jakub Kicinski
Print limits correctly in JSON context.
Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
---
tc/q_fifo.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/tc/q_fifo.c b/tc/q_fifo.c
index cb86a404d4de..61493fbbc5bc 100644
--- a/tc/q_fifo.c
+++ b/tc/q_fifo.c
@@ -69,9 +69,12 @@ static int fifo_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
qopt = RTA_DATA(opt);
if (strcmp(qu->id, "bfifo") == 0) {
SPRINT_BUF(b1);
- fprintf(f, "limit %s", sprint_size(qopt->limit, b1));
- } else
- fprintf(f, "limit %up", qopt->limit);
+ print_uint(PRINT_JSON, "limit", NULL, qopt->limit);
+ print_string(PRINT_FP, NULL, "limit %s",
+ sprint_size(qopt->limit, b1));
+ } else {
+ print_uint(PRINT_ANY, "limit", "limit %up", qopt->limit);
+ }
return 0;
}
--
2.17.1
^ permalink raw reply related
* Re: [RFC v2 bpf-next 2/5] bpf: return EOPNOTSUPP when map lookup isn't supported
From: Alexei Starovoitov @ 2018-10-05 0:10 UTC (permalink / raw)
To: Prashant Bhole
Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
David S . Miller, Quentin Monnet, netdev
In-Reply-To: <20181002053519.8000-3-bhole_prashant_q7@lab.ntt.co.jp>
On Tue, Oct 02, 2018 at 02:35:16PM +0900, Prashant Bhole wrote:
> Return ERR_PTR(-EOPNOTSUPP) from map_lookup_elem() methods of below
> map types:
> - BPF_MAP_TYPE_PROG_ARRAY
> - BPF_MAP_TYPE_STACK_TRACE
> - BPF_MAP_TYPE_XSKMAP
> - BPF_MAP_TYPE_SOCKMAP/BPF_MAP_TYPE_SOCKHASH
>
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> ---
> kernel/bpf/arraymap.c | 2 +-
> kernel/bpf/sockmap.c | 2 +-
> kernel/bpf/stackmap.c | 2 +-
> kernel/bpf/xskmap.c | 2 +-
> 4 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> index dded84cbe814..24583da9ffd1 100644
> --- a/kernel/bpf/arraymap.c
> +++ b/kernel/bpf/arraymap.c
> @@ -449,7 +449,7 @@ static void fd_array_map_free(struct bpf_map *map)
>
> static void *fd_array_map_lookup_elem(struct bpf_map *map, void *key)
> {
> - return NULL;
> + return ERR_PTR(-EOPNOTSUPP);
last time we discussed that the verifier should make sure that
lookup is not called from bpf program for these map types.
I'd like to see test cases in test_verifier.c for these map types
to make sure we don't introduce crashes.
^ permalink raw reply
* Re: [PATCH iproute2-next] tc: jsonify output of q_fifo
From: Stephen Hemminger @ 2018-10-05 0:10 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: dsahern, netdev, oss-drivers
In-Reply-To: <20181005000834.24364-1-jakub.kicinski@netronome.com>
On Thu, 4 Oct 2018 17:08:34 -0700
Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
> Print limits correctly in JSON context.
>
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> ---
> tc/q_fifo.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/tc/q_fifo.c b/tc/q_fifo.c
> index cb86a404d4de..61493fbbc5bc 100644
> --- a/tc/q_fifo.c
> +++ b/tc/q_fifo.c
> @@ -69,9 +69,12 @@ static int fifo_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
> qopt = RTA_DATA(opt);
> if (strcmp(qu->id, "bfifo") == 0) {
> SPRINT_BUF(b1);
> - fprintf(f, "limit %s", sprint_size(qopt->limit, b1));
> - } else
> - fprintf(f, "limit %up", qopt->limit);
> + print_uint(PRINT_JSON, "limit", NULL, qopt->limit);
> + print_string(PRINT_FP, NULL, "limit %s",
> + sprint_size(qopt->limit, b1));
> + } else {
> + print_uint(PRINT_ANY, "limit", "limit %up", qopt->limit);
> + }
> return 0;
> }
>
This can go to current not net-next, since it is a bug fix really.
^ permalink raw reply
* Re: [RFC v2 bpf-next 2/5] bpf: return EOPNOTSUPP when map lookup isn't supported
From: Prashant Bhole @ 2018-10-05 0:16 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
David S . Miller, Quentin Monnet, netdev
In-Reply-To: <20181005001043.btwv2k7fbp7gfcbv@ast-mbp.dhcp.thefacebook.com>
On 10/5/2018 9:10 AM, Alexei Starovoitov wrote:
> On Tue, Oct 02, 2018 at 02:35:16PM +0900, Prashant Bhole wrote:
>> Return ERR_PTR(-EOPNOTSUPP) from map_lookup_elem() methods of below
>> map types:
>> - BPF_MAP_TYPE_PROG_ARRAY
>> - BPF_MAP_TYPE_STACK_TRACE
>> - BPF_MAP_TYPE_XSKMAP
>> - BPF_MAP_TYPE_SOCKMAP/BPF_MAP_TYPE_SOCKHASH
>>
>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>> ---
>> kernel/bpf/arraymap.c | 2 +-
>> kernel/bpf/sockmap.c | 2 +-
>> kernel/bpf/stackmap.c | 2 +-
>> kernel/bpf/xskmap.c | 2 +-
>> 4 files changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
>> index dded84cbe814..24583da9ffd1 100644
>> --- a/kernel/bpf/arraymap.c
>> +++ b/kernel/bpf/arraymap.c
>> @@ -449,7 +449,7 @@ static void fd_array_map_free(struct bpf_map *map)
>>
>> static void *fd_array_map_lookup_elem(struct bpf_map *map, void *key)
>> {
>> - return NULL;
>> + return ERR_PTR(-EOPNOTSUPP);
>
> last time we discussed that the verifier should make sure that
> lookup is not called from bpf program for these map types.
> I'd like to see test cases in test_verifier.c for these map types
> to make sure we don't introduce crashes.
Hi Alexei,
Patch 05/05 adds such tests in test_verifier.c. Please review those
changes. Thank you.
-Prashant
^ permalink raw reply
* Re: [PATCH iproute2-next] tc: jsonify output of q_fifo
From: Jakub Kicinski @ 2018-10-05 0:31 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: dsahern, netdev, oss-drivers
In-Reply-To: <20181004171058.0bd85fdb@xeon-e3>
On Thu, 4 Oct 2018 17:10:58 -0700, Stephen Hemminger wrote:
> On Thu, 4 Oct 2018 17:08:34 -0700
> Jakub Kicinski <jakub.kicinski@netronome.com> wrote:
>
> > Print limits correctly in JSON context.
> >
> > Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> > ---
> > tc/q_fifo.c | 9 ++++++---
> > 1 file changed, 6 insertions(+), 3 deletions(-)
> >
> > diff --git a/tc/q_fifo.c b/tc/q_fifo.c
> > index cb86a404d4de..61493fbbc5bc 100644
> > --- a/tc/q_fifo.c
> > +++ b/tc/q_fifo.c
> > @@ -69,9 +69,12 @@ static int fifo_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
> > qopt = RTA_DATA(opt);
> > if (strcmp(qu->id, "bfifo") == 0) {
> > SPRINT_BUF(b1);
> > - fprintf(f, "limit %s", sprint_size(qopt->limit, b1));
> > - } else
> > - fprintf(f, "limit %up", qopt->limit);
> > + print_uint(PRINT_JSON, "limit", NULL, qopt->limit);
> > + print_string(PRINT_FP, NULL, "limit %s",
> > + sprint_size(qopt->limit, b1));
> > + } else {
> > + print_uint(PRINT_ANY, "limit", "limit %up", qopt->limit);
> > + }
> > return 0;
> > }
> >
>
> This can go to current not net-next, since it is a bug fix really.
No preference on my side, should I resend?
(Apologies for not CCing you!)
^ permalink raw reply
* Re: [PATCH] net: wireless: iwlegacy: Fix possible data races in il4965_send_rxon_assoc()
From: Stanislaw Gruszka @ 2018-10-05 7:54 UTC (permalink / raw)
To: Jia-Ju Bai; +Cc: kvalo, davem, linux-wireless, netdev, linux-kernel
In-Reply-To: <988494cb-c121-697e-b502-ea4e7c601f47@gmail.com>
On Thu, Oct 04, 2018 at 04:52:19PM +0800, Jia-Ju Bai wrote:
> On 2018/10/4 15:59, Stanislaw Gruszka wrote:
> >On Wed, Oct 03, 2018 at 10:07:45PM +0800, Jia-Ju Bai wrote:
> >>These possible races are detected by a runtime testing.
> >>To fix these races, the mutex lock is used in il4965_send_rxon_assoc()
> >>to protect the data.
> >Really ? I'm surprised by that, see below.
>
> My runtime testing shows that il4965_send_rxon_assoc() and
> il4965_configure_filter() are concurrently executed.
> But after seeing your reply, I need to carefully check whether my
> runtime testing is right, because I think you are right.
> In fact, I only monitored the iwl4965 driver, but did not monitor
> the iwlegacy driver, so I will do the testing again with monitoring
> the lwlegacy driver.
<snip>
> >So I wonder how this patch did not cause the deadlock ?
>
> Oh, sorry, anyway, my patch will cause double locks...
So how those runtime test were performend such you didn't
notice this ?
> >Anyway what can be done is adding:
> >
> >lockdep_assert_held(&il->mutex);
> >
> >il4965_commit_rxon() to check if we hold the mutex.
>
> I agree.
Care to post a patch ?
Thanks
Stanislaw
^ permalink raw reply
* Re: [PATCH 08/16] zd1211rw: Replace spin_is_locked() with lockdep
From: Kalle Valo @ 2018-10-05 8:35 UTC (permalink / raw)
To: Lance Roy
Cc: linux-kernel, Paul E. McKenney, Lance Roy, Daniel Drake,
Ulrich Kunitz, David S. Miller, linux-wireless, netdev
In-Reply-To: <20181003053902.6910-9-ldr709@gmail.com>
Lance Roy <ldr709@gmail.com> wrote:
> lockdep_assert_held() is better suited to checking locking requirements,
> since it won't get confused when someone else holds the lock. This is
> also a step towards possibly removing spin_is_locked().
>
> Signed-off-by: Lance Roy <ldr709@gmail.com>
> Cc: Daniel Drake <dsd@gentoo.org>
> Cc: Ulrich Kunitz <kune@deine-taler.de>
> Cc: Kalle Valo <kvalo@codeaurora.org>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: <linux-wireless@vger.kernel.org>
> Cc: <netdev@vger.kernel.org>
Patch applied to wireless-drivers-next.git, thanks.
209e957b467b zd1211rw: Replace spin_is_locked() with lockdep
--
https://patchwork.kernel.org/patch/10624325/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [RFC v2 bpf-next 2/5] bpf: return EOPNOTSUPP when map lookup isn't supported
From: Alexei Starovoitov @ 2018-10-05 1:45 UTC (permalink / raw)
To: Prashant Bhole
Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
David S . Miller, Quentin Monnet, netdev
In-Reply-To: <14f9b4ae-b1f8-d50c-fdc8-d357199cabc0@lab.ntt.co.jp>
On Fri, Oct 05, 2018 at 09:16:04AM +0900, Prashant Bhole wrote:
>
>
> On 10/5/2018 9:10 AM, Alexei Starovoitov wrote:
> > On Tue, Oct 02, 2018 at 02:35:16PM +0900, Prashant Bhole wrote:
> > > Return ERR_PTR(-EOPNOTSUPP) from map_lookup_elem() methods of below
> > > map types:
> > > - BPF_MAP_TYPE_PROG_ARRAY
> > > - BPF_MAP_TYPE_STACK_TRACE
> > > - BPF_MAP_TYPE_XSKMAP
> > > - BPF_MAP_TYPE_SOCKMAP/BPF_MAP_TYPE_SOCKHASH
> > >
> > > Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> > > ---
> > > kernel/bpf/arraymap.c | 2 +-
> > > kernel/bpf/sockmap.c | 2 +-
> > > kernel/bpf/stackmap.c | 2 +-
> > > kernel/bpf/xskmap.c | 2 +-
> > > 4 files changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/kernel/bpf/arraymap.c b/kernel/bpf/arraymap.c
> > > index dded84cbe814..24583da9ffd1 100644
> > > --- a/kernel/bpf/arraymap.c
> > > +++ b/kernel/bpf/arraymap.c
> > > @@ -449,7 +449,7 @@ static void fd_array_map_free(struct bpf_map *map)
> > > static void *fd_array_map_lookup_elem(struct bpf_map *map, void *key)
> > > {
> > > - return NULL;
> > > + return ERR_PTR(-EOPNOTSUPP);
> >
> > last time we discussed that the verifier should make sure that
> > lookup is not called from bpf program for these map types.
> > I'd like to see test cases in test_verifier.c for these map types
> > to make sure we don't introduce crashes.
>
> Hi Alexei,
> Patch 05/05 adds such tests in test_verifier.c. Please review those changes.
ahh. missed it. sorry about that. looking...
^ permalink raw reply
* Re: [RFC v2 bpf-next 5/5] selftests/bpf: verifier, check bpf_map_lookup_elem access in bpf prog
From: Alexei Starovoitov @ 2018-10-05 1:51 UTC (permalink / raw)
To: Prashant Bhole
Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
David S . Miller, Quentin Monnet, netdev
In-Reply-To: <20181002053519.8000-6-bhole_prashant_q7@lab.ntt.co.jp>
On Tue, Oct 02, 2018 at 02:35:19PM +0900, Prashant Bhole wrote:
> map_lookup_elem isn't supported by certain map types like:
> - BPF_MAP_TYPE_PROG_ARRAY
> - BPF_MAP_TYPE_STACK_TRACE
> - BPF_MAP_TYPE_XSKMAP
> - BPF_MAP_TYPE_SOCKMAP/BPF_MAP_TYPE_SOCKHASH
> Let's add verfier tests to check whether verifier prevents
> bpf_map_lookup_elem call on above programs from bpf program.
>
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> ---
> tools/testing/selftests/bpf/test_verifier.c | 121 +++++++++++++++++++-
> 1 file changed, 120 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index c7d25f23baf9..afa7e67f66e4 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -47,7 +47,7 @@
>
> #define MAX_INSNS BPF_MAXINSNS
> #define MAX_FIXUPS 8
> -#define MAX_NR_MAPS 8
> +#define MAX_NR_MAPS 13
> #define POINTER_VALUE 0xcafe4all
> #define TEST_DATA_LEN 64
>
> @@ -64,6 +64,10 @@ struct bpf_test {
> int fixup_map2[MAX_FIXUPS];
> int fixup_map3[MAX_FIXUPS];
> int fixup_map4[MAX_FIXUPS];
> + int fixup_map5[MAX_FIXUPS];
> + int fixup_map6[MAX_FIXUPS];
> + int fixup_map7[MAX_FIXUPS];
> + int fixup_map8[MAX_FIXUPS];
> int fixup_prog1[MAX_FIXUPS];
> int fixup_prog2[MAX_FIXUPS];
> int fixup_map_in_map[MAX_FIXUPS];
> @@ -4391,6 +4395,85 @@ static struct bpf_test tests[] = {
> .errstr = "invalid access to packet",
> .prog_type = BPF_PROG_TYPE_SCHED_CLS,
> },
> + {
> + "prevent map lookup in sockmap",
> + .insns = {
> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> + BPF_LD_MAP_FD(BPF_REG_1, 0),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_map_lookup_elem),
> + BPF_EXIT_INSN(),
> + },
> + .fixup_map5 = { 3 },
> + .result = REJECT,
> + .errstr = "cannot pass map_type 15 into func bpf_map_lookup_elem",
> + .prog_type = BPF_PROG_TYPE_SOCK_OPS,
> + },
> + {
> + "prevent map lookup in sockhash",
> + .insns = {
> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> + BPF_LD_MAP_FD(BPF_REG_1, 0),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_map_lookup_elem),
> + BPF_EXIT_INSN(),
> + },
> + .fixup_map6 = { 3 },
> + .result = REJECT,
> + .errstr = "cannot pass map_type 18 into func bpf_map_lookup_elem",
> + .prog_type = BPF_PROG_TYPE_SOCK_OPS,
> + },
> + {
> + "prevent map lookup in xskmap",
> + .insns = {
> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> + BPF_LD_MAP_FD(BPF_REG_1, 0),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_map_lookup_elem),
> + BPF_EXIT_INSN(),
> + },
> + .fixup_map7 = { 3 },
> + .result = REJECT,
> + .errstr = "cannot pass map_type 17 into func bpf_map_lookup_elem",
> + .prog_type = BPF_PROG_TYPE_XDP,
> + },
> + {
> + "prevent map lookup in stack trace",
> + .insns = {
> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> + BPF_LD_MAP_FD(BPF_REG_1, 0),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_map_lookup_elem),
> + BPF_EXIT_INSN(),
> + },
> + .fixup_map8 = { 3 },
> + .result = REJECT,
> + .errstr = "cannot pass map_type 7 into func bpf_map_lookup_elem",
> + .prog_type = BPF_PROG_TYPE_PERF_EVENT,
> + },
> + {
> + "prevent map lookup in prog array",
> + .insns = {
> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
> + BPF_LD_MAP_FD(BPF_REG_1, 0),
> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
> + BPF_FUNC_map_lookup_elem),
> + BPF_EXIT_INSN(),
> + },
> + .fixup_prog2 = { 3 },
> + .result = REJECT,
> + .errstr = "cannot pass map_type 3 into func bpf_map_lookup_elem",
excellent tests. exactly what I was hoping to see.
> + },
> {
> "valid map access into an array with a constant",
> .insns = {
> @@ -12755,6 +12838,10 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
> int *fixup_map2 = test->fixup_map2;
> int *fixup_map3 = test->fixup_map3;
> int *fixup_map4 = test->fixup_map4;
> + int *fixup_map5 = test->fixup_map5;
> + int *fixup_map6 = test->fixup_map6;
> + int *fixup_map7 = test->fixup_map7;
> + int *fixup_map8 = test->fixup_map8;
> int *fixup_prog1 = test->fixup_prog1;
> int *fixup_prog2 = test->fixup_prog2;
> int *fixup_map_in_map = test->fixup_map_in_map;
> @@ -12843,6 +12930,38 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
> fixup_percpu_cgroup_storage++;
> } while (*fixup_percpu_cgroup_storage);
> }
> + if (*fixup_map5) {
> + map_fds[9] = create_map(BPF_MAP_TYPE_SOCKMAP, sizeof(int),
> + sizeof(int), 1);
> + do {
> + prog[*fixup_map5].imm = map_fds[9];
> + fixup_map5++;
> + } while (*fixup_map5);
> + }
> + if (*fixup_map6) {
> + map_fds[10] = create_map(BPF_MAP_TYPE_SOCKHASH, sizeof(int),
> + sizeof(int), 1);
> + do {
> + prog[*fixup_map6].imm = map_fds[10];
> + fixup_map6++;
> + } while (*fixup_map6);
> + }
> + if (*fixup_map7) {
> + map_fds[11] = create_map(BPF_MAP_TYPE_XSKMAP, sizeof(int),
> + sizeof(int), 1);
> + do {
> + prog[*fixup_map7].imm = map_fds[11];
> + fixup_map7++;
> + } while (*fixup_map7);
> + }
> + if (*fixup_map8) {
> + map_fds[12] = create_map(BPF_MAP_TYPE_STACK_TRACE, sizeof(u32),
> + sizeof(u64), 1);
> + do {
> + prog[*fixup_map8].imm = map_fds[12];
> + fixup_map8++;
> + } while (fixup_map8);
I understand that you're following the existing naming convention
with fixup_mapN, but it was ugly before and these 4 additions
make it completely unreadable.
Could you please refactor the old names:
fixup_map1 -> fixup_map_hash_8b
fixup_map2 -> fixup_map_hash_48b (pls double check my math)
fixup_map3 -> fixup_map_hash_16b
fixup_map4 -> fixup_map_array_48b
then your new diff will use
fixup_map5 -> fixup_map_sockmap
fixup_map6 -> fixup_map_sockhash
...
and please drop rfc tag from the next respin.
Thanks!
^ permalink raw reply
* Re: [RFC v2 bpf-next 5/5] selftests/bpf: verifier, check bpf_map_lookup_elem access in bpf prog
From: Prashant Bhole @ 2018-10-05 2:07 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, Daniel Borkmann, Jakub Kicinski,
David S . Miller, Quentin Monnet, netdev
In-Reply-To: <20181005015144.ct3vsjol53ndveeh@ast-mbp.dhcp.thefacebook.com>
On 10/5/2018 10:51 AM, Alexei Starovoitov wrote:
> On Tue, Oct 02, 2018 at 02:35:19PM +0900, Prashant Bhole wrote:
>> map_lookup_elem isn't supported by certain map types like:
>> - BPF_MAP_TYPE_PROG_ARRAY
>> - BPF_MAP_TYPE_STACK_TRACE
>> - BPF_MAP_TYPE_XSKMAP
>> - BPF_MAP_TYPE_SOCKMAP/BPF_MAP_TYPE_SOCKHASH
>> Let's add verfier tests to check whether verifier prevents
>> bpf_map_lookup_elem call on above programs from bpf program.
>>
>> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
>> ---
>> tools/testing/selftests/bpf/test_verifier.c | 121 +++++++++++++++++++-
>> 1 file changed, 120 insertions(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
>> index c7d25f23baf9..afa7e67f66e4 100644
>> --- a/tools/testing/selftests/bpf/test_verifier.c
>> +++ b/tools/testing/selftests/bpf/test_verifier.c
>> @@ -47,7 +47,7 @@
>>
>> #define MAX_INSNS BPF_MAXINSNS
>> #define MAX_FIXUPS 8
>> -#define MAX_NR_MAPS 8
>> +#define MAX_NR_MAPS 13
>> #define POINTER_VALUE 0xcafe4all
>> #define TEST_DATA_LEN 64
>>
>> @@ -64,6 +64,10 @@ struct bpf_test {
>> int fixup_map2[MAX_FIXUPS];
>> int fixup_map3[MAX_FIXUPS];
>> int fixup_map4[MAX_FIXUPS];
>> + int fixup_map5[MAX_FIXUPS];
>> + int fixup_map6[MAX_FIXUPS];
>> + int fixup_map7[MAX_FIXUPS];
>> + int fixup_map8[MAX_FIXUPS];
>> int fixup_prog1[MAX_FIXUPS];
>> int fixup_prog2[MAX_FIXUPS];
>> int fixup_map_in_map[MAX_FIXUPS];
>> @@ -4391,6 +4395,85 @@ static struct bpf_test tests[] = {
>> .errstr = "invalid access to packet",
>> .prog_type = BPF_PROG_TYPE_SCHED_CLS,
>> },
>> + {
>> + "prevent map lookup in sockmap",
>> + .insns = {
>> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
>> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
>> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
>> + BPF_LD_MAP_FD(BPF_REG_1, 0),
>> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
>> + BPF_FUNC_map_lookup_elem),
>> + BPF_EXIT_INSN(),
>> + },
>> + .fixup_map5 = { 3 },
>> + .result = REJECT,
>> + .errstr = "cannot pass map_type 15 into func bpf_map_lookup_elem",
>> + .prog_type = BPF_PROG_TYPE_SOCK_OPS,
>> + },
>> + {
>> + "prevent map lookup in sockhash",
>> + .insns = {
>> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
>> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
>> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
>> + BPF_LD_MAP_FD(BPF_REG_1, 0),
>> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
>> + BPF_FUNC_map_lookup_elem),
>> + BPF_EXIT_INSN(),
>> + },
>> + .fixup_map6 = { 3 },
>> + .result = REJECT,
>> + .errstr = "cannot pass map_type 18 into func bpf_map_lookup_elem",
>> + .prog_type = BPF_PROG_TYPE_SOCK_OPS,
>> + },
>> + {
>> + "prevent map lookup in xskmap",
>> + .insns = {
>> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
>> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
>> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
>> + BPF_LD_MAP_FD(BPF_REG_1, 0),
>> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
>> + BPF_FUNC_map_lookup_elem),
>> + BPF_EXIT_INSN(),
>> + },
>> + .fixup_map7 = { 3 },
>> + .result = REJECT,
>> + .errstr = "cannot pass map_type 17 into func bpf_map_lookup_elem",
>> + .prog_type = BPF_PROG_TYPE_XDP,
>> + },
>> + {
>> + "prevent map lookup in stack trace",
>> + .insns = {
>> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
>> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
>> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
>> + BPF_LD_MAP_FD(BPF_REG_1, 0),
>> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
>> + BPF_FUNC_map_lookup_elem),
>> + BPF_EXIT_INSN(),
>> + },
>> + .fixup_map8 = { 3 },
>> + .result = REJECT,
>> + .errstr = "cannot pass map_type 7 into func bpf_map_lookup_elem",
>> + .prog_type = BPF_PROG_TYPE_PERF_EVENT,
>> + },
>> + {
>> + "prevent map lookup in prog array",
>> + .insns = {
>> + BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 0),
>> + BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
>> + BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
>> + BPF_LD_MAP_FD(BPF_REG_1, 0),
>> + BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
>> + BPF_FUNC_map_lookup_elem),
>> + BPF_EXIT_INSN(),
>> + },
>> + .fixup_prog2 = { 3 },
>> + .result = REJECT,
>> + .errstr = "cannot pass map_type 3 into func bpf_map_lookup_elem",
>
> excellent tests. exactly what I was hoping to see.
>
>> + },
>> {
>> "valid map access into an array with a constant",
>> .insns = {
>> @@ -12755,6 +12838,10 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
>> int *fixup_map2 = test->fixup_map2;
>> int *fixup_map3 = test->fixup_map3;
>> int *fixup_map4 = test->fixup_map4;
>> + int *fixup_map5 = test->fixup_map5;
>> + int *fixup_map6 = test->fixup_map6;
>> + int *fixup_map7 = test->fixup_map7;
>> + int *fixup_map8 = test->fixup_map8;
>> int *fixup_prog1 = test->fixup_prog1;
>> int *fixup_prog2 = test->fixup_prog2;
>> int *fixup_map_in_map = test->fixup_map_in_map;
>> @@ -12843,6 +12930,38 @@ static void do_test_fixup(struct bpf_test *test, struct bpf_insn *prog,
>> fixup_percpu_cgroup_storage++;
>> } while (*fixup_percpu_cgroup_storage);
>> }
>> + if (*fixup_map5) {
>> + map_fds[9] = create_map(BPF_MAP_TYPE_SOCKMAP, sizeof(int),
>> + sizeof(int), 1);
>> + do {
>> + prog[*fixup_map5].imm = map_fds[9];
>> + fixup_map5++;
>> + } while (*fixup_map5);
>> + }
>> + if (*fixup_map6) {
>> + map_fds[10] = create_map(BPF_MAP_TYPE_SOCKHASH, sizeof(int),
>> + sizeof(int), 1);
>> + do {
>> + prog[*fixup_map6].imm = map_fds[10];
>> + fixup_map6++;
>> + } while (*fixup_map6);
>> + }
>> + if (*fixup_map7) {
>> + map_fds[11] = create_map(BPF_MAP_TYPE_XSKMAP, sizeof(int),
>> + sizeof(int), 1);
>> + do {
>> + prog[*fixup_map7].imm = map_fds[11];
>> + fixup_map7++;
>> + } while (*fixup_map7);
>> + }
>> + if (*fixup_map8) {
>> + map_fds[12] = create_map(BPF_MAP_TYPE_STACK_TRACE, sizeof(u32),
>> + sizeof(u64), 1);
>> + do {
>> + prog[*fixup_map8].imm = map_fds[12];
>> + fixup_map8++;
>> + } while (fixup_map8);
>
> I understand that you're following the existing naming convention
> with fixup_mapN, but it was ugly before and these 4 additions
> make it completely unreadable.
>
> Could you please refactor the old names:
> fixup_map1 -> fixup_map_hash_8b
> fixup_map2 -> fixup_map_hash_48b (pls double check my math)
> fixup_map3 -> fixup_map_hash_16b
> fixup_map4 -> fixup_map_array_48b
>
> then your new diff will use
> fixup_map5 -> fixup_map_sockmap
> fixup_map6 -> fixup_map_sockhash
> ...
>
> and please drop rfc tag from the next respin.
> Thanks!
>
Thanks for reviewing. I will fix the naming convention in the next patch
series.
-Prashant
^ permalink raw reply
* Re: [PATCH] wil6210: fix debugfs_simple_attr.cocci warnings
From: Kalle Valo @ 2018-10-05 11:04 UTC (permalink / raw)
To: YueHaibing
Cc: Maya Erez, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
wil6210-Rm6X0d1/PG5y9aJCnZT0Uw,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1538737646-118337-1-git-send-email-yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
YueHaibing <yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> writes:
> Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
> for debugfs files.
>
> Semantic patch information:
> Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
> imposes some significant overhead as compared to
> DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
>
> Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
Just out of curiosity, what kind of overhead are we talking about here?
--
Kalle Valo
^ permalink raw reply
* [PATCH] wil6210: fix debugfs_simple_attr.cocci warnings
From: YueHaibing @ 2018-10-05 11:07 UTC (permalink / raw)
To: Maya Erez, Kalle Valo
Cc: YueHaibing, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
wil6210-Rm6X0d1/PG5y9aJCnZT0Uw,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA
Use DEFINE_DEBUGFS_ATTRIBUTE rather than DEFINE_SIMPLE_ATTRIBUTE
for debugfs files.
Semantic patch information:
Rationale: DEFINE_SIMPLE_ATTRIBUTE + debugfs_create_file()
imposes some significant overhead as compared to
DEFINE_DEBUGFS_ATTRIBUTE + debugfs_create_file_unsafe().
Generated by: scripts/coccinelle/api/debugfs/debugfs_simple_attr.cocci
Signed-off-by: YueHaibing <yuehaibing-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
drivers/net/wireless/ath/wil6210/debugfs.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index 66ffae2..aa50813 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -416,8 +416,8 @@ static int wil_debugfs_iomem_x32_get(void *data, u64 *val)
return 0;
}
-DEFINE_SIMPLE_ATTRIBUTE(fops_iomem_x32, wil_debugfs_iomem_x32_get,
- wil_debugfs_iomem_x32_set, "0x%08llx\n");
+DEFINE_DEBUGFS_ATTRIBUTE(fops_iomem_x32, wil_debugfs_iomem_x32_get,
+ wil_debugfs_iomem_x32_set, "0x%08llx\n");
static struct dentry *wil_debugfs_create_iomem_x32(const char *name,
umode_t mode,
@@ -432,7 +432,8 @@ static struct dentry *wil_debugfs_create_iomem_x32(const char *name,
data->wil = wil;
data->offset = value;
- file = debugfs_create_file(name, mode, parent, data, &fops_iomem_x32);
+ file = debugfs_create_file_unsafe(name, mode, parent, data,
+ &fops_iomem_x32);
if (!IS_ERR_OR_NULL(file))
wil->dbg_data.iomem_data_count++;
@@ -451,14 +452,15 @@ static int wil_debugfs_ulong_get(void *data, u64 *val)
return 0;
}
-DEFINE_SIMPLE_ATTRIBUTE(wil_fops_ulong, wil_debugfs_ulong_get,
- wil_debugfs_ulong_set, "0x%llx\n");
+DEFINE_DEBUGFS_ATTRIBUTE(wil_fops_ulong, wil_debugfs_ulong_get,
+ wil_debugfs_ulong_set, "0x%llx\n");
static struct dentry *wil_debugfs_create_ulong(const char *name, umode_t mode,
struct dentry *parent,
ulong *value)
{
- return debugfs_create_file(name, mode, parent, value, &wil_fops_ulong);
+ return debugfs_create_file_unsafe(name, mode, parent, value,
+ &wil_fops_ulong);
}
/**
^ permalink raw reply related
* Re:заявка за оферта
From: Dimitar Hristov @ 2018-10-05 3:46 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 370 bytes --]
Добро утро,
Дайте ни най-добрите цени за тази поръчка. Нуждаем се от 1000 бр., Ако
имаме добри цени. Някои образци също са приложени
Димитър Христов
NugradTradings Eood
София, България
37214 Pojate
Тел. +35137805277
Моб. +351648670356
[-- Attachment #2: SKG_Поръчайте мостри_6744510.r11 --]
[-- Type: application/x-rar, Size: 184685 bytes --]
^ permalink raw reply
* [PATCH net 0/4] bnxt_en: Misc. bug fixes.
From: Michael Chan @ 2018-10-05 4:25 UTC (permalink / raw)
To: davem; +Cc: netdev
4 small bug fixes related to setting firmware message enables bits, possible
memory leak when probe fails, and ring accouting when RDMA driver is loaded.
Please queue these for -stable as well. Thanks.
Michael Chan (1):
bnxt_en: Fix VNIC reservations on the PF.
Vasundhara Volam (2):
bnxt_en: Fix enables field in HWRM_QUEUE_COS2BW_CFG request
bnxt_en: get the reduced max_irqs by the ones used by RDMA
Venkat Duvvuru (1):
bnxt_en: free hwrm resources, if driver probe fails.
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 14 ++++++++------
drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c | 6 +++---
2 files changed, 11 insertions(+), 9 deletions(-)
--
2.5.1
^ permalink raw reply
* [PATCH net 1/4] bnxt_en: Fix VNIC reservations on the PF.
From: Michael Chan @ 2018-10-05 4:26 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1538713563-13878-1-git-send-email-michael.chan@broadcom.com>
The enables bit for VNIC was set wrong when calling the HWRM_FUNC_CFG
firmware call to reserve VNICs. This has the effect that the firmware
will keep a large number of VNICs for the PF, and having very few for
VFs. DPDK driver running on the VFs, which requires more VNICs, may not
work properly as a result.
Fixes: 674f50a5b026 ("bnxt_en: Implement new method to reserve rings.")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 0478e56..2564a92 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -4650,7 +4650,7 @@ __bnxt_hwrm_reserve_pf_rings(struct bnxt *bp, struct hwrm_func_cfg_input *req,
FUNC_CFG_REQ_ENABLES_NUM_STAT_CTXS : 0;
enables |= ring_grps ?
FUNC_CFG_REQ_ENABLES_NUM_HW_RING_GRPS : 0;
- enables |= vnics ? FUNC_VF_CFG_REQ_ENABLES_NUM_VNICS : 0;
+ enables |= vnics ? FUNC_CFG_REQ_ENABLES_NUM_VNICS : 0;
req->num_rx_rings = cpu_to_le16(rx_rings);
req->num_hw_ring_grps = cpu_to_le16(ring_grps);
--
2.5.1
^ permalink raw reply related
* [PATCH net 2/4] bnxt_en: Fix enables field in HWRM_QUEUE_COS2BW_CFG request
From: Michael Chan @ 2018-10-05 4:26 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1538713563-13878-1-git-send-email-michael.chan@broadcom.com>
From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
In HWRM_QUEUE_COS2BW_CFG request, enables field should have the bits
set only for the queue ids which are having the valid parameters.
This causes firmware to return error when the TC to hardware CoS queue
mapping is not 1:1 during DCBNL ETS setup.
Fixes: 2e8ef77ee0ff ("bnxt_en: Add TC to hardware QoS queue mapping logic.")
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
index ddc98c3..a85d2be 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c
@@ -98,13 +98,13 @@ static int bnxt_hwrm_queue_cos2bw_cfg(struct bnxt *bp, struct ieee_ets *ets,
bnxt_hwrm_cmd_hdr_init(bp, &req, HWRM_QUEUE_COS2BW_CFG, -1, -1);
for (i = 0; i < max_tc; i++) {
- u8 qidx;
+ u8 qidx = bp->tc_to_qidx[i];
req.enables |= cpu_to_le32(
- QUEUE_COS2BW_CFG_REQ_ENABLES_COS_QUEUE_ID0_VALID << i);
+ QUEUE_COS2BW_CFG_REQ_ENABLES_COS_QUEUE_ID0_VALID <<
+ qidx);
memset(&cos2bw, 0, sizeof(cos2bw));
- qidx = bp->tc_to_qidx[i];
cos2bw.queue_id = bp->q_info[qidx].queue_id;
if (ets->tc_tsa[i] == IEEE_8021QAZ_TSA_STRICT) {
cos2bw.tsa =
--
2.5.1
^ permalink raw reply related
* [PATCH net 3/4] bnxt_en: free hwrm resources, if driver probe fails.
From: Michael Chan @ 2018-10-05 4:26 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1538713563-13878-1-git-send-email-michael.chan@broadcom.com>
From: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
When the driver probe fails, all the resources that were allocated prior
to the failure must be freed. However, hwrm dma response memory is not
getting freed.
This patch fixes the problem described above.
Fixes: c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.")
Signed-off-by: Venkat Duvvuru <venkatkumar.duvvuru@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 2564a92..3718984 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -3017,10 +3017,11 @@ static void bnxt_free_hwrm_resources(struct bnxt *bp)
{
struct pci_dev *pdev = bp->pdev;
- dma_free_coherent(&pdev->dev, PAGE_SIZE, bp->hwrm_cmd_resp_addr,
- bp->hwrm_cmd_resp_dma_addr);
-
- bp->hwrm_cmd_resp_addr = NULL;
+ if (bp->hwrm_cmd_resp_addr) {
+ dma_free_coherent(&pdev->dev, PAGE_SIZE, bp->hwrm_cmd_resp_addr,
+ bp->hwrm_cmd_resp_dma_addr);
+ bp->hwrm_cmd_resp_addr = NULL;
+ }
}
static int bnxt_alloc_hwrm_resources(struct bnxt *bp)
@@ -9057,6 +9058,7 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
bnxt_clear_int_mode(bp);
init_err_pci_clean:
+ bnxt_free_hwrm_resources(bp);
bnxt_cleanup_pci(bp);
init_err_free:
--
2.5.1
^ permalink raw reply related
* [PATCH net 4/4] bnxt_en: get the reduced max_irqs by the ones used by RDMA
From: Michael Chan @ 2018-10-05 4:26 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1538713563-13878-1-git-send-email-michael.chan@broadcom.com>
From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
When getting the max rings supported, get the reduced max_irqs
by the ones used by RDMA.
If the number MSIX is the limiting factor, this bug may cause the
max ring count to be higher than it should be when RDMA driver is
loaded and may result in ring allocation failures.
Fixes: 30f529473ec9 ("bnxt_en: Do not modify max IRQ count after RDMA driver requests/frees IRQs.")
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 3718984..e2d9254 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -8622,7 +8622,7 @@ static void _bnxt_get_max_rings(struct bnxt *bp, int *max_rx, int *max_tx,
*max_tx = hw_resc->max_tx_rings;
*max_rx = hw_resc->max_rx_rings;
*max_cp = min_t(int, bnxt_get_max_func_cp_rings_for_en(bp),
- hw_resc->max_irqs);
+ hw_resc->max_irqs - bnxt_get_ulp_msix_num(bp));
*max_cp = min_t(int, *max_cp, hw_resc->max_stat_ctxs);
max_ring_grps = hw_resc->max_hw_ring_grps;
if (BNXT_CHIP_TYPE_NITRO_A0(bp) && BNXT_PF(bp)) {
--
2.5.1
^ permalink raw reply related
* RE: [PATCH 1/2] dt-bindings: net: add MDIO bus multiplexer driven by a regmap device
From: Pankaj Bansal @ 2018-10-05 4:32 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Florian Fainelli, netdev@vger.kernel.org, Alexandru Marginean
In-Reply-To: <20181005030818.GB7715@lunn.ch>
Hi Andrew
> -----Original Message-----
> From: Andrew Lunn [mailto:andrew@lunn.ch]
> Sent: Friday, October 5, 2018 8:38 AM
> To: Pankaj Bansal <pankaj.bansal@nxp.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>; netdev@vger.kernel.org;
> Alexandru Marginean <alexandru.marginean@nxp.com>
> Subject: Re: [PATCH 1/2] dt-bindings: net: add MDIO bus multiplexer driven
> by a regmap device
>
> > +i2c@2000000 {
> > + compatible = "fsl,vf610-i2c";
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + reg = <0x0 0x2000000 0x0 0x10000>;
> > + interrupts = <0 34 0x4>; // Level high type
> > + clock-names = "i2c";
> > + clocks = <&clockgen 4 7>;
> > + fsl-scl-gpio = <&gpio2 15 0>;
> > + status = "okay";
> > + /* The FPGA node */
> > + fpga@66 {
> > + compatible = "fsl,lx2160aqds-fpga", "fsl,fpga-qixis-i2c";
> > + reg = <0x66>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + mdio1_mux@54 {
> > + compatible = "mdio-mux-regmap", "mdio-mux";
> > + mdio-parent-bus = <&emdio2>; /* MDIO bus */
> > + reg = <0x54>; /* BRDCFG4 */
> > + mux-mask = <0x07>; /* EMI2_MDIO */
> > +
> > + #address-cells=<1>;
> > + #size-cells = <0>;
> > +
> > + mdio1_ioslot1@0 { // Slot 1
> > + reg = <0x00>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > + phy1@1 {
>
> Hi Pankaj
>
> It looks like you have tab vs space issues.
I will fix the formatting error in V2
>
> > + reg = <1>;
> > + compatible = "ethernet-phy-id0210.7441";
> > + };
>
> A blank line after here please.
OK
> > + phy1@0 {
> > + reg = <0>;
> > + compatible = "ethernet-phy-id0210.7441";
> > + };
> > + };
>
> And here
Ok
>
> > + mdio1_ioslot2@1 { // Slot 2
> > + reg = <0x01>;
> > + #address-cells = <1>;
> > + #size-cells = <0>;
> > +
> > + };
>
> and here.
Ok
>
> Andrew
^ 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