2014-06-17 02:24:22

by Guenter Roeck

[permalink] [raw]
Subject: mips:allmodconfig build failure in 3.16-rc1 due to bpf_jit code

mips:allmodconfig fails in 3.16-rc1 with lots of undefined symbols.

arch/mips/net/bpf_jit.c: In function 'is_load_to_a':
arch/mips/net/bpf_jit.c:559:7: error: 'BPF_S_LD_W_LEN' undeclared (first use in this function)
arch/mips/net/bpf_jit.c:559:7: note: each undeclared identifier is reported only once for each function it appears in
arch/mips/net/bpf_jit.c:560:7: error: 'BPF_S_LD_W_ABS' undeclared (first use in this function)
arch/mips/net/bpf_jit.c:561:7: error: 'BPF_S_LD_H_ABS' undeclared (first use in this function)
arch/mips/net/bpf_jit.c:562:7: error: 'BPF_S_LD_B_ABS' undeclared (first use in this function)
arch/mips/net/bpf_jit.c:563:7: error: 'BPF_S_ANC_CPU' undeclared (first use in this function)
arch/mips/net/bpf_jit.c:564:7: error: 'BPF_S_ANC_IFINDEX' undeclared (first use in this function)
arch/mips/net/bpf_jit.c:565:7: error: 'BPF_S_ANC_MARK' undeclared (first use in this function)

and so on.

Those symbols are not defined anywhere.

The problem is due to a conflict with commit 348059313 (net: filter: get rid of BPF_S_*
enum), which removed those definitions.

Guenter


2014-06-17 08:21:12

by Daniel Borkmann

[permalink] [raw]
Subject: Re: mips:allmodconfig build failure in 3.16-rc1 due to bpf_jit code

On 06/17/2014 04:24 AM, Guenter Roeck wrote:
> mips:allmodconfig fails in 3.16-rc1 with lots of undefined symbols.
>
> arch/mips/net/bpf_jit.c: In function 'is_load_to_a':
> arch/mips/net/bpf_jit.c:559:7: error: 'BPF_S_LD_W_LEN' undeclared (first use in this function)
> arch/mips/net/bpf_jit.c:559:7: note: each undeclared identifier is reported only once for each function it appears in
> arch/mips/net/bpf_jit.c:560:7: error: 'BPF_S_LD_W_ABS' undeclared (first use in this function)
> arch/mips/net/bpf_jit.c:561:7: error: 'BPF_S_LD_H_ABS' undeclared (first use in this function)
> arch/mips/net/bpf_jit.c:562:7: error: 'BPF_S_LD_B_ABS' undeclared (first use in this function)
> arch/mips/net/bpf_jit.c:563:7: error: 'BPF_S_ANC_CPU' undeclared (first use in this function)
> arch/mips/net/bpf_jit.c:564:7: error: 'BPF_S_ANC_IFINDEX' undeclared (first use in this function)
> arch/mips/net/bpf_jit.c:565:7: error: 'BPF_S_ANC_MARK' undeclared (first use in this function)
>
> and so on.
>
> Those symbols are not defined anywhere.
>
> The problem is due to a conflict with commit 348059313 (net: filter: get rid of BPF_S_*
> enum), which removed those definitions.

Yep, it seems both got in this merge window from different trees. Don't have mips, but
I'll have a look, and send you a patch.

2014-06-17 10:16:43

by Daniel Borkmann

[permalink] [raw]
Subject: Re: mips:allmodconfig build failure in 3.16-rc1 due to bpf_jit code

On 06/17/2014 10:20 AM, Daniel Borkmann wrote:
> On 06/17/2014 04:24 AM, Guenter Roeck wrote:
>> mips:allmodconfig fails in 3.16-rc1 with lots of undefined symbols.
>>
>> arch/mips/net/bpf_jit.c: In function 'is_load_to_a':
>> arch/mips/net/bpf_jit.c:559:7: error: 'BPF_S_LD_W_LEN' undeclared (first use in this function)
>> arch/mips/net/bpf_jit.c:559:7: note: each undeclared identifier is reported only once for each function it appears in
>> arch/mips/net/bpf_jit.c:560:7: error: 'BPF_S_LD_W_ABS' undeclared (first use in this function)
>> arch/mips/net/bpf_jit.c:561:7: error: 'BPF_S_LD_H_ABS' undeclared (first use in this function)
>> arch/mips/net/bpf_jit.c:562:7: error: 'BPF_S_LD_B_ABS' undeclared (first use in this function)
>> arch/mips/net/bpf_jit.c:563:7: error: 'BPF_S_ANC_CPU' undeclared (first use in this function)
>> arch/mips/net/bpf_jit.c:564:7: error: 'BPF_S_ANC_IFINDEX' undeclared (first use in this function)
>> arch/mips/net/bpf_jit.c:565:7: error: 'BPF_S_ANC_MARK' undeclared (first use in this function)
>>
>> and so on.
>>
>> Those symbols are not defined anywhere.
>>
>> The problem is due to a conflict with commit 348059313 (net: filter: get rid of BPF_S_*
>> enum), which removed those definitions.
>
> Yep, it seems both got in this merge window from different trees. Don't have mips, but
> I'll have a look, and send you a patch.

