* [PATCH v6 bpf-next 00/11] bpf, tracing: introduce bpf raw tracepoints
From: Alexei Starovoitov @ 2018-03-27 2:46 UTC (permalink / raw)
To: davem; +Cc: daniel, torvalds, peterz, rostedt, netdev, kernel-team, linux-api
From: Alexei Starovoitov <ast@kernel.org>
v5->v6:
- avoid changing semantics of for_each_kernel_tracepoint() function, instead
introduce kernel_tracepoint_find_by_name() helper
v4->v5:
- adopted Daniel's fancy REPEAT macro in bpf_trace.c in patch 8
v3->v4:
- adopted Linus's CAST_TO_U64 macro to cast any integer, pointer, or small
struct to u64. That nicely reduced the size of patch 1
v2->v3:
- with Linus's suggestion introduced generic COUNT_ARGS and CONCATENATE macros
(or rather moved them from apparmor)
that cleaned up patches 6 and 8
- added patch 4 to refactor trace_iwlwifi_dev_ucode_error() from 17 args to 4
Now any tracepoint with >12 args will have build error
v1->v2:
- simplified api by combing bpf_raw_tp_open(name) + bpf_attach(prog_fd) into
bpf_raw_tp_open(name, prog_fd) as suggested by Daniel.
That simplifies bpf_detach as well which is now simple close() of fd.
- fixed memory leak in error path which was spotted by Daniel.
- fixed bpf_get_stackid(), bpf_perf_event_output() called from raw tracepoints
- added more tests
- fixed allyesconfig build caught by buildbot
v1:
This patch set is a different way to address the pressing need to access
task_struct pointers in sched tracepoints from bpf programs.
The first approach simply added these pointers to sched tracepoints:
https://lkml.org/lkml/2017/12/14/753
which Peter nacked.
Few options were discussed and eventually the discussion converged on
doing bpf specific tracepoint_probe_register() probe functions.
Details here:
https://lkml.org/lkml/2017/12/20/929
Patch 1 is kernel wide cleanup of pass-struct-by-value into
pass-struct-by-reference into tracepoints.
Patches 2 and 3 are minor cleanups to address allyesconfig build
Patch 4 refactor trace_iwlwifi_dev_ucode_error from 17 to 4 args
Patch 5 introduces COUNT_ARGS macro
Patch 6 minor prep work to expose number of arguments passed
into tracepoints.
Patch 7 kernel_tracepoint_find_by_name() helper
Patch 8 introduces BPF_RAW_TRACEPOINT api.
the auto-cleanup and multiple concurrent users are must have
features of tracing api. For bpf raw tracepoints it looks like:
// load bpf prog with BPF_PROG_TYPE_RAW_TRACEPOINT type
prog_fd = bpf_prog_load(...);
// receive anon_inode fd for given bpf_raw_tracepoint
// and attach bpf program to it
raw_tp_fd = bpf_raw_tracepoint_open("xdp_exception", prog_fd);
Ctrl-C of tracing daemon or cmdline tool will automatically
detach bpf program, unload it and unregister tracepoint probe.
More details in patch 8.
Patch 9 - trivial support in libbpf
Patches 10, 11 - user space tests
samples/bpf/test_overhead performance on 1 cpu:
tracepoint base kprobe+bpf tracepoint+bpf raw_tracepoint+bpf
task_rename 1.1M 769K 947K 1.0M
urandom_read 789K 697K 750K 755K
Alexei Starovoitov (11):
treewide: remove large struct-pass-by-value from tracepoint arguments
net/mediatek: disambiguate mt76 vs mt7601u trace events
net/mac802154: disambiguate mac80215 vs mac802154 trace events
net/wireless/iwlwifi: fix iwlwifi_dev_ucode_error tracepoint
macro: introduce COUNT_ARGS() macro
tracepoint: compute num_args at build time
tracepoint: introduce kernel_tracepoint_find_by_name
bpf: introduce BPF_RAW_TRACEPOINT
libbpf: add bpf_raw_tracepoint_open helper
samples/bpf: raw tracepoint test
selftests/bpf: test for bpf_get_stackid() from raw tracepoints
drivers/infiniband/hw/hfi1/file_ops.c | 2 +-
drivers/infiniband/hw/hfi1/trace_ctxts.h | 12 +-
drivers/net/wireless/intel/iwlwifi/dvm/main.c | 7 +-
.../wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h | 39 ++---
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c | 1 +
drivers/net/wireless/intel/iwlwifi/mvm/utils.c | 7 +-
drivers/net/wireless/mediatek/mt7601u/trace.h | 6 +-
include/linux/bpf_types.h | 1 +
include/linux/kernel.h | 7 +
include/linux/trace_events.h | 37 ++++
include/linux/tracepoint-defs.h | 1 +
include/linux/tracepoint.h | 18 +-
include/trace/bpf_probe.h | 87 ++++++++++
include/trace/define_trace.h | 15 +-
include/trace/events/f2fs.h | 2 +-
include/uapi/linux/bpf.h | 11 ++
kernel/bpf/syscall.c | 78 +++++++++
kernel/trace/bpf_trace.c | 188 +++++++++++++++++++++
kernel/tracepoint.c | 9 +
net/mac802154/trace.h | 8 +-
net/wireless/trace.h | 2 +-
samples/bpf/Makefile | 1 +
samples/bpf/bpf_load.c | 14 ++
samples/bpf/test_overhead_raw_tp_kern.c | 17 ++
samples/bpf/test_overhead_user.c | 12 ++
security/apparmor/include/path.h | 7 +-
sound/firewire/amdtp-stream-trace.h | 2 +-
tools/include/uapi/linux/bpf.h | 11 ++
tools/lib/bpf/bpf.c | 11 ++
tools/lib/bpf/bpf.h | 1 +
tools/testing/selftests/bpf/test_progs.c | 91 +++++++---
31 files changed, 615 insertions(+), 90 deletions(-)
create mode 100644 include/trace/bpf_probe.h
create mode 100644 samples/bpf/test_overhead_raw_tp_kern.c
--
2.9.5
^ permalink raw reply
* [PATCH v6 bpf-next 05/11] macro: introduce COUNT_ARGS() macro
From: Alexei Starovoitov @ 2018-03-27 2:47 UTC (permalink / raw)
To: davem; +Cc: daniel, torvalds, peterz, rostedt, netdev, kernel-team, linux-api
In-Reply-To: <20180327024706.2064725-1-ast@fb.com>
From: Alexei Starovoitov <ast@kernel.org>
move COUNT_ARGS() macro from apparmor to generic header and extend it
to count till twelve.
COUNT() was an alternative name for this logic, but it's used for
different purpose in many other places.
Similarly for CONCATENATE() macro.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/kernel.h | 7 +++++++
security/apparmor/include/path.h | 7 +------
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 3fd291503576..293fa0677fba 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -919,6 +919,13 @@ static inline void ftrace_dump(enum ftrace_dump_mode oops_dump_mode) { }
#define swap(a, b) \
do { typeof(a) __tmp = (a); (a) = (b); (b) = __tmp; } while (0)
+/* This counts to 12. Any more, it will return 13th argument. */
+#define __COUNT_ARGS(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, _10, _11, _12, _n, X...) _n
+#define COUNT_ARGS(X...) __COUNT_ARGS(, ##X, 12, 11, 10, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
+
+#define __CONCAT(a, b) a ## b
+#define CONCATENATE(a, b) __CONCAT(a, b)
+
/**
* container_of - cast a member of a structure out to the containing structure
* @ptr: the pointer to the member.
diff --git a/security/apparmor/include/path.h b/security/apparmor/include/path.h
index 05fb3305671e..e042b994f2b8 100644
--- a/security/apparmor/include/path.h
+++ b/security/apparmor/include/path.h
@@ -43,15 +43,10 @@ struct aa_buffers {
DECLARE_PER_CPU(struct aa_buffers, aa_buffers);
-#define COUNT_ARGS(X...) COUNT_ARGS_HELPER(, ##X, 9, 8, 7, 6, 5, 4, 3, 2, 1, 0)
-#define COUNT_ARGS_HELPER(_0, _1, _2, _3, _4, _5, _6, _7, _8, _9, n, X...) n
-#define CONCAT(X, Y) X ## Y
-#define CONCAT_AFTER(X, Y) CONCAT(X, Y)
-
#define ASSIGN(FN, X, N) ((X) = FN(N))
#define EVAL1(FN, X) ASSIGN(FN, X, 0) /*X = FN(0)*/
#define EVAL2(FN, X, Y...) do { ASSIGN(FN, X, 1); EVAL1(FN, Y); } while (0)
-#define EVAL(FN, X...) CONCAT_AFTER(EVAL, COUNT_ARGS(X))(FN, X)
+#define EVAL(FN, X...) CONCATENATE(EVAL, COUNT_ARGS(X))(FN, X)
#define for_each_cpu_buffer(I) for ((I) = 0; (I) < MAX_PATH_BUFFERS; (I)++)
--
2.9.5
^ permalink raw reply related
* [PATCH v6 bpf-next 06/11] tracepoint: compute num_args at build time
From: Alexei Starovoitov @ 2018-03-27 2:47 UTC (permalink / raw)
To: davem; +Cc: daniel, torvalds, peterz, rostedt, netdev, kernel-team, linux-api
In-Reply-To: <20180327024706.2064725-1-ast@fb.com>
From: Alexei Starovoitov <ast@kernel.org>
compute number of arguments passed into tracepoint
at compile time and store it as part of 'struct tracepoint'.
The number is necessary to check safety of bpf program access that
is coming in subsequent patch.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/tracepoint-defs.h | 1 +
include/linux/tracepoint.h | 12 ++++++------
include/trace/define_trace.h | 14 +++++++-------
3 files changed, 14 insertions(+), 13 deletions(-)
diff --git a/include/linux/tracepoint-defs.h b/include/linux/tracepoint-defs.h
index 64ed7064f1fa..39a283c61c51 100644
--- a/include/linux/tracepoint-defs.h
+++ b/include/linux/tracepoint-defs.h
@@ -33,6 +33,7 @@ struct tracepoint {
int (*regfunc)(void);
void (*unregfunc)(void);
struct tracepoint_func __rcu *funcs;
+ u32 num_args;
};
#endif
diff --git a/include/linux/tracepoint.h b/include/linux/tracepoint.h
index c94f466d57ef..c92f4adbc0d7 100644
--- a/include/linux/tracepoint.h
+++ b/include/linux/tracepoint.h
@@ -230,18 +230,18 @@ extern void syscall_unregfunc(void);
* structures, so we create an array of pointers that will be used for iteration
* on the tracepoints.
*/
-#define DEFINE_TRACE_FN(name, reg, unreg) \
+#define DEFINE_TRACE_FN(name, reg, unreg, num_args) \
static const char __tpstrtab_##name[] \
__attribute__((section("__tracepoints_strings"))) = #name; \
struct tracepoint __tracepoint_##name \
__attribute__((section("__tracepoints"))) = \
- { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL };\
+ { __tpstrtab_##name, STATIC_KEY_INIT_FALSE, reg, unreg, NULL, num_args };\
static struct tracepoint * const __tracepoint_ptr_##name __used \
__attribute__((section("__tracepoints_ptrs"))) = \
&__tracepoint_##name;
-#define DEFINE_TRACE(name) \
- DEFINE_TRACE_FN(name, NULL, NULL);
+#define DEFINE_TRACE(name, num_args) \
+ DEFINE_TRACE_FN(name, NULL, NULL, num_args);
#define EXPORT_TRACEPOINT_SYMBOL_GPL(name) \
EXPORT_SYMBOL_GPL(__tracepoint_##name)
@@ -275,8 +275,8 @@ extern void syscall_unregfunc(void);
return false; \
}
-#define DEFINE_TRACE_FN(name, reg, unreg)
-#define DEFINE_TRACE(name)
+#define DEFINE_TRACE_FN(name, reg, unreg, num_args)
+#define DEFINE_TRACE(name, num_args)
#define EXPORT_TRACEPOINT_SYMBOL_GPL(name)
#define EXPORT_TRACEPOINT_SYMBOL(name)
diff --git a/include/trace/define_trace.h b/include/trace/define_trace.h
index d9e3d4aa3f6e..96b22ace9ae7 100644
--- a/include/trace/define_trace.h
+++ b/include/trace/define_trace.h
@@ -25,7 +25,7 @@
#undef TRACE_EVENT
#define TRACE_EVENT(name, proto, args, tstruct, assign, print) \
- DEFINE_TRACE(name)
+ DEFINE_TRACE(name, COUNT_ARGS(args))
#undef TRACE_EVENT_CONDITION
#define TRACE_EVENT_CONDITION(name, proto, args, cond, tstruct, assign, print) \
@@ -39,24 +39,24 @@
#undef TRACE_EVENT_FN
#define TRACE_EVENT_FN(name, proto, args, tstruct, \
assign, print, reg, unreg) \
- DEFINE_TRACE_FN(name, reg, unreg)
+ DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
#undef TRACE_EVENT_FN_COND
#define TRACE_EVENT_FN_COND(name, proto, args, cond, tstruct, \
assign, print, reg, unreg) \
- DEFINE_TRACE_FN(name, reg, unreg)
+ DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
#undef DEFINE_EVENT
#define DEFINE_EVENT(template, name, proto, args) \
- DEFINE_TRACE(name)
+ DEFINE_TRACE(name, COUNT_ARGS(args))
#undef DEFINE_EVENT_FN
#define DEFINE_EVENT_FN(template, name, proto, args, reg, unreg) \
- DEFINE_TRACE_FN(name, reg, unreg)
+ DEFINE_TRACE_FN(name, reg, unreg, COUNT_ARGS(args))
#undef DEFINE_EVENT_PRINT
#define DEFINE_EVENT_PRINT(template, name, proto, args, print) \
- DEFINE_TRACE(name)
+ DEFINE_TRACE(name, COUNT_ARGS(args))
#undef DEFINE_EVENT_CONDITION
#define DEFINE_EVENT_CONDITION(template, name, proto, args, cond) \
@@ -64,7 +64,7 @@
#undef DECLARE_TRACE
#define DECLARE_TRACE(name, proto, args) \
- DEFINE_TRACE(name)
+ DEFINE_TRACE(name, COUNT_ARGS(args))
#undef TRACE_INCLUDE
#undef __TRACE_INCLUDE
--
2.9.5
^ permalink raw reply related
* [PATCH v6 bpf-next 04/11] net/wireless/iwlwifi: fix iwlwifi_dev_ucode_error tracepoint
From: Alexei Starovoitov @ 2018-03-27 2:46 UTC (permalink / raw)
To: davem; +Cc: daniel, torvalds, peterz, rostedt, netdev, kernel-team, linux-api
In-Reply-To: <20180327024706.2064725-1-ast@fb.com>
From: Alexei Starovoitov <ast@kernel.org>
fix iwlwifi_dev_ucode_error tracepoint to pass pointer to a table
instead of all 17 arguments by value.
dvm/main.c and mvm/utils.c have 'struct iwl_error_event_table'
defined with very similar yet subtly different fields and offsets.
tracepoint is still common and using definition of 'struct iwl_error_event_table'
from dvm/commands.h while copying fields.
Long term this tracepoint probably should be split into two.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
drivers/net/wireless/intel/iwlwifi/dvm/main.c | 7 +---
.../wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h | 39 ++++++++++------------
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c | 1 +
drivers/net/wireless/intel/iwlwifi/mvm/utils.c | 7 +---
4 files changed, 21 insertions(+), 33 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/dvm/main.c b/drivers/net/wireless/intel/iwlwifi/dvm/main.c
index d11d72615de2..e68254e12764 100644
--- a/drivers/net/wireless/intel/iwlwifi/dvm/main.c
+++ b/drivers/net/wireless/intel/iwlwifi/dvm/main.c
@@ -1651,12 +1651,7 @@ static void iwl_dump_nic_error_log(struct iwl_priv *priv)
priv->status, table.valid);
}
- trace_iwlwifi_dev_ucode_error(trans->dev, table.error_id, table.tsf_low,
- table.data1, table.data2, table.line,
- table.blink2, table.ilink1, table.ilink2,
- table.bcon_time, table.gp1, table.gp2,
- table.gp3, table.ucode_ver, table.hw_ver,
- 0, table.brd_ver);
+ trace_iwlwifi_dev_ucode_error(trans->dev, &table, 0, table.brd_ver);
IWL_ERR(priv, "0x%08X | %-28s\n", table.error_id,
desc_lookup(table.error_id));
IWL_ERR(priv, "0x%08X | uPc\n", table.pc);
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h b/drivers/net/wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h
index 9518a82f44c2..27e3e4e96aa2 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h
@@ -126,14 +126,11 @@ TRACE_EVENT(iwlwifi_dev_tx,
__entry->framelen, __entry->skbaddr)
);
+struct iwl_error_event_table;
TRACE_EVENT(iwlwifi_dev_ucode_error,
- TP_PROTO(const struct device *dev, u32 desc, u32 tsf_low,
- u32 data1, u32 data2, u32 line, u32 blink2, u32 ilink1,
- u32 ilink2, u32 bcon_time, u32 gp1, u32 gp2, u32 rev_type,
- u32 major, u32 minor, u32 hw_ver, u32 brd_ver),
- TP_ARGS(dev, desc, tsf_low, data1, data2, line,
- blink2, ilink1, ilink2, bcon_time, gp1, gp2,
- rev_type, major, minor, hw_ver, brd_ver),
+ TP_PROTO(const struct device *dev, const struct iwl_error_event_table *table,
+ u32 hw_ver, u32 brd_ver),
+ TP_ARGS(dev, table, hw_ver, brd_ver),
TP_STRUCT__entry(
DEV_ENTRY
__field(u32, desc)
@@ -155,20 +152,20 @@ TRACE_EVENT(iwlwifi_dev_ucode_error,
),
TP_fast_assign(
DEV_ASSIGN;
- __entry->desc = desc;
- __entry->tsf_low = tsf_low;
- __entry->data1 = data1;
- __entry->data2 = data2;
- __entry->line = line;
- __entry->blink2 = blink2;
- __entry->ilink1 = ilink1;
- __entry->ilink2 = ilink2;
- __entry->bcon_time = bcon_time;
- __entry->gp1 = gp1;
- __entry->gp2 = gp2;
- __entry->rev_type = rev_type;
- __entry->major = major;
- __entry->minor = minor;
+ __entry->desc = table->error_id;
+ __entry->tsf_low = table->tsf_low;
+ __entry->data1 = table->data1;
+ __entry->data2 = table->data2;
+ __entry->line = table->line;
+ __entry->blink2 = table->blink2;
+ __entry->ilink1 = table->ilink1;
+ __entry->ilink2 = table->ilink2;
+ __entry->bcon_time = table->bcon_time;
+ __entry->gp1 = table->gp1;
+ __entry->gp2 = table->gp2;
+ __entry->rev_type = table->gp3;
+ __entry->major = table->ucode_ver;
+ __entry->minor = table->hw_ver;
__entry->hw_ver = hw_ver;
__entry->brd_ver = brd_ver;
),
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c b/drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c
index 50510fb6ab8c..6aa719865a58 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c
@@ -30,6 +30,7 @@
#ifndef __CHECKER__
#include "iwl-trans.h"
+#include "dvm/commands.h"
#define CREATE_TRACE_POINTS
#include "iwl-devtrace.h"
diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
index d65e1db7c097..5442ead876eb 100644
--- a/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
+++ b/drivers/net/wireless/intel/iwlwifi/mvm/utils.c
@@ -549,12 +549,7 @@ static void iwl_mvm_dump_lmac_error_log(struct iwl_mvm *mvm, u32 base)
IWL_ERR(mvm, "Loaded firmware version: %s\n", mvm->fw->fw_version);
- trace_iwlwifi_dev_ucode_error(trans->dev, table.error_id, table.tsf_low,
- table.data1, table.data2, table.data3,
- table.blink2, table.ilink1,
- table.ilink2, table.bcon_time, table.gp1,
- table.gp2, table.fw_rev_type, table.major,
- table.minor, table.hw_ver, table.brd_ver);
+ trace_iwlwifi_dev_ucode_error(trans->dev, &table, table.hw_ver, table.brd_ver);
IWL_ERR(mvm, "0x%08X | %-28s\n", table.error_id,
desc_lookup(table.error_id));
IWL_ERR(mvm, "0x%08X | trm_hw_status0\n", table.trm_hw_status0);
--
2.9.5
^ permalink raw reply related
* [PATCH v6 bpf-next 11/11] selftests/bpf: test for bpf_get_stackid() from raw tracepoints
From: Alexei Starovoitov @ 2018-03-27 2:47 UTC (permalink / raw)
To: davem; +Cc: daniel, torvalds, peterz, rostedt, netdev, kernel-team, linux-api
In-Reply-To: <20180327024706.2064725-1-ast@fb.com>
From: Alexei Starovoitov <ast@kernel.org>
similar to traditional traceopint test add bpf_get_stackid() test
from raw tracepoints
and reduce verbosity of existing stackmap test
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
tools/testing/selftests/bpf/test_progs.c | 91 ++++++++++++++++++++++++--------
1 file changed, 70 insertions(+), 21 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index e9df48b306df..faadbe233966 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -877,7 +877,7 @@ static void test_stacktrace_map()
err = bpf_prog_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
- goto out;
+ return;
/* Get the ID for the sched/sched_switch tracepoint */
snprintf(buf, sizeof(buf),
@@ -888,8 +888,7 @@ static void test_stacktrace_map()
bytes = read(efd, buf, sizeof(buf));
close(efd);
- if (CHECK(bytes <= 0 || bytes >= sizeof(buf),
- "read", "bytes %d errno %d\n", bytes, errno))
+ if (bytes <= 0 || bytes >= sizeof(buf))
goto close_prog;
/* Open the perf event and attach bpf progrram */
@@ -906,29 +905,24 @@ static void test_stacktrace_map()
goto close_prog;
err = ioctl(pmu_fd, PERF_EVENT_IOC_ENABLE, 0);
- if (CHECK(err, "perf_event_ioc_enable", "err %d errno %d\n",
- err, errno))
- goto close_pmu;
+ if (err)
+ goto disable_pmu;
err = ioctl(pmu_fd, PERF_EVENT_IOC_SET_BPF, prog_fd);
- if (CHECK(err, "perf_event_ioc_set_bpf", "err %d errno %d\n",
- err, errno))
+ if (err)
goto disable_pmu;
/* find map fds */
control_map_fd = bpf_find_map(__func__, obj, "control_map");
- if (CHECK(control_map_fd < 0, "bpf_find_map control_map",
- "err %d errno %d\n", err, errno))
+ if (control_map_fd < 0)
goto disable_pmu;
stackid_hmap_fd = bpf_find_map(__func__, obj, "stackid_hmap");
- if (CHECK(stackid_hmap_fd < 0, "bpf_find_map stackid_hmap",
- "err %d errno %d\n", err, errno))
+ if (stackid_hmap_fd < 0)
goto disable_pmu;
stackmap_fd = bpf_find_map(__func__, obj, "stackmap");
- if (CHECK(stackmap_fd < 0, "bpf_find_map stackmap", "err %d errno %d\n",
- err, errno))
+ if (stackmap_fd < 0)
goto disable_pmu;
/* give some time for bpf program run */
@@ -945,24 +939,78 @@ static void test_stacktrace_map()
err = compare_map_keys(stackid_hmap_fd, stackmap_fd);
if (CHECK(err, "compare_map_keys stackid_hmap vs. stackmap",
"err %d errno %d\n", err, errno))
- goto disable_pmu;
+ goto disable_pmu_noerr;
err = compare_map_keys(stackmap_fd, stackid_hmap_fd);
if (CHECK(err, "compare_map_keys stackmap vs. stackid_hmap",
"err %d errno %d\n", err, errno))
- ; /* fall through */
+ goto disable_pmu_noerr;
+ goto disable_pmu_noerr;
disable_pmu:
+ error_cnt++;
+disable_pmu_noerr:
ioctl(pmu_fd, PERF_EVENT_IOC_DISABLE);
-
-close_pmu:
close(pmu_fd);
-
close_prog:
bpf_object__close(obj);
+}
-out:
- return;
+static void test_stacktrace_map_raw_tp()
+{
+ int control_map_fd, stackid_hmap_fd, stackmap_fd;
+ const char *file = "./test_stacktrace_map.o";
+ int efd, err, prog_fd;
+ __u32 key, val, duration = 0;
+ struct bpf_object *obj;
+
+ err = bpf_prog_load(file, BPF_PROG_TYPE_RAW_TRACEPOINT, &obj, &prog_fd);
+ if (CHECK(err, "prog_load raw tp", "err %d errno %d\n", err, errno))
+ return;
+
+ efd = bpf_raw_tracepoint_open("sched_switch", prog_fd);
+ if (CHECK(efd < 0, "raw_tp_open", "err %d errno %d\n", efd, errno))
+ goto close_prog;
+
+ /* find map fds */
+ control_map_fd = bpf_find_map(__func__, obj, "control_map");
+ if (control_map_fd < 0)
+ goto close_prog;
+
+ stackid_hmap_fd = bpf_find_map(__func__, obj, "stackid_hmap");
+ if (stackid_hmap_fd < 0)
+ goto close_prog;
+
+ stackmap_fd = bpf_find_map(__func__, obj, "stackmap");
+ if (stackmap_fd < 0)
+ goto close_prog;
+
+ /* give some time for bpf program run */
+ sleep(1);
+
+ /* disable stack trace collection */
+ key = 0;
+ val = 1;
+ bpf_map_update_elem(control_map_fd, &key, &val, 0);
+
+ /* for every element in stackid_hmap, we can find a corresponding one
+ * in stackmap, and vise versa.
+ */
+ err = compare_map_keys(stackid_hmap_fd, stackmap_fd);
+ if (CHECK(err, "compare_map_keys stackid_hmap vs. stackmap",
+ "err %d errno %d\n", err, errno))
+ goto close_prog;
+
+ err = compare_map_keys(stackmap_fd, stackid_hmap_fd);
+ if (CHECK(err, "compare_map_keys stackmap vs. stackid_hmap",
+ "err %d errno %d\n", err, errno))
+ goto close_prog;
+
+ goto close_prog_noerr;
+close_prog:
+ error_cnt++;
+close_prog_noerr:
+ bpf_object__close(obj);
}
static int extract_build_id(char *build_id, size_t size)
@@ -1138,6 +1186,7 @@ int main(void)
test_tp_attach_query();
test_stacktrace_map();
test_stacktrace_build_id();
+ test_stacktrace_map_raw_tp();
printf("Summary: %d PASSED, %d FAILED\n", pass_cnt, error_cnt);
return error_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
--
2.9.5
^ permalink raw reply related
* Re: [PATCH 4/4] selftests/bpf: fix compiling errors
From: Du, Changbin @ 2018-03-27 2:33 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: changbin.du, shuah, linux-kselftest, linux-kernel,
Daniel Borkmann, netdev
In-Reply-To: <20180326145511.lzoi6wja6iht5lvq@ast-mbp.dhcp.thefacebook.com>
Hi Starovoitov,
This one does have the issue you mentioned.
[PATCH 2/4] selftests/gpio: fix paths in Makefile
And can be fixed by:
--- a/tools/testing/selftests/gpio/Makefile
+++ b/tools/testing/selftests/gpio/Makefile
@@ -1,5 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
+OUTPUT ?= $(shell pwd)
TEST_PROGS := gpio-mockup.sh
TEST_FILES := gpio-mockup-sysfs.sh $(BINARIES)
BINARIES := gpio-mockup-chardev
@@ -24,7 +25,7 @@ LDLIBS += -lmount -I/usr/include/libmount
$(BINARIES): gpio-utils.o ../../../../usr/include/linux/gpio.h
gpio-utils.o:
- make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) -C ../../../gpio
+ make ARCH=$(ARCH) CROSS_COMPILE=$(CROSS_COMPILE) OUTPUT=$(OUTPUT)/ -C ../../../gpio
../../../../usr/include/linux/gpio.h:
I will update it later.
On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin.du@intel.com wrote:
> > From: Changbin Du <changbin.du@intel.com>
> >
> > This patch fixed below errors of missing head files.
> >
> > tools/testing/selftests$ make
> > ...
> > clang -I. -I./include/uapi -I../../../include/uapi -Wno-compare-distinct-pointer-types \
> > -O2 -target bpf -emit-llvm -c test_pkt_access.c -o - | \
> > llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf//test_pkt_access.o
> > In file included from test_pkt_access.c:9:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' file not found
> > #include <asm/bitsperlong.h>
> > ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi -Wno-compare-distinct-pointer-types \
> > -O2 -target bpf -emit-llvm -c test_xdp.c -o - | \
> > llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf//test_xdp.o
> > In file included from test_xdp.c:9:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' file not found
> > #include <asm/bitsperlong.h>
> > ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi -Wno-compare-distinct-pointer-types \
> > -O2 -target bpf -emit-llvm -c test_l4lb.c -o - | \
> > llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf//test_l4lb.o
> > In file included from test_l4lb.c:10:
> > In file included from /usr/include/linux/pkt_cls.h:4:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' file not found
> > #include <asm/bitsperlong.h>
> > ^
> > 1 error generated.
> > clang -I. -I./include/uapi -I../../../include/uapi -Wno-compare-distinct-pointer-types \
> > -O2 -target bpf -emit-llvm -c test_tcp_estats.c -o - | \
> > llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf//test_tcp_estats.o
> > In file included from test_tcp_estats.c:35:
> > In file included from ../../../include/uapi/linux/bpf.h:11:
> > In file included from ./include/uapi/linux/types.h:5:
> > /usr/include/asm-generic/int-ll64.h:11:10: fatal error: 'asm/bitsperlong.h' file not found
> > #include <asm/bitsperlong.h>
> > ...
> >
> > Signed-off-by: Changbin Du <changbin.du@intel.com>
> > ---
> > tools/testing/selftests/bpf/Makefile | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 5c43c18..dc0fdc8 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> > GENFLAGS := -DHAVE_GENHDR
> > endif
> >
> > -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
> > +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> > + -I../../../include -I../../../../usr/include
> > LDLIBS += -lcap -lelf -lrt -lpthread
> >
> > # Order correspond to 'make run_tests' order
> > @@ -62,7 +63,7 @@ else
> > CPU ?= generic
> > endif
> >
> > -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> > +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include \
> > -Wno-compare-distinct-pointer-types
>
> Nack.
> I suspect that will break the build for everyone else who's doing it in the directory
> itself instead of the outer one.
>
--
Thanks,
Changbin Du
^ permalink raw reply
* Re: [PATCH 4/4] selftests/bpf: fix compiling errors
From: Du, Changbin @ 2018-03-27 2:20 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: changbin.du, shuah, linux-kselftest, linux-kernel,
Daniel Borkmann, netdev
In-Reply-To: <20180326145511.lzoi6wja6iht5lvq@ast-mbp.dhcp.thefacebook.com>
On Mon, Mar 26, 2018 at 07:55:13AM -0700, Alexei Starovoitov wrote:
> On Mon, Mar 26, 2018 at 05:23:28PM +0800, changbin.du@intel.com wrote:
> > Signed-off-by: Changbin Du <changbin.du@intel.com>
> > ---
> > tools/testing/selftests/bpf/Makefile | 5 +++--
> > 1 file changed, 3 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 5c43c18..dc0fdc8 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -10,7 +10,8 @@ ifneq ($(wildcard $(GENHDR)),)
> > GENFLAGS := -DHAVE_GENHDR
> > endif
> >
> > -CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) -I../../../include
> > +CFLAGS += -Wall -O2 -I$(APIDIR) -I$(LIBDIR) -I$(GENDIR) $(GENFLAGS) \
> > + -I../../../include -I../../../../usr/include
> > LDLIBS += -lcap -lelf -lrt -lpthread
> >
> > # Order correspond to 'make run_tests' order
> > @@ -62,7 +63,7 @@ else
> > CPU ?= generic
> > endif
> >
> > -CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi \
> > +CLANG_FLAGS = -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include \
> > -Wno-compare-distinct-pointer-types
>
> Nack.
> I suspect that will break the build for everyone else who's doing it in the directory
> itself instead of the outer one.
>
This one? But I didn't see any problem.
changbin@gvt-dell-host:~/work/linux/tools/testing/selftests/bpf$ make
make -C ../../../lib/bpf OUTPUT=/home/changbin/work/linux/tools/testing/selftests/bpf/
make[1]: Entering directory '/home/changbin/work/linux/tools/lib/bpf'
HOSTCC /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep.o
HOSTLD /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep-in.o
LINK /home/changbin/work/linux/tools/testing/selftests/bpf/fixdep
CC /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.o
CC /home/changbin/work/linux/tools/testing/selftests/bpf/bpf.o
CC /home/changbin/work/linux/tools/testing/selftests/bpf/nlattr.o
LD /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf-in.o
LINK /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a
LINK /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.so
make[1]: Leaving directory '/home/changbin/work/linux/tools/lib/bpf'
make -C ../../../lib/bpf OUTPUT=/home/changbin/work/linux/tools/testing/selftests/bpf/
make[1]: Entering directory '/home/changbin/work/linux/tools/lib/bpf'
make[1]: Leaving directory '/home/changbin/work/linux/tools/lib/bpf'
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_verifier.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_verifier
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_tag.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_tag
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_maps.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_maps
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_lru_map.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_lru_map
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_lpm_map.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_lpm_map
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_progs.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_progs
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_align.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_align
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_verifier_log.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_verifier_log
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_dev_cgroup.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_dev_cgroup
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_tcpbpf_user.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a cgroup_helpers.c -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_tcpbpf_user
gcc -Wall -O2 -I../../../include/uapi -I../../../lib -I../../../../include/generated -DHAVE_GENHDR -I../../../include -I../../../../usr/include test_libbpf_open.c /home/changbin/work/linux/tools/testing/selftests/bpf/libbpf.a -lcap -lelf -lrt -lpthread -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_libbpf_open
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_pkt_access.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_pkt_access.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_xdp.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_xdp.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_l4lb.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_l4lb.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_tcp_estats.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_tcp_estats.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_obj_id.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_obj_id.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_pkt_md_access.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_pkt_md_access.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_xdp_redirect.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_xdp_redirect.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_xdp_meta.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_xdp_meta.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c sockmap_parse_prog.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/sockmap_parse_prog.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c sockmap_verdict_prog.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/sockmap_verdict_prog.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c dev_cgroup.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/dev_cgroup.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c sample_ret0.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/sample_ret0.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_tracepoint.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_tracepoint.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types -fno-inline \
-O2 -target bpf -emit-llvm -c test_l4lb_noinline.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_l4lb_noinline.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types -fno-inline \
-O2 -target bpf -emit-llvm -c test_xdp_noinline.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_xdp_noinline.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_stacktrace_map.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_stacktrace_map.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c sample_map_ret0.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/sample_map_ret0.o
clang -I. -I./include/uapi -I../../../include/uapi -I../../../../usr/include -Wno-compare-distinct-pointer-types \
-O2 -target bpf -emit-llvm -c test_tcpbpf_kern.c -o - | \
llc -march=bpf -mcpu=generic -filetype=obj -o /home/changbin/work/linux/tools/testing/selftests/bpf/test_tcpbpf_kern.o
changbin@gvt-dell-host:~/work/linux/tools/testing/selftests/bpf$
--
Thanks,
Changbin Du
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Alexei Starovoitov @ 2018-03-27 2:08 UTC (permalink / raw)
To: Steven Rostedt
Cc: Mathieu Desnoyers, Alexei Starovoitov, David S. Miller,
Daniel Borkmann, Linus Torvalds, Peter Zijlstra, netdev,
kernel-team, linux-api
In-Reply-To: <20180326200840.2b9eda2c@gandalf.local.home>
On 3/26/18 5:08 PM, Steven Rostedt wrote:
> On Mon, 26 Mar 2018 15:35:56 -0700
> Alexei Starovoitov <ast@fb.com> wrote:
>
>
>>> This patch is not reverting to the old code properly. It introduces a
>>> static inline void function that returns NULL. Please compile-test
>>> with CONFIG_TRACEPOINTS=n before submitting a patch involving tracepoints.
>>
>> right. good catch. v2 is coming.
>
> Either fold the patch into the original patch, or I'm pulling in
> Mathieu's patch and pushing it to Linus come the merge window.
Ok. I will fold this patch into previous set and rebase the tree.
^ permalink raw reply
* Re: [PATCH v3 net 1/5] tcp: feed correct number of pkts acked to cc modules also in recovery
From: Yuchung Cheng @ 2018-03-27 2:07 UTC (permalink / raw)
To: Ilpo Järvinen; +Cc: netdev, Neal Cardwell, Eric Dumazet, Sergei Shtylyov
In-Reply-To: <1520936711-16784-2-git-send-email-ilpo.jarvinen@helsinki.fi>
On Tue, Mar 13, 2018 at 3:25 AM, Ilpo Järvinen
<ilpo.jarvinen@helsinki.fi> wrote:
>
> A miscalculation for the number of acknowledged packets occurs during
> RTO recovery whenever SACK is not enabled and a cumulative ACK covers
> any non-retransmitted skbs. The reason is that pkts_acked value
> calculated in tcp_clean_rtx_queue is not correct for slow start after
> RTO as it may include segments that were not lost and therefore did
> not need retransmissions in the slow start following the RTO. Then
> tcp_slow_start will add the excess into cwnd bloating it and
> triggering a burst.
>
> Instead, we want to pass only the number of retransmitted segments
> that were covered by the cumulative ACK (and potentially newly sent
> data segments too if the cumulative ACK covers that far).
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
> ---
> net/ipv4/tcp_input.c | 16 +++++++++++++++-
> 1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 9a1b3c1..4a26c09 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -3027,6 +3027,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> long seq_rtt_us = -1L;
> long ca_rtt_us = -1L;
> u32 pkts_acked = 0;
> + u32 rexmit_acked = 0;
> + u32 newdata_acked = 0;
> u32 last_in_flight = 0;
> bool rtt_update;
> int flag = 0;
> @@ -3056,8 +3058,10 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> }
>
> if (unlikely(sacked & TCPCB_RETRANS)) {
> - if (sacked & TCPCB_SACKED_RETRANS)
> + if (sacked & TCPCB_SACKED_RETRANS) {
> tp->retrans_out -= acked_pcount;
> + rexmit_acked += acked_pcount;
> + }
> flag |= FLAG_RETRANS_DATA_ACKED;
> } else if (!(sacked & TCPCB_SACKED_ACKED)) {
> last_ackt = skb->skb_mstamp;
> @@ -3070,6 +3074,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> reord = start_seq;
> if (!after(scb->end_seq, tp->high_seq))
> flag |= FLAG_ORIG_SACK_ACKED;
> + else
> + newdata_acked += acked_pcount;
> }
>
> if (sacked & TCPCB_SACKED_ACKED) {
> @@ -3151,6 +3157,14 @@ static int tcp_clean_rtx_queue(struct sock *sk, u32 prior_fack,
> }
>
> if (tcp_is_reno(tp)) {
> + /* Due to discontinuity on RTO in the artificial
> + * sacked_out calculations, TCP must restrict
> + * pkts_acked without SACK to rexmits and new data
> + * segments
> + */
> + if (icsk->icsk_ca_state == TCP_CA_Loss)
> + pkts_acked = rexmit_acked + newdata_acked;
> +
My understanding is there are two problems
1) your fix: the reordering logic in tcp-remove_reno_sacks requires
precise cumulatively acked count, not newly acked count?
2) current code: pkts_acked can substantially over-estimate the newly
delivered pkts in both SACK and non-SACK cases. For example, let's say
99/100 packets are already sacked, and the next ACK acks 100 pkts.
pkts_acked == 100 but really only one packet is delivered. It's wrong
to inform congestion control that 100 packets have just delivered.
AFAICT, the CCs that have pkts_acked callbacks all treat pkts_acked as
the newly delivered packets.
A better fix for both SACK and non-SACK, seems to be moving
ca_ops->pkts_acked into tcp_cong_control, where the "acked_sacked" is
calibrated? this is what BBR is currently doing to avoid these pitfalls.
> tcp_remove_reno_sacks(sk, pkts_acked);
> } else {
> int delta;
> --
> 2.7.4
>
^ permalink raw reply
* RE: [Intel-wired-lan] [next-queue PATCH v5 7/9] igb: Add MAC address support for ethtool nftuple filters
From: Brown, Aaron F @ 2018-03-27 1:40 UTC (permalink / raw)
To: Gomes, Vinicius, intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org, Sanchez-Palencia, Jesus
In-Reply-To: <87lgeecryc.fsf@intel.com>
> From: Gomes, Vinicius
> Sent: Monday, March 26, 2018 4:56 PM
> To: Brown, Aaron F <aaron.f.brown@intel.com>; intel-wired-
> lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Sanchez-Palencia, Jesus <jesus.sanchez-
> palencia@intel.com>
> Subject: RE: [Intel-wired-lan] [next-queue PATCH v5 7/9] igb: Add MAC
> address support for ethtool nftuple filters
>
> Hi Aaron,
>
> "Brown, Aaron F" <aaron.f.brown@intel.com> writes:
>
> >
> > Maybe not "this" patch, but this is the one that enables the ethtool
> commands, so replying here.
> > The filters do not seem to take effect with this version (v5) of the
> > series. The commands are accepted for i210 and rejected with
> > unsupported messages for the other adapters (as desired) and an
> > ethtool -n shows the filter, however, with either the src or dst
> > filter set I can run traffic (netperf) that should be caught by the
> > filter and rather than being directed to the single queue it is spread
> > across queues as would be expected without the filter set.
> >
> > The test system still has a kernel / driver with the v4 series of this
> > patch set and the exact same filter commands / system setup does
> > filter the traffic to the specified rx queue with the v4 series.
>
> That's interesting. The only difference is that now non steering filters
> (filters added by 'ip (m)addr', PACKET_ADD_MEMBERSHIP and the local MAC
> address, for example) do not have the QSEL bit set.
>
> On my tests I cannot detect any change in behaviour between those two
> versions of the series, for example. trying to add a filter for the
> local MAC address has no visible effect in both versions. (This raises a
> question: should this be an error, or should this override the default
> entry configuration, or this behaviour is fine?)
>
> Can you share more information about your tests? so I can reproduce it here.
Sure, on I'm running a system the i210 is eth3 and it is linked back to back (crossover cable) to a link partner. The i210 has a mac address of a0:36:9f:10:cc:d7 and the MAC address of the port it is connected to is 00:1b:21:60:29:ea. It should not matter, but eth0 is an older e1000 based adapter (82540EM) I use for connecting ssh sessions, eth1 and eth2 are other igb ports (a dual port 82575EB adapter.)
I tried both an ethtool src and ethtool dst filter with these addresses and with the older version they both seemed to work and with the latest patch in place neither seemed to work. My steps were to set the filter, check that ethtool shows the filter in place then run some netperf traffic (using a script we have called netperf_stress that cycles through a number of sessions to provide constant traffic) and from another login session I watched the rx_queue counters.
For the src filter I used the MAC address of the link partner eth3 interface, 00:1b:21:60:29:ea :
-----------------------------------------------------------------------------
u1485:[0]/root> ethtool -N eth3 flow-type ether src 00:1b:21:60:29:ea action 0
Added rule with ID 15
u1485:[0]/root> ethtool -n eth3
4 RX rings available
Total 1 rules
Filter: 15
Flow Type: Raw Ethernet
Src MAC addr: 00:1B:21:60:29:EA mask: 00:00:00:00:00:00
Dest MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF
Ethertype: 0x0 mask: 0xFFFF
Action: Direct to VF 0 queue 0
u1485:[0]/root>netperf_stress u0485-3
Test limit is NONE
Found netperf support
u0485:[0]/root
Contacted u0485-3
---------> Testing with u0485-3 - Testing since Mon Mar 26 17:35:32 PDT 2018
...
...
-----------------------------------------------------------------------------
Then on another login I watch on an ethtool stat dump grepping for rx_queue. I threw a -d to get a better visual of the differences.
-----------------------------------------------------------------------------
1485:[1]/root> watch -d 'ethtool -S eth3|grep rx_queue'
Every 2.0s: ethtool -S eth3|grep rx_queue Mon Mar 26 18:19:21 2018
rx_queue_0_packets: 13
rx_queue_0_bytes: 2204
rx_queue_0_drops: 0
rx_queue_0_csum_err: 0
rx_queue_0_alloc_failed: 0
rx_queue_1_packets: 330677
rx_queue_1_bytes: 500632203
rx_queue_1_drops: 0
rx_queue_1_csum_err: 0
rx_queue_1_alloc_failed: 0
rx_queue_2_packets: 141902
rx_queue_2_bytes: 214787953
rx_queue_2_drops: 0
rx_queue_2_csum_err: 0
rx_queue_2_alloc_failed: 0
rx_queue_3_packets: 234377
rx_queue_3_bytes: 347172517
rx_queue_3_drops: 0
rx_queue_3_csum_err: 0
rx_queue_3_alloc_failed: 0
-----------------------------------------------------------------------------
After a few moments I can tell the counters are all over the place. I do pretty much the exact same thing for the dst filter, except I use the i210's MAC address for the destination instead of the one on the link partner:
-----------------------------------------------------------------------------
u1485:[1]/root> ethtool -N eth3 delete 15
u1485:[1]/root> ethtool -N eth3 flow-type ether dst a0:36:9f:10:cc:d7 action 0
Added rule with ID 15
u1485:[1]/root> ethtool -n eth3
4 RX rings available
Total 1 rules
Filter: 15
Flow Type: Raw Ethernet
Src MAC addr: 00:00:00:00:00:00 mask: FF:FF:FF:FF:FF:FF
Dest MAC addr: A0:36:9F:10:CC:D7 mask: 00:00:00:00:00:00
Ethertype: 0x0 mask: 0xFFFF
Action: Direct to VF 0 queue 0
u1485:[1]/root> netperf_stress u0485-3
Test limit is NONE
Found netperf support
u0485:[0]/root
Contacted u0485-3
---------> Testing with u0485-3 - Testing since Mon Mar 26 17:35:32 PDT 2018
...
...
-----------------------------------------------------------------------------
And watching the rx_queue counters continues to be spread across the different queues. This is with Jeff Kirsher's next queue, kernel 4.16.0-rc4_next-queue_dev-queue_e31d20a, which has the series of 8 igb patches applied.
When I go back and run the an older build (with an earlier version of the series) of the same tree, 4.16.0-rc4_next-queue_dev-queue_84a3942, with the same procedure and same systems all the rx traffic is relegated to queue 0 (or whichever queue I assign it to) for either the src or dst filter. Here is a sample of my counters after it had been running netperf_stress over the weekend:
-----------------------------------------------------------------------------1485:[1]/root> watch -d 'ethtool -S eth3|grep rx_queue'
Every 2.0s: ethtool -S eth3|grep rx_queue Mon Mar 26 17:37:44 2018
rx_queue_0_packets: 16453268766
rx_queue_0_bytes: 23420384893393
rx_queue_0_drops: 0
rx_queue_0_csum_err: 0
rx_queue_0_alloc_failed: 0
rx_queue_1_packets: 0
rx_queue_1_bytes: 0
rx_queue_1_drops: 0
rx_queue_1_csum_err: 0
rx_queue_1_alloc_failed: 0
rx_queue_2_packets: 0
rx_queue_2_bytes: 0
rx_queue_2_drops: 0
rx_queue_2_csum_err: 0
rx_queue_2_alloc_failed: 0
rx_queue_3_packets: 0
rx_queue_3_bytes: 0
rx_queue_3_drops: 0
rx_queue_3_csum_err: 0
rx_queue_3_alloc_failed: 0
>
>
> Thank you,
> --
> Vinicius
^ permalink raw reply
* ip6_forward / NF_HOOK and counters
From: Jeff Barnhill @ 2018-03-27 1:35 UTC (permalink / raw)
To: netdev
At the end of ip6_forward(), is there a good reason why
IPSTATS_MIB_OUTFORWDATAGRAMS and IPSTATS_MIB_OUTOCTETS are incremented
before the NF_HOOK? If the hook steals or drops the packet, this
counts still go up, which seems incorrect.
v4/ip_forward() increments these counters in ip_forward_finish(). It
seems that v6 should do it in ip6_forward_finish() ?? Thoughts?
Jeff
^ permalink raw reply
* Re: [PATCH net-next] XDP router for veth
From: Md. Islam @ 2018-03-27 1:03 UTC (permalink / raw)
To: David Miller
Cc: ebiederm, Pavel Emelyanov, netdev, shemminger, Eric Dumazet,
David Ahern, Roopa Prabhu, Tom Herbert, alexei.starovoitov,
Florian Fainelli, brouer, bjorn.topel, magnus.karlsson, u9012063
In-Reply-To: <CAFgPn1CZ4E=FaHvywCoTpWGo9y5bzF7nmvwUmUQDnzE+wx=yOg@mail.gmail.com>
On Mon, Mar 26, 2018 at 9:01 PM, Md. Islam <mislam4@kent.edu> wrote:
> On Mon, Mar 26, 2018 at 10:21 AM, David Miller <davem@davemloft.net> wrote:
>> From: "Md. Islam" <mislam4@kent.edu>
>> Date: Fri, 23 Mar 2018 02:43:16 -0400
>>
>>> +#ifdef CONFIG_XDP_ROUTER
>>> + //if IP forwarding is enabled on the receiver, create xdp_buff
>>> + //from skb and call xdp_router_forward()
>>
>> Never use C++ comments, only use C style.
>>
>>> + if(is_forwarding_enabled(rcv)){
>>
>> There must be a space between 'if' and the openning parenthesis. You need
>> to also have a space before the openning curly braces.
>>
>> In fact this entire patch is full of coding style issues, please run your
>> changes through checkpatch.pl before resubmitting.
>
>
> Thanks David for the suggestions! I fixed all the formating errors. I
> also modified the forwarding logic. Now the iperf throughput improved
> from
> 53.8Gb/s to around 60Gb/s and the median RTT improved from .055 ms to
> .35 ms. The patch is attached.
Sorry, the RTT improved from .055 ms to .035 ms.
^ permalink raw reply
* Re: [PATCH net-next] XDP router for veth
From: Md. Islam @ 2018-03-27 1:01 UTC (permalink / raw)
To: David Miller
Cc: ebiederm, Pavel Emelyanov, netdev, shemminger, Eric Dumazet,
David Ahern, Roopa Prabhu, Tom Herbert, alexei.starovoitov,
Florian Fainelli, brouer, bjorn.topel, magnus.karlsson, u9012063
In-Reply-To: <20180326.102110.1068883293248258544.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 921 bytes --]
On Mon, Mar 26, 2018 at 10:21 AM, David Miller <davem@davemloft.net> wrote:
> From: "Md. Islam" <mislam4@kent.edu>
> Date: Fri, 23 Mar 2018 02:43:16 -0400
>
>> +#ifdef CONFIG_XDP_ROUTER
>> + //if IP forwarding is enabled on the receiver, create xdp_buff
>> + //from skb and call xdp_router_forward()
>
> Never use C++ comments, only use C style.
>
>> + if(is_forwarding_enabled(rcv)){
>
> There must be a space between 'if' and the openning parenthesis. You need
> to also have a space before the openning curly braces.
>
> In fact this entire patch is full of coding style issues, please run your
> changes through checkpatch.pl before resubmitting.
Thanks David for the suggestions! I fixed all the formating errors. I
also modified the forwarding logic. Now the iperf throughput improved
from
53.8Gb/s to around 60Gb/s and the median RTT improved from .055 ms to
.35 ms. The patch is attached.
[-- Attachment #2: xdp-fastpath.patch --]
[-- Type: text/x-patch, Size: 8409 bytes --]
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 944ec3c..8474eef 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -328,6 +328,18 @@ config VETH
When one end receives the packet it appears on its pair and vice
versa.
+config XDP_ROUTER
+ bool "XDP router for veth"
+ depends on IP_ADVANCED_ROUTER
+ depends on VETH
+ default y
+ ---help---
+ This option will enable IP forwarding on incoming xdp_buff.
+ Currently it is only supported by veth. Say y or n.
+
+ Currently veth uses slow path for packet forwarding. This option
+ forwards packets as soon as it is received (as XDP generic).
+
config VIRTIO_NET
tristate "Virtio network driver"
depends on VIRTIO
diff --git a/drivers/net/veth.c b/drivers/net/veth.c
index a69ad39..4ce10c9 100644
--- a/drivers/net/veth.c
+++ b/drivers/net/veth.c
@@ -111,6 +111,28 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
goto drop;
}
+#ifdef CONFIG_XDP_ROUTER
+
+ /* if IP forwarding is enabled on the receiver, create xdp_buff
+ * from skb and call xdp_router_forward()
+ */
+ if (is_forwarding_enabled(rcv)) {
+ struct xdp_buff *xdp = kmalloc(sizeof(*xdp), GFP_KERNEL);
+
+ xdp->data = skb->data;
+ xdp->data_end = skb->data + (skb->len - skb->data_len);
+ xdp->data_meta = skb;
+ if (likely(xdp_router_forward(rcv, xdp) == NET_RX_SUCCESS)) {
+ struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
+
+ u64_stats_update_begin(&stats->syncp);
+ stats->bytes += length;
+ stats->packets++;
+ u64_stats_update_end(&stats->syncp);
+ goto success;
+ }
+ }
+#endif
if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
@@ -122,6 +144,7 @@ static netdev_tx_t veth_xmit(struct sk_buff *skb, struct net_device *dev)
drop:
atomic64_inc(&priv->dropped);
}
+success:
rcu_read_unlock();
return NETDEV_TX_OK;
}
@@ -276,6 +299,61 @@ static void veth_set_rx_headroom(struct net_device *dev, int new_hr)
rcu_read_unlock();
}
+#ifdef CONFIG_XDP_ROUTER
+int veth_xdp_xmit(struct net_device *dev, struct xdp_buff *xdp)
+{
+ struct veth_priv *priv = netdev_priv(dev);
+ struct net_device *rcv;
+ struct ethhdr *ethh;
+ struct sk_buff *skb;
+ int length = xdp->data_end - xdp->data;
+
+ rcu_read_lock();
+ rcv = rcu_dereference(priv->peer);
+ if (unlikely(!rcv)) {
+ kfree(xdp);
+ goto drop;
+ }
+
+ /* Update MAC address and checksum */
+ ethh = eth_hdr_xdp(xdp);
+ ether_addr_copy(ethh->h_source, dev->dev_addr);
+ ether_addr_copy(ethh->h_dest, rcv->dev_addr);
+
+ /* if IP forwarding is enabled on the receiver,
+ * call xdp_router_forward()
+ */
+ if (is_forwarding_enabled(rcv)) {
+ if (likely(xdp_router_forward(rcv, xdp) == NET_RX_SUCCESS)) {
+ struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
+
+ u64_stats_update_begin(&stats->syncp);
+ stats->bytes += length;
+ stats->packets++;
+ u64_stats_update_end(&stats->syncp);
+ goto success;
+ }
+ }
+
+ /* Local deliver */
+ skb = (struct sk_buff *)xdp->data_meta;
+ if (likely(dev_forward_skb(rcv, skb) == NET_RX_SUCCESS)) {
+ struct pcpu_vstats *stats = this_cpu_ptr(dev->vstats);
+
+ u64_stats_update_begin(&stats->syncp);
+ stats->bytes += length;
+ stats->packets++;
+ u64_stats_update_end(&stats->syncp);
+ } else {
+drop:
+ atomic64_inc(&priv->dropped);
+ }
+success:
+ rcu_read_unlock();
+ return NETDEV_TX_OK;
+}
+#endif
+
static const struct net_device_ops veth_netdev_ops = {
.ndo_init = veth_dev_init,
.ndo_open = veth_open,
@@ -290,6 +368,9 @@ static const struct net_device_ops veth_netdev_ops = {
.ndo_get_iflink = veth_get_iflink,
.ndo_features_check = passthru_features_check,
.ndo_set_rx_headroom = veth_set_rx_headroom,
+#ifdef CONFIG_XDP_ROUTER
+ .ndo_xdp_xmit = veth_xdp_xmit,
+#endif
};
#define VETH_FEATURES (NETIF_F_SG | NETIF_F_FRAGLIST | NETIF_F_HW_CSUM | \
diff --git a/include/linux/ip.h b/include/linux/ip.h
index 492bc65..025a3ec 100644
--- a/include/linux/ip.h
+++ b/include/linux/ip.h
@@ -19,6 +19,29 @@
#include <linux/skbuff.h>
#include <uapi/linux/ip.h>
+#include <linux/filter.h>
+
+#ifdef CONFIG_XDP_ROUTER
+
+#define MIN_PACKET_SIZE 55
+
+static inline struct iphdr *ip_hdr_xdp(const struct xdp_buff *xdp)
+{
+ return (struct iphdr *)(xdp->data+ETH_HLEN);
+}
+
+static inline struct ethhdr *eth_hdr_xdp(const struct xdp_buff *xdp)
+{
+ return (struct ethhdr *)(xdp->data);
+}
+
+static inline bool is_xdp_forwardable(const struct xdp_buff *xdp)
+{
+ return xdp->data_end - xdp->data >= MIN_PACKET_SIZE;
+}
+
+#endif
+
static inline struct iphdr *ip_hdr(const struct sk_buff *skb)
{
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 4c77f39..8369e5e 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3290,6 +3290,11 @@ static inline void dev_consume_skb_any(struct sk_buff *skb)
__dev_kfree_skb_any(skb, SKB_REASON_CONSUMED);
}
+#ifdef CONFIG_XDP_ROUTER
+bool is_forwarding_enabled(struct net_device *dev);
+int xdp_router_forward(struct net_device *dev, struct xdp_buff *xdp);
+#endif
+
void generic_xdp_tx(struct sk_buff *skb, struct bpf_prog *xdp_prog);
int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb);
int netif_rx(struct sk_buff *skb);
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index f805243..623b2de 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -369,6 +369,12 @@ int fib_sync_down_dev(struct net_device *dev, unsigned long event, bool force);
int fib_sync_down_addr(struct net_device *dev, __be32 local);
int fib_sync_up(struct net_device *dev, unsigned int nh_flags);
+#ifdef CONFIG_XDP_ROUTER
+int ip_route_lookup(__be32 daddr, __be32 saddr,
+ u8 tos, struct net_device *dev,
+ struct fib_result *res);
+#endif
+
#ifdef CONFIG_IP_ROUTE_MULTIPATH
int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4,
const struct sk_buff *skb);
diff --git a/net/core/dev.c b/net/core/dev.c
index dda9d7b..18c18f3 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4090,6 +4090,56 @@ int do_xdp_generic(struct bpf_prog *xdp_prog, struct sk_buff *skb)
}
EXPORT_SYMBOL_GPL(do_xdp_generic);
+#ifdef CONFIG_XDP_ROUTER
+
+bool is_forwarding_enabled(struct net_device *dev)
+{
+ struct in_device *in_dev;
+
+ /* verify forwarding is enabled on this interface */
+ in_dev = __in_dev_get_rcu(dev);
+ if (unlikely(!in_dev || !IN_DEV_FORWARD(in_dev)))
+ return false;
+
+ return true;
+}
+EXPORT_SYMBOL_GPL(is_forwarding_enabled);
+
+int xdp_router_forward(struct net_device *dev, struct xdp_buff *xdp)
+{
+ int err;
+ struct fib_result res;
+ struct iphdr *iph;
+ struct net_device *rcv;
+
+ if (unlikely(xdp->data_end - xdp->data < MIN_PACKET_SIZE))
+ return NET_RX_DROP;
+
+ iph = (struct iphdr *)(xdp->data + ETH_HLEN);
+
+ /*currently only supports IPv4
+ */
+ if (unlikely(iph->version != 4))
+ return NET_RX_DROP;
+
+ err = ip_route_lookup(iph->daddr, iph->saddr,
+ iph->tos, dev, &res);
+ if (unlikely(err))
+ return NET_RX_DROP;
+
+ rcv = FIB_RES_DEV(res);
+ if (likely(rcv)) {
+ if (likely(rcv->netdev_ops->ndo_xdp_xmit(rcv, xdp) ==
+ NETDEV_TX_OK))
+ return NET_RX_SUCCESS;
+ }
+
+ return NET_RX_DROP;
+}
+EXPORT_SYMBOL_GPL(xdp_router_forward);
+
+#endif
+
static int netif_rx_internal(struct sk_buff *skb)
{
int ret;
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 49cc1c1..2333205 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1866,6 +1866,35 @@ static int ip_mkroute_input(struct sk_buff *skb,
return __mkroute_input(skb, res, in_dev, daddr, saddr, tos);
}
+#ifdef CONFIG_XDP_ROUTER
+
+int ip_route_lookup(__be32 daddr, __be32 saddr,
+ u8 tos, struct net_device *dev,
+ struct fib_result *res)
+{
+ struct flowi4 fl4;
+ int err;
+ struct net *net = dev_net(dev);
+
+ fl4.flowi4_oif = 0;
+ fl4.flowi4_iif = dev->ifindex;
+ fl4.flowi4_mark = 0;
+ fl4.flowi4_tos = tos & IPTOS_RT_MASK;
+ fl4.flowi4_scope = RT_SCOPE_UNIVERSE;
+ fl4.flowi4_flags = 0;
+ fl4.daddr = daddr;
+ fl4.saddr = saddr;
+
+ err = fib_lookup(net, &fl4, res, 0);
+
+ if (unlikely(err != 0 || res->type != RTN_UNICAST))
+ return -EINVAL;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(ip_route_lookup);
+#endif
+
/*
* NOTE. We drop all the packets that has local source
* addresses, because every properly looped back packet
^ permalink raw reply related
* Re: [PATCH net-next 4/8] dt-bindings: net: add DT bindings for Microsemi Ocelot Switch
From: Rob Herring @ 2018-03-27 0:34 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, Alexandre Belloni, David S . Miller,
Allan Nielsen, razvan.stefanescu, Po Liu, Thomas Petazzoni,
netdev, devicetree, linux-kernel@vger.kernel.org, Linux-MIPS
In-Reply-To: <20180326225020.GF5862@lunn.ch>
On Mon, Mar 26, 2018 at 5:50 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>> ports and port collide with the OF graph binding. It would be good if
>> this moved to ethernet-port(s) or similar.
>
> Hi Rob
>
> Well, we have been using port in DSA since March 2013. ports is a bit
> newer, June 2016.
Yes, understood.
>
> Changing DSA is not going to happen. But new switch bindings could use
> ethernet-port(s). It just makes them inconsistent with existing switch
> drivers.
I'm not saying to change existing bindings, but evolve to something
that doesn't collide on new bindings if you don't have dependencies on
what the node names are. It's mainly so we can have something to key
off of to validate bindings better.
Rob
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Steven Rostedt @ 2018-03-27 0:08 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Mathieu Desnoyers, Alexei Starovoitov, David S. Miller,
Daniel Borkmann, Linus Torvalds, Peter Zijlstra, netdev,
kernel-team, linux-api
In-Reply-To: <761ce9e6-aea4-01d8-8ff0-a17ad8a92526@fb.com>
On Mon, 26 Mar 2018 15:35:56 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> > This patch is not reverting to the old code properly. It introduces a
> > static inline void function that returns NULL. Please compile-test
> > with CONFIG_TRACEPOINTS=n before submitting a patch involving tracepoints.
>
> right. good catch. v2 is coming.
Either fold the patch into the original patch, or I'm pulling in
Mathieu's patch and pushing it to Linus come the merge window.
-- Steve
^ permalink raw reply
* Re: [PATCH v2 bpf-next] bpf, tracing: unbreak lttng
From: Steven Rostedt @ 2018-03-27 0:07 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: davem, daniel, torvalds, peterz, mathieu.desnoyers, netdev,
kernel-team, linux-api
In-Reply-To: <20180326230220.1069180-1-ast@kernel.org>
On Mon, 26 Mar 2018 16:02:20 -0700
Alexei Starovoitov <ast@kernel.org> wrote:
> for_each_kernel_tracepoint() is used by out-of-tree lttng module
> and therefore cannot be changed.
This is false and misleading. NACK.
-- Steve
> Instead introduce kernel_tracepoint_find_by_name() to find
> tracepoint by name.
>
> Fixes: 9e9afbae6514 ("tracepoint: compute num_args at build time")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> v1->v2: fix 'undef CONFIG_TRACEPOINTS' build as spotted by Mathieu
>
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, tracing: unbreak lttng
From: Steven Rostedt @ 2018-03-27 0:06 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Alexei Starovoitov, davem, daniel, torvalds, peterz,
mathieu.desnoyers, netdev, kernel-team, linux-api
In-Reply-To: <24d0ff40-c6fd-6349-4a89-dffda22cb596@fb.com>
On Mon, 26 Mar 2018 15:25:32 -0700
Alexei Starovoitov <ast@fb.com> wrote:
> On 3/26/18 3:15 PM, Steven Rostedt wrote:
> > On Mon, 26 Mar 2018 15:08:45 -0700
> > Alexei Starovoitov <ast@kernel.org> wrote:
> >
> >> for_each_kernel_tracepoint() is used by out-of-tree lttng module
> >> and therefore cannot be changed.
> >> Instead introduce kernel_tracepoint_find_by_name() to find
> >> tracepoint by name.
> >>
> >> Fixes: 9e9afbae6514 ("tracepoint: compute num_args at build time")
> >> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> >
> > I'm curious, why can't you rebase? The first patch was never acked.
>
> because I think it makes sense to keep such things in the commit log
> and in the separate diff, so next developer is aware of what kind of
> minefield the tracpoints are.
This is a bunch of BS. It's not a minefield, and you can change that
function. Mathieu is perfectly fine in modifying his code to deal with
it. He has several times in the past. But I did not agree with the
approach you were taking, that is why I'm against it. You are playing
the straw man with this.
> No wonder some maintainers refuse to add them.
Good grief. No! The reason maintainers refuse to add them is that
userspace can depend on them, and if that happens, it becomes an ABI.
Stop with this nonsense.
-- Steve
^ permalink raw reply
* Re: bpf stable request
From: Daniel Borkmann @ 2018-03-27 0:00 UTC (permalink / raw)
To: Chenbo Feng; +Cc: netdev, Lorenzo Colitti, Joel Fernandes
In-Reply-To: <360670e8-e80c-eb56-8892-cfa8ff54235a@gmail.com>
On 03/27/2018 01:52 AM, Chenbo Feng wrote:
> 0fa4fe85f4724fff89b09741c437cbee9cf8b008 bpf: skip unnecessary capability check
>
> This patch fixes the false alarms from security system such as selinux when doing the capability check. The problem exists since the sysctl_unprivileged_bpf_disabled is added in linux 4.4. So I suggest to backport this patch to all LTS stable branches starting from linux-4.4-y.
I will include this into the next batch, thanks guys!
^ permalink raw reply
* RE: [Intel-wired-lan] [next-queue PATCH v5 7/9] igb: Add MAC address support for ethtool nftuple filters
From: Vinicius Costa Gomes @ 2018-03-26 23:55 UTC (permalink / raw)
To: Brown, Aaron F, intel-wired-lan@lists.osuosl.org
Cc: netdev@vger.kernel.org, Sanchez-Palencia, Jesus
In-Reply-To: <309B89C4C689E141A5FF6A0C5FB2118B8C81BBB3@ORSMSX101.amr.corp.intel.com>
Hi Aaron,
"Brown, Aaron F" <aaron.f.brown@intel.com> writes:
>
> Maybe not "this" patch, but this is the one that enables the ethtool commands, so replying here.
> The filters do not seem to take effect with this version (v5) of the
> series. The commands are accepted for i210 and rejected with
> unsupported messages for the other adapters (as desired) and an
> ethtool -n shows the filter, however, with either the src or dst
> filter set I can run traffic (netperf) that should be caught by the
> filter and rather than being directed to the single queue it is spread
> across queues as would be expected without the filter set.
>
> The test system still has a kernel / driver with the v4 series of this
> patch set and the exact same filter commands / system setup does
> filter the traffic to the specified rx queue with the v4 series.
That's interesting. The only difference is that now non steering filters
(filters added by 'ip (m)addr', PACKET_ADD_MEMBERSHIP and the local MAC
address, for example) do not have the QSEL bit set.
On my tests I cannot detect any change in behaviour between those two
versions of the series, for example. trying to add a filter for the
local MAC address has no visible effect in both versions. (This raises a
question: should this be an error, or should this override the default
entry configuration, or this behaviour is fine?)
Can you share more information about your tests? so I can reproduce it here.
Thank you,
^ permalink raw reply
* bpf stable request
From: Chenbo Feng @ 2018-03-26 23:52 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: netdev, Lorenzo Colitti, Joel Fernandes
0fa4fe85f4724fff89b09741c437cbee9cf8b008 bpf: skip unnecessary
capability check
This patch fixes the false alarms from security system such as selinux
when doing the capability check. The problem exists since the
sysctl_unprivileged_bpf_disabled is added in linux 4.4. So I suggest to
backport this patch to all LTS stable branches starting from linux-4.4-y.
^ permalink raw reply
* Re: [PATCH net-next 0/2] net: broadcom: Adaptive interrupt coalescing
From: Florian Fainelli @ 2018-03-26 23:40 UTC (permalink / raw)
To: Tal Gilboa, netdev
Cc: davem, jaedon.shin, pgynther, opendmb, michael.chan, gospo,
saeedm
In-Reply-To: <67bfc05b-01cf-401f-d802-b3b5cf400928@mellanox.com>
On 03/26/2018 04:21 PM, Tal Gilboa wrote:
> On 3/27/2018 1:29 AM, Florian Fainelli wrote:
>> On 03/26/2018 03:04 PM, Florian Fainelli wrote:
>>> On 03/26/2018 02:16 PM, Tal Gilboa wrote:
>>>> On 3/23/2018 4:19 AM, Florian Fainelli wrote:
>>>>> Hi all,
>>>>>
>>>>> This patch series adds adaptive interrupt coalescing for the Gigabit
>>>>> Ethernet
>>>>> drivers SYSTEMPORT and GENET.
>>>>>
>>>>> This really helps lower the interrupt count and system load, as
>>>>> measured by
>>>>> vmstat for a Gigabit TCP RX session:
>>>>
>>>> I don't see an improvement in system load, the opposite - 42% vs. 100%
>>>> for SYSTEMPORT and 85% vs. 100% for GENET. Both with the same
>>>> bandwidth.
>>>
>>> Looks like I did not extract the correct data the load could spike in
>>> both cases (with and without net_dim) up to 100, but averaged over the
>>> transmission I see the following:
>>>
>>> GENET without:
>>> 1 0 0 1169568 0 25556 0 0 0 0 130079
>>> 62795 2
>>> 86 13 0 0
>>>
>>> GENET with:
>>> 1 0 0 1169536 0 25556 0 0 0 0 10566 10869 1
>>> 21 78 0 0
>>>
>>>> Am I missing something? Talking about bandwidth, I would expect 941Mb/s
>>>> (assuming this is TCP over IPv4). Do you know why the reduced interrupt
>>>> rate doesn't improve bandwidth?
>>>
>>> I am assuming that this comes down to a latency, still capturing some
>>> pcap files to analyze the TCP session with wireshark and see if that is
>>> indeed what is going on. The test machine is actually not that great
>
> I would expect 1GbE full wire speed on almost any setup. I'll try
> applying your code on my setup and see what I get.
The test machine that I am using appears to be loaded by other non
networking workload which perturbs the tests I am running, other than
than I agree, wire speed should be expected.
>
>>>
>>>> Also, any effect on the client side (you
>>>> mentioned enabling TX moderation for SYSTEMPORT)?
>>>
>>> Yes, on SYSTEMPORT, being the TCP IPv4 client, I have the following:
>>>
>>> SYSTEMPORT without:
>>> 2 0 0 191428 0 25748 0 0 0 0 86254 264
>>> 0 41
>>> 59 0 0
>>>
>>> SYSTEMPORT with:
>>> 3 0 0 190176 0 25748 0 0 0 0 45485 31332 0
>>> 100 0 0 0
>>>
>>> I don't get top to agree with these load results though but it looks
>>> like we just have the CPU spinning more, does not look like a win.
>>
>> The problem appears to be the timeout selection on TX, ignoring it
>> completely allows us to keep the load average down while maintaining the
>> bandwidth. Looks like NAPI on TX already does a good job, so interrupt
>> mitigation on TX is not such a great idea actually...
>
> I saw a similar behavior for TX. For me the issue was too many
> outstanding bytes without a completion (defined to be 256KB by sysctl
> net.ipv4.tcp_limit_output_bytes). I tested on a 100GbE connection so
> with reasonable timeout values I already waited too long (4 TSO
> sessions). For the 1GbE case this might have no effect since you need a
> very long timeout. I'm currently working on adding TX support for dim.
> If you don't see a good benefit currently you might want to wait a
> little with TX adaptive interrupt moderation. Maybe only adjust static
> moderation for now?
Yes static moderation appears to be doing just fine.
>
>>
>> Also, doing UDP TX tests shows that we can lower the interrupt count by
>> setting an appropriate tx-frames (as expected), but we won't be lowering
>> the CPU load since that is inherently a CPU intensive work. Past
>
> Do you see higher TX UDP bandwidth? If you are bounded by CPU on both
> cases I would at least expect higher bandwidth with less interrupts
> since you reduce work from the CPU.
UDP bandwidth was intentionally limited the UDP bandwidth to
800Mbits/sec, we are definitively not CPU bound (18% CPU load), but we
can still lower the interrupt count.
--
Florian
^ permalink raw reply
* [PATCH 6/6] rhashtable: allow element counting to be disabled.
From: NeilBrown @ 2018-03-26 23:33 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <152210688405.11435.13010923693146415942.stgit@noble>
If multiple CPUs are performing concurrent updates, they can
contend on accessing the element counter even when they
don't often content on hash chains or spin locks. This can
hurt performance.
The nelems counter is only used to trigger a resize at the
70% and 30% marks, so it does not need to be precise.
It is easy to calculate an approximate value when the table
is being rehashed, and this happens when a chain is found to
be 16 elements long. So just moving the counting from
"every update" to "every rehash" removes lots of contention,
but has the down-side is that it allows the max bucket size
to grow to 16 (so average is probably under 8). The normal
average is close to 1.
As a rehash can sometimes not see all (or any) elements, such as when
multiple tables are in the table chain, it is only safe to increase
nelems to match the number rehashed, never to decrease it.
If a client wants minimal contention while still maintaining
a shorter chain length, it can run a periodic task which
counts the number of elements and updates ->nelems directly.
Signed-off-by: NeilBrown <neilb@suse.com>
---
include/linux/rhashtable.h | 26 ++++++++++++++++++--------
lib/rhashtable.c | 22 +++++++++++++++-------
2 files changed, 33 insertions(+), 15 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index abdeb1f3f378..d0ce5635540f 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -134,6 +134,11 @@ struct rhashtable;
* @never_fail_insert: Insert will always succeed, even if table will become
* unbalanced. Without this, -E2BIG, -EBUSY, and -ENOMEM are possible
* errors from rhashtable_*insert*()
+ * @disable_count: Disable precise counting of number of entries. It is only
+ * updated approximately when the hash table is resized.
+ * This reduces contention in parallel updates, but means we only
+ * grow the table when a hash chain length reaches 16 or when owner
+ * directly updates ->nelems.
* @nulls_base: Base value to generate nulls marker
* @hashfn: Hash function (default: jhash2 if !(key_len % 4), or jhash)
* @obj_hashfn: Function to hash object
@@ -148,6 +153,7 @@ struct rhashtable_params {
u16 min_size;
bool automatic_shrinking;
bool never_fail_insert;
+ bool disable_count;
u8 locks_mul;
u32 nulls_base;
rht_hashfn_t hashfn;
@@ -838,10 +844,12 @@ static inline void *__rhashtable_insert_fast(
rcu_assign_pointer(*pprev, obj);
- if (params.never_fail_insert)
- atomic_add_unless(&ht->nelems, 1, INT_MAX);
- else
- atomic_inc(&ht->nelems);
+ if (!params.disable_count) {
+ if (params.never_fail_insert)
+ atomic_add_unless(&ht->nelems, 1, INT_MAX);
+ else
+ atomic_inc(&ht->nelems);
+ }
if (rht_grow_above_75(ht, tbl))
schedule_work(&ht->run_work);
@@ -1113,10 +1121,12 @@ static inline int __rhashtable_remove_fast_one(
spin_unlock_bh(lock);
if (err > 0) {
- if (params.never_fail_insert)
- atomic_add_unless(&ht->nelems, -1, INT_MAX);
- else
- atomic_dec(&ht->nelems);
+ if (!params.disable_count) {
+ if (params.never_fail_insert)
+ atomic_add_unless(&ht->nelems, -1, INT_MAX);
+ else
+ atomic_dec(&ht->nelems);
+ }
if (unlikely(ht->p.automatic_shrinking &&
rht_shrink_below_30(ht, tbl)))
schedule_work(&ht->run_work);
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 427836aace60..686193faf271 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -278,12 +278,13 @@ static int rhashtable_rehash_chain(struct rhashtable *ht,
struct bucket_table *old_tbl = rht_dereference(ht->tbl, ht);
spinlock_t *old_bucket_lock;
int err;
+ int cnt = 0;
old_bucket_lock = rht_bucket_lock(old_tbl, old_hash);
spin_lock_bh(old_bucket_lock);
while (!(err = rhashtable_rehash_one(ht, old_hash)))
- ;
+ cnt++;
if (err == -ENOENT) {
old_tbl->rehash++;
@@ -291,7 +292,7 @@ static int rhashtable_rehash_chain(struct rhashtable *ht,
}
spin_unlock_bh(old_bucket_lock);
- return err;
+ return err ?: cnt;
}
static int rhashtable_rehash_attach(struct rhashtable *ht,
@@ -324,6 +325,7 @@ static int rhashtable_rehash_table(struct rhashtable *ht)
struct rhashtable_walker *walker;
unsigned int old_hash;
int err;
+ unsigned int cnt = 0;
new_tbl = rht_dereference(old_tbl->future_tbl, ht);
if (!new_tbl)
@@ -331,12 +333,16 @@ static int rhashtable_rehash_table(struct rhashtable *ht)
for (old_hash = 0; old_hash < old_tbl->size; old_hash++) {
err = rhashtable_rehash_chain(ht, old_hash);
- if (err)
+ if (err < 0)
return err;
+ if (INT_MAX - cnt > err)
+ cnt += err;
}
/* Publish the new table pointer. */
rcu_assign_pointer(ht->tbl, new_tbl);
+ if (ht->p.disable_count && cnt > atomic_read(&ht->nelems))
+ atomic_set(&ht->nelems, cnt);
spin_lock(&ht->lock);
list_for_each_entry(walker, &old_tbl->walkers, list)
@@ -582,10 +588,12 @@ static struct bucket_table *rhashtable_insert_one(struct rhashtable *ht,
rcu_assign_pointer(*pprev, obj);
- if (ht->p.never_fail_insert)
- atomic_add_unless(&ht->nelems, 1, INT_MAX);
- else
- atomic_inc(&ht->nelems);
+ if (!ht->p.disable_count) {
+ if (ht->p.never_fail_insert)
+ atomic_add_unless(&ht->nelems, 1, INT_MAX);
+ else
+ atomic_inc(&ht->nelems);
+ }
if (rht_grow_above_75(ht, tbl))
schedule_work(&ht->run_work);
^ permalink raw reply related
* [PATCH 5/6] rhashtable: support guaranteed successful insertion.
From: NeilBrown @ 2018-03-26 23:33 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <152210688405.11435.13010923693146415942.stgit@noble>
The current rhashtable will fail an insertion if the hashtable
it "too full", one of:
- table already has 2^31 elements (-E2BIG)
- a max_size was specified and table already has that
many elements (rounded up to power of 2) (-E2BIG)
- a single chain has more than 16 elements (-EBUSY)
- table has more elements than the current table size,
and allocating a new table fails (-ENOMEM)
- a new page needed to be allocated for a nested table,
and the memory allocation failed (-ENOMEM).
A traditional hash table does not have a concept of "too full", and
insertion only fails if the key already exists. Many users of hash
tables have separate means of limiting the total number of entries,
and are not susceptible to an attack which could cause unusually large
hash chains. For those users, the need to check for errors when
inserting objects to an rhashtable is an unnecessary burden and hence
a potential source of bugs (as these failures are likely to be rare).
This patch adds a "never_fail_insert" configuration parameter which
ensures that insertion will only fail if the key already exists.
When this option is in effect:
- nelems is capped at INT_MAX and will never decrease once it reaches
that value
- max_size is largely ignored
- elements will be added to a table that is nominally "full", though
a rehash will be scheduled
- a new table will never be allocated directly by the insert
function, that is always left for the worker.
For this to trigger a rehash when long chains are detected (possibly
still useful) an extra field in the table records if a long chain
has been seen. This shares a word with the 'nest' value. As
'nest' is never changed once the table is created, updating the
new ->long_chain without locking cannot cause any corruption.
Signed-off-by: NeilBrown <neilb@suse.com>
---
include/linux/rhashtable.h | 18 +++++++++++++++---
lib/rhashtable.c | 27 +++++++++++++++++++--------
2 files changed, 34 insertions(+), 11 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 4ffd96949d4f..abdeb1f3f378 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -77,6 +77,7 @@ struct rhlist_head {
* struct bucket_table - Table of hash buckets
* @size: Number of hash buckets
* @nest: Number of bits of first-level nested table.
+ * @long_chain: %true when a chain longer than RHT_ELASTICITY seen.
* @rehash: Current bucket being rehashed
* @hash_rnd: Random seed to fold into hash
* @locks_mask: Mask to apply before accessing locks[]
@@ -89,7 +90,8 @@ struct rhlist_head {
*/
struct bucket_table {
unsigned int size;
- unsigned int nest;
+ unsigned short nest;
+ bool long_chain;
unsigned int rehash;
u32 hash_rnd;
unsigned int locks_mask;
@@ -129,6 +131,9 @@ struct rhashtable;
* @min_size: Minimum size while shrinking
* @locks_mul: Number of bucket locks to allocate per cpu (default: 32)
* @automatic_shrinking: Enable automatic shrinking of tables
+ * @never_fail_insert: Insert will always succeed, even if table will become
+ * unbalanced. Without this, -E2BIG, -EBUSY, and -ENOMEM are possible
+ * errors from rhashtable_*insert*()
* @nulls_base: Base value to generate nulls marker
* @hashfn: Hash function (default: jhash2 if !(key_len % 4), or jhash)
* @obj_hashfn: Function to hash object
@@ -142,6 +147,7 @@ struct rhashtable_params {
unsigned int max_size;
u16 min_size;
bool automatic_shrinking;
+ bool never_fail_insert;
u8 locks_mul;
u32 nulls_base;
rht_hashfn_t hashfn;
@@ -832,7 +838,10 @@ static inline void *__rhashtable_insert_fast(
rcu_assign_pointer(*pprev, obj);
- atomic_inc(&ht->nelems);
+ if (params.never_fail_insert)
+ atomic_add_unless(&ht->nelems, 1, INT_MAX);
+ else
+ atomic_inc(&ht->nelems);
if (rht_grow_above_75(ht, tbl))
schedule_work(&ht->run_work);
@@ -1104,7 +1113,10 @@ static inline int __rhashtable_remove_fast_one(
spin_unlock_bh(lock);
if (err > 0) {
- atomic_dec(&ht->nelems);
+ if (params.never_fail_insert)
+ atomic_add_unless(&ht->nelems, -1, INT_MAX);
+ else
+ atomic_dec(&ht->nelems);
if (unlikely(ht->p.automatic_shrinking &&
rht_shrink_below_30(ht, tbl)))
schedule_work(&ht->run_work);
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index fd6f320b9704..427836aace60 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -424,7 +424,7 @@ static void rht_deferred_worker(struct work_struct *work)
err = rhashtable_rehash_alloc(ht, tbl, tbl->size * 2);
else if (ht->p.automatic_shrinking && rht_shrink_below_30(ht, tbl))
err = rhashtable_shrink(ht);
- else if (tbl->nest)
+ else if (tbl->nest || tbl->long_chain)
err = rhashtable_rehash_alloc(ht, tbl, tbl->size);
if (!err)
@@ -549,14 +549,22 @@ static struct bucket_table *rhashtable_insert_one(struct rhashtable *ht,
if (new_tbl)
return new_tbl;
- if (PTR_ERR(data) != -ENOENT)
- return ERR_CAST(data);
+ if (ht->p.never_fail_insert) {
+ if (PTR_ERR(data) == -EAGAIN &&
+ atomic_read(&ht->nelems) != INT_MAX) {
+ tbl->long_chain = true;
+ schedule_work(&ht->run_work);
+ }
+ } else {
+ if (PTR_ERR(data) != -ENOENT)
+ return ERR_CAST(data);
- if (unlikely(rht_grow_above_max(ht, tbl)))
- return ERR_PTR(-E2BIG);
+ if (unlikely(rht_grow_above_max(ht, tbl)))
+ return ERR_PTR(-E2BIG);
- if (unlikely(rht_grow_above_100(ht, tbl)))
- return ERR_PTR(-EAGAIN);
+ if (unlikely(rht_grow_above_100(ht, tbl)))
+ return ERR_PTR(-EAGAIN);
+ }
pprev = rht_bucket_insert(ht, tbl, hash);
if (!pprev)
@@ -574,7 +582,10 @@ static struct bucket_table *rhashtable_insert_one(struct rhashtable *ht,
rcu_assign_pointer(*pprev, obj);
- atomic_inc(&ht->nelems);
+ if (ht->p.never_fail_insert)
+ atomic_add_unless(&ht->nelems, 1, INT_MAX);
+ else
+ atomic_inc(&ht->nelems);
if (rht_grow_above_75(ht, tbl))
schedule_work(&ht->run_work);
^ permalink raw reply related
* [PATCH 4/6] rhashtable: allow a walk of the hash table without missing objects.
From: NeilBrown @ 2018-03-26 23:33 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <152210688405.11435.13010923693146415942.stgit@noble>
When a walk of the hashtable can be done entirely under RCU,
no objects will be missed - though seeing duplicates is possible.
This is because a cursor is kept in iter->p.
Without the cursor we depend on the ->skip counter. If an object
before the current location in hash chain is removed, the ->skip
counter will be too large and would could miss a later object.
In many cases where the walker needs to drop out of RCU protection,
it will take a reference to the object and this can prevent it from
being removed from the hash table. In those cases, the last-returned
object can still be used as a cursor. rhashtable cannot detect
these cases itself.
This patch adds a new rhashtable_walk_start_continue() interface which
is passed the last object returned. This can be used if the caller
knows that the object is still in the hash table. When it is used,
a walk of the hash table will return every object that was in the
hastable for the duration of the walk, at least once. This can be
used, for example, to selectively delete objects from the table.
Signed-off-by: NeilBrown <neilb@suse.com>
---
include/linux/rhashtable.h | 28 ++++++++++++++++++++++++++--
lib/rhashtable.c | 42 ++++++++++++++++++++++++++----------------
2 files changed, 52 insertions(+), 18 deletions(-)
diff --git a/include/linux/rhashtable.h b/include/linux/rhashtable.h
index 3bd19d29f46b..4ffd96949d4f 100644
--- a/include/linux/rhashtable.h
+++ b/include/linux/rhashtable.h
@@ -387,11 +387,35 @@ void *rhashtable_insert_slow(struct rhashtable *ht, const void *key,
void rhashtable_walk_enter(struct rhashtable *ht,
struct rhashtable_iter *iter);
void rhashtable_walk_exit(struct rhashtable_iter *iter);
-int rhashtable_walk_start_check(struct rhashtable_iter *iter) __acquires(RCU);
+int rhashtable_walk_start_continue(struct rhashtable_iter *iter,
+ struct rhash_head *obj) __acquires(RCU);
+
+/**
+ * rhashtable_walk_start_check - Start a hash table walk
+ * @iter: Hash table iterator
+ *
+ * Start a hash table walk at the current iterator position. Note that we take
+ * the RCU lock in all cases including when we return an error. So you must
+ * always call rhashtable_walk_stop to clean up.
+ *
+ * Returns zero if successful.
+ *
+ * Returns -EAGAIN if resize event occured. Note that the iterator
+ * will rewind back to the beginning and you may use it immediately
+ * by calling rhashtable_walk_next.
+ *
+ * rhashtable_walk_start is defined as an inline variant that returns
+ * void. This is preferred in cases where the caller would ignore
+ * resize events and always continue.
+ */
+static inline int rhashtable_walk_start_check(struct rhashtable_iter *iter)
+{
+ return rhashtable_walk_start_continue(iter, NULL);
+}
static inline void rhashtable_walk_start(struct rhashtable_iter *iter)
{
- (void)rhashtable_walk_start_check(iter);
+ (void)rhashtable_walk_start_continue(iter, NULL);
}
void *rhashtable_walk_next(struct rhashtable_iter *iter);
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 08018198f045..fd6f320b9704 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -702,30 +702,41 @@ void rhashtable_walk_exit(struct rhashtable_iter *iter)
EXPORT_SYMBOL_GPL(rhashtable_walk_exit);
/**
- * rhashtable_walk_start_check - Start a hash table walk
- * @iter: Hash table iterator
+ * rhashtable_walk_start_continue - Restart a hash table walk from last object
+ * @iter: Hask table iterator
+ * @obj: pointer to rhash_head in last object returned.
+ *
+ * Restart a hash table walk, ensuring not to miss any objects. The
+ * previously returned object must still be in the hash table, and must be
+ * provided as an argument.
*
- * Start a hash table walk at the current iterator position. Note that we take
- * the RCU lock in all cases including when we return an error. So you must
- * always call rhashtable_walk_stop to clean up.
+ * When rhashtable_walk_start() or rhashtable_walk_start_check() is used,
+ * a deletion since the previous walk_start can result in objects being missed
+ * as a hash chain might be shorter than expected. This can be avoided by
+ * using the last returned object as a cursor.
*
- * Returns zero if successful.
+ * If the @obj passed is NULL, or not the most recently returned object,
+ * rhashtable_walk_start_continue() will act like rhashtable_walk_start_check();
*
- * Returns -EAGAIN if resize event occured. Note that the iterator
- * will rewind back to the beginning and you may use it immediately
- * by calling rhashtable_walk_next.
+ * Returns -EAGAIN if a resize event was detected. The iterator will
+ * rewind back to the beginning and can be used immediately. Seeing duplicates
+ * is possible but missing objects isn't.
+ * Returns zero if no resize event was detected. This does not guarantee
+ * that no duplicates will be seen.
*
- * rhashtable_walk_start is defined as an inline variant that returns
- * void. This is preferred in cases where the caller would ignore
- * resize events and always continue.
+ * Always takes the RCU read lock, so rhashtable_walk_stop() must always be called
+ * to clean up.
*/
-int rhashtable_walk_start_check(struct rhashtable_iter *iter)
+int rhashtable_walk_start_continue(struct rhashtable_iter *iter, struct rhash_head *obj)
__acquires(RCU)
{
struct rhashtable *ht = iter->ht;
rcu_read_lock();
+ if (!obj || iter->p != obj)
+ iter->p = NULL;
+
spin_lock(&ht->lock);
if (iter->walker.tbl)
list_del(&iter->walker.list);
@@ -733,6 +744,7 @@ int rhashtable_walk_start_check(struct rhashtable_iter *iter)
if (!iter->walker.tbl && !iter->end_of_table) {
iter->walker.tbl = rht_dereference_rcu(ht->tbl, ht);
+ iter->p = NULL;
iter->slot = 0;
iter->skip = 0;
return -EAGAIN;
@@ -740,7 +752,7 @@ int rhashtable_walk_start_check(struct rhashtable_iter *iter)
return 0;
}
-EXPORT_SYMBOL_GPL(rhashtable_walk_start_check);
+EXPORT_SYMBOL_GPL(rhashtable_walk_start_continue);
/**
* __rhashtable_walk_find_next - Find the next element in a table (or the first
@@ -922,8 +934,6 @@ void rhashtable_walk_stop(struct rhashtable_iter *iter)
iter->walker.tbl = NULL;
spin_unlock(&ht->lock);
- iter->p = NULL;
-
out:
rcu_read_unlock();
}
^ permalink raw reply related
* [PATCH 3/6] rhashtable: reset intr when rhashtable_walk_start sees new table
From: NeilBrown @ 2018-03-26 23:33 UTC (permalink / raw)
To: Thomas Graf, Herbert Xu; +Cc: netdev, linux-kernel
In-Reply-To: <152210688405.11435.13010923693146415942.stgit@noble>
The documentation claims that when rhashtable_walk_start_check()
detects a resize event, it will rewind back to the beginning
of the table. This is not true. We need to set ->slot and
->skip to be zero for it to be true.
Signed-off-by: NeilBrown <neilb@suse.com>
---
lib/rhashtable.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 24a57ca494cb..08018198f045 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -733,6 +733,8 @@ int rhashtable_walk_start_check(struct rhashtable_iter *iter)
if (!iter->walker.tbl && !iter->end_of_table) {
iter->walker.tbl = rht_dereference_rcu(ht->tbl, ht);
+ iter->slot = 0;
+ iter->skip = 0;
return -EAGAIN;
}
^ 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