[PATCH v2] PPC: bpf_jit_comp: add SKF_AD_PKTTYPE instruction

Add BPF extension SKF_AD_PKTTYPE to ppc JIT to load
skb->pkt_type field.

Before:
[   88.262622] test_bpf: #11 LD_IND_NET 86 97 99 PASS
[   88.265740] test_bpf: #12 LD_PKTTYPE 109 107 PASS

After:
[   80.605964] test_bpf: #11 LD_IND_NET 44 40 39 PASS
[   80.607370] test_bpf: #12 LD_PKTTYPE 9 9 PASS

CC: Alexei Starovoitov<redacted>
CC: Michael Ellerman<mpe@ellerman.id.au>
Cc: Matt Evans <redacted>
Signed-off-by: Denis Kirjanov <redacted>

v2: Added test rusults
---
 arch/powerpc/include/asm/ppc-opcode.h | 1 +
 arch/powerpc/net/bpf_jit.h            | 7 +++++++
 arch/powerpc/net/bpf_jit_comp.c       | 5 +++++
 3 files changed, 13 insertions(+)

diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
index 6f85362..1a52877 100644
--- a/arch/powerpc/include/asm/ppc-opcode.h
+++ b/arch/powerpc/include/asm/ppc-opcode.h

@@ -204,6 +204,7 @@
 #define PPC_INST_ERATSX_DOT		0x7c000127
 
 /* Misc instructions for BPF compiler */
+#define PPC_INST_LBZ			0x88000000
 #define PPC_INST_LD			0xe8000000
 #define PPC_INST_LHZ			0xa0000000
 #define PPC_INST_LHBRX			0x7c00062c

diff --git a/arch/powerpc/net/bpf_jit.h b/arch/powerpc/net/bpf_jit.h
index 9aee27c..c406aa9 100644
--- a/arch/powerpc/net/bpf_jit.h
+++ b/arch/powerpc/net/bpf_jit.h

@@ -87,6 +87,9 @@ DECLARE_LOAD_FUNC(sk_load_byte_msh);
 #define PPC_STD(r, base, i)	EMIT(PPC_INST_STD | ___PPC_RS(r) |	      \
 				     ___PPC_RA(base) | ((i) & 0xfffc))
 
+
+#define PPC_LBZ(r, base, i)	EMIT(PPC_INST_LBZ | ___PPC_RT(r) |	      \
+				     ___PPC_RA(base) | IMM_L(i))
 #define PPC_LD(r, base, i)	EMIT(PPC_INST_LD | ___PPC_RT(r) |	      \
 				     ___PPC_RA(base) | IMM_L(i))
 #define PPC_LWZ(r, base, i)	EMIT(PPC_INST_LWZ | ___PPC_RT(r) |	      \

@@ -96,6 +99,10 @@ DECLARE_LOAD_FUNC(sk_load_byte_msh);
 #define PPC_LHBRX(r, base, b)	EMIT(PPC_INST_LHBRX | ___PPC_RT(r) |	      \
 				     ___PPC_RA(base) | ___PPC_RB(b))
 /* Convenience helpers for the above with 'far' offsets: */
+#define PPC_LBZ_OFFS(r, base, i) do { if ((i) < 32768) PPC_LBZ(r, base, i);   \
+		else {	PPC_ADDIS(r, base, IMM_HA(i));			      \
+			PPC_LBZ(r, r, IMM_L(i)); } } while(0)
+
 #define PPC_LD_OFFS(r, base, i) do { if ((i) < 32768) PPC_LD(r, base, i);     \
 		else {	PPC_ADDIS(r, base, IMM_HA(i));			      \
 			PPC_LD(r, r, IMM_L(i)); } } while(0)

diff --git a/arch/powerpc/net/bpf_jit_comp.c b/arch/powerpc/net/bpf_jit_comp.c
index cbae2df..d110e28 100644
--- a/arch/powerpc/net/bpf_jit_comp.c
+++ b/arch/powerpc/net/bpf_jit_comp.c

