| Message ID | 1414649535-3956-1-git-send-email-kda@linux-powerpc.org |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
On Wed, Oct 29, 2014 at 11:12 PM, Denis Kirjanov <kda@linux-powerpc.org> 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? -- 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
On 10/30/14, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > On Wed, Oct 29, 2014 at 11:12 PM, Denis Kirjanov <kda@linux-powerpc.org> > 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? 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 >
ping On 10/31/14, Denis Kirjanov <kirjanov@gmail.com> wrote: > On 10/30/14, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: >> On Wed, Oct 29, 2014 at 11:12 PM, Denis Kirjanov <kda@linux-powerpc.org> >> 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? > > 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 > -- 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
From: Denis Kirjanov <kda@linux-powerpc.org> Date: Sat, 1 Nov 2014 20:19:09 +0400 > ping What specifically are you waiting for? -- 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, 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 <davem@davemloft.net> wrote: > From: Denis Kirjanov <kda@linux-powerpc.org> > Date: Sat, 1 Nov 2014 20:19:09 +0400 > >> ping > > What specifically are you waiting for? > -- 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
From: Denis Kirjanov <kda@linux-powerpc.org> 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. -- 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
On 11/01/2014 07:00 PM, David Miller wrote: > From: Denis Kirjanov <kda@linux-powerpc.org> > 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. 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. >> 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. -- 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
On 10/31/2014 07:09 AM, Denis Kirjanov wrote: > On 10/30/14, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: >> On Wed, Oct 29, 2014 at 11:12 PM, Denis Kirjanov <kda@linux-powerpc.org> >> 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? > > 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. -- 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
From: Denis Kirjanov <kda@linux-powerpc.org> 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<alexei.starovoitov@gmail.com> > CC: Michael Ellerman<mpe@ellerman.id.au> > Cc: Matt Evans <matt@ozlabs.org> > Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org> > > v2: Added test rusults So, can I apply this now? -- 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
On Mon, Nov 3, 2014 at 9:06 AM, David Miller <davem@davemloft.net> wrote: > From: Denis Kirjanov <kda@linux-powerpc.org> > 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<alexei.starovoitov@gmail.com> >> CC: Michael Ellerman<mpe@ellerman.id.au> >> Cc: Matt Evans <matt@ozlabs.org> >> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org> >> >> 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. -- 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
From: Alexei Starovoitov <alexei.starovoitov@gmail.com> Date: Mon, 3 Nov 2014 09:21:03 -0800 > On Mon, Nov 3, 2014 at 9:06 AM, David Miller <davem@davemloft.net> wrote: >> From: Denis Kirjanov <kda@linux-powerpc.org> >> 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<alexei.starovoitov@gmail.com> >>> CC: Michael Ellerman<mpe@ellerman.id.au> >>> Cc: Matt Evans <matt@ozlabs.org> >>> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org> >>> >>> 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. -- 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
On Mon, 2014-11-03 at 09:21 -0800, Alexei Starovoitov wrote: > On Mon, Nov 3, 2014 at 9:06 AM, David Miller <davem@davemloft.net> wrote: > > From: Denis Kirjanov <kda@linux-powerpc.org> > > 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<alexei.starovoitov@gmail.com> > >> CC: Michael Ellerman<mpe@ellerman.id.au> > >> Cc: Matt Evans <matt@ozlabs.org> > >> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org> > >> > >> 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 -- 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 --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 /*
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<alexei.starovoitov@gmail.com> CC: Michael Ellerman<mpe@ellerman.id.au> Cc: Matt Evans <matt@ozlabs.org> Signed-off-by: Denis Kirjanov <kda@linux-powerpc.org> 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(+)