From: Jiri Benc <[email protected]>
[ Upstream commit 11aca65ec4db09527d3e9b6b41a0615b7da4386b ]
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.
To fix this, convert to 'static __always_inline'.
v2: Use 'static __always_inline' instead of 'static inline
__attribute__((always_inline))'
Fixes: c99a84eac026 ("selftests/bpf: test for seg6local End.BPF action")
Signed-off-by: Jiri Benc <[email protected]>
Acked-by: Yonghong Song <[email protected]>
Signed-off-by: Daniel Borkmann <[email protected]>
Signed-off-by: Sasha Levin <[email protected]>
---
.../testing/selftests/bpf/progs/test_lwt_seg6local.c | 12 ++++++------
1 file changed, 6 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..e2f6ed0a583d 100644
--- a/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
+++ b/tools/testing/selftests/bpf/progs/test_lwt_seg6local.c
@@ -61,7 +61,7 @@ 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 __always_inline struct ip6_srh_t *get_srh(struct __sk_buff *skb)
{
void *cursor, *data_end;
struct ip6_srh_t *srh;
@@ -95,7 +95,7 @@ __attribute__((always_inline)) struct ip6_srh_t *get_srh(struct __sk_buff *skb)
return srh;
}
-__attribute__((always_inline))
+static __always_inline
int update_tlv_pad(struct __sk_buff *skb, uint32_t new_pad,
uint32_t old_pad, uint32_t pad_off)
{
@@ -125,7 +125,7 @@ int update_tlv_pad(struct __sk_buff *skb, uint32_t new_pad,
return 0;
}
-__attribute__((always_inline))
+static __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 +184,7 @@ int is_valid_tlv_boundary(struct __sk_buff *skb, struct ip6_srh_t *srh,
return 0;
}
-__attribute__((always_inline))
+static __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 +228,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 __always_inline
int delete_tlv(struct __sk_buff *skb, struct ip6_srh_t *srh,
uint32_t tlv_off)
{
@@ -266,7 +266,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 __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.20.1
On Mon, 15 Jul 2019 09:46:31 -0400, Sasha Levin wrote:
> From: Jiri Benc <[email protected]>
>
> [ Upstream commit 11aca65ec4db09527d3e9b6b41a0615b7da4386b ]
>
> Selftests are reporting this failure in test_lwt_seg6local.sh:
I don't think this is critical in any way and I don't think this is a
stable material. How was this selected?
Jiri
On Wed, Jul 17, 2019 at 11:43:34AM +0200, Jiri Benc wrote:
>On Mon, 15 Jul 2019 09:46:31 -0400, Sasha Levin wrote:
>> From: Jiri Benc <[email protected]>
>>
>> [ Upstream commit 11aca65ec4db09527d3e9b6b41a0615b7da4386b ]
>>
>> Selftests are reporting this failure in test_lwt_seg6local.sh:
>
>I don't think this is critical in any way and I don't think this is a
>stable material. How was this selected?
It fixes a bug, right?
--
Thanks,
Sasha
On Wed, 17 Jul 2019 19:47:57 -0400, Sasha Levin wrote:
> It fixes a bug, right?
A bug in selftests. And quite likely, it probably happens only with
some compiler versions.
I don't think patches only touching tools/testing/selftests/ qualify
for stable in general. They don't affect the end users.
Jiri
From: Jiri Benc <[email protected]>
Date: Thu, 18 Jul 2019 09:36:54 +0200
> On Wed, 17 Jul 2019 19:47:57 -0400, Sasha Levin wrote:
>> It fixes a bug, right?
>
> A bug in selftests. And quite likely, it probably happens only with
> some compiler versions.
>
> I don't think patches only touching tools/testing/selftests/ qualify
> for stable in general. They don't affect the end users.
It has a significant impact on automated testing which lots of
individuals and entities perform, therefore I think it very much is
-stable material.
On Thu, Jul 18, 2019 at 09:36:54AM +0200, Jiri Benc wrote:
>On Wed, 17 Jul 2019 19:47:57 -0400, Sasha Levin wrote:
>> It fixes a bug, right?
>
>A bug in selftests. And quite likely, it probably happens only with
>some compiler versions.
>
>I don't think patches only touching tools/testing/selftests/ qualify
>for stable in general. They don't affect the end users.
I'd argue that a bug in your tests is just as (if not even more) worse
than a bug in the code.
--
Thanks,
Sasha
On Thu, 18 Jul 2019 11:55:34 -0700 (PDT), David Miller wrote:
> It has a significant impact on automated testing which lots of
> individuals and entities perform, therefore I think it very much is
> -stable material.
Okay.
Thanks,
Jiri