[PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local

[PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local

[Jiri Benc]

Selftests are reporting this failure in test_lwt_seg6local.sh:

+ ip netns exec ns2 ip -6 route add fb00::6 encap bpf in obj test_lwt_seg6local.o sec encap_srh dev veth2
Error fetching program/map!
Failed to parse eBPF program: Operation not permitted

The problem is __attribute__((always_inline)) alone is not enough to prevent
clang from inserting those functions in .text. In that case, .text is not
marked as relocateable.

See the output of objdump -h test_lwt_seg6local.o:

Idx Name          Size      VMA               LMA               File off  Algn
  0 .text         00003530  0000000000000000  0000000000000000  00000040  2**3
                  CONTENTS, ALLOC, LOAD, READONLY, CODE

This causes the iproute bpf loader to fail in bpf_fetch_prog_sec:
bpf_has_call_data returns true but bpf_fetch_prog_relo fails as there's no
relocateable .text section in the file.

Add 'static inline' to fix this.

Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
Signed-off-by: Jiri Benc <jbenc@redhat.com>
---
 .../selftests/bpf/progs/test_lwt_seg6local.c        | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c b/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
index 0575751bc1bc..5848c1b52554 100644
--- a/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
+++ b/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
@@ -61,7 +61,8 @@ struct sr6_tlv_t {
 	unsigned char value[0];
 } BPF_PACKET_HEADER;
 
-__attribute__((always_inline)) struct ip6_srh_t *get_srh(struct __sk_buff *skb)
+static inline __attribute__((always_inline))
+struct ip6_srh_t *get_srh(struct __sk_buff *skb)
 {
 	void *cursor, *data_end;
 	struct ip6_srh_t *srh;
@@ -95,7 +96,7 @@ __attribute__((always_inline)) struct ip6_srh_t *get_srh(struct __sk_buff *skb)
 	return srh;
 }
 
-__attribute__((always_inline))
+static inline __attribute__((always_inline))
 int update_tlv_pad(struct __sk_buff *skb, uint32_t new_pad,
 		   uint32_t old_pad, uint32_t pad_off)
 {
@@ -125,7 +126,7 @@ int update_tlv_pad(struct __sk_buff *skb, uint32_t new_pad,
 	return 0;
 }
 
-__attribute__((always_inline))
+static inline __attribute__((always_inline))
 int is_valid_tlv_boundary(struct __sk_buff *skb, struct ip6_srh_t *srh,
 			  uint32_t *tlv_off, uint32_t *pad_size,
 			  uint32_t *pad_off)
@@ -184,7 +185,7 @@ int is_valid_tlv_boundary(struct __sk_buff *skb, struct ip6_srh_t *srh,
 	return 0;
 }
 
-__attribute__((always_inline))
+static inline __attribute__((always_inline))
 int add_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh, uint32_t tlv_off,
 	    struct sr6_tlv_t *itlv, uint8_t tlv_size)
 {
@@ -228,7 +229,7 @@ int add_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh, uint32_t tlv_off,
 	return update_tlv_pad(skb, new_pad, pad_size, pad_off);
 }
 
-__attribute__((always_inline))
+static inline __attribute__((always_inline))
 int delete_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh,
 	       uint32_t tlv_off)
 {
@@ -266,7 +267,7 @@ int delete_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh,
 	return update_tlv_pad(skb, new_pad, pad_size, pad_off);
 }
 
-__attribute__((always_inline))
+static inline __attribute__((always_inline))
 int has_egr_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh)
 {
 	int tlv_offset = sizeof(struct ip6_t) + sizeof(struct ip6_srh_t) +
-- 
2.18.1

Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local

[Song Liu]

On Sat, Jun 29, 2019 at 7:43 AM Jiri Benc <jbenc@redhat.com> wrote:
>
> Selftests are reporting this failure in test_lwt_seg6local.sh:
>
> + ip netns exec ns2 ip -6 route add fb00::6 encap bpf in obj test_lwt_seg6local.o sec encap_srh dev veth2
> Error fetching program/map!
> Failed to parse eBPF program: Operation not permitted
>
> The problem is __attribute__((always_inline)) alone is not enough to prevent
> clang from inserting those functions in .text. In that case, .text is not
> marked as relocateable.
>
> See the output of objdump -h test_lwt_seg6local.o:
>
> Idx Name          Size      VMA               LMA               File off  Algn
>   0 .text         00003530  0000000000000000  0000000000000000  00000040  2**3
>                   CONTENTS, ALLOC, LOAD, READONLY, CODE
>
> This causes the iproute bpf loader to fail in bpf_fetch_prog_sec:
> bpf_has_call_data returns true but bpf_fetch_prog_relo fails as there's no
> relocateable .text section in the file.
>
> Add 'static inline' to fix this.
>
> Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
> Signed-off-by: Jiri Benc <jbenc@redhat.com>

Maybe use "__always_inline" as most other tests do?

Thanks,
Song

Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local

[Song Liu]

On Sat, Jun 29, 2019 at 11:04 AM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Sat, Jun 29, 2019 at 7:43 AM Jiri Benc <jbenc@redhat.com> wrote:
> >
> > Selftests are reporting this failure in test_lwt_seg6local.sh:
> >
> > + ip netns exec ns2 ip -6 route add fb00::6 encap bpf in obj test_lwt_seg6local.o sec encap_srh dev veth2
> > Error fetching program/map!
> > Failed to parse eBPF program: Operation not permitted
> >
> > The problem is __attribute__((always_inline)) alone is not enough to prevent
> > clang from inserting those functions in .text. In that case, .text is not
> > marked as relocateable.
> >
> > See the output of objdump -h test_lwt_seg6local.o:
> >
> > Idx Name          Size      VMA               LMA               File off  Algn
> >   0 .text         00003530  0000000000000000  0000000000000000  00000040  2**3
> >                   CONTENTS, ALLOC, LOAD, READONLY, CODE
> >
> > This causes the iproute bpf loader to fail in bpf_fetch_prog_sec:
> > bpf_has_call_data returns true but bpf_fetch_prog_relo fails as there's no
> > relocateable .text section in the file.
> >
> > Add 'static inline' to fix this.
> >
> > Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
> > Signed-off-by: Jiri Benc <jbenc@redhat.com>
>
> Maybe use "__always_inline" as most other tests do?

I meant "static __always_inline".

Song

Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local

[Jiri Benc]

On Sat, 29 Jun 2019 11:04:54 -0700, Song Liu wrote:
> > Maybe use "__always_inline" as most other tests do?  
> 
> I meant "static __always_inline".

Sure, I can do that. It doesn't seem to be as consistent as you
suggest, though.

There are three different forms used in selftests/bpf/progs:

static __always_inline
static inline __attribute__((__always_inline__))
static inline __attribute__((always_inline))

As this is a bug causing selftests to fail (at least for some clang/llvm
versions), how about applying this to bpf.git as a minimal fix and
unifying the progs in bpf-next?

Thanks,

 Jiri

Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local

[Y Song]

On Sat, Jun 29, 2019 at 11:05 AM Song Liu <liu.song.a23@gmail.com> wrote:
>
> On Sat, Jun 29, 2019 at 11:04 AM Song Liu <liu.song.a23@gmail.com> wrote:
> >
> > On Sat, Jun 29, 2019 at 7:43 AM Jiri Benc <jbenc@redhat.com> wrote:
> > >
> > > Selftests are reporting this failure in test_lwt_seg6local.sh:
> > >
> > > + ip netns exec ns2 ip -6 route add fb00::6 encap bpf in obj test_lwt_seg6local.o sec encap_srh dev veth2
> > > Error fetching program/map!
> > > Failed to parse eBPF program: Operation not permitted
> > >
> > > The problem is __attribute__((always_inline)) alone is not enough to prevent
> > > clang from inserting those functions in .text. In that case, .text is not
> > > marked as relocateable.
> > >
> > > See the output of objdump -h test_lwt_seg6local.o:
> > >
> > > Idx Name          Size      VMA               LMA               File off  Algn
> > >   0 .text         00003530  0000000000000000  0000000000000000  00000040  2**3
> > >                   CONTENTS, ALLOC, LOAD, READONLY, CODE
> > >
> > > This causes the iproute bpf loader to fail in bpf_fetch_prog_sec:
> > > bpf_has_call_data returns true but bpf_fetch_prog_relo fails as there's no
> > > relocateable .text section in the file.
> > >
> > > Add 'static inline' to fix this.
> > >
> > > Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
> > > Signed-off-by: Jiri Benc <jbenc@redhat.com>
> >
> > Maybe use "__always_inline" as most other tests do?
>
> I meant "static __always_inline".

By default, we have
# define __always_inline        inline __attribute__((always_inline))

So just use __always_inline should be less verbose in your patch.

BTW, what compiler did you use have this behavior?
Did you have issues with `static __attribute__((always_inline))`?

>
> Song

Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local

[Jiri Benc]

On Mon, 1 Jul 2019 11:39:27 -0700, Y Song wrote:
> By default, we have
> # define __always_inline        inline __attribute__((always_inline))
> 
> So just use __always_inline should be less verbose in your patch.

I'll resubmit, converting everything to __always_inline, targeting
bpf-next.

> BTW, what compiler did you use have this behavior?

clang version 7.0.1 (tags/RELEASE_701/final)
LLVM version 7.0.1

> Did you have issues with `static __attribute__((always_inline))`?

That seems to work, too.

 Jiri