All of lore.kernel.org
 help / color / mirror / Atom feed
* running an eBPF program
@ 2017-05-26 11:00 Adel Fuchs
  2017-05-27 20:23 ` Y Song
  0 siblings, 1 reply; 9+ messages in thread
From: Adel Fuchs @ 2017-05-26 11:00 UTC (permalink / raw)
  To: netdev

Hi

I'm trying to run this eBPF program:

https://git.kernel.org/pub/scm/linux/kernel/git/shemminger/iproute2.git/tree/examples/bpf


and I get this error:


:~/iproute2/examples/bpf$sudo tc filter add dev enx00e11100329b parent
1: bpf obj bpf.o exp /tmp/bpf-uds flowid 1:1 action bpf obj bpf.o sec
action-mark            action bpf obj bpf.o sec action-rand ok

[sudo] password for adel:



Prog section 'classifier' rejected: Permission denied (13)!

- Type:         3

- Instructions: 218 (0 over limit)

- License:      GPL



Verifier analysis:



0: (bf) r6 = r1

1: (18) r9 = 0xffe0000e

3: (69) r0 = *(u16 *)(r6 +16)

invalid bpf_context access off=16 size=2



Error fetching program/map!

Failed to retrieve (e)BPF data!


Any suggestions?

Thanks,

Adel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: running an eBPF program
  2017-05-26 11:00 running an eBPF program Adel Fuchs
@ 2017-05-27 20:23 ` Y Song
  2017-05-27 20:52   ` Y Song
  0 siblings, 1 reply; 9+ messages in thread
From: Y Song @ 2017-05-27 20:23 UTC (permalink / raw)
  To: Adel Fuchs; +Cc: netdev

>From verifier error message:
======
0: (bf) r6 = r1

1: (18) r9 = 0xffe0000e

3: (69) r0 = *(u16 *)(r6 +16)

invalid bpf_context access off=16 size=2
======

The offset 16 of struct __sk_buff is hash.
What instruction #3 tries to do is to access 2 bytes of the hash value
instead of full 4 bytes.
This is explicitly not allowed in verifier due to endianness issue.

Look at iproute2 example code, it looks like the following may be responsible:

bpf_tailcall.c:#define MAX_JMP_SIZE    2
bpf_tailcall.c:        tail_call(skb, &jmp_tc, skb->hash & (MAX_JMP_SIZE - 1));

I am thinking of implementing something in LLVM to prevent
optimization from LD4=>LD2/DL1 for context access like this.


On Fri, May 26, 2017 at 4:00 AM, Adel Fuchs <adelfuchs@gmail.com> wrote:
> Hi
>
> I'm trying to run this eBPF program:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/shemminger/iproute2.git/tree/examples/bpf
>
>
> and I get this error:
>
>
> :~/iproute2/examples/bpf$sudo tc filter add dev enx00e11100329b parent
> 1: bpf obj bpf.o exp /tmp/bpf-uds flowid 1:1 action bpf obj bpf.o sec
> action-mark            action bpf obj bpf.o sec action-rand ok
>
> [sudo] password for adel:
>
>
>
> Prog section 'classifier' rejected: Permission denied (13)!
>
> - Type:         3
>
> - Instructions: 218 (0 over limit)
>
> - License:      GPL
>
>
>
> Verifier analysis:
>
>
>
> 0: (bf) r6 = r1
>
> 1: (18) r9 = 0xffe0000e
>
> 3: (69) r0 = *(u16 *)(r6 +16)
>
> invalid bpf_context access off=16 size=2
>
>
>
> Error fetching program/map!
>
> Failed to retrieve (e)BPF data!
>
>
> Any suggestions?
>
> Thanks,
>
> Adel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: running an eBPF program
  2017-05-27 20:23 ` Y Song
@ 2017-05-27 20:52   ` Y Song
  2017-05-28  0:11     ` David Miller
  0 siblings, 1 reply; 9+ messages in thread
From: Y Song @ 2017-05-27 20:52 UTC (permalink / raw)
  To: Adel Fuchs; +Cc: netdev

On Sat, May 27, 2017 at 1:23 PM, Y Song <ys114321@gmail.com> wrote:
>
> From verifier error message:
> ======
> 0: (bf) r6 = r1
>
> 1: (18) r9 = 0xffe0000e
>
> 3: (69) r0 = *(u16 *)(r6 +16)
>
> invalid bpf_context access off=16 size=2
> ======
>
> The offset 16 of struct __sk_buff is hash.
> What instruction #3 tries to do is to access 2 bytes of the hash value
> instead of full 4 bytes.
> This is explicitly not allowed in verifier due to endianness issue.


I can reproduce the issue now. My previous statement saying to access
"hash" field is not correct. It is accessing the protocol field.

