* [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
@ 2019-06-29 14:43 Jiri Benc
2019-06-29 18:04 ` Song Liu
0 siblings, 1 reply; 6+ messages in thread
From: Jiri Benc @ 2019-06-29 14:43 UTC (permalink / raw)
To: netdev, bpf; +Cc: Alexei Starovoitov, Daniel Borkmann, Mathieu Xhonneux
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
2019-06-29 14:43 [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local Jiri Benc
@ 2019-06-29 18:04 ` Song Liu
2019-06-29 18:04 ` Song Liu
0 siblings, 1 reply; 6+ messages in thread
From: Song Liu @ 2019-06-29 18:04 UTC (permalink / raw)
To: Jiri Benc
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann, Mathieu Xhonneux
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
2019-06-29 18:04 ` Song Liu
@ 2019-06-29 18:04 ` Song Liu
2019-07-01 8:12 ` Jiri Benc
2019-07-01 18:39 ` Y Song
0 siblings, 2 replies; 6+ messages in thread
From: Song Liu @ 2019-06-29 18:04 UTC (permalink / raw)
To: Jiri Benc
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann, Mathieu Xhonneux
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
2019-06-29 18:04 ` Song Liu
@ 2019-07-01 8:12 ` Jiri Benc
2019-07-01 18:39 ` Y Song
1 sibling, 0 replies; 6+ messages in thread
From: Jiri Benc @ 2019-07-01 8:12 UTC (permalink / raw)
To: Song Liu
Cc: Networking, bpf, Alexei Starovoitov, Daniel Borkmann, Mathieu Xhonneux
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
2019-06-29 18:04 ` Song Liu
2019-07-01 8:12 ` Jiri Benc
@ 2019-07-01 18:39 ` Y Song
2019-07-02 17:29 ` Jiri Benc
1 sibling, 1 reply; 6+ messages in thread
From: Y Song @ 2019-07-01 18:39 UTC (permalink / raw)
To: Song Liu
Cc: Jiri Benc, Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Mathieu Xhonneux
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local
2019-07-01 18:39 ` Y Song
@ 2019-07-02 17:29 ` Jiri Benc
0 siblings, 0 replies; 6+ messages in thread
From: Jiri Benc @ 2019-07-02 17:29 UTC (permalink / raw)
To: Y Song
Cc: Song Liu, Networking, bpf, Alexei Starovoitov, Daniel Borkmann,
Mathieu Xhonneux
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-07-02 17:29 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-29 14:43 [PATCH bpf] selftests: bpf: fix inlines in test_lwt_seg6local Jiri Benc
2019-06-29 18:04 ` Song Liu
2019-06-29 18:04 ` Song Liu
2019-07-01 8:12 ` Jiri Benc
2019-07-01 18:39 ` Y Song
2019-07-02 17:29 ` Jiri Benc
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.