[PATCH] samples/bpf: make xdp_monitor use raw_tracepoints

[PATCH] samples/bpf: make xdp_monitor use raw_tracepoints

[Eric Sage]

raw_tracepoints are an eBPF alternative to standard tracepoints which
attach to a tracepoint without the perf layer being executed, making
them faster.

Since xdp_monitor is supposed to have as little impact on the system as
possible it is switched to using them by append raw_ to the SEC names.

There was also a small issues with 'samples/bpf/bpf_load' - it was
loading the raw_tracepoints with the tracing subsystem name still
attached, which the bpf syscall rejects with a No such file or directory
error. This is now fixed.

Signed-off-by: Eric Sage <eric@sage.org>
---
 samples/bpf/bpf_load.c         |  5 +++--
 samples/bpf/xdp_monitor_kern.c | 26 +++++++++++++-------------
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
index 4574b1939e49..6f57eee8e913 100644
--- a/samples/bpf/bpf_load.c
+++ b/samples/bpf/bpf_load.c
@@ -156,9 +156,10 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
 	}
 
 	if (is_raw_tracepoint) {
-		efd = bpf_raw_tracepoint_open(event + 15, fd);
+		efd = bpf_raw_tracepoint_open(event + 19, fd);
 		if (efd < 0) {
-			printf("tracepoint %s %s\n", event + 15, strerror(errno));
+			printf("tracepoint %s %s\n", event + 19,
+						strerror(errno));
 			return -1;
 		}
 		event_fd[prog_cnt - 1] = efd;
diff --git a/samples/bpf/xdp_monitor_kern.c b/samples/bpf/xdp_monitor_kern.c
index ad10fe700d7d..6f67c38468b9 100644
--- a/samples/bpf/xdp_monitor_kern.c
+++ b/samples/bpf/xdp_monitor_kern.c
@@ -23,10 +23,10 @@ struct bpf_map_def SEC("maps") exception_cnt = {
 };
 
 /* Tracepoint format: /sys/kernel/debug/tracing/events/xdp/xdp_redirect/format
+ * Notice: For raw_tracepoints first 8 bytes are not part of 'format' struct
  * Code in:                kernel/include/trace/events/xdp.h
  */
 struct xdp_redirect_ctx {
-	u64 __pad;		// First 8 bytes are not accessible by bpf code
 	int prog_id;		//	offset:8;  size:4; signed:1;
 	u32 act;		//	offset:12  size:4; signed:0;
 	int ifindex;		//	offset:16  size:4; signed:1;
@@ -65,44 +65,44 @@ int xdp_redirect_collect_stat(struct xdp_redirect_ctx *ctx)
 	 */
 }
 
-SEC("tracepoint/xdp/xdp_redirect_err")
+SEC("raw_tracepoint/xdp/xdp_redirect_err")
 int trace_xdp_redirect_err(struct xdp_redirect_ctx *ctx)
 {
 	return xdp_redirect_collect_stat(ctx);
 }
 
 
-SEC("tracepoint/xdp/xdp_redirect_map_err")
+SEC("raw_tracepoint/xdp/xdp_redirect_map_err")
 int trace_xdp_redirect_map_err(struct xdp_redirect_ctx *ctx)
 {
 	return xdp_redirect_collect_stat(ctx);
 }
 
 /* Likely unloaded when prog starts */
-SEC("tracepoint/xdp/xdp_redirect")
+SEC("raw_tracepoint/xdp/xdp_redirect")
 int trace_xdp_redirect(struct xdp_redirect_ctx *ctx)
 {
 	return xdp_redirect_collect_stat(ctx);
 }
 
 /* Likely unloaded when prog starts */
-SEC("tracepoint/xdp/xdp_redirect_map")
+SEC("raw_tracepoint/xdp/xdp_redirect_map")
 int trace_xdp_redirect_map(struct xdp_redirect_ctx *ctx)
 {
 	return xdp_redirect_collect_stat(ctx);
 }
 
 /* Tracepoint format: /sys/kernel/debug/tracing/events/xdp/xdp_exception/format
+ * Notice: For raw_tracepoints first 8 bytes are not part of 'format' struct
  * Code in:                kernel/include/trace/events/xdp.h
  */
 struct xdp_exception_ctx {
-	u64 __pad;	// First 8 bytes are not accessible by bpf code
 	int prog_id;	//	offset:8;  size:4; signed:1;
 	u32 act;	//	offset:12; size:4; signed:0;
 	int ifindex;	//	offset:16; size:4; signed:1;
 };
 
-SEC("tracepoint/xdp/xdp_exception")
+SEC("raw_tracepoint/xdp/xdp_exception")
 int trace_xdp_exception(struct xdp_exception_ctx *ctx)
 {
 	u64 *cnt;
@@ -144,10 +144,10 @@ struct bpf_map_def SEC("maps") cpumap_kthread_cnt = {
 };
 
 /* Tracepoint: /sys/kernel/debug/tracing/events/xdp/xdp_cpumap_enqueue/format
+ * Notice: For raw_tracepoints first 8 bytes are not part of 'format' struct
  * Code in:         kernel/include/trace/events/xdp.h
  */
 struct cpumap_enqueue_ctx {
-	u64 __pad;		// First 8 bytes are not accessible by bpf code
 	int map_id;		//	offset:8;  size:4; signed:1;
 	u32 act;		//	offset:12; size:4; signed:0;
 	int cpu;		//	offset:16; size:4; signed:1;
@@ -156,7 +156,7 @@ struct cpumap_enqueue_ctx {
 	int to_cpu;		//	offset:28; size:4; signed:1;
 };
 
-SEC("tracepoint/xdp/xdp_cpumap_enqueue")
+SEC("raw_tracepoint/xdp/xdp_cpumap_enqueue")
 int trace_xdp_cpumap_enqueue(struct cpumap_enqueue_ctx *ctx)
 {
 	u32 to_cpu = ctx->to_cpu;
@@ -179,10 +179,10 @@ int trace_xdp_cpumap_enqueue(struct cpumap_enqueue_ctx *ctx)
 }
 
 /* Tracepoint: /sys/kernel/debug/tracing/events/xdp/xdp_cpumap_kthread/format
+ * Notice: For raw_tracepoints first 8 bytes are not part of 'format' struct
  * Code in:         kernel/include/trace/events/xdp.h
  */
 struct cpumap_kthread_ctx {
-	u64 __pad;		// First 8 bytes are not accessible by bpf code
 	int map_id;		//	offset:8;  size:4; signed:1;
 	u32 act;		//	offset:12; size:4; signed:0;
 	int cpu;		//	offset:16; size:4; signed:1;
@@ -191,7 +191,7 @@ struct cpumap_kthread_ctx {
 	int sched;		//	offset:28; size:4; signed:1;
 };
 
-SEC("tracepoint/xdp/xdp_cpumap_kthread")
+SEC("raw_tracepoint/xdp/xdp_cpumap_kthread")
 int trace_xdp_cpumap_kthread(struct cpumap_kthread_ctx *ctx)
 {
 	struct datarec *rec;
@@ -218,10 +218,10 @@ struct bpf_map_def SEC("maps") devmap_xmit_cnt = {
 };
 
 /* Tracepoint: /sys/kernel/debug/tracing/events/xdp/xdp_devmap_xmit/format
+ * Notice: For raw_tracepoints first 8 bytes are not part of 'format' struct
  * Code in:         kernel/include/trace/events/xdp.h
  */
 struct devmap_xmit_ctx {
-	u64 __pad;		// First 8 bytes are not accessible by bpf code
 	int map_id;		//	offset:8;  size:4; signed:1;
 	u32 act;		//	offset:12; size:4; signed:0;
 	u32 map_index;		//	offset:16; size:4; signed:0;
@@ -232,7 +232,7 @@ struct devmap_xmit_ctx {
 	int err;		//	offset:36; size:4; signed:1;
 };
 
-SEC("tracepoint/xdp/xdp_devmap_xmit")
+SEC("raw_tracepoint/xdp/xdp_devmap_xmit")
 int trace_xdp_devmap_xmit(struct devmap_xmit_ctx *ctx)
 {
 	struct datarec *rec;
-- 
2.18.1

Re: [PATCH] samples/bpf: make xdp_monitor use raw_tracepoints

[Jesper Dangaard Brouer]

On Mon,  7 Oct 2019 04:57:26 +0000 Eric Sage <eric@sage.org> wrote:

> raw_tracepoints are an eBPF alternative to standard tracepoints which
> attach to a tracepoint without the perf layer being executed, making
> them faster.
> 
> Since xdp_monitor is supposed to have as little impact on the system as
> possible it is switched to using them by append raw_ to the SEC names.
> 
> There was also a small issues with 'samples/bpf/bpf_load' - it was
> loading the raw_tracepoints with the tracing subsystem name still
> attached, which the bpf syscall rejects with a No such file or directory
> error. This is now fixed.
> 
> Signed-off-by: Eric Sage <eric@sage.org>
> ---
>  samples/bpf/bpf_load.c         |  5 +++--
>  samples/bpf/xdp_monitor_kern.c | 26 +++++++++++++-------------
>  2 files changed, 16 insertions(+), 15 deletions(-)

If there is an issue in the loader 'samples/bpf/bpf_load.c' then we
should of-cause fix it, but you should be aware that we are in general
trying to deprecate this loader, and we want to convert users over to
libbpf.

This patch seems like a good first step forward.  Longer term, I would
like to see this converted into using libbpf.  The library are missing
attach helpers for regular tracepoints, but for raw_tracepoints it does
contain bpf_raw_tracepoint_open().

You can see an example of how xdp_monitor have been converted into
using libbpf and raw_tracepoints here (by Jiri Olsa):

 https://github.com/xdp-project/xdp-tutorial/blob/master/tracing02-xdp-monitor/trace_prog_kern.c


> diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
> index 4574b1939e49..6f57eee8e913 100644
> --- a/samples/bpf/bpf_load.c
> +++ b/samples/bpf/bpf_load.c
> @@ -156,9 +156,10 @@ static int load_and_attach(const char *event, struct bpf_insn *prog, int size)
>  	}
>  
>  	if (is_raw_tracepoint) {
> -		efd = bpf_raw_tracepoint_open(event + 15, fd);
> +		efd = bpf_raw_tracepoint_open(event + 19, fd);
>  		if (efd < 0) {
> -			printf("tracepoint %s %s\n", event + 15, strerror(errno));
> +			printf("tracepoint %s %s\n", event + 19,
> +						strerror(errno));

Are you sure this is the correct fix?

You might break: test_overhead_raw_tp_kern.c


>  			return -1;
>  		}
>  		event_fd[prog_cnt - 1] = efd;
> diff --git a/samples/bpf/xdp_monitor_kern.c b/samples/bpf/xdp_monitor_kern.c
> index ad10fe700d7d..6f67c38468b9 100644
> --- a/samples/bpf/xdp_monitor_kern.c
> +++ b/samples/bpf/xdp_monitor_kern.c
> @@ -23,10 +23,10 @@ struct bpf_map_def SEC("maps") exception_cnt = {
>  };
>  
>  /* Tracepoint format: /sys/kernel/debug/tracing/events/xdp/xdp_redirect/format
> + * Notice: For raw_tracepoints first 8 bytes are not part of 'format' struct
>   * Code in:                kernel/include/trace/events/xdp.h
>   */
>  struct xdp_redirect_ctx {
> -	u64 __pad;		// First 8 bytes are not accessible by bpf code
>  	int prog_id;		//	offset:8;  size:4; signed:1;
>  	u32 act;		//	offset:12  size:4; signed:0;
>  	int ifindex;		//	offset:16  size:4; signed:1;
> @@ -65,44 +65,44 @@ int xdp_redirect_collect_stat(struct xdp_redirect_ctx *ctx)
>  	 */
>  }
>  
> -SEC("tracepoint/xdp/xdp_redirect_err")
> +SEC("raw_tracepoint/xdp/xdp_redirect_err")
>  int trace_xdp_redirect_err(struct xdp_redirect_ctx *ctx)
>  {
>  	return xdp_redirect_collect_stat(ctx);
>  }
>  
>  
> -SEC("tracepoint/xdp/xdp_redirect_map_err")
> +SEC("raw_tracepoint/xdp/xdp_redirect_map_err")
>  int trace_xdp_redirect_map_err(struct xdp_redirect_ctx *ctx)
>  {
>  	return xdp_redirect_collect_stat(ctx);
>  }
>  
>  /* Likely unloaded when prog starts */
> -SEC("tracepoint/xdp/xdp_redirect")
> +SEC("raw_tracepoint/xdp/xdp_redirect")
>  int trace_xdp_redirect(struct xdp_redirect_ctx *ctx)
>  {
>  	return xdp_redirect_collect_stat(ctx);
>  }
>  
>  /* Likely unloaded when prog starts */
> -SEC("tracepoint/xdp/xdp_redirect_map")
> +SEC("raw_tracepoint/xdp/xdp_redirect_map")
>  int trace_xdp_redirect_map(struct xdp_redirect_ctx *ctx)
>  {
>  	return xdp_redirect_collect_stat(ctx);
>  }
>  
>  /* Tracepoint format: /sys/kernel/debug/tracing/events/xdp/xdp_exception/format
> + * Notice: For raw_tracepoints first 8 bytes are not part of 'format' struct
>   * Code in:                kernel/include/trace/events/xdp.h
>   */
>  struct xdp_exception_ctx {
> -	u64 __pad;	// First 8 bytes are not accessible by bpf code
>  	int prog_id;	//	offset:8;  size:4; signed:1;
>  	u32 act;	//	offset:12; size:4; signed:0;
>  	int ifindex;	//	offset:16; size:4; signed:1;
>  };
>  
> -SEC("tracepoint/xdp/xdp_exception")
> +SEC("raw_tracepoint/xdp/xdp_exception")
>  int trace_xdp_exception(struct xdp_exception_ctx *ctx)
>  {
>  	u64 *cnt;
> @@ -144,10 +144,10 @@ struct bpf_map_def SEC("maps") cpumap_kthread_cnt = {
>  };
>  
>  /* Tracepoint: /sys/kernel/debug/tracing/events/xdp/xdp_cpumap_enqueue/format
> + * Notice: For raw_tracepoints first 8 bytes are not part of 'format' struct
>   * Code in:         kernel/include/trace/events/xdp.h
>   */
>  struct cpumap_enqueue_ctx {
> -	u64 __pad;		// First 8 bytes are not accessible by bpf code
>  	int map_id;		//	offset:8;  size:4; signed:1;
>  	u32 act;		//	offset:12; size:4; signed:0;
>  	int cpu;		//	offset:16; size:4; signed:1;
> @@ -156,7 +156,7 @@ struct cpumap_enqueue_ctx {
>  	int to_cpu;		//	offset:28; size:4; signed:1;
>  };
>  
> -SEC("tracepoint/xdp/xdp_cpumap_enqueue")
> +SEC("raw_tracepoint/xdp/xdp_cpumap_enqueue")
>  int trace_xdp_cpumap_enqueue(struct cpumap_enqueue_ctx *ctx)
>  {
>  	u32 to_cpu = ctx->to_cpu;
> @@ -179,10 +179,10 @@ int trace_xdp_cpumap_enqueue(struct cpumap_enqueue_ctx *ctx)
>  }
>  
>  /* Tracepoint: /sys/kernel/debug/tracing/events/xdp/xdp_cpumap_kthread/format
> + * Notice: For raw_tracepoints first 8 bytes are not part of 'format' struct
>   * Code in:         kernel/include/trace/events/xdp.h
>   */
>  struct cpumap_kthread_ctx {
> -	u64 __pad;		// First 8 bytes are not accessible by bpf code
>  	int map_id;		//	offset:8;  size:4; signed:1;
>  	u32 act;		//	offset:12; size:4; signed:0;
>  	int cpu;		//	offset:16; size:4; signed:1;
> @@ -191,7 +191,7 @@ struct cpumap_kthread_ctx {
>  	int sched;		//	offset:28; size:4; signed:1;
>  };
>  
> -SEC("tracepoint/xdp/xdp_cpumap_kthread")
> +SEC("raw_tracepoint/xdp/xdp_cpumap_kthread")
>  int trace_xdp_cpumap_kthread(struct cpumap_kthread_ctx *ctx)
>  {
>  	struct datarec *rec;
> @@ -218,10 +218,10 @@ struct bpf_map_def SEC("maps") devmap_xmit_cnt = {
>  };
>  
>  /* Tracepoint: /sys/kernel/debug/tracing/events/xdp/xdp_devmap_xmit/format
> + * Notice: For raw_tracepoints first 8 bytes are not part of 'format' struct
>   * Code in:         kernel/include/trace/events/xdp.h
>   */
>  struct devmap_xmit_ctx {
> -	u64 __pad;		// First 8 bytes are not accessible by bpf code
>  	int map_id;		//	offset:8;  size:4; signed:1;
>  	u32 act;		//	offset:12; size:4; signed:0;
>  	u32 map_index;		//	offset:16; size:4; signed:0;
> @@ -232,7 +232,7 @@ struct devmap_xmit_ctx {
>  	int err;		//	offset:36; size:4; signed:1;
>  };
>  
> -SEC("tracepoint/xdp/xdp_devmap_xmit")
> +SEC("raw_tracepoint/xdp/xdp_devmap_xmit")
>  int trace_xdp_devmap_xmit(struct devmap_xmit_ctx *ctx)
>  {
>  	struct datarec *rec;



-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH] samples/bpf: make xdp_monitor use raw_tracepoints

[Andrii Nakryiko]

On Mon, Oct 7, 2019 at 2:00 AM Jesper Dangaard Brouer <brouer@redhat.com> wrote:
>
>
> On Mon,  7 Oct 2019 04:57:26 +0000 Eric Sage <eric@sage.org> wrote:
>
> > raw_tracepoints are an eBPF alternative to standard tracepoints which
> > attach to a tracepoint without the perf layer being executed, making
> > them faster.
> >
> > Since xdp_monitor is supposed to have as little impact on the system as
> > possible it is switched to using them by append raw_ to the SEC names.
> >
> > There was also a small issues with 'samples/bpf/bpf_load' - it was
> > loading the raw_tracepoints with the tracing subsystem name still
> > attached, which the bpf syscall rejects with a No such file or directory
> > error. This is now fixed.
> >
> > Signed-off-by: Eric Sage <eric@sage.org>
> > ---
> >  samples/bpf/bpf_load.c         |  5 +++--
> >  samples/bpf/xdp_monitor_kern.c | 26 +++++++++++++-------------
> >  2 files changed, 16 insertions(+), 15 deletions(-)
>
> If there is an issue in the loader 'samples/bpf/bpf_load.c' then we
> should of-cause fix it, but you should be aware that we are in general
> trying to deprecate this loader, and we want to convert users over to
> libbpf.
>
> This patch seems like a good first step forward.  Longer term, I would
> like to see this converted into using libbpf.  The library are missing
> attach helpers for regular tracepoints, but for raw_tracepoints it does
> contain bpf_raw_tracepoint_open().

libbpf has both bpf_program__attach_tracepoint() and
bpf_program__attach_raw_tracepoint(), please use them, not lower-level
bpf_raw_tracepoint_open(). See selftests/bpf/prog_tests/attach_probe.c
for examples.

>
> You can see an example of how xdp_monitor have been converted into
> using libbpf and raw_tracepoints here (by Jiri Olsa):
>
>  https://github.com/xdp-project/xdp-tutorial/blob/master/tracing02-xdp-monitor/trace_prog_kern.c
>
>

[...]

Re: [PATCH] samples/bpf: make xdp_monitor use raw_tracepoints

[Eric Sage]

I'm no longer able to build the samples with 'make M=samples/bpf'.

I get errors in task_fd_query_user.c like:

samples/bpf/task_fd_query_user.c:153:29: error: ‘PERF_EVENT_IOC_ENABLE’
undeclared.

Am I missing a dependancy?

Re: [PATCH] samples/bpf: make xdp_monitor use raw_tracepoints

[Jesper Dangaard Brouer]

On Tue, 15 Oct 2019 21:21:04 -0700
Eric Sage <eric@sage.org> wrote:

> I'm no longer able to build the samples with 'make M=samples/bpf'.
> 
> I get errors in task_fd_query_user.c like:
> 
> samples/bpf/task_fd_query_user.c:153:29: error: ‘PERF_EVENT_IOC_ENABLE’
> undeclared.
> 
> Am I missing a dependancy?

Have you remembered to run:

 make headers_install

(As described in samples/bpf/README)

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH] samples/bpf: make xdp_monitor use raw_tracepoints

[Eric Sage]

On Wed, Oct 16, 2019 at 03:34:26PM +0200, Jesper Dangaard Brouer wrote:
> On Tue, 15 Oct 2019 21:21:04 -0700
> Eric Sage <eric@sage.org> wrote:
> 
> > I'm no longer able to build the samples with 'make M=samples/bpf'.
> > 
> > I get errors in task_fd_query_user.c like:
> > 
> > samples/bpf/task_fd_query_user.c:153:29: error: ‘PERF_EVENT_IOC_ENABLE’
> > undeclared.
> > 
> > Am I missing a dependancy?
> 
> Have you remembered to run:
> 
>  make headers_install
> 
> (As described in samples/bpf/README)

Yes, I've done that. I've tried:

  make mrproper
  cp /boot/config-5.2.18-200.fc30.x86_64 .config
  make olddefconfig
  make headers_install
  make M=samples/bpf

which ends with the errors I described.
> -- 
> Best regards,
>   Jesper Dangaard Brouer
>   MSc.CS, Principal Kernel Engineer at Red Hat
>   LinkedIn: http://www.linkedin.com/in/brouer