static __inline__ bool flow_dissector(struct __sk_buff *skb,
                                      struct flow_keys *flow)
{
        int poff, nh_off = BPF_LL_OFF + ETH_HLEN;
        __be16 proto = skb->protocol;
        __u8 ip_proto;

The plan so far is to see whether we can fix the issue in LLVM side.

Yonghong

>
>
> Look at iproute2 example code, it looks like the following may be responsible:
>
> bpf_tailcall.c:#define MAX_JMP_SIZE    2
> bpf_tailcall.c:        tail_call(skb, &jmp_tc, skb->hash & (MAX_JMP_SIZE - 1));
>
> I am thinking of implementing something in LLVM to prevent
> optimization from LD4=>LD2/DL1 for context access like this.
>
>
> On Fri, May 26, 2017 at 4:00 AM, Adel Fuchs <adelfuchs@gmail.com> wrote:
> > Hi
> >
> > I'm trying to run this eBPF program:
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/shemminger/iproute2.git/tree/examples/bpf
> >
> >
> > and I get this error:
> >
> >
> > :~/iproute2/examples/bpf$sudo tc filter add dev enx00e11100329b parent
> > 1: bpf obj bpf.o exp /tmp/bpf-uds flowid 1:1 action bpf obj bpf.o sec
> > action-mark            action bpf obj bpf.o sec action-rand ok
> >
> > [sudo] password for adel:
> >
> >
> >
> > Prog section 'classifier' rejected: Permission denied (13)!
> >
> > - Type:         3
> >
> > - Instructions: 218 (0 over limit)
> >
> > - License:      GPL
> >
> >
> >
> > Verifier analysis:
> >
> >
> >
> > 0: (bf) r6 = r1
> >
> > 1: (18) r9 = 0xffe0000e
> >
> > 3: (69) r0 = *(u16 *)(r6 +16)
> >
> > invalid bpf_context access off=16 size=2
> >
> >
> >
> > Error fetching program/map!
> >
> > Failed to retrieve (e)BPF data!
> >
> >
> > Any suggestions?
> >
> > Thanks,
> >
> > Adel

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: running an eBPF program
  2017-05-27 20:52   ` Y Song
@ 2017-05-28  0:11     ` David Miller
  2017-05-28  5:08       ` Y Song
  0 siblings, 1 reply; 9+ messages in thread
From: David Miller @ 2017-05-28  0:11 UTC (permalink / raw)
  To: ys114321; +Cc: adelfuchs, netdev

From: Y Song <ys114321@gmail.com>
Date: Sat, 27 May 2017 13:52:27 -0700

> On Sat, May 27, 2017 at 1:23 PM, Y Song <ys114321@gmail.com> wrote:
>>
>> From verifier error message:
>> ======
>> 0: (bf) r6 = r1
>>
>> 1: (18) r9 = 0xffe0000e
>>
>> 3: (69) r0 = *(u16 *)(r6 +16)
>>
>> invalid bpf_context access off=16 size=2
>> ======
>>
>> The offset 16 of struct __sk_buff is hash.
>> What instruction #3 tries to do is to access 2 bytes of the hash value
>> instead of full 4 bytes.
>> This is explicitly not allowed in verifier due to endianness issue.
> 
> 
> I can reproduce the issue now. My previous statement saying to access
> "hash" field is not correct. It is accessing the protocol field.
> 
> static __inline__ bool flow_dissector(struct __sk_buff *skb,
>                                       struct flow_keys *flow)
> {
>         int poff, nh_off = BPF_LL_OFF + ETH_HLEN;
>         __be16 proto = skb->protocol;
>         __u8 ip_proto;
> 
> The plan so far is to see whether we can fix the issue in LLVM side.

If the compiler properly asks for "__sk_buff + 16" on little-endian
and "__sk_buff + 20" on big-endian, the verifier should instead be
fixed to allow the access to pass.

I can't see any reason why LLVM won't set the offset properly like
that, and it's a completely legitimate optimization that we shouldn't
try to stop LLVM from performing.

It also makes it so that we don't have to fix having absurdly defined
__sk_buff's protocol field as a u32.

Thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: running an eBPF program
  2017-05-28  0:11     ` David Miller
@ 2017-05-28  5:08       ` Y Song
       [not found]         ` <CAErYV9HhBoWnMVZS4XBJPN0TM4V2=U2Y+NBSyY8bqHJopvWTGA@mail.gmail.com>
       [not found]         ` <CAErYV9FSQ1Fs-++1MPGXh=_aY3PnBKKCLTH_rvxGWaaz=6aBNg@mail.gmail.com>
  0 siblings, 2 replies; 9+ messages in thread
From: Y Song @ 2017-05-28  5:08 UTC (permalink / raw)
  To: David Miller; +Cc: Adel Fuchs, netdev

On Sat, May 27, 2017 at 5:11 PM, David Miller <davem@davemloft.net> wrote:
> From: Y Song <ys114321@gmail.com>
> Date: Sat, 27 May 2017 13:52:27 -0700
>
>> On Sat, May 27, 2017 at 1:23 PM, Y Song <ys114321@gmail.com> wrote:
>>>
>>> From verifier error message:
>>> ======
>>> 0: (bf) r6 = r1
>>>
>>> 1: (18) r9 = 0xffe0000e
>>>
>>> 3: (69) r0 = *(u16 *)(r6 +16)
>>>
>>> invalid bpf_context access off=16 size=2
>>> ======
>>>
>>> The offset 16 of struct __sk_buff is hash.
>>> What instruction #3 tries to do is to access 2 bytes of the hash value
>>> instead of full 4 bytes.
>>> This is explicitly not allowed in verifier due to endianness issue.
>>
>>
>> I can reproduce the issue now. My previous statement saying to access
>> "hash" field is not correct. It is accessing the protocol field.
>>
>> static __inline__ bool flow_dissector(struct __sk_buff *skb,
>>                                       struct flow_keys *flow)
>> {
>>         int poff, nh_off = BPF_LL_OFF + ETH_HLEN;
>>         __be16 proto = skb->protocol;
>>         __u8 ip_proto;
>>
>> The plan so far is to see whether we can fix the issue in LLVM side.
>
> If the compiler properly asks for "__sk_buff + 16" on little-endian
> and "__sk_buff + 20" on big-endian, the verifier should instead be
> fixed to allow the access to pass.
>
> I can't see any reason why LLVM won't set the offset properly like
> that, and it's a completely legitimate optimization that we shouldn't
> try to stop LLVM from performing.

I do agree that such optimization in LLVM is perfect fine and actually
beneficial.
The only reason I was thinking was to avoid introduce endianness into verifier.
Maybe not too much work there. Let me do some experiments and come with
a patch for that.

Thanks!

Yonghong

>
> It also makes it so that we don't have to fix having absurdly defined
> __sk_buff's protocol field as a u32.
>
> Thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Fwd: running an eBPF program
       [not found]         ` <CAErYV9HhBoWnMVZS4XBJPN0TM4V2=U2Y+NBSyY8bqHJopvWTGA@mail.gmail.com>
@ 2017-05-28  7:40           ` Adel Fuchs
  0 siblings, 0 replies; 9+ messages in thread
From: Adel Fuchs @ 2017-05-28  7:40 UTC (permalink / raw)
  To: netdev

Hi,
Is there any way to run this eBPF program without that patch?
Alternatively, is there any other eBPF sample that does run properly?
I need to run a program that filters packets according to IP address
or port.
Thanks,
Adel

On Sun, May 28, 2017 at 8:08 AM, Y Song <ys114321@gmail.com> wrote:
> On Sat, May 27, 2017 at 5:11 PM, David Miller <davem@davemloft.net> wrote:
>> From: Y Song <ys114321@gmail.com>
>> Date: Sat, 27 May 2017 13:52:27 -0700
>>
>>> On Sat, May 27, 2017 at 1:23 PM, Y Song <ys114321@gmail.com> wrote:
>>>>
>>>> From verifier error message:
>>>> ======
>>>> 0: (bf) r6 = r1
>>>>
>>>> 1: (18) r9 = 0xffe0000e
>>>>
>>>> 3: (69) r0 = *(u16 *)(r6 +16)
>>>>
>>>> invalid bpf_context access off=16 size=2
>>>> ======
>>>>
>>>> The offset 16 of struct __sk_buff is hash.
>>>> What instruction #3 tries to do is to access 2 bytes of the hash value
>>>> instead of full 4 bytes.
>>>> This is explicitly not allowed in verifier due to endianness issue.
>>>
>>>
>>> I can reproduce the issue now. My previous statement saying to access
>>> "hash" field is not correct. It is accessing the protocol field.
>>>
>>> static __inline__ bool flow_dissector(struct __sk_buff *skb,
>>>                                       struct flow_keys *flow)
>>> {
>>>         int poff, nh_off = BPF_LL_OFF + ETH_HLEN;
>>>         __be16 proto = skb->protocol;
>>>         __u8 ip_proto;
>>>
>>> The plan so far is to see whether we can fix the issue in LLVM side.
>>
>> If the compiler properly asks for "__sk_buff + 16" on little-endian
>> and "__sk_buff + 20" on big-endian, the verifier should instead be
>> fixed to allow the access to pass.
>>
>> I can't see any reason why LLVM won't set the offset properly like
>> that, and it's a completely legitimate optimization that we shouldn't
>> try to stop LLVM from performing.
>
> I do agree that such optimization in LLVM is perfect fine and actually
> beneficial.
> The only reason I was thinking was to avoid introduce endianness into verifier.
> Maybe not too much work there. Let me do some experiments and come with
> a patch for that.
>
> Thanks!
>
> Yonghong
>
>>
>> It also makes it so that we don't have to fix having absurdly defined
>> __sk_buff's protocol field as a u32.
>>
>> Thanks.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: running an eBPF program
       [not found]         ` <CAErYV9FSQ1Fs-++1MPGXh=_aY3PnBKKCLTH_rvxGWaaz=6aBNg@mail.gmail.com>
@ 2017-05-29  5:31           ` Y Song
  2017-06-01  6:43             ` Adel Fuchs
  0 siblings, 1 reply; 9+ messages in thread
From: Y Song @ 2017-05-29  5:31 UTC (permalink / raw)
  To: Adel Fuchs; +Cc: netdev

On Sun, May 28, 2017 at 12:38 AM, Adel Fuchs <adelfuchs@gmail.com> wrote:
> Hi,
> Is there any way to run this eBPF program without that patch?
> Alternatively, is there any other eBPF sample that does run properly? I need
> to run a program that filters packets according to IP address or port.

The following is temporary workaround you can use:
        int poff, nh_off = BPF_LL_OFF + ETH_HLEN;
-       __be16 proto = skb->protocol;
+       __be16 proto = (__be16)*(volatile __u32 *)&skb->protocol;
        __u8 ip_proto;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: running an eBPF program
  2017-05-29  5:31           ` Y Song
@ 2017-06-01  6:43             ` Adel Fuchs
  2017-06-01  6:54               ` Adel Fuchs
  0 siblings, 1 reply; 9+ messages in thread
From: Adel Fuchs @ 2017-06-01  6:43 UTC (permalink / raw)
  To: Y Song; +Cc: netdev

Hi,
Thanks for your reply. Where do I need to add this workaround?

Thanks,
Adel

On Mon, May 29, 2017 at 8:31 AM, Y Song <ys114321@gmail.com> wrote:
> On Sun, May 28, 2017 at 12:38 AM, Adel Fuchs <adelfuchs@gmail.com> wrote:
>> Hi,
>> Is there any way to run this eBPF program without that patch?
>> Alternatively, is there any other eBPF sample that does run properly? I need
>> to run a program that filters packets according to IP address or port.
>
> The following is temporary workaround you can use:
>         int poff, nh_off = BPF_LL_OFF + ETH_HLEN;
> -       __be16 proto = skb->protocol;
> +       __be16 proto = (__be16)*(volatile __u32 *)&skb->protocol;
>         __u8 ip_proto;

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: running an eBPF program
  2017-06-01  6:43             ` Adel Fuchs
@ 2017-06-01  6:54               ` Adel Fuchs
  0 siblings, 0 replies; 9+ messages in thread
From: Adel Fuchs @ 2017-06-01  6:54 UTC (permalink / raw)
  To: Y Song; +Cc: netdev

OK. Found it. Thanks! it works!

On Thu, Jun 1, 2017 at 9:43 AM, Adel Fuchs <adelfuchs@gmail.com> wrote:
> Hi,
> Thanks for your reply. Where do I need to add this workaround?
>
> Thanks,
> Adel
>
> On Mon, May 29, 2017 at 8:31 AM, Y Song <ys114321@gmail.com> wrote:
>> On Sun, May 28, 2017 at 12:38 AM, Adel Fuchs <adelfuchs@gmail.com> wrote:
>>> Hi,
>>> Is there any way to run this eBPF program without that patch?
>>> Alternatively, is there any other eBPF sample that does run properly? I need
>>> to run a program that filters packets according to IP address or port.
>>
>> The following is temporary workaround you can use:
>>         int poff, nh_off = BPF_LL_OFF + ETH_HLEN;
>> -       __be16 proto = skb->protocol;
>> +       __be16 proto = (__be16)*(volatile __u32 *)&skb->protocol;
>>         __u8 ip_proto;

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-06-01  6:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-26 11:00 running an eBPF program Adel Fuchs
2017-05-27 20:23 ` Y Song
2017-05-27 20:52   ` Y Song
2017-05-28  0:11     ` David Miller
2017-05-28  5:08       ` Y Song
     [not found]         ` <CAErYV9HhBoWnMVZS4XBJPN0TM4V2=U2Y+NBSyY8bqHJopvWTGA@mail.gmail.com>
2017-05-28  7:40           ` Fwd: " Adel Fuchs
     [not found]         ` <CAErYV9FSQ1Fs-++1MPGXh=_aY3PnBKKCLTH_rvxGWaaz=6aBNg@mail.gmail.com>
2017-05-29  5:31           ` Y Song
2017-06-01  6:43             ` Adel Fuchs
2017-06-01  6:54               ` Adel Fuchs

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.