[PATCH v2 1/2] powerpc: bpf: Use correct mask while accessing the VLAN tag

[PATCH v2 1/2] powerpc: bpf: Use correct mask while accessing the VLAN tag

[Denis Kirjanov]

To get a full tag (and not just a VID) we should access the TCI
except the VLAN_TAG_PRESENT field (which means that 802.1q header
is present). Also ensure that the VLAN_TAG_PRESENT stay on its place

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

diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index 6dcdade..892167b 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c
@@ -390,10 +390,12 @@ static int bpf_jit_build_body(struct sk_filter *fp, u32 *image,
 		case BPF_ANC | SKF_AD_VLAN_TAG:
 		case BPF_ANC | SKF_AD_VLAN_TAG_PRESENT:
 			BUILD_BUG_ON(FIELD_SIZEOF(struct sk_buff, vlan_tci) != 2);
+			BUILD_BUG_ON(VLAN_TAG_PRESENT != 0x1000);
+
 			PPC_LHZ_OFFS(r_A, r_skb, offsetof(struct sk_buff,
 							  vlan_tci));
 			if (code == (BPF_ANC | SKF_AD_VLAN_TAG))
-				PPC_ANDI(r_A, r_A, VLAN_VID_MASK);
+				PPC_ANDI(r_A, r_A, ~VLAN_TAG_PRESENT);
 			else
 				PPC_ANDI(r_A, r_A, VLAN_TAG_PRESENT);
 			break;
-- 
2.0.0

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

[Denis Kirjanov]

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(-)

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,
-- 
2.0.0

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

[Michael Ellerman]

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

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

[Daniel Borkmann]

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

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

[Michael Ellerman]

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

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

[Daniel Borkmann]

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

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

[Denis Kirjanov]

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
>
>
>

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

[David Miller]

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.

Re: [PATCH v2 1/2] powerpc: bpf: Use correct mask while accessing the VLAN tag

[David Miller]

From: Denis Kirjanov <kda@linux-powerpc.org>
Date: Wed, 25 Jun 2014 21:34:56 +0400

> To get a full tag (and not just a VID) we should access the TCI
> except the VLAN_TAG_PRESENT field (which means that 802.1q header
> is present). Also ensure that the VLAN_TAG_PRESENT stay on its place
> 
> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org>

Applied.