Could you give the attached patch a try?

Thanks,

Daniel


Attachments:
0001-mips-filter-fix-build-error-in-BPF-JIT.patch (14.13 kB)

2014-06-17 10:39:51

by Guenter Roeck

[permalink] [raw]
Subject: Re: mips:allmodconfig build failure in 3.16-rc1 due to bpf_jit code

On 06/17/2014 03:16 AM, Daniel Borkmann wrote:
> On 06/17/2014 10:20 AM, Daniel Borkmann wrote:
>> On 06/17/2014 04:24 AM, Guenter Roeck wrote:
>>> mips:allmodconfig fails in 3.16-rc1 with lots of undefined symbols.
>>>
>>> arch/mips/net/bpf_jit.c: In function 'is_load_to_a':
>>> arch/mips/net/bpf_jit.c:559:7: error: 'BPF_S_LD_W_LEN' undeclared (first use in this function)
>>> arch/mips/net/bpf_jit.c:559:7: note: each undeclared identifier is reported only once for each function it appears in
>>> arch/mips/net/bpf_jit.c:560:7: error: 'BPF_S_LD_W_ABS' undeclared (first use in this function)
>>> arch/mips/net/bpf_jit.c:561:7: error: 'BPF_S_LD_H_ABS' undeclared (first use in this function)
>>> arch/mips/net/bpf_jit.c:562:7: error: 'BPF_S_LD_B_ABS' undeclared (first use in this function)
>>> arch/mips/net/bpf_jit.c:563:7: error: 'BPF_S_ANC_CPU' undeclared (first use in this function)
>>> arch/mips/net/bpf_jit.c:564:7: error: 'BPF_S_ANC_IFINDEX' undeclared (first use in this function)
>>> arch/mips/net/bpf_jit.c:565:7: error: 'BPF_S_ANC_MARK' undeclared (first use in this function)
>>>
>>> and so on.
>>>
>>> Those symbols are not defined anywhere.
>>>
>>> The problem is due to a conflict with commit 348059313 (net: filter: get rid of BPF_S_*
>>> enum), which removed those definitions.
>>
>> Yep, it seems both got in this merge window from different trees. Don't have mips, but
>> I'll have a look, and send you a patch.
>
> Could you give the attached patch a try?
>

Yes, this fixes the build problem. Obviously I have no idea if it works ;-)

Guenter

2014-06-17 10:54:31

by Markos Chandras

[permalink] [raw]
Subject: Re: mips:allmodconfig build failure in 3.16-rc1 due to bpf_jit code

On 06/17/2014 11:16 AM, Daniel Borkmann wrote:
> On 06/17/2014 10:20 AM, Daniel Borkmann wrote:
>> On 06/17/2014 04:24 AM, Guenter Roeck wrote:
>>> mips:allmodconfig fails in 3.16-rc1 with lots of undefined symbols.
>>>
>>> arch/mips/net/bpf_jit.c: In function 'is_load_to_a':
>>> arch/mips/net/bpf_jit.c:559:7: error: 'BPF_S_LD_W_LEN' undeclared
>>> (first use in this function)
>>> arch/mips/net/bpf_jit.c:559:7: note: each undeclared identifier is
>>> reported only once for each function it appears in
>>> arch/mips/net/bpf_jit.c:560:7: error: 'BPF_S_LD_W_ABS' undeclared
>>> (first use in this function)
>>> arch/mips/net/bpf_jit.c:561:7: error: 'BPF_S_LD_H_ABS' undeclared
>>> (first use in this function)
>>> arch/mips/net/bpf_jit.c:562:7: error: 'BPF_S_LD_B_ABS' undeclared
>>> (first use in this function)
>>> arch/mips/net/bpf_jit.c:563:7: error: 'BPF_S_ANC_CPU' undeclared
>>> (first use in this function)
>>> arch/mips/net/bpf_jit.c:564:7: error: 'BPF_S_ANC_IFINDEX' undeclared
>>> (first use in this function)
>>> arch/mips/net/bpf_jit.c:565:7: error: 'BPF_S_ANC_MARK' undeclared
>>> (first use in this function)
>>>
>>> and so on.
>>>
>>> Those symbols are not defined anywhere.
>>>
>>> The problem is due to a conflict with commit 348059313 (net: filter:
>>> get rid of BPF_S_*
>>> enum), which removed those definitions.
>>
>> Yep, it seems both got in this merge window from different trees.
>> Don't have mips, but
>> I'll have a look, and send you a patch.
>
> Could you give the attached patch a try?
>
> Thanks,
>
> Daniel

Hi Daniel,

This fixes the build and seems to work ok (tried with dhcp and tcpdump).
Thanks for fixing it.

Reviewed-by: Markos Chandras <[email protected]>
Tested-by: Markos Chandras <[email protected]>

--
markos

2014-06-17 10:56:33

by Daniel Borkmann

[permalink] [raw]
Subject: Re: mips:allmodconfig build failure in 3.16-rc1 due to bpf_jit code

On 06/17/2014 12:39 PM, Guenter Roeck wrote:
> On 06/17/2014 03:16 AM, Daniel Borkmann wrote:
>> On 06/17/2014 10:20 AM, Daniel Borkmann wrote:
>>> On 06/17/2014 04:24 AM, Guenter Roeck wrote:
>>>> mips:allmodconfig fails in 3.16-rc1 with lots of undefined symbols.
>>>>
>>>> arch/mips/net/bpf_jit.c: In function 'is_load_to_a':
>>>> arch/mips/net/bpf_jit.c:559:7: error: 'BPF_S_LD_W_LEN' undeclared (first use in this function)
>>>> arch/mips/net/bpf_jit.c:559:7: note: each undeclared identifier is reported only once for each function it appears in
>>>> arch/mips/net/bpf_jit.c:560:7: error: 'BPF_S_LD_W_ABS' undeclared (first use in this function)
>>>> arch/mips/net/bpf_jit.c:561:7: error: 'BPF_S_LD_H_ABS' undeclared (first use in this function)
>>>> arch/mips/net/bpf_jit.c:562:7: error: 'BPF_S_LD_B_ABS' undeclared (first use in this function)
>>>> arch/mips/net/bpf_jit.c:563:7: error: 'BPF_S_ANC_CPU' undeclared (first use in this function)
>>>> arch/mips/net/bpf_jit.c:564:7: error: 'BPF_S_ANC_IFINDEX' undeclared (first use in this function)
>>>> arch/mips/net/bpf_jit.c:565:7: error: 'BPF_S_ANC_MARK' undeclared (first use in this function)
>>>>
>>>> and so on.
>>>>
>>>> Those symbols are not defined anywhere.
>>>>
>>>> The problem is due to a conflict with commit 348059313 (net: filter: get rid of BPF_S_*
>>>> enum), which removed those definitions.
>>>
>>> Yep, it seems both got in this merge window from different trees. Don't have mips, but
>>> I'll have a look, and send you a patch.
>>
>> Could you give the attached patch a try?
>
> Yes, this fixes the build problem. Obviously I have no idea if it works ;-)

Thanks Guenter!

We have a test suite for BPF under lib/test_bpf.c that was designed for the
interpreter and for BPF JIT developers in mind and pretty much covers most
cases. E.g. one possibility to run it would be:

0) compile module test_bpf (Kernel Hacking -> Test BPF filter functionality)
1) echo 1 > /proc/sys/net/core/bpf_jit_enable [<- enables BPF JIT]
2) modprobe test_bpf [<- runs test suite]

The results will be visible in the klog via dmesg (if at least one test fails,
the module init handler will return an error).

Best,

Daniel

2014-06-17 10:58:38

by Daniel Borkmann

[permalink] [raw]
Subject: Re: mips:allmodconfig build failure in 3.16-rc1 due to bpf_jit code

On 06/17/2014 12:54 PM, Markos Chandras wrote:
...
> This fixes the build and seems to work ok (tried with dhcp and tcpdump).
> Thanks for fixing it.
>
> Reviewed-by: Markos Chandras <[email protected]>
> Tested-by: Markos Chandras <[email protected]>