@@ -407,6 +407,11 @@ static int bpf_jit_build_body(struct bpf_prog *fp, u32 *image,
 			PPC_LHZ_OFFS(r_A, r_skb, offsetof(struct sk_buff,
 							  queue_mapping));
 			break;
+		case BPF_ANC | SKF_AD_PKTTYPE:
+			PPC_LBZ_OFFS(r_A, r_skb, PKT_TYPE_OFFSET());
+			PPC_ANDI(r_A, r_A, PKT_TYPE_MAX);
+			PPC_SRWI(r_A, r_A, 5);
+			break;
 		case BPF_ANC | SKF_AD_CPU:
 #ifdef CONFIG_SMP
 			/*

-- 
2.1.0

On Wed, Oct 29, 2014 at 11:12 PM, Denis Kirjanov [off-list ref] wrote:

Add BPF extension SKF_AD_PKTTYPE to ppc JIT to load
skb->pkt_type field.

Before:
[   88.262622] test_bpf: #11 LD_IND_NET 86 97 99 PASS
[   88.265740] test_bpf: #12 LD_PKTTYPE 109 107 PASS

After:
[   80.605964] test_bpf: #11 LD_IND_NET 44 40 39 PASS
[   80.607370] test_bpf: #12 LD_PKTTYPE 9 9 PASS

if you'd only quoted #12, it would all make sense ;)
but #11 test is not using PKTTYPE. So your patch shouldn't
make a difference. Are these numbers with JIT on and off?

On 10/30/14, Alexei Starovoitov [off-list ref] wrote:

On Wed, Oct 29, 2014 at 11:12 PM, Denis Kirjanov [off-list ref]
wrote:

quoted

Add BPF extension SKF_AD_PKTTYPE to ppc JIT to load
skb->pkt_type field.

Before:
[   88.262622] test_bpf: #11 LD_IND_NET 86 97 99 PASS
[   88.265740] test_bpf: #12 LD_PKTTYPE 109 107 PASS

After:
[   80.605964] test_bpf: #11 LD_IND_NET 44 40 39 PASS
[   80.607370] test_bpf: #12 LD_PKTTYPE 9 9 PASS

if you'd only quoted #12, it would all make sense ;)
but #11 test is not using PKTTYPE. So your patch shouldn't
make a difference. Are these numbers with JIT on and off?

Right.

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

-- 
Regards,
Denis

ping

On 10/31/14, Denis Kirjanov [off-list ref] wrote:

On 10/30/14, Alexei Starovoitov [off-list ref] wrote:

quoted

On Wed, Oct 29, 2014 at 11:12 PM, Denis Kirjanov [off-list ref]
wrote:

quoted

Add BPF extension SKF_AD_PKTTYPE to ppc JIT to load
skb->pkt_type field.

Before:
[   88.262622] test_bpf: #11 LD_IND_NET 86 97 99 PASS
[   88.265740] test_bpf: #12 LD_PKTTYPE 109 107 PASS

After:
[   80.605964] test_bpf: #11 LD_IND_NET 44 40 39 PASS
[   80.607370] test_bpf: #12 LD_PKTTYPE 9 9 PASS

if you'd only quoted #12, it would all make sense ;)
but #11 test is not using PKTTYPE. So your patch shouldn't
make a difference. Are these numbers with JIT on and off?

Right.

quoted

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

--
Regards,
Denis

David, you need a feedback from other guys to apply this patch, right?

Alexei wanted some output before/after the patch.
Michael Ellerman wanted the explanation what a BPF_ANC | SKF_AD_PKTTYPE means.
So I'm waiting  the ack/nack from them...

On 11/1/14, David Miller [off-list ref] wrote:

From: Denis Kirjanov <redacted>
Date: Sat, 1 Nov 2014 20:19:09 +0400

quoted

ping

What specifically are you waiting for?

From: Denis Kirjanov <redacted>
Date: Sat, 1 Nov 2014 20:19:09 +0400

ping

What specifically are you waiting for?

From: Denis Kirjanov <redacted>
Date: Sat, 1 Nov 2014 21:49:27 +0400

David, you need a feedback from other guys to apply this patch, right?

Alexei wanted some output before/after the patch.
Michael Ellerman wanted the explanation what a BPF_ANC | SKF_AD_PKTTYPE means.
So I'm waiting  the ack/nack from them...

I don't really think performance metrics are necessary just for adding
SKF_AD_PKTTYPE support, that's sort of an over the top requirement
if you ask me.

It's pretty obvious that we should support as many operations as
possible to each JIT, because all of program has to do is use that
unsupported opcode and then we have none of that program being JIT'd.

On 11/01/2014 07:00 PM, David Miller wrote:

From: Denis Kirjanov <redacted>
Date: Sat, 1 Nov 2014 21:49:27 +0400

quoted

David, you need a feedback from other guys to apply this patch, right?

Alexei wanted some output before/after the patch.
Michael Ellerman wanted the explanation what a BPF_ANC | SKF_AD_PKTTYPE means.

The BPF_ANC | SKF_AD_PKTTYPE case means that this is an ancillary
operation aka BPF extension which loads the value of skb->pkt_type
into the accumulator.

A similar transformation, that is, from BPF into eBPF insns can be
found in convert_bpf_extensions() in the SKF_AD_PKTTYPE case, or
commit 709f6c58d4dc ("sparc: bpf_jit: add SKF_AD_PKTTYPE support
to JIT") that recently enabled the same in sparc.

quoted

So I'm waiting  the ack/nack from them...

I don't really think performance metrics are necessary just for adding
SKF_AD_PKTTYPE support, that's sort of an over the top requirement
if you ask me.

Right, lib/test_bpf.c actually brings the quoted output w/ numbers
for free. I think the important point was that the 'After:' case
with ``echo 1 > /proc/sys/net/core/bpf_jit_enable'' runs through for
that test case, which has been shown here.

It's pretty obvious that we should support as many operations as
possible to each JIT, because all of program has to do is use that
unsupported opcode and then we have none of that program being JIT'd.

On 10/31/2014 07:09 AM, Denis Kirjanov wrote:

On 10/30/14, Alexei Starovoitov [off-list ref] wrote:

quoted

On Wed, Oct 29, 2014 at 11:12 PM, Denis Kirjanov [off-list ref]
wrote:

quoted

Add BPF extension SKF_AD_PKTTYPE to ppc JIT to load
skb->pkt_type field.

Before:
[   88.262622] test_bpf: #11 LD_IND_NET 86 97 99 PASS
[   88.265740] test_bpf: #12 LD_PKTTYPE 109 107 PASS

After:
[   80.605964] test_bpf: #11 LD_IND_NET 44 40 39 PASS
[   80.607370] test_bpf: #12 LD_PKTTYPE 9 9 PASS

if you'd only quoted #12, it would all make sense ;)
but #11 test is not using PKTTYPE. So your patch shouldn't
make a difference. Are these numbers with JIT on and off?

Right.

Ok.

Please mention this in future log messages, as it was not quite
clear that "before" was actually with JIT off, and "after" was
with JIT on.

One could have read it that actually both cases were with JIT on,
and thus the inconsistent result for LD_IND_NET is a bit confusing
since you've quoted it here as well.

From: Denis Kirjanov <redacted>
Date: Thu, 30 Oct 2014 09:12:15 +0300

Add BPF extension SKF_AD_PKTTYPE to ppc JIT to load
skb->pkt_type field.

Before:
[   88.262622] test_bpf: #11 LD_IND_NET 86 97 99 PASS
[   88.265740] test_bpf: #12 LD_PKTTYPE 109 107 PASS

After:
[   80.605964] test_bpf: #11 LD_IND_NET 44 40 39 PASS
[   80.607370] test_bpf: #12 LD_PKTTYPE 9 9 PASS

CC: Alexei Starovoitov<redacted>
CC: Michael Ellerman<mpe@ellerman.id.au>
Cc: Matt Evans <redacted>
Signed-off-by: Denis Kirjanov <redacted>

v2: Added test rusults

So, can I apply this now?

On Mon, Nov 3, 2014 at 9:06 AM, David Miller [off-list ref] wrote:

From: Denis Kirjanov <redacted>
Date: Thu, 30 Oct 2014 09:12:15 +0300

quoted

Add BPF extension SKF_AD_PKTTYPE to ppc JIT to load
skb->pkt_type field.

Before:
[   88.262622] test_bpf: #11 LD_IND_NET 86 97 99 PASS
[   88.265740] test_bpf: #12 LD_PKTTYPE 109 107 PASS

After:
[   80.605964] test_bpf: #11 LD_IND_NET 44 40 39 PASS
[   80.607370] test_bpf: #12 LD_PKTTYPE 9 9 PASS

CC: Alexei Starovoitov<redacted>
CC: Michael Ellerman<mpe@ellerman.id.au>
Cc: Matt Evans <redacted>
Signed-off-by: Denis Kirjanov <redacted>

v2: Added test rusults

So, can I apply this now?

I think this question is more towards ppc folks,
since both Daniel and myself said before that it looks ok.
Philippe just tested the previous version of this patch on ppc64le...
I'm guessing that Matt (original author of bpf jit for ppc) is not replying,
because he has no objections.
Either way the addition is tiny and contained, so can go in now.

From: Alexei Starovoitov <redacted>
Date: Mon, 3 Nov 2014 09:21:03 -0800

On Mon, Nov 3, 2014 at 9:06 AM, David Miller [off-list ref] wrote:

quoted

From: Denis Kirjanov <redacted>
Date: Thu, 30 Oct 2014 09:12:15 +0300

quoted

Add BPF extension SKF_AD_PKTTYPE to ppc JIT to load
skb->pkt_type field.

Before:
[   88.262622] test_bpf: #11 LD_IND_NET 86 97 99 PASS
[   88.265740] test_bpf: #12 LD_PKTTYPE 109 107 PASS

After:
[   80.605964] test_bpf: #11 LD_IND_NET 44 40 39 PASS
[   80.607370] test_bpf: #12 LD_PKTTYPE 9 9 PASS

CC: Alexei Starovoitov<redacted>
CC: Michael Ellerman<mpe@ellerman.id.au>
Cc: Matt Evans <redacted>
Signed-off-by: Denis Kirjanov <redacted>

v2: Added test rusults

So, can I apply this now?

I think this question is more towards ppc folks,
since both Daniel and myself said before that it looks ok.
Philippe just tested the previous version of this patch on ppc64le...
I'm guessing that Matt (original author of bpf jit for ppc) is not replying,
because he has no objections.
Either way the addition is tiny and contained, so can go in now.

Ok, I have applied this to net-next, thanks everyone.

On Mon, 2014-11-03 at 09:21 -0800, Alexei Starovoitov wrote:

On Mon, Nov 3, 2014 at 9:06 AM, David Miller [off-list ref] wrote:

quoted

From: Denis Kirjanov <redacted>
Date: Thu, 30 Oct 2014 09:12:15 +0300

quoted

Add BPF extension SKF_AD_PKTTYPE to ppc JIT to load
skb->pkt_type field.

Before:
[   88.262622] test_bpf: #11 LD_IND_NET 86 97 99 PASS
[   88.265740] test_bpf: #12 LD_PKTTYPE 109 107 PASS

After:
[   80.605964] test_bpf: #11 LD_IND_NET 44 40 39 PASS
[   80.607370] test_bpf: #12 LD_PKTTYPE 9 9 PASS

CC: Alexei Starovoitov<redacted>
CC: Michael Ellerman<mpe@ellerman.id.au>
Cc: Matt Evans <redacted>
Signed-off-by: Denis Kirjanov <redacted>

v2: Added test rusults

So, can I apply this now?

I think this question is more towards ppc folks,
since both Daniel and myself said before that it looks ok.

Yeah sorry, as I said I don't really know enough about BPF to ack it.

Philippe just tested the previous version of this patch on ppc64le...
I'm guessing that Matt (original author of bpf jit for ppc) is not replying,
because he has no objections.

Actually that might be because he works at ARM now :)

If you can CC Philippe on future BPF patches for powerpc that would probably
help.

cheers