* Re: [PATCH net-next 1/3] net: dsa: assign switch device in legacy code
From: Andrew Lunn @ 2017-08-05 0:55 UTC (permalink / raw)
To: Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Florian Fainelli
In-Reply-To: <20170804222320.10986-2-vivien.didelot@savoirfairelinux.com>
> @@ -251,8 +251,9 @@ dsa_switch_setup(struct dsa_switch_tree *dst, struct net_device *master,
> ds->cd = cd;
> ds->ops = ops;
> ds->priv = priv;
> + ds->dev = parent;
Hi Vivien
Is this even needed? dsa_switch_alloc() does ds->dev = dev.
Andrew
^ permalink raw reply
* Re: [PATCH net] bpf: fix byte order test in test_verifier
From: David Miller @ 2017-08-04 23:09 UTC (permalink / raw)
To: daniel; +Cc: yhs, alexei.starovoitov, netdev
In-Reply-To: <2914429672dfe6d0420a81dff36853326a7f604d.1501877778.git.daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Fri, 4 Aug 2017 22:24:41 +0200
> We really must check with #if __BYTE_ORDER == XYZ instead of
> just presence of #ifdef __LITTLE_ENDIAN. I noticed that when
> actually running this on big endian machine, the latter test
> resolves to true for user space, same for #ifdef __BIG_ENDIAN.
>
> E.g., looking at endian.h from libc, both are also defined
> there, so we really must test this against __BYTE_ORDER instead
> for proper insns selection. For the kernel, such checks are
> fine though e.g. see 13da9e200fe4 ("Revert "endian: #define
> __BYTE_ORDER"") and 415586c9e6d3 ("UAPI: fix endianness conditionals
> in M32R's asm/stat.h") for some more context, but not for
> user space. Lets also make sure to properly include endian.h.
> After that, suite passes for me:
>
> ./test_verifier: ELF 64-bit MSB executable, [...]
>
> Linux foo 4.13.0-rc3+ #4 SMP Fri Aug 4 06:59:30 EDT 2017 s390x s390x s390x GNU/Linux
>
> Before fix: Summary: 505 PASSED, 11 FAILED
> After fix: Summary: 516 PASSED, 0 FAILED
>
> Fixes: 18f3d6be6be1 ("selftests/bpf: Add test cases to test narrower ctx field loads")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Applied and queued up for -stable, thanks!
^ permalink raw reply
* [PATCH net-next v4 2/2] bpf: add a test case for syscalls/sys_{enter|exit}_* tracepoints
From: Yonghong Song @ 2017-08-04 23:00 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170804230010.2792119-1-yhs@fb.com>
Signed-off-by: Yonghong Song <yhs@fb.com>
---
samples/bpf/Makefile | 4 +++
samples/bpf/syscall_tp_kern.c | 62 +++++++++++++++++++++++++++++++++++++
samples/bpf/syscall_tp_user.c | 71 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 137 insertions(+)
create mode 100644 samples/bpf/syscall_tp_kern.c
create mode 100644 samples/bpf/syscall_tp_user.c
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index 770d46c..f1010fe 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -39,6 +39,7 @@ hostprogs-y += per_socket_stats_example
hostprogs-y += load_sock_ops
hostprogs-y += xdp_redirect
hostprogs-y += xdp_redirect_map
+hostprogs-y += syscall_tp
# Libbpf dependencies
LIBBPF := ../../tools/lib/bpf/bpf.o
@@ -82,6 +83,7 @@ test_map_in_map-objs := bpf_load.o $(LIBBPF) test_map_in_map_user.o
per_socket_stats_example-objs := $(LIBBPF) cookie_uid_helper_example.o
xdp_redirect-objs := bpf_load.o $(LIBBPF) xdp_redirect_user.o
xdp_redirect_map-objs := bpf_load.o $(LIBBPF) xdp_redirect_map_user.o
+syscall_tp-objs := bpf_load.o $(LIBBPF) syscall_tp_user.o
# Tell kbuild to always build the programs
always := $(hostprogs-y)
@@ -125,6 +127,7 @@ always += tcp_iw_kern.o
always += tcp_clamp_kern.o
always += xdp_redirect_kern.o
always += xdp_redirect_map_kern.o
+always += syscall_tp_kern.o
HOSTCFLAGS += -I$(objtree)/usr/include
HOSTCFLAGS += -I$(srctree)/tools/lib/
@@ -163,6 +166,7 @@ HOSTLOADLIBES_xdp_tx_iptunnel += -lelf
HOSTLOADLIBES_test_map_in_map += -lelf
HOSTLOADLIBES_xdp_redirect += -lelf
HOSTLOADLIBES_xdp_redirect_map += -lelf
+HOSTLOADLIBES_syscall_tp += -lelf
# Allows pointing LLC/CLANG to a LLVM backend with bpf support, redefine on cmdline:
# make samples/bpf/ LLC=~/git/llvm/build/bin/llc CLANG=~/git/llvm/build/bin/clang
diff --git a/samples/bpf/syscall_tp_kern.c b/samples/bpf/syscall_tp_kern.c
new file mode 100644
index 0000000..9149c52
--- /dev/null
+++ b/samples/bpf/syscall_tp_kern.c
@@ -0,0 +1,62 @@
+/* Copyright (c) 2017 Facebook
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#include <uapi/linux/bpf.h>
+#include "bpf_helpers.h"
+
+struct syscalls_enter_open_args {
+ unsigned long long unused;
+ long syscall_nr;
+ long filename_ptr;
+ long flags;
+ long mode;
+};
+
+struct syscalls_exit_open_args {
+ unsigned long long unused;
+ long syscall_nr;
+ long ret;
+};
+
+struct bpf_map_def SEC("maps") enter_open_map = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(u32),
+ .value_size = sizeof(u32),
+ .max_entries = 1,
+};
+
+struct bpf_map_def SEC("maps") exit_open_map = {
+ .type = BPF_MAP_TYPE_ARRAY,
+ .key_size = sizeof(u32),
+ .value_size = sizeof(u32),
+ .max_entries = 1,
+};
+
+static __always_inline void count(void *map)
+{
+ u32 key = 0;
+ u32 *value, init_val = 1;
+
+ value = bpf_map_lookup_elem(map, &key);
+ if (value)
+ *value += 1;
+ else
+ bpf_map_update_elem(map, &key, &init_val, BPF_NOEXIST);
+}
+
+SEC("tracepoint/syscalls/sys_enter_open")
+int trace_enter_open(struct syscalls_enter_open_args *ctx)
+{
+ count((void *)&enter_open_map);
+ return 0;
+}
+
+SEC("tracepoint/syscalls/sys_exit_open")
+int trace_enter_exit(struct syscalls_exit_open_args *ctx)
+{
+ count((void *)&exit_open_map);
+ return 0;
+}
diff --git a/samples/bpf/syscall_tp_user.c b/samples/bpf/syscall_tp_user.c
new file mode 100644
index 0000000..a3cb91e
--- /dev/null
+++ b/samples/bpf/syscall_tp_user.c
@@ -0,0 +1,71 @@
+/* Copyright (c) 2017 Facebook
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of version 2 of the GNU General Public
+ * License as published by the Free Software Foundation.
+ */
+#include <stdio.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <signal.h>
+#include <linux/bpf.h>
+#include <string.h>
+#include <linux/perf_event.h>
+#include <errno.h>
+#include <assert.h>
+#include <stdbool.h>
+#include <sys/resource.h>
+#include "libbpf.h"
+#include "bpf_load.h"
+
+/* This program verifies bpf attachment to tracepoint sys_enter_* and sys_exit_*.
+ * This requires kernel CONFIG_FTRACE_SYSCALLS to be set.
+ */
+
+static void verify_map(int map_id)
+{
+ __u32 key = 0;
+ __u32 val;
+
+ if (bpf_map_lookup_elem(map_id, &key, &val) != 0) {
+ fprintf(stderr, "map_lookup failed: %s\n", strerror(errno));
+ return;
+ }
+ if (val == 0)
+ fprintf(stderr, "failed: map #%d returns value 0\n", map_id);
+}
+
+int main(int argc, char **argv)
+{
+ struct rlimit r = {RLIM_INFINITY, RLIM_INFINITY};
+ char filename[256];
+ int fd;
+
+ setrlimit(RLIMIT_MEMLOCK, &r);
+ snprintf(filename, sizeof(filename), "%s_kern.o", argv[0]);
+
+ if (load_bpf_file(filename)) {
+ fprintf(stderr, "%s", bpf_log_buf);
+ return 1;
+ }
+
+ /* current load_bpf_file has perf_event_open default pid = -1
+ * and cpu = 0, which permits attached bpf execution on
+ * all cpus for all pid's. bpf program execution ignores
+ * cpu affinity.
+ */
+ /* trigger some "open" operations */
+ fd = open(filename, O_RDONLY);
+ if (fd < 0) {
+ fprintf(stderr, "open failed: %s\n", strerror(errno));
+ return 1;
+ }
+ close(fd);
+
+ /* verify the map */
+ verify_map(map_fd[0]);
+ verify_map(map_fd[1]);
+
+ return 0;
+}
--
2.9.4
^ permalink raw reply related
* [PATCH net-next v4 1/2] bpf: add support for sys_enter_* and sys_exit_* tracepoints
From: Yonghong Song @ 2017-08-04 23:00 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170804230010.2792119-1-yhs@fb.com>
Currently, bpf programs cannot be attached to sys_enter_* and sys_exit_*
style tracepoints. The iovisor/bcc issue #748
(https://github.com/iovisor/bcc/issues/748) documents this issue.
For example, if you try to attach a bpf program to tracepoints
syscalls/sys_enter_newfstat, you will get the following error:
# ./tools/trace.py t:syscalls:sys_enter_newfstat
Ioctl(PERF_EVENT_IOC_SET_BPF): Invalid argument
Failed to attach BPF to tracepoint
The main reason is that syscalls/sys_enter_* and syscalls/sys_exit_*
tracepoints are treated differently from other tracepoints and there
is no bpf hook to it.
This patch adds bpf support for these syscalls tracepoints by
. permitting bpf attachment in ioctl PERF_EVENT_IOC_SET_BPF
. calling bpf programs in perf_syscall_enter and perf_syscall_exit
The legality of bpf program ctx access is also checked.
Function trace_event_get_offsets returns correct max offset for each
specific syscall tracepoint, which is compared against the maximum offset
access in bpf program.
Signed-off-by: Yonghong Song <yhs@fb.com>
---
include/linux/syscalls.h | 12 ++++++++++
kernel/events/core.c | 10 ++++----
kernel/trace/trace_syscalls.c | 53 +++++++++++++++++++++++++++++++++++++++++--
3 files changed, 69 insertions(+), 6 deletions(-)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 3cb15ea..c917021 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -172,8 +172,20 @@ extern struct trace_event_functions exit_syscall_print_funcs;
static struct syscall_metadata __used \
__attribute__((section("__syscalls_metadata"))) \
*__p_syscall_meta_##sname = &__syscall_meta_##sname;
+
+static inline int is_syscall_trace_event(struct trace_event_call *tp_event)
+{
+ return tp_event->class == &event_class_syscall_enter ||
+ tp_event->class == &event_class_syscall_exit;
+}
+
#else
#define SYSCALL_METADATA(sname, nb, ...)
+
+static inline int is_syscall_trace_event(struct trace_event_call *tp_event)
+{
+ return 0;
+}
#endif
#define SYSCALL_DEFINE0(sname) \
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 426c2ff..a7a6c1d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8050,7 +8050,7 @@ static void perf_event_free_bpf_handler(struct perf_event *event)
static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
{
- bool is_kprobe, is_tracepoint;
+ bool is_kprobe, is_tracepoint, is_syscall_tp;
struct bpf_prog *prog;
if (event->attr.type != PERF_TYPE_TRACEPOINT)
@@ -8061,7 +8061,8 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
is_kprobe = event->tp_event->flags & TRACE_EVENT_FL_UKPROBE;
is_tracepoint = event->tp_event->flags & TRACE_EVENT_FL_TRACEPOINT;
- if (!is_kprobe && !is_tracepoint)
+ is_syscall_tp = is_syscall_trace_event(event->tp_event);
+ if (!is_kprobe && !is_tracepoint && !is_syscall_tp)
/* bpf programs can only be attached to u/kprobe or tracepoint */
return -EINVAL;
@@ -8070,13 +8071,14 @@ static int perf_event_set_bpf_prog(struct perf_event *event, u32 prog_fd)
return PTR_ERR(prog);
if ((is_kprobe && prog->type != BPF_PROG_TYPE_KPROBE) ||
- (is_tracepoint && prog->type != BPF_PROG_TYPE_TRACEPOINT)) {
+ (is_tracepoint && prog->type != BPF_PROG_TYPE_TRACEPOINT) ||
+ (is_syscall_tp && prog->type != BPF_PROG_TYPE_TRACEPOINT)) {
/* valid fd, but invalid bpf program type */
bpf_prog_put(prog);
return -EINVAL;
}
- if (is_tracepoint) {
+ if (is_tracepoint || is_syscall_tp) {
int off = trace_event_get_offsets(event->tp_event);
if (prog->aux->max_ctx_offset > off) {
diff --git a/kernel/trace/trace_syscalls.c b/kernel/trace/trace_syscalls.c
index 5e10395..7a1a920 100644
--- a/kernel/trace/trace_syscalls.c
+++ b/kernel/trace/trace_syscalls.c
@@ -559,11 +559,29 @@ static DECLARE_BITMAP(enabled_perf_exit_syscalls, NR_syscalls);
static int sys_perf_refcount_enter;
static int sys_perf_refcount_exit;
+static int perf_call_bpf_enter(struct bpf_prog *prog, struct pt_regs *regs,
+ struct syscall_metadata *sys_data,
+ struct syscall_trace_enter *rec) {
+ struct syscall_tp_t {
+ unsigned long long regs;
+ unsigned long syscall_nr;
+ unsigned long args[sys_data->nb_args];
+ } param;
+ int i;
+
+ *(struct pt_regs **)¶m = regs;
+ param.syscall_nr = rec->nr;
+ for (i = 0; i < sys_data->nb_args; i++)
+ param.args[i] = rec->args[i];
+ return trace_call_bpf(prog, ¶m);
+}
+
static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
{
struct syscall_metadata *sys_data;
struct syscall_trace_enter *rec;
struct hlist_head *head;
+ struct bpf_prog *prog;
int syscall_nr;
int rctx;
int size;
@@ -578,8 +596,9 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
if (!sys_data)
return;
+ prog = READ_ONCE(sys_data->enter_event->prog);
head = this_cpu_ptr(sys_data->enter_event->perf_events);
- if (hlist_empty(head))
+ if (!prog && hlist_empty(head))
return;
/* get the size after alignment with the u32 buffer size field */
@@ -594,6 +613,13 @@ static void perf_syscall_enter(void *ignore, struct pt_regs *regs, long id)
rec->nr = syscall_nr;
syscall_get_arguments(current, regs, 0, sys_data->nb_args,
(unsigned long *)&rec->args);
+
+ if ((prog && !perf_call_bpf_enter(prog, regs, sys_data, rec)) ||
+ hlist_empty(head)) {
+ perf_swevent_put_recursion_context(rctx);
+ return;
+ }
+
perf_trace_buf_submit(rec, size, rctx,
sys_data->enter_event->event.type, 1, regs,
head, NULL);
@@ -633,11 +659,26 @@ static void perf_sysenter_disable(struct trace_event_call *call)
mutex_unlock(&syscall_trace_lock);
}
+static int perf_call_bpf_exit(struct bpf_prog *prog, struct pt_regs *regs,
+ struct syscall_trace_exit *rec) {
+ struct syscall_tp_t {
+ unsigned long long regs;
+ unsigned long syscall_nr;
+ unsigned long ret;
+ } param;
+
+ *(struct pt_regs **)¶m = regs;
+ param.syscall_nr = rec->nr;
+ param.ret = rec->ret;
+ return trace_call_bpf(prog, ¶m);
+}
+
static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
{
struct syscall_metadata *sys_data;
struct syscall_trace_exit *rec;
struct hlist_head *head;
+ struct bpf_prog *prog;
int syscall_nr;
int rctx;
int size;
@@ -652,8 +693,9 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
if (!sys_data)
return;
+ prog = READ_ONCE(sys_data->exit_event->prog);
head = this_cpu_ptr(sys_data->exit_event->perf_events);
- if (hlist_empty(head))
+ if (!prog && hlist_empty(head))
return;
/* We can probably do that at build time */
@@ -666,6 +708,13 @@ static void perf_syscall_exit(void *ignore, struct pt_regs *regs, long ret)
rec->nr = syscall_nr;
rec->ret = syscall_get_return_value(current, regs);
+
+ if ((prog && !perf_call_bpf_exit(prog, regs, rec)) ||
+ hlist_empty(head)) {
+ perf_swevent_put_recursion_context(rctx);
+ return;
+ }
+
perf_trace_buf_submit(rec, size, rctx, sys_data->exit_event->event.type,
1, regs, head, NULL);
}
--
2.9.4
^ permalink raw reply related
* [PATCH net-next v4 0/2] bpf: add support for sys_{enter|exit}_* tracepoints
From: Yonghong Song @ 2017-08-04 23:00 UTC (permalink / raw)
To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
Currently, bpf programs cannot be attached to sys_enter_* and sys_exit_*
style tracepoints. The main reason is that syscalls/sys_enter_* and syscalls/sys_exit_*
tracepoints are treated differently from other tracepoints and there
is no bpf hook to it.
This patch set adds bpf support for these syscalls tracepoints and also
adds a test case for it.
Changelogs:
v3 -> v4:
- Check the legality of ctx offset access for syscall tracepoint as well.
trace_event_get_offsets will return correct max offset for each
specific syscall tracepoint.
- Use variable length array to avoid hardcode 6 as the maximum
arguments beyond syscall_nr.
v2 -> v3:
- Fix a build issue
v1 -> v2:
- Do not use TRACE_EVENT_FL_CAP_ANY to identify syscall tracepoint.
Instead use trace_event_call->class.
Yonghong Song (2):
bpf: add support for sys_enter_* and sys_exit_* tracepoints
bpf: add a test case for syscalls/sys_{enter|exit}_* tracepoints
include/linux/syscalls.h | 12 ++++++++
kernel/events/core.c | 10 +++---
kernel/trace/trace_syscalls.c | 53 ++++++++++++++++++++++++++++++--
samples/bpf/Makefile | 4 +++
samples/bpf/syscall_tp_kern.c | 62 +++++++++++++++++++++++++++++++++++++
samples/bpf/syscall_tp_user.c | 71 +++++++++++++++++++++++++++++++++++++++++++
6 files changed, 206 insertions(+), 6 deletions(-)
create mode 100644 samples/bpf/syscall_tp_kern.c
create mode 100644 samples/bpf/syscall_tp_user.c
--
2.9.4
^ permalink raw reply
* Re: [PATCH net-next] lwtunnel: replace EXPORT_SYMBOL with EXPORT_SYMBOL_GPL
From: Roopa Prabhu @ 2017-08-04 22:36 UTC (permalink / raw)
To: Andrew Lunn; +Cc: davem@davemloft.net, netdev@vger.kernel.org
In-Reply-To: <20170804222528.GD11696@lunn.ch>
On Fri, Aug 4, 2017 at 3:25 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> On Fri, Aug 04, 2017 at 03:23:37PM -0700, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>> net/core/lwtunnel.c | 26 +++++++++++++-------------
>> 1 file changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
>> index d9cb353..8693ff8 100644
>> --- a/net/core/lwtunnel.c
>> +++ b/net/core/lwtunnel.c
>> @@ -65,7 +65,7 @@ struct lwtunnel_state *lwtunnel_state_alloc(int encap_len)
>>
>> return lws;
>> }
>> -EXPORT_SYMBOL(lwtunnel_state_alloc);
>> +EXPORT_SYMBOL_GPL_GPL(lwtunnel_state_alloc);
>
> Hi Roopa
>
> GPL_GPL?
>
oops, i had fixed it but sent the wrong version. thx
^ permalink raw reply
* Re: uapi: MAX_ADDR_LEN vs. numeric 32
From: Mikko Rapeli @ 2017-08-04 22:34 UTC (permalink / raw)
To: Dmitry V. Levin; +Cc: netdev, Pavel Emelyanov
In-Reply-To: <20170804222519.GA11085@altlinux.org>
On Sat, Aug 05, 2017 at 01:25:19AM +0300, Dmitry V. Levin wrote:
>
> On Sat, Aug 05, 2017 at 12:33:25AM +0300, Mikko Rapeli wrote:
> >
> > I find using MAX_ADDR_LEN better than numeric 32, though I doubt this will
> > change any time soon. Would you mind if I change packet_diag.h and
> > if_link.h to use that instead and fix the userspace compilation
> > problems by including netdevice.h?
>
> The alternative fix, that is, to include <linux/netdevice.h>
> which pulls in other headers and a lot of definitions with them,
> has been mentioned in the discussion, too.
> We decided that the fix that was applied would be the least of all evils.
Ok, that's fine then. I'll drop my netdevice.h inclusion patch.
Thanks,
-Mikko
^ permalink raw reply
* Re: [PATCH net] bpf: fix byte order test in test_verifier
From: Yonghong Song @ 2017-08-04 22:26 UTC (permalink / raw)
To: Daniel Borkmann, davem; +Cc: alexei.starovoitov, netdev
In-Reply-To: <2914429672dfe6d0420a81dff36853326a7f604d.1501877778.git.daniel@iogearbox.net>
On 8/4/17 1:24 PM, Daniel Borkmann wrote:
> We really must check with #if __BYTE_ORDER == XYZ instead of
> just presence of #ifdef __LITTLE_ENDIAN. I noticed that when
> actually running this on big endian machine, the latter test
> resolves to true for user space, same for #ifdef __BIG_ENDIAN.
>
> E.g., looking at endian.h from libc, both are also defined
> there, so we really must test this against __BYTE_ORDER instead
> for proper insns selection. For the kernel, such checks are
> fine though e.g. see 13da9e200fe4 ("Revert "endian: #define
> __BYTE_ORDER"") and 415586c9e6d3 ("UAPI: fix endianness conditionals
> in M32R's asm/stat.h") for some more context, but not for
> user space. Lets also make sure to properly include endian.h.
> After that, suite passes for me:
>
> ./test_verifier: ELF 64-bit MSB executable, [...]
>
> Linux foo 4.13.0-rc3+ #4 SMP Fri Aug 4 06:59:30 EDT 2017 s390x s390x s390x GNU/Linux
>
> Before fix: Summary: 505 PASSED, 11 FAILED
> After fix: Summary: 516 PASSED, 0 FAILED
>
> Fixes: 18f3d6be6be1 ("selftests/bpf: Add test cases to test narrower ctx field loads")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> ---
> tools/testing/selftests/bpf/test_verifier.c | 19 ++++++++++---------
> 1 file changed, 10 insertions(+), 9 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
> index addea82..d3ed732 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -8,6 +8,7 @@
> * License as published by the Free Software Foundation.
> */
>
> +#include <endian.h>
> #include <asm/types.h>
> #include <linux/types.h>
> #include <stdint.h>
> @@ -1098,7 +1099,7 @@ struct test_val {
> "check skb->hash byte load permitted",
> .insns = {
> BPF_MOV64_IMM(BPF_REG_0, 0),
> -#ifdef __LITTLE_ENDIAN
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_1,
> offsetof(struct __sk_buff, hash)),
> #else
> @@ -1135,7 +1136,7 @@ struct test_val {
> "check skb->hash byte load not permitted 3",
> .insns = {
> BPF_MOV64_IMM(BPF_REG_0, 0),
> -#ifdef __LITTLE_ENDIAN
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_1,
> offsetof(struct __sk_buff, hash) + 3),
> #else
> @@ -1244,7 +1245,7 @@ struct test_val {
> "check skb->hash half load permitted",
> .insns = {
> BPF_MOV64_IMM(BPF_REG_0, 0),
> -#ifdef __LITTLE_ENDIAN
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
> offsetof(struct __sk_buff, hash)),
> #else
> @@ -1259,7 +1260,7 @@ struct test_val {
> "check skb->hash half load not permitted",
> .insns = {
> BPF_MOV64_IMM(BPF_REG_0, 0),
> -#ifdef __LITTLE_ENDIAN
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
> offsetof(struct __sk_buff, hash) + 2),
> #else
> @@ -5422,7 +5423,7 @@ struct test_val {
> "check bpf_perf_event_data->sample_period byte load permitted",
> .insns = {
> BPF_MOV64_IMM(BPF_REG_0, 0),
> -#ifdef __LITTLE_ENDIAN
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_1,
> offsetof(struct bpf_perf_event_data, sample_period)),
> #else
> @@ -5438,7 +5439,7 @@ struct test_val {
> "check bpf_perf_event_data->sample_period half load permitted",
> .insns = {
> BPF_MOV64_IMM(BPF_REG_0, 0),
> -#ifdef __LITTLE_ENDIAN
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
> offsetof(struct bpf_perf_event_data, sample_period)),
> #else
> @@ -5454,7 +5455,7 @@ struct test_val {
> "check bpf_perf_event_data->sample_period word load permitted",
> .insns = {
> BPF_MOV64_IMM(BPF_REG_0, 0),
> -#ifdef __LITTLE_ENDIAN
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_W, BPF_REG_0, BPF_REG_1,
> offsetof(struct bpf_perf_event_data, sample_period)),
> #else
> @@ -5481,7 +5482,7 @@ struct test_val {
> "check skb->data half load not permitted",
> .insns = {
> BPF_MOV64_IMM(BPF_REG_0, 0),
> -#ifdef __LITTLE_ENDIAN
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
> offsetof(struct __sk_buff, data)),
> #else
> @@ -5497,7 +5498,7 @@ struct test_val {
> "check skb->tc_classid half load not permitted for lwt prog",
> .insns = {
> BPF_MOV64_IMM(BPF_REG_0, 0),
> -#ifdef __LITTLE_ENDIAN
> +#if __BYTE_ORDER == __LITTLE_ENDIAN
> BPF_LDX_MEM(BPF_H, BPF_REG_0, BPF_REG_1,
> offsetof(struct __sk_buff, tc_classid)),
> #else
>
The change looks good to me.
Acked-by: Yonghong <yhs@fb.com>
^ permalink raw reply
* Re: [PATCH net-next] lwtunnel: replace EXPORT_SYMBOL with EXPORT_SYMBOL_GPL
From: Andrew Lunn @ 2017-08-04 22:25 UTC (permalink / raw)
To: Roopa Prabhu; +Cc: davem, netdev
In-Reply-To: <1501885417-18966-1-git-send-email-roopa@cumulusnetworks.com>
On Fri, Aug 04, 2017 at 03:23:37PM -0700, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> net/core/lwtunnel.c | 26 +++++++++++++-------------
> 1 file changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
> index d9cb353..8693ff8 100644
> --- a/net/core/lwtunnel.c
> +++ b/net/core/lwtunnel.c
> @@ -65,7 +65,7 @@ struct lwtunnel_state *lwtunnel_state_alloc(int encap_len)
>
> return lws;
> }
> -EXPORT_SYMBOL(lwtunnel_state_alloc);
> +EXPORT_SYMBOL_GPL_GPL(lwtunnel_state_alloc);
Hi Roopa
GPL_GPL?
Andrew
^ permalink raw reply
* Re: uapi: MAX_ADDR_LEN vs. numeric 32
From: Dmitry V. Levin @ 2017-08-04 22:25 UTC (permalink / raw)
To: Mikko Rapeli; +Cc: netdev, Pavel Emelyanov
In-Reply-To: <20170804213325.GC28459@lakka.kapsi.fi>
[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]
Hi,
On Sat, Aug 05, 2017 at 12:33:25AM +0300, Mikko Rapeli wrote:
> First, thanks Dmitry for fixing several uapi compilation problems in
> user space. I got a bit demotivated
That's quite understandable.
> about the slow review progress, e.g.
> no feedback what so ever, on some of the patches, but lets try again...
>
> I rebased my tree now and saw
>
> commit 745cb7f8a5de0805cade3de3991b7a95317c7c73
> Author: Dmitry V. Levin <ldv@altlinux.org>
> Date: Tue Mar 7 23:50:50 2017 +0300
>
> uapi: fix linux/packet_diag.h userspace compilation error
>
> which does:
>
> --- a/include/uapi/linux/packet_diag.h
> +++ b/include/uapi/linux/packet_diag.h
> @@ -64,7 +64,7 @@ struct packet_diag_mclist {
> __u32 pdmc_count;
> __u16 pdmc_type;
> __u16 pdmc_alen;
> - __u8 pdmc_addr[MAX_ADDR_LEN];
> + __u8 pdmc_addr[32]; /* MAX_ADDR_LEN */
> };
>
> struct packet_diag_ring {
>
> In my tree I had fixed that case with:
>
> --- a/include/uapi/linux/packet_diag.h
> +++ b/include/uapi/linux/packet_diag.h
> @@ -2,6 +2,7 @@
> #define __PACKET_DIAG_H__
>
> #include <linux/types.h>
> +#include <linux/netdevice.h>
>
> struct packet_diag_req {
> __u8 sdiag_family;
>
> since netdevice.h has the definition also in user space
>
> #define MAX_ADDR_LEN 32 /* Largest hardware address length */
>
> I find using MAX_ADDR_LEN better than numeric 32, though I doubt this will
> change any time soon. Would you mind if I change packet_diag.h and
> if_link.h to use that instead and fix the userspace compilation
> problems by including netdevice.h?
The alternative fix, that is, to include <linux/netdevice.h>
which pulls in other headers and a lot of definitions with them,
has been mentioned in the discussion, too.
We decided that the fix that was applied would be the least of all evils.
--
ldv
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH net-next] lwtunnel: replace EXPORT_SYMBOL with EXPORT_SYMBOL_GPL
From: Roopa Prabhu @ 2017-08-04 22:23 UTC (permalink / raw)
To: davem; +Cc: netdev
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
net/core/lwtunnel.c | 26 +++++++++++++-------------
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/net/core/lwtunnel.c b/net/core/lwtunnel.c
index d9cb353..8693ff8 100644
--- a/net/core/lwtunnel.c
+++ b/net/core/lwtunnel.c
@@ -65,7 +65,7 @@ struct lwtunnel_state *lwtunnel_state_alloc(int encap_len)
return lws;
}
-EXPORT_SYMBOL(lwtunnel_state_alloc);
+EXPORT_SYMBOL_GPL_GPL(lwtunnel_state_alloc);
static const struct lwtunnel_encap_ops __rcu *
lwtun_encaps[LWTUNNEL_ENCAP_MAX + 1] __read_mostly;
@@ -80,7 +80,7 @@ int lwtunnel_encap_add_ops(const struct lwtunnel_encap_ops *ops,
&lwtun_encaps[num],
NULL, ops) ? 0 : -1;
}
-EXPORT_SYMBOL(lwtunnel_encap_add_ops);
+EXPORT_SYMBOL_GPL(lwtunnel_encap_add_ops);
int lwtunnel_encap_del_ops(const struct lwtunnel_encap_ops *ops,
unsigned int encap_type)
@@ -99,7 +99,7 @@ int lwtunnel_encap_del_ops(const struct lwtunnel_encap_ops *ops,
return ret;
}
-EXPORT_SYMBOL(lwtunnel_encap_del_ops);
+EXPORT_SYMBOL_GPL(lwtunnel_encap_del_ops);
int lwtunnel_build_state(u16 encap_type,
struct nlattr *encap, unsigned int family,
@@ -138,7 +138,7 @@ int lwtunnel_build_state(u16 encap_type,
return ret;
}
-EXPORT_SYMBOL(lwtunnel_build_state);
+EXPORT_SYMBOL_GPL(lwtunnel_build_state);
int lwtunnel_valid_encap_type(u16 encap_type, struct netlink_ext_ack *extack)
{
@@ -175,7 +175,7 @@ int lwtunnel_valid_encap_type(u16 encap_type, struct netlink_ext_ack *extack)
return ret;
}
-EXPORT_SYMBOL(lwtunnel_valid_encap_type);
+EXPORT_SYMBOL_GPL(lwtunnel_valid_encap_type);
int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int remaining,
struct netlink_ext_ack *extack)
@@ -205,7 +205,7 @@ int lwtunnel_valid_encap_type_attr(struct nlattr *attr, int remaining,
return 0;
}
-EXPORT_SYMBOL(lwtunnel_valid_encap_type_attr);
+EXPORT_SYMBOL_GPL(lwtunnel_valid_encap_type_attr);
void lwtstate_free(struct lwtunnel_state *lws)
{
@@ -219,7 +219,7 @@ void lwtstate_free(struct lwtunnel_state *lws)
}
module_put(ops->owner);
}
-EXPORT_SYMBOL(lwtstate_free);
+EXPORT_SYMBOL_GPL(lwtstate_free);
int lwtunnel_fill_encap(struct sk_buff *skb, struct lwtunnel_state *lwtstate)
{
@@ -259,7 +259,7 @@ int lwtunnel_fill_encap(struct sk_buff *skb, struct lwtunnel_state *lwtstate)
return (ret == -EOPNOTSUPP ? 0 : ret);
}
-EXPORT_SYMBOL(lwtunnel_fill_encap);
+EXPORT_SYMBOL_GPL(lwtunnel_fill_encap);
int lwtunnel_get_encap_size(struct lwtunnel_state *lwtstate)
{
@@ -281,7 +281,7 @@ int lwtunnel_get_encap_size(struct lwtunnel_state *lwtstate)
return ret;
}
-EXPORT_SYMBOL(lwtunnel_get_encap_size);
+EXPORT_SYMBOL_GPL(lwtunnel_get_encap_size);
int lwtunnel_cmp_encap(struct lwtunnel_state *a, struct lwtunnel_state *b)
{
@@ -309,7 +309,7 @@ int lwtunnel_cmp_encap(struct lwtunnel_state *a, struct lwtunnel_state *b)
return ret;
}
-EXPORT_SYMBOL(lwtunnel_cmp_encap);
+EXPORT_SYMBOL_GPL(lwtunnel_cmp_encap);
int lwtunnel_output(struct net *net, struct sock *sk, struct sk_buff *skb)
{
@@ -343,7 +343,7 @@ int lwtunnel_output(struct net *net, struct sock *sk, struct sk_buff *skb)
return ret;
}
-EXPORT_SYMBOL(lwtunnel_output);
+EXPORT_SYMBOL_GPL(lwtunnel_output);
int lwtunnel_xmit(struct sk_buff *skb)
{
@@ -378,7 +378,7 @@ int lwtunnel_xmit(struct sk_buff *skb)
return ret;
}
-EXPORT_SYMBOL(lwtunnel_xmit);
+EXPORT_SYMBOL_GPL(lwtunnel_xmit);
int lwtunnel_input(struct sk_buff *skb)
{
@@ -412,4 +412,4 @@ int lwtunnel_input(struct sk_buff *skb)
return ret;
}
-EXPORT_SYMBOL(lwtunnel_input);
+EXPORT_SYMBOL_GPL(lwtunnel_input);
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 3/3] net: dsa: remove useless args of dsa_slave_create
From: Vivien Didelot @ 2017-08-04 22:23 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170804222320.10986-1-vivien.didelot@savoirfairelinux.com>
dsa_slave_create currently takes 4 arguments while it only needs the
related dsa_port and its name. Remove all other arguments.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa2.c | 2 +-
net/dsa/dsa_priv.h | 3 +--
net/dsa/legacy.c | 2 +-
net/dsa/slave.c | 14 +++++++-------
4 files changed, 10 insertions(+), 11 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index 2a0120493cf1..cceaa4dd9f53 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -275,7 +275,7 @@ static int dsa_user_port_apply(struct dsa_port *port)
if (!name)
name = "eth%d";
- err = dsa_slave_create(ds, ds->dev, port->index, name);
+ err = dsa_slave_create(port, name);
if (err) {
dev_warn(ds->dev, "Failed to create slave %d: %d\n",
port->index, err);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 46851c91c7fe..73426f9c2cca 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -148,8 +148,7 @@ int dsa_port_vlan_dump(struct dsa_port *dp,
extern const struct dsa_device_ops notag_netdev_ops;
void dsa_slave_mii_bus_init(struct dsa_switch *ds);
void dsa_cpu_port_ethtool_init(struct ethtool_ops *ops);
-int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
- int port, const char *name);
+int dsa_slave_create(struct dsa_port *port, const char *name);
void dsa_slave_destroy(struct net_device *slave_dev);
int dsa_slave_suspend(struct net_device *slave_dev);
int dsa_slave_resume(struct net_device *slave_dev);
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 05be0bc10735..db7c1eaad078 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -194,7 +194,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds,
if (!(ds->enabled_port_mask & (1 << i)))
continue;
- ret = dsa_slave_create(ds, ds->dev, i, cd->port_names[i]);
+ ret = dsa_slave_create(&ds->ports[i], cd->port_names[i]);
if (ret < 0)
netdev_err(master, "[%d]: can't create dsa slave device for port %d(%s): %d\n",
index, i, cd->port_names[i], ret);
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index e196562035b1..c8eb33746850 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1165,9 +1165,9 @@ int dsa_slave_resume(struct net_device *slave_dev)
return 0;
}
-int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
- int port, const char *name)
+int dsa_slave_create(struct dsa_port *port, const char *name)
{
+ struct dsa_switch *ds = port->ds;
struct dsa_switch_tree *dst = ds->dst;
struct net_device *master;
struct net_device *slave_dev;
@@ -1197,13 +1197,13 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
netdev_for_each_tx_queue(slave_dev, dsa_slave_set_lockdep_class_one,
NULL);
- SET_NETDEV_DEV(slave_dev, parent);
- slave_dev->dev.of_node = ds->ports[port].dn;
+ SET_NETDEV_DEV(slave_dev, port->ds->dev);
+ slave_dev->dev.of_node = port->dn;
slave_dev->vlan_features = master->vlan_features;
p = netdev_priv(slave_dev);
u64_stats_init(&p->stats64.syncp);
- p->dp = &ds->ports[port];
+ p->dp = port;
INIT_LIST_HEAD(&p->mall_tc_list);
p->xmit = dst->tag_ops->xmit;
@@ -1211,12 +1211,12 @@ int dsa_slave_create(struct dsa_switch *ds, struct device *parent,
p->old_link = -1;
p->old_duplex = -1;
- ds->ports[port].netdev = slave_dev;
+ port->netdev = slave_dev;
ret = register_netdev(slave_dev);
if (ret) {
netdev_err(master, "error %d registering interface %s\n",
ret, slave_dev->name);
- ds->ports[port].netdev = NULL;
+ port->netdev = NULL;
free_netdev(slave_dev);
return ret;
}
--
2.13.3
^ permalink raw reply related
* [PATCH net-next 2/3] net: dsa: remove useless args of dsa_cpu_dsa_setup
From: Vivien Didelot @ 2017-08-04 22:23 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170804222320.10986-1-vivien.didelot@savoirfairelinux.com>
dsa_cpu_dsa_setup currently takes 4 arguments but they are all available
from the dsa_port argument. Remove all others.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/dsa.c | 10 +++++-----
net/dsa/dsa2.c | 4 ++--
net/dsa/dsa_priv.h | 3 +--
net/dsa/legacy.c | 4 +---
4 files changed, 9 insertions(+), 12 deletions(-)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 0ba842c08dd3..64db6eece3c1 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -67,17 +67,17 @@ const struct dsa_device_ops *dsa_device_ops[DSA_TAG_LAST] = {
[DSA_TAG_PROTO_NONE] = &none_ops,
};
-int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
- struct dsa_port *dport, int port)
+int dsa_cpu_dsa_setup(struct dsa_port *port)
{
- struct device_node *port_dn = dport->dn;
+ struct device_node *port_dn = port->dn;
+ struct dsa_switch *ds = port->ds;
struct phy_device *phydev;
int ret, mode;
if (of_phy_is_fixed_link(port_dn)) {
ret = of_phy_register_fixed_link(port_dn);
if (ret) {
- dev_err(dev, "failed to register fixed PHY\n");
+ dev_err(ds->dev, "failed to register fixed PHY\n");
return ret;
}
phydev = of_phy_find_device(port_dn);
@@ -90,7 +90,7 @@ int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
genphy_config_init(phydev);
genphy_read_status(phydev);
if (ds->ops->adjust_link)
- ds->ops->adjust_link(ds, port, phydev);
+ ds->ops->adjust_link(ds, port->index, phydev);
put_device(&phydev->mdio.dev);
}
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index c442051d5a55..2a0120493cf1 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -219,7 +219,7 @@ static int dsa_dsa_port_apply(struct dsa_port *port)
struct dsa_switch *ds = port->ds;
int err;
- err = dsa_cpu_dsa_setup(ds, ds->dev, port, port->index);
+ err = dsa_cpu_dsa_setup(port);
if (err) {
dev_warn(ds->dev, "Failed to setup dsa port %d: %d\n",
port->index, err);
@@ -243,7 +243,7 @@ static int dsa_cpu_port_apply(struct dsa_port *port)
struct dsa_switch *ds = port->ds;
int err;
- err = dsa_cpu_dsa_setup(ds, ds->dev, port, port->index);
+ err = dsa_cpu_dsa_setup(port);
if (err) {
dev_warn(ds->dev, "Failed to setup cpu port %d: %d\n",
port->index, err);
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 7aa0656296c2..46851c91c7fe 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -101,8 +101,7 @@ struct dsa_slave_priv {
};
/* dsa.c */
-int dsa_cpu_dsa_setup(struct dsa_switch *ds, struct device *dev,
- struct dsa_port *dport, int port);
+int dsa_cpu_dsa_setup(struct dsa_port *port);
void dsa_cpu_dsa_destroy(struct dsa_port *dport);
const struct dsa_device_ops *dsa_resolve_tag_protocol(int tag_protocol);
int dsa_cpu_port_ethtool_setup(struct dsa_port *cpu_dp);
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index c565787e1c78..05be0bc10735 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -80,15 +80,13 @@ dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
/* basic switch operations **************************************************/
static int dsa_cpu_dsa_setups(struct dsa_switch *ds)
{
- struct dsa_port *dport;
int ret, port;
for (port = 0; port < ds->num_ports; port++) {
if (!(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port)))
continue;
- dport = &ds->ports[port];
- ret = dsa_cpu_dsa_setup(ds, ds->dev, dport, port);
+ ret = dsa_cpu_dsa_setup(&ds->ports[port]);
if (ret)
return ret;
}
--
2.13.3
^ permalink raw reply related
* [PATCH net-next 1/3] net: dsa: assign switch device in legacy code
From: Vivien Didelot @ 2017-08-04 22:23 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
In-Reply-To: <20170804222320.10986-1-vivien.didelot@savoirfairelinux.com>
Assign the parent device to the dev member of the newly allocated
dsa_switch structure in the legacy dsa_switch_setup function, so that
the underlying dsa_switch_setup_one and dsa_cpu_dsa_setups functions can
access it instead of requiring an additional struct device argument.
Signed-off-by: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
---
net/dsa/legacy.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/net/dsa/legacy.c b/net/dsa/legacy.c
index 1d7a3282f2a7..c565787e1c78 100644
--- a/net/dsa/legacy.c
+++ b/net/dsa/legacy.c
@@ -78,7 +78,7 @@ dsa_switch_probe(struct device *parent, struct device *host_dev, int sw_addr,
}
/* basic switch operations **************************************************/
-static int dsa_cpu_dsa_setups(struct dsa_switch *ds, struct device *dev)
+static int dsa_cpu_dsa_setups(struct dsa_switch *ds)
{
struct dsa_port *dport;
int ret, port;
@@ -88,15 +88,15 @@ static int dsa_cpu_dsa_setups(struct dsa_switch *ds, struct device *dev)
continue;
dport = &ds->ports[port];
- ret = dsa_cpu_dsa_setup(ds, dev, dport, port);
+ ret = dsa_cpu_dsa_setup(ds, ds->dev, dport, port);
if (ret)
return ret;
}
return 0;
}
-static int dsa_switch_setup_one(struct dsa_switch *ds, struct net_device *master,
- struct device *parent)
+static int dsa_switch_setup_one(struct dsa_switch *ds,
+ struct net_device *master)
{
const struct dsa_switch_ops *ops = ds->ops;
struct dsa_switch_tree *dst = ds->dst;
@@ -176,7 +176,7 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct net_device *master
}
if (!ds->slave_mii_bus && ops->phy_read) {
- ds->slave_mii_bus = devm_mdiobus_alloc(parent);
+ ds->slave_mii_bus = devm_mdiobus_alloc(ds->dev);
if (!ds->slave_mii_bus)
return -ENOMEM;
dsa_slave_mii_bus_init(ds);
@@ -196,14 +196,14 @@ static int dsa_switch_setup_one(struct dsa_switch *ds, struct net_device *master
if (!(ds->enabled_port_mask & (1 << i)))
continue;
- ret = dsa_slave_create(ds, parent, i, cd->port_names[i]);
+ ret = dsa_slave_create(ds, ds->dev, i, cd->port_names[i]);
if (ret < 0)
netdev_err(master, "[%d]: can't create dsa slave device for port %d(%s): %d\n",
index, i, cd->port_names[i], ret);
}
/* Perform configuration of the CPU and DSA ports */
- ret = dsa_cpu_dsa_setups(ds, parent);
+ ret = dsa_cpu_dsa_setups(ds);
if (ret < 0)
netdev_err(master, "[%d] : can't configure CPU and DSA ports\n",
index);
@@ -251,8 +251,9 @@ dsa_switch_setup(struct dsa_switch_tree *dst, struct net_device *master,
ds->cd = cd;
ds->ops = ops;
ds->priv = priv;
+ ds->dev = parent;
- ret = dsa_switch_setup_one(ds, master, parent);
+ ret = dsa_switch_setup_one(ds, master);
if (ret)
return ERR_PTR(ret);
--
2.13.3
^ permalink raw reply related
* [PATCH net-next 0/3] net: dsa: remove unnecessary arguments
From: Vivien Didelot @ 2017-08-04 22:23 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, kernel, David S. Miller, Florian Fainelli,
Andrew Lunn, Vivien Didelot
Several DSA core functions take many arguments, mostly because the
legacy code does not assign ds->dev. This patch series assigns ds->dev
in legacy and removes the unnecessary arguments of these functions,
where either the dsa_switch or dsa_port argument is enough.
Vivien Didelot (3):
net: dsa: assign switch device in legacy code
net: dsa: remove useless args of dsa_cpu_dsa_setup
net: dsa: remove useless args of dsa_slave_create
net/dsa/dsa.c | 10 +++++-----
net/dsa/dsa2.c | 6 +++---
net/dsa/dsa_priv.h | 6 ++----
net/dsa/legacy.c | 19 +++++++++----------
net/dsa/slave.c | 14 +++++++-------
5 files changed, 26 insertions(+), 29 deletions(-)
--
2.13.3
^ permalink raw reply
* Re: [PATCH v5] ss: Enclose IPv6 address in brackets
From: Stephen Hemminger @ 2017-08-04 22:13 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Florian Lehner, netdev
In-Reply-To: <1501879587.25002.49.camel@edumazet-glaptop3.roam.corp.google.com>
On Fri, 04 Aug 2017 13:46:27 -0700
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2017-08-04 at 12:05 -0700, Stephen Hemminger wrote:
> > On Fri, 4 Aug 2017 20:02:52 +0200
> > Florian Lehner <dev@der-flo.net> wrote:
> >
> > > diff --git a/misc/ss.c b/misc/ss.c
> > > index 12763c9..83683b5 100644
> > > --- a/misc/ss.c
> > > +++ b/misc/ss.c
> > > @@ -1046,8 +1046,9 @@ do_numeric:
> > >
> > > static void inet_addr_print(const inet_prefix *a, int port, unsigned
> > > int ifindex)
> > > {
> >
> > Your email client is wrapping long lines which leads to malformed patch.
> >
> > You didn't need buf2, and the code was more complex than it needed to be.
> >
> > Rather than waiting for yet another version, I just merged in similar
> > code.
>
>
> Also, is this new format accepted for the filter ?
>
> ss src [::1]:22
It looks like ss would always do that (from earliest git version).
^ permalink raw reply
* Re: [net-next PATCH] net: devmap fix mutex in rcu critical section
From: John Fastabend @ 2017-08-04 21:58 UTC (permalink / raw)
To: davem, daniel, alexander.levin; +Cc: netdev
In-Reply-To: <20170804212132.7678.11425.stgit@john-Precision-Tower-5810>
On 08/04/2017 02:21 PM, John Fastabend wrote:
> Originally we used a mutex to protect concurrent devmap update
> and delete operations from racing with netdev unregister notifier
> callbacks.
>
[...]
> }
> @@ -396,22 +385,20 @@ static int dev_map_notification(struct notifier_block *notifier,
>
Daniel reminds me this is not in a rcu_read_lock/unlock() section as
needed, so v2 on its way. Thanks!
> switch (event) {
> case NETDEV_UNREGISTER:
> - mutex_lock(&dev_map_list_mutex);
rcu_read_lock();
> list_for_each_entry(dtab, &dev_map_list, list) {
> for (i = 0; i < dtab->map.max_entries; i++) {
> - struct bpf_dtab_netdev *dev;
> + struct bpf_dtab_netdev *dev, *odev;
>
> - dev = dtab->netdev_map[i];
> + dev = READ_ONCE(dtab->netdev_map[i]);
> if (!dev ||
> dev->dev->ifindex != netdev->ifindex)
> continue;
> - dev = xchg(&dtab->netdev_map[i], NULL);
> - if (dev)
> + odev = cmpxchg(&dtab->netdev_map[i], dev, NULL);
> + if (dev == odev)
> call_rcu(&dev->rcu,
> __dev_map_entry_free);
> }
> }
rcu_read_unlock();
> - mutex_unlock(&dev_map_list_mutex);
> break;
> default:
> break;
>
^ permalink raw reply
* uapi: MAX_ADDR_LEN vs. numeric 32
From: Mikko Rapeli @ 2017-08-04 21:33 UTC (permalink / raw)
To: Dmitry V. Levin; +Cc: netdev, Pavel Emelyanov
Hi,
First, thanks Dmitry for fixing several uapi compilation problems in
user space. I got a bit demotivated about the slow review progress, e.g.
no feedback what so ever, on some of the patches, but lets try again...
I rebased my tree now and saw
commit 745cb7f8a5de0805cade3de3991b7a95317c7c73
Author: Dmitry V. Levin <ldv@altlinux.org>
Date: Tue Mar 7 23:50:50 2017 +0300
uapi: fix linux/packet_diag.h userspace compilation error
which does:
--- a/include/uapi/linux/packet_diag.h
+++ b/include/uapi/linux/packet_diag.h
@@ -64,7 +64,7 @@ struct packet_diag_mclist {
__u32 pdmc_count;
__u16 pdmc_type;
__u16 pdmc_alen;
- __u8 pdmc_addr[MAX_ADDR_LEN];
+ __u8 pdmc_addr[32]; /* MAX_ADDR_LEN */
};
struct packet_diag_ring {
In my tree I had fixed that case with:
--- a/include/uapi/linux/packet_diag.h
+++ b/include/uapi/linux/packet_diag.h
@@ -2,6 +2,7 @@
#define __PACKET_DIAG_H__
#include <linux/types.h>
+#include <linux/netdevice.h>
struct packet_diag_req {
__u8 sdiag_family;
since netdevice.h has the definition also in user space
#define MAX_ADDR_LEN 32 /* Largest hardware address length */
I find using MAX_ADDR_LEN better than numeric 32, though I doubt this will
change any time soon. Would you mind if I change packet_diag.h and
if_link.h to use that instead and fix the userspace compilation
problems by including netdevice.h?
Thanks,
-Mikko
^ permalink raw reply
* Re: [PATCH] PCI: Update ACS quirk for more Intel 10G NICs
From: Roland Dreier @ 2017-08-04 21:47 UTC (permalink / raw)
To: Alex Williamson
Cc: Bjorn Helgaas, Bjorn Helgaas, linux-pci, netdev@vger.kernel.org,
Emil Tantilov
In-Reply-To: <20170803161623.31e5fb86@w520.home>
> I think the conclusion is that a hard-wired ACS capability is a
> positive indication of isolation for a multifunction device, the code
> is intended to support this and appears to do so, and Roland was going
> to investigate the sightings that inspired this patch in more detail.
> Dropping for now is appropriate. Thanks,
That's right. I confirmed that the issue we found was due to another PCI quirk.
It may make sense to add more 82599 variants to the table, but the X540 and X550
work without a quirk.
Sorry for the noise.
- R.
^ permalink raw reply
* [PATCH] of_mdio: use of_property_read_u32_array()
From: Sergei Shtylyov @ 2017-08-04 21:43 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Rob Herring, Frank Rowand, netdev,
devicetree
Cc: Sergei Shtylyov
[-- Attachment #1: of_mdio-use-of_property_read_u32_array.patch --]
[-- Type: text/plain, Size: 2069 bytes --]
The "fixed-link" prop support predated of_property_read_u32_array(), so
basically had to open-code it. Using the modern API saves 24 bytes of the
object code (ARM gcc 4.8.5); the only behavior change would be that the
prop length check is now less strict (however the strict pre-check done
in of_phy_is_fixed_link() is left intact anyway)...
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
The patch is against the 'dt/next' branch of Rob Herring's 'linux-git' repo
plus the previously posted patch killing the useless local variable in
of_phy_register_fixed_link().
drivers/of/of_mdio.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
Index: linux/drivers/of/of_mdio.c
===================================================================
--- linux.orig/drivers/of/of_mdio.c
+++ linux/drivers/of/of_mdio.c
@@ -421,10 +421,10 @@ int of_phy_register_fixed_link(struct de
{
struct fixed_phy_status status = {};
struct device_node *fixed_link_node;
- const __be32 *fixed_link_prop;
+ u32 fixed_link_prop[5];
struct phy_device *phy;
const char *managed;
- int link_gpio, len;
+ int link_gpio;
if (of_property_read_string(np, "managed", &managed) == 0) {
if (strcmp(managed, "in-band-status") == 0) {
@@ -459,13 +459,13 @@ int of_phy_register_fixed_link(struct de
}
/* Old binding */
- fixed_link_prop = of_get_property(np, "fixed-link", &len);
- if (fixed_link_prop && len == (5 * sizeof(__be32))) {
+ if (of_property_read_u32_array(np, "fixed-link", fixed_link_prop,
+ ARRAY_SIZE(fixed_link_prop)) == 0) {
status.link = 1;
- status.duplex = be32_to_cpu(fixed_link_prop[1]);
- status.speed = be32_to_cpu(fixed_link_prop[2]);
- status.pause = be32_to_cpu(fixed_link_prop[3]);
- status.asym_pause = be32_to_cpu(fixed_link_prop[4]);
+ status.duplex = fixed_link_prop[1];
+ status.speed = fixed_link_prop[2];
+ status.pause = fixed_link_prop[3];
+ status.asym_pause = fixed_link_prop[4];
phy = fixed_phy_register(PHY_POLL, &status, -1, np);
return PTR_ERR_OR_ZERO(phy);
}
^ permalink raw reply
* [net-next PATCH] net: devmap fix mutex in rcu critical section
From: John Fastabend @ 2017-08-04 21:21 UTC (permalink / raw)
To: davem, daniel, alexander.levin; +Cc: netdev, john.fastabend
Originally we used a mutex to protect concurrent devmap update
and delete operations from racing with netdev unregister notifier
callbacks.
The notifier hook is needed because we increment the netdev ref
count when a dev is added to the devmap. This ensures the netdev
reference is valid in the datapath. However, we don't want to block
unregister events, hence the initial mutex and notifier handler.
The concern was in the notifier hook we search the map for dev
entries that hold a refcnt on the net device being torn down. But,
in order to do this we require two steps,
(i) dereference the netdev: dev = rcu_dereference(map[i])
(ii) test ifindex: dev->ifindex == removing_ifindex
and then finally we can swap in the NULL dev in the map via an
xchg operation,
xchg(map[i], NULL)
The danger here is a concurrent update could run a different
xchg op concurrently leading us to replace the new dev with a
NULL dev incorrectly.
CPU 1 CPU 2
notifier hook bpf devmap update
dev = rcu_dereference(map[i])
dev = rcu_dereference(map[i])
xchg(map[i]), new_dev);
rcu_call(dev,...)
xchg(map[i], NULL)
The above flow would create the incorrect state with the dev
reference in the update path being lost. To resolve this the
original code used a mutex around the above block. However,
updates, deletes, and lookups occur inside rcu critical sections
so we can't use a mutex in this context safely.
Fortunately, by writing slightly better code we can avoid the
mutex altogether. If CPU 1 in the above example uses a cmpxchg
and _only_ replaces the dev reference in the map when it is in
fact the expected dev the race is removed completely. The two
cases being illustrated here, first the racy condition,
CPU 1 CPU 2
notifier hook bpf devmap update
dev = rcu_dereference(map[i])
dev = rcu_dereference(map[i])
xchg(map[i]), new_dev);
rcu_call(dev,...)
odev = cmpxchg(map[i], dev, NULL)
Now we can test the cmpxchg return value, detect odev != dev and
abort. Or in the good case,
CPU 1 CPU 2
notifier hook bpf devmap update
dev = rcu_dereference(map[i])
odev = cmpxchg(map[i], dev, NULL)
[...]
Now 'odev == dev' and we can do proper cleanup.
And viola the original race we tried to solve with a mutex is
corrected and the trace noted by Sasha below is resolved due
to removal of the mutex.
BUG: sleeping function called from invalid context at kernel/locking/mutex.c:747
in_atomic(): 1, irqs_disabled(): 0, pid: 16315, name: syz-executor1
1 lock held by syz-executor1/16315:
#0: (rcu_read_lock){......}, at: [<ffffffff8c363bc2>] map_delete_elem kernel/bpf/syscall.c:577 [inline]
#0: (rcu_read_lock){......}, at: [<ffffffff8c363bc2>] SYSC_bpf kernel/bpf/syscall.c:1427 [inline]
#0: (rcu_read_lock){......}, at: [<ffffffff8c363bc2>] SyS_bpf+0x1d32/0x4ba0 kernel/bpf/syscall.c:1388
Fixes: 2ddf71e23cc2 ("net: add notifier hooks for devmap bpf map")
Reported-by: Sasha Levin <alexander.levin@verizon.com>
Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/devmap.c | 27 +++++++--------------------
1 file changed, 7 insertions(+), 20 deletions(-)
diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
index d439ee0..087f7230 100644
--- a/kernel/bpf/devmap.c
+++ b/kernel/bpf/devmap.c
@@ -40,11 +40,9 @@
* contain a reference to the net device and remove them. This is a two step
* process (a) dereference the bpf_dtab_netdev object in netdev_map and (b)
* check to see if the ifindex is the same as the net_device being removed.
- * Unfortunately, the xchg() operations do not protect against this. To avoid
- * potentially removing incorrect objects the dev_map_list_mutex protects
- * conflicting netdev unregister and BPF syscall operations. Updates and
- * deletes from a BPF program (done in rcu critical section) are blocked
- * because of this mutex.
+ * When removing the dev a cmpxchg() is used to ensure the correct dev is
+ * removed, in the case of a concurrent update or delete operation it is
+ * possible that the initially referenced dev is no longer in the map.
*/
#include <linux/bpf.h>
#include <linux/jhash.h>
@@ -68,7 +66,6 @@ struct bpf_dtab {
struct list_head list;
};
-static DEFINE_MUTEX(dev_map_list_mutex);
static LIST_HEAD(dev_map_list);
static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
@@ -128,9 +125,7 @@ static struct bpf_map *dev_map_alloc(union bpf_attr *attr)
if (!dtab->netdev_map)
goto free_dtab;
- mutex_lock(&dev_map_list_mutex);
list_add_tail(&dtab->list, &dev_map_list);
- mutex_unlock(&dev_map_list_mutex);
return &dtab->map;
free_dtab:
@@ -169,7 +164,6 @@ static void dev_map_free(struct bpf_map *map)
* at this point we we can still race with netdev notifier, hence the
* lock.
*/
- mutex_lock(&dev_map_list_mutex);
for (i = 0; i < dtab->map.max_entries; i++) {
struct bpf_dtab_netdev *dev;
@@ -185,7 +179,6 @@ static void dev_map_free(struct bpf_map *map)
* _must_ be complete
*/
list_del(&dtab->list);
- mutex_unlock(&dev_map_list_mutex);
free_percpu(dtab->flush_needed);
bpf_map_area_free(dtab->netdev_map);
kfree(dtab);
@@ -322,11 +315,9 @@ static int dev_map_delete_elem(struct bpf_map *map, void *key)
* the driver tear down ensures all soft irqs are complete before
* removing the net device in the case of dev_put equals zero.
*/
- mutex_lock(&dev_map_list_mutex);
old_dev = xchg(&dtab->netdev_map[k], NULL);
if (old_dev)
call_rcu(&old_dev->rcu, __dev_map_entry_free);
- mutex_unlock(&dev_map_list_mutex);
return 0;
}
@@ -369,11 +360,9 @@ static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
* Remembering the driver side flush operation will happen before the
* net device is removed.
*/
- mutex_lock(&dev_map_list_mutex);
old_dev = xchg(&dtab->netdev_map[i], dev);
if (old_dev)
call_rcu(&old_dev->rcu, __dev_map_entry_free);
- mutex_unlock(&dev_map_list_mutex);
return 0;
}
@@ -396,22 +385,20 @@ static int dev_map_notification(struct notifier_block *notifier,
switch (event) {
case NETDEV_UNREGISTER:
- mutex_lock(&dev_map_list_mutex);
list_for_each_entry(dtab, &dev_map_list, list) {
for (i = 0; i < dtab->map.max_entries; i++) {
- struct bpf_dtab_netdev *dev;
+ struct bpf_dtab_netdev *dev, *odev;
- dev = dtab->netdev_map[i];
+ dev = READ_ONCE(dtab->netdev_map[i]);
if (!dev ||
dev->dev->ifindex != netdev->ifindex)
continue;
- dev = xchg(&dtab->netdev_map[i], NULL);
- if (dev)
+ odev = cmpxchg(&dtab->netdev_map[i], dev, NULL);
+ if (dev == odev)
call_rcu(&dev->rcu,
__dev_map_entry_free);
}
}
- mutex_unlock(&dev_map_list_mutex);
break;
default:
break;
^ permalink raw reply related
* Re: [PATCH v8 1/4] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING
From: Casey Leedom @ 2017-08-04 21:06 UTC (permalink / raw)
To: Ding Tianhong, ashok.raj@intel.com, bhelgaas@google.com,
helgaas@kernel.org, Michael Werner, Ganesh GR,
asit.k.mallick@intel.com, patrick.j.cramer@intel.com,
Suravee.Suthikulpanit@amd.com, Bob.Shaw@amd.com,
l.stach@pengutronix.de, amira@mellanox.com,
gabriele.paoloni@huawei.com, David.Laight@aculab.com,
jeffrey.t.kirsher@intel.com, catalin.marinas@arm.com,
will.deacon@arm.com, mark.rutland@arm.com, robin.murphy@arm.com,
davem@davemloft.net, alexander.duyck@gmail.com,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
linuxarm@huawei.com
In-Reply-To: <1501767889-7772-2-git-send-email-dingtianhong@huawei.com>
| From: Ding Tianhong <dingtianhong@huawei.com>
| Sent: Thursday, August 3, 2017 6:44 AM
|
| diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
| index 6967c6b..1e1cdbe 100644
| --- a/drivers/pci/quirks.c
| +++ b/drivers/pci/quirks.c
| @@ -4016,6 +4016,44 @@ static void quirk_tw686x_class(struct pci_dev *pdev)
| quirk_tw686x_class);
|
| /*
| + * Some devices have problems with Transaction Layer Packets with the Relaxed
| + * Ordering Attribute set. Such devices should mark themselves and other
| + * Device Drivers should check before sending TLPs with RO set.
| + */
| +static void quirk_relaxedordering_disable(struct pci_dev *dev)
| +{
| + dev->dev_flags |= PCI_DEV_FLAGS_NO_RELAXED_ORDERING;
| +}
| +
| +/*
| + * Intel E5-26xx Root Complex has a Flow Control Credit issue which can
| + * cause performance problems with Upstream Transaction Layer Packets with
| + * Relaxed Ordering set.
| + */
| +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_INTEL, 0x6f02, PCI_CLASS_NOT_DEFINED, 8,
| + quirk_relaxedordering_disable);
| +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_INTEL, 0x6f04, PCI_CLASS_NOT_DEFINED, 8,
| + quirk_relaxedordering_disable);
| +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_INTEL, 0x6f08, PCI_CLASS_NOT_DEFINED, 8,
| + quirk_relaxedordering_disable);
| + ...
It looks like this is missing the set of Root Complex IDs that were noted in
the document to which Patrick Cramer sent us a reference:
https://software.intel.com/sites/default/files/managed/9e/bc/64-ia-32-architectures-optimization-manual.pdf
In section 3.9.1 we have:
3.9.1 Optimizing PCIe Performance for Accesses Toward Coherent Memory
and Toward MMIO Regions (P2P)
In order to maximize performance for PCIe devices in the processors
listed in Table 3-6 below, the soft- ware should determine whether the
accesses are toward coherent memory (system memory) or toward MMIO
regions (P2P access to other devices). If the access is toward MMIO
region, then software can command HW to set the RO bit in the TLP
header, as this would allow hardware to achieve maximum throughput for
these types of accesses. For accesses toward coherent memory, software
can command HW to clear the RO bit in the TLP header (no RO), as this
would allow hardware to achieve maximum throughput for these types of
accesses.
Table 3-6. Intel Processor CPU RP Device IDs for Processors Optimizing
PCIe Performance
Processor CPU RP Device IDs
Intel Xeon processors based on 6F01H-6F0EH
Broadwell microarchitecture
Intel Xeon processors based on 2F01H-2F0EH
Haswell microarchitecture
The PCI Device IDs you have there are the first ones that I guessed at
having the performance problem with Relaxed Ordering. We now apparently
have a complete list from Intel.
I don't want to phrase this as a "NAK" because you've gone around the
mulberry bush a bunch of times already. So maybe just go with what you've
got in version 8 of your patch and then do a follow on patch to complete the
table?
Casey
^ permalink raw reply
* Re: [PATCH v8 1/4] PCI: Add new PCIe Fabric End Node flag, PCI_DEV_FLAGS_NO_RELAXED_ORDERING
From: Casey Leedom @ 2017-08-04 21:06 UTC (permalink / raw)
To: Ding Tianhong, ashok.raj@intel.com, bhelgaas@google.com,
helgaas@kernel.org, Michael Werner, Ganesh GR,
asit.k.mallick@intel.com, patrick.j.cramer@intel.com,
Suravee.Suthikulpanit@amd.com, Bob.Shaw@amd.com,
l.stach@pengutronix.de, amira@mellanox.com,
gabriele.paoloni@huawei.com, David.Laight@aculab.com,
jeffrey.t.kirsher@intel.com, catalin.marinas@arm.com
In-Reply-To: <1501767889-7772-2-git-send-email-dingtianhong@huawei.com>
| From: Ding Tianhong <dingtianhong@huawei.com>
| Sent: Thursday, August 3, 2017 6:44 AM
|
| diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
| index 6967c6b..1e1cdbe 100644
| --- a/drivers/pci/quirks.c
| +++ b/drivers/pci/quirks.c
| @@ -4016,6 +4016,44 @@ static void quirk_tw686x_class(struct pci_dev *pdev)
| quirk_tw686x_class);
|
| /*
| + * Some devices have problems with Transaction Layer Packets with the Relaxed
| + * Ordering Attribute set. Such devices should mark themselves and other
| + * Device Drivers should check before sending TLPs with RO set.
| + */
| +static void quirk_relaxedordering_disable(struct pci_dev *dev)
| +{
| + dev->dev_flags |= PCI_DEV_FLAGS_NO_RELAXED_ORDERING;
| +}
| +
| +/*
| + * Intel E5-26xx Root Complex has a Flow Control Credit issue which can
| + * cause performance problems with Upstream Transaction Layer Packets with
| + * Relaxed Ordering set.
| + */
| +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_INTEL, 0x6f02, PCI_CLASS_NOT_DEFINED, 8,
| + quirk_relaxedordering_disable);
| +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_INTEL, 0x6f04, PCI_CLASS_NOT_DEFINED, 8,
| + quirk_relaxedordering_disable);
| +DECLARE_PCI_FIXUP_CLASS_EARLY(PCI_VENDOR_ID_INTEL, 0x6f08, PCI_CLASS_NOT_DEFINED, 8,
| + quirk_relaxedordering_disable);
| + ...
It looks like this is missing the set of Root Complex IDs that were noted in
the document to which Patrick Cramer sent us a reference:
https://software.intel.com/sites/default/files/managed/9e/bc/64-ia-32-architectures-optimization-manual.pdf
In section 3.9.1 we have:
3.9.1 Optimizing PCIe Performance for Accesses Toward Coherent Memory
and Toward MMIO Regions (P2P)
In order to maximize performance for PCIe devices in the processors
listed in Table 3-6 below, the soft- ware should determine whether the
accesses are toward coherent memory (system memory) or toward MMIO
regions (P2P access to other devices). If the access is toward MMIO
region, then software can command HW to set the RO bit in the TLP
header, as this would allow hardware to achieve maximum throughput for
these types of accesses. For accesses toward coherent memory, software
can command HW to clear the RO bit in the TLP header (no RO), as this
would allow hardware to achieve maximum throughput for these types of
accesses.
Table 3-6. Intel Processor CPU RP Device IDs for Processors Optimizing
PCIe Performance
Processor CPU RP Device IDs
Intel Xeon processors based on 6F01H-6F0EH
Broadwell microarchitecture
Intel Xeon processors based on 2F01H-2F0EH
Haswell microarchitecture
The PCI Device IDs you have there are the first ones that I guessed at
having the performance problem with Relaxed Ordering. We now apparently
have a complete list from Intel.
I don't want to phrase this as a "NAK" because you've gone around the
mulberry bush a bunch of times already. So maybe just go with what you've
got in version 8 of your patch and then do a follow on patch to complete the
table?
Casey
^ permalink raw reply
* Re: [PATCH v7 2/3] PCI: Enable PCIe Relaxed Ordering if supported
From: Casey Leedom @ 2017-08-04 20:48 UTC (permalink / raw)
To: Raj, Ashok
Cc: Ding Tianhong, Alexander Duyck, Alex Williamson, Sinan Kaya,
bhelgaas@google.com, helgaas@kernel.org, Michael Werner,
Ganesh GR, asit.k.mallick@intel.com, patrick.j.cramer@intel.com,
Suravee.Suthikulpanit@amd.com, Bob.Shaw@amd.com,
l.stach@pengutronix.de, amira@mellanox.com,
gabriele.paoloni@huawei.com, David.Laight@aculab.com,
"jeffrey.t.kir
In-Reply-To: <20170804202141.GA39054@otc-nc-03>
| From: Raj, Ashok <ashok.raj@intel.com>
| Sent: Friday, August 4, 2017 1:21 PM
|
| On Fri, Aug 04, 2017 at 08:20:37PM +0000, Casey Leedom wrote:
| > ...
| > As I've noted a number of times, it would be great if the Intel Hardware
| > Engineers who attempted to implement the Relaxed Ordering semantics in the
| > current generation of Root Complexes had left the ability to turn off the
| > logic which is obviously not working. If there was a way to disable the
| > logic via an undocumented register, then we could have the Linux PCI Quirk
| > do that. Since Relaxed Ordering is just a hint, it's completely legitimate
| > to completely ignore it.
|
| Suppose you are looking for the existence of a chicken bit to instruct the
| port to ignore RO traffic. So all we would do is turn the chicken bit on
| but would permit p2p traffic to be allowed since we won't turn off the
| capability as currently proposed.
|
| Let me look into that keep you posted.
Huh, I'd never heard it called a "chicken bit" before, but yes, that's what
I'm talking about.
Whenever our Hardware Designers implement new functionality in our hardware,
they almost always put in A. several "knobs" which can control fundamental
parameters of the new Hardware Feature, and B. a mechanism of completely
disabling it if necessary. This stems from the incredibly long Design ->
Deployment cyle for Hardware (as opposed to the edit->compile->run cycle for s!
It's obvious that handling Relaxed Ordering is a new Hardware Feature for
Intel's Root Complexes since previous versions simply ignored it (because
that's legal[1]). If I was a Hardware Engineer tasked with implementing
Relaxed Ordering semantics for a device, I would certainly have also
implemented a switch to turn it off in case there were unintended
consequences (performance in this case).
And if there is such a mechanism to simply disable processing of Relaxed
Ordering semantics in the Root Complex, that would be a far easier "fix" for
this problem ... and leave the code in place to continue sending Relaxed
Ordering TLPs for a future Root Complex implementation which got it right ...
Casey
[1] One can't ~quite~ just ignore the Relaxed Ordering Attribute on an
incoming Transaction Layer Packet Request: PCIe completion rules (see
section 2.2.9 of the PCIe 3.0 specificatin) require that the Relaxed
Ordering and No Snoop Attributes in a Request TLP be reflected back
verbatim in any corresponding Response TLP. (The rules for ID-Based
Ordering are more complex.)
^ permalink raw reply
* Re: [PATCH v5] ss: Enclose IPv6 address in brackets
From: Eric Dumazet @ 2017-08-04 20:46 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: Florian Lehner, netdev
In-Reply-To: <20170804120506.37c6ad60@xeon-e3>
On Fri, 2017-08-04 at 12:05 -0700, Stephen Hemminger wrote:
> On Fri, 4 Aug 2017 20:02:52 +0200
> Florian Lehner <dev@der-flo.net> wrote:
>
> > diff --git a/misc/ss.c b/misc/ss.c
> > index 12763c9..83683b5 100644
> > --- a/misc/ss.c
> > +++ b/misc/ss.c
> > @@ -1046,8 +1046,9 @@ do_numeric:
> >
> > static void inet_addr_print(const inet_prefix *a, int port, unsigned
> > int ifindex)
> > {
>
> Your email client is wrapping long lines which leads to malformed patch.
>
> You didn't need buf2, and the code was more complex than it needed to be.
>
> Rather than waiting for yet another version, I just merged in similar
> code.
Also, is this new format accepted for the filter ?
ss src [::1]:22
^ 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