Re: [PATCH bpf 2/2] net/flow_dissector: correctly cap nhoff and thoff in case of BPF
On Wed, Dec 5, 2018 at 8:41 PM Stanislav Fomichev wrote:
>
> We want to make sure that the following condition holds:
> 0 <= nhoff <= thoff <= skb->len
>
> BPF program can set out-of-bounds nhoff and thoff, which is dangerous, see
> recent commit d0c081b49137 ("flow_dissector: properly cap thoff field")'.
>
> Signed-off-by: Stanislav Fomichev
Acked-by: Song Liu
> ---
> net/core/flow_dissector.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index ff5556d80570..af68207ee56c 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -791,9 +791,12 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
> /* Restore state */
> memcpy(cb, _saved, sizeof(cb_saved));
>
> + flow_keys.nhoff = clamp_t(u16, flow_keys.nhoff, 0, skb->len);
> + flow_keys.thoff = clamp_t(u16, flow_keys.thoff,
> + flow_keys.nhoff, skb->len);
> +
> __skb_flow_bpf_to_target(_keys, flow_dissector,
> target_container);
> - key_control->thoff = min_t(u16, key_control->thoff, skb->len);
> rcu_read_unlock();
> return result == BPF_OK;
> }
> --
> 2.20.0.rc1.387.gf8505762e3-goog
>
Re: [PATCH bpf 1/2] selftests/bpf: use thoff instead of nhoff in BPF flow dissector
On Wed, Dec 5, 2018 at 8:41 PM Stanislav Fomichev wrote:
>
> We are returning thoff from the flow dissector, not the nhoff. Pass
> thoff along with nhoff to the bpf program (initially thoff == nhoff)
> and expect flow dissector amend/return thoff, not nhoff.
>
> This avoids confusion, when by the time bpf flow dissector exits,
> nhoff == thoff, which doesn't make much sense.
>
> Signed-off-by: Stanislav Fomichev
Acked-by: Song Liu
> ---
> net/core/flow_dissector.c | 1 +
> tools/testing/selftests/bpf/bpf_flow.c | 36 --
> 2 files changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 588f475019d4..ff5556d80570 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -783,6 +783,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
> /* Pass parameters to the BPF program */
> cb->qdisc_cb.flow_keys = _keys;
> flow_keys.nhoff = nhoff;
> + flow_keys.thoff = nhoff;
>
> bpf_compute_data_pointers((struct sk_buff *)skb);
> result = BPF_PROG_RUN(attached, skb);
> diff --git a/tools/testing/selftests/bpf/bpf_flow.c
> b/tools/testing/selftests/bpf/bpf_flow.c
> index 107350a7821d..df9d32fd2055 100644
> --- a/tools/testing/selftests/bpf/bpf_flow.c
> +++ b/tools/testing/selftests/bpf/bpf_flow.c
> @@ -70,18 +70,18 @@ static __always_inline void
> *bpf_flow_dissect_get_header(struct __sk_buff *skb,
> {
> void *data_end = (void *)(long)skb->data_end;
> void *data = (void *)(long)skb->data;
> - __u16 nhoff = skb->flow_keys->nhoff;
> + __u16 thoff = skb->flow_keys->thoff;
> __u8 *hdr;
>
> /* Verifies this variable offset does not overflow */
> - if (nhoff > (USHRT_MAX - hdr_size))
> + if (thoff > (USHRT_MAX - hdr_size))
> return NULL;
>
> - hdr = data + nhoff;
> + hdr = data + thoff;
> if (hdr + hdr_size <= data_end)
> return hdr;
>
> - if (bpf_skb_load_bytes(skb, nhoff, buffer, hdr_size))
> + if (bpf_skb_load_bytes(skb, thoff, buffer, hdr_size))
> return NULL;
>
> return buffer;
> @@ -158,13 +158,13 @@ static __always_inline int parse_ip_proto(struct
> __sk_buff *skb, __u8 proto)
> /* Only inspect standard GRE packets with version 0 */
> return BPF_OK;
>
> - keys->nhoff += sizeof(*gre); /* Step over GRE Flags and Proto
> */
> + keys->thoff += sizeof(*gre); /* Step over GRE Flags and Proto
> */
> if (GRE_IS_CSUM(gre->flags))
> - keys->nhoff += 4; /* Step over chksum and Padding */
> + keys->thoff += 4; /* Step over chksum and Padding */
> if (GRE_IS_KEY(gre->flags))
> - keys->nhoff += 4; /* Step over key */
> + keys->thoff += 4; /* Step over key */
> if (GRE_IS_SEQ(gre->flags))
> - keys->nhoff += 4; /* Step over sequence number */
> + keys->thoff += 4; /* Step over sequence number */
>
> keys->is_encap = true;
>
> @@ -174,7 +174,7 @@ static __always_inline int parse_ip_proto(struct
> __sk_buff *skb, __u8 proto)
> if (!eth)
> return BPF_DROP;
>
> - keys->nhoff += sizeof(*eth);
> + keys->thoff += sizeof(*eth);
>
> return parse_eth_proto(skb, eth->h_proto);
> } else {
> @@ -191,7 +191,6 @@ static __always_inline int parse_ip_proto(struct
> __sk_buff *skb, __u8 proto)
> if ((__u8 *)tcp + (tcp->doff << 2) > data_end)
> return BPF_DROP;
>
> - keys->thoff = keys->nhoff;
> keys->sport = tcp->source;
> keys->dport = tcp->dest;
> return BPF_OK;
> @@ -201,7 +200,6 @@ static __always_inline int parse_ip_proto(struct
> __sk_buff *skb, __u8 proto)
> if (!udp)
> return BPF_DROP;
>
> - keys->thoff = keys->nhoff;
> keys->sport = udp->source;
> keys->dport = udp->dest;
> return BPF_OK;
> @@ -252,8 +250,8 @@ PROG(IP)(struct __sk_buff *skb)
> keys->ipv4_src = iph->saddr;
> keys->ipv4_dst = iph->da
Re: [PATCH bpf-next v2 0/5] support flow dissector in BPF_PROG_TEST_RUN
On Mon, Dec 3, 2018 at 8:01 PM Stanislav Fomichev wrote: > > This patch series adds support for testing flow dissector BPF programs by > extending already existing BPF_PROG_TEST_RUN. The goal is to have a > packet as an input and `struct bpf_flow_key' as an output. That way > we can easily test flow dissector programs' behavior. > I've also modified existing test_progs.c test to do a simple flow > dissector run as well. > > * first patch introduces new __skb_flow_bpf_dissect to simplify > sharing between __skb_flow_bpf_dissect and BPF_PROG_TEST_RUN > * second patch adds actual BPF_PROG_TEST_RUN support > * third patch converts BPF flow dissector to thoff > * forth patch correctly caps nhoff and thoff returned from bpf flow > dissector > * fifth patch adds example usage to the selftests > > v2 changes: > > * new patch to use thoff instead of nhoff in bpf flow dissector > * new patch to correctly cap thoff for BPF case > * add missing memset(flow_keys, 0, ...) to __skb_flow_bpf_dissect > * set test iterations to 10 > > Stanislav Fomichev (5): > net/flow_dissector: move bpf case into __skb_flow_bpf_dissect > bpf: add BPF_PROG_TEST_RUN support for flow dissector > selftests/bpf: use thoff instead of nhoff in BPF flow dissector > net/flow_dissector: correctly cap nhoff and thoff in case of BPF > selftests/bpf: add simple BPF_PROG_TEST_RUN examples for flow > dissector > > include/linux/bpf.h | 3 + > include/linux/skbuff.h| 5 ++ > net/bpf/test_run.c| 71 ++- > net/core/filter.c | 1 + > net/core/flow_dissector.c | 88 --- > tools/testing/selftests/bpf/Makefile | 3 + > tools/testing/selftests/bpf/bpf_flow.c| 36 > .../selftests/bpf/flow_dissector_load.c | 43 ++--- > .../selftests/bpf/flow_dissector_load.h | 55 > tools/testing/selftests/bpf/test_progs.c | 78 +++- > 10 files changed, 289 insertions(+), 94 deletions(-) > create mode 100644 tools/testing/selftests/bpf/flow_dissector_load.h > > -- > 2.20.0.rc1.387.gf8505762e3-goog > For the series: Acked-by: Song Liu
Re: [PATCH bpf-next v2 3/5] selftests/bpf: use thoff instead of nhoff in BPF flow dissector
On Tue, Dec 4, 2018 at 3:26 PM Stanislav Fomichev wrote:
>
> On 12/04, Song Liu wrote:
> > On Mon, Dec 3, 2018 at 8:01 PM Stanislav Fomichev wrote:
> > >
> > > We are returning thoff from the flow dissector, not the nhoff. Pass
> > > thoff along with nhoff to the bpf program (initially thoff == nhoff)
> > > and expect flow dissector amend/return thoff, not nhoff.
> > >
> > > This avoids confusion, when by the time bpf flow dissector exits,
> > > nhoff == thoff, which doesn't make much sense (this is relevant
> > > in the context of the next patch, where I add simple selftest
> > > and manually construct expected flow_keys).
> > >
> > > Signed-off-by: Stanislav Fomichev
> > > ---
> > > net/core/flow_dissector.c | 1 +
> > > tools/testing/selftests/bpf/bpf_flow.c | 36 --
> > > 2 files changed, 18 insertions(+), 19 deletions(-)
> > >
> > > diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> > > index 3c8a78decbc0..ac19da6f390b 100644
> > > --- a/net/core/flow_dissector.c
> > > +++ b/net/core/flow_dissector.c
> > > @@ -708,6 +708,7 @@ bool __skb_flow_bpf_dissect(struct bpf_prog *prog,
> > > memset(flow_keys, 0, sizeof(*flow_keys));
> > > cb->qdisc_cb.flow_keys = flow_keys;
> > > flow_keys->nhoff = skb_network_offset(skb);
> > > + flow_keys->thoff = flow_keys->nhoff;
> >
> > Do we need this fix without this set? If yes, do we need it for bpf
> > tree as well?
> No, I don't think so. This just changes input to the flow dissector
> slightly (going forward).
> It used to be nhoff in, thoff out. Now it's thoff in (with nhoff for
> backwards compatibility) and thoff out.
Thanks for the explanation.
Acked-by: Song Liu
>
> >
> > Thanks,
> > Song
> >
> > >
> > > bpf_compute_data_pointers((struct sk_buff *)skb);
> > > result = BPF_PROG_RUN(prog, skb);
> > > diff --git a/tools/testing/selftests/bpf/bpf_flow.c
> > > b/tools/testing/selftests/bpf/bpf_flow.c
> > > index b9798f558ca7..284660f5aa95 100644
> > > --- a/tools/testing/selftests/bpf/bpf_flow.c
> > > +++ b/tools/testing/selftests/bpf/bpf_flow.c
> > > @@ -70,18 +70,18 @@ static __always_inline void
> > > *bpf_flow_dissect_get_header(struct __sk_buff *skb,
> > > {
> > > void *data_end = (void *)(long)skb->data_end;
> > > void *data = (void *)(long)skb->data;
> > > - __u16 nhoff = skb->flow_keys->nhoff;
> > > + __u16 thoff = skb->flow_keys->thoff;
> > > __u8 *hdr;
> > >
> > > /* Verifies this variable offset does not overflow */
> > > - if (nhoff > (USHRT_MAX - hdr_size))
> > > + if (thoff > (USHRT_MAX - hdr_size))
> > > return NULL;
> > >
> > > - hdr = data + nhoff;
> > > + hdr = data + thoff;
> > > if (hdr + hdr_size <= data_end)
> > > return hdr;
> > >
> > > - if (bpf_skb_load_bytes(skb, nhoff, buffer, hdr_size))
> > > + if (bpf_skb_load_bytes(skb, thoff, buffer, hdr_size))
> > > return NULL;
> > >
> > > return buffer;
> > > @@ -158,13 +158,13 @@ static __always_inline int parse_ip_proto(struct
> > > __sk_buff *skb, __u8 proto)
> > > /* Only inspect standard GRE packets with version
> > > 0 */
> > > return BPF_OK;
> > >
> > > - keys->nhoff += sizeof(*gre); /* Step over GRE Flags and
> > > Proto */
> > > + keys->thoff += sizeof(*gre); /* Step over GRE Flags and
> > > Proto */
> > > if (GRE_IS_CSUM(gre->flags))
> > > - keys->nhoff += 4; /* Step over chksum and Padding
> > > */
> > > + keys->thoff += 4; /* Step over chksum and Padding
> > > */
> > > if (GRE_IS_KEY(gre->flags))
> > > - keys->nhoff += 4; /* Step over key */
> > > + keys->thoff += 4; /* Step over key */
> > > if (GRE_IS_SEQ(gre->flags))
> > > - keys->nhoff += 4; /* Step over sequence number */
> > > + keys->thoff += 4; /* Step over sequence number */
> > >
&
Re: [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector
On Mon, Dec 3, 2018 at 3:08 PM Stanislav Fomichev wrote:
>
> On 12/03, Song Liu wrote:
> > On Mon, Dec 3, 2018 at 11:00 AM Stanislav Fomichev wrote:
> > >
> > > The input is packet data, the output is struct bpf_flow_key. This should
> > > make it easy to test flow dissector programs without elaborate
> > > setup.
> > >
> > > Signed-off-by: Stanislav Fomichev
> > > ---
> > > include/linux/bpf.h | 3 ++
> > > net/bpf/test_run.c | 76 +
> > > net/core/filter.c | 1 +
> > > 3 files changed, 74 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > > index e82b7039fc66..7a572d15d5dd 100644
> > > --- a/include/linux/bpf.h
> > > +++ b/include/linux/bpf.h
> > > @@ -373,6 +373,9 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog,
> > > const union bpf_attr *kattr,
> > > union bpf_attr __user *uattr);
> > > int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr
> > > *kattr,
> > > union bpf_attr __user *uattr);
> > > +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> > > +const union bpf_attr *kattr,
> > > +union bpf_attr __user *uattr);
> > >
> > > /* an array of programs to be executed under rcu_lock.
> > > *
> > > diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> > > index c89c22c49015..bfa05d31c6e3 100644
> > > --- a/net/bpf/test_run.c
> > > +++ b/net/bpf/test_run.c
> > > @@ -14,21 +14,33 @@
> > > #include
> > >
> > > static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void
> > > *ctx,
> > > - struct bpf_cgroup_storage
> > > *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
> > > + struct bpf_cgroup_storage
> > > *storage[MAX_BPF_CGROUP_STORAGE_TYPE],
> > > + struct bpf_flow_keys *flow_keys)
> > > {
> > > u32 ret;
> > >
> > > preempt_disable();
> > > rcu_read_lock();
> > > bpf_cgroup_storage_set(storage);
> > > - ret = BPF_PROG_RUN(prog, ctx);
> > > +
> > > + switch (prog->type) {
> > > + case BPF_PROG_TYPE_FLOW_DISSECTOR:
> > > + ret = __skb_flow_bpf_dissect(prog, ctx,
> > > _keys_dissector,
> > > +flow_keys);
> > > + break;
> > > + default:
> > > + ret = BPF_PROG_RUN(prog, ctx);
> > > + break;
> > > + }
> > > +
> >
> > Is it possible to fold the logic above into
> > bpf_prog_test_run_flow_dissector()?
> > In that way, the logic flow is similar to other bpf_prog_test_run_XXX()
> > functions.
> I can probably do everything that __skb_flow_bpf_dissect does inside of
> bpf_prog_test_run_flow_dissector (pass flow_keys in cb); that would remove
> this ugly program type check down the stack. But my additional goal here was
> to test __skb_flow_bpf_dissect itself as well.
>
> Attached some possible prototype below. The only issue I see with that
> approach is that we need to clear flow_keys on each test iteration.
> I think in my current patch I'm actually missing a memset(_keys, 0, ...)
> for each iteration of __skb_flow_bpf_dissect;
> I haven't tested multiple runs :-/
>
> So I'd prefer to continue doing this prog type check (plus, add a missing
> memset for flow_keys on each iteration in v2). WDYT?
I think v2 is still using original logic? Actually, after a second
thought, I think
it is OK to change bpf_test_run_one() (unless Alexei and/or Daniel has better
suggestions.
Thanks,
Song
>
> ---
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index e82b7039fc66..7a572d15d5dd 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -373,6 +373,9 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const
> union bpf_attr *kattr,
> union bpf_attr __user *uattr);
> int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> union bpf_attr __user *uattr);
> +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> +const union bpf_attr *kattr,
> +union bpf_attr __user *uattr);
>
> /* an array of programs to
Re: [PATCH bpf-next v2 4/5] net/flow_dissector: correctly cap nhoff and thoff in case of BPF
On Mon, Dec 3, 2018 at 8:01 PM Stanislav Fomichev wrote:
>
> We want to make sure that the following condition holds:
> 0 <= nhoff <= thoff <= skb->len
>
> BPF program can set out-of-bounds nhoff and thoff, which is dangerous, see
> recent commit d0c081b49137 ("flow_dissector: properly cap thoff field")'.
>
> Signed-off-by: Stanislav Fomichev
> ---
> net/core/flow_dissector.c | 6 --
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index ac19da6f390b..bb1a54747d64 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -716,6 +716,10 @@ bool __skb_flow_bpf_dissect(struct bpf_prog *prog,
> /* Restore state */
> memcpy(cb, _saved, sizeof(cb_saved));
>
> + flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff, 0, skb->len);
> + flow_keys->thoff = clamp_t(u16, flow_keys->thoff,
> + flow_keys->nhoff, skb->len);
> +
> return result == BPF_OK;
> }
>
> @@ -808,8 +812,6 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
> _keys);
> __skb_flow_bpf_to_target(_keys, flow_dissector,
> target_container);
> - key_control->thoff = min_t(u16, key_control->thoff,
> - skb->len);
> rcu_read_unlock();
> return ret;
> }
> --
> 2.20.0.rc1.387.gf8505762e3-goog
>
Same question as 3/5:
Do we need this fix without this set? If yes, do we need it for bpf
tree as well?
Thanks,
Song
Re: [PATCH bpf-next v2 3/5] selftests/bpf: use thoff instead of nhoff in BPF flow dissector
On Mon, Dec 3, 2018 at 8:01 PM Stanislav Fomichev wrote:
>
> We are returning thoff from the flow dissector, not the nhoff. Pass
> thoff along with nhoff to the bpf program (initially thoff == nhoff)
> and expect flow dissector amend/return thoff, not nhoff.
>
> This avoids confusion, when by the time bpf flow dissector exits,
> nhoff == thoff, which doesn't make much sense (this is relevant
> in the context of the next patch, where I add simple selftest
> and manually construct expected flow_keys).
>
> Signed-off-by: Stanislav Fomichev
> ---
> net/core/flow_dissector.c | 1 +
> tools/testing/selftests/bpf/bpf_flow.c | 36 --
> 2 files changed, 18 insertions(+), 19 deletions(-)
>
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index 3c8a78decbc0..ac19da6f390b 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -708,6 +708,7 @@ bool __skb_flow_bpf_dissect(struct bpf_prog *prog,
> memset(flow_keys, 0, sizeof(*flow_keys));
> cb->qdisc_cb.flow_keys = flow_keys;
> flow_keys->nhoff = skb_network_offset(skb);
> + flow_keys->thoff = flow_keys->nhoff;
Do we need this fix without this set? If yes, do we need it for bpf
tree as well?
Thanks,
Song
>
> bpf_compute_data_pointers((struct sk_buff *)skb);
> result = BPF_PROG_RUN(prog, skb);
> diff --git a/tools/testing/selftests/bpf/bpf_flow.c
> b/tools/testing/selftests/bpf/bpf_flow.c
> index b9798f558ca7..284660f5aa95 100644
> --- a/tools/testing/selftests/bpf/bpf_flow.c
> +++ b/tools/testing/selftests/bpf/bpf_flow.c
> @@ -70,18 +70,18 @@ static __always_inline void
> *bpf_flow_dissect_get_header(struct __sk_buff *skb,
> {
> void *data_end = (void *)(long)skb->data_end;
> void *data = (void *)(long)skb->data;
> - __u16 nhoff = skb->flow_keys->nhoff;
> + __u16 thoff = skb->flow_keys->thoff;
> __u8 *hdr;
>
> /* Verifies this variable offset does not overflow */
> - if (nhoff > (USHRT_MAX - hdr_size))
> + if (thoff > (USHRT_MAX - hdr_size))
> return NULL;
>
> - hdr = data + nhoff;
> + hdr = data + thoff;
> if (hdr + hdr_size <= data_end)
> return hdr;
>
> - if (bpf_skb_load_bytes(skb, nhoff, buffer, hdr_size))
> + if (bpf_skb_load_bytes(skb, thoff, buffer, hdr_size))
> return NULL;
>
> return buffer;
> @@ -158,13 +158,13 @@ static __always_inline int parse_ip_proto(struct
> __sk_buff *skb, __u8 proto)
> /* Only inspect standard GRE packets with version 0 */
> return BPF_OK;
>
> - keys->nhoff += sizeof(*gre); /* Step over GRE Flags and Proto
> */
> + keys->thoff += sizeof(*gre); /* Step over GRE Flags and Proto
> */
> if (GRE_IS_CSUM(gre->flags))
> - keys->nhoff += 4; /* Step over chksum and Padding */
> + keys->thoff += 4; /* Step over chksum and Padding */
> if (GRE_IS_KEY(gre->flags))
> - keys->nhoff += 4; /* Step over key */
> + keys->thoff += 4; /* Step over key */
> if (GRE_IS_SEQ(gre->flags))
> - keys->nhoff += 4; /* Step over sequence number */
> + keys->thoff += 4; /* Step over sequence number */
>
> keys->is_encap = true;
>
> @@ -174,7 +174,7 @@ static __always_inline int parse_ip_proto(struct
> __sk_buff *skb, __u8 proto)
> if (!eth)
> return BPF_DROP;
>
> - keys->nhoff += sizeof(*eth);
> + keys->thoff += sizeof(*eth);
>
> return parse_eth_proto(skb, eth->h_proto);
> } else {
> @@ -191,7 +191,6 @@ static __always_inline int parse_ip_proto(struct
> __sk_buff *skb, __u8 proto)
> if ((__u8 *)tcp + (tcp->doff << 2) > data_end)
> return BPF_DROP;
>
> - keys->thoff = keys->nhoff;
> keys->sport = tcp->source;
> keys->dport = tcp->dest;
> return BPF_OK;
> @@ -201,7 +200,6 @@ static __always_inline int parse_ip_proto(struct
> __sk_buff *skb, __u8 proto)
> if (!udp)
> return BPF_DROP;
>
> - keys->thoff = keys->nhoff;
> keys->sport = udp->source;
> keys->dport
Re: [PATCH bpf-next 2/3] bpf: add BPF_PROG_TEST_RUN support for flow dissector
On Mon, Dec 3, 2018 at 11:00 AM Stanislav Fomichev wrote:
>
> The input is packet data, the output is struct bpf_flow_key. This should
> make it easy to test flow dissector programs without elaborate
> setup.
>
> Signed-off-by: Stanislav Fomichev
> ---
> include/linux/bpf.h | 3 ++
> net/bpf/test_run.c | 76 +
> net/core/filter.c | 1 +
> 3 files changed, 74 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index e82b7039fc66..7a572d15d5dd 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -373,6 +373,9 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const
> union bpf_attr *kattr,
> union bpf_attr __user *uattr);
> int bpf_prog_test_run_skb(struct bpf_prog *prog, const union bpf_attr *kattr,
> union bpf_attr __user *uattr);
> +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> +const union bpf_attr *kattr,
> +union bpf_attr __user *uattr);
>
> /* an array of programs to be executed under rcu_lock.
> *
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index c89c22c49015..bfa05d31c6e3 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -14,21 +14,33 @@
> #include
>
> static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx,
> - struct bpf_cgroup_storage
> *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
> + struct bpf_cgroup_storage
> *storage[MAX_BPF_CGROUP_STORAGE_TYPE],
> + struct bpf_flow_keys *flow_keys)
> {
> u32 ret;
>
> preempt_disable();
> rcu_read_lock();
> bpf_cgroup_storage_set(storage);
> - ret = BPF_PROG_RUN(prog, ctx);
> +
> + switch (prog->type) {
> + case BPF_PROG_TYPE_FLOW_DISSECTOR:
> + ret = __skb_flow_bpf_dissect(prog, ctx, _keys_dissector,
> +flow_keys);
> + break;
> + default:
> + ret = BPF_PROG_RUN(prog, ctx);
> + break;
> + }
> +
Is it possible to fold the logic above into bpf_prog_test_run_flow_dissector()?
In that way, the logic flow is similar to other bpf_prog_test_run_XXX()
functions.
Thanks,
Song
> rcu_read_unlock();
> preempt_enable();
>
> return ret;
> }
>
> -static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32
> *time)
> +static u32 bpf_test_run(struct bpf_prog *prog, void *ctx, u32 repeat, u32
> *time,
> + struct bpf_flow_keys *flow_keys)
> {
> struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE] = { 0
> };
> enum bpf_cgroup_storage_type stype;
> @@ -49,7 +61,7 @@ static u32 bpf_test_run(struct bpf_prog *prog, void *ctx,
> u32 repeat, u32 *time)
> repeat = 1;
> time_start = ktime_get_ns();
> for (i = 0; i < repeat; i++) {
> - ret = bpf_test_run_one(prog, ctx, storage);
> + ret = bpf_test_run_one(prog, ctx, storage, flow_keys);
> if (need_resched()) {
> if (signal_pending(current))
> break;
> @@ -165,7 +177,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const
> union bpf_attr *kattr,
> __skb_push(skb, hh_len);
> if (is_direct_pkt_access)
> bpf_compute_data_pointers(skb);
> - retval = bpf_test_run(prog, skb, repeat, );
> + retval = bpf_test_run(prog, skb, repeat, , NULL);
> if (!is_l2) {
> if (skb_headroom(skb) < hh_len) {
> int nhead = HH_DATA_ALIGN(hh_len - skb_headroom(skb));
> @@ -212,7 +224,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const
> union bpf_attr *kattr,
> rxqueue =
> __netif_get_rx_queue(current->nsproxy->net_ns->loopback_dev, 0);
> xdp.rxq = >xdp_rxq;
>
> - retval = bpf_test_run(prog, , repeat, );
> + retval = bpf_test_run(prog, , repeat, , NULL);
> if (xdp.data != data + XDP_PACKET_HEADROOM + NET_IP_ALIGN ||
> xdp.data_end != xdp.data + size)
> size = xdp.data_end - xdp.data;
> @@ -220,3 +232,55 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const
> union bpf_attr *kattr,
> kfree(data);
> return ret;
> }
> +
> +int bpf_prog_test_run_flow_dissector(struct bpf_prog *prog,
> +const union bpf_attr *kattr,
> +uni
Re: [PATCH v2] samples: bpf: fix: seg fault with NULL pointer arg
On Mon, Dec 3, 2018 at 2:40 AM Daniel T. Lee wrote:
>
> When NULL pointer accidentally passed to write_kprobe_events,
> due to strlen(NULL), segmentation fault happens.
> Changed code returns -1 to deal with this situation.
>
> Bug issued with Smatch, static analysis.
>
> Signed-off-by: Daniel T. Lee
Acked-by: Song Liu
> ---
> samples/bpf/bpf_load.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
> index 434ea34a5954..eae7b635343d 100644
> --- a/samples/bpf/bpf_load.c
> +++ b/samples/bpf/bpf_load.c
> @@ -58,7 +58,9 @@ static int write_kprobe_events(const char *val)
> {
> int fd, ret, flags;
>
> - if ((val != NULL) && (val[0] == '\0'))
> + if (val == NULL)
> + return -1;
> + else if (val[0] == '\0')
> flags = O_WRONLY | O_TRUNC;
> else
> flags = O_WRONLY | O_APPEND;
> --
> 2.17.1
>
Re: [PATCH bpf] mips: bpf: fix encoding bug for mm_srlv32_op
On Sat, Dec 1, 2018 at 1:12 AM Jiong Wang wrote:
>
> For micro-mips, srlv inside POOL32A encoding space should use 0x50
> sub-opcode, NOT 0x90.
>
> Some early version ISA doc describes the encoding as 0x90 for both srlv and
> srav, this looks to me was a typo. I checked Binutils libopcode
> implementation which is using 0x50 for srlv and 0x90 for srav.
>
> Fixes: f31318fdf324 ("MIPS: uasm: Add srlv uasm instruction")
> CC: Markos Chandras
> CC: Paul Burton
> Acked-by: Jakub Kicinski
> Signed-off-by: Jiong Wang
Acked-by: Song Liu
> ---
> arch/mips/include/uapi/asm/inst.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/mips/include/uapi/asm/inst.h
> b/arch/mips/include/uapi/asm/inst.h
> index c05dcf5..80f35e7 100644
> --- a/arch/mips/include/uapi/asm/inst.h
> +++ b/arch/mips/include/uapi/asm/inst.h
> @@ -370,7 +370,7 @@ enum mm_32a_minor_op {
> mm_pool32axf_op = 0x03c,
> mm_srl32_op = 0x040,
> mm_sra_op = 0x080,
> - mm_srlv32_op = 0x090,
> + mm_srlv32_op = 0x050,
> mm_rotr_op = 0x0c0,
> mm_lwxs_op = 0x118,
> mm_addu32_op = 0x150,
> --
> 2.7.4
>
Re: [PATCH] samples: bpf: fix: seg fault with NULL pointer arg
On Sat, Dec 1, 2018 at 1:07 PM Daniel T. Lee wrote:
>
> When NULL pointer accidentally passed to write_kprobe_events,
> due to strlen(NULL), segmentation fault happens.
> Changed code returns -1 to deal with this situation.
>
> Bug issued with Smatch, static analysis.
>
> Signed-off-by: Daniel T. Lee
> ---
> samples/bpf/bpf_load.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
> index 434ea34a5954..c670bd2200d2 100644
> --- a/samples/bpf/bpf_load.c
> +++ b/samples/bpf/bpf_load.c
> @@ -58,7 +58,9 @@ static int write_kprobe_events(const char *val)
> {
> int fd, ret, flags;
>
> - if ((val != NULL) && (val[0] == '\0'))
> + if (val == NULL)
> + return -1;
> + else if ((val != NULL) && (val[0] == '\0'))
We only need
+ else if (val[0] == '\0')
right?
Thanks,
Song
Re: [PATCH bpf-next] libbpf: Fix license in README.rst
On Sun, Dec 2, 2018 at 1:04 PM Andrey Ignatov wrote:
>
> The whole libbpf is licensed as (LGPL-2.1 OR BSD-2-Clause). I missed it
> while adding README.rst. Fix it and use same license as all other files
> in libbpf do. Since I'm the only author of README.rst so far, no others'
> permissions should be needed.
>
> Fixes: 76d1b894c515 ("libbpf: Document API and ABI conventions")
> Signed-off-by: Andrey Ignatov
Acked-by: Song Liu
> ---
> tools/lib/bpf/README.rst | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/README.rst b/tools/lib/bpf/README.rst
> index 2ced9e061c4b..056f38310722 100644
> --- a/tools/lib/bpf/README.rst
> +++ b/tools/lib/bpf/README.rst
> @@ -1,4 +1,4 @@
> -.. SPDX-License-Identifier: GPL-2.0
> +.. SPDX-License-Identifier: (LGPL-2.1 OR BSD-2-Clause)
>
> libbpf API naming convention
>
> --
> 2.17.1
>
Re: [PATCH bpf-next v2] bpf: allow BPF read access to qdisc pkt_len
On Sun, Dec 2, 2018 at 5:18 PM Willem de Bruijn
wrote:
>
> From: Petar Penkov
>
> The pkt_len field in qdisc_skb_cb stores the skb length as it will
> appear on the wire after segmentation. For byte accounting, this value
> is more accurate than skb->len. It is computed on entry to the TC
> layer, so only valid there.
>
> Allow read access to this field from BPF tc classifier and action
> programs. The implementation is analogous to tc_classid, aside from
> restricting to read access.
>
> To distinguish it from skb->len and self-describe export as wire_len.
>
> Changes v1->v2
> - Rename pkt_len to wire_len
>
> Signed-off-by: Petar Penkov
> Signed-off-by: Vlad Dumitrescu
> Signed-off-by: Willem de Bruijn
Acked-by: Song Liu
> ---
> include/uapi/linux/bpf.h| 1 +
> net/core/filter.c | 16 +++
> tools/include/uapi/linux/bpf.h | 1 +
> tools/testing/selftests/bpf/test_verifier.c | 32 +
> 4 files changed, 50 insertions(+)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 8050caea7495..0183b8e70a9e 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2497,6 +2497,7 @@ struct __sk_buff {
> __u32 data_meta;
> struct bpf_flow_keys *flow_keys;
> __u64 tstamp;
> + __u32 wire_len;
> };
>
> struct bpf_tunnel_key {
> diff --git a/net/core/filter.c b/net/core/filter.c
> index bd0df75dc7b6..3d54af4c363d 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5773,6 +5773,7 @@ static bool sk_filter_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, flow_keys):
> case bpf_ctx_range_till(struct __sk_buff, family, local_port):
> case bpf_ctx_range(struct __sk_buff, tstamp):
> + case bpf_ctx_range(struct __sk_buff, wire_len):
> return false;
> }
>
> @@ -5797,6 +5798,7 @@ static bool cg_skb_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, tc_classid):
> case bpf_ctx_range(struct __sk_buff, data_meta):
> case bpf_ctx_range(struct __sk_buff, flow_keys):
> + case bpf_ctx_range(struct __sk_buff, wire_len):
> return false;
> case bpf_ctx_range(struct __sk_buff, data):
> case bpf_ctx_range(struct __sk_buff, data_end):
> @@ -5843,6 +5845,7 @@ static bool lwt_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, data_meta):
> case bpf_ctx_range(struct __sk_buff, flow_keys):
> case bpf_ctx_range(struct __sk_buff, tstamp):
> + case bpf_ctx_range(struct __sk_buff, wire_len):
> return false;
> }
>
> @@ -6273,6 +6276,7 @@ static bool sk_skb_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, data_meta):
> case bpf_ctx_range(struct __sk_buff, flow_keys):
> case bpf_ctx_range(struct __sk_buff, tstamp):
> + case bpf_ctx_range(struct __sk_buff, wire_len):
> return false;
> }
>
> @@ -6360,6 +6364,7 @@ static bool flow_dissector_is_valid_access(int off, int
> size,
> case bpf_ctx_range(struct __sk_buff, data_meta):
> case bpf_ctx_range_till(struct __sk_buff, family, local_port):
> case bpf_ctx_range(struct __sk_buff, tstamp):
> + case bpf_ctx_range(struct __sk_buff, wire_len):
> return false;
> }
>
> @@ -6685,6 +6690,17 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type
> type,
> bpf_target_off(struct sk_buff,
> tstamp, 8,
> target_size));
> + break;
> +
> + case offsetof(struct __sk_buff, wire_len):
> + BUILD_BUG_ON(FIELD_SIZEOF(struct qdisc_skb_cb, pkt_len) != 4);
> +
> + off = si->off;
> + off -= offsetof(struct __sk_buff, wire_len);
> + off += offsetof(struct sk_buff, cb);
> + off += offsetof(struct qdisc_skb_cb, pkt_len);
> + *target_size = 4;
> + *insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->src_reg, off);
> }
>
> return insn - insn_buf;
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 8050caea7495..0183b8e70a9e 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -2497,6 +2497,7 @@ struct __sk_buff {
> __u32 data_meta;
> struct bpf_flow_keys *f
Re: [PATCH net] bpf: uninitialized variables in test code
On Thu, Nov 29, 2018 at 2:28 AM Dan Carpenter wrote:
>
> Smatch complains that if bpf_test_run() fails with -ENOMEM at the
> begining then the "duration" is uninitialized. We then copy the
> unintialized variables to the user inside the bpf_test_finish()
> function. The functions require CAP_SYS_ADMIN so it's not really an
> information leak.
>
> Fixes: 1cf1cae963c2 ("bpf: introduce BPF_PROG_TEST_RUN command")
> Signed-off-by: Dan Carpenter
Acked-by: Song Liu
> ---
> net/bpf/test_run.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index c89c22c49015..49304192a031 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -114,7 +114,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const
> union bpf_attr *kattr,
> bool is_l2 = false, is_direct_pkt_access = false;
> u32 size = kattr->test.data_size_in;
> u32 repeat = kattr->test.repeat;
> - u32 retval, duration;
> + u32 retval, duration = 0;
> int hh_len = ETH_HLEN;
> struct sk_buff *skb;
> struct sock *sk;
> @@ -196,7 +196,7 @@ int bpf_prog_test_run_xdp(struct bpf_prog *prog, const
> union bpf_attr *kattr,
> u32 repeat = kattr->test.repeat;
> struct netdev_rx_queue *rxqueue;
> struct xdp_buff xdp = {};
> - u32 retval, duration;
> + u32 retval, duration = 0;
> void *data;
> int ret;
>
> --
> 2.11.0
>
Re: [PATCH bpf-next] bpf: allow BPF read access to qdisc pkt_len
On Fri, Nov 30, 2018 at 12:09 PM Willem de Bruijn
wrote:
>
> From: Petar Penkov
>
> The pkt_len field in qdisc_skb_cb stores the skb length as it will
> appear on the wire after segmentation. For byte accounting, this value
> is more accurate than skb->len. It is computed on entry to the TC
> layer, so only valid there.
>
> Allow read access to this field from BPF tc classifier and action
> programs. The implementation is analogous to tc_classid, aside from
> restricting to read access.
>
> Signed-off-by: Petar Penkov
> Signed-off-by: Vlad Dumitrescu
> Signed-off-by: Willem de Bruijn
> ---
> include/uapi/linux/bpf.h| 1 +
> net/core/filter.c | 16 +++
> tools/include/uapi/linux/bpf.h | 1 +
> tools/testing/selftests/bpf/test_verifier.c | 32 +
> 4 files changed, 50 insertions(+)
Please split this into 3 patches:
1 for include/uapi/linux/bpf.h and filter.c
1 for tools/include/uapi/linux/bpf.h
1 for tools/testing/selftests/bpf/test_verifier.c
Other than this
Acked-by: Song Liu
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index 597afdbc1ab9..ce028482d423 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2483,6 +2483,7 @@ struct __sk_buff {
> __u32 data_meta;
> struct bpf_flow_keys *flow_keys;
> __u64 tstamp;
> + __u32 pkt_len;
> };
>
> struct bpf_tunnel_key {
> diff --git a/net/core/filter.c b/net/core/filter.c
> index bd0df75dc7b6..af071ef22c0a 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -5773,6 +5773,7 @@ static bool sk_filter_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, flow_keys):
> case bpf_ctx_range_till(struct __sk_buff, family, local_port):
> case bpf_ctx_range(struct __sk_buff, tstamp):
> + case bpf_ctx_range(struct __sk_buff, pkt_len):
> return false;
> }
>
> @@ -5797,6 +5798,7 @@ static bool cg_skb_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, tc_classid):
> case bpf_ctx_range(struct __sk_buff, data_meta):
> case bpf_ctx_range(struct __sk_buff, flow_keys):
> + case bpf_ctx_range(struct __sk_buff, pkt_len):
> return false;
> case bpf_ctx_range(struct __sk_buff, data):
> case bpf_ctx_range(struct __sk_buff, data_end):
> @@ -5843,6 +5845,7 @@ static bool lwt_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, data_meta):
> case bpf_ctx_range(struct __sk_buff, flow_keys):
> case bpf_ctx_range(struct __sk_buff, tstamp):
> + case bpf_ctx_range(struct __sk_buff, pkt_len):
> return false;
> }
>
> @@ -6273,6 +6276,7 @@ static bool sk_skb_is_valid_access(int off, int size,
> case bpf_ctx_range(struct __sk_buff, data_meta):
> case bpf_ctx_range(struct __sk_buff, flow_keys):
> case bpf_ctx_range(struct __sk_buff, tstamp):
> + case bpf_ctx_range(struct __sk_buff, pkt_len):
> return false;
> }
>
> @@ -6360,6 +6364,7 @@ static bool flow_dissector_is_valid_access(int off, int
> size,
> case bpf_ctx_range(struct __sk_buff, data_meta):
> case bpf_ctx_range_till(struct __sk_buff, family, local_port):
> case bpf_ctx_range(struct __sk_buff, tstamp):
> + case bpf_ctx_range(struct __sk_buff, pkt_len):
> return false;
> }
>
> @@ -6685,6 +6690,17 @@ static u32 bpf_convert_ctx_access(enum bpf_access_type
> type,
> bpf_target_off(struct sk_buff,
> tstamp, 8,
> target_size));
> + break;
> +
> + case offsetof(struct __sk_buff, pkt_len):
> + BUILD_BUG_ON(FIELD_SIZEOF(struct qdisc_skb_cb, pkt_len) != 4);
> +
> + off = si->off;
> + off -= offsetof(struct __sk_buff, pkt_len);
> + off += offsetof(struct sk_buff, cb);
> + off += offsetof(struct qdisc_skb_cb, pkt_len);
> + *target_size = 4;
> + *insn++ = BPF_LDX_MEM(BPF_W, si->dst_reg, si->src_reg, off);
> }
>
> return insn - insn_buf;
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 597afdbc1ab9..ce028482d423 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -2483,6 +2483,7 @@ struct __sk_buff {
> __u32 data_meta;
>
[PATCH bpf-next v2] tools/bpf: make libbpf _GNU_SOURCE friendly
During porting libbpf to bcc, I got some warnings like below: ... [ 2%] Building C object src/cc/CMakeFiles/bpf-shared.dir/libbpf/src/libbpf.c.o /home/yhs/work/bcc2/src/cc/libbpf/src/libbpf.c:12:0: warning: "_GNU_SOURCE" redefined [enabled by default] #define _GNU_SOURCE ... [ 3%] Building C object src/cc/CMakeFiles/bpf-shared.dir/libbpf/src/libbpf_errno.c.o /home/yhs/work/bcc2/src/cc/libbpf/src/libbpf_errno.c: In function ‘libbpf_strerror’: /home/yhs/work/bcc2/src/cc/libbpf/src/libbpf_errno.c:45:7: warning: assignment makes integer from pointer without a cast [enabled by default] ret = strerror_r(err, buf, size); ... bcc is built with _GNU_SOURCE defined and this caused the above warning. This patch intends to make libpf _GNU_SOURCE friendly by . define _GNU_SOURCE in libbpf.c unless it is not defined . undefine _GNU_SOURCE as non-gnu version of strerror_r is expected. Signed-off-by: Yonghong Song --- tools/lib/bpf/libbpf.c | 2 ++ tools/lib/bpf/libbpf_errno.c | 1 + 2 files changed, 3 insertions(+) Changelog: v1 -> v2: . undef _GNU_SOURCE instead of multiversioning strerror_r, suggested by Jakub. diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c index ed4212a4c5f9..59b748ebd15f 100644 --- a/tools/lib/bpf/libbpf.c +++ b/tools/lib/bpf/libbpf.c @@ -9,7 +9,9 @@ * Copyright (C) 2017 Nicira, Inc. */ +#ifndef _GNU_SOURCE #define _GNU_SOURCE +#endif #include #include #include diff --git a/tools/lib/bpf/libbpf_errno.c b/tools/lib/bpf/libbpf_errno.c index d83b17f8435c..4343e40588c6 100644 --- a/tools/lib/bpf/libbpf_errno.c +++ b/tools/lib/bpf/libbpf_errno.c @@ -7,6 +7,7 @@ * Copyright (C) 2017 Nicira, Inc. */ +#undef _GNU_SOURCE #include #include -- 2.17.1
Re: [PATCH bpf-next] tools/bpf: make libbpf _GNU_SOURCE friendly
On 11/29/18 1:00 PM, Jakub Kicinski wrote:
> On Thu, 29 Nov 2018 12:38:03 -0800, Yonghong Song wrote:
>> diff --git a/tools/lib/bpf/libbpf_errno.c b/tools/lib/bpf/libbpf_errno.c
>> index d83b17f8435c..286e497c50ec 100644
>> --- a/tools/lib/bpf/libbpf_errno.c
>> +++ b/tools/lib/bpf/libbpf_errno.c
>> @@ -40,9 +40,19 @@ int libbpf_strerror(int err, char *buf, size_t size)
>> err = err > 0 ? err : -err;
>>
>> if (err < __LIBBPF_ERRNO__START) {
>> +#ifdef _GNU_SOURCE
>> +const char *ret_buf;
>> +#endif
>> int ret;
>>
>> +#ifdef _GNU_SOURCE
>> +ret_buf = strerror_r(err, buf, size);
>> +if (ret_buf != buf)
>> +snprintf(buf, size, "%s", ret_buf);
>> +ret = 0;
>> +#else
>> ret = strerror_r(err, buf, size);
>> +#endif
>> buf[size - 1] = '\0';
>> return ret;
>> }
>
> That is kinda strange, the whole point for this file was to have
> non-GNU strerror_r, would doing #undef _GNU_SOURCE at the top not work?
Thanks for suggestion as I did not the history of this file.
Yes, #undef _GNU_SOURCE works. Will send a revision shortly.
[PATCH bpf-next] tools/bpf: make libbpf _GNU_SOURCE friendly
During porting libbpf to bcc, I got some warnings like below:
...
[ 2%] Building C object
src/cc/CMakeFiles/bpf-shared.dir/libbpf/src/libbpf.c.o
/home/yhs/work/bcc2/src/cc/libbpf/src/libbpf.c:12:0:
warning: "_GNU_SOURCE" redefined [enabled by default]
#define _GNU_SOURCE
...
[ 3%] Building C object
src/cc/CMakeFiles/bpf-shared.dir/libbpf/src/libbpf_errno.c.o
/home/yhs/work/bcc2/src/cc/libbpf/src/libbpf_errno.c: In function
‘libbpf_strerror’:
/home/yhs/work/bcc2/src/cc/libbpf/src/libbpf_errno.c:45:7:
warning: assignment makes integer from pointer without a cast [enabled by
default]
ret = strerror_r(err, buf, size);
...
bcc is built with _GNU_SOURCE defined and this caused the above warning.
This patch intends to make libpf _GNU_SOURCE friendly by
. define _GNU_SOURCE unless it is not defined
. gnu version strerror_r has different return value than non-gnu version,
so strerror_r return value is handled differently if _GNU_SOURCE is defined.
Signed-off-by: Yonghong Song
---
tools/lib/bpf/libbpf.c | 2 ++
tools/lib/bpf/libbpf_errno.c | 10 ++
2 files changed, 12 insertions(+)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index ed4212a4c5f9..59b748ebd15f 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -9,7 +9,9 @@
* Copyright (C) 2017 Nicira, Inc.
*/
+#ifndef _GNU_SOURCE
#define _GNU_SOURCE
+#endif
#include
#include
#include
diff --git a/tools/lib/bpf/libbpf_errno.c b/tools/lib/bpf/libbpf_errno.c
index d83b17f8435c..286e497c50ec 100644
--- a/tools/lib/bpf/libbpf_errno.c
+++ b/tools/lib/bpf/libbpf_errno.c
@@ -40,9 +40,19 @@ int libbpf_strerror(int err, char *buf, size_t size)
err = err > 0 ? err : -err;
if (err < __LIBBPF_ERRNO__START) {
+#ifdef _GNU_SOURCE
+ const char *ret_buf;
+#endif
int ret;
+#ifdef _GNU_SOURCE
+ ret_buf = strerror_r(err, buf, size);
+ if (ret_buf != buf)
+ snprintf(buf, size, "%s", ret_buf);
+ ret = 0;
+#else
ret = strerror_r(err, buf, size);
+#endif
buf[size - 1] = '\0';
return ret;
}
--
2.17.1
Re: [PATCH] bpf: Fix various lib and testsuite build failures on 32-bit.
On Wed, Nov 28, 2018 at 12:59 PM David Miller wrote:
>
>
> Cannot cast a u64 to a pointer on 32-bit without an intervening (long)
> cast otherwise GCC warns.
>
> Signed-off-by: David S. Miller
Acked-by: Song Liu
> --
>
> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
> index eadcf8d..c2d641f 100644
> --- a/tools/lib/bpf/btf.c
> +++ b/tools/lib/bpf/btf.c
> @@ -466,7 +466,7 @@ int btf__get_from_id(__u32 id, struct btf **btf)
> goto exit_free;
> }
>
> - *btf = btf__new((__u8 *)btf_info.btf, btf_info.btf_size, NULL);
> + *btf = btf__new((__u8 *)(long)btf_info.btf, btf_info.btf_size, NULL);
> if (IS_ERR(*btf)) {
> err = PTR_ERR(*btf);
> *btf = NULL;
> diff --git a/tools/testing/selftests/bpf/test_progs.c
> b/tools/testing/selftests/bpf/test_progs.c
> index c1e688f6..1c57abb 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -524,7 +524,7 @@ static void test_bpf_obj_id(void)
> load_time < now - 60 || load_time > now + 60 ||
> prog_infos[i].created_by_uid != my_uid ||
> prog_infos[i].nr_map_ids != 1 ||
> - *(int *)prog_infos[i].map_ids != map_infos[i].id ||
> + *(int *)(long)prog_infos[i].map_ids !=
> map_infos[i].id ||
> strcmp((char *)prog_infos[i].name,
> expected_prog_name),
> "get-prog-info(fd)",
> "err %d errno %d i %d type %d(%d) info_len %u(%Zu)
> jit_enabled %d jited_prog_len %u xlated_prog_len %u jited_prog %d xlated_prog
> %d load_time %lu(%lu) uid %u(%u) nr_map_ids %u(%u) map_id %u(%u) name
> %s(%s)\n",
> @@ -539,7 +539,7 @@ static void test_bpf_obj_id(void)
> load_time, now,
> prog_infos[i].created_by_uid, my_uid,
> prog_infos[i].nr_map_ids, 1,
> - *(int *)prog_infos[i].map_ids, map_infos[i].id,
> + *(int *)(long)prog_infos[i].map_ids,
> map_infos[i].id,
> prog_infos[i].name, expected_prog_name))
> goto done;
> }
> @@ -585,7 +585,7 @@ static void test_bpf_obj_id(void)
> bzero(_info, sizeof(prog_info));
> info_len = sizeof(prog_info);
>
> - saved_map_id = *(int *)(prog_infos[i].map_ids);
> + saved_map_id = *(int *)((long)prog_infos[i].map_ids);
> prog_info.map_ids = prog_infos[i].map_ids;
> prog_info.nr_map_ids = 2;
> err = bpf_obj_get_info_by_fd(prog_fd, _info, _len);
> @@ -593,12 +593,12 @@ static void test_bpf_obj_id(void)
> prog_infos[i].xlated_prog_insns = 0;
> CHECK(err || info_len != sizeof(struct bpf_prog_info) ||
> memcmp(_info, _infos[i], info_len) ||
> - *(int *)prog_info.map_ids != saved_map_id,
> + *(int *)(long)prog_info.map_ids != saved_map_id,
> "get-prog-info(next_id->fd)",
> "err %d errno %d info_len %u(%Zu) memcmp %d map_id
> %u(%u)\n",
> err, errno, info_len, sizeof(struct bpf_prog_info),
> memcmp(_info, _infos[i], info_len),
> - *(int *)prog_info.map_ids, saved_map_id);
> + *(int *)(long)prog_info.map_ids, saved_map_id);
> close(prog_fd);
> }
> CHECK(nr_id_found != nr_iters,
Re: [PATCH bpf] tools: bpftool: fix a bitfield pretty print issue
On Wed, Nov 28, 2018 at 10:09 AM Yonghong Song wrote:
>
> Commit b12d6ec09730 ("bpf: btf: add btf print functionality")
> added btf pretty print functionality to bpftool.
> There is a problem though in printing a bitfield whose type
> has modifiers.
>
> For example, for a type like
> typedef int ___int;
> struct tmp_t {
> int a:3;
> ___int b:3;
> };
> Suppose we have a map
> struct bpf_map_def SEC("maps") tmpmap = {
> .type = BPF_MAP_TYPE_HASH,
> .key_size = sizeof(__u32),
> .value_size = sizeof(struct tmp_t),
> .max_entries = 1,
> };
> and the hash table is populated with one element with
> key 0 and value (.a = 1 and .b = 2).
>
> In BTF, the struct member "b" will have a type "typedef" which
> points to an int type. The current implementation does not
> pass the bit offset during transition from typedef to int type,
> hence incorrectly print the value as
> $ bpftool m d id 79
> [{
> "key": 0,
> "value": {
> "a": 0x1,
> "b": 0x1
> }
> }
> ]
>
> This patch fixed the issue by carrying bit_offset along the type
> chain during bit_field print. The correct result can be printed as
> $ bpftool m d id 76
> [{
> "key": 0,
> "value": {
> "a": 0x1,
> "b": 0x2
> }
> }
> ]
>
> The kernel pretty print is implemented correctly and does not
> have this issue.
>
> Fixes: b12d6ec09730 ("bpf: btf: add btf print functionality")
> Signed-off-by: Yonghong Song
Acked-by: Song Liu
> ---
> tools/bpf/bpftool/btf_dumper.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
> index 55bc512a1831..e4e6e2b3fd84 100644
> --- a/tools/bpf/bpftool/btf_dumper.c
> +++ b/tools/bpf/bpftool/btf_dumper.c
> @@ -32,7 +32,7 @@ static void btf_dumper_ptr(const void *data, json_writer_t
> *jw,
> }
>
> static int btf_dumper_modifier(const struct btf_dumper *d, __u32 type_id,
> - const void *data)
> + __u8 bit_offset, const void *data)
> {
> int actual_type_id;
>
> @@ -40,7 +40,7 @@ static int btf_dumper_modifier(const struct btf_dumper *d,
> __u32 type_id,
> if (actual_type_id < 0)
> return actual_type_id;
>
> - return btf_dumper_do_type(d, actual_type_id, 0, data);
> + return btf_dumper_do_type(d, actual_type_id, bit_offset, data);
> }
>
> static void btf_dumper_enum(const void *data, json_writer_t *jw)
> @@ -237,7 +237,7 @@ static int btf_dumper_do_type(const struct btf_dumper *d,
> __u32 type_id,
> case BTF_KIND_VOLATILE:
> case BTF_KIND_CONST:
> case BTF_KIND_RESTRICT:
> - return btf_dumper_modifier(d, type_id, data);
> + return btf_dumper_modifier(d, type_id, bit_offset, data);
> default:
> jsonw_printf(d->jw, "(unsupported-kind");
> return -EINVAL;
> --
> 2.17.1
>
[PATCH bpf] tools: bpftool: fix a bitfield pretty print issue
Commit b12d6ec09730 ("bpf: btf: add btf print functionality")
added btf pretty print functionality to bpftool.
There is a problem though in printing a bitfield whose type
has modifiers.
For example, for a type like
typedef int ___int;
struct tmp_t {
int a:3;
___int b:3;
};
Suppose we have a map
struct bpf_map_def SEC("maps") tmpmap = {
.type = BPF_MAP_TYPE_HASH,
.key_size = sizeof(__u32),
.value_size = sizeof(struct tmp_t),
.max_entries = 1,
};
and the hash table is populated with one element with
key 0 and value (.a = 1 and .b = 2).
In BTF, the struct member "b" will have a type "typedef" which
points to an int type. The current implementation does not
pass the bit offset during transition from typedef to int type,
hence incorrectly print the value as
$ bpftool m d id 79
[{
"key": 0,
"value": {
"a": 0x1,
"b": 0x1
}
}
]
This patch fixed the issue by carrying bit_offset along the type
chain during bit_field print. The correct result can be printed as
$ bpftool m d id 76
[{
"key": 0,
"value": {
"a": 0x1,
"b": 0x2
}
}
]
The kernel pretty print is implemented correctly and does not
have this issue.
Fixes: b12d6ec09730 ("bpf: btf: add btf print functionality")
Signed-off-by: Yonghong Song
---
tools/bpf/bpftool/btf_dumper.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
index 55bc512a1831..e4e6e2b3fd84 100644
--- a/tools/bpf/bpftool/btf_dumper.c
+++ b/tools/bpf/bpftool/btf_dumper.c
@@ -32,7 +32,7 @@ static void btf_dumper_ptr(const void *data, json_writer_t
*jw,
}
static int btf_dumper_modifier(const struct btf_dumper *d, __u32 type_id,
- const void *data)
+ __u8 bit_offset, const void *data)
{
int actual_type_id;
@@ -40,7 +40,7 @@ static int btf_dumper_modifier(const struct btf_dumper *d,
__u32 type_id,
if (actual_type_id < 0)
return actual_type_id;
- return btf_dumper_do_type(d, actual_type_id, 0, data);
+ return btf_dumper_do_type(d, actual_type_id, bit_offset, data);
}
static void btf_dumper_enum(const void *data, json_writer_t *jw)
@@ -237,7 +237,7 @@ static int btf_dumper_do_type(const struct btf_dumper *d,
__u32 type_id,
case BTF_KIND_VOLATILE:
case BTF_KIND_CONST:
case BTF_KIND_RESTRICT:
- return btf_dumper_modifier(d, type_id, data);
+ return btf_dumper_modifier(d, type_id, bit_offset, data);
default:
jsonw_printf(d->jw, "(unsupported-kind");
return -EINVAL;
--
2.17.1
[PATCH bpf v2 1/4] bpf: btf: implement btf_name_valid_identifier()
Function btf_name_valid_identifier() have been implemented in
bpf-next commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC and
BTF_KIND_FUNC_PROTO"). Backport this function so later patch
can use it.
Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)")
Signed-off-by: Yonghong Song
---
kernel/bpf/btf.c | 25 +
1 file changed, 25 insertions(+)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index ee4c82667d65..93c233ab2db6 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5,6 +5,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -426,6 +427,30 @@ static bool btf_name_offset_valid(const struct btf *btf,
u32 offset)
offset < btf->hdr.str_len;
}
+/* Only C-style identifier is permitted. This can be relaxed if
+ * necessary.
+ */
+static bool btf_name_valid_identifier(const struct btf *btf, u32 offset)
+{
+ /* offset must be valid */
+ const char *src = >strings[offset];
+ const char *src_limit;
+
+ if (!isalpha(*src) && *src != '_')
+ return false;
+
+ /* set a limit on identifier length */
+ src_limit = src + KSYM_NAME_LEN;
+ src++;
+ while (*src && src < src_limit) {
+ if (!isalnum(*src) && *src != '_')
+ return false;
+ src++;
+ }
+
+ return !*src;
+}
+
static const char *btf_name_by_offset(const struct btf *btf, u32 offset)
{
if (!offset)
--
2.17.1
[PATCH bpf v2 3/4] tools/bpf: fix two test_btf unit test cases
From: Martin KaFai Lau
There are two unit test cases, which should encode
TYPEDEF type, but instead encode PTR type.
The error is flagged out after enforcing name
checking in the previous patch.
Fixes: c0fa1b6c3efc ("bpf: btf: Add BTF tests")
Signed-off-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
tools/testing/selftests/bpf/test_btf.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_btf.c
b/tools/testing/selftests/bpf/test_btf.c
index f42b3396d622..b361bb851829 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -432,11 +432,11 @@ static struct btf_raw_test raw_tests[] = {
/* const void* */ /* [3] */
BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 2),
/* typedef const void * const_void_ptr */
- BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 3),
- /* struct A { *//* [4] */
+ BTF_TYPEDEF_ENC(NAME_TBD, 3), /* [4] */
+ /* struct A { *//* [5] */
BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 1),
sizeof(void *)),
/* const_void_ptr m; */
- BTF_MEMBER_ENC(NAME_TBD, 3, 0),
+ BTF_MEMBER_ENC(NAME_TBD, 4, 0),
/* } */
BTF_END_RAW,
},
@@ -494,10 +494,10 @@ static struct btf_raw_test raw_tests[] = {
BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 0),
/* const void* */ /* [3] */
BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 2),
- /* typedef const void * const_void_ptr */ /* [4] */
- BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 3),
- /* const_void_ptr[4] */ /* [5] */
- BTF_TYPE_ARRAY_ENC(3, 1, 4),
+ /* typedef const void * const_void_ptr */
+ BTF_TYPEDEF_ENC(NAME_TBD, 3), /* [4] */
+ /* const_void_ptr[4] */
+ BTF_TYPE_ARRAY_ENC(4, 1, 4),/* [5] */
BTF_END_RAW,
},
.str_sec = "\0const_void_ptr",
--
2.17.1
[PATCH bpf v2 0/4] bpf: btf: check name validity for various types
This patch set added name checking for PTR, ARRAY, VOLATILE, TYPEDEF,
CONST, RESTRICT, STRUCT, UNION, ENUM and FWD types. Such a strict
name checking makes BTF more sound in the kernel and future
BTF-to-header-file converesion ([1]) less fragile.
Patch #1 implemented btf_name_valid_identifier() for name checking
which will be used in Patch #2.
Patch #2 checked name validity for the above mentioned types.
Patch #3 fixed two existing test_btf unit tests exposed by the strict
name checking.
Patch #4 added additional test cases.
This patch set is against bpf tree.
Patch #1 has been implemented in bpf-next commit
Commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC
and BTF_KIND_FUNC_PROTO"), so there is no need to apply this
patch to bpf-next. In case this patch is applied to bpf-next,
there will be a minor conflict like
diff --cc kernel/bpf/btf.c
index a09b2f94ab25,93c233ab2db6..
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@@ -474,7 -451,7 +474,11 @@@ static bool btf_name_valid_identifier(c
return !*src;
}
++<<<<<<< HEAD
+const char *btf_name_by_offset(const struct btf *btf, u32 offset)
++===
+ static const char *btf_name_by_offset(const struct btf *btf, u32 offset)
++>>>>>>> fa9566b0847d... bpf: btf: implement btf_name_valid_identifier()
{
if (!offset)
return "(anon)";
Just resolve the conflict by taking the "const char ..." line.
Patches #2, #3 and #4 can be applied to bpf-next without conflict.
[1]: http://vger.kernel.org/lpc-bpf2018.html#session-2
Yonghong Song (4):
bpf: btf: implement btf_name_valid_identifier()
bpf: btf: check name validity for various types
tools/bpf: fix two test_btf unit test cases
tools/bpf: add addition type tests to test_btf
kernel/bpf/btf.c | 82 ++
tools/testing/selftests/bpf/test_btf.c | 375 -
2 files changed, 450 insertions(+), 7 deletions(-)
--
2.17.1
[PATCH bpf v2 4/4] tools/bpf: add addition type tests to test_btf
The following additional unit testcases are added to test_btf:
...
BTF raw test[42] (typedef (invalid name, name_off = 0)): OK
BTF raw test[43] (typedef (invalid name, invalid identifier)): OK
BTF raw test[44] (ptr type (invalid name, name_off <> 0)): OK
BTF raw test[45] (volatile type (invalid name, name_off <> 0)): OK
BTF raw test[46] (const type (invalid name, name_off <> 0)): OK
BTF raw test[47] (restrict type (invalid name, name_off <> 0)): OK
BTF raw test[48] (fwd type (invalid name, name_off = 0)): OK
BTF raw test[49] (fwd type (invalid name, invalid identifier)): OK
BTF raw test[50] (array type (invalid name, name_off <> 0)): OK
BTF raw test[51] (struct type (name_off = 0)): OK
BTF raw test[52] (struct type (invalid name, invalid identifier)): OK
BTF raw test[53] (struct member (name_off = 0)): OK
BTF raw test[54] (struct member (invalid name, invalid identifier)): OK
BTF raw test[55] (enum type (name_off = 0)): OK
BTF raw test[56] (enum type (invalid name, invalid identifier)): OK
BTF raw test[57] (enum member (invalid name, name_off = 0)): OK
BTF raw test[58] (enum member (invalid name, invalid identifier)): OK
...
Fixes: c0fa1b6c3efc ("bpf: btf: Add BTF tests")
Acked-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
tools/testing/selftests/bpf/test_btf.c | 361 +
1 file changed, 361 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_btf.c
b/tools/testing/selftests/bpf/test_btf.c
index b361bb851829..38e1cbaaffdb 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -1292,6 +1292,367 @@ static struct btf_raw_test raw_tests[] = {
.err_str = "type != 0",
},
+{
+ .descr = "typedef (invalid name, name_off = 0)",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_TYPEDEF_ENC(0, 1), /* [2] */
+ BTF_END_RAW,
+ },
+ .str_sec = "\0__int",
+ .str_sec_size = sizeof("\0__int"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "typedef_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 4,
+ .btf_load_err = true,
+ .err_str = "Invalid name",
+},
+
+{
+ .descr = "typedef (invalid name, invalid identifier)",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_TYPEDEF_ENC(NAME_TBD, 1), /* [2] */
+ BTF_END_RAW,
+ },
+ .str_sec = "\0__!int",
+ .str_sec_size = sizeof("\0__!int"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "typedef_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 4,
+ .btf_load_err = true,
+ .err_str = "Invalid name",
+},
+
+{
+ .descr = "ptr type (invalid name, name_off <> 0)",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1]
*/
+ BTF_TYPE_ENC(NAME_TBD,
+BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 1), /* [2]
*/
+ BTF_END_RAW,
+ },
+ .str_sec = "\0__int",
+ .str_sec_size = sizeof("\0__int"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "ptr_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 4,
+ .btf_load_err = true,
+ .err_str = "Invalid name",
+},
+
+{
+ .descr = "volatile type (invalid name, name_off <> 0)",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1]
*/
+ BTF_TYPE_ENC(NAME_TBD,
+BTF_INFO_ENC(BTF_KIND_VOLATILE, 0, 0), 1), /* [2]
*/
+ BTF_END_RAW,
+ },
+ .str_sec = "\0__int",
+ .str_sec_size = sizeof("\0__int"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "volatile_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 4,
+ .btf_load_err = true,
+ .err_str = "Invalid name",
+},
+
+{
+ .descr = "const type (invalid name, name_off <> 0)",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1]
*/
+ BTF_TYPE_ENC(NAME_TBD,
+BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 1),/* [2]
*/
+
[PATCH bpf v2 2/4] bpf: btf: check name validity for various types
This patch added name checking for the following types:
. BTF_KIND_PTR, BTF_KIND_ARRAY, BTF_KIND_VOLATILE,
BTF_KIND_CONST, BTF_KIND_RESTRICT:
the name must be null
. BTF_KIND_STRUCT, BTF_KIND_UNION: the struct/member name
is either null or a valid identifier
. BTF_KIND_ENUM: the enum type name is either null or a valid
identifier; the enumerator name must be a valid identifier.
. BTF_KIND_FWD: the name must be a valid identifier
. BTF_KIND_TYPEDEF: the name must be a valid identifier
For those places a valid name is required, the name must be
a valid C identifier. This can be relaxed later if we found
use cases for a different (non-C) frontend.
Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)")
Acked-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
kernel/bpf/btf.c | 57
1 file changed, 57 insertions(+)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 93c233ab2db6..4da543d6bea2 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -1168,6 +1168,22 @@ static int btf_ref_type_check_meta(struct
btf_verifier_env *env,
return -EINVAL;
}
+ /* typedef type must have a valid name, and other ref types,
+* volatile, const, restrict, should have a null name.
+*/
+ if (BTF_INFO_KIND(t->info) == BTF_KIND_TYPEDEF) {
+ if (!t->name_off ||
+ !btf_name_valid_identifier(env->btf, t->name_off)) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+ } else {
+ if (t->name_off) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+ }
+
btf_verifier_log_type(env, t, NULL);
return 0;
@@ -1325,6 +1341,13 @@ static s32 btf_fwd_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* fwd type must have a valid name */
+ if (!t->name_off ||
+ !btf_name_valid_identifier(env->btf, t->name_off)) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+
btf_verifier_log_type(env, t, NULL);
return 0;
@@ -1381,6 +1404,12 @@ static s32 btf_array_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* array type should not have a name */
+ if (t->name_off) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+
if (btf_type_vlen(t)) {
btf_verifier_log_type(env, t, "vlen != 0");
return -EINVAL;
@@ -1557,6 +1586,13 @@ static s32 btf_struct_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* struct type either no name or a valid one */
+ if (t->name_off &&
+ !btf_name_valid_identifier(env->btf, t->name_off)) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+
btf_verifier_log_type(env, t, NULL);
last_offset = 0;
@@ -1568,6 +1604,12 @@ static s32 btf_struct_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* struct member either no name or a valid one */
+ if (member->name_off &&
+ !btf_name_valid_identifier(btf, member->name_off)) {
+ btf_verifier_log_member(env, t, member, "Invalid name");
+ return -EINVAL;
+ }
/* A member cannot be in type void */
if (!member->type || !BTF_TYPE_ID_VALID(member->type)) {
btf_verifier_log_member(env, t, member,
@@ -1755,6 +1797,13 @@ static s32 btf_enum_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* enum type either no name or a valid one */
+ if (t->name_off &&
+ !btf_name_valid_identifier(env->btf, t->name_off)) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+
btf_verifier_log_type(env, t, NULL);
for (i = 0; i < nr_enums; i++) {
@@ -1764,6 +1813,14 @@ static s32 btf_enum_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* enum member must have a valid name */
+ if (!enums[i].name_off ||
+ !btf_name_valid_identifier(btf, enums[i].name_off)) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+
+
btf_verifier_log(env, "\t%s val=%d\n",
btf_name_by_offset(btf, enums[i].name_off),
enums[i].val);
--
2.17.1
Re: [PATCH bpf-next 1/3] bpf: btf: check name validity for various types
On 11/27/18 11:02 AM, Alexei Starovoitov wrote:
> On Mon, Nov 26, 2018 at 09:17:13PM -0800, Yonghong Song wrote:
>> Commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC
>> and BTF_KIND_FUNC_PROTO") checked the name validity
>> for BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO types such that:
>> . BTF_KIND_FUNC must have a valid identifier name
>> . BTF_KIND_PROTO must have a null name
>> . The argument name of BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO,
>> if not null, must be a valid identifier.
>>
>> This patch added name checking for the following types:
>> . BTF_KIND_PTR, BTF_KIND_ARRAY, BTF_KIND_VOLATILE,
>> BTF_KIND_CONST, BTF_KIND_RESTRICT:
>> the name must be null
>> . BTF_KIND_STRUCT, BTF_KIND_UNION: the struct/member name
>> is either null or a valid identifier
>> . BTF_KIND_ENUM: the enum type name is either null or a valid
>> identifier; the enumerator name must be a valid identifier.
>> . BTF_KIND_FWD: the name must be a valid identifier
>> . BTF_KIND_TYPEDEF: the name must be a valid identifier
>>
>> For those places a valid name is required, the name must be
>> a valid C identifier. This can be relaxed later if we found
>> use cases for a different (non-C) frontend.
>>
>> Acked-by: Martin KaFai Lau
>> Signed-off-by: Yonghong Song
> ...
>> return 0;
>> @@ -1409,6 +1432,12 @@ static s32 btf_array_check_meta(struct
>> btf_verifier_env *env,
>> return -EINVAL;
>> }
>>
>> +/* array type should not have a name */
>> +if (t->name_off) {
>> +btf_verifier_log_type(env, t, "Invalid name");
>> +return -EINVAL;
>> +}
>> +
>> if (btf_type_vlen(t)) {
>> btf_verifier_log_type(env, t, "vlen != 0");
>> return -EINVAL;
>> @@ -1585,6 +1614,13 @@ static s32 btf_struct_check_meta(struct
>> btf_verifier_env *env,
>> return -EINVAL;
>> }
>>
>> +/* struct type either no name or a valid one */
>> +if (t->name_off &&
>> +!btf_name_valid_identifier(env->btf, t->name_off)) {
>
> Looks like some of these changes need to go into bpf tree.
> please split it up and let's try to minimize the conflicts between bpf and
> bpf-next
Make sense. Will restructure and resubmit for bpf in the next version.
> Thanks!
>
Re: [PATCH] selftests/bpf: add config fragment CONFIG_FTRACE_SYSCALLS
On 11/27/18 7:24 AM, Naresh Kamboju wrote: > CONFIG_FTRACE_SYSCALLS=y is required for get_cgroup_id_user test case > this test reads a file from debug trace path > /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id > > Signed-off-by: Naresh Kamboju Acked-by: Yonghong Song > --- > tools/testing/selftests/bpf/config | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/tools/testing/selftests/bpf/config > b/tools/testing/selftests/bpf/config > index 7f90d3645af8..37f947ec44ed 100644 > --- a/tools/testing/selftests/bpf/config > +++ b/tools/testing/selftests/bpf/config > @@ -22,3 +22,4 @@ CONFIG_NET_CLS_FLOWER=m > CONFIG_LWTUNNEL=y > CONFIG_BPF_STREAM_PARSER=y > CONFIG_XDP_SOCKETS=y > +CONFIG_FTRACE_SYSCALLS=y >
[PATCH bpf-next 1/3] bpf: btf: check name validity for various types
Commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC
and BTF_KIND_FUNC_PROTO") checked the name validity
for BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO types such that:
. BTF_KIND_FUNC must have a valid identifier name
. BTF_KIND_PROTO must have a null name
. The argument name of BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO,
if not null, must be a valid identifier.
This patch added name checking for the following types:
. BTF_KIND_PTR, BTF_KIND_ARRAY, BTF_KIND_VOLATILE,
BTF_KIND_CONST, BTF_KIND_RESTRICT:
the name must be null
. BTF_KIND_STRUCT, BTF_KIND_UNION: the struct/member name
is either null or a valid identifier
. BTF_KIND_ENUM: the enum type name is either null or a valid
identifier; the enumerator name must be a valid identifier.
. BTF_KIND_FWD: the name must be a valid identifier
. BTF_KIND_TYPEDEF: the name must be a valid identifier
For those places a valid name is required, the name must be
a valid C identifier. This can be relaxed later if we found
use cases for a different (non-C) frontend.
Acked-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
kernel/bpf/btf.c | 57
1 file changed, 57 insertions(+)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index a09b2f94ab25..793acba40b4c 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -1195,6 +1195,22 @@ static int btf_ref_type_check_meta(struct
btf_verifier_env *env,
return -EINVAL;
}
+ /* typedef type must have a valid name, and other ref types,
+* volatile, const, restrict, should have a null name.
+*/
+ if (BTF_INFO_KIND(t->info) == BTF_KIND_TYPEDEF) {
+ if (!t->name_off ||
+ !btf_name_valid_identifier(env->btf, t->name_off)) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+ } else {
+ if (t->name_off) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+ }
+
btf_verifier_log_type(env, t, NULL);
return 0;
@@ -1353,6 +1369,13 @@ static s32 btf_fwd_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* fwd type must have a valid name */
+ if (!t->name_off ||
+ !btf_name_valid_identifier(env->btf, t->name_off)) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+
btf_verifier_log_type(env, t, NULL);
return 0;
@@ -1409,6 +1432,12 @@ static s32 btf_array_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* array type should not have a name */
+ if (t->name_off) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+
if (btf_type_vlen(t)) {
btf_verifier_log_type(env, t, "vlen != 0");
return -EINVAL;
@@ -1585,6 +1614,13 @@ static s32 btf_struct_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* struct type either no name or a valid one */
+ if (t->name_off &&
+ !btf_name_valid_identifier(env->btf, t->name_off)) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+
btf_verifier_log_type(env, t, NULL);
last_offset = 0;
@@ -1596,6 +1632,12 @@ static s32 btf_struct_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* struct member either no name or a valid one */
+ if (member->name_off &&
+ !btf_name_valid_identifier(btf, member->name_off)) {
+ btf_verifier_log_member(env, t, member, "Invalid name");
+ return -EINVAL;
+ }
/* A member cannot be in type void */
if (!member->type || !BTF_TYPE_ID_VALID(member->type)) {
btf_verifier_log_member(env, t, member,
@@ -1783,6 +1825,13 @@ static s32 btf_enum_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* enum type either no name or a valid one */
+ if (t->name_off &&
+ !btf_name_valid_identifier(env->btf, t->name_off)) {
+ btf_verifier_log_type(env, t, "Invalid name");
+ return -EINVAL;
+ }
+
btf_verifier_log_type(env, t, NULL);
for (i = 0; i < nr_enums; i++) {
@@ -1792,6 +1841,14 @@ static s32 btf_enum_check_meta(struct btf_verifier_env
*env,
return -EINVAL;
}
+ /* enum member must have a valid na
[PATCH bpf-next 2/3] tools/bpf: fix two test_btf unit test cases
From: Martin KaFai Lau
There are two unit test cases, which should encode
TYPEDEF type, but instead encode PTR type.
The error is flagged out after enforcing name
checking in the previous patch.
Fixes: c0fa1b6c3efc ("bpf: btf: Add BTF tests")
Signed-off-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
tools/testing/selftests/bpf/test_btf.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_btf.c
b/tools/testing/selftests/bpf/test_btf.c
index bae7308b7ec5..99a92923e3f9 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -447,13 +447,13 @@ static struct btf_raw_test raw_tests[] = {
/* const void *//* [2] */
BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 0),
/* const void* */ /* [3] */
- BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 2),
+ BTF_PTR_ENC(2),
/* typedef const void * const_void_ptr */
- BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 3),
- /* struct A { *//* [4] */
+ BTF_TYPEDEF_ENC(NAME_TBD, 3), /* [4] */
+ /* struct A { *//* [5] */
BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_STRUCT, 0, 1),
sizeof(void *)),
/* const_void_ptr m; */
- BTF_MEMBER_ENC(NAME_TBD, 3, 0),
+ BTF_MEMBER_ENC(NAME_TBD, 4, 0),
/* } */
BTF_END_RAW,
},
@@ -510,11 +510,11 @@ static struct btf_raw_test raw_tests[] = {
/* const void *//* [2] */
BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 0),
/* const void* */ /* [3] */
- BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 2),
- /* typedef const void * const_void_ptr */ /* [4] */
- BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 3),
- /* const_void_ptr[4] */ /* [5] */
- BTF_TYPE_ARRAY_ENC(3, 1, 4),
+ BTF_PTR_ENC(2),
+ /* typedef const void * const_void_ptr */
+ BTF_TYPEDEF_ENC(NAME_TBD, 3), /* [4] */
+ /* const_void_ptr[4] */
+ BTF_TYPE_ARRAY_ENC(4, 1, 4),/* [5] */
BTF_END_RAW,
},
.str_sec = "\0const_void_ptr",
--
2.17.1
[PATCH bpf-next 3/3] tools/bpf: add addition type tests to test_btf
The following additional unit testcases are added to test_btf:
BTF raw test[65] (typedef (invalid name, name_off = 0)): OK
BTF raw test[66] (typedef (invalid name, invalid identifier)): OK
BTF raw test[67] (ptr type (invalid name, name_off <> 0)): OK
BTF raw test[68] (volatile type (invalid name, name_off <> 0)): OK
BTF raw test[69] (const type (invalid name, name_off <> 0)): OK
BTF raw test[70] (restrict type (invalid name, name_off <> 0)): OK
BTF raw test[71] (fwd type (invalid name, name_off = 0)): OK
BTF raw test[72] (fwd type (invalid name, invalid identifier)): OK
BTF raw test[73] (array type (invalid name, name_off <> 0)): OK
BTF raw test[74] (struct type (name_off = 0)): OK
BTF raw test[75] (struct type (invalid name, invalid identifier)): OK
BTF raw test[76] (struct member (name_off = 0)): OK
BTF raw test[77] (struct member (invalid name, invalid identifier)): OK
BTF raw test[78] (enum type (name_off = 0)): OK
BTF raw test[79] (enum type (invalid name, invalid identifier)): OK
BTF raw test[80] (enum member (invalid name, name_off = 0)): OK
BTF raw test[81] (enum member (invalid name, invalid identifier)): OK
Acked-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
tools/testing/selftests/bpf/test_btf.c | 362 +
1 file changed, 362 insertions(+)
diff --git a/tools/testing/selftests/bpf/test_btf.c
b/tools/testing/selftests/bpf/test_btf.c
index 99a92923e3f9..a7267815311f 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -1849,6 +1849,368 @@ static struct btf_raw_test raw_tests[] = {
.err_str = "Invalid type_id",
},
+{
+ .descr = "typedef (invalid name, name_off = 0)",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_TYPEDEF_ENC(0, 1), /* [2] */
+ BTF_END_RAW,
+ },
+ .str_sec = "\0__int",
+ .str_sec_size = sizeof("\0__int"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "typedef_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 4,
+ .btf_load_err = true,
+ .err_str = "Invalid name",
+},
+
+{
+ .descr = "typedef (invalid name, invalid identifier)",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_TYPEDEF_ENC(NAME_TBD, 1), /* [2] */
+ BTF_END_RAW,
+ },
+ .str_sec = "\0__!int",
+ .str_sec_size = sizeof("\0__!int"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "typedef_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 4,
+ .btf_load_err = true,
+ .err_str = "Invalid name",
+},
+
+{
+ .descr = "ptr type (invalid name, name_off <> 0)",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1]
*/
+ BTF_TYPE_ENC(NAME_TBD,
+BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 1), /* [2]
*/
+ BTF_END_RAW,
+ },
+ .str_sec = "\0__int",
+ .str_sec_size = sizeof("\0__int"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "ptr_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 4,
+ .btf_load_err = true,
+ .err_str = "Invalid name",
+},
+
+{
+ .descr = "volatile type (invalid name, name_off <> 0)",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1]
*/
+ BTF_TYPE_ENC(NAME_TBD,
+BTF_INFO_ENC(BTF_KIND_VOLATILE, 0, 0), 1), /* [2]
*/
+ BTF_END_RAW,
+ },
+ .str_sec = "\0__int",
+ .str_sec_size = sizeof("\0__int"),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "volatile_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 4,
+ .btf_load_err = true,
+ .err_str = "Invalid name",
+},
+
+{
+ .descr = "const type (invalid name, name_off <> 0)",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1]
*/
+ BTF_TYPE_ENC(NAME_TBD,
+BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 1),/* [2]
*/
+ BTF_END_RAW,
+ },
+ .
[PATCH bpf-next 0/3] bpf: btf: check name validity for various types
Commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC
and BTF_KIND_FUNC_PROTO") checked the name validity
for BTF_KIND_FUNC/FUNC_PROTO types.
This patch added name checking for PTR, ARRAY, VOLATILE, TYPEDEF,
CONST, RESTRICT, STRUCT, UNION, ENUM and FWD types. Such a strict
name checking makes BTF more sound in the kernel and future
BTF-to-header-file converesion ([1]) less fragile.
[1]: http://vger.kernel.org/lpc-bpf2018.html#session-2
Yonghong Song (3):
bpf: btf: check name validity for various types
tools/bpf: fix two test_btf unit test cases
tools/bpf: add addition type tests to test_btf
kernel/bpf/btf.c | 57
tools/testing/selftests/bpf/test_btf.c | 380 -
2 files changed, 428 insertions(+), 9 deletions(-)
--
2.17.1
[PATCH bpf-next 0/2] bpf: support proper non-jit func info
Commit 838e96904ff3 ("bpf: Introduce bpf_func_info")
added bpf func info support. The userspace is able
to get better ksym's for bpf programs with jit, and
is able to print out func prototypes.
For a program containing func-to-func calls, the existing
implementation returns user specified number of function
calls and BTF types if jit is enabled. If the jit is not
enabled, it only returns the type for the main function.
This is undesirable. Interpreter may still be used
and we should keep feature identical regardless of
whether jit is enabled or not.
This patch fixed this discrepancy.
The following example shows bpftool output for
the bpf program in selftests test_btf_haskv.o when jit
is disabled:
$ bpftool prog dump xlated id 1490
int _dummy_tracepoint(struct dummy_tracepoint_args * arg):
0: (85) call pc+2#__bpf_prog_run_args32
1: (b7) r0 = 0
2: (95) exit
int test_long_fname_1(struct dummy_tracepoint_args * arg):
3: (85) call pc+1#__bpf_prog_run_args32
4: (95) exit
int test_long_fname_2(struct dummy_tracepoint_args * arg):
5: (b7) r2 = 0
6: (63) *(u32 *)(r10 -4) = r2
7: (79) r1 = *(u64 *)(r1 +8)
8: (15) if r1 == 0x0 goto pc+9
9: (bf) r2 = r10
10: (07) r2 += -4
11: (18) r1 = map[id:1173]
13: (85) call bpf_map_lookup_elem#77088
14: (15) if r0 == 0x0 goto pc+3
15: (61) r1 = *(u32 *)(r0 +4)
16: (07) r1 += 1
17: (63) *(u32 *)(r0 +4) = r1
18: (95) exit
$ bpftool prog dump jited id 1490
no instructions returned
Yonghong Song (2):
bpf: btf: support proper non-jit func info
tools/bpf: change selftest test_btf for both jit and non-jit
include/linux/bpf.h| 6 ++-
include/linux/bpf_verifier.h | 1 -
kernel/bpf/core.c | 3 +-
kernel/bpf/syscall.c | 33
kernel/bpf/verifier.c | 55 ++
tools/testing/selftests/bpf/test_btf.c | 33 ++--
6 files changed, 55 insertions(+), 76 deletions(-)
--
2.17.1
[PATCH bpf-next 1/2] bpf: btf: support proper non-jit func info
Commit 838e96904ff3 ("bpf: Introduce bpf_func_info")
added bpf func info support. The userspace is able
to get better ksym's for bpf programs with jit, and
is able to print out func prototypes.
For a program containing func-to-func calls, the existing
implementation returns user specified number of function
calls and BTF types if jit is enabled. If the jit is not
enabled, it only returns the type for the main function.
This is undesirable. Interpreter may still be used
and we should keep feature identical regardless of
whether jit is enabled or not.
This patch fixed this discrepancy.
Fixes: 838e96904ff3 ("bpf: Introduce bpf_func_info")
Signed-off-by: Yonghong Song
---
include/linux/bpf.h | 6 ++--
include/linux/bpf_verifier.h | 1 -
kernel/bpf/core.c| 3 +-
kernel/bpf/syscall.c | 33 ++
kernel/bpf/verifier.c| 55 +---
5 files changed, 52 insertions(+), 46 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 7f0e225bf630..e82b7039fc66 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -299,7 +299,8 @@ struct bpf_prog_aux {
u32 max_pkt_offset;
u32 stack_depth;
u32 id;
- u32 func_cnt;
+ u32 func_cnt; /* used by non-func prog as the number of func progs */
+ u32 func_idx; /* 0 for non-func prog, the index in func array for func
prog */
bool offload_requested;
struct bpf_prog **func;
void *jit_data; /* JIT specific data. arch dependent */
@@ -317,7 +318,8 @@ struct bpf_prog_aux {
#endif
struct bpf_prog_offload *offload;
struct btf *btf;
- u32 type_id; /* type id for this prog/func */
+ struct bpf_func_info *func_info;
+ u32 func_info_cnt;
union {
struct work_struct work;
struct rcu_head rcu;
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index 204382f46fd8..11f5df1092d9 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -204,7 +204,6 @@ static inline bool bpf_verifier_log_needed(const struct
bpf_verifier_log *log)
struct bpf_subprog_info {
u32 start; /* insn idx of function entry point */
u16 stack_depth; /* max. stack depth used by this function */
- u32 type_id; /* btf type_id for this subprog */
};
/* single container for all structs
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 16d77012ad3e..002d67c62c8b 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -411,7 +411,8 @@ static void bpf_get_prog_name(const struct bpf_prog *prog,
char *sym)
/* prog->aux->name will be ignored if full btf name is available */
if (prog->aux->btf) {
- type = btf_type_by_id(prog->aux->btf, prog->aux->type_id);
+ type = btf_type_by_id(prog->aux->btf,
+
prog->aux->func_info[prog->aux->func_idx].type_id);
func_name = btf_name_by_offset(prog->aux->btf, type->name_off);
snprintf(sym, (size_t)(end - sym), "_%s", func_name);
return;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 998377808102..85cbeec06e50 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -1214,6 +1214,7 @@ static void __bpf_prog_put(struct bpf_prog *prog, bool
do_idr_lock)
bpf_prog_free_id(prog, do_idr_lock);
bpf_prog_kallsyms_del_all(prog);
btf_put(prog->aux->btf);
+ kvfree(prog->aux->func_info);
call_rcu(>aux->rcu, __bpf_prog_put_rcu);
}
@@ -2219,46 +2220,28 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog
*prog,
}
if (prog->aux->btf) {
+ u32 krec_size = sizeof(struct bpf_func_info);
u32 ucnt, urec_size;
info.btf_id = btf_id(prog->aux->btf);
ucnt = info.func_info_cnt;
- info.func_info_cnt = prog->aux->func_cnt ? : 1;
+ info.func_info_cnt = prog->aux->func_info_cnt;
urec_size = info.func_info_rec_size;
- info.func_info_rec_size = sizeof(struct bpf_func_info);
+ info.func_info_rec_size = krec_size;
if (ucnt) {
/* expect passed-in urec_size is what the kernel
expects */
if (urec_size != info.func_info_rec_size)
return -EINVAL;
if (bpf_dump_raw_ok()) {
- struct bpf_func_info kern_finfo;
char __user *user_finfo;
- u32 i, insn_offset;
user_finfo = u64_to_user_ptr(info.func_info);
-
[PATCH bpf-next 2/2] tools/bpf: change selftest test_btf for both jit and non-jit
The selftest test_btf is changed to test both jit and non-jit.
The test result should be the same regardless of whether jit
is enabled or not.
Signed-off-by: Yonghong Song
---
tools/testing/selftests/bpf/test_btf.c | 33 +++---
1 file changed, 3 insertions(+), 30 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_btf.c
b/tools/testing/selftests/bpf/test_btf.c
index bcbda7037840..b4c8725b3004 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -29,7 +29,6 @@
static uint32_t pass_cnt;
static uint32_t error_cnt;
static uint32_t skip_cnt;
-static bool jit_enabled;
#define CHECK(condition, format...) ({ \
int __ret = !!(condition); \
@@ -65,24 +64,6 @@ static int __base_pr(const char *format, ...)
return err;
}
-static bool is_jit_enabled(void)
-{
- const char *jit_sysctl = "/proc/sys/net/core/bpf_jit_enable";
- bool enabled = false;
- int sysctl_fd;
-
- sysctl_fd = open(jit_sysctl, 0, O_RDONLY);
- if (sysctl_fd != -1) {
- char tmpc;
-
- if (read(sysctl_fd, , sizeof(tmpc)) == 1)
- enabled = (tmpc != '0');
- close(sysctl_fd);
- }
-
- return enabled;
-}
-
#define BTF_INFO_ENC(kind, root, vlen) \
((!!(root) << 31) | ((kind) << 24) | ((vlen) & BTF_MAX_VLEN))
@@ -2547,8 +2528,8 @@ static int do_test_file(unsigned int test_num)
test->btf_kv_notfound))
goto done;
- if (!jit_enabled || !has_btf_ext)
- goto skip_jit;
+ if (!has_btf_ext)
+ goto skip;
/* get necessary program info */
info_len = sizeof(struct bpf_prog_info);
@@ -2636,7 +2617,7 @@ static int do_test_file(unsigned int test_num)
finfo = (void *)finfo + rec_size;
}
-skip_jit:
+skip:
fprintf(stderr, "OK");
done:
@@ -3270,12 +3251,6 @@ static int do_test_func_type(int test_num)
err = -1;
goto done;
}
- if (!jit_enabled) {
- skip_cnt++;
- fprintf(stderr, "SKIPPED, please enable sysctl
bpf_jit_enable\n");
- err = 0;
- goto done;
- }
/* get necessary lens */
info_len = sizeof(struct bpf_prog_info);
@@ -3452,8 +3427,6 @@ int main(int argc, char **argv)
if (args.always_log)
libbpf_set_print(__base_pr, __base_pr, __base_pr);
- jit_enabled = is_jit_enabled();
-
if (args.raw_test)
err |= test_raw();
--
2.17.1
Re: [PATCH bpf-next 1/2] libbpf: Add version script for DSO
On 11/21/18 9:40 AM, Andrey Ignatov wrote:
> More and more projects use libbpf and one day it'll likely be packaged
> and distributed as DSO and that requires ABI versioning so that both
> compatible and incompatible changes to ABI can be introduced in a safe
> way in the future without breaking executables dynamically linked with a
> previous version of the library.
>
> Usual way to do ABI versioning is version script for the linker. Add
> such a script for libbpf. All global symbols currently exported via
> LIBBPF_API macro are added to the version script libbpf.map.
>
> The version name LIBBPF_0.0.1 is constructed from the name of the
> library + version specified by $(LIBBPF_VERSION) in Makefile.
>
> Version script does not duplicate the work done by LIBBPF_API macro, it
> rather complements it. The macro is used at compile time and can be used
> by compiler to do optimization that can't be done at link time, it is
> purely about global symbol visibility. The version script, in turn, is
> used at link time and takes care of ABI versioning. Both techniques are
> described in details in [1].
>
> Whenever ABI is changed in the future, version script should be changed
> appropriately.
Maybe we should clarify the policy of how version numbers should be
change? Each commit which changes default global symbol ABI? Each kernel
release?
>
> [1] https://www.akkadia.org/drepper/dsohowto.pdf
>
> Signed-off-by: Andrey Ignatov
> ---
> tools/lib/bpf/Makefile | 4 +-
> tools/lib/bpf/libbpf.map | 120 +++
> 2 files changed, 123 insertions(+), 1 deletion(-)
> create mode 100644 tools/lib/bpf/libbpf.map
>
> diff --git a/tools/lib/bpf/Makefile b/tools/lib/bpf/Makefile
> index 425b480bda75..d76c41fa2d39 100644
> --- a/tools/lib/bpf/Makefile
> +++ b/tools/lib/bpf/Makefile
> @@ -145,6 +145,7 @@ include $(srctree)/tools/build/Makefile.include
>
> BPF_IN:= $(OUTPUT)libbpf-in.o
> LIB_FILE := $(addprefix $(OUTPUT),$(LIB_FILE))
> +VERSION_SCRIPT := libbpf.map
>
> CMD_TARGETS = $(LIB_FILE)
>
> @@ -170,7 +171,8 @@ $(BPF_IN): force elfdep bpfdep
> $(Q)$(MAKE) $(build)=libbpf
>
> $(OUTPUT)libbpf.so: $(BPF_IN)
> - $(QUIET_LINK)$(CC) --shared $^ -o $@
> + $(QUIET_LINK)$(CC) --shared -Wl,--version-script=$(VERSION_SCRIPT) \
> + $^ -o $@
>
> $(OUTPUT)libbpf.a: $(BPF_IN)
> $(QUIET_LINK)$(RM) $@; $(AR) rcs $@ $^
> diff --git a/tools/lib/bpf/libbpf.map b/tools/lib/bpf/libbpf.map
> new file mode 100644
> index ..9fe416b68c7d
> --- /dev/null
> +++ b/tools/lib/bpf/libbpf.map
> @@ -0,0 +1,120 @@
> +LIBBPF_0.0.1 {
> + global:
> + bpf_btf_get_fd_by_id;
Do you think we could use this opportunities to
make naming more consistent? For example,
bpf_btf_get_fd_by_id => btf__get_fd_by_id?
> + bpf_create_map;
> + bpf_create_map_in_map;
> + bpf_create_map_in_map_node;
> + bpf_create_map_name;
> + bpf_create_map_node;
> + bpf_create_map_xattr;
> + bpf_load_btf;
> + bpf_load_program;
> + bpf_load_program_xattr;
> + bpf_map__btf_key_type_id;
> + bpf_map__btf_value_type_id;
> + bpf_map__def;
> + bpf_map_delete_elem; > +bpf_map__fd;
> + bpf_map_get_fd_by_id;
> + bpf_map_get_next_id;
> + bpf_map_get_next_key; > +
> bpf_map__is_offload_neutral;
> + bpf_map_lookup_and_delete_elem;
> + bpf_map_lookup_elem;
> + bpf_map__name;
> + bpf_map__next;
> + bpf_map__pin;
> + bpf_map__prev;
> + bpf_map__priv;
> + bpf_map__reuse_fd;
> + bpf_map__set_ifindex;
> + bpf_map__set_priv;
> + bpf_map__unpin;
> + bpf_map_update_elem;
> + bpf_object__btf_fd;
> + bpf_object__close;
> + bpf_object__find_map_by_name;
> + bpf_object__find_map_by_offset;
> + bpf_object__find_program_by_title;
> + bpf_object__kversion;
> + bpf_object__load;
> + bpf_object__name;
> + bpf_object__next;
> + bpf_object__open;
> + bpf_object__open_buffer;
> + bpf_object__open_xattr;
> + bpf_object__pin;
> + bpf_object__pin_maps;
> + bpf_object__pin_programs;
> + bpf_object__priv;
> + bpf_object__set_priv;
> + bpf_object__unload;
> + bpf_object__unpin_maps;
> + bpf_object__unpin_programs;
> + bpf_obj_get;
> + bpf_obj_get_info_by_fd;
> + bpf_obj_pin;
> + bpf_perf_event_read_simple;
> + bpf_prog_attach;
> + bpf_prog_detach;
> + bpf_prog_detach2;
> + bpf_prog_get_fd_by_id;
> +
[PATCH bpf-next] bpf: fix a libbpf loader issue
Commit 2993e0515bb4 ("tools/bpf: add support to read .BTF.ext sections")
added support to read .BTF.ext sections from an object file, create
and pass prog_btf_fd and func_info to the kernel.
The program btf_fd (prog->btf_fd) is initialized to be -1 to please
zclose so we do not need special handling dur prog close.
Passing -1 to the kernel, however, will cause loading error.
Passing btf_fd 0 to the kernel if prog->btf_fd is invalid
fixed the problem.
Fixes: 2993e0515bb4 ("tools/bpf: add support to read .BTF.ext sections")
Reported-by: Andrey Ignatov
Reported-by: Emre Cantimur
Tested-by: Andrey Ignatov
Signed-off-by: Yonghong Song
---
tools/lib/bpf/libbpf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index cb6565d79603..f022ac82e882 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -1387,7 +1387,7 @@ load_program(struct bpf_program *prog, struct bpf_insn
*insns, int insns_cnt,
load_attr.license = license;
load_attr.kern_version = kern_version;
load_attr.prog_ifindex = prog->prog_ifindex;
- load_attr.prog_btf_fd = prog->btf_fd;
+ load_attr.prog_btf_fd = prog->btf_fd >= 0 ? prog->btf_fd : 0;
load_attr.func_info = prog->func_info;
load_attr.func_info_rec_size = prog->func_info_rec_size;
load_attr.func_info_cnt = func_info_cnt;
--
2.17.1
Re: selftests/bpf :get_cgroup_id_user: File not found: /sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id
On Wed, Nov 21, 2018 at 3:44 AM Naresh Kamboju
wrote:
>
> Kselftest bpf get_cgroup_id_user is failed on all devices.
>
> selftests: bpf: get_cgroup_id_user
> main:PASS:setup_cgroup_environment
> main:PASS:create_and_get_cgroup
> main:PASS:join_cgroup
> main:PASS:bpf_prog_load
> main:PASS:bpf_find_map
> main:PASS:bpf_find_map
> main:FAIL:open err -1 errno 2
> not ok 1..15 selftests: bpf: get_cgroup_id_user [FAIL]
>
> The strace output shows,
> This expected file not found,
> "/sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id"
>
> bpf(BPF_MAP_UPDATE_ELEM, {map_fd=5, key=0x7fff0c68c138,
> value=0x7fff0c68c13c, flags=BPF_ANY}, 72) = 0
> open(\"/sys/kernel/debug/tracing/events/syscalls/sys_enter_nanosleep/id\",
> O_RDONLY) = -1 ENOENT (No such file or directory)
> write(1, \"main:FAIL:open err -1 errno 2\n\", 30main:FAIL:open err -1 errno 2
>
> Am I missing any pre-requirement ?
You probably miss kernel config CONFIG_FTRACE_SYSCALLS.
>
> Best regards
> Naresh Kamboju
Re: [PATCH bpf-next v2] libbpf: make sure bpf headers are c++ include-able
On Tue, Nov 20, 2018 at 1:37 PM Stanislav Fomichev wrote: > > Wrap headers in extern "C", to turn off C++ mangling. > This simplifies including libbpf in c++ and linking against it. > > v2 changes: > * do the same for btf.h > > Signed-off-by: Stanislav Fomichev Acked-by: Yonghong Song > --- > tools/lib/bpf/bpf.h| 9 + > tools/lib/bpf/btf.h| 8 > tools/lib/bpf/libbpf.h | 9 + > 3 files changed, 26 insertions(+)
[PATCH bpf-next v2] bpf: fix a compilation error when CONFIG_BPF_SYSCALL is not defined
Kernel test robot (l...@intel.com) reports a compilation error at
https://www.spinics.net/lists/netdev/msg534913.html
introduced by commit 838e96904ff3 ("bpf: Introduce bpf_func_info").
If CONFIG_BPF is defined and CONFIG_BPF_SYSCALL is not defined,
the following error will appear:
kernel/bpf/core.c:414: undefined reference to `btf_type_by_id'
kernel/bpf/core.c:415: undefined reference to `btf_name_by_offset'
When CONFIG_BPF_SYSCALL is not defined,
let us define stub inline functions for btf_type_by_id()
and btf_name_by_offset() in include/linux/btf.h.
This way, the compilation failure can be avoided.
Fixes: 838e96904ff3 ("bpf: Introduce bpf_func_info")
Reported-by: kbuild test robot
Cc: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
include/linux/btf.h | 14 ++
1 file changed, 14 insertions(+)
Changelog:
v1 -> v2:
. Two functions should be static inline functions
if CONFIG_BPF_SYSCALL is not defined.
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 7f2c0a4a45ea..8c2199b5d250 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -46,7 +46,21 @@ void btf_type_seq_show(const struct btf *btf, u32 type_id,
void *obj,
struct seq_file *m);
int btf_get_fd_by_id(u32 id);
u32 btf_id(const struct btf *btf);
+
+#ifdef CONFIG_BPF_SYSCALL
const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id);
const char *btf_name_by_offset(const struct btf *btf, u32 offset);
+#else
+static inline const struct btf_type *btf_type_by_id(const struct btf *btf,
+ u32 type_id)
+{
+ return NULL;
+}
+static inline const char *btf_name_by_offset(const struct btf *btf,
+u32 offset)
+{
+ return NULL;
+}
+#endif
#endif
--
2.17.1
[PATCH bpf-next] bpf: fix a compilation error when CONFIG_BPF_SYSCALL is not defined
Kernel test robot (l...@intel.com) reports a compilation error at
https://www.spinics.net/lists/netdev/msg534913.html
introduced by commit 838e96904ff3 ("bpf: Introduce bpf_func_info").
If CONFIG_BPF is defined and CONFIG_BPF_SYSCALL is not defined,
the following error will appear:
kernel/bpf/core.c:414: undefined reference to `btf_type_by_id'
kernel/bpf/core.c:415: undefined reference to `btf_name_by_offset'
When CONFIG_BPF_SYSCALL is not defined,
let us define stub inline functions for btf_type_by_id()
and btf_name_by_offset() in include/linux/btf.h.
This way, the compilation failure can be avoided.
Fixes: 838e96904ff3 ("bpf: Introduce bpf_func_info")
Reported-by: kbuild test robot
Cc: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
include/linux/btf.h | 12
1 file changed, 12 insertions(+)
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 7f2c0a4a45ea..a5e893071861 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -46,7 +46,19 @@ void btf_type_seq_show(const struct btf *btf, u32 type_id,
void *obj,
struct seq_file *m);
int btf_get_fd_by_id(u32 id);
u32 btf_id(const struct btf *btf);
+
+#ifdef CONFIG_BPF_SYSCALL
const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id);
const char *btf_name_by_offset(const struct btf *btf, u32 offset);
+#else
+const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id)
+{
+ return NULL;
+}
+const char *btf_name_by_offset(const struct btf *btf, u32 offset)
+{
+ return NULL;
+}
+#endif
#endif
--
2.17.1
Re: [PATCH bpf-next] libbpf: make sure bpf headers are c++ include-able
On Tue, Nov 20, 2018 at 10:19 AM Stanislav Fomichev wrote:
>
> Wrap headers in extern "C", to turn off C++ mangling.
> This simplifies including libbpf in c++ and linking against it.
>
> Signed-off-by: Stanislav Fomichev
> ---
> tools/lib/bpf/bpf.h| 9 +
> tools/lib/bpf/libbpf.h | 9 +
Do you want to add tools/lib/bpf/btf.h as well? it has some functions
which could be used outside libbpf as well.
> 2 files changed, 18 insertions(+)
>
> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> index 26a51538213c..9ea3aec82d8a 100644
> --- a/tools/lib/bpf/bpf.h
> +++ b/tools/lib/bpf/bpf.h
> @@ -27,6 +27,10 @@
> #include
> #include
>
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
Some (but not all) __cplusplus extern wrappers wraps standard include as well
like "#include ", "#include ". probably does not
matter here
since they may not have function prototype. But just want to point it out so you
are aware of this and may double check.
> +
> #ifndef LIBBPF_API
> #define LIBBPF_API __attribute__((visibility("default")))
> #endif
> @@ -128,4 +132,9 @@ LIBBPF_API int bpf_load_btf(void *btf, __u32 btf_size,
> char *log_buf,
> LIBBPF_API int bpf_task_fd_query(int pid, int fd, __u32 flags, char *buf,
> __u32 *buf_len, __u32 *prog_id, __u32
> *fd_type,
> __u64 *probe_offset, __u64 *probe_addr);
> +
> +#ifdef __cplusplus
> +} /* extern "C" */
> +#endif
> +
> #endif /* __LIBBPF_BPF_H */
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index b1686a787102..74e57e041705 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -16,6 +16,10 @@
> #include // for size_t
> #include
>
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> #ifndef LIBBPF_API
> #define LIBBPF_API __attribute__((visibility("default")))
> #endif
> @@ -335,4 +339,9 @@ int libbpf_nl_get_qdisc(int sock, unsigned int nl_pid,
> int ifindex,
> libbpf_dump_nlmsg_t dump_qdisc_nlmsg, void *cookie);
> int libbpf_nl_get_filter(int sock, unsigned int nl_pid, int ifindex, int
> handle,
> libbpf_dump_nlmsg_t dump_filter_nlmsg, void *cookie);
> +
> +#ifdef __cplusplus
> +} /* extern "C" */
> +#endif
> +
> #endif /* __LIBBPF_LIBBPF_H */
> --
> 2.19.1.1215.g8438c0b245-goog
>
Re: [PATCH v2 0/4] Fix unsafe BPF_PROG_TEST_RUN interface
On Tue, Nov 20, 2018 at 7:43 AM Lorenz Bauer wrote: > > Right now, there is no safe way to use BPF_PROG_TEST_RUN with data_out. > This is because bpf_test_finish copies the output buffer to user space > without checking its size. This can lead to the kernel overwriting > data in user space after the buffer if xdp_adjust_head and friends are > in play. > > Changes in v2: > * Make the syscall return ENOSPC if data_size_out is too small > * Make bpf_prog_test_run return EINVAL if size_out is missing > * Document the new behaviour of data_size_out > > Lorenz Bauer (4): > bpf: respect size hint to BPF_PROG_TEST_RUN if present > tools: sync uapi/linux/bpf.h > libbpf: require size hint in bpf_prog_test_run > selftests: add a test for bpf_prog_test_run output size For the series, if we decided to take this approach rather than amending another field in the uapi as described in https://www.spinics.net/lists/netdev/msg534277.html, then Acked-by: Yonghong Song
Re: [PATCH 3/3] selftests: add a test for bpf_prog_test_run output size
On Tue, Nov 20, 2018 at 3:35 AM Lorenz Bauer wrote:
>
> On Sun, 18 Nov 2018 at 05:59, Y Song wrote:
> >
> > On Fri, Nov 16, 2018 at 12:55 PM Lorenz Bauer wrote:
> > >
> > > Make sure that bpf_prog_test_run returns the correct length
> > > in the size_out argument and that the kernel respects the
> > > output size hint.
> > >
> > > Signed-off-by: Lorenz Bauer
> > > ---
> > > tools/testing/selftests/bpf/test_progs.c | 34
> > > 1 file changed, 34 insertions(+)
> > >
> > > diff --git a/tools/testing/selftests/bpf/test_progs.c
> > > b/tools/testing/selftests/bpf/test_progs.c
> > > index 560d7527b86b..6ab98e10e86f 100644
> > > --- a/tools/testing/selftests/bpf/test_progs.c
> > > +++ b/tools/testing/selftests/bpf/test_progs.c
> > > @@ -124,6 +124,39 @@ static void test_pkt_access(void)
> > > bpf_object__close(obj);
> > > }
> > >
> > > +static void test_output_size_hint(void)
> > > +{
> > > + const char *file = "./test_pkt_access.o";
> > > + struct bpf_object *obj;
> > > + __u32 retval, size, duration;
> > > + int err, prog_fd;
> > > + char buf[10];
> > > +
> > > + err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, ,
> > > _fd);
> > > + if (err) {
> > > + error_cnt++;
> > > + return;
> > > + }
> > CHECK can also be used here.
> > if (CHECK(...)) {
> >goto done;
> > }
> > where label "done" is right before bpf_object__close.
>
> I just copied this part from test_pkt_access, happy to change it though.
> However, I think "goto done" would lead to freeing an unallocated
> object in this case?
Right, you can just return here.
>
> --
> Lorenz Bauer | Systems Engineer
> 25 Lavington St., London SE1 0NZ
>
> www.cloudflare.com
Re: [PATCH v2 bpf-next 0/2] bpf: adding support for mapinmap in libbpf
On Mon, Nov 19, 2018 at 9:40 PM Nikita V. Shirokov wrote: > > in this patch series i'm adding a helper for libbpf which would allow > it to load map-in-map(BPF_MAP_TYPE_ARRAY_OF_MAPS and > BPF_MAP_TYPE_HASH_OF_MAPS). > first patch contains new helper + explains proposed workflow > second patch contains tests which also could be used as example of > usage > > v1->v2: > - addressing nits > - removing const identifier from fd in new helper > - starting to check return val for bpf_map_update_elem > > Nikita V. Shirokov (2): > bpf: adding support for map in map in libbpf > bpf: adding tests for mapinmap helpber in libbpf For the whole series. Acked-by: Yonghong Song > > tools/lib/bpf/libbpf.c | 7 +++ > tools/lib/bpf/libbpf.h | 2 + > tools/testing/selftests/bpf/Makefile| 3 +- > tools/testing/selftests/bpf/test_mapinmap.c | 49 + > tools/testing/selftests/bpf/test_maps.c | 82 > + > 5 files changed, 142 insertions(+), 1 deletion(-) > create mode 100644 tools/testing/selftests/bpf/test_mapinmap.c > > -- > 2.15.1 >
Re: [PATCH iproute2] bpf: initialise map symbol before retrieving and comparing its type
On Mon, Nov 19, 2018 at 5:28 PM Quentin Monnet
wrote:
>
> In order to compare BPF map symbol type correctly in regard to the
> latest LLVM, commit 7a04dd84a7f9 ("bpf: check map symbol type properly
> with newer llvm compiler") compares map symbol type to both NOTYPE and
> OBJECT. To do so, it first retrieves the type from "sym.st_info" and
> stores it into a temporary variable.
>
> However, the type is collected from the symbol "sym" before this latter
> symbol is actually updated. gelf_getsym() is called after that and
> updates "sym", and when comparison with OBJECT or NOTYPE happens it is
> done on the type of the symbol collected in the previous passage of the
> loop (or on an uninitialised symbol on the first passage). This may
> eventually break map collection from the ELF file.
>
> Fix this by assigning the type to the temporary variable only after the
> call to gelf_getsym().
>
> Fixes: 7a04dd84a7f9 ("bpf: check map symbol type properly with newer llvm
> compiler")
> Reported-by: Ron Philip
> Signed-off-by: Quentin Monnet
> Reviewed-by: Jiong Wang
Thanks for the fix!
Acked-by: Yonghong Song
> ---
> lib/bpf.c | 10 +++---
> 1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/lib/bpf.c b/lib/bpf.c
> index 45f279fa4a41..6aff8f7bad7f 100644
> --- a/lib/bpf.c
> +++ b/lib/bpf.c
> @@ -1758,11 +1758,12 @@ static const char *bpf_map_fetch_name(struct
> bpf_elf_ctx *ctx, int which)
> int i;
>
> for (i = 0; i < ctx->sym_num; i++) {
> - int type = GELF_ST_TYPE(sym.st_info);
> + int type;
>
> if (gelf_getsym(ctx->sym_tab, i, ) != )
> continue;
>
> + type = GELF_ST_TYPE(sym.st_info);
> if (GELF_ST_BIND(sym.st_info) != STB_GLOBAL ||
> (type != STT_NOTYPE && type != STT_OBJECT) ||
> sym.st_shndx != ctx->sec_maps ||
> @@ -1851,11 +1852,12 @@ static int bpf_map_num_sym(struct bpf_elf_ctx *ctx)
> GElf_Sym sym;
>
> for (i = 0; i < ctx->sym_num; i++) {
> - int type = GELF_ST_TYPE(sym.st_info);
> + int type;
>
> if (gelf_getsym(ctx->sym_tab, i, ) != )
> continue;
>
> + type = GELF_ST_TYPE(sym.st_info);
> if (GELF_ST_BIND(sym.st_info) != STB_GLOBAL ||
> (type != STT_NOTYPE && type != STT_OBJECT) ||
> sym.st_shndx != ctx->sec_maps)
> @@ -1931,10 +1933,12 @@ static int bpf_map_verify_all_offs(struct bpf_elf_ctx
> *ctx, int end)
> * the table again.
> */
> for (i = 0; i < ctx->sym_num; i++) {
> - int type = GELF_ST_TYPE(sym.st_info);
> + int type;
>
> if (gelf_getsym(ctx->sym_tab, i, ) != )
> continue;
> +
> + type = GELF_ST_TYPE(sym.st_info);
> if (GELF_ST_BIND(sym.st_info) != STB_GLOBAL ||
> (type != STT_NOTYPE && type != STT_OBJECT) ||
> sym.st_shndx != ctx->sec_maps)
> --
> 2.7.4
>
Re: [PATCH bpf-next] bpf: libbpf: retry program creation without the name
On Mon, Nov 19, 2018 at 4:52 PM Stanislav Fomichev wrote:
>
> [Recent commit 23499442c319 ("bpf: libbpf: retry map creation without
> the name") fixed this issue for maps, let's do the same for programs.]
>
> Since commit 88cda1c9da02 ("bpf: libbpf: Provide basic API support
> to specify BPF obj name"), libbpf unconditionally sets bpf_attr->name
> for programs. Pre v4.14 kernels don't know about programs names and
> return an error about unexpected non-zero data. Retry sys_bpf without
> a program name to cover older kernels.
>
> Signed-off-by: Stanislav Fomichev
Acked-by: Yonghong Song
Re: [PATCH bpf-next 2/2] bpf: adding tests for mapinmap helpber in libbpf
On Mon, Nov 19, 2018 at 4:13 PM Nikita V. Shirokov wrote:
>
> adding test/example of bpf_map__add_inner_map_fd usage
>
> Signed-off-by: Nikita V. Shirokov
> ---
> tools/testing/selftests/bpf/Makefile| 3 +-
> tools/testing/selftests/bpf/test_mapinmap.c | 53
> tools/testing/selftests/bpf/test_maps.c | 76
> +
> 3 files changed, 131 insertions(+), 1 deletion(-)
> create mode 100644 tools/testing/selftests/bpf/test_mapinmap.c
>
> diff --git a/tools/testing/selftests/bpf/Makefile
> b/tools/testing/selftests/bpf/Makefile
> index 57b4712a6276..a3ea69dc9bdf 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -38,7 +38,8 @@ TEST_GEN_FILES = test_pkt_access.o test_xdp.o test_l4lb.o
> test_tcp_estats.o test
> test_lwt_seg6local.o sendmsg4_prog.o sendmsg6_prog.o
> test_lirc_mode2_kern.o \
> get_cgroup_id_kern.o socket_cookie_prog.o
> test_select_reuseport_kern.o \
> test_skb_cgroup_id_kern.o bpf_flow.o netcnt_prog.o \
> - test_sk_lookup_kern.o test_xdp_vlan.o test_queue_map.o
> test_stack_map.o
> + test_sk_lookup_kern.o test_xdp_vlan.o test_queue_map.o
> test_stack_map.o \
> + test_mapinmap.o
>
> # Order correspond to 'make run_tests' order
> TEST_PROGS := test_kmod.sh \
> diff --git a/tools/testing/selftests/bpf/test_mapinmap.c
> b/tools/testing/selftests/bpf/test_mapinmap.c
> new file mode 100644
> index ..8aef6c652c9c
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/test_mapinmap.c
> @@ -0,0 +1,53 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2018 Facebook */
> +#include
> +#include
> +#include
> +#include "bpf_helpers.h"
> +
> +
nit: extra new line
> +struct bpf_map_def SEC("maps") mim_array = {
> + .type = BPF_MAP_TYPE_ARRAY_OF_MAPS,
> + .key_size = sizeof(int),
> + /* must be sizeof(__u32) for map in map */
> + .value_size = sizeof(__u32),
> + .max_entries = 1,
> + .map_flags = 0,
> +};
> +
> +struct bpf_map_def SEC("maps") mim_hash = {
> + .type = BPF_MAP_TYPE_HASH_OF_MAPS,
> + .key_size = sizeof(int),
> + /* must be sizeof(__u32) for map in map */
> + .value_size = sizeof(__u32),
> + .max_entries = 1,
> + .map_flags = 0,
> +};
> +
> +
> +
nit: extra new lines.
> +SEC("xdp_mimtest")
> +int xdp_mimtest0(struct xdp_md *ctx)
> +{
> + int value = 123;
> + int key = 0;
> + void *map;
> +
> + map = bpf_map_lookup_elem(_array, );
> + if (!map)
> + return XDP_DROP;
> +
> + bpf_map_update_elem(map, , , 0);
> +
> + map = bpf_map_lookup_elem(_hash, );
> + if (!map)
> + return XDP_DROP;
> +
> + bpf_map_update_elem(map, , , 0);
> +
> + return XDP_PASS;
> +}
> +
> +
nit: extra new line
> +int _version SEC("version") = 1;
> +char _license[] SEC("license") = "GPL";
> diff --git a/tools/testing/selftests/bpf/test_maps.c
> b/tools/testing/selftests/bpf/test_maps.c
> index 4db2116e52be..a49ab294971d 100644
> --- a/tools/testing/selftests/bpf/test_maps.c
> +++ b/tools/testing/selftests/bpf/test_maps.c
> @@ -1080,6 +1080,80 @@ static void test_sockmap(int tasks, void *data)
> exit(1);
> }
>
> +#define MAPINMAP_PROG "./test_mapinmap.o"
> +static void test_mapinmap(void)
> +{
> + struct bpf_program *prog;
> + struct bpf_object *obj;
> + struct bpf_map *map;
> + int mim_fd, fd;
> + int pos = 0;
> +
> + obj = bpf_object__open(MAPINMAP_PROG);
> +
> + fd = bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(int), sizeof(int),
> + 2, 0);
> + if (fd < 0) {
> + printf("Failed to create hashmap '%s'!\n", strerror(errno));
> + exit(1);
> + }
> + printf("fd is %d\n", fd);
not sure whether you need to print fd in the console or not. Any particular
reason?
> +
> + map = bpf_object__find_map_by_name(obj, "mim_array");
> + if (IS_ERR(map)) {
> + printf("Failed to load array of maps from test prog\n");
> + goto out_mapinmap;
> + }
> + bpf_map__add_inner_map_fd(map, fd);
> +
> + map = bpf_object__find_map_by_name(obj, "mim_hash");
> + if (IS_ERR(map)) {
> + printf("Failed to load hash of maps from test prog\n");
> + goto out_mapinmap;
> + }
> + bpf_map__add_inner_map_fd(map, fd);
> +
> +
nit: extra new line
> + bpf_object__for_each_program(prog, obj) {
> + bpf_program__set_xdp(prog);
> + }
> + bpf_object__load(obj);
> +
> + map = bpf_object__find_map_by_name(obj, "mim_array");
> + if (IS_ERR(map)) {
> + printf("Failed to load array of maps from test prog\n");
> + goto out_mapinmap;
> + }
> + mim_fd = bpf_map__fd(map);
> + if (mim_fd < 0) {
> + printf("Failed to get descriptor
Re: [PATCH bpf-next 1/2] bpf: adding support for map in map in libbpf
On Mon, Nov 19, 2018 at 4:13 PM Nikita V. Shirokov wrote:
>
> idea is pretty simple. for specified map (pointed by struct bpf_map)
> we would provide descriptor of already loaded map, which is going to be
> used as a prototype for inner map. proposed workflow:
> 1) open bpf's object (bpf_object__open)
> 2) create bpf's map which is going to be used as a prototype
> 3) find (by name) map-in-map which you want to load and update w/
> descriptor of inner map w/ a new helper from this patch
> 4) load bpf program w/ bpf_object__load
>
> inner_map_fd is ignored by any other maps asidef from (hash|array) of
> maps
>
> Signed-off-by: Nikita V. Shirokov
> ---
> tools/lib/bpf/libbpf.c | 7 +++
> tools/lib/bpf/libbpf.h | 2 ++
> 2 files changed, 9 insertions(+)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index a01eb9584e52..a2ee1b1a93b6 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -163,6 +163,7 @@ struct bpf_map {
> char *name;
> size_t offset;
> int map_ifindex;
> + int inner_map_fd;
> struct bpf_map_def def;
> __u32 btf_key_type_id;
> __u32 btf_value_type_id;
> @@ -1146,6 +1147,7 @@ bpf_object__create_maps(struct bpf_object *obj)
> create_attr.btf_fd = 0;
> create_attr.btf_key_type_id = 0;
> create_attr.btf_value_type_id = 0;
> + create_attr.inner_map_fd = map->inner_map_fd;
>
> if (obj->btf && !bpf_map_find_btf_info(map, obj->btf)) {
> create_attr.btf_fd = btf__fd(obj->btf);
> @@ -2562,6 +2564,11 @@ void bpf_map__set_ifindex(struct bpf_map *map, __u32
> ifindex)
> map->map_ifindex = ifindex;
> }
>
> +void bpf_map__add_inner_map_fd(struct bpf_map *map, const int fd)
Do we need "const" attribute here?
> +{
> + map->inner_map_fd = fd;
> +}
> +
> static struct bpf_map *
> __bpf_map__iter(struct bpf_map *m, struct bpf_object *obj, int i)
> {
> diff --git a/tools/lib/bpf/libbpf.h b/tools/lib/bpf/libbpf.h
> index b1686a787102..7cb00cd41789 100644
> --- a/tools/lib/bpf/libbpf.h
> +++ b/tools/lib/bpf/libbpf.h
> @@ -293,6 +293,8 @@ LIBBPF_API void bpf_map__set_ifindex(struct bpf_map *map,
> __u32 ifindex);
> LIBBPF_API int bpf_map__pin(struct bpf_map *map, const char *path);
> LIBBPF_API int bpf_map__unpin(struct bpf_map *map, const char *path);
>
> +LIBBPF_API void bpf_map__add_inner_map_fd(struct bpf_map *map, const int fd);
> +
> LIBBPF_API long libbpf_get_error(const void *ptr);
>
> struct bpf_prog_load_attr {
> --
> 2.15.1
>
Re: [PATCH 0/3] Fix unsafe BPF_PROG_TEST_RUN interface
On Fri, Nov 16, 2018 at 12:54 PM Lorenz Bauer wrote: > > Right now, there is no safe way to use BPF_PROG_TEST_RUN with data_out. > This is because bpf_test_finish copies the output buffer to user space > without checking its size. This can lead to the kernel overwriting > data in user space after the buffer if xdp_adjust_head and friends are > in play. > > Fix this by using bpf_attr.test.data_size_out as a size hint. The old > behaviour is retained if size_hint is zero. There is a slight change of user space behavior for this patch. Without this patch, the value bpf_attr.test.data_size_out is output only. For example, output buffer : out_buf (user allocated size 10) data_size_out is a random value (e.g., 1), The actual data to copy is 5. In today's implementation, the kernel will copy 5 and set data_size_out is 5. With this patch, the kernel will copy 1 and set data_size_out is 5. I am not 100% sure at this time whether we CAN overload data_size_out since it MAY break existing applications. Alternativley, we can append another field to bpf_attr.test __u32 data_out_size; this will provide the data_out buffer size. Inside kernel, if the user input attr is smaller than kernel and does not have data_out_size, the current behavior should be used. Otherwise, data_out_size is data_out buffer size. Daniel and Alexei, which option do you think is reasonable? > > Interestingly, do_test_single() in test_verifier.c already assumes > that this is the intended use of data_size_out, and sets it to the > output buffer size. > > Lorenz Bauer (3): > bpf: respect size hint to BPF_PROG_TEST_RUN if present > libbpf: require size hint in bpf_prog_test_run > selftests: add a test for bpf_prog_test_run output size > > net/bpf/test_run.c | 9 - > tools/lib/bpf/bpf.c | 4 ++- > tools/testing/selftests/bpf/test_progs.c | 44 > 3 files changed, 55 insertions(+), 2 deletions(-) > > -- > 2.17.1 >
Re: [PATCH 3/3] selftests: add a test for bpf_prog_test_run output size
On Fri, Nov 16, 2018 at 12:55 PM Lorenz Bauer wrote:
>
> Make sure that bpf_prog_test_run returns the correct length
> in the size_out argument and that the kernel respects the
> output size hint.
>
> Signed-off-by: Lorenz Bauer
> ---
> tools/testing/selftests/bpf/test_progs.c | 34
> 1 file changed, 34 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/test_progs.c
> b/tools/testing/selftests/bpf/test_progs.c
> index 560d7527b86b..6ab98e10e86f 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c
> @@ -124,6 +124,39 @@ static void test_pkt_access(void)
> bpf_object__close(obj);
> }
>
> +static void test_output_size_hint(void)
> +{
> + const char *file = "./test_pkt_access.o";
> + struct bpf_object *obj;
> + __u32 retval, size, duration;
> + int err, prog_fd;
> + char buf[10];
> +
> + err = bpf_prog_load(file, BPF_PROG_TYPE_SCHED_CLS, , _fd);
> + if (err) {
> + error_cnt++;
> + return;
> + }
CHECK can also be used here.
if (CHECK(...)) {
goto done;
}
where label "done" is right before bpf_object__close.
> +
> + memset(buf, 0, sizeof(buf));
> +
> + size = 5;
> + err = bpf_prog_test_run(prog_fd, 1, _v4, sizeof(pkt_v4),
> + buf, , , );
> + CHECK(err || retval, "run",
> + "err %d errno %d retval %d\n",
> + err, errno, retval);
> +
> + CHECK(size != sizeof(pkt_v4), "out_size",
> + "incorrect output size, want %lu have %u\n",
> + sizeof(pkt_v4), size);
> +
> + CHECK(buf[5] != 0, "overflow",
> + "prog_test_run ignored size hint\n");
> +
> + bpf_object__close(obj);
> +}
> +
> static void test_xdp(void)
> {
> struct vip key4 = {.protocol = 6, .family = AF_INET};
> @@ -1847,6 +1880,7 @@ int main(void)
> jit_enabled = is_jit_enabled();
>
> test_pkt_access();
> + test_output_size_hint();
> test_xdp();
> test_xdp_adjust_tail();
> test_l4lb_all();
> --
> 2.17.1
>
Re: [PATCH 2/3] libbpf: require size hint in bpf_prog_test_run
On Fri, Nov 16, 2018 at 12:54 PM Lorenz Bauer wrote: > > Require size_out to be non-NULL if data_out is given. This prevents > accidental overwriting of process memory after the output buffer. > > Adjust callers of bpf_prog_test_run to this behaviour. > > Signed-off-by: Lorenz Bauer > --- > tools/lib/bpf/bpf.c | 4 +++- > tools/testing/selftests/bpf/test_progs.c | 10 ++ > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c > index 03f9bcc4ef50..127a9aa6170e 100644 > --- a/tools/lib/bpf/bpf.c > +++ b/tools/lib/bpf/bpf.c > @@ -403,10 +403,12 @@ int bpf_prog_test_run(int prog_fd, int repeat, void > *data, __u32 size, > attr.test.data_in = ptr_to_u64(data); > attr.test.data_out = ptr_to_u64(data_out); > attr.test.data_size_in = size; > + if (data_out) > + attr.test.data_size_out = *size_out; Maybe it is better to return error (-EINVAL) instead of segfault if size_out is NULL? we should try to avoid segfault inside the library. This will change original API behavior, but I think it is okay since it is in the user space. > attr.test.repeat = repeat; > > ret = sys_bpf(BPF_PROG_TEST_RUN, , sizeof(attr)); > - if (size_out) > + if (data_out) > *size_out = attr.test.data_size_out; > if (retval) > *retval = attr.test.retval; > diff --git a/tools/testing/selftests/bpf/test_progs.c > b/tools/testing/selftests/bpf/test_progs.c > index 2d3c04f45530..560d7527b86b 100644 > --- a/tools/testing/selftests/bpf/test_progs.c > +++ b/tools/testing/selftests/bpf/test_progs.c > @@ -150,6 +150,7 @@ static void test_xdp(void) > bpf_map_update_elem(map_fd, , , 0); > bpf_map_update_elem(map_fd, , , 0); > > + size = sizeof(buf); > err = bpf_prog_test_run(prog_fd, 1, _v4, sizeof(pkt_v4), > buf, , , ); > > @@ -158,6 +159,7 @@ static void test_xdp(void) > "err %d errno %d retval %d size %d\n", > err, errno, retval, size); > > + size = sizeof(buf); > err = bpf_prog_test_run(prog_fd, 1, _v6, sizeof(pkt_v6), > buf, , , ); > CHECK(err || retval != XDP_TX || size != 114 || > @@ -182,6 +184,7 @@ static void test_xdp_adjust_tail(void) > return; > } > > + size = sizeof(buf); > err = bpf_prog_test_run(prog_fd, 1, _v4, sizeof(pkt_v4), > buf, , , ); > > @@ -189,6 +192,7 @@ static void test_xdp_adjust_tail(void) > "ipv4", "err %d errno %d retval %d size %d\n", > err, errno, retval, size); > > + size = sizeof(buf); > err = bpf_prog_test_run(prog_fd, 1, _v6, sizeof(pkt_v6), > buf, , , ); > CHECK(err || retval != XDP_TX || size != 54, > @@ -252,6 +256,7 @@ static void test_l4lb(const char *file) > goto out; > bpf_map_update_elem(map_fd, _num, _def, 0); > > + size = sizeof(buf); > err = bpf_prog_test_run(prog_fd, NUM_ITER, _v4, sizeof(pkt_v4), > buf, , , ); > CHECK(err || retval != 7/*TC_ACT_REDIRECT*/ || size != 54 || > @@ -259,6 +264,7 @@ static void test_l4lb(const char *file) > "err %d errno %d retval %d size %d magic %x\n", > err, errno, retval, size, *magic); > > + size = sizeof(buf); > err = bpf_prog_test_run(prog_fd, NUM_ITER, _v6, sizeof(pkt_v6), > buf, , , ); > CHECK(err || retval != 7/*TC_ACT_REDIRECT*/ || size != 74 || > @@ -341,6 +347,7 @@ static void test_xdp_noinline(void) > goto out; > bpf_map_update_elem(map_fd, _num, _def, 0); > > + size = sizeof(buf); > err = bpf_prog_test_run(prog_fd, NUM_ITER, _v4, sizeof(pkt_v4), > buf, , , ); > CHECK(err || retval != 1 || size != 54 || > @@ -348,6 +355,7 @@ static void test_xdp_noinline(void) > "err %d errno %d retval %d size %d magic %x\n", > err, errno, retval, size, *magic); > > + size = sizeof(buf); > err = bpf_prog_test_run(prog_fd, NUM_ITER, _v6, sizeof(pkt_v6), > buf, , , ); > CHECK(err || retval != 1 || size != 74 || > @@ -1795,6 +1803,7 @@ static void test_queue_stack_map(int type) > pkt_v4.iph.saddr = vals[MAP_SIZE - 1 - i] * 5; > } > > + size = sizeof(buf); > err = bpf_prog_test_run(prog_fd, 1, _v4, sizeof(pkt_v4), > buf, , , ); > if (err || retval || size != sizeof(pkt_v4) || > @@ -1808,6 +1817,7 @@ static void test_queue_stack_map(int type) > err, errno, retval, size, iph->daddr); > > /* Queue is empty, program should return
Re: [PATCH 1/3] bpf: respect size hint to BPF_PROG_TEST_RUN if present
On Fri, Nov 16, 2018 at 12:54 PM Lorenz Bauer wrote:
>
> Use data_size_out as a size hint when copying test output to user space.
> A program using BPF_PERF_OUTPUT can compare its own buffer length with
> data_size_out after the syscall to detect whether truncation has taken
> place. Callers which so far did not set data_size_in are not affected.
>
> Signed-off-by: Lorenz Bauer
> ---
> net/bpf/test_run.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
> index c89c22c49015..30c57b7f4ba4 100644
> --- a/net/bpf/test_run.c
> +++ b/net/bpf/test_run.c
> @@ -74,8 +74,15 @@ static int bpf_test_finish(const union bpf_attr *kattr,
> {
> void __user *data_out = u64_to_user_ptr(kattr->test.data_out);
> int err = -EFAULT;
> + u32 copy_size = size;
>
> - if (data_out && copy_to_user(data_out, data, size))
> + /* Clamp copy if the user has provided a size hint, but copy the full
> +* buffer if not to retain old behaviour.
> +*/
> + if (kattr->test.data_size_out && copy_size >
> kattr->test.data_size_out)
> + copy_size = kattr->test.data_size_out;
> +
> + if (data_out && copy_to_user(data_out, data, copy_size))
> goto out;
> if (copy_to_user(>test.data_size_out, , sizeof(size)))
> goto out;
if copy_size < size, maybe we should return -ENOSPC so user space is aware
of insufficient size and takes proper action? This behavior will then
be consistent
with BPF_PROG_QUERY subcommand where prog_cnt is the in/out parameter and
-ENOSPC is returned if kernel does not have enough space to copy out
the whole data.
Also, since data_size_out field now has more complex semantics, could you add
some comments in uapi/linux/bpf.h so it will be relatively clear from
uapi header
how this field will be used?
> --
> 2.17.1
>
Re: [PATCH v3 0/4] bpf: allow zero-initialising hash map seed
> On Nov 16, 2018, at 3:41 AM, Lorenz Bauer wrote: > > Allow forcing the seed of a hash table to zero, for deterministic > execution during benchmarking and testing. > > Changes from v2: > * Change ordering of BPF_F_ZERO_SEED in linux/bpf.h > > Comments adressed from v1: > * Add comment to discourage production use to linux/bpf.h > * Require CAP_SYS_ADMIN > > Lorenz Bauer (4): > bpf: allow zero-initializing hash map seed > bpf: move BPF_F_QUERY_EFFECTIVE after map flags > tools: sync linux/bpf.h > tools: add selftest for BPF_F_ZERO_SEED > > include/uapi/linux/bpf.h| 9 ++-- > kernel/bpf/hashtab.c| 13 - > tools/include/uapi/linux/bpf.h | 13 +++-- > tools/testing/selftests/bpf/test_maps.c | 68 + > 4 files changed, 84 insertions(+), 19 deletions(-) > > -- > 2.17.1 >
Re: [PATCH v3 0/4] bpf: allow zero-initialising hash map seed
> On Nov 16, 2018, at 3:41 AM, Lorenz Bauer wrote: > > Allow forcing the seed of a hash table to zero, for deterministic > execution during benchmarking and testing. > > Changes from v2: > * Change ordering of BPF_F_ZERO_SEED in linux/bpf.h > > Comments adressed from v1: > * Add comment to discourage production use to linux/bpf.h > * Require CAP_SYS_ADMIN > > Lorenz Bauer (4): > bpf: allow zero-initializing hash map seed > bpf: move BPF_F_QUERY_EFFECTIVE after map flags > tools: sync linux/bpf.h > tools: add selftest for BPF_F_ZERO_SEED > > include/uapi/linux/bpf.h| 9 ++-- > kernel/bpf/hashtab.c| 13 - > tools/include/uapi/linux/bpf.h | 13 +++-- > tools/testing/selftests/bpf/test_maps.c | 68 + > 4 files changed, 84 insertions(+), 19 deletions(-) > > -- > 2.17.1 > For the series: Acked-by: Song Liu
Re: [PATCH bpf-next v2 02/13] bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO
On 11/8/18 2:56 PM, Edward Cree wrote:
> On 08/11/18 19:42, Alexei Starovoitov wrote:
>> same link let's continue at 1pm PST.
> So, one thing we didn't really get onto was maps, and you mentioned that it
> wasn't really clear what I was proposing there.
> What I have in mind comes in two parts:
> 1) map type. A new BTF_KIND_MAP with metadata 'key_type', 'value_type'
> (both are type_ids referencing other BTF type records), describing the
> type "map from key_type to value_type".
> 2) record in the 'instances' table. This would have a name_off (the
> name of the map), a type_id (pointing at a BTF_KIND_MAP in the 'types'
> table), and potentially also some indication of what symbol (from
> section 'maps') refers to this map. This is pretty much the exact
> same metadata that a function in the 'instances' table has, the only
> differences being
> (a) function's type_id points at a BTF_KIND_FUNC record
> (b) function's symbol indication refers from .text section
> (c) in future functions may be nested inside other functions, whereas
> AIUI a map can't live inside a function. (But a variable, which is
> the other thing that would want to go in an 'instances' table, can.)
> So the 'instances' table record structure looks like
>
> struct btf_instance {
> __u32 type_id; /* Type of object declared. An index into type section */
> __u32 name_off; /* Name of object. An offset into string section */
> __u32 parent; /* Containing object if any (else 0). An index into
> instance section */
> };
>
> and we extend the BTF header:
>
> struct btf_header {
> __u16 magic;
> __u8 version;
> __u8 flags;
> __u32 hdr_len;
>
> /* All offsets are in bytes relative to the end of this header */
> __u32 type_off; /* offset of type section */
> __u32 type_len; /* length of type section */
> __u32 str_off; /* offset of string section */
> __u32 str_len; /* length of string section */
> __u32 inst_off; /* offset of instance section */
> __u32 inst_len; /* length of instance section */
> };
>
> Then in the .BTF.ext section, we have both
>
> struct bpf_func_info {
> __u32 prog_symbol; /* Index of symbol giving address of subprog */
> __u32 inst_id; /* Index into instance section */
> }
>
> struct bpf_map_info {
> {
> __u32 map_symbol; /* Index of symbol creating this map */
> __u32 inst_id; /* Index into instance section */
> }
>
> (either living in different subsections, or in a single table with
> the addition of a kind field, or in a single table relying on the
> ultimately referenced type to distinguish funcs from maps).
>
> Note that the name (in btf_instance) of a map or function need not
> match the name of the corresponding symbol; we use the .BTF.ext
> section to tie together btf_instance IDs and symbol IDs. Then in
> the case of functions (subprogs), the prog_symbol can be looked
> up in the ELF symbol table to find the address (== insn_offset)
> of the subprog, as well as the section containing it (since that
> might not be .text). Similarly in the case of maps the BTF info
> about the map is connected with the info in the maps section.
>
> Now when the loader has munged this, what it passes to the kernel
> might not have map_symbol, but instead map_fd. Instead of
> prog_symbol it will have whatever identifies the subprog in the
> blob of stuff it feeds to the kernel (so probably insn_offset).
>
> All this would of course require a bit more compiler support than
> the current BPF_ANNOTATE_KV_PAIR, since that just causes the
> existing BTF machinery to declare a specially constructed struct
> type. At the C level you could still have BPF_ANNOTATE_KV_PAIR
> and the 'bpf_map_foo' name, but then the compiler would
> recognise that and convert it into an instance record by looking
> up the name 'foo' in its "maps" section. That way the special
> bpf_map_* handling (which ties map names to symbol names,
Compiler in general does not do transformation based on variable
or struct type names by default, so this probably should stay
in the loader.
> also) would be entirely compiler-internal and not 'leak out' into
> the definition of the format. Frontends for other languages
> which do possess a native map type (e.g. Python dict) might have
> other ways of indicating the key/value type of a map at source
> level (e.g. PEP 484) and could directly generate the appropriate
> BTF_KIND_MAP and bpf_map_info records rather than (as they would
> with the current design) having to encode the information as a
> struct bpf_map_foo type-definition.
You mean a python application can generate bpf byte codes and
BTFs (include map types)? That will be different from the C/LLVM
use case. The python app. probably will be the loader as well.
One option is to pass BPF
Re: [PATCH bpf-next v4 02/13] bpf: btf: Add BTF_KIND_FUNC
On 11/8/18 12:52 PM, Edward Cree wrote:
> On 08/11/18 20:36, Yonghong Song wrote:
>> This patch adds BTF_KIND_FUNC support to the type section.
>> BTF_KIND_FUNC is used to specify the signature of a
>> defined subprogram or the pointee of a function pointer.
>>
>> In BTF, the function type related data structures are
>>struct bpf_param {
>> __u32 name_off; /* parameter name */
>> __u32 type; /* parameter type */
>>};
>>struct bpf_type {
>> __u32 name_off; /* function name */
>> __u32 info; /* BTF_KIND_FUNC and num of parameters (#vlen) */
>> __u32 type; /* return type */
>>}
>> The data layout of the function type:
>>struct bpf_type
>>#vlen number of bpf_param's
>>
>> For a defined subprogram with valid function body,
>>. function name and all parameter names except the vararg
>> must be valid C identifier.
> Given that there's an intention to support other frontends besides
> C, what's the reason for this restriction?
This (C) is the typical usage today. If later on other frontend
generates bpf programs with more relaxed symbol name requirement,
we can certainly relax the rule.
>
>> For the pointee of a function pointer,
>>. function name and all parameter names will
>> have name_off = 0 to indicate a non-existing name.
> Why can't function pointer parameters have names?
Currently, both bcc and llvm does not retain function pointer
arguments in dwarf. For LLVM, the IR generation for function pointer
type discards the argument name. So I did the checking because
llvm does not generate it.
We can relax the restrictions later if the compiler starts
to keep argument names in the IR.
> E.g. imagine something like struct net_device_ops. All those
> function pointers have named parameters and that's relevant info
> when debugging.
>
> -Ed
>
[PATCH bpf-next v4 12/13] tools/bpf: enhance test_btf file testing to test func info
Change the bpf programs test_btf_haskv.c and test_btf_nokv.c to
have two sections, and enhance test_btf.c test_file feature
to test btf func_info returned by the kernel.
Signed-off-by: Yonghong Song
---
tools/testing/selftests/bpf/test_btf.c | 117 +--
tools/testing/selftests/bpf/test_btf_haskv.c | 16 ++-
tools/testing/selftests/bpf/test_btf_nokv.c | 16 ++-
3 files changed, 136 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_btf.c
b/tools/testing/selftests/bpf/test_btf.c
index a5fcc30781d5..08a07358851b 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -2226,13 +2226,13 @@ static struct btf_file_test file_tests[] = {
},
};
-static int file_has_btf_elf(const char *fn)
+static int file_has_btf_elf(const char *fn, bool *has_btf_ext)
{
Elf_Scn *scn = NULL;
GElf_Ehdr ehdr;
+ int ret = 0;
int elf_fd;
Elf *elf;
- int ret;
if (CHECK(elf_version(EV_CURRENT) == EV_NONE,
"elf_version(EV_CURRENT) == EV_NONE"))
@@ -2264,14 +2264,12 @@ static int file_has_btf_elf(const char *fn)
}
sh_name = elf_strptr(elf, ehdr.e_shstrndx, sh.sh_name);
- if (!strcmp(sh_name, BTF_ELF_SEC)) {
+ if (!strcmp(sh_name, BTF_ELF_SEC))
ret = 1;
- goto done;
- }
+ if (!strcmp(sh_name, BTF_EXT_ELF_SEC))
+ *has_btf_ext = true;
}
- ret = 0;
-
done:
close(elf_fd);
elf_end(elf);
@@ -2281,15 +2279,24 @@ static int file_has_btf_elf(const char *fn)
static int do_test_file(unsigned int test_num)
{
const struct btf_file_test *test = _tests[test_num - 1];
+ const char *expected_fnames[] = {"_dummy_tracepoint",
+"test_long_fname_1",
+"test_long_fname_2"};
+ struct bpf_prog_info info = {};
struct bpf_object *obj = NULL;
+ struct bpf_func_info *finfo;
struct bpf_program *prog;
+ __u32 info_len, rec_size;
+ bool has_btf_ext = false;
+ struct btf *btf = NULL;
+ void *func_info = NULL;
struct bpf_map *map;
- int err;
+ int i, err, prog_fd;
fprintf(stderr, "BTF libbpf test[%u] (%s): ", test_num,
test->file);
- err = file_has_btf_elf(test->file);
+ err = file_has_btf_elf(test->file, _btf_ext);
if (err == -1)
return err;
@@ -2317,6 +2324,7 @@ static int do_test_file(unsigned int test_num)
err = bpf_object__load(obj);
if (CHECK(err < 0, "bpf_object__load: %d", err))
goto done;
+ prog_fd = bpf_program__fd(prog);
map = bpf_object__find_map_by_name(obj, "btf_map");
if (CHECK(!map, "btf_map not found")) {
@@ -2331,9 +2339,100 @@ static int do_test_file(unsigned int test_num)
test->btf_kv_notfound))
goto done;
+ if (!jit_enabled || !has_btf_ext)
+ goto skip_jit;
+
+ /* get necessary program info */
+ info_len = sizeof(struct bpf_prog_info);
+ err = bpf_obj_get_info_by_fd(prog_fd, , _len);
+
+ if (CHECK(err == -1, "invalid get info (1st) errno:%d", errno)) {
+ fprintf(stderr, "%s\n", btf_log_buf);
+ err = -1;
+ goto done;
+ }
+ if (CHECK(info.func_info_cnt != 3,
+ "incorrect info.func_info_cnt (1st) %d",
+ info.func_info_cnt)) {
+ err = -1;
+ goto done;
+ }
+ rec_size = info.func_info_rec_size;
+ if (CHECK(rec_size < 4,
+ "incorrect info.func_info_rec_size (1st) %d\n", rec_size)) {
+ err = -1;
+ goto done;
+ }
+
+ func_info = malloc(info.func_info_cnt * rec_size);
+ if (CHECK(!func_info, "out of memeory")) {
+ err = -1;
+ goto done;
+ }
+
+ /* reset info to only retrieve func_info related data */
+ memset(, 0, sizeof(info));
+ info.func_info_cnt = 3;
+ info.func_info_rec_size = rec_size;
+ info.func_info = ptr_to_u64(func_info);
+
+ err = bpf_obj_get_info_by_fd(prog_fd, , _len);
+
+ if (CHECK(err == -1, "invalid get info (2nd) errno:%d", errno)) {
+ fprintf(stderr, "%s\n", btf_log_buf);
+ err = -1;
+ goto done;
+ }
+ if (CHECK(info.func_info_cnt != 3,
+ "incorrect info.func_info_cnt (2nd) %d",
+ info.func_info_cnt)) {
+ err = -1;
+ goto done;
+ }
+ if (CHECK(info.func_info_
[PATCH bpf-next v4 10/13] tools/bpf: do not use pahole if clang/llvm can generate BTF sections
Add additional checks in tools/testing/selftests/bpf and
samples/bpf such that if clang/llvm compiler can generate
BTF sections, do not use pahole.
Signed-off-by: Yonghong Song
---
samples/bpf/Makefile | 8
tools/testing/selftests/bpf/Makefile | 8
2 files changed, 16 insertions(+)
diff --git a/samples/bpf/Makefile b/samples/bpf/Makefile
index be0a961450bc..35444f4a846b 100644
--- a/samples/bpf/Makefile
+++ b/samples/bpf/Makefile
@@ -208,12 +208,20 @@ endif
BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris)
BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF)
BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i
'usage.*llvm')
+BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \
+ $(CLANG) -target bpf -O2 -g -c -x c - -o
./llvm_btf_verify.o; \
+ readelf -S ./llvm_btf_verify.o | grep BTF; \
+ /bin/rm -f ./llvm_btf_verify.o)
+ifneq ($(BTF_LLVM_PROBE),)
+ EXTRA_CFLAGS += -g
+else
ifneq ($(and $(BTF_LLC_PROBE),$(BTF_PAHOLE_PROBE),$(BTF_OBJCOPY_PROBE)),)
EXTRA_CFLAGS += -g
LLC_FLAGS += -mattr=dwarfris
DWARF2BTF = y
endif
+endif
# Trick to allow make to be run from this directory
all:
diff --git a/tools/testing/selftests/bpf/Makefile
b/tools/testing/selftests/bpf/Makefile
index e39dfb4e7970..63a321894419 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -124,7 +124,14 @@ $(OUTPUT)/test_stack_map.o: test_queue_stack_map.h
BTF_LLC_PROBE := $(shell $(LLC) -march=bpf -mattr=help 2>&1 | grep dwarfris)
BTF_PAHOLE_PROBE := $(shell $(BTF_PAHOLE) --help 2>&1 | grep BTF)
BTF_OBJCOPY_PROBE := $(shell $(LLVM_OBJCOPY) --help 2>&1 | grep -i
'usage.*llvm')
+BTF_LLVM_PROBE := $(shell echo "int main() { return 0; }" | \
+ $(CLANG) -target bpf -O2 -g -c -x c - -o
./llvm_btf_verify.o; \
+ readelf -S ./llvm_btf_verify.o | grep BTF; \
+ /bin/rm -f ./llvm_btf_verify.o)
+ifneq ($(BTF_LLVM_PROBE),)
+ CLANG_FLAGS += -g
+else
ifneq ($(BTF_LLC_PROBE),)
ifneq ($(BTF_PAHOLE_PROBE),)
ifneq ($(BTF_OBJCOPY_PROBE),)
@@ -134,6 +141,7 @@ ifneq ($(BTF_OBJCOPY_PROBE),)
endif
endif
endif
+endif
$(OUTPUT)/%.o: %.c
$(CLANG) $(CLANG_FLAGS) \
--
2.17.1
[PATCH bpf-next v4 13/13] tools/bpf: bpftool: add support for func types
This patch added support to print function signature
if btf func_info is available. Note that ksym
now uses function name instead of prog_name as
prog_name has a limit of 16 bytes including
ending '\0'.
The following is a sample output for selftests
test_btf with file test_btf_haskv.o for translated insns
and jited insns respectively.
$ bpftool prog dump xlated id 1
int _dummy_tracepoint(struct dummy_tracepoint_args * arg):
0: (85) call pc+2#bpf_prog_2dcecc18072623fc_test_long_fname_1
1: (b7) r0 = 0
2: (95) exit
int test_long_fname_1(struct dummy_tracepoint_args * arg):
3: (85) call pc+1#bpf_prog_89d64e4abf0f0126_test_long_fname_2
4: (95) exit
int test_long_fname_2(struct dummy_tracepoint_args * arg):
5: (b7) r2 = 0
6: (63) *(u32 *)(r10 -4) = r2
7: (79) r1 = *(u64 *)(r1 +8)
...
22: (07) r1 += 1
23: (63) *(u32 *)(r0 +4) = r1
24: (95) exit
$ bpftool prog dump jited id 1
int _dummy_tracepoint(struct dummy_tracepoint_args * arg):
bpf_prog_b07ccb89267cf242__dummy_tracepoint:
0: push %rbp
1: mov%rsp,%rbp
..
3c: add$0x28,%rbp
40: leaveq
41: retq
int test_long_fname_1(struct dummy_tracepoint_args * arg):
bpf_prog_2dcecc18072623fc_test_long_fname_1:
0: push %rbp
1: mov%rsp,%rbp
..
3a: add$0x28,%rbp
3e: leaveq
3f: retq
int test_long_fname_2(struct dummy_tracepoint_args * arg):
bpf_prog_89d64e4abf0f0126_test_long_fname_2:
0: push %rbp
1: mov%rsp,%rbp
..
80: add$0x28,%rbp
84: leaveq
85: retq
Signed-off-by: Yonghong Song
---
tools/bpf/bpftool/btf_dumper.c| 98 +++
tools/bpf/bpftool/main.h | 2 +
tools/bpf/bpftool/prog.c | 56 ++
tools/bpf/bpftool/xlated_dumper.c | 33 +++
tools/bpf/bpftool/xlated_dumper.h | 3 +
5 files changed, 192 insertions(+)
diff --git a/tools/bpf/bpftool/btf_dumper.c b/tools/bpf/bpftool/btf_dumper.c
index 55bc512a1831..64b63d53a143 100644
--- a/tools/bpf/bpftool/btf_dumper.c
+++ b/tools/bpf/bpftool/btf_dumper.c
@@ -249,3 +249,101 @@ int btf_dumper_type(const struct btf_dumper *d, __u32
type_id,
{
return btf_dumper_do_type(d, type_id, 0, data);
}
+
+#define BTF_PRINT_ARG(...) \
+ do {\
+ pos += snprintf(func_sig + pos, size - pos, \
+ __VA_ARGS__); \
+ if (pos >= size)\
+ return -1; \
+ } while (0)
+#define BTF_PRINT_TYPE(type) \
+ do {\
+ pos = __btf_dumper_type_only(btf, type, func_sig, \
+pos, size);\
+ if (pos == -1) \
+ return -1; \
+ } while (0)
+
+static int __btf_dumper_type_only(struct btf *btf, __u32 type_id,
+ char *func_sig, int pos, int size)
+{
+ const struct btf_type *t = btf__type_by_id(btf, type_id);
+ const struct btf_array *array;
+ int i, vlen;
+
+ switch (BTF_INFO_KIND(t->info)) {
+ case BTF_KIND_INT:
+ BTF_PRINT_ARG("%s ", btf__name_by_offset(btf, t->name_off));
+ break;
+ case BTF_KIND_STRUCT:
+ BTF_PRINT_ARG("struct %s ",
+ btf__name_by_offset(btf, t->name_off));
+ break;
+ case BTF_KIND_UNION:
+ BTF_PRINT_ARG("union %s ",
+ btf__name_by_offset(btf, t->name_off));
+ break;
+ case BTF_KIND_ENUM:
+ BTF_PRINT_ARG("enum %s ",
+ btf__name_by_offset(btf, t->name_off));
+ break;
+ case BTF_KIND_ARRAY:
+ array = (struct btf_array *)(t + 1);
+ BTF_PRINT_TYPE(array->type);
+ BTF_PRINT_ARG("[%d]", array->nelems);
+ break;
+ case BTF_KIND_PTR:
+ BTF_PRINT_TYPE(t->type);
+ BTF_PRINT_ARG("* ");
+ break;
+ case BTF_KIND_UNKN:
+ case BTF_KIND_FWD:
+ case BTF_KIND_TYPEDEF:
+ return -1;
+ case BTF_KIND_VOLATILE:
+ BTF_PRINT_ARG("volatile ");
+ BTF_PRINT_TYPE(t->type);
+ break;
+ case BTF_KIND_CONST:
+ BTF_PRINT_ARG("const ");
+ BTF_PRINT_TYPE(t->type);
+
[PATCH bpf-next v4 04/13] tools/bpf: add btf func unit tests in selftest test_btf
Add several BTF_KIND_FUNC unit tests in bpf selftest test_btf.
Signed-off-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
tools/lib/bpf/btf.c| 3 +
tools/testing/selftests/bpf/test_btf.c | 262 +
2 files changed, 265 insertions(+)
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 449591aa9900..6da816d64ca1 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -165,6 +165,9 @@ static int btf_parse_type_sec(struct btf *btf,
btf_print_fn_t err_log)
case BTF_KIND_ENUM:
next_type += vlen * sizeof(struct btf_enum);
break;
+ case BTF_KIND_FUNC:
+ next_type += vlen * sizeof(struct btf_param);
+ break;
case BTF_KIND_TYPEDEF:
case BTF_KIND_PTR:
case BTF_KIND_FWD:
diff --git a/tools/testing/selftests/bpf/test_btf.c
b/tools/testing/selftests/bpf/test_btf.c
index f42b3396d622..0e63c99c44b1 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -1374,6 +1374,268 @@ static struct btf_raw_test raw_tests[] = {
.map_create_err = true,
},
+{
+ .descr = "func pointer #1",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_TYPE_INT_ENC(0, 0, 0, 32, 4), /* [2] */
+ /* int (*func)(int, unsigned int) */
+ BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 2), 1), /* [3]
*/
+ 0, 1, 0, 2,
+ BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 3), /* [4]
*/
+ BTF_END_RAW,
+ },
+ .str_sec = "",
+ .str_sec_size = sizeof(""),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "func_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 4,
+},
+
+{
+ .descr = "func pointer #2",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_TYPE_INT_ENC(0, 0, 0, 32, 4), /* [2] */
+ /* void (*func)(int, unsigned int, ) */
+ BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 3), 0), /* [3]
*/
+ 0, 1, 0, 2, 0, 0,
+ BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 3), /* [4]
*/
+ BTF_END_RAW,
+ },
+ .str_sec = "",
+ .str_sec_size = sizeof(""),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "func_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 4,
+},
+
+{
+ .descr = "func pointer #3",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_TYPE_INT_ENC(0, 0, 0, 32, 4), /* [2] */
+ /* void (*func)(void, int, unsigned int) */
+ BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 3), 0), /* [3]
*/
+ 0, 1, 0, 0, 0, 2,
+ BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 3), /* [4]
*/
+ BTF_END_RAW,
+ },
+ .str_sec = "",
+ .str_sec_size = sizeof(""),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "func_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+ .key_type_id = 1,
+ .value_type_id = 1,
+ .max_entries = 4,
+ .btf_load_err = true,
+ .err_str = "Invalid arg#2",
+},
+
+{
+ .descr = "func pointer #4",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_TYPE_INT_ENC(0, 0, 0, 32, 4), /* [2] */
+ /*
+* Testing:
+* BTF_KIND_CONST => BTF_KIND_TYPEDEF => BTF_KIND_PTR =>
+* BTF_KIND_FUNC
+*/
+ /* typedef void (*func_ptr)(int, unsigned int) */
+ BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_TYPEDEF, 0, 0), 5),/* [3]
*/
+ /* const func_ptr */
+ BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_CONST, 0, 0), 3), /* [4]
*/
+ BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_PTR, 0, 0), 6), /* [5]
*/
+ BTF_TYPE_ENC(0, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 2), 0), /* [6]
*/
+ 0, 1, 0, 2,
+ BTF_END_RAW,
+ },
+ .str_sec = "",
+ .str_sec_size = sizeof(""),
+ .map_type = BPF_MAP_TYPE_ARRAY,
+ .map_name = "func_type_check_btf",
+ .key_size = sizeof(int),
+ .value_size = sizeof(int),
+
[PATCH bpf-next v4 11/13] tools/bpf: refactor to implement btf_get_from_id() in lib/bpf
The function get_btf() is implemented in tools/bpf/bpftool/map.c
to get a btf structure given a map_info. This patch
refactored this function to be function btf_get_from_id()
in tools/lib/bpf so that it can be used later.
Signed-off-by: Yonghong Song
---
tools/bpf/bpftool/map.c | 68 ++--
tools/lib/bpf/btf.c | 69 +
tools/lib/bpf/btf.h | 1 +
3 files changed, 72 insertions(+), 66 deletions(-)
diff --git a/tools/bpf/bpftool/map.c b/tools/bpf/bpftool/map.c
index 7bf38f0e152e..1b8a75fa0471 100644
--- a/tools/bpf/bpftool/map.c
+++ b/tools/bpf/bpftool/map.c
@@ -215,70 +215,6 @@ static int do_dump_btf(const struct btf_dumper *d,
return ret;
}
-static int get_btf(struct bpf_map_info *map_info, struct btf **btf)
-{
- struct bpf_btf_info btf_info = { 0 };
- __u32 len = sizeof(btf_info);
- __u32 last_size;
- int btf_fd;
- void *ptr;
- int err;
-
- err = 0;
- *btf = NULL;
- btf_fd = bpf_btf_get_fd_by_id(map_info->btf_id);
- if (btf_fd < 0)
- return 0;
-
- /* we won't know btf_size until we call bpf_obj_get_info_by_fd(). so
-* let's start with a sane default - 4KiB here - and resize it only if
-* bpf_obj_get_info_by_fd() needs a bigger buffer.
-*/
- btf_info.btf_size = 4096;
- last_size = btf_info.btf_size;
- ptr = malloc(last_size);
- if (!ptr) {
- err = -ENOMEM;
- goto exit_free;
- }
-
- bzero(ptr, last_size);
- btf_info.btf = ptr_to_u64(ptr);
- err = bpf_obj_get_info_by_fd(btf_fd, _info, );
-
- if (!err && btf_info.btf_size > last_size) {
- void *temp_ptr;
-
- last_size = btf_info.btf_size;
- temp_ptr = realloc(ptr, last_size);
- if (!temp_ptr) {
- err = -ENOMEM;
- goto exit_free;
- }
- ptr = temp_ptr;
- bzero(ptr, last_size);
- btf_info.btf = ptr_to_u64(ptr);
- err = bpf_obj_get_info_by_fd(btf_fd, _info, );
- }
-
- if (err || btf_info.btf_size > last_size) {
- err = errno;
- goto exit_free;
- }
-
- *btf = btf__new((__u8 *)btf_info.btf, btf_info.btf_size, NULL);
- if (IS_ERR(*btf)) {
- err = PTR_ERR(*btf);
- *btf = NULL;
- }
-
-exit_free:
- close(btf_fd);
- free(ptr);
-
- return err;
-}
-
static json_writer_t *get_btf_writer(void)
{
json_writer_t *jw = jsonw_new(stdout);
@@ -765,7 +701,7 @@ static int do_dump(int argc, char **argv)
prev_key = NULL;
- err = get_btf(, );
+ err = btf_get_from_id(info.btf_id, );
if (err) {
p_err("failed to get btf");
goto exit_free;
@@ -909,7 +845,7 @@ static int do_lookup(int argc, char **argv)
}
/* here means bpf_map_lookup_elem() succeeded */
- err = get_btf(, );
+ err = btf_get_from_id(info.btf_id, );
if (err) {
p_err("failed to get btf");
goto exit_free;
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 11579ed4ef6c..2a914ab65cbc 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -49,6 +49,11 @@ struct bpf_func_info_min {
__u32 type_id;
};
+static inline __u64 ptr_to_u64(const void *ptr)
+{
+ return (__u64) (unsigned long) ptr;
+}
+
static int btf_add_type(struct btf *btf, struct btf_type *t)
{
if (btf->types_size - btf->nr_types < 2) {
@@ -409,6 +414,70 @@ const char *btf__name_by_offset(const struct btf *btf,
__u32 offset)
return NULL;
}
+int btf_get_from_id(__u32 id, struct btf **btf)
+{
+ struct bpf_btf_info btf_info = { 0 };
+ __u32 len = sizeof(btf_info);
+ __u32 last_size;
+ int btf_fd;
+ void *ptr;
+ int err;
+
+ err = 0;
+ *btf = NULL;
+ btf_fd = bpf_btf_get_fd_by_id(id);
+ if (btf_fd < 0)
+ return 0;
+
+ /* we won't know btf_size until we call bpf_obj_get_info_by_fd(). so
+* let's start with a sane default - 4KiB here - and resize it only if
+* bpf_obj_get_info_by_fd() needs a bigger buffer.
+*/
+ btf_info.btf_size = 4096;
+ last_size = btf_info.btf_size;
+ ptr = malloc(last_size);
+ if (!ptr) {
+ err = -ENOMEM;
+ goto exit_free;
+ }
+
+ bzero(ptr, last_size);
+ btf_info.btf = ptr_to_u64(ptr);
+ err = bpf_obj_get_info_by_fd(btf_fd, _info, );
+
+ if (!err && btf_info.btf_size > last_size) {
+ void *temp_ptr;
+
+ last_size = btf_info.btf_size;
+ temp_ptr = realloc(ptr, last_size);
+ if (!temp_ptr) {
+
[PATCH bpf-next v4 09/13] tools/bpf: add support to read .BTF.ext sections
The .BTF section is already available to encode types.
These types can be used for map
pretty print. The whole .BTF will be passed to the
kernel as well for which kernel can verify and return
to the user space for pretty print etc.
The llvm patch at https://reviews.llvm.org/D53736
will generate .BTF section and one more section .BTF.ext.
The .BTF.ext section encodes function type
information and line information. Note that
this patch set only supports function type info.
The functionality is implemented in libbpf.
The .BTF section can be directly loaded into the
kernel, and the .BTF.ext section cannot. The loader
may need to do some relocation and merging,
similar to merging multiple code sections, before
loading into the kernel.
Acked-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
tools/lib/bpf/bpf.c| 46 ++-
tools/lib/bpf/btf.c| 274 +
tools/lib/bpf/btf.h| 50
tools/lib/bpf/libbpf.c | 87 ++---
4 files changed, 442 insertions(+), 15 deletions(-)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 06fc5e91ac51..ce1822194590 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -177,6 +177,7 @@ int bpf_load_program_xattr(const struct
bpf_load_program_attr *load_attr,
char *log_buf, size_t log_buf_sz)
{
union bpf_attr attr;
+ void *finfo = NULL;
__u32 name_len;
int fd;
@@ -207,12 +208,55 @@ int bpf_load_program_xattr(const struct
bpf_load_program_attr *load_attr,
if (fd >= 0 || !log_buf || !log_buf_sz)
return fd;
+ /* After bpf_prog_load, the kernel may modify certain attributes
+* to give user space a hint how to deal with loading failure.
+* Check to see whether we can make some changes and load again.
+*/
+ if (errno == E2BIG && attr.func_info_cnt &&
+ attr.func_info_rec_size < load_attr->func_info_rec_size) {
+ __u32 actual_rec_size = load_attr->func_info_rec_size;
+ __u32 expected_rec_size = attr.func_info_rec_size;
+ __u32 finfo_cnt = load_attr->func_info_cnt;
+ __u64 finfo_len = actual_rec_size * finfo_cnt;
+ const void *orecord;
+ void *nrecord;
+ int i;
+
+ finfo = malloc(finfo_len);
+ if (!finfo)
+ /* further try with log buffer won't help */
+ return fd;
+
+ /* zero out bytes kernel does not understand */
+ orecord = load_attr->func_info;
+ nrecord = finfo;
+ for (i = 0; i < load_attr->func_info_cnt; i++) {
+ memcpy(nrecord, orecord, expected_rec_size);
+ memset(nrecord + expected_rec_size, 0,
+ actual_rec_size - expected_rec_size);
+ orecord += actual_rec_size;
+ nrecord += actual_rec_size;
+ }
+
+ /* try with corrected func info records */
+ attr.func_info = ptr_to_u64(finfo);
+ attr.func_info_rec_size = load_attr->func_info_rec_size;
+
+ fd = sys_bpf(BPF_PROG_LOAD, , sizeof(attr));
+
+ if (fd >= 0 || !log_buf || !log_buf_sz)
+ goto done;
+ }
+
/* Try again with log */
attr.log_buf = ptr_to_u64(log_buf);
attr.log_size = log_buf_sz;
attr.log_level = 1;
log_buf[0] = 0;
- return sys_bpf(BPF_PROG_LOAD, , sizeof(attr));
+ fd = sys_bpf(BPF_PROG_LOAD, , sizeof(attr));
+done:
+ free(finfo);
+ return fd;
}
int bpf_load_program(enum bpf_prog_type type, const struct bpf_insn *insns,
diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 6da816d64ca1..11579ed4ef6c 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -37,6 +37,18 @@ struct btf {
int fd;
};
+struct btf_ext {
+ void *func_info;
+ __u32 func_info_rec_size;
+ __u32 func_info_len;
+};
+
+/* The minimum bpf_func_info checked by the loader */
+struct bpf_func_info_min {
+ __u32 insn_offset;
+ __u32 type_id;
+};
+
static int btf_add_type(struct btf *btf, struct btf_type *t)
{
if (btf->types_size - btf->nr_types < 2) {
@@ -396,3 +408,265 @@ const char *btf__name_by_offset(const struct btf *btf,
__u32 offset)
else
return NULL;
}
+
+static int btf_ext_validate_func_info(const void *finfo, __u32 size,
+ btf_print_fn_t err_log)
+{
+ int sec_hdrlen = sizeof(struct btf_sec_func_info);
+ __u32 size_left, num_records, record_size;
+ const struct btf_sec_func_info *sinfo;
+ __u64 total_record_size;
+
+ /* At least a func_info record size */
+ if (size < sizeof(__u32)) {
+
[PATCH bpf-next v4 08/13] tools/bpf: extends test_btf to test load/retrieve func_type info
A two function bpf program is loaded with btf and func_info.
After successful prog load, the bpf_get_info syscall is called
to retrieve prog info to ensure the types returned from the
kernel matches the types passed to the kernel from the
user space.
Several negative tests are also added to test loading/retriving
of func_type info.
Acked-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
tools/testing/selftests/bpf/test_btf.c | 315 -
1 file changed, 312 insertions(+), 3 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_btf.c
b/tools/testing/selftests/bpf/test_btf.c
index 0e63c99c44b1..a5fcc30781d5 100644
--- a/tools/testing/selftests/bpf/test_btf.c
+++ b/tools/testing/selftests/bpf/test_btf.c
@@ -5,6 +5,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -22,9 +23,13 @@
#include "bpf_rlimit.h"
#include "bpf_util.h"
+#define MAX_INSNS 512
+#define MAX_SUBPROGS 16
+
static uint32_t pass_cnt;
static uint32_t error_cnt;
static uint32_t skip_cnt;
+static bool jit_enabled;
#define CHECK(condition, format...) ({ \
int __ret = !!(condition); \
@@ -60,6 +65,24 @@ static int __base_pr(const char *format, ...)
return err;
}
+static bool is_jit_enabled(void)
+{
+ const char *jit_sysctl = "/proc/sys/net/core/bpf_jit_enable";
+ bool enabled = false;
+ int sysctl_fd;
+
+ sysctl_fd = open(jit_sysctl, 0, O_RDONLY);
+ if (sysctl_fd != -1) {
+ char tmpc;
+
+ if (read(sysctl_fd, , sizeof(tmpc)) == 1)
+ enabled = (tmpc != '0');
+ close(sysctl_fd);
+ }
+
+ return enabled;
+}
+
#define BTF_INFO_ENC(kind, root, vlen) \
((!!(root) << 31) | ((kind) << 24) | ((vlen) & BTF_MAX_VLEN))
@@ -103,6 +126,7 @@ static struct args {
bool get_info_test;
bool pprint_test;
bool always_log;
+ bool func_type_test;
} args;
static char btf_log_buf[BTF_LOG_BUF_SIZE];
@@ -2739,16 +2763,293 @@ static int test_pprint(void)
return err;
}
+static struct btf_func_type_test {
+ const char *descr;
+ const char *str_sec;
+ __u32 raw_types[MAX_NR_RAW_TYPES];
+ __u32 str_sec_size;
+ struct bpf_insn insns[MAX_INSNS];
+ __u32 prog_type;
+ __u32 func_info[MAX_SUBPROGS][2];
+ __u32 func_info_rec_size;
+ __u32 func_info_cnt;
+ bool expected_prog_load_failure;
+} func_type_test[] = {
+
+{
+ .descr = "func_type test #1",
+ .str_sec = "\0int\0unsigned int\0funcA\0a\0b\0funcB\0a\0b",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(NAME_TBD, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_TYPE_INT_ENC(NAME_TBD, 0, 0, 32, 4), /* [2] */
+ BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 2), 1),
/* [3] */
+ NAME_TBD, 1, NAME_TBD, 2,
+ BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 2), 1),
/* [4] */
+ NAME_TBD, 2, NAME_TBD, 1,
+ BTF_END_RAW,
+ },
+ .str_sec_size = sizeof("\0int\0unsigned int\0funcA\0a\0b\0funcB\0a\0b"),
+ .insns = {
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ },
+ .prog_type = BPF_PROG_TYPE_TRACEPOINT,
+ .func_info = { {0, 3}, {3, 4} },
+ .func_info_rec_size = 8,
+ .func_info_cnt = 2,
+},
+
+{
+ .descr = "func_type test #2",
+ .str_sec = "\0int\0unsigned int\0funcA\0a\0b\0funcB\0a\0b",
+ .raw_types = {
+ BTF_TYPE_INT_ENC(NAME_TBD, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
+ BTF_TYPE_INT_ENC(NAME_TBD, 0, 0, 32, 4), /* [2] */
+ BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 2), 1),
/* [3] */
+ NAME_TBD, 1, NAME_TBD, 2,
+ BTF_TYPE_ENC(NAME_TBD, BTF_INFO_ENC(BTF_KIND_FUNC, 0, 2), 1),
/* [4] */
+ NAME_TBD, 2, NAME_TBD, 1,
+ BTF_END_RAW,
+ },
+ .str_sec_size = sizeof("\0int\0unsigned int\0funcA\0a\0b\0funcB\0a\0b"),
+ .insns = {
+ BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 1, 0, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 1),
+ BPF_EXIT_INSN(),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ },
+ .prog_type = BPF_PROG_TYPE_TRACEPOINT,
+ .func_info = { {0, 3}, {3, 4} },
+ /* incorrect func_info_rec_size */
+ .func_info_rec_size = 4,
+ .func_info_cnt = 2,
+ .expected_prog_load_failure = true,
+},
+
+{
+ .descr = "func_type test #3",
+
[PATCH bpf-next v4 06/13] tools/bpf: sync kernel uapi bpf.h header to tools directory
The kernel uapi bpf.h is synced to tools directory.
Acked-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
tools/include/uapi/linux/bpf.h | 13 +
1 file changed, 13 insertions(+)
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 852dc17ab47a..28db552a1eed 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -335,6 +335,10 @@ union bpf_attr {
* (context accesses, allowed helpers, etc).
*/
__u32 expected_attach_type;
+ __u32 prog_btf_fd;/* fd pointing to BTF type data
*/
+ __u32 func_info_rec_size; /* userspace
bpf_func_info size */
+ __aligned_u64 func_info; /* func info */
+ __u32 func_info_cnt; /* number of bpf_func_info
records */
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
@@ -2631,6 +2635,10 @@ struct bpf_prog_info {
__u32 nr_jited_func_lens;
__aligned_u64 jited_ksyms;
__aligned_u64 jited_func_lens;
+ __u32 btf_id;
+ __u32 func_info_rec_size;
+ __aligned_u64 func_info;
+ __u32 func_info_cnt;
} __attribute__((aligned(8)));
struct bpf_map_info {
@@ -2942,4 +2950,9 @@ struct bpf_flow_keys {
};
};
+struct bpf_func_info {
+ __u32 insn_offset;
+ __u32 type_id;
+};
+
#endif /* _UAPI__LINUX_BPF_H__ */
--
2.17.1
[PATCH bpf-next v4 01/13] bpf: btf: Break up btf_type_is_void()
This patch breaks up btf_type_is_void() into
btf_type_is_void() and btf_type_is_fwd().
It also adds btf_type_nosize() to better describe it is
testing a type has nosize info.
Signed-off-by: Martin KaFai Lau
---
kernel/bpf/btf.c | 37 ++---
1 file changed, 22 insertions(+), 15 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index ee4c82667d65..2a50d87de485 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -306,15 +306,22 @@ static bool btf_type_is_modifier(const struct btf_type *t)
static bool btf_type_is_void(const struct btf_type *t)
{
- /* void => no type and size info.
-* Hence, FWD is also treated as void.
-*/
- return t == _void || BTF_INFO_KIND(t->info) == BTF_KIND_FWD;
+ return t == _void;
+}
+
+static bool btf_type_is_fwd(const struct btf_type *t)
+{
+ return BTF_INFO_KIND(t->info) == BTF_KIND_FWD;
+}
+
+static bool btf_type_nosize(const struct btf_type *t)
+{
+ return btf_type_is_void(t) || btf_type_is_fwd(t);
}
-static bool btf_type_is_void_or_null(const struct btf_type *t)
+static bool btf_type_nosize_or_null(const struct btf_type *t)
{
- return !t || btf_type_is_void(t);
+ return !t || btf_type_nosize(t);
}
/* union is only a special case of struct:
@@ -826,7 +833,7 @@ const struct btf_type *btf_type_id_size(const struct btf
*btf,
u32 size = 0;
size_type = btf_type_by_id(btf, size_type_id);
- if (btf_type_is_void_or_null(size_type))
+ if (btf_type_nosize_or_null(size_type))
return NULL;
if (btf_type_has_size(size_type)) {
@@ -842,7 +849,7 @@ const struct btf_type *btf_type_id_size(const struct btf
*btf,
size = btf->resolved_sizes[size_type_id];
size_type_id = btf->resolved_ids[size_type_id];
size_type = btf_type_by_id(btf, size_type_id);
- if (btf_type_is_void(size_type))
+ if (btf_type_nosize_or_null(size_type))
return NULL;
}
@@ -1164,7 +1171,7 @@ static int btf_modifier_resolve(struct btf_verifier_env
*env,
}
/* "typedef void new_void", "const void"...etc */
- if (btf_type_is_void(next_type))
+ if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type))
goto resolved;
if (!env_type_is_resolve_sink(env, next_type) &&
@@ -1178,7 +1185,7 @@ static int btf_modifier_resolve(struct btf_verifier_env
*env,
* pretty print).
*/
if (!btf_type_id_size(btf, _type_id, _type_size) &&
- !btf_type_is_void(btf_type_id_resolve(btf, _type_id))) {
+ !btf_type_nosize(btf_type_id_resolve(btf, _type_id))) {
btf_verifier_log_type(env, v->t, "Invalid type_id");
return -EINVAL;
}
@@ -1205,7 +1212,7 @@ static int btf_ptr_resolve(struct btf_verifier_env *env,
}
/* "void *" */
- if (btf_type_is_void(next_type))
+ if (btf_type_is_void(next_type) || btf_type_is_fwd(next_type))
goto resolved;
if (!env_type_is_resolve_sink(env, next_type) &&
@@ -1235,7 +1242,7 @@ static int btf_ptr_resolve(struct btf_verifier_env *env,
}
if (!btf_type_id_size(btf, _type_id, _type_size) &&
- !btf_type_is_void(btf_type_id_resolve(btf, _type_id))) {
+ !btf_type_nosize(btf_type_id_resolve(btf, _type_id))) {
btf_verifier_log_type(env, v->t, "Invalid type_id");
return -EINVAL;
}
@@ -1396,7 +1403,7 @@ static int btf_array_resolve(struct btf_verifier_env *env,
/* Check array->index_type */
index_type_id = array->index_type;
index_type = btf_type_by_id(btf, index_type_id);
- if (btf_type_is_void_or_null(index_type)) {
+ if (btf_type_nosize_or_null(index_type)) {
btf_verifier_log_type(env, v->t, "Invalid index");
return -EINVAL;
}
@@ -1415,7 +1422,7 @@ static int btf_array_resolve(struct btf_verifier_env *env,
/* Check array->type */
elem_type_id = array->type;
elem_type = btf_type_by_id(btf, elem_type_id);
- if (btf_type_is_void_or_null(elem_type)) {
+ if (btf_type_nosize_or_null(elem_type)) {
btf_verifier_log_type(env, v->t,
"Invalid elem");
return -EINVAL;
@@ -1615,7 +1622,7 @@ static int btf_struct_resolve(struct btf_verifier_env
*env,
const struct btf_type *member_type = btf_type_by_id(env->btf,
member_type_id);
- if (btf_type_is_void_or_null(member_type)) {
+ if (btf_type_nosize_or_null(member_type)) {
btf_verifier_log_member(env, v->t, member,
"Invalid member");
return -EINVAL;
--
2.17.1
[PATCH bpf-next v4 07/13] tools/bpf: add new fields for program load in lib/bpf
The new fields are added for program load in lib/bpf so
application uses api bpf_load_program_xattr() is able
to load program with btf and func_info data.
This functionality will be used in next patch
by bpf selftest test_btf.
Acked-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
tools/lib/bpf/bpf.c | 4
tools/lib/bpf/bpf.h | 4
2 files changed, 8 insertions(+)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index 03f9bcc4ef50..06fc5e91ac51 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -196,6 +196,10 @@ int bpf_load_program_xattr(const struct
bpf_load_program_attr *load_attr,
attr.log_level = 0;
attr.kern_version = load_attr->kern_version;
attr.prog_ifindex = load_attr->prog_ifindex;
+ attr.prog_btf_fd = load_attr->prog_btf_fd;
+ attr.func_info_rec_size = load_attr->func_info_rec_size;
+ attr.func_info_cnt = load_attr->func_info_cnt;
+ attr.func_info = ptr_to_u64(load_attr->func_info);
memcpy(attr.prog_name, load_attr->name,
min(name_len, BPF_OBJ_NAME_LEN - 1));
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 26a51538213c..8bdfd806253a 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -74,6 +74,10 @@ struct bpf_load_program_attr {
const char *license;
__u32 kern_version;
__u32 prog_ifindex;
+ __u32 prog_btf_fd;
+ __u32 func_info_rec_size;
+ const void *func_info;
+ __u32 func_info_cnt;
};
/* Flags to direct loading requirements */
--
2.17.1
[PATCH bpf-next v4 02/13] bpf: btf: Add BTF_KIND_FUNC
This patch adds BTF_KIND_FUNC support to the type section.
BTF_KIND_FUNC is used to specify the signature of a
defined subprogram or the pointee of a function pointer.
In BTF, the function type related data structures are
struct bpf_param {
__u32 name_off; /* parameter name */
__u32 type; /* parameter type */
};
struct bpf_type {
__u32 name_off; /* function name */
__u32 info; /* BTF_KIND_FUNC and num of parameters (#vlen) */
__u32 type; /* return type */
}
The data layout of the function type:
struct bpf_type
#vlen number of bpf_param's
For a defined subprogram with valid function body,
. function name and all parameter names except the vararg
must be valid C identifier.
For the pointee of a function pointer,
. function name and all parameter names will
have name_off = 0 to indicate a non-existing name.
As a concrete example, for the C program below,
int foo(int (*bar)(int)) { return bar(5); }
two func types will be generated:
FuncType #1: subprogram "foo" with parameter "bar"
FuncType #2: pointee of function pointer "int (*)(int)"
Signed-off-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
include/uapi/linux/btf.h | 17 ++-
kernel/bpf/btf.c | 309 ++-
2 files changed, 290 insertions(+), 36 deletions(-)
diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index 972265f32871..9e7c74a83ee7 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -40,7 +40,8 @@ struct btf_type {
/* "size" is used by INT, ENUM, STRUCT and UNION.
* "size" tells the size of the type it is describing.
*
-* "type" is used by PTR, TYPEDEF, VOLATILE, CONST and RESTRICT.
+* "type" is used by PTR, TYPEDEF, VOLATILE, CONST, RESTRICT
+* and FUNC.
* "type" is a type_id referring to another type.
*/
union {
@@ -64,8 +65,9 @@ struct btf_type {
#define BTF_KIND_VOLATILE 9 /* Volatile */
#define BTF_KIND_CONST 10 /* Const*/
#define BTF_KIND_RESTRICT 11 /* Restrict */
-#define BTF_KIND_MAX 11
-#define NR_BTF_KINDS 12
+#define BTF_KIND_FUNC 12 /* Function */
+#define BTF_KIND_MAX 12
+#define NR_BTF_KINDS 13
/* For some specific BTF_KIND, "struct btf_type" is immediately
* followed by extra data.
@@ -110,4 +112,13 @@ struct btf_member {
__u32 offset; /* offset in bits */
};
+/* BTF_KIND_FUNC is followed by multiple "struct btf_param".
+ * The exact number of btf_param is stored in the vlen (of the
+ * info in "struct btf_type").
+ */
+struct btf_param {
+ __u32 name_off;
+ __u32 type;
+};
+
#endif /* _UAPI__LINUX_BTF_H__ */
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 2a50d87de485..62140da0c10d 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -5,6 +5,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -259,6 +260,7 @@ static const char * const btf_kind_str[NR_BTF_KINDS] = {
[BTF_KIND_VOLATILE] = "VOLATILE",
[BTF_KIND_CONST]= "CONST",
[BTF_KIND_RESTRICT] = "RESTRICT",
+ [BTF_KIND_FUNC] = "FUNC",
};
struct btf_kind_operations {
@@ -281,6 +283,9 @@ struct btf_kind_operations {
static const struct btf_kind_operations * const kind_ops[NR_BTF_KINDS];
static struct btf_type btf_void;
+static int btf_resolve(struct btf_verifier_env *env,
+ const struct btf_type *t, u32 type_id);
+
static bool btf_type_is_modifier(const struct btf_type *t)
{
/* Some of them is not strictly a C modifier
@@ -314,9 +319,27 @@ static bool btf_type_is_fwd(const struct btf_type *t)
return BTF_INFO_KIND(t->info) == BTF_KIND_FWD;
}
+static bool btf_type_is_func(const struct btf_type *t)
+{
+ return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC;
+}
+
+static bool btf_type_is_func_prog(const struct btf_type *t)
+{
+ return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC &&
+ t->name_off;
+}
+
+static bool btf_type_is_func_proto(const struct btf_type *t)
+{
+ return BTF_INFO_KIND(t->info) == BTF_KIND_FUNC &&
+ !t->name_off;
+}
+
static bool btf_type_nosize(const struct btf_type *t)
{
- return btf_type_is_void(t) || btf_type_is_fwd(t);
+ return btf_type_is_void(t) || btf_type_is_fwd(t) ||
+ btf_type_is_func(t);
}
static bool btf_type_nosize_or_null(const struct btf_type *t)
@@ -433,6 +456,27 @@ static bool btf_name_offset_valid(const struct btf *btf,
u32 offset)
offset < btf->hdr.str_len;
}
+static bool btf_name_valid_identifier(const struct btf *btf, u32 offset)
+{
+ /* offset must be valid */
+
[PATCH bpf-next v4 03/13] tools/bpf: sync kernel btf.h header
The kernel uapi btf.h is synced to the tools directory.
Signed-off-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
tools/include/uapi/linux/btf.h | 17 ++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/tools/include/uapi/linux/btf.h b/tools/include/uapi/linux/btf.h
index 972265f32871..9e7c74a83ee7 100644
--- a/tools/include/uapi/linux/btf.h
+++ b/tools/include/uapi/linux/btf.h
@@ -40,7 +40,8 @@ struct btf_type {
/* "size" is used by INT, ENUM, STRUCT and UNION.
* "size" tells the size of the type it is describing.
*
-* "type" is used by PTR, TYPEDEF, VOLATILE, CONST and RESTRICT.
+* "type" is used by PTR, TYPEDEF, VOLATILE, CONST, RESTRICT
+* and FUNC.
* "type" is a type_id referring to another type.
*/
union {
@@ -64,8 +65,9 @@ struct btf_type {
#define BTF_KIND_VOLATILE 9 /* Volatile */
#define BTF_KIND_CONST 10 /* Const*/
#define BTF_KIND_RESTRICT 11 /* Restrict */
-#define BTF_KIND_MAX 11
-#define NR_BTF_KINDS 12
+#define BTF_KIND_FUNC 12 /* Function */
+#define BTF_KIND_MAX 12
+#define NR_BTF_KINDS 13
/* For some specific BTF_KIND, "struct btf_type" is immediately
* followed by extra data.
@@ -110,4 +112,13 @@ struct btf_member {
__u32 offset; /* offset in bits */
};
+/* BTF_KIND_FUNC is followed by multiple "struct btf_param".
+ * The exact number of btf_param is stored in the vlen (of the
+ * info in "struct btf_type").
+ */
+struct btf_param {
+ __u32 name_off;
+ __u32 type;
+};
+
#endif /* _UAPI__LINUX_BTF_H__ */
--
2.17.1
[PATCH bpf-next v4 05/13] bpf: get better bpf_prog ksyms based on btf func type_id
This patch added interface to load a program with the following
additional information:
. prog_btf_fd
. func_info, func_info_rec_size and func_info_cnt
where func_info will provide function range and type_id
corresponding to each function.
The func_info_rec_size is introduced in the UAPI to specify
struct bpf_func_info size passed from user space. This
intends to make bpf_func_info structure growable in the future.
If the kernel gets a different bpf_func_info size from userspace,
it will try to handle user request with part of bpf_func_info
it can understand. In this patch, kernel can understand
struct bpf_func_info {
__u32 insn_offset;
__u32 type_id;
};
If user passed a bpf func_info record size of 16 bytes, the
kernel can still handle part of records with the above definition.
If verifier agrees with function range provided by the user,
the bpf_prog ksym for each function will use the func name
provided in the type_id, which is supposed to provide better
encoding as it is not limited by 16 bytes program name
limitation and this is better for bpf program which contains
multiple subprograms.
The bpf_prog_info interface is also extended to
return btf_id, func_info, func_info_rec_size and func_info_cnt
to userspace, so userspace can print out the function prototype
for each xlated function. The insn_offset in the returned
func_info corresponds to the insn offset for xlated functions.
With other jit related fields in bpf_prog_info, userspace can also
print out function prototypes for each jited function.
Acked-by: Martin KaFai Lau
Signed-off-by: Yonghong Song
---
include/linux/bpf.h | 5 +-
include/linux/bpf_verifier.h | 1 +
include/linux/btf.h | 2 +
include/uapi/linux/bpf.h | 13
kernel/bpf/btf.c | 4 +-
kernel/bpf/core.c| 13
kernel/bpf/syscall.c | 59 +++--
kernel/bpf/verifier.c| 121 ++-
8 files changed, 210 insertions(+), 8 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 33014ae73103..5835aa70e301 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -312,6 +312,8 @@ struct bpf_prog_aux {
void *security;
#endif
struct bpf_prog_offload *offload;
+ struct btf *btf;
+ u32 type_id; /* type id for this prog/func */
union {
struct work_struct work;
struct rcu_head rcu;
@@ -523,7 +525,8 @@ static inline void bpf_long_memcpy(void *dst, const void
*src, u32 size)
}
/* verify correctness of eBPF program */
-int bpf_check(struct bpf_prog **fp, union bpf_attr *attr);
+int bpf_check(struct bpf_prog **fp, union bpf_attr *attr,
+ union bpf_attr __user *uattr);
void bpf_patch_call_args(struct bpf_insn *insn, u32 stack_depth);
/* Map specifics */
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index d93e89761a8b..7f52528e6a06 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -204,6 +204,7 @@ static inline bool bpf_verifier_log_needed(const struct
bpf_verifier_log *log)
struct bpf_subprog_info {
u32 start; /* insn idx of function entry point */
u16 stack_depth; /* max. stack depth used by this function */
+ u32 type_id; /* btf type_id for this subprog */
};
/* single container for all structs
diff --git a/include/linux/btf.h b/include/linux/btf.h
index e076c4697049..7f2c0a4a45ea 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -46,5 +46,7 @@ void btf_type_seq_show(const struct btf *btf, u32 type_id,
void *obj,
struct seq_file *m);
int btf_get_fd_by_id(u32 id);
u32 btf_id(const struct btf *btf);
+const struct btf_type *btf_type_by_id(const struct btf *btf, u32 type_id);
+const char *btf_name_by_offset(const struct btf *btf, u32 offset);
#endif
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 852dc17ab47a..28db552a1eed 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -335,6 +335,10 @@ union bpf_attr {
* (context accesses, allowed helpers, etc).
*/
__u32 expected_attach_type;
+ __u32 prog_btf_fd;/* fd pointing to BTF type data
*/
+ __u32 func_info_rec_size; /* userspace
bpf_func_info size */
+ __aligned_u64 func_info; /* func info */
+ __u32 func_info_cnt; /* number of bpf_func_info
records */
};
struct { /* anonymous struct used by BPF_OBJ_* commands */
@@ -2631,6 +2635,10 @@ struct bpf_prog_info {
__u32 nr_jited_func_lens;
__aligned_u64 jited_ksyms;
__aligned_u64 jited_func_lens;
+ __u32 btf_id;
+ __u32 func_info_rec_size;
+ __aligned_u64 func_info;
+ __u32 func_info_cnt;
} __attribute__((aligned(8)));
struct bpf_map_info {
@@ -2942,4
[PATCH bpf-next v4 00/13] bpf: add btf func info support
The BTF support was added to kernel by Commit 69b693f0aefa
("bpf: btf: Introduce BPF Type Format (BTF)"), which introduced
.BTF section into ELF file and is primarily
used for map pretty print.
pahole is used to convert dwarf to BTF for ELF files.
This patch added func info support to the kernel so we can
get better ksym's for bpf function calls. Basically,
function call types are passed to kernel and the kernel
extract function names from these types in order to contruct ksym
for these functions.
The llvm patch at https://reviews.llvm.org/D53736
will generate .BTF section and one more section .BTF.ext.
The .BTF.ext section encodes function type
information. The following is a sample output for selftests
test_btf with file test_btf_haskv.o for translated insns
and jited insns respectively.
$ bpftool prog dump xlated id 1
int _dummy_tracepoint(struct dummy_tracepoint_args * arg):
0: (85) call pc+2#bpf_prog_2dcecc18072623fc_test_long_fname_1
1: (b7) r0 = 0
2: (95) exit
int test_long_fname_1(struct dummy_tracepoint_args * arg):
3: (85) call pc+1#bpf_prog_89d64e4abf0f0126_test_long_fname_2
4: (95) exit
int test_long_fname_2(struct dummy_tracepoint_args * arg):
5: (b7) r2 = 0
6: (63) *(u32 *)(r10 -4) = r2
7: (79) r1 = *(u64 *)(r1 +8)
...
22: (07) r1 += 1
23: (63) *(u32 *)(r0 +4) = r1
24: (95) exit
$ bpftool prog dump jited id 1
int _dummy_tracepoint(struct dummy_tracepoint_args * arg):
bpf_prog_b07ccb89267cf242__dummy_tracepoint:
0: push %rbp
1: mov%rsp,%rbp
..
3c: add$0x28,%rbp
40: leaveq
41: retq
int test_long_fname_1(struct dummy_tracepoint_args * arg):
bpf_prog_2dcecc18072623fc_test_long_fname_1:
0: push %rbp
1: mov%rsp,%rbp
..
3a: add$0x28,%rbp
3e: leaveq
3f: retq
int test_long_fname_2(struct dummy_tracepoint_args * arg):
bpf_prog_89d64e4abf0f0126_test_long_fname_2:
0: push %rbp
1: mov%rsp,%rbp
..
80: add$0x28,%rbp
84: leaveq
85: retq
For the patchset,
Patch #1 refactors the code to break up btf_type_is_void().
Patch #2 introduces new BTF type BTF_KIND_FUNC.
Patch #3 syncs btf.h header to tools directory.
Patch #4 adds btf func type self tests in test_btf.
Patch #5 adds kernel interface to load func_info to kernel
and pass func_info back to userspace.
Patch #6 syncs bpf.h header to tools directory.
Patch #7 adds news btf/func_info related fields in libbpf
program load function.
Patch #8 extends selftest test_btf to test load/retrieve func_info.
Patch #9 adds .BTF.ext func info support.
Patch #10 changes Makefile to avoid using pahole if llvm is capable of
generating BTF sections.
Patch #11 refactors to have btf_get_from_id() in libbpf for reuse.
Patch #12 enhance test_btf file testing to test func info.
Patch #13 adds bpftool support for func signature dump.
Changelogs:
v3 -> v4:
. Remove BTF_KIND_FUNC_PROTO. BTF_KIND_FUNC is used for
both function pointer and subprogram. The name_off field
is used to distinguish both.
. The record size is added to the func_info subsection
in .BTF.ext to enable future extension.
. The bpf_prog_info interface change to make it similar
bpf_prog_load.
. Related kernel and libbpf changes to accommodate the
new .BTF.ext and kernel interface changes.
v2 -> v3:
. Removed kernel btf extern functions btf_type_id_func()
and btf_get_name_by_id(). Instead, exposing existing
functions btf_type_by_id() and btf_name_by_offset().
. Added comments about ELF section .BTF.ext layout.
. Better codes in btftool as suggested by Edward Cree.
v1 -> v2:
. Added missing sign-off.
. Limited the func_name/struct_member_name length for validity test.
. Removed/changed several verifier messages.
. Modified several commit messages to remove line_off reference.
Yonghong Song (13):
bpf: btf: Break up btf_type_is_void()
bpf: btf: Add BTF_KIND_FUNC
tools/bpf: sync kernel btf.h header
tools/bpf: add btf func unit tests in selftest test_btf
bpf: get better bpf_prog ksyms based on btf func type_id
tools/bpf: sync kernel uapi bpf.h header to tools directory
tools/bpf: add new fields for program load in lib/bpf
tools/bpf: extends test_btf to test load/retrieve func_type info
tools/bpf: add support to read .BTF.ext sections
tools/bpf: do not use pahole if clang/llvm can generate BTF sections
tools/bpf: refactor to implement btf_get_from_id() in lib/bpf
tools/bpf: enhance test_btf file testing to test func info
tools/bpf: bpftool: add support for func types
include/linux/bpf.h | 5 +-
include/linux/bpf_verifier.h | 1 +
include/linux/btf.h | 2 +
include/uapi/linux/bpf.h | 13 +
inclu
Re: [RFC perf,bpf 1/5] perf, bpf: Introduce PERF_RECORD_BPF_EVENT
Hi Peter,
> On Nov 8, 2018, at 7:00 AM, Peter Zijlstra wrote:
>
> On Wed, Nov 07, 2018 at 06:25:04PM +0000, Song Liu wrote:
>>
>>
>>> On Nov 7, 2018, at 12:40 AM, Peter Zijlstra wrote:
>>>
>>> On Tue, Nov 06, 2018 at 12:52:42PM -0800, Song Liu wrote:
>>>> For better performance analysis of BPF programs, this patch introduces
>>>> PERF_RECORD_BPF_EVENT, a new perf_event_type that exposes BPF program
>>>> load/unload information to user space.
>>>>
>>>> /*
>>>>* Record different types of bpf events:
>>>>* enum perf_bpf_event_type {
>>>>* PERF_BPF_EVENT_UNKNOWN = 0,
>>>>* PERF_BPF_EVENT_PROG_LOAD= 1,
>>>>* PERF_BPF_EVENT_PROG_UNLOAD = 2,
>>>>* };
>>>>*
>>>>* struct {
>>>>* struct perf_event_header header;
>>>>* u16 type;
>>>>* u16 flags;
>>>>* u32 id; // prog_id or map_id
>>>>* };
>>>>*/
>>>> PERF_RECORD_BPF_EVENT = 17,
>>>>
>>>> PERF_RECORD_BPF_EVENT contains minimal information about the BPF program.
>>>> Perf utility (or other user space tools) should listen to this event and
>>>> fetch more details about the event via BPF syscalls
>>>> (BPF_PROG_GET_FD_BY_ID, BPF_OBJ_GET_INFO_BY_FD, etc.).
>>>
>>> Why !? You're failing to explain why it cannot provide the full
>>> information there.
>>
>> Aha, I missed this part. I will add the following to next version. Please
>> let me know if anything is not clear.
>
>>
>> This design decision is picked for the following reasons. First, BPF
>> programs could be loaded-and-jited and/or unloaded before/during/after
>> perf-record run. Once a BPF programs is unloaded, it is impossible to
>> recover details of the program. It is impossible to provide the
>> information through a simple key (like the build ID). Second, BPF prog
>> annotation is under fast developments. Multiple informations will be
>> added to bpf_prog_info in the next few releases. Including all the
>> information of a BPF program in the perf ring buffer requires frequent
>> changes to the perf ABI, and thus makes it very difficult to manage
>> compatibility of perf utility.
>
> So I don't agree with that reasoning. If you want symbol information
> you'll just have to commit to some form of ABI. That bpf_prog_info is an
> ABI too.
At the beginning of the perf-record run, perf need to query bpf_prog_info
of already loaded BPF programs. Therefore, we need to commit to the
bpf_prog_info ABI. If we also include full information of the BPF program
in the perf ring buffer, we will commit to TWO ABIs.
Also, perf-record write the event to perf.data file, so the data need to be
serialized. This is implemented in patch 4/5. To include the data in the
ring buffer, we will need another piece of code in the kernel to do the
same serialization work.
On the other hand, processing BPF load/unload events synchronously should
not introduce too much overhead for meaningful use cases. If many BPF progs
are being loaded/unloaded within short period of time, it is not the steady
state that profiling works care about.
Would these resolve your concerns?
Thanks,
Song
[PATCH bpf v2] tools/bpftool: copy a few net uapi headers to tools directory
Commit f6f3bac08ff9 ("tools/bpf: bpftool: add net support")
added certain networking support to bpftool.
The implementation relies on a relatively recent uapi header file
linux/tc_act/tc_bpf.h on the host which contains the marco
definition of TCA_ACT_BPF_ID.
Unfortunately, this is not the case for all distributions.
See the email message below where rhel-7.2 does not have
an up-to-date linux/tc_act/tc_bpf.h.
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1799211.html
Further investigation found that linux/pkt_cls.h is also needed for macro
TCA_BPF_TAG.
This patch fixed the issue by copying linux/tc_act/tc_bpf.h
and linux/pkt_cls.h from kernel include/uapi directory to
tools/include/uapi directory so building the bpftool does not depend
on host system for these files.
Fixes: f6f3bac08ff9 ("tools/bpf: bpftool: add net support")
Reported-by: kernel test robot
Cc: Li Zhijian
Signed-off-by: Yonghong Song
---
tools/include/uapi/linux/pkt_cls.h | 612 +++
tools/include/uapi/linux/tc_act/tc_bpf.h | 37 ++
2 files changed, 649 insertions(+)
create mode 100644 tools/include/uapi/linux/pkt_cls.h
create mode 100644 tools/include/uapi/linux/tc_act/tc_bpf.h
Changelogs:
v1 -> v2:
. copy linux/pkt_cls.h as well.
diff --git a/tools/include/uapi/linux/pkt_cls.h
b/tools/include/uapi/linux/pkt_cls.h
new file mode 100644
index ..401d0c1e612d
--- /dev/null
+++ b/tools/include/uapi/linux/pkt_cls.h
@@ -0,0 +1,612 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef __LINUX_PKT_CLS_H
+#define __LINUX_PKT_CLS_H
+
+#include
+#include
+
+#define TC_COOKIE_MAX_SIZE 16
+
+/* Action attributes */
+enum {
+ TCA_ACT_UNSPEC,
+ TCA_ACT_KIND,
+ TCA_ACT_OPTIONS,
+ TCA_ACT_INDEX,
+ TCA_ACT_STATS,
+ TCA_ACT_PAD,
+ TCA_ACT_COOKIE,
+ __TCA_ACT_MAX
+};
+
+#define TCA_ACT_MAX __TCA_ACT_MAX
+#define TCA_OLD_COMPAT (TCA_ACT_MAX+1)
+#define TCA_ACT_MAX_PRIO 32
+#define TCA_ACT_BIND 1
+#define TCA_ACT_NOBIND 0
+#define TCA_ACT_UNBIND 1
+#define TCA_ACT_NOUNBIND 0
+#define TCA_ACT_REPLACE1
+#define TCA_ACT_NOREPLACE 0
+
+#define TC_ACT_UNSPEC (-1)
+#define TC_ACT_OK 0
+#define TC_ACT_RECLASSIFY 1
+#define TC_ACT_SHOT2
+#define TC_ACT_PIPE3
+#define TC_ACT_STOLEN 4
+#define TC_ACT_QUEUED 5
+#define TC_ACT_REPEAT 6
+#define TC_ACT_REDIRECT7
+#define TC_ACT_TRAP8 /* For hw path, this means "trap to cpu"
+ * and don't further process the frame
+ * in hardware. For sw path, this is
+ * equivalent of TC_ACT_STOLEN - drop
+ * the skb and act like everything
+ * is alright.
+ */
+#define TC_ACT_VALUE_MAX TC_ACT_TRAP
+
+/* There is a special kind of actions called "extended actions",
+ * which need a value parameter. These have a local opcode located in
+ * the highest nibble, starting from 1. The rest of the bits
+ * are used to carry the value. These two parts together make
+ * a combined opcode.
+ */
+#define __TC_ACT_EXT_SHIFT 28
+#define __TC_ACT_EXT(local) ((local) << __TC_ACT_EXT_SHIFT)
+#define TC_ACT_EXT_VAL_MASK ((1 << __TC_ACT_EXT_SHIFT) - 1)
+#define TC_ACT_EXT_OPCODE(combined) ((combined) & (~TC_ACT_EXT_VAL_MASK))
+#define TC_ACT_EXT_CMP(combined, opcode) (TC_ACT_EXT_OPCODE(combined) ==
opcode)
+
+#define TC_ACT_JUMP __TC_ACT_EXT(1)
+#define TC_ACT_GOTO_CHAIN __TC_ACT_EXT(2)
+#define TC_ACT_EXT_OPCODE_MAX TC_ACT_GOTO_CHAIN
+
+/* Action type identifiers*/
+enum {
+ TCA_ID_UNSPEC=0,
+ TCA_ID_POLICE=1,
+ /* other actions go here */
+ __TCA_ID_MAX=255
+};
+
+#define TCA_ID_MAX __TCA_ID_MAX
+
+struct tc_police {
+ __u32 index;
+ int action;
+#define TC_POLICE_UNSPEC TC_ACT_UNSPEC
+#define TC_POLICE_OK TC_ACT_OK
+#define TC_POLICE_RECLASSIFY TC_ACT_RECLASSIFY
+#define TC_POLICE_SHOT TC_ACT_SHOT
+#define TC_POLICE_PIPE TC_ACT_PIPE
+
+ __u32 limit;
+ __u32 burst;
+ __u32 mtu;
+ struct tc_ratespec rate;
+ struct tc_ratespec peakrate;
+ int refcnt;
+ int bindcnt;
+ __u32 capab;
+};
+
+struct tcf_t {
+ __u64 install;
+ __u64 lastuse;
+ __u64 expires;
+ __u64 firstuse;
+};
+
+struct tc_cnt {
+ int refcnt;
+ int bindcnt;
+};
+
+#define tc_gen \
+ __u32 index; \
+ __u32 capab; \
+ int action; \
+ int
Re: [PATCH bpf] tools/bpftool: copy uapi/linux/tc_act/tc_bpf.h to tools directory
On 11/7/18 5:15 PM, Li Zhijian wrote:
> On 11/8/2018 9:00 AM, Yonghong Song wrote:
>> Commit f6f3bac08ff9 ("tools/bpf: bpftool: add net support")
>> added certain networking support to bpftool.
>> The implementation relies on a relatively recent uapi header file
>> linux/tc_act/tc_bpf.h on the host which contains the marco
>> definition of TCA_ACT_BPF_ID.
>>
>> Unfortunately, this is not the case for all distributions.
>> See the email message below where rhel-7.2 does not have
>> an up-to-date linux/tc_act/tc_bpf.h.
>>
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mail-2Darchive.com_linux-2Dkernel-40vger.kernel.org_msg1799211.html=DwICaQ=5VD0RTtNlTh3ycd41b3MUw=DA8e1B5r073vIqRrFz7MRA=BQIJZUzQfMmUkyvlAHMs7zygIFBysR_MlkAyHN59-7E=pVed3MjGDG_PMeBrhrqb3m57NRinnlcdL1QjAOj4iLA=
>>
>>
>
> i have not tested this patch, but basing on the early commit
> 6f3bac08ff9 ("tools/bpf: bpftool: add net support")
> i cooked up similar patch locally, but i noticed that it also requires an
> up-to-date linux/pkt_cls.h as well to avoid compiling errors:
Thanks for testing. I will add linux/pkt_cls.h as well for the
version 2.
>
> root@lkp-bdw-ep3 ~/linux-f6f3bac08f/tools/bpf/bpftool# make V=1
> [...snip...]
> gcc -O2 -W -Wall -Wextra -Wno-unused-parameter -Wshadow
> -Wno-missing-field-initializers -DPACKAGE='"bpftool"'
> -D__EXPORTED_HEADERS__ -I/root/linux-f6f3bac08f/kernel/bpf/
> -I/root/linux-f6f3bac08f/tools/include
> -I/root/linux-f6f3bac08f/tools/include/uapi
> -I/root/linux-f6f3bac08f/tools/lib/bpf
> -I/root/linux-f6f3bac08f/tools/perf -DBPFTOOL_VERSION='"4.19.0-rc2"'
> -DCOMPAT_NEED_REALLOCARRAY -c -MMD -o netlink_dumper.o netlink_dumper.c
> make -C /root/linux-f6f3bac08f/tools/lib/bpf/ OUTPUT= libbpf.a
> make[1]: Entering directory '/root/linux-f6f3bac08f/tools/lib/bpf'
> netlink_dumper.c: In function 'do_bpf_filter_dump':
> netlink_dumper.c:153:9: error: 'TCA_BPF_ID' undeclared (first use in
> this function)
> if (tb[TCA_BPF_ID])
> ^~
> netlink_dumper.c:153:9: note: each undeclared identifier is reported
> only once for each function it appears in
> netlink_dumper.c:155:9: error: 'TCA_BPF_TAG' undeclared (first use in
> this function)
> if (tb[TCA_BPF_TAG])
> ^~~
> Makefile:96: recipe for target 'netlink_dumper.o' failed
> make: *** [netlink_dumper.o] Error 1
> make: *** Waiting for unfinished jobs
> make -f /root/linux-f6f3bac08f/tools/build/Makefile.build dir=. obj=libbpf
> make[1]: Leaving directory '/root/linux-f6f3bac08f/tools/lib/bpf'
>
> Thanks
> Zhijian
>
>>
>> This patch fixed the issue by copying linux/tc_act/tc_bpf.h from
>> kernel include/uapi directory to tools/include/uapi directory so
>> building the bpftool does not depend on host system for this file.
>>
>> Fixes: f6f3bac08ff9 ("tools/bpf: bpftool: add net support")
>> Reported-by: kernel test robot
>> Signed-off-by: Yonghong Song
>> ---
>> tools/include/uapi/linux/tc_act/tc_bpf.h | 37
>> 1 file changed, 37 insertions(+)
>> create mode 100644 tools/include/uapi/linux/tc_act/tc_bpf.h
>>
>> diff --git a/tools/include/uapi/linux/tc_act/tc_bpf.h
>> b/tools/include/uapi/linux/tc_act/tc_bpf.h
>> new file mode 100644
>> index ..6e89a5df49a4
>> --- /dev/null
>> +++ b/tools/include/uapi/linux/tc_act/tc_bpf.h
>> @@ -0,0 +1,37 @@
>> +/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
>> +/*
>> + * Copyright (c) 2015 Jiri Pirko
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + */
>> +
>> +#ifndef __LINUX_TC_BPF_H
>> +#define __LINUX_TC_BPF_H
>> +
>> +#include
>> +
>> +#define TCA_ACT_BPF 13
>> +
>> +struct tc_act_bpf {
>> + tc_gen;
>> +};
>> +
>> +enum {
>> + TCA_ACT_BPF_UNSPEC,
>> + TCA_ACT_BPF_TM,
>> + TCA_ACT_BPF_PARMS,
>> + TCA_ACT_BPF_OPS_LEN,
>> + TCA_ACT_BPF_OPS,
>> + TCA_ACT_BPF_FD,
>> + TCA_ACT_BPF_NAME,
>> + TCA_ACT_BPF_PAD,
>> + TCA_ACT_BPF_TAG,
>> + TCA_ACT_BPF_ID,
>> + __TCA_ACT_BPF_MAX,
>> +};
>> +#define TCA_ACT_BPF_MAX (__TCA_ACT_BPF_MAX - 1)
>> +
>> +#endif
[PATCH bpf] tools/bpftool: copy uapi/linux/tc_act/tc_bpf.h to tools directory
Commit f6f3bac08ff9 ("tools/bpf: bpftool: add net support")
added certain networking support to bpftool.
The implementation relies on a relatively recent uapi header file
linux/tc_act/tc_bpf.h on the host which contains the marco
definition of TCA_ACT_BPF_ID.
Unfortunately, this is not the case for all distributions.
See the email message below where rhel-7.2 does not have
an up-to-date linux/tc_act/tc_bpf.h.
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1799211.html
This patch fixed the issue by copying linux/tc_act/tc_bpf.h from
kernel include/uapi directory to tools/include/uapi directory so
building the bpftool does not depend on host system for this file.
Fixes: f6f3bac08ff9 ("tools/bpf: bpftool: add net support")
Reported-by: kernel test robot
Signed-off-by: Yonghong Song
---
tools/include/uapi/linux/tc_act/tc_bpf.h | 37
1 file changed, 37 insertions(+)
create mode 100644 tools/include/uapi/linux/tc_act/tc_bpf.h
diff --git a/tools/include/uapi/linux/tc_act/tc_bpf.h
b/tools/include/uapi/linux/tc_act/tc_bpf.h
new file mode 100644
index ..6e89a5df49a4
--- /dev/null
+++ b/tools/include/uapi/linux/tc_act/tc_bpf.h
@@ -0,0 +1,37 @@
+/* SPDX-License-Identifier: GPL-2.0+ WITH Linux-syscall-note */
+/*
+ * Copyright (c) 2015 Jiri Pirko
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#ifndef __LINUX_TC_BPF_H
+#define __LINUX_TC_BPF_H
+
+#include
+
+#define TCA_ACT_BPF 13
+
+struct tc_act_bpf {
+ tc_gen;
+};
+
+enum {
+ TCA_ACT_BPF_UNSPEC,
+ TCA_ACT_BPF_TM,
+ TCA_ACT_BPF_PARMS,
+ TCA_ACT_BPF_OPS_LEN,
+ TCA_ACT_BPF_OPS,
+ TCA_ACT_BPF_FD,
+ TCA_ACT_BPF_NAME,
+ TCA_ACT_BPF_PAD,
+ TCA_ACT_BPF_TAG,
+ TCA_ACT_BPF_ID,
+ __TCA_ACT_BPF_MAX,
+};
+#define TCA_ACT_BPF_MAX (__TCA_ACT_BPF_MAX - 1)
+
+#endif
--
2.17.1
Re: [PATCH v2 1/3] bpf: allow zero-initializing hash map seed
On Thu, Oct 25, 2018 at 8:12 AM Lorenz Bauer wrote:
>
> On Tue, 9 Oct 2018 at 01:08, Song Liu wrote:
> >
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -253,6 +253,8 @@ enum bpf_attach_type {
> > > #define BPF_F_NO_COMMON_LRU(1U << 1)
> > > /* Specify numa node during map creation */
> > > #define BPF_F_NUMA_NODE(1U << 2)
> > > +/* Zero-initialize hash function seed. This should only be used for
> > > testing. */
> > > +#define BPF_F_ZERO_SEED(1U << 6)
> >
> > Please add this line after
> > #define BPF_F_STACK_BUILD_ID(1U << 5)
>
> I wanted to keep the flags for BPF_MAP_CREATE grouped together.
> Maybe the correct value is (1U << 3)? It seemed like the other flags
> were allocated to avoid
> overlap between different BPF commands, however, so I tried to follow suit.
I think it should be (1U << 6). We probably should move BPF_F_QUERY_EFFECTIVE
to after BPF_F_STACK_BUILD_ID (and BPF_F_ZERO_SEED).
Also, please rebase against the latest bpf-next tree and resubmit the set.
Thanks,
Song
Re: [PATCH bpf-next] bpf_load: add map name to load_maps error message
On Mon, Oct 29, 2018 at 3:12 PM John Fastabend wrote:
>
> On 10/29/2018 02:14 PM, Shannon Nelson wrote:
> > To help when debugging bpf/xdp load issues, have the load_map()
> > error message include the number and name of the map that
> > failed.
> >
> > Signed-off-by: Shannon Nelson
> > ---
> > samples/bpf/bpf_load.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/samples/bpf/bpf_load.c b/samples/bpf/bpf_load.c
> > index 89161c9..5de0357 100644
> > --- a/samples/bpf/bpf_load.c
> > +++ b/samples/bpf/bpf_load.c
> > @@ -282,8 +282,8 @@ static int load_maps(struct bpf_map_data *maps, int
> > nr_maps,
> > numa_node);
> > }
> > if (map_fd[i] < 0) {
> > - printf("failed to create a map: %d %s\n",
> > -errno, strerror(errno));
> > + printf("failed to create map %d (%s): %d %s\n",
> > +i, maps[i].name, errno, strerror(errno));
> > return 1;
> > }
> > maps[i].fd = map_fd[i];
> >
>
> LGTM
>
> Acked-by: John Fastabend
Acked-by: Song Liu
Re: Help with the BPF verifier
On 11/2/18 8:42 AM, Edward Cree wrote:
> On 02/11/18 15:02, Arnaldo Carvalho de Melo wrote:
>> Yeah, didn't work as well:
>
>> And the -vv in 'perf trace' didn't seem to map to further details in the
>> output of the verifier debug:
> Yeah for log_level 2 you probably need to make source-level changes to either
> perf or libbpf (I think the latter). It's annoying that essentially no
> tools
> plumb through an option for that, someone should fix them ;-)
>
>> libbpf: -- BEGIN DUMP LOG ---
>> libbpf:
>> 0: (bf) r6 = r1
>> 1: (bf) r1 = r10
>> 2: (07) r1 += -328
>> 3: (b7) r7 = 64
>> 4: (b7) r2 = 64
>> 5: (bf) r3 = r6
>> 6: (85) call bpf_probe_read#4
>> 7: (79) r1 = *(u64 *)(r10 -320)
>> 8: (15) if r1 == 0x101 goto pc+4
>> R0=inv(id=0) R1=inv(id=0) R6=ctx(id=0,off=0,imm=0) R7=inv64 R10=fp0,call_-1
>> 9: (55) if r1 != 0x2 goto pc+22
>> R0=inv(id=0) R1=inv2 R6=ctx(id=0,off=0,imm=0) R7=inv64 R10=fp0,call_-1
>> 10: (bf) r1 = r6
>> 11: (07) r1 += 16
>> 12: (05) goto pc+2
>> 15: (79) r3 = *(u64 *)(r1 +0)
>> dereference of modified ctx ptr R1 off=16 disallowed
> Aha, we at least got a different error message this time.
> And indeed llvm has done that optimisation, rather than the more obvious
> 11: r3 = *(u64 *)(r1 +16)
> because it wants to have lots of reads share a single insn. You may be able
> to defeat that optimisation by adding compiler barriers, idk. Maybe someone
> with llvm knowledge can figure out how to stop it (ideally, llvm would know
> when it's generating for bpf backend and not do that). -O0? ¯\_(ツ)_/¯
The optimization mostly likes below:
br1:
...
r1 += 16
goto merge
br2:
...
r1 += 20
goto merge
merge:
*(u64 *)(r1 + 0)
The compiler tries to merge common loads. There is no easy way to
stop this compiler optimization without turning off a lot of other
optimizations. The easiest way is to add barriers
__asm__ __volatile__("": : :"memory")
after the ctx memory access to prevent their down stream merging.
> Alternatively, your prog looks short enough that maybe you could kick the C
> habit and write it directly in eBPF asm, that way no-one is optimising
> things
> behind your back. (I realise this option won't appeal to everyone ;-)
The LLVM supports BPF inline assembly as well. Some examples here
https://github.com/llvm-mirror/llvm/blob/master/test/CodeGen/BPF/inline_asm.ll
You may try it for selective ctx access to work around some
compiler optimizations. I personally have not used it yet and actually
not sure whether it actually works or not :-)
> The reason the verifier disallows this, iirc, is because it needs to be able
> to rewrite the offsets on ctx accesses (see convert_ctx_accesses()) in case
> > underlying kernel struct doesn't match the layout of the ctx ABI.
To do this
> it needs the ctx offset to live entirely in the insn doing the access,
> otherwise different paths could lead to the same insn accessing different
> ctx
> offsets with different fixups needed — can't be done.
>
> -Ed
>
[PATCH v2 bpf 3/3] bpf: show main program address and length in bpf_prog_info
Currently, when there is no subprog (prog->aux->func_cnt == 0),
bpf_prog_info does not return any jited_ksyms or jited_func_lens. This
patch adds main program address (prog->bpf_func) and main program
length (prog->jited_len) to bpf_prog_info.
Signed-off-by: Song Liu
---
kernel/bpf/syscall.c | 33 -
1 file changed, 24 insertions(+), 9 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 34a9eef5992c..9418174c276c 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2158,11 +2158,11 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog
*prog,
}
ulen = info.nr_jited_ksyms;
- info.nr_jited_ksyms = prog->aux->func_cnt;
+ info.nr_jited_ksyms = prog->aux->func_cnt ? : 1;
if (info.nr_jited_ksyms && ulen) {
if (bpf_dump_raw_ok()) {
+ unsigned long ksym_addr;
u64 __user *user_ksyms;
- ulong ksym_addr;
u32 i;
/* copy the address of the kernel symbol
@@ -2170,9 +2170,17 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
*/
ulen = min_t(u32, info.nr_jited_ksyms, ulen);
user_ksyms = u64_to_user_ptr(info.jited_ksyms);
- for (i = 0; i < ulen; i++) {
- ksym_addr = (ulong)
prog->aux->func[i]->bpf_func;
- if (put_user((u64) ksym_addr, _ksyms[i]))
+ if (prog->aux->func_cnt) {
+ for (i = 0; i < ulen; i++) {
+ ksym_addr = (unsigned long)
+ prog->aux->func[i]->bpf_func;
+ if (put_user((u64) ksym_addr,
+_ksyms[i]))
+ return -EFAULT;
+ }
+ } else {
+ ksym_addr = (unsigned long) prog->bpf_func;
+ if (put_user((u64) ksym_addr, _ksyms[0]))
return -EFAULT;
}
} else {
@@ -2181,7 +2189,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
}
ulen = info.nr_jited_func_lens;
- info.nr_jited_func_lens = prog->aux->func_cnt;
+ info.nr_jited_func_lens = prog->aux->func_cnt ? : 1;
if (info.nr_jited_func_lens && ulen) {
if (bpf_dump_raw_ok()) {
u32 __user *user_lens;
@@ -2190,9 +2198,16 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
/* copy the JITed image lengths for each function */
ulen = min_t(u32, info.nr_jited_func_lens, ulen);
user_lens = u64_to_user_ptr(info.jited_func_lens);
- for (i = 0; i < ulen; i++) {
- func_len = prog->aux->func[i]->jited_len;
- if (put_user(func_len, _lens[i]))
+ if (prog->aux->func_cnt) {
+ for (i = 0; i < ulen; i++) {
+ func_len =
+ prog->aux->func[i]->jited_len;
+ if (put_user(func_len, _lens[i]))
+ return -EFAULT;
+ }
+ } else {
+ func_len = prog->jited_len;
+ if (put_user(func_len, _lens[0]))
return -EFAULT;
}
} else {
--
2.17.1
[PATCH v2 bpf 0/3] show more accurrate bpf program address
Changes v1 -> v2: 1. Added main program length to bpf_prog_info->jited_fun_lens (3/3). 2. Updated commit message of 1/3 and 2/3 with more background about the address masking, and why it is still save after the changes. 3. Replace "ulong" with "unsigned long". This set improves bpf program address showed in /proc/kallsyms and in bpf_prog_info. First, real program address is showed instead of page address. Second, when there is no subprogram, bpf_prog_info->jited_ksyms and bpf_prog_info->jited_fun_lens returns the main prog address and length. Song Liu (3): bpf: show real jited prog address in /proc/kallsyms bpf: show real jited address in bpf_prog_info->jited_ksyms bpf: show main program address and length in bpf_prog_info kernel/bpf/core.c| 4 +--- kernel/bpf/syscall.c | 34 -- 2 files changed, 25 insertions(+), 13 deletions(-) -- 2.17.1
[PATCH v2 bpf 1/3] bpf: show real jited prog address in /proc/kallsyms
Currently, /proc/kallsyms shows page address of jited bpf program. The
main reason here is to not expose randomized start address. However,
This is not ideal for detailed profiling (find hot instructions from
stack traces). This patch replaces the page address with real prog start
address.
This change is OK because these addresses are still protected by sysctl
kptr_restrict (see kallsyms_show_value()), and only programs loaded by
root are added to kallsyms (see bpf_prog_kallsyms_add()).
Signed-off-by: Song Liu
---
kernel/bpf/core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 6377225b2082..1a796e0799ec 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -553,7 +553,6 @@ bool is_bpf_text_address(unsigned long addr)
int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
char *sym)
{
- unsigned long symbol_start, symbol_end;
struct bpf_prog_aux *aux;
unsigned int it = 0;
int ret = -ERANGE;
@@ -566,10 +565,9 @@ int bpf_get_kallsym(unsigned int symnum, unsigned long
*value, char *type,
if (it++ != symnum)
continue;
- bpf_get_prog_addr_region(aux->prog, _start, _end);
bpf_get_prog_name(aux->prog, sym);
- *value = symbol_start;
+ *value = (unsigned long)aux->prog->bpf_func;
*type = BPF_SYM_ELF_TYPE;
ret = 0;
--
2.17.1
[PATCH v2 bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms
Currently, jited_ksyms in bpf_prog_info shows page addresses of jited
bpf program. The main reason here is to not expose randomized start
address. However, this is not ideal for detailed profiling (find hot
instructions from stack traces). This patch replaces the page address
with real prog start address.
This change is OK because bpf_prog_get_info_by_fd() is only available
to root.
Signed-off-by: Song Liu
---
kernel/bpf/syscall.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ccb93277aae2..34a9eef5992c 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2172,7 +2172,6 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
user_ksyms = u64_to_user_ptr(info.jited_ksyms);
for (i = 0; i < ulen; i++) {
ksym_addr = (ulong)
prog->aux->func[i]->bpf_func;
- ksym_addr &= PAGE_MASK;
if (put_user((u64) ksym_addr, _ksyms[i]))
return -EFAULT;
}
--
2.17.1
Re: [PATCH bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms
> On Nov 2, 2018, at 3:19 AM, Daniel Borkmann wrote:
>
> On 11/02/2018 11:09 AM, Daniel Borkmann wrote:
>> On 11/01/2018 08:00 AM, Song Liu wrote:
>>> Currently, jited_ksyms in bpf_prog_info shows page addresses of jited
>>> bpf program. This is not ideal for detailed profiling (find hot
>>> instructions from stack traces). This patch replaces the page address
>>> with real prog start address.
>>>
>>> Signed-off-by: Song Liu
>>> ---
>>> kernel/bpf/syscall.c | 1 -
>>> 1 file changed, 1 deletion(-)
>>>
>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>>> index ccb93277aae2..34a9eef5992c 100644
>>> --- a/kernel/bpf/syscall.c
>>> +++ b/kernel/bpf/syscall.c
>>> @@ -2172,7 +2172,6 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog
>>> *prog,
>>> user_ksyms = u64_to_user_ptr(info.jited_ksyms);
>>> for (i = 0; i < ulen; i++) {
>>> ksym_addr = (ulong)
>>> prog->aux->func[i]->bpf_func;
>>> - ksym_addr &= PAGE_MASK;
>>
>> Note that the masking was done on purpose here and in patch 1/3 in order to
>> not expose randomized start address to kallsyms at least. I suppose it's
>> okay to change it here and for kallsyms given bpf_prog_get_info_by_fd() dump
>> is for root only, and in each of the two cases we additionally apply
>> kallsyms_show_value() logic, so for unpriv this is zeroed out plus only root
>> loaded programs are added under kallsyms (capable(CAP_SYS_ADMIN)) anyway.
>
> (Btw, something like above should have been in changelog to provide some more
> historical context of why we used to do it like that and explaining why it is
> okay to change it this way.)
Thanks Daniel!
I will send v2 with these fixes.
Song
Re: [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload
Hi Arnaldo, On Wed, Oct 17, 2018 at 5:11 AM Arnaldo Carvalho de Melo wrote: > > Adding Alexey, Jiri and Namhyung as they worked/are working on > multithreading 'perf record'. I have read Alexey's work on enabling aio for perf-record (https://lkml.org/lkml/2018/10/15/169). But I feel it is not really related to this work. Did I miss anything here? For VIP events, I think we need more mmap. Currently, the default setting uses 1 mmap per cpu. To capture VIP events, I think we need another mmap per CPU. The VIP events will be send to the special mmap. Then, user space will process these event before the end of perf-record. Does this approach make sense? Thanks! Song
[PATCH bpf 1/3] bpf: show real jited prog address in /proc/kallsyms
Currently, /proc/kallsyms shows page address of jited bpf program. This
is not ideal for detailed profiling (find hot instructions from stack
traces). This patch replaces the page address with real prog start
address.
Signed-off-by: Song Liu
---
kernel/bpf/core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
index 6377225b2082..1a796e0799ec 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -553,7 +553,6 @@ bool is_bpf_text_address(unsigned long addr)
int bpf_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
char *sym)
{
- unsigned long symbol_start, symbol_end;
struct bpf_prog_aux *aux;
unsigned int it = 0;
int ret = -ERANGE;
@@ -566,10 +565,9 @@ int bpf_get_kallsym(unsigned int symnum, unsigned long
*value, char *type,
if (it++ != symnum)
continue;
- bpf_get_prog_addr_region(aux->prog, _start, _end);
bpf_get_prog_name(aux->prog, sym);
- *value = symbol_start;
+ *value = (unsigned long)aux->prog->bpf_func;
*type = BPF_SYM_ELF_TYPE;
ret = 0;
--
2.17.1
[PATCH bpf 2/3] bpf: show real jited address in bpf_prog_info->jited_ksyms
Currently, jited_ksyms in bpf_prog_info shows page addresses of jited
bpf program. This is not ideal for detailed profiling (find hot
instructions from stack traces). This patch replaces the page address
with real prog start address.
Signed-off-by: Song Liu
---
kernel/bpf/syscall.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index ccb93277aae2..34a9eef5992c 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2172,7 +2172,6 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
user_ksyms = u64_to_user_ptr(info.jited_ksyms);
for (i = 0; i < ulen; i++) {
ksym_addr = (ulong)
prog->aux->func[i]->bpf_func;
- ksym_addr &= PAGE_MASK;
if (put_user((u64) ksym_addr, _ksyms[i]))
return -EFAULT;
}
--
2.17.1
[PATCH bpf 0/3] show more accurrate bpf program address
This set improves bpf program address showed in /proc/kallsyms and in bpf_prog_info. First, real program address is showed instead of page address. Second, when there is no subprogram, bpf_prog_info->jited_ksyms returns the main prog address. Song Liu (3): bpf: show real jited prog address in /proc/kallsyms bpf: show real jited address in bpf_prog_info->jited_ksyms bpf: show main program address in bpf_prog_info->jited_ksyms kernel/bpf/core.c| 4 +--- kernel/bpf/syscall.c | 17 - 2 files changed, 13 insertions(+), 8 deletions(-) -- 2.17.1
[PATCH bpf 3/3] bpf: show main program address in bpf_prog_info->jited_ksyms
Currently, when there is not subprog (prog->aux->func_cnt == 0),
bpf_prog_info does not return any jited_ksyms. This patch adds
main program address (prog->bpf_func) to jited_ksyms.
Signed-off-by: Song Liu
---
kernel/bpf/syscall.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 34a9eef5992c..7293b17ca62a 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -2158,7 +2158,7 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
}
ulen = info.nr_jited_ksyms;
- info.nr_jited_ksyms = prog->aux->func_cnt;
+ info.nr_jited_ksyms = prog->aux->func_cnt ? : 1;
if (info.nr_jited_ksyms && ulen) {
if (bpf_dump_raw_ok()) {
u64 __user *user_ksyms;
@@ -2170,9 +2170,17 @@ static int bpf_prog_get_info_by_fd(struct bpf_prog *prog,
*/
ulen = min_t(u32, info.nr_jited_ksyms, ulen);
user_ksyms = u64_to_user_ptr(info.jited_ksyms);
- for (i = 0; i < ulen; i++) {
- ksym_addr = (ulong)
prog->aux->func[i]->bpf_func;
- if (put_user((u64) ksym_addr, _ksyms[i]))
+ if (prog->aux->func_cnt) {
+ for (i = 0; i < ulen; i++) {
+ ksym_addr = (ulong)
+ prog->aux->func[i]->bpf_func;
+ if (put_user((u64) ksym_addr,
+_ksyms[i]))
+ return -EFAULT;
+ }
+ } else {
+ ksym_addr = (ulong) prog->bpf_func;
+ if (put_user((u64) ksym_addr, _ksyms[0]))
return -EFAULT;
}
} else {
--
2.17.1
[PATCH iproute2] bpf: check map symbol type properly with newer llvm compiler
With llvm 7.0 or earlier, the map symbol type is STT_NOTYPE.
-bash-4.4$ cat t.c
__attribute__((section("maps"))) int g;
-bash-4.4$ clang -target bpf -O2 -c t.c
-bash-4.4$ readelf -s t.o
Symbol table '.symtab' contains 2 entries:
Num:Value Size TypeBind Vis Ndx Name
0: 0 NOTYPE LOCAL DEFAULT UND
1: 0 NOTYPE GLOBAL DEFAULT3 g
-bash-4.4$
The following llvm commit enables BPF target to generate
proper symbol type and size.
commit bf6ec206615b9718869d48b4e5400d0c6e3638dd
Author: Yonghong Song
Date: Wed Sep 19 16:04:13 2018 +
[bpf] Symbol sizes and types in object file
Clang-compiled object files currently don't include the symbol sizes and
types. Some tools however need that information. For example, ctfconvert
uses that information to generate FreeBSD's CTF representation from ELF
files.
With this patch, symbol sizes and types are included in object files.
Signed-off-by: Paul Chaignon
Reported-by: Yutaro Hayakawa
Hence, for llvm 8.0.0 (currently trunk), symbol type will be not NOTYPE, but
OBJECT.
-bash-4.4$ clang -target bpf -O2 -c t.c
-bash-4.4$ readelf -s t.o
Symbol table '.symtab' contains 3 entries:
Num:Value Size TypeBind Vis Ndx Name
0: 0 NOTYPE LOCAL DEFAULT UND
1: 0 FILELOCAL DEFAULT ABS t.c
2: 4 OBJECT GLOBAL DEFAULT3 g
-bash-4.4$
This patch makes sure bpf library accepts both NOTYPE and OBJECT types
of global map symbols.
Signed-off-by: Yonghong Song
---
lib/bpf.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/lib/bpf.c b/lib/bpf.c
index d093d0bd..45f279fa 100644
--- a/lib/bpf.c
+++ b/lib/bpf.c
@@ -1758,11 +1758,13 @@ static const char *bpf_map_fetch_name(struct
bpf_elf_ctx *ctx, int which)
int i;
for (i = 0; i < ctx->sym_num; i++) {
+ int type = GELF_ST_TYPE(sym.st_info);
+
if (gelf_getsym(ctx->sym_tab, i, ) != )
continue;
if (GELF_ST_BIND(sym.st_info) != STB_GLOBAL ||
- GELF_ST_TYPE(sym.st_info) != STT_NOTYPE ||
+ (type != STT_NOTYPE && type != STT_OBJECT) ||
sym.st_shndx != ctx->sec_maps ||
sym.st_value / ctx->map_len != which)
continue;
@@ -1849,11 +1851,13 @@ static int bpf_map_num_sym(struct bpf_elf_ctx *ctx)
GElf_Sym sym;
for (i = 0; i < ctx->sym_num; i++) {
+ int type = GELF_ST_TYPE(sym.st_info);
+
if (gelf_getsym(ctx->sym_tab, i, ) != )
continue;
if (GELF_ST_BIND(sym.st_info) != STB_GLOBAL ||
- GELF_ST_TYPE(sym.st_info) != STT_NOTYPE ||
+ (type != STT_NOTYPE && type != STT_OBJECT) ||
sym.st_shndx != ctx->sec_maps)
continue;
num++;
@@ -1927,10 +1931,12 @@ static int bpf_map_verify_all_offs(struct bpf_elf_ctx
*ctx, int end)
* the table again.
*/
for (i = 0; i < ctx->sym_num; i++) {
+ int type = GELF_ST_TYPE(sym.st_info);
+
if (gelf_getsym(ctx->sym_tab, i, ) != )
continue;
if (GELF_ST_BIND(sym.st_info) != STB_GLOBAL ||
- GELF_ST_TYPE(sym.st_info) != STT_NOTYPE ||
+ (type != STT_NOTYPE && type != STT_OBJECT) ||
sym.st_shndx != ctx->sec_maps)
continue;
if (sym.st_value == off)
--
2.17.1
Re: [PATCH] bpf: tcp_bpf_recvmsg should return EAGAIN when nonblocking and no data
On Mon, Oct 29, 2018 at 1:32 PM John Fastabend wrote:
>
> On 10/29/2018 12:31 PM, John Fastabend wrote:
> > We return 0 in the case of a nonblocking socket that has no data
> > available. However, this is incorrect and may confuse applications.
> > After this patch we do the correct thing and return the error
> > EAGAIN.
> >
> > Quoting return codes from recvmsg manpage,
> >
> > EAGAIN or EWOULDBLOCK
> > The socket is marked nonblocking and the receive operation would
> > block, or a receive timeout had been set and the timeout expired
> > before data was received.
> >
> > Signed-off-by: John Fastabend
Acked-by: Song Liu
> > ---
>
> Add fixes tag.
>
> Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
>
>
>
>
Re: [PATCH bpf] tools/bpf: add unlimited rlimit for flow_dissector_load
On Mon, Oct 29, 2018 at 2:58 PM Yonghong Song wrote: > > On our test machine, bpf selftest test_flow_dissector.sh failed > with the following error: > # ./test_flow_dissector.sh > bpffs not mounted. Mounting... > libbpf: failed to create map (name: 'jmp_table'): Operation not permitted > libbpf: failed to load object 'bpf_flow.o' > ./flow_dissector_load: bpf_prog_load bpf_flow.o > selftests: test_flow_dissector [FAILED] > > Let us increase the rlimit to remove the above map > creation failure. > > Signed-off-by: Yonghong Song Acked-by: Song Liu > --- > tools/testing/selftests/bpf/flow_dissector_load.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/tools/testing/selftests/bpf/flow_dissector_load.c > b/tools/testing/selftests/bpf/flow_dissector_load.c > index d3273b5b3173..ae8180b11d5f 100644 > --- a/tools/testing/selftests/bpf/flow_dissector_load.c > +++ b/tools/testing/selftests/bpf/flow_dissector_load.c > @@ -11,6 +11,8 @@ > #include > #include > > +#include "bpf_rlimit.h" > + > const char *cfg_pin_path = "/sys/fs/bpf/flow_dissector"; > const char *cfg_map_name = "jmp_table"; > bool cfg_attach = true; > -- > 2.17.1 >
[PATCH bpf] tools/bpf: add unlimited rlimit for flow_dissector_load
On our test machine, bpf selftest test_flow_dissector.sh failed with the following error: # ./test_flow_dissector.sh bpffs not mounted. Mounting... libbpf: failed to create map (name: 'jmp_table'): Operation not permitted libbpf: failed to load object 'bpf_flow.o' ./flow_dissector_load: bpf_prog_load bpf_flow.o selftests: test_flow_dissector [FAILED] Let us increase the rlimit to remove the above map creation failure. Signed-off-by: Yonghong Song --- tools/testing/selftests/bpf/flow_dissector_load.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/testing/selftests/bpf/flow_dissector_load.c b/tools/testing/selftests/bpf/flow_dissector_load.c index d3273b5b3173..ae8180b11d5f 100644 --- a/tools/testing/selftests/bpf/flow_dissector_load.c +++ b/tools/testing/selftests/bpf/flow_dissector_load.c @@ -11,6 +11,8 @@ #include #include +#include "bpf_rlimit.h" + const char *cfg_pin_path = "/sys/fs/bpf/flow_dissector"; const char *cfg_map_name = "jmp_table"; bool cfg_attach = true; -- 2.17.1
Re: [PATCH bpf 7/7] bpf: make direct packet write unclone more robust
On Wed, Oct 24, 2018 at 3:08 PM Daniel Borkmann wrote:
>
> On 10/24/2018 11:42 PM, Song Liu wrote:
> > On Wed, Oct 24, 2018 at 1:06 PM Daniel Borkmann
> > wrote:
> >>
> >> Given this seems to be quite fragile and can easily slip through the
> >> cracks, lets make direct packet write more robust by requiring that
> >> future program types which allow for such write must provide a prologue
> >> callback. In case of XDP and sk_msg it's noop, thus add a generic noop
> >> handler there. The latter starts out with NULL data/data_end
> >> unconditionally
> >> when sg pages are shared.
> >>
> >> Signed-off-by: Daniel Borkmann
> >> Acked-by: Alexei Starovoitov
> >> ---
> >> kernel/bpf/verifier.c | 6 +-
> >> net/core/filter.c | 11 +++
> >> 2 files changed, 16 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> >> index 5fc9a65..171a2c8 100644
> >> --- a/kernel/bpf/verifier.c
> >> +++ b/kernel/bpf/verifier.c
> >> @@ -5709,7 +5709,11 @@ static int convert_ctx_accesses(struct
> >> bpf_verifier_env *env)
> >> bool is_narrower_load;
> >> u32 target_size;
> >>
> >> - if (ops->gen_prologue) {
> >> + if (ops->gen_prologue || env->seen_direct_write) {
> >> + if (!ops->gen_prologue) {
> >> + verbose(env, "bpf verifier is misconfigured\n");
> >> + return -EINVAL;
> >> + }
> >
> > nit: how about this?
> >
> > diff --git i/kernel/bpf/verifier.c w/kernel/bpf/verifier.c
> > index 6fbe7a8afed7..d35078024e35 100644
> > --- i/kernel/bpf/verifier.c
> > +++ w/kernel/bpf/verifier.c
> > @@ -5286,6 +5286,11 @@ static int convert_ctx_accesses(struct
> > bpf_verifier_env *env)
> > bool is_narrower_load;
> > u32 target_size;
> >
> > + if (!ops->gen_prologue && env->seen_direct_write) {
> > + verbose(env, "bpf verifier is misconfigured\n");
> > + return -EINVAL;
> > + }
> > +
> > if (ops->gen_prologue) {
> > cnt = ops->gen_prologue(insn_buf, env->seen_direct_write,
> > env->prog);
> >
>
> Hm, probably matter of different style preference, personally I'd prefer
> the one as is though.
Yeah, it is just a nitpick.
Thanks!
Acked-by: Song Liu
Re: [PATCH bpf 0/7] Batch of direct packet access fixes for BPF
On Wed, Oct 24, 2018 at 1:08 PM Daniel Borkmann wrote: > > Several fixes to get direct packet access in order from verifier > side. Also test suite fix to run cg_skb as unpriv and an improvement > to make direct packet write less error prone in future. > > Thanks! > > Daniel Borkmann (7): > bpf: fix test suite to enable all unpriv program types > bpf: disallow direct packet access for unpriv in cg_skb > bpf: fix direct packet access for flow dissector progs > bpf: fix cg_skb types to hint access type in may_access_direct_pkt_data > bpf: fix direct packet write into pop/peek helpers > bpf: fix leaking uninitialized memory on pop/peek helpers > bpf: make direct packet write unclone more robust > > kernel/bpf/helpers.c| 2 -- > kernel/bpf/queue_stack_maps.c | 2 ++ > kernel/bpf/verifier.c | 13 ++--- > net/core/filter.c | 17 + > tools/testing/selftests/bpf/test_verifier.c | 15 +-- > 5 files changed, 42 insertions(+), 7 deletions(-) > > -- > 2.9.5 > Other than the nitpick on 7/7, for the series: Acked-by: Song Liu
Re: [PATCH bpf 7/7] bpf: make direct packet write unclone more robust
On Wed, Oct 24, 2018 at 1:06 PM Daniel Borkmann wrote:
>
> Given this seems to be quite fragile and can easily slip through the
> cracks, lets make direct packet write more robust by requiring that
> future program types which allow for such write must provide a prologue
> callback. In case of XDP and sk_msg it's noop, thus add a generic noop
> handler there. The latter starts out with NULL data/data_end unconditionally
> when sg pages are shared.
>
> Signed-off-by: Daniel Borkmann
> Acked-by: Alexei Starovoitov
> ---
> kernel/bpf/verifier.c | 6 +-
> net/core/filter.c | 11 +++
> 2 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 5fc9a65..171a2c8 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -5709,7 +5709,11 @@ static int convert_ctx_accesses(struct
> bpf_verifier_env *env)
> bool is_narrower_load;
> u32 target_size;
>
> - if (ops->gen_prologue) {
> + if (ops->gen_prologue || env->seen_direct_write) {
> + if (!ops->gen_prologue) {
> + verbose(env, "bpf verifier is misconfigured\n");
> + return -EINVAL;
> + }
nit: how about this?
diff --git i/kernel/bpf/verifier.c w/kernel/bpf/verifier.c
index 6fbe7a8afed7..d35078024e35 100644
--- i/kernel/bpf/verifier.c
+++ w/kernel/bpf/verifier.c
@@ -5286,6 +5286,11 @@ static int convert_ctx_accesses(struct
bpf_verifier_env *env)
bool is_narrower_load;
u32 target_size;
+ if (!ops->gen_prologue && env->seen_direct_write) {
+ verbose(env, "bpf verifier is misconfigured\n");
+ return -EINVAL;
+ }
+
if (ops->gen_prologue) {
cnt = ops->gen_prologue(insn_buf, env->seen_direct_write,
env->prog);
Re: [PATCH v2 bpf] bpf: devmap: fix wrong interface selection in notifier_call
On Wed, Oct 24, 2018 at 4:16 AM Taehee Yoo wrote:
>
> The dev_map_notification() removes interface in devmap if
> unregistering interface's ifindex is same.
> But only checking ifindex is not enough because other netns can have
> same ifindex. so that wrong interface selection could occurred.
> Hence netdev pointer comparison code is added.
>
> v2: compare netdev pointer instead of using net_eq() (Daniel Borkmann)
> v1: Initial patch
>
> Fixes: 2ddf71e23cc2 ("net: add notifier hooks for devmap bpf map")
> Signed-off-by: Taehee Yoo
Acked-by: Song Liu
> ---
> kernel/bpf/devmap.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/devmap.c b/kernel/bpf/devmap.c
> index 141710b82a6c..191b79948424 100644
> --- a/kernel/bpf/devmap.c
> +++ b/kernel/bpf/devmap.c
> @@ -512,8 +512,7 @@ static int dev_map_notification(struct notifier_block
> *notifier,
> struct bpf_dtab_netdev *dev, *odev;
>
> dev = READ_ONCE(dtab->netdev_map[i]);
> - if (!dev ||
> - dev->dev->ifindex != netdev->ifindex)
> + if (!dev || netdev != dev->dev)
> continue;
> odev = cmpxchg(>netdev_map[i], dev,
> NULL);
> if (dev == odev)
> --
> 2.17.1
>
[PATCH v8 bpf-next 1/2] bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB
BPF programs of BPF_PROG_TYPE_CGROUP_SKB need to access headers in the
skb. This patch enables direct access of skb for these programs.
Two helper functions bpf_compute_and_save_data_end() and
bpf_restore_data_end() are introduced. There are used in
__cgroup_bpf_run_filter_skb(), to compute proper data_end for the
BPF program, and restore original data afterwards.
Signed-off-by: Song Liu
---
include/linux/filter.h | 21 +
kernel/bpf/cgroup.c| 6 ++
net/core/filter.c | 36 +++-
3 files changed, 62 insertions(+), 1 deletion(-)
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 5771874bc01e..91b4c934f02e 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -548,6 +548,27 @@ static inline void bpf_compute_data_pointers(struct
sk_buff *skb)
cb->data_end = skb->data + skb_headlen(skb);
}
+/* Similar to bpf_compute_data_pointers(), except that save orginal
+ * data in cb->data and cb->meta_data for restore.
+ */
+static inline void bpf_compute_and_save_data_end(
+ struct sk_buff *skb, void **saved_data_end)
+{
+ struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb;
+
+ *saved_data_end = cb->data_end;
+ cb->data_end = skb->data + skb_headlen(skb);
+}
+
+/* Restore data saved by bpf_compute_data_pointers(). */
+static inline void bpf_restore_data_end(
+ struct sk_buff *skb, void *saved_data_end)
+{
+ struct bpf_skb_data_end *cb = (struct bpf_skb_data_end *)skb->cb;
+
+ cb->data_end = saved_data_end;
+}
+
static inline u8 *bpf_skb_cb(struct sk_buff *skb)
{
/* eBPF programs may read/write skb->cb[] area to transfer meta
diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c
index 00f6ed2e4f9a..9425c2fb872f 100644
--- a/kernel/bpf/cgroup.c
+++ b/kernel/bpf/cgroup.c
@@ -553,6 +553,7 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
{
unsigned int offset = skb->data - skb_network_header(skb);
struct sock *save_sk;
+ void *saved_data_end;
struct cgroup *cgrp;
int ret;
@@ -566,8 +567,13 @@ int __cgroup_bpf_run_filter_skb(struct sock *sk,
save_sk = skb->sk;
skb->sk = sk;
__skb_push(skb, offset);
+
+ /* compute pointers for the bpf prog */
+ bpf_compute_and_save_data_end(skb, _data_end);
+
ret = BPF_PROG_RUN_ARRAY(cgrp->bpf.effective[type], skb,
bpf_prog_run_save_cb);
+ bpf_restore_data_end(skb, saved_data_end);
__skb_pull(skb, offset);
skb->sk = save_sk;
return ret == 1 ? 0 : -EPERM;
diff --git a/net/core/filter.c b/net/core/filter.c
index 1a3ac6c46873..e3ca30bd6840 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -5346,6 +5346,40 @@ static bool sk_filter_is_valid_access(int off, int size,
return bpf_skb_is_valid_access(off, size, type, prog, info);
}
+static bool cg_skb_is_valid_access(int off, int size,
+ enum bpf_access_type type,
+ const struct bpf_prog *prog,
+ struct bpf_insn_access_aux *info)
+{
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, tc_classid):
+ case bpf_ctx_range(struct __sk_buff, data_meta):
+ case bpf_ctx_range(struct __sk_buff, flow_keys):
+ return false;
+ }
+ if (type == BPF_WRITE) {
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, mark):
+ case bpf_ctx_range(struct __sk_buff, priority):
+ case bpf_ctx_range_till(struct __sk_buff, cb[0], cb[4]):
+ break;
+ default:
+ return false;
+ }
+ }
+
+ switch (off) {
+ case bpf_ctx_range(struct __sk_buff, data):
+ info->reg_type = PTR_TO_PACKET;
+ break;
+ case bpf_ctx_range(struct __sk_buff, data_end):
+ info->reg_type = PTR_TO_PACKET_END;
+ break;
+ }
+
+ return bpf_skb_is_valid_access(off, size, type, prog, info);
+}
+
static bool lwt_is_valid_access(int off, int size,
enum bpf_access_type type,
const struct bpf_prog *prog,
@@ -7038,7 +7072,7 @@ const struct bpf_prog_ops xdp_prog_ops = {
const struct bpf_verifier_ops cg_skb_verifier_ops = {
.get_func_proto = cg_skb_func_proto,
- .is_valid_access= sk_filter_is_valid_access,
+ .is_valid_access= cg_skb_is_valid_access,
.convert_ctx_access = bpf_convert_ctx_access,
};
--
2.17.1
[PATCH v8 bpf-next 0/2] bpf: add cg_skb_is_valid_access
Changes v7 -> v8: 1. Dynamically allocate the dummy sk to avoid race conditions. Changes v6 -> v7: 1. Make dummy sk a global variable (test_run_sk). Changes v5 -> v6: 1. Fixed dummy sk in bpf_prog_test_run_skb() as suggested by Eric Dumazet. Changes v4 -> v5: 1. Replaced bpf_compute_and_save_data_pointers() with bpf_compute_and_save_data_end(); Replaced bpf_restore_data_pointers() with bpf_restore_data_end(). 2. Fixed indentation in test_verifier.c Changes v3 -> v4: 1. Fixed crash issue reported by Alexei. Changes v2 -> v3: 1. Added helper function bpf_compute_and_save_data_pointers() and bpf_restore_data_pointers(). Changes v1 -> v2: 1. Updated the list of read-only fields, and read-write fields. 2. Added dummy sk to bpf_prog_test_run_skb(). This set enables BPF program of type BPF_PROG_TYPE_CGROUP_SKB to access some __skb_buff data directly. Song Liu (2): bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB bpf: add tests for direct packet access from CGROUP_SKB include/linux/filter.h | 21 +++ kernel/bpf/cgroup.c | 6 + net/bpf/test_run.c | 15 ++ net/core/filter.c | 36 - tools/testing/selftests/bpf/test_verifier.c | 171 5 files changed, 248 insertions(+), 1 deletion(-) -- 2.17.1
[PATCH v8 bpf-next 2/2] bpf: add tests for direct packet access from CGROUP_SKB
Tests are added to make sure CGROUP_SKB cannot access:
tc_classid, data_meta, flow_keys
and can read and write:
mark, prority, and cb[0-4]
and can read other fields.
To make selftest with skb->sk work, a dummy sk is added in
bpf_prog_test_run_skb().
Signed-off-by: Song Liu
---
net/bpf/test_run.c | 15 ++
tools/testing/selftests/bpf/test_verifier.c | 171
2 files changed, 186 insertions(+)
diff --git a/net/bpf/test_run.c b/net/bpf/test_run.c
index 0c423b8cd75c..c89c22c49015 100644
--- a/net/bpf/test_run.c
+++ b/net/bpf/test_run.c
@@ -10,6 +10,8 @@
#include
#include
#include
+#include
+#include
static __always_inline u32 bpf_test_run_one(struct bpf_prog *prog, void *ctx,
struct bpf_cgroup_storage *storage[MAX_BPF_CGROUP_STORAGE_TYPE])
@@ -115,6 +117,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const
union bpf_attr *kattr,
u32 retval, duration;
int hh_len = ETH_HLEN;
struct sk_buff *skb;
+ struct sock *sk;
void *data;
int ret;
@@ -137,11 +140,21 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const
union bpf_attr *kattr,
break;
}
+ sk = kzalloc(sizeof(struct sock), GFP_USER);
+ if (!sk) {
+ kfree(data);
+ return -ENOMEM;
+ }
+ sock_net_set(sk, current->nsproxy->net_ns);
+ sock_init_data(NULL, sk);
+
skb = build_skb(data, 0);
if (!skb) {
kfree(data);
+ kfree(sk);
return -ENOMEM;
}
+ skb->sk = sk;
skb_reserve(skb, NET_SKB_PAD + NET_IP_ALIGN);
__skb_put(skb, size);
@@ -159,6 +172,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const
union bpf_attr *kattr,
if (pskb_expand_head(skb, nhead, 0, GFP_USER)) {
kfree_skb(skb);
+ kfree(sk);
return -ENOMEM;
}
}
@@ -171,6 +185,7 @@ int bpf_prog_test_run_skb(struct bpf_prog *prog, const
union bpf_attr *kattr,
size = skb_headlen(skb);
ret = bpf_test_finish(kattr, uattr, skb->data, size, retval, duration);
kfree_skb(skb);
+ kfree(sk);
return ret;
}
diff --git a/tools/testing/selftests/bpf/test_verifier.c
b/tools/testing/selftests/bpf/test_verifier.c
index cf4cd32b6772..f1ae8d09770f 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -4862,6 +4862,177 @@ static struct bpf_test tests[] = {
.result = REJECT,
.flags = F_NEEDS_EFFICIENT_UNALIGNED_ACCESS,
},
+ {
+ "direct packet read test#1 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1,
+ offsetof(struct __sk_buff, data)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_3, BPF_REG_1,
+ offsetof(struct __sk_buff, data_end)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, len)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ offsetof(struct __sk_buff, pkt_type)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_6, BPF_REG_1,
+ offsetof(struct __sk_buff, mark)),
+ BPF_STX_MEM(BPF_W, BPF_REG_1, BPF_REG_6,
+ offsetof(struct __sk_buff, mark)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_7, BPF_REG_1,
+ offsetof(struct __sk_buff, queue_mapping)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_8, BPF_REG_1,
+ offsetof(struct __sk_buff, protocol)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_9, BPF_REG_1,
+ offsetof(struct __sk_buff, vlan_present)),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_2),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_0, 8),
+ BPF_JMP_REG(BPF_JGT, BPF_REG_0, BPF_REG_3, 1),
+ BPF_LDX_MEM(BPF_B, BPF_REG_0, BPF_REG_2, 0),
+ BPF_MOV64_IMM(BPF_REG_0, 0),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .prog_type = BPF_PROG_TYPE_CGROUP_SKB,
+ },
+ {
+ "direct packet read test#2 for CGROUP_SKB",
+ .insns = {
+ BPF_LDX_MEM(BPF_W, BPF_REG_4, BPF_REG_1,
+ offsetof(struct __sk_buff, vlan_tci)),
+ BPF_LDX_MEM(BPF_W, BPF_REG_5, BPF_REG_1,
+ of
[PATCH v7 bpf-next 0/2] bpf: add cg_skb_is_valid_access
Changes v6 -> v7: 1. Make dummy sk a global variable (test_run_sk). Changes v5 -> v6: 1. Fixed dummy sk in bpf_prog_test_run_skb() as suggested by Eric Dumazet. Changes v4 -> v5: 1. Replaced bpf_compute_and_save_data_pointers() with bpf_compute_and_save_data_end(); Replaced bpf_restore_data_pointers() with bpf_restore_data_end(). 2. Fixed indentation in test_verifier.c Changes v3 -> v4: 1. Fixed crash issue reported by Alexei. Changes v2 -> v3: 1. Added helper function bpf_compute_and_save_data_pointers() and bpf_restore_data_pointers(). Changes v1 -> v2: 1. Updated the list of read-only fields, and read-write fields. 2. Added dummy sk to bpf_prog_test_run_skb(). This set enables BPF program of type BPF_PROG_TYPE_CGROUP_SKB to access some __skb_buff data directly. Song Liu (2): bpf: add cg_skb_is_valid_access for BPF_PROG_TYPE_CGROUP_SKB bpf: add tests for direct packet access from CGROUP_SKB include/linux/filter.h | 21 +++ kernel/bpf/cgroup.c | 6 + net/bpf/test_run.c | 8 + net/core/filter.c | 36 - tools/testing/selftests/bpf/test_verifier.c | 171 5 files changed, 241 insertions(+), 1 deletion(-) -- 2.17.1