Great, thanks for your help!

2014-06-17 11:09:33

by Markos Chandras

[permalink] [raw]
Subject: Re: mips:allmodconfig build failure in 3.16-rc1 due to bpf_jit code

On 06/17/2014 11:56 AM, Daniel Borkmann wrote:
> On 06/17/2014 12:39 PM, Guenter Roeck wrote:
>> On 06/17/2014 03:16 AM, Daniel Borkmann wrote:
>>> On 06/17/2014 10:20 AM, Daniel Borkmann wrote:
>>>> On 06/17/2014 04:24 AM, Guenter Roeck wrote:
>>>>> mips:allmodconfig fails in 3.16-rc1 with lots of undefined symbols.
>>>>>
>>>>> arch/mips/net/bpf_jit.c: In function 'is_load_to_a':
>>>>> arch/mips/net/bpf_jit.c:559:7: error: 'BPF_S_LD_W_LEN' undeclared
>>>>> (first use in this function)
>>>>> arch/mips/net/bpf_jit.c:559:7: note: each undeclared identifier is
>>>>> reported only once for each function it appears in
>>>>> arch/mips/net/bpf_jit.c:560:7: error: 'BPF_S_LD_W_ABS' undeclared
>>>>> (first use in this function)
>>>>> arch/mips/net/bpf_jit.c:561:7: error: 'BPF_S_LD_H_ABS' undeclared
>>>>> (first use in this function)
>>>>> arch/mips/net/bpf_jit.c:562:7: error: 'BPF_S_LD_B_ABS' undeclared
>>>>> (first use in this function)
>>>>> arch/mips/net/bpf_jit.c:563:7: error: 'BPF_S_ANC_CPU' undeclared
>>>>> (first use in this function)
>>>>> arch/mips/net/bpf_jit.c:564:7: error: 'BPF_S_ANC_IFINDEX'
>>>>> undeclared (first use in this function)
>>>>> arch/mips/net/bpf_jit.c:565:7: error: 'BPF_S_ANC_MARK' undeclared
>>>>> (first use in this function)
>>>>>
>>>>> and so on.
>>>>>
>>>>> Those symbols are not defined anywhere.
>>>>>
>>>>> The problem is due to a conflict with commit 348059313 (net:
>>>>> filter: get rid of BPF_S_*
>>>>> enum), which removed those definitions.
>>>>
>>>> Yep, it seems both got in this merge window from different trees.
>>>> Don't have mips, but
>>>> I'll have a look, and send you a patch.
>>>
>>> Could you give the attached patch a try?
>>
>> Yes, this fixes the build problem. Obviously I have no idea if it
>> works ;-)
>
> Thanks Guenter!
>
> We have a test suite for BPF under lib/test_bpf.c that was designed for the
> interpreter and for BPF JIT developers in mind and pretty much covers most
> cases. E.g. one possibility to run it would be:
>
> 0) compile module test_bpf (Kernel Hacking -> Test BPF filter
> functionality)
> 1) echo 1 > /proc/sys/net/core/bpf_jit_enable [<- enables
> BPF JIT]
> 2) modprobe test_bpf [<- runs test
> suite]
>
> The results will be visible in the klog via dmesg (if at least one test
> fails,
> the module init handler will return an error).
>
> Best,
>
> Daniel
Hi Daniel,

Thanks for these instructions. I will try them myself once I find some
time since I don't think bpf_jit for MIPS has ever been tested with all
the opcodes.

--
markos

2014-06-17 11:21:47

by Daniel Borkmann

[permalink] [raw]
Subject: Re: mips:allmodconfig build failure in 3.16-rc1 due to bpf_jit code

On 06/17/2014 01:09 PM, Markos Chandras wrote:
...
> Thanks for these instructions. I will try them myself once I find some
> time since I don't think bpf_jit for MIPS has ever been tested with all
> the opcodes.

Sounds great! If you find some tests are missing, please feel free to
submit them as well via netdev.

Best,

Daniel

2014-06-17 19:38:28

by Alexei Starovoitov

[permalink] [raw]
Subject: Re: mips:allmodconfig build failure in 3.16-rc1 due to bpf_jit code

On Tue, Jun 17, 2014 at 4:21 AM, Daniel Borkmann <[email protected]> wrote:
> On 06/17/2014 01:09 PM, Markos Chandras wrote:
> ...
>>
>> Thanks for these instructions. I will try them myself once I find some
>>
>> time since I don't think bpf_jit for MIPS has ever been tested with all
>> the opcodes.
>
>
> Sounds great! If you find some tests are missing, please feel free to
> submit them as well via netdev.
>
> Best,
>
> Daniel

Daniel,

thank you for taking care of it so quickly :)
from the BPF perspective the fix looks good:
Acked-by: Alexei Starovoitov <[email protected]>

Markos,

please do run the testsuite.
Doing quick code review of mips jit, it looks like:

- your version of pkt_type_offset() will work for little endian only.
(we've recently fixed it in net/core/filter.c)

- vlan tag handling is incorrect, since it's missing shifts.
classic BPF standard for vlan_tag_present has to return 1 or 0
and not just emit_and(r_A, r_s0, VLAN_TAG_PRESENT, ctx);

- pr_warn("%s: Unhandled opcode: 0x%02x\n", __FILE__,
is way too heavy, since when jit is on, unprivileged user can spam log.

- /* sa is 5-bits long */
BUG_ON(sa >= BIT(5));
is wrong too. Malicious user can cause kernel crash…
Also shift A>>=33 was always allowed by classic BPF checker, so
JITs have to silently do C-equivalent version of such shift.

- /* Determine if immediate is within the 16-bit signed range */
static inline bool is_range16(s32 imm)
{
if (imm >= SBIT(15) || imm < -SBIT(15))
return true;
the function name and comment are doing the opposite of
actual code, which makes harder to follow.

- the rest looks pretty good!

Also you'll get a lot more mileage out of mips jit if you use eBPF
instruction set as a base for JITing. You wouldn't need to worry
about vlan, pkt_type and other classic extensions. You'll get all
extensions for free, plus seccomp, tracing, etc.

Thanks
Alexei

2014-06-18 08:28:34

by Markos Chandras

[permalink] [raw]
Subject: Re: mips:allmodconfig build failure in 3.16-rc1 due to bpf_jit code

On 06/17/2014 08:38 PM, Alexei Starovoitov wrote:
> On Tue, Jun 17, 2014 at 4:21 AM, Daniel Borkmann <[email protected]> wrote:
>> On 06/17/2014 01:09 PM, Markos Chandras wrote:
>> ...
>>>
>>> Thanks for these instructions. I will try them myself once I find some
>>>
>>> time since I don't think bpf_jit for MIPS has ever been tested with all
>>> the opcodes.
>>
>>
>> Sounds great! If you find some tests are missing, please feel free to
>> submit them as well via netdev.
>>
>> Best,
>>
>> Daniel
>
> Daniel,
>
> thank you for taking care of it so quickly :)
> from the BPF perspective the fix looks good:
> Acked-by: Alexei Starovoitov <[email protected]>
>
> Markos,
>
> please do run the testsuite.
> Doing quick code review of mips jit, it looks like:
>
> - your version of pkt_type_offset() will work for little endian only.
> (we've recently fixed it in net/core/filter.c)
>
> - vlan tag handling is incorrect, since it's missing shifts.
> classic BPF standard for vlan_tag_present has to return 1 or 0
> and not just emit_and(r_A, r_s0, VLAN_TAG_PRESENT, ctx);
>
> - pr_warn("%s: Unhandled opcode: 0x%02x\n", __FILE__,
> is way too heavy, since when jit is on, unprivileged user can spam log.
>
> - /* sa is 5-bits long */
> BUG_ON(sa >= BIT(5));
> is wrong too. Malicious user can cause kernel crash…
> Also shift A>>=33 was always allowed by classic BPF checker, so
> JITs have to silently do C-equivalent version of such shift.
>
> - /* Determine if immediate is within the 16-bit signed range */
> static inline bool is_range16(s32 imm)
> {
> if (imm >= SBIT(15) || imm < -SBIT(15))
> return true;
> the function name and comment are doing the opposite of
> actual code, which makes harder to follow.
>
> - the rest looks pretty good!
>
> Also you'll get a lot more mileage out of mips jit if you use eBPF
> instruction set as a base for JITing. You wouldn't need to worry
> about vlan, pkt_type and other classic extensions. You'll get all
> extensions for free, plus seccomp, tracing, etc.
>
> Thanks
> Alexei
>
Hi Alexei,

Thanks a lot for the feedback. I have already identified a few problems
which I have already fixed. I would like to move to eBPF but I can't
promise I can do it soon, so i think it's best to make sure that classic
BPF works fine for 3.16 and then I will make my plans for eBPF.

--
markos