PPC: bpf_jit_comp: add SKF_AD_PKTTYPE instruction

PATCH In review Denis Kirjanov 6 participants 13 messages Oct 30, 2014 – Nov 3, 2014

Subsystems (5)

Networking Network drivers PowerPC BPF +1 more subsystems

lore

Share & tools

lore

Denis Kirjanov

author

2014-10-30 06:39 UTC #0 lore raw

PATCH v2 1/1 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<[email protected]>

CC Michael Ellerman<[email protected]>

Cc Matt Evans <[email protected]>

Signed-off-by Denis Kirjanov <[email protected]>

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

arch/powerpc/include/asm/ppc-opcode.h +1 -0

index 6f85362..1a52877 100644
@@ -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

arch/powerpc/net/bpf_jit.h +7 -0

index 9aee27c..c406aa9 100644
@@ -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)

arch/powerpc/net/bpf_jit_comp.c +5 -0

index cbae2df..d110e28 100644
@@ -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
 			/*

Signature · 1 line

2.1.0

Alexei Starovoitov

2014-10-30 20:32 UTC #1 lore raw

Quoted 10 lines · Denis Kirjanov

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?

Denis Kirjanov

2014-10-31 06:09 UTC #2 lore raw

Quoted 9 lines · Alexei Starovoitov

Quoted 7 lines · Denis Kirjanov
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

Quoted 7 lines

Quoted 3 lines
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 5 lines

--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines

Regards,
Denis

Denis Kirjanov

author

2014-11-01 16:19 UTC #3 lore raw

ping

Quoted 31 lines · Denis Kirjanov

Quoted 9 lines · Alexei Starovoitov
Quoted 7 lines · Denis Kirjanov
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
Quoted 7 lines
Quoted 3 lines
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 5 lines
--
To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Regards,
Denis

David Miller

maintainer

2014-11-01 17:40 UTC #4 lore raw

From: Denis Kirjanov <[email protected]>
Date: Sat, 1 Nov 2014 20:19:09 +0400

Quoted 1 line

ping

What specifically are you waiting for?

Denis Kirjanov

author

2014-11-01 17:49 UTC #5 lore raw

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

Quoted 7 lines · David Miller

From: Denis Kirjanov <[email protected]>
Date: Sat, 1 Nov 2014 20:19:09 +0400
Quoted 1 line
ping
What specifically are you waiting for?

_______________________________________________
Linuxppc-dev mailing list
[email protected]
https://lists.ozlabs.org/listinfo/linuxppc-dev

David Miller

maintainer

2014-11-01 18:00 UTC #6 lore raw

From: Denis Kirjanov <[email protected]>
Date: Sat, 1 Nov 2014 21:49:27 +0400

Quoted 5 lines

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.

Daniel Borkmann

2014-11-01 18:31 UTC #7 lore raw

Quoted 7 lines · David Miller

From: Denis Kirjanov <[email protected]>
Date: Sat, 1 Nov 2014 21:49:27 +0400
Quoted 4 lines
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 5 lines

Quoted 1 line
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.

Quoted 3 lines

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.

Daniel Borkmann

2014-11-01 18:40 UTC #8 lore raw

In reply to Denis Kirjanov

Quoted 14 lines · Denis Kirjanov

Quoted 13 lines · Alexei Starovoitov
Quoted 10 lines · Denis Kirjanov
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

Quoted 5 lines

Quoted 3 lines
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.

David Miller

maintainer

2014-11-03 17:06 UTC #9 lore raw

In reply to Denis Kirjanov

From: Denis Kirjanov <[email protected]>
Date: Thu, 30 Oct 2014 09:12:15 +0300

Quoted 11 lines

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

Quoted 6 lines

CC Alexei Starovoitov<[email protected]>
CC Michael Ellerman<[email protected]>
Cc Matt Evans <[email protected]>
Signed-off-by Denis Kirjanov <[email protected]>
v2: Added test rusults

So, can I apply this now?

Alexei Starovoitov

2014-11-03 17:21 UTC #10 lore raw

Quoted 14 lines · David Miller

From: Denis Kirjanov <[email protected]>
Date: Thu, 30 Oct 2014 09:12:15 +0300
Quoted 11 lines
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

Quoted 8 lines

Quoted 6 lines
CC Alexei Starovoitov<[email protected]>
CC Michael Ellerman<[email protected]>
Cc Matt Evans <[email protected]>
Signed-off-by Denis Kirjanov <[email protected]>
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.

David Miller

maintainer

2014-11-03 20:30 UTC #11 lore raw

From: Alexei Starovoitov <[email protected]>
Date: Mon, 3 Nov 2014 09:21:03 -0800

Quoted 22 lines · David Miller

Quoted 21 lines
From: Denis Kirjanov <[email protected]>
Date: Thu, 30 Oct 2014 09:12:15 +0300
Quoted 17 lines
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<[email protected]>
CC Michael Ellerman<[email protected]>
Cc Matt Evans <[email protected]>
Signed-off-by Denis Kirjanov <[email protected]>
v2: Added test rusults

Quoted 8 lines

Quoted 1 line
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.

Michael Ellerman

reviewer

2014-11-03 23:43 UTC #12 lore raw

In reply to Alexei Starovoitov

Quoted 20 lines · Alexei Starovoitov

Quoted 19 lines · David Miller
From: Denis Kirjanov <[email protected]>
Date: Thu, 30 Oct 2014 09:12:15 +0300
Quoted 16 lines
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<[email protected]>
CC Michael Ellerman<[email protected]>
Cc Matt Evans <[email protected]>
Signed-off-by Denis Kirjanov <[email protected]>

Quoted 6 lines

Quoted 3 lines
Quoted 1 line
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.

Quoted 3 lines

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