diff mbox

[v2,2/2] powerpc: bpf: Fix the broken LD_VLAN_TAG_PRESENT test

Message ID 1403717697-3911-2-git-send-email-kda@linux-powerpc.org
State Accepted, archived
Delegated to: David Miller
Headers show

Commit Message

Denis Kirjanov June 25, 2014, 5:34 p.m. UTC
We have to return the boolean here if the tag presents
or not, not just ANDing the TCI with the mask which results to:

[  709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT
[  709.412245] ret 4096 != 1
[  709.412332] ret 4096 != 1
[  709.412333] FAIL (2 times)

Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>
---
 arch/powerpc/net/bpf_jit_comp.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Michael Ellerman June 26, 2014, 8:30 a.m. UTC | #1
On Wed, 2014-06-25 at 21:34 +0400, Denis Kirjanov wrote:
> We have to return the boolean here if the tag presents
> or not, not just ANDing the TCI with the mask which results to:
> 
> [  709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT
> [  709.412245] ret 4096 != 1
> [  709.412332] ret 4096 != 1
> [  709.412333] FAIL (2 times)

Hi Denis,

Is this the same version of test_bpf that is in Linus' tree?

I don't see any fails with that version, am I missing something?

  [  187.690640] test_bpf: #18 LD_VLAN_TAG_PRESENT 46 50 PASS

cheers


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Borkmann June 26, 2014, 8:36 a.m. UTC | #2
On 06/26/2014 10:30 AM, Michael Ellerman wrote:
> On Wed, 2014-06-25 at 21:34 +0400, Denis Kirjanov wrote:
>> We have to return the boolean here if the tag presents
>> or not, not just ANDing the TCI with the mask which results to:
>>
>> [  709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT
>> [  709.412245] ret 4096 != 1
>> [  709.412332] ret 4096 != 1
>> [  709.412333] FAIL (2 times)
>
> Hi Denis,
>
> Is this the same version of test_bpf that is in Linus' tree?
>
> I don't see any fails with that version, am I missing something?
>
>    [  187.690640] test_bpf: #18 LD_VLAN_TAG_PRESENT 46 50 PASS

Did you try to modprobe test_bpf after enabling JIT, i.e. :

   echo 1 > /proc/sys/net/core/bpf_jit_enable

Last time I tested with ppc64, I saw the interpreter passing
while JIT failed, which the fix above should address.

Cheers,

Daniel
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Denis Kirjanov June 26, 2014, 3:43 p.m. UTC | #3
On 6/26/14, Michael Ellerman <mpe@ellerman.id.au> wrote:
> On Wed, 2014-06-25 at 21:34 +0400, Denis Kirjanov wrote:
>> We have to return the boolean here if the tag presents
>> or not, not just ANDing the TCI with the mask which results to:
>>
>> [  709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT
>> [  709.412245] ret 4096 != 1
>> [  709.412332] ret 4096 != 1
>> [  709.412333] FAIL (2 times)
>
> Hi Denis,
>
> Is this the same version of test_bpf that is in Linus' tree?

You didn't enable the JIT.
> I don't see any fails with that version, am I missing something?
>
>   [  187.690640] test_bpf: #18 LD_VLAN_TAG_PRESENT 46 50 PASS
>
> cheers
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michael Ellerman June 27, 2014, 5:08 a.m. UTC | #4
On Thu, 2014-06-26 at 10:36 +0200, Daniel Borkmann wrote:
> On 06/26/2014 10:30 AM, Michael Ellerman wrote:
> > On Wed, 2014-06-25 at 21:34 +0400, Denis Kirjanov wrote:
> >> We have to return the boolean here if the tag presents
> >> or not, not just ANDing the TCI with the mask which results to:
> >>
> >> [  709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT
> >> [  709.412245] ret 4096 != 1
> >> [  709.412332] ret 4096 != 1
> >> [  709.412333] FAIL (2 times)
> >
> > Hi Denis,
> >
> > Is this the same version of test_bpf that is in Linus' tree?
> >
> > I don't see any fails with that version, am I missing something?
> >
> >    [  187.690640] test_bpf: #18 LD_VLAN_TAG_PRESENT 46 50 PASS
> 
> Did you try to modprobe test_bpf after enabling JIT, i.e. :
> 
>    echo 1 > /proc/sys/net/core/bpf_jit_enable
> 
> Last time I tested with ppc64, I saw the interpreter passing
> while JIT failed, which the fix above should address.

Right, I thought CONFIG_BPF_JIT=y was sufficient.

But that just makes it available, I need to *also* enable it at runtime.

cheers


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Daniel Borkmann June 27, 2014, 9:46 a.m. UTC | #5
On 06/27/2014 07:08 AM, Michael Ellerman wrote:
> On Thu, 2014-06-26 at 10:36 +0200, Daniel Borkmann wrote:
>> On 06/26/2014 10:30 AM, Michael Ellerman wrote:
>>> On Wed, 2014-06-25 at 21:34 +0400, Denis Kirjanov wrote:
>>>> We have to return the boolean here if the tag presents
>>>> or not, not just ANDing the TCI with the mask which results to:
>>>>
>>>> [  709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT
>>>> [  709.412245] ret 4096 != 1
>>>> [  709.412332] ret 4096 != 1
>>>> [  709.412333] FAIL (2 times)
>>>
>>> Hi Denis,
>>>
>>> Is this the same version of test_bpf that is in Linus' tree?
>>>
>>> I don't see any fails with that version, am I missing something?
>>>
>>>     [  187.690640] test_bpf: #18 LD_VLAN_TAG_PRESENT 46 50 PASS
>>
>> Did you try to modprobe test_bpf after enabling JIT, i.e. :
>>
>>     echo 1 > /proc/sys/net/core/bpf_jit_enable
>>
>> Last time I tested with ppc64, I saw the interpreter passing
>> while JIT failed, which the fix above should address.
>
> Right, I thought CONFIG_BPF_JIT=y was sufficient.
>
> But that just makes it available, I need to *also* enable it at runtime.

Yes, it's an extra runtime knob which still needs to be enabled by
the admin for security reasons. This knob also allows you to enable
the debugging interface `echo 2 >/proc/sys/net/core/bpf_jit_enable`
where the disassembly can be read out via: tools/net/bpf_jit_disasm.c
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller June 27, 2014, 11:14 p.m. UTC | #6
From: Denis Kirjanov <kda@linux-powerpc.org>
Date: Wed, 25 Jun 2014 21:34:57 +0400

> We have to return the boolean here if the tag presents
> or not, not just ANDing the TCI with the mask which results to:
> 
> [  709.412097] test_bpf: #18 LD_VLAN_TAG_PRESENT
> [  709.412245] ret 4096 != 1
> [  709.412332] ret 4096 != 1
> [  709.412333] FAIL (2 times)
> 
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>

Applied.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 892167b..82e82ca 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -394,10 +394,12 @@  static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
 
 			PPC_LHZ_OFFS(r_A, r_skb, offsetof(struct sk_buff,
 							  vlan_tci));
-			if (code == (BPF_ANC | SKF_AD_VLAN_TAG))
+			if (code == (BPF_ANC | SKF_AD_VLAN_TAG)) {
 				PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT);
-			else
+			} else {
 				PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT);
+				PPC_SRWI(r_A, r_A, 12);
+			}
 			break;
 		case BPF_ANC | SKF_AD_QUEUE:
 			BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff,