Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1472176

Re: [PATCH net-next 2/6] bpf: introduce BPF_PROG_TYPE_PERF_EVENT program type

From Daniel Borkmann <daniel@iogearbox.net>
Newsgroups linux.kernel
Subject Re: [PATCH net-next 2/6] bpf: introduce BPF_PROG_TYPE_PERF_EVENT program type
Date 2016-08-30 02:20 +0200
Message-ID <sbEWt-5PV-5@gated-at.bofh.it> (permalink)
References <saBHj-793-3@gated-at.bofh.it> <saBHk-793-21@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


On 08/27/2016 04:31 AM, Alexei Starovoitov wrote:
> Introduce BPF_PROG_TYPE_PERF_EVENT programs that can be attached to
> HW and SW perf events (PERF_TYPE_HARDWARE and PERF_TYPE_SOFTWARE
> correspondingly in uapi/linux/perf_event.h)
>
> The program visible context meta structure is
> struct bpf_perf_event_data {
>      struct pt_regs regs;
>       __u64 sample_period;
> };
> which is accessible directly from the program:
> int bpf_prog(struct bpf_perf_event_data *ctx)
> {
>    ... ctx->sample_period ...
>    ... ctx->regs.ip ...
> }
>
> The bpf verifier rewrites the accesses into kernel internal
> struct bpf_perf_event_data_kern which allows changing
> struct perf_sample_data without affecting bpf programs.
> New fields can be added to the end of struct bpf_perf_event_data
> in the future.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Two things I noticed below, otherwise for BPF bits:

Acked-by: Daniel Borkmann <daniel@iogearbox.net>

[...]
>
> +static bool pe_prog_is_valid_access(int off, int size, enum bpf_access_type type,
> +				    enum bpf_reg_type *reg_type)
> +{
> +	if (off < 0 || off >= sizeof(struct bpf_perf_event_data))
> +		return false;
> +	if (type != BPF_READ)
> +		return false;
> +	if (off % size != 0)
> +		return false;
> +	if (off == offsetof(struct bpf_perf_event_data, sample_period) &&
> +	    size != sizeof(u64))
> +		return false;
> +	if (size != sizeof(long))
> +		return false;

Wouldn't this one rather need to be:

if (off == offsetof(struct bpf_perf_event_data, sample_period) {
	if (size != sizeof(u64))
		return false;
} else {
	if (size != sizeof(long))
		return false;
}

Otherwise on 32bit accessing sample_period might fail?

> +	return true;
> +}
> +
> +static u32 pe_prog_convert_ctx_access(enum bpf_access_type type, int dst_reg,
> +				      int src_reg, int ctx_off,
> +				      struct bpf_insn *insn_buf,
> +				      struct bpf_prog *prog)
> +{
> +	struct bpf_insn *insn = insn_buf;
> +
> +	switch (ctx_off) {
> +	case offsetof(struct bpf_perf_event_data, sample_period):

Would be good to add a test as we usually have done:

BUILD_BUG_ON(FIELD_SIZEOF(struct perf_sample_data, period) != 8);

> +		*insn++ = BPF_LDX_MEM(bytes_to_bpf_size(FIELD_SIZEOF(struct bpf_perf_event_data_kern, data)),
> +				      dst_reg, src_reg,
> +				      offsetof(struct bpf_perf_event_data_kern, data));
> +		*insn++ = BPF_LDX_MEM(BPF_DW, dst_reg, dst_reg,
> +				      offsetof(struct perf_sample_data, period));
> +		break;
> +	default:
> +		*insn++ = BPF_LDX_MEM(bytes_to_bpf_size(FIELD_SIZEOF(struct bpf_perf_event_data_kern, regs)),
> +				      dst_reg, src_reg,
> +				      offsetof(struct bpf_perf_event_data_kern, regs));
> +		*insn++ = BPF_LDX_MEM(bytes_to_bpf_size(sizeof(long)),
> +				      dst_reg, dst_reg, ctx_off);
> +		break;
> +	}
> +	return insn - insn_buf;
> +}
> +

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[PATCH net-next 0/6] perf, bpf: add support for bpf in sw/hw perf_events Alexei Starovoitov <ast@fb.com> - 2016-08-27 04:40 +0200
  [PATCH net-next 1/6] bpf: support 8-byte metafield access Alexei Starovoitov <ast@fb.com> - 2016-08-27 04:40 +0200
    Re: [PATCH net-next 1/6] bpf: support 8-byte metafield access Daniel Borkmann <daniel@iogearbox.net> - 2016-08-30 02:00 +0200
  [PATCH net-next 6/6] samples/bpf: add sampleip example Alexei Starovoitov <ast@fb.com> - 2016-08-27 04:40 +0200
  [PATCH net-next 4/6] perf, bpf: add perf events core support for BPF_PROG_TYPE_PERF_EVENT programs Alexei Starovoitov <ast@fb.com> - 2016-08-27 04:40 +0200
    Re: [PATCH net-next 4/6] perf, bpf: add perf events core support for  BPF_PROG_TYPE_PERF_EVENT programs Peter Zijlstra <peterz@infradead.org> - 2016-08-29 14:20 +0200
      Re: [PATCH net-next 4/6] perf, bpf: add perf events core support for  BPF_PROG_TYPE_PERF_EVENT programs Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-31 05:50 +0200
  [PATCH net-next 2/6] bpf: introduce BPF_PROG_TYPE_PERF_EVENT program type Alexei Starovoitov <ast@fb.com> - 2016-08-27 04:40 +0200
    Re: [PATCH net-next 2/6] bpf: introduce BPF_PROG_TYPE_PERF_EVENT  program type Daniel Borkmann <daniel@iogearbox.net> - 2016-08-30 02:20 +0200
  [PATCH net-next 5/6] samples/bpf: add perf_event+bpf example Alexei Starovoitov <ast@fb.com> - 2016-08-27 04:40 +0200
  Re: [PATCH net-next 0/6] perf, bpf: add support for bpf in sw/hw  perf_events Peter Zijlstra <peterz@infradead.org> - 2016-08-29 13:00 +0200
    Re: [PATCH net-next 0/6] perf, bpf: add support for bpf in sw/hw  perf_events Alexei Starovoitov <alexei.starovoitov@gmail.com> - 2016-08-30 01:10 +0200
  Re: [PATCH net-next 0/6] perf, bpf: add support for bpf in sw/hw  perf_events Peter Zijlstra <peterz@infradead.org> - 2016-08-29 14:20 +0200
    Re: [PATCH net-next 0/6] perf, bpf: add support for bpf in sw/hw perf_events Brendan Gregg <brendan.d.gregg@gmail.com> - 2016-08-30 04:30 +0200

csiph-web