bpf, tracing: introduce bpf raw tracepoints
Subsystems (24)
Alexei Starovoitov
maintainerv8 cover bpf, tracing: introduce bpf raw tracepoints
v7->v8:
- moved 'u32 num_args' from 'struct tracepoint' into 'struct bpf_raw_event_map' that increases memory overhead, but can be optimized/compressed later. Now it's zero changes in tracepoint.[ch]
v6->v7:
- adopted Steven's bpf_raw_tp_map section approach to find tracepoint and corresponding bpf probe function instead of kallsyms approach. dropped kernel_tracepoint_find_by_name() patch
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 6
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 patch 6
- 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 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 6.
Patch 7 - trivial support in libbpf
Patches 8, 9 - 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 (9):
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
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/asm-generic/vmlinux.lds.h | 10 ++ include/linux/bpf_types.h | 1 + include/linux/kernel.h | 7 + include/linux/trace_events.h | 42 +++++ include/linux/tracepoint-defs.h | 6 + include/trace/bpf_probe.h | 92 +++++++++++ include/trace/define_trace.h | 1 + include/trace/events/f2fs.h | 2 +- include/uapi/linux/bpf.h | 11 ++ kernel/bpf/syscall.c | 78 +++++++++ kernel/trace/bpf_trace.c | 183 +++++++++++++++++++++ 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 +++++++--- 30 files changed, 607 insertions(+), 77 deletions(-) create mode 100644 include/trace/bpf_probe.h create mode 100644 samples/bpf/test_overhead_raw_tp_kern.c
Signature · 1 line
2.9.5
Alexei Starovoitov
maintainerPATCH v8 1/9 treewide: remove large struct-pass-by-value from tracepoint arguments
- fix trace_hfi1_ctxt_info() to pass large struct by reference instead of by value
- convert 'type array[]' tracepoint arguments into 'type *array', since compiler will warn that sizeof('type array[]') == sizeof('type *array') and later should be used instead
The CAST_TO_U64 macro in the later patch will enforce that tracepoint arguments can only be integers, pointers, or less than 8 byte structures. Larger structures should be passed by reference.
--- drivers/infiniband/hw/hfi1/file_ops.c | 2 +- drivers/infiniband/hw/hfi1/trace_ctxts.h | 12 ++++++------ include/trace/events/f2fs.h | 2 +- net/wireless/trace.h | 2 +- sound/firewire/amdtp-stream-trace.h | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-)
drivers/infiniband/hw/hfi1/file_ops.c +1 -1
@@ -1153,7 +1153,7 @@ static int get_ctxt_info(struct hfi1_filedata *fd, unsigned long arg, u32 len) cinfo.sdma_ring_size = fd->cq->nentries; cinfo.rcvegr_size = uctxt->egrbufs.rcvtid_size; -trace_hfi1_ctxt_info (uctxt->dd, uctxt->ctxt, fd->subctxt, cinfo); +trace_hfi1_ctxt_info (uctxt->dd, uctxt->ctxt, fd->subctxt, &cinfo);if (copy_to_user ((void __user *)arg, &cinfo, len))return -EFAULT ;
drivers/infiniband/hw/hfi1/trace_ctxts.h +6 -6
@@ -106,7 +106,7 @@ TRACE_EVENT(hfi1_uctxtdata, TRACE_EVENT(hfi1_ctxt_info, TP_PROTO(struct hfi1_devdata *dd, unsigned int ctxt, unsigned int subctxt, - struct hfi1_ctxt_info cinfo), + struct hfi1_ctxt_info *cinfo), TP_ARGS(dd, ctxt, subctxt, cinfo), TP_STRUCT__entry(DD_DEV_ENTRY(dd) __field(unsigned int, ctxt)
@@ -120,11 +120,11 @@ TRACE_EVENT(hfi1_ctxt_info, TP_fast_assign(DD_DEV_ASSIGN(dd); __entry->ctxt = ctxt; __entry->subctxt = subctxt; - __entry->egrtids = cinfo.egrtids; - __entry->rcvhdrq_cnt = cinfo.rcvhdrq_cnt; - __entry->rcvhdrq_size = cinfo.rcvhdrq_entsize; - __entry->sdma_ring_size = cinfo.sdma_ring_size; - __entry->rcvegr_size = cinfo.rcvegr_size; + __entry->egrtids = cinfo->egrtids; + __entry->rcvhdrq_cnt = cinfo->rcvhdrq_cnt; + __entry->rcvhdrq_size = cinfo->rcvhdrq_entsize; + __entry->sdma_ring_size = cinfo->sdma_ring_size; + __entry->rcvegr_size = cinfo->rcvegr_size; ), TP_printk("[%s] ctxt %u:%u " CINFO_FMT, __get_str(dev),
include/trace/events/f2fs.h +1 -1
@@ -491,7 +491,7 @@ DEFINE_EVENT(f2fs__truncate_node, f2fs_truncate_node, TRACE_EVENT(f2fs_truncate_partial_nodes, - TP_PROTO(struct inode *inode, nid_t nid[], int depth, int err), + TP_PROTO(struct inode *inode, nid_t *nid, int depth, int err), TP_ARGS(inode, nid, depth, err),
net/wireless/trace.h +1 -1
@@ -3137,7 +3137,7 @@ TRACE_EVENT(rdev_start_radar_detection, TRACE_EVENT(rdev_set_mcast_rate, TP_PROTO(struct wiphy *wiphy, struct net_device *netdev, - int mcast_rate[NUM_NL80211_BANDS]), + int *mcast_rate), TP_ARGS(wiphy, netdev, mcast_rate), TP_STRUCT__entry( WIPHY_ENTRY
sound/firewire/amdtp-stream-trace.h +1 -1
@@ -14,7 +14,7 @@#include <linux/tracepoint.h> TRACE_EVENT(in_packet, - TP_PROTO(const struct amdtp_stream *s, u32 cycles, u32 cip_header[2], unsigned int payload_length, unsigned int index), + TP_PROTO(const struct amdtp_stream *s, u32 cycles, u32 *cip_header, unsigned int payload_length, unsigned int index), TP_ARGS(s, cycles, cip_header, payload_length, index), TP_STRUCT__entry( __field(unsigned int, second)
Signature · 1 line
2.9.5
Alexei Starovoitov
maintainerPATCH v8 2/9 net/mediatek: disambiguate mt76 vs mt7601u trace events
In reply to Alexei Starovoitovtwo trace events defined with the same name and both unused. They conflict in allyesconfig build. Rename one of them.
--- drivers/net/wireless/mediatek/mt7601u/trace.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/net/wireless/mediatek/mt7601u/trace.h +3 -3
@@ -34,7 +34,7 @@#define REG_PR_FMT "%04x=%08x"#define REG_PR_ARG __entry->reg, __entry->val -DECLARE_EVENT_CLASS(dev_reg_evt, +DECLARE_EVENT_CLASS(dev_reg_evtu, TP_PROTO(struct mt7601u_dev *dev, u32 reg, u32 val), TP_ARGS(dev, reg, val), TP_STRUCT__entry(
@@ -51,12 +51,12 @@ DECLARE_EVENT_CLASS(dev_reg_evt, ) ); -DEFINE_EVENT (dev_reg_evt, reg_read, +DEFINE_EVENT (dev_reg_evtu, reg_read, TP_PROTO(struct mt7601u_dev *dev, u32 reg, u32 val),TP_ARGS (dev, reg, val) ); -DEFINE_EVENT (dev_reg_evt, reg_write, +DEFINE_EVENT (dev_reg_evtu, reg_write, TP_PROTO(struct mt7601u_dev *dev, u32 reg, u32 val),TP_ARGS (dev, reg, val) );
Signature · 1 line
2.9.5
Alexei Starovoitov
maintainerPATCH v8 3/9 net/mac802154: disambiguate mac80215 vs mac802154 trace events
In reply to Alexei Starovoitovtwo trace events defined with the same name and both unused. They conflict in allyesconfig build. Rename one of them.
--- net/mac802154/trace.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
net/mac802154/trace.h +4 -4
@@ -33,7 +33,7 @@/* Tracing for driver callbacks */ -DECLARE_EVENT_CLASS(local_only_evt, +DECLARE_EVENT_CLASS(local_only_evt4, TP_PROTO(struct ieee802154_local *local), TP_ARGS(local), TP_STRUCT__entry(
@@ -45,7 +45,7 @@ DECLARE_EVENT_CLASS(local_only_evt,TP_printk (LOCAL_PR_FMT ,LOCAL_PR_ARG ) ); -DEFINE_EVENT(local_only_evt, 802154_drv_return_void, +DEFINE_EVENT(local_only_evt4, 802154_drv_return_void, TP_PROTO(struct ieee802154_local *local),TP_ARGS (local) );
@@ -65,12 +65,12 @@ TRACE_EVENT(802154_drv_return_int, __entry->ret) ); -DEFINE_EVENT(local_only_evt, 802154_drv_start, +DEFINE_EVENT(local_only_evt4, 802154_drv_start, TP_PROTO(struct ieee802154_local *local),TP_ARGS (local) ); -DEFINE_EVENT(local_only_evt, 802154_drv_stop, +DEFINE_EVENT(local_only_evt4, 802154_drv_stop, TP_PROTO(struct ieee802154_local *local),TP_ARGS (local) );
Signature · 1 line
2.9.5
Alexei Starovoitov
maintainerPATCH v8 4/9 net/wireless/iwlwifi: fix iwlwifi_dev_ucode_error tracepoint
In reply to Alexei Starovoitovfix 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.
--- 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(-)
drivers/net/wireless/intel/iwlwifi/dvm/main.c +1 -6
@@ -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);
drivers/net/wireless/intel/iwlwifi/iwl-devtrace-iwlwifi.h +18 -21
@@ -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; ),
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c +1 -0
@@ -30,6 +30,7 @@#ifndef __CHECKER__#include "iwl-trans.h" +#include "dvm/commands.h" #define CREATE_TRACE_POINTS #include "iwl-devtrace.h"
drivers/net/wireless/intel/iwlwifi/mvm/utils.c +1 -6
@@ -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);
Signature · 1 line
2.9.5
Alexei Starovoitov
maintainerPATCH v8 5/9 macro: introduce COUNT_ARGS() macro
In reply to Alexei Starovoitovmove 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.
--- include/linux/kernel.h | 7 +++++++ security/apparmor/include/path.h | 7 +------ 2 files changed, 8 insertions(+), 6 deletions(-)
include/linux/kernel.h +7 -0
@@ -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.
security/apparmor/include/path.h +1 -6
@@ -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)++)
Signature · 1 line
2.9.5
Alexei Starovoitov
maintainerPATCH v8 6/9 bpf: introduce BPF_RAW_TRACEPOINT
In reply to Alexei StarovoitovIntroduce BPF_PROG_TYPE_RAW_TRACEPOINT bpf program type to access kernel internal arguments of the tracepoints in their raw form.
Quoted 1 line
From bpf program point of view the access to the arguments look like:
struct bpf_raw_tracepoint_args {
__u64 args[0];
};int bpf_prog(struct bpf_raw_tracepoint_args *ctx) {
// program can read args[N] where N depends on tracepoint // and statically verified at program load+attach time
}
kprobe+bpf infrastructure allows programs access function arguments. This feature allows programs access raw tracepoint arguments.
Similar to proposed 'dynamic ftrace events' there are no abi guarantees to what the tracepoints arguments are and what their meaning is. The program needs to type cast args properly and use bpf_probe_read() helper to access struct fields when argument is a pointer.
For every tracepoint __bpf_trace_##call function is prepared. In assembler it looks like:
(gdb) disassemble __bpf_trace_xdp_exception
Dump of assembler code for function __bpf_trace_xdp_exception:
0xffffffff81132080 <+0>: mov %ecx,%ecx 0xffffffff81132082 <+2>: jmpq 0xffffffff811231f0 <bpf_trace_run3>
where
TRACE_EVENT(xdp_exception,
TP_PROTO(const struct net_device *dev,
const struct bpf_prog *xdp, u32 act),
The above assembler snippet is casting 32-bit 'act' field into 'u64' to pass into bpf_trace_run3(), while 'dev' and 'xdp' args are passed as-is. All of ~500 of __bpf_trace_*() functions are only 5-10 byte long and in total this approach adds 7k bytes to .text.
This approach gives the lowest possible overhead
while calling trace_xdp_exception() from kernel C code and transitioning into bpf land.
Since tracepoint+bpf are used at speeds of 1M+ events per second this is valuable optimization.
The new BPF_RAW_TRACEPOINT_OPEN sys_bpf command is introduced that returns anon_inode FD of 'bpf-raw-tracepoint' object.
The user space 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 with prog attached raw_tp_fd = bpf_raw_tracepoint_open("xdp_exception", prog_fd);
Ctrl-C of tracing daemon or cmdline tool that uses this feature will automatically detach bpf program, unload it and unregister tracepoint probe.
On the kernel side the __bpf_raw_tp_map section of pointers to tracepoint definition and to __bpf_trace_*() probe function is used to find a tracepoint with "xdp_exception" name and corresponding __bpf_trace_xdp_exception() probe function which are passed to tracepoint_probe_register() to connect probe with tracepoint.
Addition of bpf_raw_tracepoint doesn't interfere with ftrace and perf tracepoint mechanisms. perf_event_open() can be used in parallel on the same tracepoint.
Multiple bpf_raw_tracepoint_open("xdp_exception", prog_fd) are permitted. Each with its own bpf program. The kernel will execute
all tracepoint probes and all attached bpf programs.
In the future bpf_raw_tracepoints can be extended with query/introspection logic.
__bpf_raw_tp_map section logic was contributed by Steven Rostedt
--- include/asm-generic/vmlinux.lds.h | 10 +++ include/linux/bpf_types.h | 1 + include/linux/trace_events.h | 42 +++++++++ include/linux/tracepoint-defs.h | 6 ++ include/trace/bpf_probe.h | 92 +++++++++++++++++++ include/trace/define_trace.h | 1 + include/uapi/linux/bpf.h | 11 +++ kernel/bpf/syscall.c | 78 ++++++++++++++++ kernel/trace/bpf_trace.c | 183 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 424 insertions(+) create mode 100644 include/trace/bpf_probe.h
include/asm-generic/vmlinux.lds.h +10 -0
@@ -178,6 +178,15 @@#define TRACE_SYSCALLS ()#endif +#ifdef CONFIG_BPF_EVENTS +#define BPF_RAW_TP () STRUCT_ALIGN(); \ + VMLINUX_SYMBOL(__start__bpf_raw_tp) = .; \ + KEEP(*(__bpf_raw_tp_map)) \ + VMLINUX_SYMBOL(__stop__bpf_raw_tp) = .; +#else +#define BPF_RAW_TP () +#endif +#ifdef CONFIG_SERIAL_EARLYCON #define EARLYCON_TABLE () STRUCT_ALIGN(); \ VMLINUX_SYMBOL(__earlycon_table) = .; \
@@ -249,6 +258,7 @@ LIKELY_PROFILE() \ BRANCH_PROFILE() \ TRACE_PRINTKS() \ + BPF_RAW_TP() \TRACEPOINT_STR ()/*
include/linux/bpf_types.h +1 -0
@@ -19,6 +19,7 @@ BPF_PROG_TYPE(BPF_PROG_TYPE_SK_MSG, sk_msg) BPF_PROG_TYPE(BPF_PROG_TYPE_KPROBE, kprobe) BPF_PROG_TYPE(BPF_PROG_TYPE_TRACEPOINT, tracepoint) BPF_PROG_TYPE(BPF_PROG_TYPE_PERF_EVENT, perf_event) +BPF_PROG_TYPE(BPF_PROG_TYPE_RAW_TRACEPOINT, raw_tracepoint)#endif #ifdef CONFIG_CGROUP_BPF BPF_PROG_TYPE (BPF_PROG_TYPE_CGROUP_DEVICE , cg_dev)
include/linux/trace_events.h +42 -0
60 lines not on this page · raw message
include/linux/tracepoint-defs.h +6 -0
@@ -35,4 +35,10 @@ struct tracepoint { struct tracepoint_func __rcu *funcs; }; +struct bpf_raw_event_map { +struct tracepoint *tp; +void *bpf_func; +u32 num_args; +}__aligned(32) ; +#endif
include/trace/bpf_probe.h new file +92 -0
92 lines not on this page · raw message
include/trace/define_trace.h +1 -0
@@ -95,6 +95,7 @@#ifdef TRACEPOINTS_ENABLED #include <trace/trace_events.h> #include <trace/perf.h> +#include <trace/bpf_probe.h> #endif #undef TRACE_EVENT
include/uapi/linux/bpf.h +11 -0
@@ -94,6 +94,7 @@ enum bpf_cmd {BPF_MAP_GET_FD_BY_ID ,BPF_OBJ_GET_INFO_BY_FD ,BPF_PROG_QUERY , + BPF_RAW_TRACEPOINT_OPEN, }; enum bpf_map_type {
@@ -134,6 +135,7 @@ enum bpf_prog_type {BPF_PROG_TYPE_SK_SKB ,BPF_PROG_TYPE_CGROUP_DEVICE ,BPF_PROG_TYPE_SK_MSG , + BPF_PROG_TYPE_RAW_TRACEPOINT, }; enum bpf_attach_type {
@@ -344,6 +346,11 @@ union bpf_attr {__aligned_u64 prog_ids;__u32 prog_cnt; } query; + +struct { +__u64 name; +__u32 prog_fd; + } raw_tracepoint; } __attribute__((aligned(8)));/* BPF helper function descriptions:
@@ -1152,4 +1159,8 @@ struct bpf_cgroup_dev_ctx { __u32 minor; }; +struct bpf_raw_tracepoint_args { +__u64 args[0 ]; +}; +#endif /* _UAPI__LINUX_BPF_H__ */
kernel/bpf/syscall.c +78 -0
90 lines not on this page · raw message
kernel/trace/bpf_trace.c +183 -0
192 lines not on this page · raw message
Signature · 1 line
2.9.5
Alexei Starovoitov
maintainerPATCH v8 7/9 libbpf: add bpf_raw_tracepoint_open helper
In reply to Alexei Starovoitovadd bpf_raw_tracepoint_open(const char *name, int prog_fd) api to libbpf
--- tools/include/uapi/linux/bpf.h | 11 +++++++++++ tools/lib/bpf/bpf.c | 11 +++++++++++ tools/lib/bpf/bpf.h | 1 + 3 files changed, 23 insertions(+)
tools/include/uapi/linux/bpf.h +11 -0
@@ -94,6 +94,7 @@ enum bpf_cmd {BPF_MAP_GET_FD_BY_ID ,BPF_OBJ_GET_INFO_BY_FD ,BPF_PROG_QUERY , + BPF_RAW_TRACEPOINT_OPEN, }; enum bpf_map_type {
@@ -134,6 +135,7 @@ enum bpf_prog_type {BPF_PROG_TYPE_SK_SKB ,BPF_PROG_TYPE_CGROUP_DEVICE ,BPF_PROG_TYPE_SK_MSG , + BPF_PROG_TYPE_RAW_TRACEPOINT, }; enum bpf_attach_type {
@@ -344,6 +346,11 @@ union bpf_attr {__aligned_u64 prog_ids;__u32 prog_cnt; } query; + +struct { +__u64 name; +__u32 prog_fd; + } raw_tracepoint; } __attribute__((aligned(8)));/* BPF helper function descriptions:
@@ -1151,4 +1158,8 @@ struct bpf_cgroup_dev_ctx { __u32 minor; }; +struct bpf_raw_tracepoint_args { +__u64 args[0 ]; +}; +#endif /* _UAPI__LINUX_BPF_H__ */
tools/lib/bpf/bpf.c +11 -0
@@ -428,6 +428,17 @@ int bpf_obj_get_info_by_fd(int prog_fd, void *info, __u32 *info_len)return err; } +int bpf_raw_tracepoint_open (const char *name,int prog_fd) +{ +union bpf_attr attr; + +bzero (&attr,sizeof (attr)); + attr.raw_tracepoint.name =ptr_to_u64 (name); + attr.raw_tracepoint.prog_fd = prog_fd; + +return sys_bpf (BPF_RAW_TRACEPOINT_OPEN , &attr,sizeof (attr)); +} +int bpf_set_link_xdp_fd (int ifindex,int fd,__u32 flags) {struct sockaddr_nl sa;
tools/lib/bpf/bpf.h +1 -0
@@ -79,4 +79,5 @@ int bpf_map_get_fd_by_id(__u32 id);int bpf_obj_get_info_by_fd (int prog_fd,void *info,__u32 *info_len);int bpf_prog_query (int target_fd,enum bpf_attach_type type,__u32 query_flags,__u32 *attach_flags,__u32 *prog_ids,__u32 *prog_cnt); +int bpf_raw_tracepoint_open (const char *name,int prog_fd);#endif
Signature · 1 line
2.9.5
Alexei Starovoitov
maintainerPATCH v8 8/9 samples/bpf: raw tracepoint test
In reply to Alexei Starovoitovadd empty raw_tracepoint bpf program to test overhead similar to kprobe and traditional tracepoint tests
--- 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 ++++++++++++ 4 files changed, 44 insertions(+) create mode 100644 samples/bpf/test_overhead_raw_tp_kern.c
samples/bpf/Makefile +1 -0
@@ -119,6 +119,7 @@ always += offwaketime_kern.o always += spintest_kern.o always += map_perf_test_kern.o always += test_overhead_tp_kern.o +always += test_overhead_raw_tp_kern.o always += test_overhead_kprobe_kern.o always += parse_varlen.o parse_simple.o parse_ldabs.o always += test_cgrp2_tc_kern.o
samples/bpf/bpf_load.c +14 -0
@@ -61,6 +61,7 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)bool is_kprobe =strncmp (event,"kprobe/" ,7 ) ==0 ;bool is_kretprobe =strncmp (event,"kretprobe/" ,10 ) ==0 ;bool is_tracepoint =strncmp (event,"tracepoint/" ,11 ) ==0 ; +bool is_raw_tracepoint =strncmp (event,"raw_tracepoint/" ,15 ) ==0 ;bool is_xdp =strncmp (event,"xdp" ,3 ) ==0 ;bool is_perf_event =strncmp (event,"perf_event" ,10 ) ==0 ;bool is_cgroup_skb =strncmp (event,"cgroup/skb" ,10 ) ==0 ;
@@ -85,6 +86,8 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size) prog_type =BPF_PROG_TYPE_KPROBE ; }else if (is_tracepoint) { prog_type =BPF_PROG_TYPE_TRACEPOINT ; + }else if (is_raw_tracepoint) { + prog_type =BPF_PROG_TYPE_RAW_TRACEPOINT ; }else if (is_xdp) { prog_type =BPF_PROG_TYPE_XDP ; }else if (is_perf_event) {
@@ -131,6 +134,16 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)return populate_prog_array (event, fd); } +if (is_raw_tracepoint) { + efd =bpf_raw_tracepoint_open (event +15 , fd); +if (efd <0 ) { +printf ("tracepoint %s %s\n" , event +15 ,strerror (errno)); +return -1 ; + } + event_fd[prog_cnt -1 ] = efd; +return 0 ; + } +if (is_kprobe || is_kretprobe) {if (is_kprobe) event +=7 ;
@@ -587,6 +600,7 @@ static int do_load_bpf_file(const char *path, fixup_map_cb fixup_map) if (memcmp(shname, "kprobe/", 7) == 0 || memcmp(shname, "kretprobe/", 10) == 0 || memcmp(shname, "tracepoint/", 11) == 0 || + memcmp(shname, "raw_tracepoint/", 15) == 0 || memcmp(shname, "xdp", 3) == 0 || memcmp(shname, "perf_event", 10) == 0 || memcmp(shname, "socket", 6) == 0 ||
samples/bpf/test_overhead_raw_tp_kern.c new file +17 -0
@@ -0,0 +1,17 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (c) 2018 Facebook */ +#include <uapi/linux/bpf.h> +#include "bpf_helpers.h" + +SEC("raw_tracepoint/task_rename") +int prog (struct bpf_raw_tracepoint_args *ctx) +{ +return 0 ; +} + +SEC("raw_tracepoint/urandom_read") +int prog2 (struct bpf_raw_tracepoint_args *ctx) +{ +return 0 ; +} +char _license[] SEC("license") = "GPL";
samples/bpf/test_overhead_user.c +12 -0
@@ -158,5 +158,17 @@ int main(int argc, char **argv)unload_progs (); } +if (test_flags &0xC0 ) { +snprintf (filename,sizeof (filename), +"%s_raw_tp_kern.o" , argv[0 ]); +if (load_bpf_file (filename)) { +printf ("%s" , bpf_log_buf); +return 1 ; + } +printf ("w/RAW_TRACEPOINT\n" ); +run_perf_test (num_cpu, test_flags >>6 ); +unload_progs (); + } +return 0 ; }
Signature · 1 line
2.9.5
Alexei Starovoitov
maintainerPATCH v8 9/9 selftests/bpf: test for bpf_get_stackid() from raw tracepoints
In reply to Alexei Starovoitovsimilar to traditional traceopint test add bpf_get_stackid() test from raw tracepoints
and reduce verbosity of existing stackmap test
--- tools/testing/selftests/bpf/test_progs.c | 91 ++++++++++++++++++++++++-------- 1 file changed, 70 insertions(+), 21 deletions(-)
tools/testing/selftests/bpf/test_progs.c +70 -21
@@ -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 ;
Signature · 1 line
2.9.5
Steven Rostedt
maintainerRe: [PATCH v8 bpf-next 6/9] bpf: introduce BPF_RAW_TRACEPOINT
In reply to Alexei Starovoitovinclude/linux/tracepoint-defs.h struct tracepoint { · line 43
Quoted 11 lines · On Wed, 28 Mar 2018 12:05:37 -0700 Alexei Starovoitov <[email protected]> wrote:
+++
b/include/linux/tracepoint-defs.hinclude/linux/tracepoint-defs.h +6
-0@@ -35,4 +35,10 @@ struct tracepoint {struct tracepoint_func __rcu *funcs; }; +struct bpf_raw_event_map { +struct tracepoint *tp; +void *bpf_func; +u32 num_args; +}__aligned(32) ; +
If you prefer v7, I'm fine with that. For cache issues, I can pull out the funcs from the tracepoint structure like I posted.
Mathieu, your thoughts?
-- Steve
Re: [PATCH v8 bpf-next 6/9] bpf: introduce BPF_RAW_TRACEPOINT
Quoted 8 lines · Steven Rostedt
Quoted 5 lines · On Wed, 28 Mar 2018 12:05:37 -0700 Alexei Starovoitov <[email protected]> wrote:
+++
b/include/linux/tracepoint-defs.hinclude/linux/tracepoint-defs.h +0
-0@@ -35,4 +35,10 @@ struct tracepoint {struct tracepoint_func __rcu *funcs; };
Quoted 9 lines
include/linux/tracepoint-defs.h struct tracepoint { · line 43
Quoted 6 lines
include/linux/tracepoint-defs.h +6
-0+struct bpf_raw_event_map { +struct tracepoint *tp; +void *bpf_func; +u32 num_args; +}__aligned(32) ; +If you prefer v7, I'm fine with that. For cache issues, I can pull out the funcs from the tracepoint structure like I posted.
I very much prefer to land this v8 as-is and optimize later.
I still have bpfilter/microkernel patches to finish which were practically ready two weeks ago and got delayed but this set.
Re: [PATCH v8 bpf-next 6/9] bpf: introduce BPF_RAW_TRACEPOINT
Quoted 19 lines · On Wed, 28 Mar 2018 12:38:48 -0700 Alexei Starovoitov <[email protected]> wrote:
Quoted 8 lines · Steven Rostedt
Quoted 5 lines · On Wed, 28 Mar 2018 12:05:37 -0700 Alexei Starovoitov <[email protected]> wrote:
+++
b/include/linux/tracepoint-defs.hinclude/linux/tracepoint-defs.h +0
-0@@ -35,4 +35,10 @@ struct tracepoint {struct tracepoint_func __rcu *funcs; };Quoted 9 lines
include/linux/tracepoint-defs.h struct tracepoint { · line 43
Quoted 6 lines
include/linux/tracepoint-defs.h +6
-0+struct bpf_raw_event_map { +struct tracepoint *tp; +void *bpf_func; +u32 num_args; +}__aligned(32) ; +If you prefer v7, I'm fine with that. For cache issues, I can pull out the funcs from the tracepoint structure like I posted.
Quoted 4 lines
I very much prefer to land this v8 as-is and optimize later.
I still have bpfilter/microkernel patches to finish which were practically ready two weeks ago and got delayed but this set.
Then by all means, you have my Ack. We can optimize later.
For the whole series... (v7 or v8)
Thanks!
-- Steve
Mathieu Desnoyers
reviewerRe: [PATCH v8 bpf-next 6/9] bpf: introduce BPF_RAW_TRACEPOINT
Quoted 24 lines · ----- On Mar 28, 2018, at 3:40 PM, rostedt [email protected] wrote:
Quoted 21 lines · On Wed, 28 Mar 2018 12:38:48 -0700 Alexei Starovoitov <[email protected]> wrote:
Quoted 8 lines · Steven Rostedt
Quoted 5 lines · On Wed, 28 Mar 2018 12:05:37 -0700 Alexei Starovoitov <[email protected]> wrote:
+++
b/include/linux/tracepoint-defs.hinclude/linux/tracepoint-defs.h +0
-0@@ -35,4 +35,10 @@ struct tracepoint {struct tracepoint_func __rcu *funcs; };Quoted 9 lines
include/linux/tracepoint-defs.h struct tracepoint { · line 43
Quoted 6 lines
include/linux/tracepoint-defs.h +6
-0+struct bpf_raw_event_map { +struct tracepoint *tp; +void *bpf_func; +u32 num_args; +}__aligned(32) ; +If you prefer v7, I'm fine with that. For cache issues, I can pull out the funcs from the tracepoint structure like I posted.
I very much prefer to land this v8 as-is and optimize later.
Quoted 8 lines
Quoted 2 lines
I still have bpfilter/microkernel patches to finish which were practically ready two weeks ago and got delayed but this set.
Then by all means, you have my Ack. We can optimize later.
For the whole series... (v7 or v8)
I'm fine with either v7 or v8 as well.
Thanks,
Mathieu
Quoted 4 lines
Thanks!
-- Steve
Signature · 3 lines
Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com
Quoted 4 lines · Alexei Starovoitov
v7->v8:
- moved 'u32num_args' from 'struct tracepoint' into 'structbpf_raw_event_map' that increases memory overhead, but can be optimized/compressed later. Now it's zero changes in tracepoint.[ch]
[...]
Quoted 31 lines
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 specifictracepoint_probe_register()probe functions. Details here:
https://lkml.org/lkml/2017/12/20/929Patch 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_errorfrom 17 to 4 argsPatch 5 introduces
COUNT_ARGSmacroPatch 6 introduces
BPF_RAW_TRACEPOINTapi.
the auto-cleanup and multiple concurrent users are must have features of tracing api. For bpf raw tracepoints it looks like:
// load bpf prog withBPF_PROG_TYPE_RAW_TRACEPOINTtypeprog_fd=bpf_prog_load(...);// receive
anon_inodefd for givenbpf_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 6.
Quoted 8 lines
Patch 7 - trivial support in libbpf
Patches 8, 9 - user space tests
samples/bpf/test_overheadperformance 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
Applied to bpf-next, thanks everyone!
Re: [v8, bpf-next, 4/9] net/wireless/iwlwifi: fix iwlwifi_dev_ucode_error tracepoint
In reply to a message not in the archiveQuoted 7 lines · Alexei Starovoitov
fix
iwlwifi_dev_ucode_errortracepoint to pass pointer to a table instead of all 17 arguments by value.dvm/main.candmvm/utils.chave 'structiwl_error_event_table' defined with very similar yet subtly different fields and offsets. tracepoint is still common and using definition of 'structiwl_error_event_table' fromdvm/commands.hwhile copying fields.
Long term this tracepoint probably should be split into two.
It would've been nice to CC the wireless list for wireless related patches ...
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c line 33
Quoted 7 lines
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c +1
-0@@ -30,6 +30,7 @@#ifndef __CHECKER__#include "iwl-trans.h" +#include "dvm/commands.h"
In particular, this breaks the whole driver abstraction.
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c static void iwl_mvm_dump_lmac_error_log(struct iwl_mvm *mvm, u32 base) · line 552
Quoted 12 lines
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c +2
-6+++ 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);
This is also utterly wrong because mvm has - for better or worse - a different type "struct iwl_error_event_table" in this file ...
This really should never have gotten into the tree.
johannes
Re: [v8, bpf-next, 4/9] net/wireless/iwlwifi: fix iwlwifi_dev_ucode_error tracepoint
In reply to a message not in the archiveQuoted 11 lines · Johannes Berg
Quoted 7 lines · Alexei Starovoitov
fix
iwlwifi_dev_ucode_errortracepoint to pass pointer to a table instead of all 17 arguments by value.dvm/main.candmvm/utils.chave 'structiwl_error_event_table' defined with very similar yet subtly different fields and offsets. tracepoint is still common and using definition of 'structiwl_error_event_table' fromdvm/commands.hwhile copying fields.
Long term this tracepoint probably should be split into two.It would've been nice to CC the wireless list for wireless related patches ...
Ohh. I didn't realize that networking wireless doesn't fall under netdev. I thought wireless folks are silent because they are embarrassed by a function with 17 arguments.
Quoted 15 lines
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c line 33
Quoted 7 lines
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c +1
-0@@ -30,6 +30,7 @@#ifndef __CHECKER__#include "iwl-trans.h" +#include "dvm/commands.h" In particular, this breaks the whole driver abstraction.
Quoted 5 lines
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c +1
-0+++ 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);
Quoted 10 lines
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c static void iwl_mvm_dump_lmac_error_log(struct iwl_mvm *mvm, u32 base) · line 552
Quoted 7 lines
drivers/net/wireless/intel/iwlwifi/iwl-devtrace.c +1
-6-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);This is also utterly wrong because mvm has - for better or worse - a different type "struct
iwl_error_event_table" in this file ...
As I was trying to explain in the commit log the single struct is used in both places, but differences in two
"struct iwl_error_event_table" are carefully matched field and by field. For two extra fields it was not possible and they are passed separately as you can see above. I still believe that tracepoint output is still exactly the same before and after the patch.
I guess you see the breakage because new fields got added into one "struct iwl_error_event_table",
but were not added to its evil twin "struct iwl_error_event_table" with the same name after the patch landed ?
imo wireless folks need to avoid such naming conflicts. I suggest to isolate common fields into separate base struct and give two children structs different names.
Steven Rostedt
maintainerRe: [v8, bpf-next, 4/9] net/wireless/iwlwifi: fix iwlwifi_dev_ucode_error tracepoint
Quoted 3 lines · On Thu, 24 May 2018 16:28:39 -0700 Alexei Starovoitov…
Ohh. I didn't realize that networking wireless doesn't fall under netdev. I thought wireless folks are silent because they are embarrassed by a function with 17 arguments.
Please lets refrain from the demeaning comments.
I agree with your argument, but not the tone.
-- Steve
Re: [v8, bpf-next, 4/9] net/wireless/iwlwifi: fix iwlwifi_dev_ucode_error tracepoint
In reply to Alexei StarovoitovQuoted 10 lines · Alexei Starovoitov
Quoted 9 lines · Johannes Berg
Quoted 7 lines · Alexei Starovoitov
fix
iwlwifi_dev_ucode_errortracepoint to pass pointer to a table instead of all 17 arguments by value.dvm/main.candmvm/utils.chave 'structiwl_error_event_table' defined with very similar yet subtly different fields and offsets. tracepoint is still common and using definition of 'structiwl_error_event_table' fromdvm/commands.hwhile copying fields.
Long term this tracepoint probably should be split into two.
Quoted 6 lines
Quoted 2 lines
It would've been nice to CC the wireless list for wireless related patches ...
Ohh. I didn't realize that networking wireless doesn't fall under netdev. I thought wireless folks are silent because they are embarrassed by a function with 17 arguments.
Really, this is the level of discussion now? You don't even bother to CC the driver maintainers and your first reaction is that they are just too embarrassed to answer? Oh man...
But if you continue doing these kind of "drive-by cleanups" to a subsystem you are not familiar with, at least don't assume anything and instead use get_maintainer script to find the maintainers so that the patches get reviewed and applied to the correct tree:
$ scripts/get_maintainer.pl --no-rolestats --no-git -f drivers/net/wireless/intel/iwlwifi/dvm/main.c Johannes Berg <[email protected]> Emmanuel Grumbach <[email protected]> Luca Coelho <[email protected]> Intel Linux Wireless <[email protected]> Kalle Valo <[email protected]> "David S. Miller" <[email protected]> Daniel Borkmann <[email protected]> Alexei Starovoitov <[email protected]> Kees Cook <[email protected]> [email protected] [email protected] [email protected]
Signature · 1 line
Kalle Valo