* Re: [PATCH bpf-next 4/6] bpf: add queue and stack maps
From: Mauricio Vasquez @ 2018-10-09 13:05 UTC (permalink / raw)
To: Song Liu; +Cc: Alexei Starovoitov, Daniel Borkmann, Networking
In-Reply-To: <CAPhsuW6B_VJ8t8VoDhmk=H9J6PwkczscPqsB4fW_cFr1K72z6g@mail.gmail.com>
On 10/08/2018 08:36 PM, Song Liu wrote:
> On Mon, Oct 8, 2018 at 12:12 PM Mauricio Vasquez B
> <mauricio.vasquez@polito.it> wrote:
>> Queue/stack maps implement a FIFO/LIFO data storage for ebpf programs.
>> These maps support peek, pop and push operations that are exposed to eBPF
>> programs through the new bpf_map[peek/pop/push] helpers. Those operations
>> are exposed to userspace applications through the already existing
>> syscalls in the following way:
>>
>> BPF_MAP_LOOKUP_ELEM -> peek
>> BPF_MAP_LOOKUP_AND_DELETE_ELEM -> pop
>> BPF_MAP_UPDATE_ELEM -> push
>>
>> Queue/stack maps are implemented using a buffer, tail and head indexes,
>> hence BPF_F_NO_PREALLOC is not supported.
>>
>> As opposite to other maps, queue and stack do not use RCU for protecting
>> maps values, the bpf_map[peek/pop] have a ARG_PTR_TO_UNINIT_MAP_VALUE
>> argument that is a pointer to a memory zone where to save the value of a
>> map. Basically the same as ARG_PTR_TO_UNINIT_MEM, but the size has not
>> be passed as an extra argument.
>>
>> Our main motivation for implementing queue/stack maps was to keep track
>> of a pool of elements, like network ports in a SNAT, however we forsee
>> other use cases, like for exampling saving last N kernel events in a map
>> and then analysing from userspace.
>>
>> 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 | 288 +++++++++++++++++++++++++++++++++++++++++
>> kernel/bpf/syscall.c | 30 +++-
>> kernel/bpf/verifier.c | 28 +++-
>> net/core/filter.c | 6 +
>> 10 files changed, 426 insertions(+), 18 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 */
> How about we put ARG_PTR_TO_UNINIT_MAP_VALUE and related logic to a
> separate patch?
I thought it too, but this is a really small change (6 additions, 3
deletions). Does it worth a separated patch?
>
>> /* 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..12a93fb37449
>> --- /dev/null
>> +++ b/kernel/bpf/queue_stack_maps.c
>> @@ -0,0 +1,288 @@
>> +// 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 size; /* max_entries + 1 */
>> +
>> + 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->head == qs->tail;
>> +}
>> +
>> +static bool queue_stack_map_is_full(struct bpf_queue_stack *qs)
>> +{
>> + u32 head = qs->head + 1;
>> +
>> + if (unlikely(head >= qs->size))
>> + head = 0;
>> +
>> + return head == qs->tail;
>> +}
>> +
>> +/* 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);
>> + struct bpf_queue_stack *qs;
>> + u32 size, value_size;
>> + u64 queue_size, cost;
>> +
>> + size = attr->max_entries + 1;
>> + value_size = attr->value_size;
>> +
>> + queue_size = sizeof(*qs) + (u64) value_size * size;
>> +
>> + 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->size = size;
>> +
>> + 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) {
>> + if (unlikely(++qs->tail >= qs->size))
>> + qs->tail = 0;
>> + }
>> +
>> +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;
>> + if (unlikely(index >= qs->size))
>> + index = qs->size - 1;
>> +
>> + ptr = &qs->elements[index * qs->map.value_size];
>> + memcpy(value, ptr, qs->map.value_size);
>> +
>> + if (delete)
>> + qs->head = index;
>> +
>> +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;
>> + goto out;
>> + }
>> + /* advance tail pointer to overwrite oldest element */
>> + if (unlikely(++qs->tail >= qs->size))
>> + qs->tail = 0;
>> + }
>> +
>> + dst = &qs->elements[qs->head * qs->map.value_size];
>> + memcpy(dst, value, qs->map.value_size);
>> +
>> + if (unlikely(++qs->head >= qs->size))
>> + qs->head = 0;
>> +
>> +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 c33d9303f72f..c135d205fd09 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);
>> @@ -1023,16 +1029,22 @@ static int map_lookup_and_delete_elem(union bpf_attr *attr)
>> */
>> preempt_disable();
>> __this_cpu_inc(bpf_prog_active);
>> - if (!map->ops->map_lookup_and_delete_elem) {
>> - err = -ENOTSUPP;
>> - goto free_value;
>> + if (map->map_type == BPF_MAP_TYPE_QUEUE ||
>> + map->map_type == BPF_MAP_TYPE_STACK) {
>> + err = map->ops->map_pop_elem(map, value);
>> + } else {
>> + if (!map->ops->map_lookup_and_delete_elem) {
>> + err = -ENOTSUPP;
>> + goto free_value;
> similar to previous patch: either we move this check, or we add
> __this_cpu_dec() and preempt_enable().
In this case the check cannot be moved, I'll change it to fix the
problem and make it more readable.
>
> Thanks,
> Song
>
^ permalink raw reply
* Re: [PATCH] qtnfmac: avoid uninitialized variable access
From: Sergey Matyukevich @ 2018-10-09 20:25 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Igor Mitsyanko, Igor Mitsyanko, Avinash Patil, Sergey Matyukevich,
Kalle Valo, David S. Miller, Andrey Shevchenko,
linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20181009155757.494212-1-arnd@arndb.de>
Hello Arnd,
> When qtnf_trans_send_cmd_with_resp() fails, we have not yet initialized
> 'resp', as pointed out by a valid gcc warning:
>
> drivers/net/wireless/quantenna/qtnfmac/commands.c: In function 'qtnf_cmd_send_with_reply':
> drivers/net/wireless/quantenna/qtnfmac/commands.c:133:54: error: 'resp' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> Since 'resp_skb' is also not set here, we can skip all further
> processing and just print the warning and return the failure code.
>
> Fixes: c6ed298ffe09 ("qtnfmac: cleanup and unify command error handling")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Thanks for the patch! And for reminding me that I forgot to enable
gcc warnings in CI builds in addition to sparse checks.
Reviewed-by: Sergey Matyukevich <sergey.matyukevich.os@quantenna.com>
Regards,
Sergey
^ permalink raw reply
* [PATCH net] net/sched: cls_api: add missing validation of netlink attributes
From: Davide Caratti @ 2018-10-09 13:10 UTC (permalink / raw)
To: David S. Miller, David Ahern, Jamal Hadi Salim; +Cc: netdev
Similarly to what has been done in 8b4c3cdd9dd8 ("net: sched: Add policy
validation for tc attributes"), add validation for TCA_CHAIN and TCA_KIND
netlink attributes.
tested with:
# ./tdc.py -c filter
Fixes: 5bc1701881e39 ("net: sched: introduce multichain support for filters")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/cls_api.c | 16 +++++++++++-----
1 file changed, 11 insertions(+), 5 deletions(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index 0a75cb2e5e7b..fb1afc0e130d 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -37,6 +37,11 @@ static LIST_HEAD(tcf_proto_base);
/* Protects list of registered TC modules. It is pure SMP lock. */
static DEFINE_RWLOCK(cls_mod_lock);
+const struct nla_policy cls_tca_policy[TCA_MAX + 1] = {
+ [TCA_KIND] = { .type = NLA_STRING },
+ [TCA_CHAIN] = { .type = NLA_U32 },
+};
+
/* Find classifier type by string name */
static const struct tcf_proto_ops *__tcf_proto_lookup_ops(const char *kind)
@@ -1211,7 +1216,7 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
replay:
tp_created = 0;
- err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL, extack);
+ err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, cls_tca_policy, extack);
if (err < 0)
return err;
@@ -1360,7 +1365,7 @@ static int tc_del_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
if (!netlink_ns_capable(skb, net->user_ns, CAP_NET_ADMIN))
return -EPERM;
- err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL, extack);
+ err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, cls_tca_policy, extack);
if (err < 0)
return err;
@@ -1475,7 +1480,7 @@ static int tc_get_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
void *fh = NULL;
int err;
- err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL, extack);
+ err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, cls_tca_policy, extack);
if (err < 0)
return err;
@@ -1838,7 +1843,7 @@ static int tc_ctl_chain(struct sk_buff *skb, struct nlmsghdr *n,
return -EPERM;
replay:
- err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, NULL, extack);
+ err = nlmsg_parse(n, sizeof(*t), tca, TCA_MAX, cls_tca_policy, extack);
if (err < 0)
return err;
@@ -1949,7 +1954,8 @@ static int tc_dump_chain(struct sk_buff *skb, struct netlink_callback *cb)
if (nlmsg_len(cb->nlh) < sizeof(*tcm))
return skb->len;
- err = nlmsg_parse(cb->nlh, sizeof(*tcm), tca, TCA_MAX, NULL, NULL);
+ err = nlmsg_parse(cb->nlh, sizeof(*tcm), tca, TCA_MAX, cls_tca_policy,
+ NULL);
if (err)
return err;
--
2.17.1
^ permalink raw reply related
* Re: [RFC PATCH 05/11] net: ethernet: ti: cpsw: add support for port interface mode selection phy
From: Grygorii Strashko @ 2018-10-09 20:28 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S. Miller, netdev, Tony Lindgren, Rob Herring,
Kishon Vijay Abraham I, Sekhar Nori, linux-kernel, linux-omap,
devicetree
In-Reply-To: <20181009005048.GB23588@lunn.ch>
Hi Andrew,
On 10/08/2018 07:50 PM, Andrew Lunn wrote:
>> /* Configure GMII_SEL register */
>> - cpsw_phy_sel(cpsw->dev, slave->phy->interface, slave->slave_num);
>> + if (!IS_ERR(slave->data->ifphy))
>> + phy_set_netif_mode(slave->data->ifphy, slave->data->phy_if);
>
> Is slave->data->phy_if also passed to phy_connect()? So you are going
> to end up with both the MAC and the PHY inserting RGMII delays, and it
> not working.
No. This logic not changed comparing to how it was.
* "rgmii" (RX and TX delays are added by the MAC when required)
rgmii_id = 0 --> CPSW: 0 : Internal Delay, PHY - no delay
* "rgmii-id" (RGMII with internal RX and TX delays provided by the PHY, the
MAC should not add the RX or TX delays in this case)
* "rgmii-rxid" (RGMII with internal RX delay provided by the PHY, the MAC
should not add an RX delay in this case)
* "rgmii-txid" (RGMII with internal TX delay provided by the PHY, the MAC
should not add an TX delay in this case)
rgmii_id = 1 --> CPSW: 1 : No Internal Delay, PHY/board - delay
>
> You need to somehow decide if the MAC is going to do the delay, or the
> PHY. But not both.
Again, this series does not change logic - only interfaces and DT.
Thank you for review.
--
regards,
-grygorii
^ permalink raw reply
* [iproute PATCH] bridge: fdb: Fix for missing keywords in non-JSON output
From: Phil Sutter @ 2018-10-09 12:44 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
While migrating to JSON print library, some keywords were dropped from
standard output by accident. Add them back to unbreak output parsers.
Fixes: c7c1a1ef51aea ("bridge: colorize output and use JSON print library")
Signed-off-by: Phil Sutter <phil@nwl.cc>
---
bridge/fdb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/bridge/fdb.c b/bridge/fdb.c
index 4dbc894ceab93..6487fac579c20 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -182,7 +182,7 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
if (!is_json_context())
fprintf(fp, "dev ");
print_color_string(PRINT_ANY, COLOR_IFNAME,
- "ifname", "%s ",
+ "ifname", "dev %s ",
ll_index_to_name(r->ndm_ifindex));
}
@@ -199,7 +199,7 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
print_color_string(PRINT_ANY,
ifa_family_color(family),
- "dst", "%s ", dst);
+ "dst", "dst %s ", dst);
}
if (vid)
@@ -246,7 +246,7 @@ int print_fdb(const struct sockaddr_nl *who, struct nlmsghdr *n, void *arg)
if (tb[NDA_MASTER])
- print_string(PRINT_ANY, "master", "%s ",
+ print_string(PRINT_ANY, "master", "master %s ",
ll_index_to_name(rta_getattr_u32(tb[NDA_MASTER])));
print_string(PRINT_ANY, "state", "%s\n",
--
2.19.0
^ permalink raw reply related
* Re: [RFC PATCH 02/11] dt-bindings: phy: add cpsw port interface mode selection phy bindings
From: Tony Lindgren @ 2018-10-09 20:30 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Rob Herring, Kishon Vijay Abraham I,
Sekhar Nori, linux-kernel, linux-omap, devicetree
In-Reply-To: <3fa09831-0f70-dfcc-3fd9-f877215a4631@ti.com>
* Grygorii Strashko <grygorii.strashko@ti.com> [181009 20:10]:
>
>
> On 10/09/2018 09:40 AM, Tony Lindgren wrote:
> > * Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
> >> +Examples:
> >> + phy_gmii_sel: phy-gmii-sel {
> >> + compatible = "ti,am3352-phy-gmii-sel";
> >> + syscon-scm = <&scm_conf>;
> >> + #phy-cells = <2>;
> >> + };
> >
> > Now that this driver can live in it's proper place in the
>
> right
>
> > dts, you may want to consider just using standard reg
> > property for it instead of the syscon-scm. And also get
> > rid of the syscon reads and writes.
>
> Could you help clarify how to get syscon in this case?
> syscon_node_to_regmap(dev->parent->of_node)?
Hmm I don't think you need syscon at all now. You can just
ioremap the register(s) and use readl/writel and that's it.
Or use regmap without syscon if you prefer that.
The ioremap in this case should be hitting cached ranges
anyways, so no extra overhead there.
> Also, there are could be more then one gmii_sel registers in SCM in the future,
> so I hidden offsets in of_match data.
> As result, "reg" not needed at all now.
But then you have to patch driver for various SoCs
instead of just configuring the standard reg property
in the dts file :)
Regards,
Tony
^ permalink raw reply
* [PATCH] ethtool: fix a missing-check bug
From: Wenwen Wang @ 2018-10-09 13:15 UTC (permalink / raw)
To: Wenwen Wang
Cc: Kangjie Lu, David S. Miller, Florian Fainelli, Kees Cook,
Ilya Lesokhin, Edward Cree, Yury Norov, Alan Brady,
Eugenia Emantayev, Stephen Hemminger,
open list:NETWORKING [GENERAL], open list
In ethtool_get_rxnfc(), the eth command 'cmd' is compared against
'ETHTOOL_GRXFH' to see whether it is necessary to adjust the variable
'info_size'. Then the whole structure of 'info' is copied from the
user-space buffer 'useraddr' with 'info_size' bytes. In the following
execution, 'info' may be copied again from the buffer 'useraddr' depending
on the 'cmd' and the 'info.flow_type'. However, after these two copies,
there is no check between 'cmd' and 'info.cmd'. In fact, 'cmd' is also
copied from the buffer 'useraddr' in dev_ethtool(), which is the caller
function of ethtool_get_rxnfc(). Given that 'useraddr' is in the user
space, a malicious user can race to change the eth command in the buffer
between these copies. By doing so, the attacker can supply inconsistent
data and cause undefined behavior because in the following execution 'info'
will be passed to ops->get_rxnfc().
This patch adds a necessary check on 'info.cmd' and 'cmd' to confirm that
they are still same after the two copies in ethtool_get_rxnfc(). Otherwise,
an error code EINVAL will be returned.
Signed-off-by: Wenwen Wang <wang6495@umn.edu>
---
net/core/ethtool.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index c9993c6..0136625 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -1015,6 +1015,9 @@ static noinline_for_stack int ethtool_get_rxnfc(struct net_device *dev,
return -EINVAL;
}
+ if (info.cmd != cmd)
+ return -EINVAL;
+
if (info.cmd == ETHTOOL_GRXCLSRLALL) {
if (info.rule_cnt > 0) {
if (info.rule_cnt <= KMALLOC_MAX_SIZE / sizeof(u32))
--
2.7.4
^ permalink raw reply related
* Re: [PATCH net-next 04/19] net: usb: aqc111: Various callbacks implementation
From: Bjørn Mork @ 2018-10-09 13:27 UTC (permalink / raw)
To: Oliver Neukum
Cc: Igor Russkikh, David S . Miller, Dmitry Bezrukov,
linux-usb@vger.kernel.org, netdev@vger.kernel.org
In-Reply-To: <1539006434.10342.14.camel@suse.com>
Oliver Neukum <oneukum@suse.com> writes:
> On Fr, 2018-10-05 at 10:24 +0000, Igor Russkikh wrote:
>> From: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>
>>
>> Reset, stop callbacks, driver unbind callback.
>> More register defines required for these callbacks.
>>
>> Signed-off-by: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>
>> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
>> ---
>> drivers/net/usb/aqc111.c | 48 ++++++++++++++++++++++
>> drivers/net/usb/aqc111.h | 101 +++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 149 insertions(+)
>>
>> diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
>> index 7f3e5a615750..22bb259d71fb 100644
>> --- a/drivers/net/usb/aqc111.c
>> +++ b/drivers/net/usb/aqc111.c
>> @@ -169,12 +169,60 @@ static int aqc111_bind(struct usbnet *dev, struct usb_interface *intf)
>>
>> static void aqc111_unbind(struct usbnet *dev, struct usb_interface *intf)
>> {
>> + u8 reg8;
>> + u16 reg16;
>> +
>> + /* Force bz */
>> + reg16 = SFR_PHYPWR_RSTCTL_BZ;
>> + aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_PHYPWR_RSTCTL,
>> + 2, 2, ®16);
>
> No, I am sorry, you are doing DMA on the kernel stack. That is not
> allowed. These functions will all have to be fixed.
Huh? No, he doesn't. That's the whole point with
usbnet_read_cmd_nopm(), isn't it?
Bjørn
^ permalink raw reply
* Re: [PATCH net-next 03/19] net: usb: aqc111: Add implementation of read and write commands
From: Bjørn Mork @ 2018-10-09 13:33 UTC (permalink / raw)
To: Igor Russkikh
Cc: David S . Miller, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <ebf0867900ae849581fbd20b52ee9e855e6345c8.1538734658.git.igor.russkikh@aquantia.com>
Igor Russkikh <Igor.Russkikh@aquantia.com> writes:
> +static int __aqc111_read_cmd(struct usbnet *dev, u8 cmd, u16 value,
> + u16 index, u16 size, void *data, int nopm)
> +{
> + int ret;
> + int (*fn)(struct usbnet *dev, u8 cmd, u8 reqtype, u16 value,
> + u16 index, void *data, u16 size);
> +
> + if (nopm)
> + fn = usbnet_read_cmd_nopm;
> + else
> + fn = usbnet_read_cmd;
> +
> + ret = fn(dev, cmd, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> + value, index, data, size);
> + if (size == 2)
> + le16_to_cpus(data);
> +
> + if (unlikely(ret < 0))
> + netdev_warn(dev->net,
> + "Failed to read(0x%x) reg index 0x%04x: %d\n",
> + cmd, index, ret);
> + return ret;
> +}
> +
> +static int aqc111_read_cmd_nopm(struct usbnet *dev, u8 cmd, u16 value,
> + u16 index, u16 size, void *data)
> +{
> + return __aqc111_read_cmd(dev, cmd, value, index, size, data, 1);
> +}
> +
> +static int aqc111_read_cmd(struct usbnet *dev, u8 cmd, u16 value,
> + u16 index, u16 size, void *data)
> +{
> + return __aqc111_read_cmd(dev, cmd, value, index, size, data, 0);
> +}
> +
Why would you want to do something like this instead of simply
implementing aqc111_read_cmd_nopm() and aqc111_read_cmd() as separate
functions? The function pointer stuff is incredibly ugly, as Oliver
pointed out. It wasn't done like that in usbnet.c, so why should we do
it like that here?
And the "if (size == 2) le16_to_cpus(data)" looks like something that
will come back and haunt you. Will this code never read larger
integers? Maybe add some sanity checks then, just in case...
Or simply add more helpers. An additional pair of helpers for reading
16bit integers might simplify your code quite a bit.
Bjørn
^ permalink raw reply
* Re: [PATCH net-next 01/19] net: usb: aqc111: Driver skeleton for Aquantia AQtion USB to 5GbE
From: Bjørn Mork @ 2018-10-09 13:37 UTC (permalink / raw)
To: Igor Russkikh
Cc: David S . Miller, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <d7ac7ff4c2fc479021286d0f7549d9b2e0aac803.1538734658.git.igor.russkikh@aquantia.com>
Igor Russkikh <Igor.Russkikh@aquantia.com> writes:
>> +static const struct driver_info aqc111_info = {
> + .description = "Aquantia AQtion USB to 5GbE Controller",
> +};
> +
> +#define AQC111_USB_ETH_DEV(vid, pid, table) \
> + .match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
> + USB_DEVICE_ID_MATCH_INT_CLASS, \
> + USB_DEVICE(vid, pid), \
> + .bInterfaceClass = USB_CLASS_VENDOR_SPEC, \
> + .driver_info = (unsigned long)&table, \
> +}, \
> +{ \
> + .match_flags = USB_DEVICE_ID_MATCH_DEVICE | \
> + USB_DEVICE_ID_MATCH_INT_INFO, \
> + USB_DEVICE(vid, pid), \
> + .bInterfaceClass = USB_CLASS_COMM, \
> + .bInterfaceSubClass = USB_CDC_SUBCLASS_ETHERNET, \
> + .bInterfaceProtocol = USB_CDC_PROTO_NONE
> +
Is the missing .driver_info for the CDC class intentional? If so, then
why include it at all?
Bjørn
^ permalink raw reply
* Re: [PATCH net-next 09/19] net: usb: aqc111: Implement RX data path
From: Bjørn Mork @ 2018-10-09 13:39 UTC (permalink / raw)
To: Igor Russkikh
Cc: David S . Miller, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <81aef6c50a93d22c6b91e966c89d7520dcbaaf87.1538734658.git.igor.russkikh@aquantia.com>
Igor Russkikh <Igor.Russkikh@aquantia.com> writes:
> From: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>
>
> Signed-off-by: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
> ---
You'd want some description here.
Bjørn
^ permalink raw reply
* Re: linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2018-10-09 20:58 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: David Miller, Networking, Linux-Next Mailing List,
Linux Kernel Mailing List, Al Viro
In-Reply-To: <7cb98153-14df-96e4-edee-518775f49ec5@mojatatu.com>
[-- Attachment #1: Type: text/plain, Size: 237 bytes --]
Hi Jamal,
On Tue, 9 Oct 2018 06:02:25 -0400 Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> Attached should fix it. Al, please double check.
OK, I will use that resolution from today.
Thanks.
--
Cheers,
Stephen Rothwell
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [PATCH net-next] cxgb4: Add thermal zone support
From: Ganesh Goudar @ 2018-10-09 13:44 UTC (permalink / raw)
To: netdev, davem; +Cc: nirranjan, indranil, dt, Ganesh Goudar
Add thermal zone support to monitor ASIC's temperature.
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
drivers/net/ethernet/chelsio/cxgb4/Makefile | 1 +
drivers/net/ethernet/chelsio/cxgb4/cxgb4.h | 18 ++++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 8 ++
drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c | 114 +++++++++++++++++++++
drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h | 1 +
5 files changed, 142 insertions(+)
create mode 100644 drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
diff --git a/drivers/net/ethernet/chelsio/cxgb4/Makefile b/drivers/net/ethernet/chelsio/cxgb4/Makefile
index bea6a05..91d8a88 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/Makefile
+++ b/drivers/net/ethernet/chelsio/cxgb4/Makefile
@@ -12,3 +12,4 @@ cxgb4-objs := cxgb4_main.o l2t.o smt.o t4_hw.o sge.o clip_tbl.o cxgb4_ethtool.o
cxgb4-$(CONFIG_CHELSIO_T4_DCB) += cxgb4_dcb.o
cxgb4-$(CONFIG_CHELSIO_T4_FCOE) += cxgb4_fcoe.o
cxgb4-$(CONFIG_DEBUG_FS) += cxgb4_debugfs.o
+cxgb4-$(CONFIG_THERMAL) += cxgb4_thermal.o
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index b5010bd..95909f0 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -52,6 +52,7 @@
#include <linux/ptp_clock_kernel.h>
#include <linux/ptp_classify.h>
#include <linux/crash_dump.h>
+#include <linux/thermal.h>
#include <asm/io.h>
#include "t4_chip_type.h"
#include "cxgb4_uld.h"
@@ -890,6 +891,14 @@ struct mps_encap_entry {
atomic_t refcnt;
};
+#ifdef CONFIG_THERMAL
+struct ch_thermal {
+ struct thermal_zone_device *tzdev;
+ int trip_temp;
+ int trip_type;
+};
+#endif
+
struct adapter {
void __iomem *regs;
void __iomem *bar2;
@@ -1008,6 +1017,9 @@ struct adapter {
/* Dump buffer for collecting logs in kdump kernel */
struct vmcoredd_data vmcoredd;
+#ifdef CONFIG_THERMAL
+ struct ch_thermal ch_thermal;
+#endif
};
/* Support for "sched-class" command to allow a TX Scheduling Class to be
@@ -1862,4 +1874,10 @@ void cxgb4_ring_tx_db(struct adapter *adap, struct sge_txq *q, int n);
int t4_set_vlan_acl(struct adapter *adap, unsigned int mbox, unsigned int vf,
u16 vlan);
int cxgb4_dcb_enabled(const struct net_device *dev);
+
+#ifdef CONFIG_THERMAL
+int cxgb4_thermal_init(struct adapter *adap);
+int cxgb4_thermal_remove(struct adapter *adap);
+#endif /* CONFIG_THERMAL */
+
#endif /* __CXGB4_H__ */
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
index 1a93efa..03cc073 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
@@ -5864,6 +5864,11 @@ static int init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
if (!is_t4(adapter->params.chip))
cxgb4_ptp_init(adapter);
+#ifdef CONFIG_THERMAL
+ if (!is_t4(adapter->params.chip) && (adapter->flags & FW_OK))
+ cxgb4_thermal_init(adapter);
+#endif /* CONFIG_THERMAL */
+
print_adapter_info(adapter);
return 0;
@@ -5929,6 +5934,9 @@ static void remove_one(struct pci_dev *pdev)
if (!is_t4(adapter->params.chip))
cxgb4_ptp_stop(adapter);
+#ifdef CONFIG_THERMAL
+ cxgb4_thermal_remove(adapter);
+#endif
/* If we allocated filters, free up state associated with any
* valid filters ...
diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
new file mode 100644
index 0000000..28052e750
--- /dev/null
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_thermal.c
@@ -0,0 +1,114 @@
+/*
+ * Copyright (C) 2017 Chelsio Communications. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * The full GNU General Public License is included in this distribution in
+ * the file called "COPYING".
+ *
+ * Written by: Ganesh Goudar (ganeshgr@chelsio.com)
+ */
+
+#include "cxgb4.h"
+
+#define CXGB4_NUM_TRIPS 1
+
+static int cxgb4_thermal_get_temp(struct thermal_zone_device *tzdev,
+ int *temp)
+{
+ struct adapter *adap = tzdev->devdata;
+ u32 param, val;
+ int ret;
+
+ param = (FW_PARAMS_MNEM_V(FW_PARAMS_MNEM_DEV) |
+ FW_PARAMS_PARAM_X_V(FW_PARAMS_PARAM_DEV_DIAG) |
+ FW_PARAMS_PARAM_Y_V(FW_PARAM_DEV_DIAG_TMP));
+
+ ret = t4_query_params(adap, adap->mbox, adap->pf, 0, 1,
+ ¶m, &val);
+ if (ret < 0 || val == 0)
+ return -1;
+
+ *temp = val * 1000;
+ return 0;
+}
+
+static int cxgb4_thermal_get_trip_type(struct thermal_zone_device *tzdev,
+ int trip, enum thermal_trip_type *type)
+{
+ struct adapter *adap = tzdev->devdata;
+
+ if (!adap->ch_thermal.trip_temp)
+ return -EINVAL;
+
+ *type = adap->ch_thermal.trip_type;
+ return 0;
+}
+
+static int cxgb4_thermal_get_trip_temp(struct thermal_zone_device *tzdev,
+ int trip, int *temp)
+{
+ struct adapter *adap = tzdev->devdata;
+
+ if (!adap->ch_thermal.trip_temp)
+ return -EINVAL;
+
+ *temp = adap->ch_thermal.trip_temp;
+ return 0;
+}
+
+static struct thermal_zone_device_ops cxgb4_thermal_ops = {
+ .get_temp = cxgb4_thermal_get_temp,
+ .get_trip_type = cxgb4_thermal_get_trip_type,
+ .get_trip_temp = cxgb4_thermal_get_trip_temp,
+};
+
+int cxgb4_thermal_init(struct adapter *adap)
+{
+ struct ch_thermal *ch_thermal = &adap->ch_thermal;
+ int num_trip = CXGB4_NUM_TRIPS;
+ u32 param, val;
+ int ret;
+
+ /* on older firmwares we may not get the trip temperature,
+ * set the num of trips to 0.
+ */
+ param = (FW_PARAMS_MNEM_V(FW_PARAMS_MNEM_DEV) |
+ FW_PARAMS_PARAM_X_V(FW_PARAMS_PARAM_DEV_DIAG) |
+ FW_PARAMS_PARAM_Y_V(FW_PARAM_DEV_DIAG_MAXTMPTHRESH));
+
+ ret = t4_query_params(adap, adap->mbox, adap->pf, 0, 1,
+ ¶m, &val);
+ if (ret < 0) {
+ num_trip = 0; /* could not get trip temperature */
+ } else {
+ ch_thermal->trip_temp = val * 1000;
+ ch_thermal->trip_type = THERMAL_TRIP_CRITICAL;
+ }
+
+ ch_thermal->tzdev = thermal_zone_device_register("cxgb4", num_trip,
+ 0, adap,
+ &cxgb4_thermal_ops,
+ NULL, 0, 0);
+ if (IS_ERR(ch_thermal->tzdev)) {
+ ret = PTR_ERR(ch_thermal->tzdev);
+ dev_err(adap->pdev_dev, "Failed to register thermal zone\n");
+ ch_thermal->tzdev = NULL;
+ return ret;
+ }
+ return 0;
+}
+
+int cxgb4_thermal_remove(struct adapter *adap)
+{
+ if (adap->ch_thermal.tzdev)
+ thermal_zone_device_unregister(adap->ch_thermal.tzdev);
+ return 0;
+}
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h b/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
index 6d2bc87..57584ab 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4fw_api.h
@@ -1332,6 +1332,7 @@ enum fw_params_param_dev_phyfw {
enum fw_params_param_dev_diag {
FW_PARAM_DEV_DIAG_TMP = 0x00,
FW_PARAM_DEV_DIAG_VDD = 0x01,
+ FW_PARAM_DEV_DIAG_MAXTMPTHRESH = 0x02,
};
enum fw_params_param_dev_fwcache {
--
2.1.0
^ permalink raw reply related
* Re: [PATCH net-next 08/19] net: usb: aqc111: Implement TX data path
From: Bjørn Mork @ 2018-10-09 13:50 UTC (permalink / raw)
To: Igor Russkikh
Cc: David S . Miller, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <65674cc0cd1b025d859b1b0a5410b21ca9f88176.1538734658.git.igor.russkikh@aquantia.com>
Igor Russkikh <Igor.Russkikh@aquantia.com> writes:
> +struct aq_tx_packet_desc {
> + struct {
> + u32 length:21;
> + u32 checksum:7;
> + u32 drop_padding:1;
> + u32 vlan_tag:1;
> + u32 cphi:1;
> + u32 dicf:1;
> + };
> + struct {
> + u32 max_seg_size:15;
> + u32 reserved:1;
> + u32 vlan_info:16;
> + };
> +};
You might want to shift and mask instead to avoid going insane when
trying to use this header on a BE system...
Bjørn
^ permalink raw reply
* Re: [RFC PATCH 00/11] net: ethernet: ti: cpsw: replace cpsw-phy-sel with phy driver
From: Grygorii Strashko @ 2018-10-09 21:12 UTC (permalink / raw)
To: Tony Lindgren
Cc: David S. Miller, netdev, Rob Herring, Kishon Vijay Abraham I,
Sekhar Nori, linux-kernel, linux-omap, devicetree
In-Reply-To: <20181009143631.GK5662@atomide.com>
On 10/09/2018 09:36 AM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
>> 2) introduce new PHY API for network interface mode selection which will use
>> already defined set of modes from phy_interface_t.
>>
>> Option 2 was selected for this series.
>
> Looks good to me :) The dts files will cause merge conflicts with
> what I have pending for the ti-sysc changes so please send the dts
> changes in a separate series when posting without RFC.
expected. I did on top of mauster, but will rebase and resend if approved.
--
regards,
-grygorii
^ permalink raw reply
* [PATCH v2] rxrpc: use correct kvec num while send response packet in rxrpc_reject_packets
From: YueHaibing @ 2018-10-09 14:15 UTC (permalink / raw)
To: David Howells, davem; +Cc: YueHaibing, linux-afs, netdev, kernel-janitors
In-Reply-To: <1539052273-34824-1-git-send-email-yuehaibing@huawei.com>
Fixes gcc '-Wunused-but-set-variable' warning:
net/rxrpc/output.c: In function 'rxrpc_reject_packets':
net/rxrpc/output.c:527:11: warning:
variable 'ioc' set but not used [-Wunused-but-set-variable]
'ioc' is the correct kvec num while send response packet.
Fixes: commit ece64fec164f ("rxrpc: Emit BUSY packets when supposed to rather than ABORTs")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
net/rxrpc/output.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/rxrpc/output.c b/net/rxrpc/output.c
index e8fb892..a141ee3 100644
--- a/net/rxrpc/output.c
+++ b/net/rxrpc/output.c
@@ -572,7 +572,8 @@ void rxrpc_reject_packets(struct rxrpc_local *local)
whdr.flags ^= RXRPC_CLIENT_INITIATED;
whdr.flags &= RXRPC_CLIENT_INITIATED;
- ret = kernel_sendmsg(local->socket, &msg, iov, 2, size);
+ ret = kernel_sendmsg(local->socket, &msg,
+ iov, ioc, size);
if (ret < 0)
trace_rxrpc_tx_fail(local->debug_id, 0, ret,
rxrpc_tx_point_reject);
^ permalink raw reply related
* Re: [PATCH net-next] tcp: forbid direct reclaim if MSG_DONTWAIT is set in send path
From: Eric Dumazet @ 2018-10-09 14:12 UTC (permalink / raw)
To: Yafang Shao; +Cc: David Miller, netdev, LKML
In-Reply-To: <1539086718-4119-2-git-send-email-laoar.shao@gmail.com>
On Tue, Oct 9, 2018 at 5:05 AM Yafang Shao <laoar.shao@gmail.com> wrote:
>
> By default, the sk->sk_allocation is GFP_KERNEL, that means if there's
> no enough memory it will do both direct reclaim and background reclaim.
> If the size of system memory is great, the direct reclaim may cause great
> latency spike.
>
> When we set MSG_DONTWAIT in send syscalls, we really don't want it to be
> blocked, so we'd better clear __GFP_DIRECT_RECLAIM when allocate skb in the
> send path. Then, it will return immediately if there's no enough memory to
> be allocated, and then the appliation has a chance to do some other stuffs
> instead of being blocked here.
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
> ---
> net/ipv4/tcp.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 43ef83b..fe4f5ce 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -1182,6 +1182,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
> bool process_backlog = false;
> bool zc = false;
> long timeo;
> + gfp_t gfp;
>
> flags = msg->msg_flags;
>
> @@ -1255,6 +1256,9 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
> /* Ok commence sending. */
> copied = 0;
>
> + gfp = flags & MSG_DONTWAIT ? sk->sk_allocation & ~__GFP_DIRECT_RECLAIM :
> + sk->sk_allocation;
> +
> restart:
> mss_now = tcp_send_mss(sk, &size_goal, flags);
>
> @@ -1283,8 +1287,7 @@ int tcp_sendmsg_locked(struct sock *sk, struct msghdr *msg, size_t size)
> }
> first_skb = tcp_rtx_and_write_queues_empty(sk);
> linear = select_size(first_skb, zc);
> - skb = sk_stream_alloc_skb(sk, linear, sk->sk_allocation,
> - first_skb);
> + skb = sk_stream_alloc_skb(sk, linear, gfp, first_skb);
> if (!skb)
> goto wait_for_memory;
How have you tested this patch exactly ?
Most of TCP payloads are added in page fragments, and you have not
changed the page allocation fragments.
Also, I do not see how an application will get future notifications
that it can retry the failed system call ?
How are you really going to deal with this in high performance applications ?
I would rather prefer a socket setsockopt() to eventually be able to
flip __GFP_DIRECT_RECLAIM in sk->sk_allocation,
to not add all these tests in fast path, but honestly I do not see how
applications can really make use of this.
^ permalink raw reply
* [PATCH v2 1/2] net: if_arp: Fix incorrect indents
From: Håkon Bugge @ 2018-10-09 14:27 UTC (permalink / raw)
To: netdev
Cc: stephen, David S . Miller, Kate Stewart, Thomas Gleixner,
Greg Kroah-Hartman, Philippe Ombredanne, linux-kernel
In-Reply-To: <20181009142724.2213012-1-Haakon.Bugge@oracle.com>
Fixing incorrect indents and align comments.
Signed-off-by: Håkon Bugge <haakon.bugge@oracle.com>
---
include/uapi/linux/if_arp.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/include/uapi/linux/if_arp.h b/include/uapi/linux/if_arp.h
index 4605527ca41b..b68b4b3d9172 100644
--- a/include/uapi/linux/if_arp.h
+++ b/include/uapi/linux/if_arp.h
@@ -114,18 +114,18 @@
/* ARP ioctl request. */
struct arpreq {
- struct sockaddr arp_pa; /* protocol address */
- struct sockaddr arp_ha; /* hardware address */
- int arp_flags; /* flags */
- struct sockaddr arp_netmask; /* netmask (only for proxy arps) */
- char arp_dev[16];
+ struct sockaddr arp_pa; /* protocol address */
+ struct sockaddr arp_ha; /* hardware address */
+ int arp_flags; /* flags */
+ struct sockaddr arp_netmask; /* netmask (only for proxy arps) */
+ char arp_dev[16];
};
struct arpreq_old {
- struct sockaddr arp_pa; /* protocol address */
- struct sockaddr arp_ha; /* hardware address */
- int arp_flags; /* flags */
- struct sockaddr arp_netmask; /* netmask (only for proxy arps) */
+ struct sockaddr arp_pa; /* protocol address */
+ struct sockaddr arp_ha; /* hardware address */
+ int arp_flags; /* flags */
+ struct sockaddr arp_netmask; /* netmask (only for proxy arps) */
};
/* ARP Flag values. */
--
2.14.3
^ permalink raw reply related
* Re: [PATCH net-next 07/19] net: usb: aqc111: Add support for getting and setting of MAC address
From: Igor Russkikh @ 2018-10-09 14:34 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <20181006010346.GA32455@lunn.ch>
Hi Andrew,
>> + if (ret < 0)
>> + goto out;
>> +
>> + memcpy(dev->net->dev_addr, buf, ETH_ALEN);
>> + memcpy(dev->net->perm_addr, dev->net->dev_addr, ETH_ALEN);
>
> Is this really the permanent address? If i call aqc111_set_mac_addr()
> followed by aqc111_get_mac() i still get what is in the OTP EEPROM?
Thats actually a confusion with function name here.
Think its better to name it aqc111_init_mac() since it gets called
only once on bind.
It really initializes perm_addr once, thus standard ndev callback will give
the perm mac you want.
>
> You initialized it above as {0}. You don't need to memset it here.
>
>> + ret = aqc111_get_mac(dev, buf);
>
> Do you even need to zero it? If aqc111_get_mac() fails, it will be
> left undefined, but you fail the bind anyway.
We even don't need this `buf` here at all. We'll move it into
above init_mac function.
BR, Igor
^ permalink raw reply
* Re: [net-next,v2,2/4] net/smc: ipv6 support for smc_diag.c
From: Ursula Braun @ 2018-10-09 14:41 UTC (permalink / raw)
To: Eugene Syromiatnikov
Cc: davem, netdev, linux-s390, schwidefsky, heiko.carstens, raspl,
kgraul
In-Reply-To: <20181007011152.GA11112@asgard.redhat.com>
On 10/07/2018 03:11 AM, Eugene Syromiatnikov wrote:
> On Wed, May 02, 2018 at 04:56:45PM +0200, Ursula Braun wrote:
>> From: Karsten Graul <kgraul@linux.ibm.com>
>>
>> Update smc_diag.c to support ipv6 addresses on the diagnosis interface.
>>
>> Signed-off-by: Karsten Graul <kgraul@linux.ibm.com>
>> Signed-off-by: Ursula Braun <ubraun@linux.ibm.com>
>> ---
>> net/smc/smc_diag.c | 39 ++++++++++++++++++++++++++++++---------
>> 1 file changed, 30 insertions(+), 9 deletions(-)
>>
>> diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
>> index 427b91c1c964..05dd7e6d314d 100644
>> --- a/net/smc/smc_diag.c
>> +++ b/net/smc/smc_diag.c
>> @@ -38,17 +38,27 @@ static void smc_diag_msg_common_fill(struct smc_diag_msg *r, struct sock *sk)
>> {
>> struct smc_sock *smc = smc_sk(sk);
>>
>> - r->diag_family = sk->sk_family;
>> if (!smc->clcsock)
>> return;
>> r->id.idiag_sport = htons(smc->clcsock->sk->sk_num);
>> r->id.idiag_dport = smc->clcsock->sk->sk_dport;
>> r->id.idiag_if = smc->clcsock->sk->sk_bound_dev_if;
>> sock_diag_save_cookie(sk, r->id.idiag_cookie);
>> - memset(&r->id.idiag_src, 0, sizeof(r->id.idiag_src));
>> - memset(&r->id.idiag_dst, 0, sizeof(r->id.idiag_dst));
>> - r->id.idiag_src[0] = smc->clcsock->sk->sk_rcv_saddr;
>> - r->id.idiag_dst[0] = smc->clcsock->sk->sk_daddr;
>> + if (sk->sk_protocol == SMCPROTO_SMC) {
>> + r->diag_family = PF_INET;
>> + memset(&r->id.idiag_src, 0, sizeof(r->id.idiag_src));
>> + memset(&r->id.idiag_dst, 0, sizeof(r->id.idiag_dst));
>> + r->id.idiag_src[0] = smc->clcsock->sk->sk_rcv_saddr;
>> + r->id.idiag_dst[0] = smc->clcsock->sk->sk_daddr;
>> +#if IS_ENABLED(CONFIG_IPV6)
>> + } else if (sk->sk_protocol == SMCPROTO_SMC6) {
>> + r->diag_family = PF_INET6;
>> + memcpy(&r->id.idiag_src, &smc->clcsock->sk->sk_v6_rcv_saddr,
>> + sizeof(smc->clcsock->sk->sk_v6_rcv_saddr));
>> + memcpy(&r->id.idiag_dst, &smc->clcsock->sk->sk_v6_daddr,
>> + sizeof(smc->clcsock->sk->sk_v6_daddr));
>> +#endif
>> + }
>
> This change makes it impossible to distinguish an inet_sock_diag
> response message from SMC sock_diag response (previously it reported
> AF_SMC in diag_family which allows deciding whether that a part of
> struct smc_diag_msg or struct inet_diag_msg).
>
Eugene,
we are considering the following patch:
---
net/smc/smc_diag.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/smc/smc_diag.c b/net/smc/smc_diag.c
index dbf64a93d68a..371b4cf31fcd 100644
--- a/net/smc/smc_diag.c
+++ b/net/smc/smc_diag.c
@@ -38,6 +38,7 @@ static void smc_diag_msg_common_fill(struct smc_diag_msg *r, struct sock *sk)
{
struct smc_sock *smc = smc_sk(sk);
+ r->diag_family = sk->sk_family;
if (!smc->clcsock)
return;
r->id.idiag_sport = htons(smc->clcsock->sk->sk_num);
@@ -45,14 +46,12 @@ static void smc_diag_msg_common_fill(struct smc_diag_msg *r, struct sock *sk)
r->id.idiag_if = smc->clcsock->sk->sk_bound_dev_if;
sock_diag_save_cookie(sk, r->id.idiag_cookie);
if (sk->sk_protocol == SMCPROTO_SMC) {
- r->diag_family = PF_INET;
memset(&r->id.idiag_src, 0, sizeof(r->id.idiag_src));
memset(&r->id.idiag_dst, 0, sizeof(r->id.idiag_dst));
r->id.idiag_src[0] = smc->clcsock->sk->sk_rcv_saddr;
r->id.idiag_dst[0] = smc->clcsock->sk->sk_daddr;
#if IS_ENABLED(CONFIG_IPV6)
} else if (sk->sk_protocol == SMCPROTO_SMC6) {
- r->diag_family = PF_INET6;
memcpy(&r->id.idiag_src, &smc->clcsock->sk->sk_v6_rcv_saddr,
sizeof(smc->clcsock->sk->sk_v6_rcv_saddr));
memcpy(&r->id.idiag_dst, &smc->clcsock->sk->sk_v6_daddr,
^ permalink raw reply related
* Re: [PATCH net] net/sched: cls_api: add missing validation of netlink attributes
From: David Ahern @ 2018-10-09 14:46 UTC (permalink / raw)
To: Davide Caratti, David S. Miller, Jamal Hadi Salim; +Cc: netdev
In-Reply-To: <05f98d2d220d443c157fc797fecc22692eeaa0da.1539090183.git.dcaratti@redhat.com>
On 10/9/18 7:10 AM, Davide Caratti wrote:
> Similarly to what has been done in 8b4c3cdd9dd8 ("net: sched: Add policy
> validation for tc attributes"), add validation for TCA_CHAIN and TCA_KIND
> netlink attributes.
>
> tested with:
> # ./tdc.py -c filter
>
> Fixes: 5bc1701881e39 ("net: sched: introduce multichain support for filters")
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
> ---
> net/sched/cls_api.c | 16 +++++++++++-----
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
> index 0a75cb2e5e7b..fb1afc0e130d 100644
> --- a/net/sched/cls_api.c
> +++ b/net/sched/cls_api.c
> @@ -37,6 +37,11 @@ static LIST_HEAD(tcf_proto_base);
> /* Protects list of registered TC modules. It is pure SMP lock. */
> static DEFINE_RWLOCK(cls_mod_lock);
>
> +const struct nla_policy cls_tca_policy[TCA_MAX + 1] = {
> + [TCA_KIND] = { .type = NLA_STRING },
> + [TCA_CHAIN] = { .type = NLA_U32 },
> +};
> +
That should be static since it can not be used outside this module.
it be nice to have a tc_common module so this stuff does not have to be
defined multiple times.
^ permalink raw reply
* Re: [PATCH net-next 07/19] net: usb: aqc111: Add support for getting and setting of MAC address
From: Andrew Lunn @ 2018-10-09 14:46 UTC (permalink / raw)
To: Igor Russkikh
Cc: David S . Miller, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <082aefef-5927-181e-e505-685c1ca51492@aquantia.com>
On Tue, Oct 09, 2018 at 02:34:36PM +0000, Igor Russkikh wrote:
> Hi Andrew,
>
> >> + if (ret < 0)
> >> + goto out;
> >> +
> >> + memcpy(dev->net->dev_addr, buf, ETH_ALEN);
> >> + memcpy(dev->net->perm_addr, dev->net->dev_addr, ETH_ALEN);
> >
> > Is this really the permanent address? If i call aqc111_set_mac_addr()
> > followed by aqc111_get_mac() i still get what is in the OTP EEPROM?
>
> Thats actually a confusion with function name here.
> Think its better to name it aqc111_init_mac() since it gets called
> only once on bind.
Hi Igor
Or aqc111_get_otp_mac()?
Andrew
^ permalink raw reply
* Re: [RFC PATCH 02/11] dt-bindings: phy: add cpsw port interface mode selection phy bindings
From: Grygorii Strashko @ 2018-10-09 22:04 UTC (permalink / raw)
To: Tony Lindgren
Cc: David S. Miller, netdev, Rob Herring, Kishon Vijay Abraham I,
Sekhar Nori, linux-kernel, linux-omap, devicetree
In-Reply-To: <20181009203017.GM5662@atomide.com>
On 10/09/2018 03:30 PM, Tony Lindgren wrote:
> * Grygorii Strashko <grygorii.strashko@ti.com> [181009 20:10]:
>>
>>
>> On 10/09/2018 09:40 AM, Tony Lindgren wrote:
>>> * Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
>>>> +Examples:
>>>> + phy_gmii_sel: phy-gmii-sel {
>>>> + compatible = "ti,am3352-phy-gmii-sel";
>>>> + syscon-scm = <&scm_conf>;
>>>> + #phy-cells = <2>;
>>>> + };
>>>
>>> Now that this driver can live in it's proper place in the
>>
>> right
>>
>>> dts, you may want to consider just using standard reg
>>> property for it instead of the syscon-scm. And also get
>>> rid of the syscon reads and writes.
>>
>> Could you help clarify how to get syscon in this case?
>> syscon_node_to_regmap(dev->parent->of_node)?
>
> Hmm I don't think you need syscon at all now. You can just
> ioremap the register(s) and use readl/writel and that's it.
> Or use regmap without syscon if you prefer that.
It will overlap with already remapped SCM syscon and i'd like to avoid this.
+ it seems common practice to use syscon for devices/drivers which are
child to SCM node - makes overall system more consistent.
>
> The ioremap in this case should be hitting cached ranges
> anyways, so no extra overhead there.
>
>> Also, there are could be more then one gmii_sel registers in SCM in the future,
>> so I hidden offsets in of_match data.
>> As result, "reg" not needed at all now.
>
> But then you have to patch driver for various SoCs
> instead of just configuring the standard reg property
> in the dts file :)
Problem is that they are not guarantee to be standard between SoC's families
(number of regs and fields placement), as result it might require to change
driver any way for various SoCs to handle properly new fields placement.
I prefer to fix driver then fight with DT ;) as it's static for SoC family
and need to be changed only once when new SoC family introduced.
--
regards,
-grygorii
^ permalink raw reply
* Re: [RFC PATCH 02/11] dt-bindings: phy: add cpsw port interface mode selection phy bindings
From: Tony Lindgren @ 2018-10-09 22:07 UTC (permalink / raw)
To: Grygorii Strashko
Cc: David S. Miller, netdev, Rob Herring, Kishon Vijay Abraham I,
Sekhar Nori, linux-kernel, linux-omap, devicetree
In-Reply-To: <f9dfd775-0679-f82b-b53b-065e9f7be7c7@ti.com>
* Grygorii Strashko <grygorii.strashko@ti.com> [181009 22:04]:
>
>
> On 10/09/2018 03:30 PM, Tony Lindgren wrote:
> > * Grygorii Strashko <grygorii.strashko@ti.com> [181009 20:10]:
> >>
> >>
> >> On 10/09/2018 09:40 AM, Tony Lindgren wrote:
> >>> * Grygorii Strashko <grygorii.strashko@ti.com> [181008 23:54]:
> >>>> +Examples:
> >>>> + phy_gmii_sel: phy-gmii-sel {
> >>>> + compatible = "ti,am3352-phy-gmii-sel";
> >>>> + syscon-scm = <&scm_conf>;
> >>>> + #phy-cells = <2>;
> >>>> + };
> >>>
> >>> Now that this driver can live in it's proper place in the
> >>
> >> right
> >>
> >>> dts, you may want to consider just using standard reg
> >>> property for it instead of the syscon-scm. And also get
> >>> rid of the syscon reads and writes.
> >>
> >> Could you help clarify how to get syscon in this case?
> >> syscon_node_to_regmap(dev->parent->of_node)?
> >
> > Hmm I don't think you need syscon at all now. You can just
> > ioremap the register(s) and use readl/writel and that's it.
> > Or use regmap without syscon if you prefer that.
>
> It will overlap with already remapped SCM syscon and i'd like to avoid this.
> + it seems common practice to use syscon for devices/drivers which are
> child to SCM node - makes overall system more consistent.
Well it was just set up with syscon in deperation earlier with
drivers just blindly mapping registers outside of their
range..
> > The ioremap in this case should be hitting cached ranges
> > anyways, so no extra overhead there.
> >
> >> Also, there are could be more then one gmii_sel registers in SCM in the future,
> >> so I hidden offsets in of_match data.
> >> As result, "reg" not needed at all now.
> >
> > But then you have to patch driver for various SoCs
> > instead of just configuring the standard reg property
> > in the dts file :)
>
> Problem is that they are not guarantee to be standard between SoC's families
> (number of regs and fields placement), as result it might require to change
> driver any way for various SoCs to handle properly new fields placement.
>
> I prefer to fix driver then fight with DT ;) as it's static for SoC family
> and need to be changed only once when new SoC family introduced.
Fine with me, that can be changed later too no problem.
Regards,
Tony
^ permalink raw reply
* [PATCH 1/7] fore200e: simplify fore200e_bus usage
From: Christoph Hellwig @ 2018-10-09 14:57 UTC (permalink / raw)
To: Chas Williams, netdev; +Cc: linux-atm-general, linux-kernel
In-Reply-To: <20181009145720.32578-1-hch@lst.de>
There is no need to have a global array of the ops, instead PCI and sbus
can have their own instances assigned in *_probe. Also switch to C99
initializers.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
drivers/atm/fore200e.c | 121 +++++++++++++++++++----------------------
1 file changed, 56 insertions(+), 65 deletions(-)
diff --git a/drivers/atm/fore200e.c b/drivers/atm/fore200e.c
index 99a38115b0a8..008bd8541c61 100644
--- a/drivers/atm/fore200e.c
+++ b/drivers/atm/fore200e.c
@@ -106,7 +106,6 @@
static const struct atmdev_ops fore200e_ops;
-static const struct fore200e_bus fore200e_bus[];
static LIST_HEAD(fore200e_boards);
@@ -664,9 +663,31 @@ fore200e_pca_proc_read(struct fore200e* fore200e, char *page)
pci_dev->bus->number, PCI_SLOT(pci_dev->devfn), PCI_FUNC(pci_dev->devfn));
}
+static const struct fore200e_bus fore200e_pci_ops = {
+ .model_name = "PCA-200E",
+ .proc_name = "pca200e",
+ .descr_alignment = 32,
+ .buffer_alignment = 4,
+ .status_alignment = 32,
+ .read = fore200e_pca_read,
+ .write = fore200e_pca_write,
+ .dma_map = fore200e_pca_dma_map,
+ .dma_unmap = fore200e_pca_dma_unmap,
+ .dma_sync_for_cpu = fore200e_pca_dma_sync_for_cpu,
+ .dma_sync_for_device = fore200e_pca_dma_sync_for_device,
+ .dma_chunk_alloc = fore200e_pca_dma_chunk_alloc,
+ .dma_chunk_free = fore200e_pca_dma_chunk_free,
+ .configure = fore200e_pca_configure,
+ .map = fore200e_pca_map,
+ .reset = fore200e_pca_reset,
+ .prom_read = fore200e_pca_prom_read,
+ .unmap = fore200e_pca_unmap,
+ .irq_check = fore200e_pca_irq_check,
+ .irq_ack = fore200e_pca_irq_ack,
+ .proc_read = fore200e_pca_proc_read,
+};
#endif /* CONFIG_PCI */
-
#ifdef CONFIG_SBUS
static u32 fore200e_sba_read(volatile u32 __iomem *addr)
@@ -855,8 +876,32 @@ static int fore200e_sba_proc_read(struct fore200e *fore200e, char *page)
return sprintf(page, " SBUS slot/device:\t\t%d/'%s'\n",
(regs ? regs->which_io : 0), op->dev.of_node->name);
}
-#endif /* CONFIG_SBUS */
+static const struct fore200e_bus fore200e_sbus_ops = {
+ .model_name = "SBA-200E",
+ .proc_name = "sba200e",
+ .descr_alignment = 32,
+ .buffer_alignent = 64,
+ .status_alignment = 32,
+ .read = fore200e_sba_read,
+ .write = fore200e_sba_write,
+ .dma_map = fore200e_sba_dma_map,
+ .dma_unap = fore200e_sba_dma_unmap,
+ .dma_sync_for_cpu = fore200e_sba_dma_sync_for_cpu,
+ .dma_sync_for_device = fore200e_sba_dma_sync_for_device,
+ .dma_chunk_alloc = fore200e_sba_dma_chunk_alloc,
+ .dma_chunk_free = fore200e_sba_dma_chunk_free,
+ .configure = fore200e_sba_configure,
+ .map = fore200e_sba_map,
+ .reset = fore200e_sba_reset,
+ .prom_read = fore200e_sba_prom_read,
+ .unmap = fore200e_sba_unmap,
+ .irq_enable = fore200e_sba_irq_enable,
+ .irq_check = fore200e_sba_irq_check,
+ .irq_ack = fore200e_sba_irq_ack,
+ .proc_read = fore200e_sba_proc_read,
+};
+#endif /* CONFIG_SBUS */
static void
fore200e_tx_irq(struct fore200e* fore200e)
@@ -2631,7 +2676,6 @@ static const struct of_device_id fore200e_sba_match[];
static int fore200e_sba_probe(struct platform_device *op)
{
const struct of_device_id *match;
- const struct fore200e_bus *bus;
struct fore200e *fore200e;
static int index = 0;
int err;
@@ -2639,18 +2683,17 @@ static int fore200e_sba_probe(struct platform_device *op)
match = of_match_device(fore200e_sba_match, &op->dev);
if (!match)
return -EINVAL;
- bus = match->data;
fore200e = kzalloc(sizeof(struct fore200e), GFP_KERNEL);
if (!fore200e)
return -ENOMEM;
- fore200e->bus = bus;
+ fore200e->bus = &fore200e_sbus_ops;
fore200e->bus_dev = op;
fore200e->irq = op->archdata.irqs[0];
fore200e->phys_base = op->resource[0].start;
- sprintf(fore200e->name, "%s-%d", bus->model_name, index);
+ sprintf(fore200e->name, "SBA-200E-%d", index);
err = fore200e_init(fore200e, &op->dev);
if (err < 0) {
@@ -2678,7 +2721,6 @@ static int fore200e_sba_remove(struct platform_device *op)
static const struct of_device_id fore200e_sba_match[] = {
{
.name = SBA200E_PROM_NAME,
- .data = (void *) &fore200e_bus[1],
},
{},
};
@@ -2698,7 +2740,6 @@ static struct platform_driver fore200e_sba_driver = {
static int fore200e_pca_detect(struct pci_dev *pci_dev,
const struct pci_device_id *pci_ent)
{
- const struct fore200e_bus* bus = (struct fore200e_bus*) pci_ent->driver_data;
struct fore200e* fore200e;
int err = 0;
static int index = 0;
@@ -2719,20 +2760,19 @@ static int fore200e_pca_detect(struct pci_dev *pci_dev,
goto out_disable;
}
- fore200e->bus = bus;
+ fore200e->bus = &fore200e_pci_ops;
fore200e->bus_dev = pci_dev;
fore200e->irq = pci_dev->irq;
fore200e->phys_base = pci_resource_start(pci_dev, 0);
- sprintf(fore200e->name, "%s-%d", bus->model_name, index - 1);
+ sprintf(fore200e->name, "PCA-200E-%d", index - 1);
pci_set_master(pci_dev);
- printk(FORE200E "device %s found at 0x%lx, IRQ %s\n",
- fore200e->bus->model_name,
+ printk(FORE200E "device PCA-200E found at 0x%lx, IRQ %s\n",
fore200e->phys_base, fore200e_irq_itoa(fore200e->irq));
- sprintf(fore200e->name, "%s-%d", bus->model_name, index);
+ sprintf(fore200e->name, "PCA-200E-%d", index);
err = fore200e_init(fore200e, &pci_dev->dev);
if (err < 0) {
@@ -2767,8 +2807,7 @@ static void fore200e_pca_remove_one(struct pci_dev *pci_dev)
static const struct pci_device_id fore200e_pca_tbl[] = {
- { PCI_VENDOR_ID_FORE, PCI_DEVICE_ID_FORE_PCA200E, PCI_ANY_ID, PCI_ANY_ID,
- 0, 0, (unsigned long) &fore200e_bus[0] },
+ { PCI_VENDOR_ID_FORE, PCI_DEVICE_ID_FORE_PCA200E, PCI_ANY_ID, PCI_ANY_ID },
{ 0, }
};
@@ -3108,8 +3147,7 @@ module_init(fore200e_module_init);
module_exit(fore200e_module_cleanup);
-static const struct atmdev_ops fore200e_ops =
-{
+static const struct atmdev_ops fore200e_ops = {
.open = fore200e_open,
.close = fore200e_close,
.ioctl = fore200e_ioctl,
@@ -3121,53 +3159,6 @@ static const struct atmdev_ops fore200e_ops =
.owner = THIS_MODULE
};
-
-static const struct fore200e_bus fore200e_bus[] = {
-#ifdef CONFIG_PCI
- { "PCA-200E", "pca200e", 32, 4, 32,
- fore200e_pca_read,
- fore200e_pca_write,
- fore200e_pca_dma_map,
- fore200e_pca_dma_unmap,
- fore200e_pca_dma_sync_for_cpu,
- fore200e_pca_dma_sync_for_device,
- fore200e_pca_dma_chunk_alloc,
- fore200e_pca_dma_chunk_free,
- fore200e_pca_configure,
- fore200e_pca_map,
- fore200e_pca_reset,
- fore200e_pca_prom_read,
- fore200e_pca_unmap,
- NULL,
- fore200e_pca_irq_check,
- fore200e_pca_irq_ack,
- fore200e_pca_proc_read,
- },
-#endif
-#ifdef CONFIG_SBUS
- { "SBA-200E", "sba200e", 32, 64, 32,
- fore200e_sba_read,
- fore200e_sba_write,
- fore200e_sba_dma_map,
- fore200e_sba_dma_unmap,
- fore200e_sba_dma_sync_for_cpu,
- fore200e_sba_dma_sync_for_device,
- fore200e_sba_dma_chunk_alloc,
- fore200e_sba_dma_chunk_free,
- fore200e_sba_configure,
- fore200e_sba_map,
- fore200e_sba_reset,
- fore200e_sba_prom_read,
- fore200e_sba_unmap,
- fore200e_sba_irq_enable,
- fore200e_sba_irq_check,
- fore200e_sba_irq_ack,
- fore200e_sba_proc_read,
- },
-#endif
- {}
-};
-
MODULE_LICENSE("GPL");
#ifdef CONFIG_PCI
#ifdef __LITTLE_ENDIAN__
--
2.19.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox