Re: [RFC][PATCH bpf v2 1/2] bpf: allow 64-bit offsets for bpf function calls

[Michael Holzheu]

Am Thu, 22 Feb 2018 13:06:40 +0100
schrieb Michael Holzheu <holz...@linux.vnet.ibm.com>:

> Am Fri, 16 Feb 2018 21:20:09 +0530
> schrieb "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com>:
> 
> > Daniel Borkmann wrote:
> > > On 02/15/2018 05:25 PM, Daniel Borkmann wrote:
> > >> On 02/13/2018 05:05 AM, Sandipan Das wrote:
> > >>> The imm field of a bpf_insn is a signed 32-bit integer. For
> > >>> JIT-ed bpf-to-bpf function calls, it stores the offset from
> > >>> __bpf_call_base to the start of the callee function.
> > >>>
> > >>> For some architectures, such as powerpc64, it was found that
> > >>> this offset may be as large as 64 bits because of which this
> > >>> cannot be accomodated in the imm field without truncation.
> > >>>
> > >>> To resolve this, we additionally make aux->func within each
> > >>> bpf_prog associated with the functions to point to the list
> > >>> of all function addresses determined by the verifier.
> > >>>
> > >>> We keep the value assigned to the off field of the bpf_insn
> > >>> as a way to index into aux->func and also set aux->func_cnt
> > >>> so that this can be used for performing basic upper bound
> > >>> checks for the off field.
> > >>>
> > >>> Signed-off-by: Sandipan Das <sandi...@linux.vnet.ibm.com>
> > >>> ---
> > >>> v2: Make aux->func point to the list of functions determined
> > >>> by the verifier rather than allocating a separate callee
> > >>> list for each function.
> > >> 
> > >> Approach looks good to me; do you know whether s390x JIT would
> > >> have similar requirement? I think one limitation that would still
> > >> need to be addressed later with such approach would be regarding the
> > >> xlated prog dump in bpftool, see 'BPF calls via JIT' in 7105e828c087
> > >> ("bpf: allow for correlation of maps and helpers in dump"). Any
> > >> ideas for this (potentially if we could use off + imm for calls,
> > >> we'd get to 48 bits, but that seems still not be enough as you say)?
> > 
> > All good points. I'm not really sure how s390x works, so I can't comment 
> > on that, but I'm copying Michael Holzheu for his consideration.
> > 
> > With the existing scheme, 48 bits won't be enough, so we rejected that 
> > approach. I can also see how this will be a problem with bpftool, but I 
> > haven't looked into it in detail. I wonder if we can annotate the output 
> > to indicate the function being referred to?
> > 
> > > 
> > > One other random thought, although I'm not sure how feasible this
> > > is for ppc64 JIT to realize ... but idea would be to have something
> > > like the below:
> > > 
> > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > > index 29ca920..daa7258 100644
> > > --- a/kernel/bpf/core.c
> > > +++ b/kernel/bpf/core.c
> > > @@ -512,6 +512,11 @@ int bpf_get_kallsym(unsigned int symnum, unsigned 
> > > long *value, char *type,
> > >   return ret;
> > >  }
> > > 
> > > +void * __weak bpf_jit_image_alloc(unsigned long size)
> > > +{
> > > + return module_alloc(size);
> > > +}
> > > +
> > >  struct bpf_binary_header *
> > >  bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
> > >unsigned int alignment,
> > > @@ -525,7 +530,7 @@ bpf_jit_binary_alloc(unsigned int proglen, u8 
> > > **image_ptr,
> > >* random section of illegal instructions.
> > >*/
> > >   size = round_up(proglen + sizeof(*hdr) + 128, PAGE_SIZE);
> > > - hdr = module_alloc(size);
> > > + hdr = bpf_jit_image_alloc(size);
> > >   if (hdr == NULL)
> > >   return NULL;
> > > 
> > > And ppc64 JIT could override bpf_jit_image_alloc() in a similar way
> > > like some archs would override the module_alloc() helper through a
> > > custom implementation, usually via __vmalloc_node_range(), so we
> > > could perhaps fit the range for BPF JITed images in a way that they
> > > could use the 32bit imm in the end? There are not that many progs
> > > loaded typically, so the range could be a bit narrower in such case
> > > anyway. (Not sure if this would work out though, but I thought to
> > > bring it up.)
> > 
> > That'd 

Re: [RFC][PATCH bpf v2 1/2] bpf: allow 64-bit offsets for bpf function calls

[Michael Holzheu]

Am Fri, 16 Feb 2018 21:20:09 +0530
schrieb "Naveen N. Rao" <naveen.n@linux.vnet.ibm.com>:

> Daniel Borkmann wrote:
> > On 02/15/2018 05:25 PM, Daniel Borkmann wrote:
> >> On 02/13/2018 05:05 AM, Sandipan Das wrote:
> >>> The imm field of a bpf_insn is a signed 32-bit integer. For
> >>> JIT-ed bpf-to-bpf function calls, it stores the offset from
> >>> __bpf_call_base to the start of the callee function.
> >>>
> >>> For some architectures, such as powerpc64, it was found that
> >>> this offset may be as large as 64 bits because of which this
> >>> cannot be accomodated in the imm field without truncation.
> >>>
> >>> To resolve this, we additionally make aux->func within each
> >>> bpf_prog associated with the functions to point to the list
> >>> of all function addresses determined by the verifier.
> >>>
> >>> We keep the value assigned to the off field of the bpf_insn
> >>> as a way to index into aux->func and also set aux->func_cnt
> >>> so that this can be used for performing basic upper bound
> >>> checks for the off field.
> >>>
> >>> Signed-off-by: Sandipan Das <sandi...@linux.vnet.ibm.com>
> >>> ---
> >>> v2: Make aux->func point to the list of functions determined
> >>> by the verifier rather than allocating a separate callee
> >>> list for each function.
> >> 
> >> Approach looks good to me; do you know whether s390x JIT would
> >> have similar requirement? I think one limitation that would still
> >> need to be addressed later with such approach would be regarding the
> >> xlated prog dump in bpftool, see 'BPF calls via JIT' in 7105e828c087
> >> ("bpf: allow for correlation of maps and helpers in dump"). Any
> >> ideas for this (potentially if we could use off + imm for calls,
> >> we'd get to 48 bits, but that seems still not be enough as you say)?
> 
> All good points. I'm not really sure how s390x works, so I can't comment 
> on that, but I'm copying Michael Holzheu for his consideration.
> 
> With the existing scheme, 48 bits won't be enough, so we rejected that 
> approach. I can also see how this will be a problem with bpftool, but I 
> haven't looked into it in detail. I wonder if we can annotate the output 
> to indicate the function being referred to?
> 
> > 
> > One other random thought, although I'm not sure how feasible this
> > is for ppc64 JIT to realize ... but idea would be to have something
> > like the below:
> > 
> > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> > index 29ca920..daa7258 100644
> > --- a/kernel/bpf/core.c
> > +++ b/kernel/bpf/core.c
> > @@ -512,6 +512,11 @@ int bpf_get_kallsym(unsigned int symnum, unsigned long 
> > *value, char *type,
> > return ret;
> >  }
> > 
> > +void * __weak bpf_jit_image_alloc(unsigned long size)
> > +{
> > +   return module_alloc(size);
> > +}
> > +
> >  struct bpf_binary_header *
> >  bpf_jit_binary_alloc(unsigned int proglen, u8 **image_ptr,
> >  unsigned int alignment,
> > @@ -525,7 +530,7 @@ bpf_jit_binary_alloc(unsigned int proglen, u8 
> > **image_ptr,
> >  * random section of illegal instructions.
> >  */
> > size = round_up(proglen + sizeof(*hdr) + 128, PAGE_SIZE);
> > -   hdr = module_alloc(size);
> > +   hdr = bpf_jit_image_alloc(size);
> > if (hdr == NULL)
> > return NULL;
> > 
> > And ppc64 JIT could override bpf_jit_image_alloc() in a similar way
> > like some archs would override the module_alloc() helper through a
> > custom implementation, usually via __vmalloc_node_range(), so we
> > could perhaps fit the range for BPF JITed images in a way that they
> > could use the 32bit imm in the end? There are not that many progs
> > loaded typically, so the range could be a bit narrower in such case
> > anyway. (Not sure if this would work out though, but I thought to
> > bring it up.)
> 
> That'd be a good option to consider. I don't think we want to allocate 
> anything from the linear memory range since users could load 
> unprivileged BPF programs and consume a lot of memory that way. I doubt 
> if we can map vmalloc'ed memory into the 0xc0 address range, but I'm not 
> entirely sure.
> 
> Michael,
> Is the above possible? The question is if we can have BPF programs be 
> allocated within 4GB of __bpf_call_base (which is a kernel symbol), so 
> that calls to those programs can be encoded in a 32-bit immediate field 
> in a BPF instruction. As an extension, we may be able to extend it to 
> 48-bits by combining with another BPF instruction field (offset). In 
> either case, the vmalloc'ed address range won't work.
> 
> The alternative is to pass the full 64-bit address of the BPF program in 
> an auxiliary field (as proposed in this patch set) but we need to fix it 
> up for 'bpftool' as well.

Hi Naveen,

Our s390 kernel maintainer Martin Schwidefsky took over
eBPF responsibility for s390 from me.

@Martin: Can you answer Navee's question?

Michael

Re: [PATCH bpf-next 07/13] bpf, s390x: remove obsolete exception handling from div/mod

[Michael Holzheu]

Am Fri, 26 Jan 2018 23:33:42 +0100
schrieb Daniel Borkmann <dan...@iogearbox.net>:

> Since we've changed div/mod exception handling for src_reg in
> eBPF verifier itself, 

Maybe add the commit that introduced that to the patch description?

> remove the leftovers from s390x JIT.
> 
> Signed-off-by: Daniel Borkmann <dan...@iogearbox.net>
> Cc: Michael Holzheu <holz...@linux.vnet.ibm.com>
> ---
>  arch/s390/net/bpf_jit_comp.c | 10 --
>  1 file changed, 10 deletions(-)
> 
> diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
> index e501887..78a19c9 100644
> --- a/arch/s390/net/bpf_jit_comp.c
> +++ b/arch/s390/net/bpf_jit_comp.c
> @@ -610,11 +610,6 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, 
> struct bpf_prog *fp, int i
>   {
>   int rc_reg = BPF_OP(insn->code) == BPF_DIV ? REG_W1 : REG_W0;
> 
> - jit->seen |= SEEN_RET0;
> - /* ltr %src,%src (if src == 0 goto fail) */
> - EMIT2(0x1200, src_reg, src_reg);
> - /* jz  */
> - EMIT4_PCREL(0xa784, jit->ret0_ip - jit->prg);
>   /* lhi %w0,0 */
>   EMIT4_IMM(0xa708, REG_W0, 0);
>   /* lr %w1,%dst */
> @@ -630,11 +625,6 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, 
> struct bpf_prog *fp, int i
>   {
>   int rc_reg = BPF_OP(insn->code) == BPF_DIV ? REG_W1 : REG_W0;
> 
> - jit->seen |= SEEN_RET0;
> - /* ltgr %src,%src (if src == 0 goto fail) */
> - EMIT4(0xb902, src_reg, src_reg);
> - /* jz  */
> - EMIT4_PCREL(0xa784, jit->ret0_ip - jit->prg);
>   /* lghi %w0,0 */
>       EMIT4_IMM(0xa709, REG_W0, 0);
>   /* lgr %w1,%dst */

If the check is done in the verifier now, this looks good to me.

Reviewed-by: Michael Holzheu <holz...@linux.vnet.ibm.com>

Re: [PATCH bpf 1/5] bpf, s390x: do not reload skb pointers in non-skb context

[Michael Holzheu]

Am Thu, 14 Dec 2017 21:07:23 +0100
schrieb Daniel Borkmann :

> The assumption of unconditionally reloading skb pointers on
> BPF helper calls where bpf_helper_changes_pkt_data() holds
> true is wrong. There can be different contexts where the
> BPF helper would enforce a reload such as in case of XDP.
> Here, we do have a struct xdp_buff instead of struct sk_buff
> as context, thus this will access garbage.
> 
> JITs only ever need to deal with cached skb pointer reload
> when ld_abs/ind was seen, therefore guard the reload behind
> SEEN_SKB only. Tested on s390x.

Hello Daniel,

Sorry for the late answer - I have been on vacation up to now.
Thanks for fixing / testing this for s390x.

Michael

Re: [PATCH net 1/2] bpf, s390: fix jit branch offset related to ldimm64

[Michael Holzheu]

Am Fri, 04 Aug 2017 15:52:47 +0200
schrieb Daniel Borkmann <dan...@iogearbox.net>:

> On 08/04/2017 03:44 PM, Michael Holzheu wrote:
> > Am Fri,  4 Aug 2017 14:20:54 +0200
> > schrieb Daniel Borkmann <dan...@iogearbox.net>:
> [...]
> >
> > What about "Cc: sta...@vger.kernel.org"?
> 
> Handled by Dave, see also: Documentation/networking/netdev-FAQ.txt +117

Thanks, good to know! At least I would vote for "Cc: stable".

Michael

Re: [PATCH net 1/2] bpf, s390: fix jit branch offset related to ldimm64

[Michael Holzheu]

Am Fri,  4 Aug 2017 14:20:54 +0200
schrieb Daniel Borkmann <dan...@iogearbox.net>:

> While testing some other work that required JIT modifications, I
> run into test_bpf causing a hang when JIT enabled on s390. The
> problematic test case was the one from ddc665a4bb4b (bpf, arm64:
> fix jit branch offset related to ldimm64), and turns out that we
> do have a similar issue on s390 as well. In bpf_jit_prog() we
> update next instruction address after returning from bpf_jit_insn()
> with an insn_count. bpf_jit_insn() returns either -1 in case of
> error (e.g. unsupported insn), 1 or 2. The latter is only the
> case for ldimm64 due to spanning 2 insns, however, next address
> is only set to i + 1 not taking actual insn_count into account,
> thus fix is to use insn_count instead of 1. bpf_jit_enable in
> mode 2 provides also disasm on s390:
> 
> Before fix:
> 
>   03ff800349b6: a7f40003   brc 15,3ff800349bc ; target
>   03ff800349ba:    unknown
>   03ff800349bc: e3b0f0700024   stg %r11,112(%r15)
>   03ff800349c2: e3e0f0880024   stg %r14,136(%r15)
>   03ff800349c8: 0db0   basr%r11,%r0
>   03ff800349ca: c0ef   llilf   %r14,0
>   03ff800349d0: e320b0360004   lg  %r2,54(%r11)
>   03ff800349d6: e330b03e0004   lg  %r3,62(%r11)
>   03ff800349dc: ec23ffeda065   clgrj   %r2,%r3,10,3ff800349b6 ; jmp
>   03ff800349e2: e3e0b0460004   lg  %r14,70(%r11)
>   03ff800349e8: e3e0b04e0004   lg  %r14,78(%r11)
>   03ff800349ee: b904002e   lgr %r2,%r14
>   03ff800349f2: e3b0f074   lg  %r11,112(%r15)
>   03ff800349f8: e3e0f0880004   lg  %r14,136(%r15)
>   03ff800349fe: 07fe   bcr 15,%r14
> 
> After fix:
> 
>   03ff80ef3db4: a7f40003   brc 15,3ff80ef3dba
>   03ff80ef3db8:    unknown
>   03ff80ef3dba: e3b0f0700024   stg %r11,112(%r15)
>   03ff80ef3dc0: e3e0f0880024   stg %r14,136(%r15)
>   03ff80ef3dc6: 0db0   basr%r11,%r0
>   03ff80ef3dc8: c0ef   llilf   %r14,0
>   03ff80ef3dce: e320b0360004   lg  %r2,54(%r11)
>   03ff80ef3dd4: e330b03e0004   lg  %r3,62(%r11)
>   03ff80ef3dda: ec230006a065   clgrj   %r2,%r3,10,3ff80ef3de6 ; jmp
>   03ff80ef3de0: e3e0b0460004   lg  %r14,70(%r11)
>   03ff80ef3de6: e3e0b04e0004   lg  %r14,78(%r11)  ; target
>   03ff80ef3dec: b904002e   lgr %r2,%r14
>   03ff80ef3df0: e3b0f074   lg  %r11,112(%r15)
>   03ff80ef3df6: e3e0f0880004   lg  %r14,136(%r15)
>   03ff80ef3dfc: 07fe   bcr 15,%r14
> 
> test_bpf.ko suite runs fine after the fix.
> 
> Fixes: 054623105728 ("s390/bpf: Add s390x eBPF JIT compiler backend")
> Signed-off-by: Daniel Borkmann <dan...@iogearbox.net>
> Tested-by: Michael Holzheu <holz...@linux.vnet.ibm.com>

What about "Cc: sta...@vger.kernel.org"?

Michael

[PATCH] bpf/samples: Fix PT_REGS_IP on s390x and use it

[Michael Holzheu]

The files "sampleip_kern.c" and "trace_event_kern.c" directly access
"ctx->regs.ip" which is not available on s390x. Fix this and use the
PT_REGS_IP() macro instead.

Also fix the macro for s390x and use "psw.addr" from "pt_regs".

Reported-by: Zvonko Kosic <zvonko.ko...@de.ibm.com>
Signed-off-by: Michael Holzheu <holz...@linux.vnet.ibm.com>
Acked-by: Alexei Starovoitov <a...@kernel.org>
---
 samples/bpf/bpf_helpers.h  | 2 +-
 samples/bpf/sampleip_kern.c| 2 +-
 samples/bpf/trace_event_kern.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
index 90f44bd..dadd516 100644
--- a/samples/bpf/bpf_helpers.h
+++ b/samples/bpf/bpf_helpers.h
@@ -113,7 +113,7 @@ static int (*bpf_skb_under_cgroup)(void *ctx, void *map, 
int index) =
 #define PT_REGS_FP(x) ((x)->gprs[11]) /* Works only with CONFIG_FRAME_POINTER 
*/
 #define PT_REGS_RC(x) ((x)->gprs[2])
 #define PT_REGS_SP(x) ((x)->gprs[15])
-#define PT_REGS_IP(x) ((x)->ip)
+#define PT_REGS_IP(x) ((x)->psw.addr)
 
 #elif defined(__aarch64__)
 
diff --git a/samples/bpf/sampleip_kern.c b/samples/bpf/sampleip_kern.c
index 774a681..ceabf31 100644
--- a/samples/bpf/sampleip_kern.c
+++ b/samples/bpf/sampleip_kern.c
@@ -25,7 +25,7 @@ int do_sample(struct bpf_perf_event_data *ctx)
u64 ip;
u32 *value, init_val = 1;
 
-   ip = ctx->regs.ip;
+   ip = PT_REGS_IP(>regs);
value = bpf_map_lookup_elem(_map, );
if (value)
*value += 1;
diff --git a/samples/bpf/trace_event_kern.c b/samples/bpf/trace_event_kern.c
index 71a8ed3..41b6115 100644
--- a/samples/bpf/trace_event_kern.c
+++ b/samples/bpf/trace_event_kern.c
@@ -50,7 +50,7 @@ int bpf_prog1(struct bpf_perf_event_data *ctx)
key.userstack = bpf_get_stackid(ctx, , USER_STACKID_FLAGS);
if ((int)key.kernstack < 0 && (int)key.userstack < 0) {
bpf_trace_printk(fmt, sizeof(fmt), cpu, ctx->sample_period,
-ctx->regs.ip);
+PT_REGS_IP(>regs));
return 0;
}
 
-- 
2.8.4

[PATCH net-next v2 2/5] s390/bpf: Fix multiple macro expansions

[Michael Holzheu]

The EMIT6_DISP_LH macro passes the disp parameter to the _EMIT6_DISP_LH
macro. The _EMIT6_DISP_LH macro uses the disp parameter twice:

 unsigned int __disp_h = ((u32)disp)  0xff000;
 unsigned int __disp_l = ((u32)disp)  0x00fff;

The EMIT6_DISP_LH is used several times with EMIT_CONST_U64() as disp
parameter. Therefore always two constants are created per usage of
EMIT6_DISP_LH.

Fix this and add variable _disp to avoid multiple expansions.

* v2: Move _disp to _EMIT6_DISP_LH as suggested by Joe Perches

Fixes: 054623105728 (s390/bpf: Add s390x eBPF JIT compiler backend)
Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 arch/s390/net/bpf_jit_comp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 01ad166..66926ab 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -214,8 +214,9 @@ static inline void reg_set_seen(struct bpf_jit *jit, u32 b1)
 
 #define _EMIT6_DISP_LH(op1, op2, disp) \
 ({ \
-   unsigned int __disp_h = ((u32)disp)  0xff000;  \
-   unsigned int __disp_l = ((u32)disp)  0x00fff;  \
+   u32 _disp = (u32) disp; \
+   unsigned int __disp_h = _disp  0xff000;\
+   unsigned int __disp_l = _disp  0x00fff;\
_EMIT6(op1 | __disp_l, op2 | __disp_h  4);\
 })
 
-- 
2.3.8

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

[PATCH net-next v2 0/5] s390/bpf: recache skb-data/hlen for skb_vlan_push/pop

[Michael Holzheu]

Here the s390 backend for Alexei's patch 4e10df9a60d9 (bpf: introduce
bpf_skb_vlan_push/pop() helpers) plus two bugfixes and two minor
improvements.

The first patch s390/bpf: clear correct BPF accumulator register will
also go upstream via Martin's fixes branch.

* v2: Integrated suggestions from Joe Perches

Michael Holzheu (5):
  s390/bpf: clear correct BPF accumulator register
  s390/bpf: Fix multiple macro expansions
  s390/bpf: increase BPF_SIZE_MAX
  s390/bpf: Only clear A and X for converted BPF programs
  s390/bpf: recache skb-data/hlen for skb_vlan_push/pop

 arch/s390/net/bpf_jit.h  |  5 ++-
 arch/s390/net/bpf_jit_comp.c | 93 +++-
 2 files changed, 53 insertions(+), 45 deletions(-)

-- 
2.3.8

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

[PATCH net-next v2 3/5] s390/bpf: increase BPF_SIZE_MAX

[Michael Holzheu]

Currently we have the restriction that jitted BPF programs can
have a maximum size of one page. The reason is that we use short
displacements for the literal pool.

The 20 bit displacements are available since z990 and BPF requires
z196 as minimum. Therefore we can remove this restriction and use
everywhere 20 bit signed long displacements.

Acked-by: Martin Schwidefsky schwidef...@de.ibm.com
Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 arch/s390/net/bpf_jit_comp.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 66926ab..04af367 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -45,7 +45,7 @@ struct bpf_jit {
int labels[1];  /* Labels for local jumps */
 };
 
-#define BPF_SIZE_MAX   4096/* Max size for program */
+#define BPF_SIZE_MAX   0x7 /* Max size for program (20 bit signed displ) */
 
 #define SEEN_SKB   1   /* skb access */
 #define SEEN_MEM   2   /* use mem[] for temporary storage */
@@ -203,15 +203,6 @@ static inline void reg_set_seen(struct bpf_jit *jit, u32 
b1)
_EMIT6(op1 | __disp, op2);  \
 })
 
-#define EMIT6_DISP(op1, op2, b1, b2, b3, disp) \
-({ \
-   _EMIT6_DISP(op1 | reg(b1, b2)  16 |   \
-   reg_high(b3)  8, op2, disp);  \
-   REG_SET_SEEN(b1);   \
-   REG_SET_SEEN(b2);   \
-   REG_SET_SEEN(b3);   \
-})
-
 #define _EMIT6_DISP_LH(op1, op2, disp) \
 ({ \
u32 _disp = (u32) disp; \
@@ -981,8 +972,8 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, 
struct bpf_prog *fp, int i
REG_SET_SEEN(BPF_REG_5);
jit-seen |= SEEN_FUNC;
/* lg %w1,d(imm)(%l) */
-   EMIT6_DISP(0xe300, 0x0004, REG_W1, REG_0, REG_L,
-  EMIT_CONST_U64(func));
+   EMIT6_DISP_LH(0xe300, 0x0004, REG_W1, REG_0, REG_L,
+ EMIT_CONST_U64(func));
/* basr %r14,%w1 */
EMIT2(0x0d00, REG_14, REG_W1);
/* lgr %b0,%r2: load return value into %b0 */
-- 
2.3.8

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

[PATCH net-next v2 5/5] s390/bpf: recache skb-data/hlen for skb_vlan_push/pop

[Michael Holzheu]

Allow eBPF programs attached to TC qdiscs call skb_vlan_push/pop
via helper functions. These functions may change skb-data/hlen.
This data is cached by s390 JIT to improve performance of ld_abs/ld_ind
instructions. Therefore after a change we have to reload the data.

In case of usage of skb_vlan_push/pop, in the prologue we store
the SKB pointer on the stack and restore it after BPF_JMP_CALL
to skb_vlan_push/pop.

Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 arch/s390/net/bpf_jit.h  |  5 +++-
 arch/s390/net/bpf_jit_comp.c | 55 ++--
 2 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/arch/s390/net/bpf_jit.h b/arch/s390/net/bpf_jit.h
index f6498ee..f010c93 100644
--- a/arch/s390/net/bpf_jit.h
+++ b/arch/s390/net/bpf_jit.h
@@ -36,6 +36,8 @@ extern u8 sk_load_word[], sk_load_half[], sk_load_byte[];
  *   |   BPF stack   | |
  *   |   | |
  *   +---+ |
+ *   | 8 byte skbp   | |
+ * R15+170 - +---+ |
  *   | 8 byte hlen   | |
  * R15+168 - +---+ |
  *   | 4 byte align  | |
@@ -51,11 +53,12 @@ extern u8 sk_load_word[], sk_load_half[], sk_load_byte[];
  * We get 160 bytes stack space from calling function, but only use
  * 12 * 8 byte for old backchain, r15..r6, and tail_call_cnt.
  */
-#define STK_SPACE  (MAX_BPF_STACK + 8 + 4 + 4 + 160)
+#define STK_SPACE  (MAX_BPF_STACK + 8 + 8 + 4 + 4 + 160)
 #define STK_160_UNUSED (160 - 12 * 8)
 #define STK_OFF(STK_SPACE - STK_160_UNUSED)
 #define STK_OFF_TMP160 /* Offset of tmp buffer on stack */
 #define STK_OFF_HLEN   168 /* Offset of SKB header length on stack */
+#define STK_OFF_SKBP   170 /* Offset of SKB pointer on stack */
 
 #define STK_OFF_R6 (160 - 11 * 8)  /* Offset of r6 on stack */
 #define STK_OFF_TCCNT  (160 - 12 * 8)  /* Offset of tail_call_cnt on stack */
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 3dd0163..bbbac6d 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -53,6 +53,7 @@ struct bpf_jit {
 #define SEEN_LITERAL   8   /* code uses literals */
 #define SEEN_FUNC  16  /* calls C functions */
 #define SEEN_TAIL_CALL 32  /* code uses tail calls */
+#define SEEN_SKB_CHANGE64  /* code changes skb data */
 #define SEEN_STACK (SEEN_FUNC | SEEN_MEM | SEEN_SKB)
 
 /*
@@ -382,6 +383,26 @@ static void save_restore_regs(struct bpf_jit *jit, int op)
 }
 
 /*
+ * For SKB access %b1 contains the SKB pointer. For bpf_jit.S
+ * we store the SKB header length on the stack and the SKB data
+ * pointer in REG_SKB_DATA.
+ */
+static void emit_load_skb_data_hlen(struct bpf_jit *jit)
+{
+   /* Header length: llgf %w1,len(%b1) */
+   EMIT6_DISP_LH(0xe300, 0x0016, REG_W1, REG_0, BPF_REG_1,
+ offsetof(struct sk_buff, len));
+   /* s %w1,data_len(%b1) */
+   EMIT4_DISP(0x5b00, REG_W1, BPF_REG_1,
+  offsetof(struct sk_buff, data_len));
+   /* stg %w1,ST_OFF_HLEN(%r0,%r15) */
+   EMIT6_DISP_LH(0xe300, 0x0024, REG_W1, REG_0, REG_15, STK_OFF_HLEN);
+   /* lg %skb_data,data_off(%b1) */
+   EMIT6_DISP_LH(0xe300, 0x0004, REG_SKB_DATA, REG_0,
+ BPF_REG_1, offsetof(struct sk_buff, data));
+}
+
+/*
  * Emit function prologue
  *
  * Save registers and create stack frame if necessary.
@@ -421,25 +442,12 @@ static void bpf_jit_prologue(struct bpf_jit *jit, bool 
is_classic)
EMIT6_DISP_LH(0xe300, 0x0024, REG_W1, REG_0,
  REG_15, 152);
}
-   /*
-* For SKB access %b1 contains the SKB pointer. For bpf_jit.S
-* we store the SKB header length on the stack and the SKB data
-* pointer in REG_SKB_DATA.
-*/
-   if (jit-seen  SEEN_SKB) {
-   /* Header length: llgf %w1,len(%b1) */
-   EMIT6_DISP_LH(0xe300, 0x0016, REG_W1, REG_0, BPF_REG_1,
- offsetof(struct sk_buff, len));
-   /* s %w1,data_len(%b1) */
-   EMIT4_DISP(0x5b00, REG_W1, BPF_REG_1,
-  offsetof(struct sk_buff, data_len));
-   /* stg %w1,ST_OFF_HLEN(%r0,%r15) */
+   if (jit-seen  SEEN_SKB)
+   emit_load_skb_data_hlen(jit);
+   if (jit-seen  SEEN_SKB_CHANGE)
+   /* stg %b1,ST_OFF_SKBP(%r0,%r15) */
EMIT6_DISP_LH(0xe300, 0x0024, REG_W1, REG_0, REG_15,
- STK_OFF_HLEN);
-   /* lg %skb_data,data_off(%b1) */
-   EMIT6_DISP_LH(0xe300, 0x0004, REG_SKB_DATA, REG_0,
- BPF_REG_1, offsetof(struct sk_buff, data));
-   }
+ STK_OFF_SKBP);
/* Clear A (%b0) and X (%b7) registers for converted BPF programs

[PATCH net-next v2 4/5] s390/bpf: Only clear A and X for converted BPF programs

[Michael Holzheu]

Only classic BPF programs that have been converted to eBPF need to clear
the A and X registers. We can check for converted programs with:

  bpf_prog-type == BPF_PROG_TYPE_UNSPEC

So add the check and skip initialization for real eBPF programs.

Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 arch/s390/net/bpf_jit_comp.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 04af367..3dd0163 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -387,7 +387,7 @@ static void save_restore_regs(struct bpf_jit *jit, int op)
  * Save registers and create stack frame if necessary.
  * See stack frame layout desription in bpf_jit.h!
  */
-static void bpf_jit_prologue(struct bpf_jit *jit)
+static void bpf_jit_prologue(struct bpf_jit *jit, bool is_classic)
 {
if (jit-seen  SEEN_TAIL_CALL) {
/* xc STK_OFF_TCCNT(4,%r15),STK_OFF_TCCNT(%r15) */
@@ -440,13 +440,15 @@ static void bpf_jit_prologue(struct bpf_jit *jit)
EMIT6_DISP_LH(0xe300, 0x0004, REG_SKB_DATA, REG_0,
  BPF_REG_1, offsetof(struct sk_buff, data));
}
-   /* BPF compatibility: clear A (%b0) and X (%b7) registers */
-   if (REG_SEEN(BPF_REG_A))
-   /* lghi %ba,0 */
-   EMIT4_IMM(0xa709, BPF_REG_A, 0);
-   if (REG_SEEN(BPF_REG_X))
-   /* lghi %bx,0 */
-   EMIT4_IMM(0xa709, BPF_REG_X, 0);
+   /* Clear A (%b0) and X (%b7) registers for converted BPF programs */
+   if (is_classic) {
+   if (REG_SEEN(BPF_REG_A))
+   /* lghi %ba,0 */
+   EMIT4_IMM(0xa709, BPF_REG_A, 0);
+   if (REG_SEEN(BPF_REG_X))
+   /* lghi %bx,0 */
+   EMIT4_IMM(0xa709, BPF_REG_X, 0);
+   }
 }
 
 /*
@@ -1232,7 +1234,7 @@ static int bpf_jit_prog(struct bpf_jit *jit, struct 
bpf_prog *fp)
jit-lit = jit-lit_start;
jit-prg = 0;
 
-   bpf_jit_prologue(jit);
+   bpf_jit_prologue(jit, fp-type == BPF_PROG_TYPE_UNSPEC);
for (i = 0; i  fp-len; i += insn_count) {
insn_count = bpf_jit_insn(jit, fp, i);
if (insn_count  0)
-- 
2.3.8

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

[PATCH net-next v2 1/5] s390/bpf: clear correct BPF accumulator register

[Michael Holzheu]

Currently we assumed the following BPF to eBPF register mapping:

 - BPF_REG_A - BPF_REG_7
 - BPF_REG_X - BPF_REG_8

Unfortunately this mapping is wrong. The correct mapping is:

 - BPF_REG_A - BPF_REG_0
 - BPF_REG_X - BPF_REG_7

So clear the correct registers and use the BPF_REG_A and BPF_REG_X
macros instead of BPF_REG_0/7.

Fixes: 054623105728 (s390/bpf: Add s390x eBPF JIT compiler backend)
Cc: sta...@vger.kernel.org # 4.0+
Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 arch/s390/net/bpf_jit_comp.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 79c731e..01ad166 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -448,13 +448,13 @@ static void bpf_jit_prologue(struct bpf_jit *jit)
EMIT6_DISP_LH(0xe300, 0x0004, REG_SKB_DATA, REG_0,
  BPF_REG_1, offsetof(struct sk_buff, data));
}
-   /* BPF compatibility: clear A (%b7) and X (%b8) registers */
-   if (REG_SEEN(BPF_REG_7))
-   /* lghi %b7,0 */
-   EMIT4_IMM(0xa709, BPF_REG_7, 0);
-   if (REG_SEEN(BPF_REG_8))
-   /* lghi %b8,0 */
-   EMIT4_IMM(0xa709, BPF_REG_8, 0);
+   /* BPF compatibility: clear A (%b0) and X (%b7) registers */
+   if (REG_SEEN(BPF_REG_A))
+   /* lghi %ba,0 */
+   EMIT4_IMM(0xa709, BPF_REG_A, 0);
+   if (REG_SEEN(BPF_REG_X))
+   /* lghi %bx,0 */
+   EMIT4_IMM(0xa709, BPF_REG_X, 0);
 }
 
 /*
-- 
2.3.8

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

Re: [PATCH net-next v2 2/5] s390/bpf: Fix multiple macro expansions

[Michael Holzheu]

On Wed, 29 Jul 2015 11:31:25 -0700 (PDT)
David Miller da...@davemloft.net wrote:

 
 Please, when updating patches in a series, always resubmit the entire
 series not just the patches you want to change.
 
 Thank you.

Sure, I will do that.

Sorry for the trouble!
Michael

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

Re: [PATCH net-next 2/5] s390/bpf: Fix multiple macro expansions

[Michael Holzheu]

On Tue, 28 Jul 2015 20:13:41 -0700
Joe Perches j...@perches.com wrote:

 On Tue, 2015-07-28 at 16:09 +0200, Michael Holzheu wrote:
  The EMIT6_DISP_LH macro passes the disp parameter to the _EMIT6_DISP_LH
  macro. The _EMIT6_DISP_LH macro uses the disp parameter twice:
  
   unsigned int __disp_h = ((u32)disp)  0xff000;
   unsigned int __disp_l = ((u32)disp)  0x00fff;
  
  The EMIT6_DISP_LH is used several times with EMIT_CONST_U64() as disp
  parameter. Therefore always two constants are created per usage of
  EMIT6_DISP_LH.
  
  Fix this and add variable __disp to avoid multiple expansions.
  
  Fixes: 054623105728 (s390/bpf: Add s390x eBPF JIT compiler backend)
  Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
  ---
   arch/s390/net/bpf_jit_comp.c | 3 ++-
   1 file changed, 2 insertions(+), 1 deletion(-)
  
  diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
  index 01ad166..de0f0bc 100644
  --- a/arch/s390/net/bpf_jit_comp.c
  +++ b/arch/s390/net/bpf_jit_comp.c
  @@ -221,8 +221,9 @@ static inline void reg_set_seen(struct nbpf_jit *jit, 
  u32 b1)
   
   #define EMIT6_DISP_LH(op1, op2, b1, b2, b3, disp)  \
   ({ \
  +   int __disp = (disp);\
  _EMIT6_DISP_LH(op1 | reg(b1, b2)  16 |\
  -  reg_high(b3)  8, op2, disp);   \
  +  reg_high(b3)  8, op2, __disp); \
  REG_SET_SEEN(b1);   \
  REG_SET_SEEN(b2);   \
  REG_SET_SEEN(b3);   \
 
 Perhaps it'd be better to change _EMIT6_DISP_LH

Looks a bit better. I will send the updated patch.

Thanks!
Michael

 ---
 diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
 index 3a15baa..409e206 100644
 --- a/arch/s390/net/bpf_jit_comp.c
 +++ b/arch/s390/net/bpf_jit_comp.c
 @@ -205,8 +205,9 @@ static inline void reg_set_seen(struct bpf_jit *jit,
 u32 b1)
 
  #define _EMIT6_DISP_LH(op1, op2, disp)   \
  ({   \
 - unsigned int __disp_h = ((u32)disp)  0xff000;  \
 - unsigned int __disp_l = ((u32)disp)  0x00fff;  \
 + u32 _disp = (u32)disp;  \
 + unsigned int __disp_h = _disp  0xff000;\
 + unsigned int __disp_l = _disp  0x00fff;\
   _EMIT6(op1 | __disp_l, op2 | __disp_h  4);\
  })
 
 
 

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

[PATCH net-next v2 2/5] s390/bpf: Fix multiple macro expansions

[Michael Holzheu]

The EMIT6_DISP_LH macro passes the disp parameter to the _EMIT6_DISP_LH
macro. The _EMIT6_DISP_LH macro uses the disp parameter twice:

 unsigned int __disp_h = ((u32)disp)  0xff000;
 unsigned int __disp_l = ((u32)disp)  0x00fff;

The EMIT6_DISP_LH is used several times with EMIT_CONST_U64() as disp
parameter. Therefore always two constants are created per usage of
EMIT6_DISP_LH.

Fix this and add variable _disp to avoid multiple expansions.

* v2: Move _disp to _EMIT6_DISP_LH as suggested by Joe Perches

Fixes: 054623105728 (s390/bpf: Add s390x eBPF JIT compiler backend)
Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 arch/s390/net/bpf_jit_comp.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 01ad166..66926ab 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -214,8 +214,9 @@ static inline void reg_set_seen(struct bpf_jit *jit, u32 b1)
 
 #define _EMIT6_DISP_LH(op1, op2, disp) \
 ({ \
-   unsigned int __disp_h = ((u32)disp)  0xff000;  \
-   unsigned int __disp_l = ((u32)disp)  0x00fff;  \
+   u32 _disp = (u32) disp; \
+   unsigned int __disp_h = _disp  0xff000;\
+   unsigned int __disp_l = _disp  0x00fff;\
_EMIT6(op1 | __disp_l, op2 | __disp_h  4);\
 })
 
-- 
2.3.8

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

[PATCH net-next 5/5] s390/bpf: recache skb-data/hlen for skb_vlan_push/pop

[Michael Holzheu]

Allow eBPF programs attached to TC qdiscs call skb_vlan_push/pop
via helper functions. These functions may change skb-data/hlen.
This data is cached by s390 JIT to improve performance of ld_abs/ld_ind
instructions. Therefore after a change we have to reload the data.

In case of usage of skb_vlan_push/pop, in the prologue we store
the SKB pointer on the stack and restore it after BPF_JMP_CALL
to skb_vlan_push/pop.

Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 arch/s390/net/bpf_jit.h  |  5 +++-
 arch/s390/net/bpf_jit_comp.c | 55 ++--
 2 files changed, 37 insertions(+), 23 deletions(-)

diff --git a/arch/s390/net/bpf_jit.h b/arch/s390/net/bpf_jit.h
index f6498ee..f010c93 100644
--- a/arch/s390/net/bpf_jit.h
+++ b/arch/s390/net/bpf_jit.h
@@ -36,6 +36,8 @@ extern u8 sk_load_word[], sk_load_half[], sk_load_byte[];
  *   |   BPF stack   | |
  *   |   | |
  *   +---+ |
+ *   | 8 byte skbp   | |
+ * R15+170 - +---+ |
  *   | 8 byte hlen   | |
  * R15+168 - +---+ |
  *   | 4 byte align  | |
@@ -51,11 +53,12 @@ extern u8 sk_load_word[], sk_load_half[], sk_load_byte[];
  * We get 160 bytes stack space from calling function, but only use
  * 12 * 8 byte for old backchain, r15..r6, and tail_call_cnt.
  */
-#define STK_SPACE  (MAX_BPF_STACK + 8 + 4 + 4 + 160)
+#define STK_SPACE  (MAX_BPF_STACK + 8 + 8 + 4 + 4 + 160)
 #define STK_160_UNUSED (160 - 12 * 8)
 #define STK_OFF(STK_SPACE - STK_160_UNUSED)
 #define STK_OFF_TMP160 /* Offset of tmp buffer on stack */
 #define STK_OFF_HLEN   168 /* Offset of SKB header length on stack */
+#define STK_OFF_SKBP   170 /* Offset of SKB pointer on stack */
 
 #define STK_OFF_R6 (160 - 11 * 8)  /* Offset of r6 on stack */
 #define STK_OFF_TCCNT  (160 - 12 * 8)  /* Offset of tail_call_cnt on stack */
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index a025ddc..ece46d4 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -53,6 +53,7 @@ struct bpf_jit {
 #define SEEN_LITERAL   8   /* code uses literals */
 #define SEEN_FUNC  16  /* calls C functions */
 #define SEEN_TAIL_CALL 32  /* code uses tail calls */
+#define SEEN_SKB_CHANGE64  /* code changes skb data */
 #define SEEN_STACK (SEEN_FUNC | SEEN_MEM | SEEN_SKB)
 
 /*
@@ -382,6 +383,26 @@ static void save_restore_regs(struct bpf_jit *jit, int op)
 }
 
 /*
+ * For SKB access %b1 contains the SKB pointer. For bpf_jit.S
+ * we store the SKB header length on the stack and the SKB data
+ * pointer in REG_SKB_DATA.
+ */
+static void emit_load_skb_data_hlen(struct bpf_jit *jit)
+{
+   /* Header length: llgf %w1,len(%b1) */
+   EMIT6_DISP_LH(0xe300, 0x0016, REG_W1, REG_0, BPF_REG_1,
+ offsetof(struct sk_buff, len));
+   /* s %w1,data_len(%b1) */
+   EMIT4_DISP(0x5b00, REG_W1, BPF_REG_1,
+  offsetof(struct sk_buff, data_len));
+   /* stg %w1,ST_OFF_HLEN(%r0,%r15) */
+   EMIT6_DISP_LH(0xe300, 0x0024, REG_W1, REG_0, REG_15, STK_OFF_HLEN);
+   /* lg %skb_data,data_off(%b1) */
+   EMIT6_DISP_LH(0xe300, 0x0004, REG_SKB_DATA, REG_0,
+ BPF_REG_1, offsetof(struct sk_buff, data));
+}
+
+/*
  * Emit function prologue
  *
  * Save registers and create stack frame if necessary.
@@ -421,25 +442,12 @@ static void bpf_jit_prologue(struct bpf_jit *jit, bool 
is_classic)
EMIT6_DISP_LH(0xe300, 0x0024, REG_W1, REG_0,
  REG_15, 152);
}
-   /*
-* For SKB access %b1 contains the SKB pointer. For bpf_jit.S
-* we store the SKB header length on the stack and the SKB data
-* pointer in REG_SKB_DATA.
-*/
-   if (jit-seen  SEEN_SKB) {
-   /* Header length: llgf %w1,len(%b1) */
-   EMIT6_DISP_LH(0xe300, 0x0016, REG_W1, REG_0, BPF_REG_1,
- offsetof(struct sk_buff, len));
-   /* s %w1,data_len(%b1) */
-   EMIT4_DISP(0x5b00, REG_W1, BPF_REG_1,
-  offsetof(struct sk_buff, data_len));
-   /* stg %w1,ST_OFF_HLEN(%r0,%r15) */
+   if (jit-seen  SEEN_SKB)
+   emit_load_skb_data_hlen(jit);
+   if (jit-seen  SEEN_SKB_CHANGE)
+   /* stg %b1,ST_OFF_SKBP(%r0,%r15) */
EMIT6_DISP_LH(0xe300, 0x0024, REG_W1, REG_0, REG_15,
- STK_OFF_HLEN);
-   /* lg %skb_data,data_off(%b1) */
-   EMIT6_DISP_LH(0xe300, 0x0004, REG_SKB_DATA, REG_0,
- BPF_REG_1, offsetof(struct sk_buff, data));
-   }
+ STK_OFF_SKBP);
/* Clear A (%b0) and X (%b7) registers for converted BPF programs

[PATCH net-next 2/5] s390/bpf: Fix multiple macro expansions

[Michael Holzheu]

The EMIT6_DISP_LH macro passes the disp parameter to the _EMIT6_DISP_LH
macro. The _EMIT6_DISP_LH macro uses the disp parameter twice:

 unsigned int __disp_h = ((u32)disp)  0xff000;
 unsigned int __disp_l = ((u32)disp)  0x00fff;

The EMIT6_DISP_LH is used several times with EMIT_CONST_U64() as disp
parameter. Therefore always two constants are created per usage of
EMIT6_DISP_LH.

Fix this and add variable __disp to avoid multiple expansions.

Fixes: 054623105728 (s390/bpf: Add s390x eBPF JIT compiler backend)
Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 arch/s390/net/bpf_jit_comp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 01ad166..de0f0bc 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -221,8 +221,9 @@ static inline void reg_set_seen(struct bpf_jit *jit, u32 b1)
 
 #define EMIT6_DISP_LH(op1, op2, b1, b2, b3, disp)  \
 ({ \
+   int __disp = (disp);\
_EMIT6_DISP_LH(op1 | reg(b1, b2)  16 |\
-  reg_high(b3)  8, op2, disp);   \
+  reg_high(b3)  8, op2, __disp); \
REG_SET_SEEN(b1);   \
REG_SET_SEEN(b2);   \
REG_SET_SEEN(b3);   \
-- 
2.3.8

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

[PATCH net-next 1/5] s390/bpf: clear correct BPF accumulator register

[Michael Holzheu]

Currently we assumed the following BPF to eBPF register mapping:

 - BPF_REG_A - BPF_REG_7
 - BPF_REG_X - BPF_REG_8

Unfortunately this mapping is wrong. The correct mapping is:

 - BPF_REG_A - BPF_REG_0
 - BPF_REG_X - BPF_REG_7

So clear the correct registers and use the BPF_REG_A and BPF_REG_X
macros instead of BPF_REG_0/7.

Fixes: 054623105728 (s390/bpf: Add s390x eBPF JIT compiler backend)
Cc: sta...@vger.kernel.org # 4.0+
Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 arch/s390/net/bpf_jit_comp.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 79c731e..01ad166 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -448,13 +448,13 @@ static void bpf_jit_prologue(struct bpf_jit *jit)
EMIT6_DISP_LH(0xe300, 0x0004, REG_SKB_DATA, REG_0,
  BPF_REG_1, offsetof(struct sk_buff, data));
}
-   /* BPF compatibility: clear A (%b7) and X (%b8) registers */
-   if (REG_SEEN(BPF_REG_7))
-   /* lghi %b7,0 */
-   EMIT4_IMM(0xa709, BPF_REG_7, 0);
-   if (REG_SEEN(BPF_REG_8))
-   /* lghi %b8,0 */
-   EMIT4_IMM(0xa709, BPF_REG_8, 0);
+   /* BPF compatibility: clear A (%b0) and X (%b7) registers */
+   if (REG_SEEN(BPF_REG_A))
+   /* lghi %ba,0 */
+   EMIT4_IMM(0xa709, BPF_REG_A, 0);
+   if (REG_SEEN(BPF_REG_X))
+   /* lghi %bx,0 */
+   EMIT4_IMM(0xa709, BPF_REG_X, 0);
 }
 
 /*
-- 
2.3.8

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

[PATCH net-next 3/5] s390/bpf: increase BPF_SIZE_MAX

[Michael Holzheu]

Currently we have the restriction that jitted BPF programs can
have a maximum size of one page. The reason is that we use short
displacements for the literal pool.

The 20 bit displacements are available since z990 and BPF requires
z196 as minimum. Therefore we can remove this restriction and use
everywhere 20 bit signed long displacements.

Acked-by: Martin Schwidefsky schwidef...@de.ibm.com
Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 arch/s390/net/bpf_jit_comp.c | 15 +++
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index de0f0bc..bea5cfc 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -45,7 +45,7 @@ struct bpf_jit {
int labels[1];  /* Labels for local jumps */
 };
 
-#define BPF_SIZE_MAX   4096/* Max size for program */
+#define BPF_SIZE_MAX   0x7 /* Max size for program (20 bit signed displ) */
 
 #define SEEN_SKB   1   /* skb access */
 #define SEEN_MEM   2   /* use mem[] for temporary storage */
@@ -203,15 +203,6 @@ static inline void reg_set_seen(struct bpf_jit *jit, u32 
b1)
_EMIT6(op1 | __disp, op2);  \
 })
 
-#define EMIT6_DISP(op1, op2, b1, b2, b3, disp) \
-({ \
-   _EMIT6_DISP(op1 | reg(b1, b2)  16 |   \
-   reg_high(b3)  8, op2, disp);  \
-   REG_SET_SEEN(b1);   \
-   REG_SET_SEEN(b2);   \
-   REG_SET_SEEN(b3);   \
-})
-
 #define _EMIT6_DISP_LH(op1, op2, disp) \
 ({ \
unsigned int __disp_h = ((u32)disp)  0xff000;  \
@@ -981,8 +972,8 @@ static noinline int bpf_jit_insn(struct bpf_jit *jit, 
struct bpf_prog *fp, int i
REG_SET_SEEN(BPF_REG_5);
jit-seen |= SEEN_FUNC;
/* lg %w1,d(imm)(%l) */
-   EMIT6_DISP(0xe300, 0x0004, REG_W1, REG_0, REG_L,
-  EMIT_CONST_U64(func));
+   EMIT6_DISP_LH(0xe300, 0x0004, REG_W1, REG_0, REG_L,
+ EMIT_CONST_U64(func));
/* basr %r14,%w1 */
EMIT2(0x0d00, REG_14, REG_W1);
/* lgr %b0,%r2: load return value into %b0 */
-- 
2.3.8

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

[PATCH net-next 0/5] s390/bpf: recache skb-data/hlen for skb_vlan_push/pop

[Michael Holzheu]

Hi Dave,

Here the s390 backend for Alexei's patch 4e10df9a60d9 (bpf: introduce
bpf_skb_vlan_push/pop() helpers) plus two bugfixes and two minor
improvements.

The first patch s390/bpf: clear correct BPF accumulator register will
also go upstream via Martin's fixes branch.

Ok for you?

Regards,
Michael

Michael Holzheu (5):
  s390/bpf: clear correct BPF accumulator register
  s390/bpf: Fix multiple macro expansions
  s390/bpf: increase BPF_SIZE_MAX
  s390/bpf: Only clear A and X for converted BPF programs
  s390/bpf: recache skb-data/hlen for skb_vlan_push/pop

 arch/s390/net/bpf_jit.h  |  5 ++-
 arch/s390/net/bpf_jit_comp.c | 91 +++-
 2 files changed, 52 insertions(+), 44 deletions(-)

-- 
2.3.8

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

[PATCH net-next 4/5] s390/bpf: Only clear A and X for converted BPF programs

[Michael Holzheu]

Only classic BPF programs that have been converted to eBPF need to clear
the A and X registers. We can check for converted programs with:

  bpf_prog-type == BPF_PROG_TYPE_UNSPEC

So add the check and skip initialization for real eBPF programs.

Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 arch/s390/net/bpf_jit_comp.c | 20 +++-
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index bea5cfc..a025ddc 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -387,7 +387,7 @@ static void save_restore_regs(struct bpf_jit *jit, int op)
  * Save registers and create stack frame if necessary.
  * See stack frame layout desription in bpf_jit.h!
  */
-static void bpf_jit_prologue(struct bpf_jit *jit)
+static void bpf_jit_prologue(struct bpf_jit *jit, bool is_classic)
 {
if (jit-seen  SEEN_TAIL_CALL) {
/* xc STK_OFF_TCCNT(4,%r15),STK_OFF_TCCNT(%r15) */
@@ -440,13 +440,15 @@ static void bpf_jit_prologue(struct bpf_jit *jit)
EMIT6_DISP_LH(0xe300, 0x0004, REG_SKB_DATA, REG_0,
  BPF_REG_1, offsetof(struct sk_buff, data));
}
-   /* BPF compatibility: clear A (%b0) and X (%b7) registers */
-   if (REG_SEEN(BPF_REG_A))
-   /* lghi %ba,0 */
-   EMIT4_IMM(0xa709, BPF_REG_A, 0);
-   if (REG_SEEN(BPF_REG_X))
-   /* lghi %bx,0 */
-   EMIT4_IMM(0xa709, BPF_REG_X, 0);
+   /* Clear A (%b0) and X (%b7) registers for converted BPF programs */
+   if (is_classic) {
+   if (REG_SEEN(BPF_REG_A))
+   /* lghi %ba,0 */
+   EMIT4_IMM(0xa709, BPF_REG_A, 0);
+   if (REG_SEEN(BPF_REG_X))
+   /* lghi %bx,0 */
+   EMIT4_IMM(0xa709, BPF_REG_X, 0);
+   }
 }
 
 /*
@@ -1232,7 +1234,7 @@ static int bpf_jit_prog(struct bpf_jit *jit, struct 
bpf_prog *fp)
jit-lit = jit-lit_start;
jit-prg = 0;
 
-   bpf_jit_prologue(jit);
+   bpf_jit_prologue(jit, fp-type == BPF_PROG_TYPE_UNSPEC);
for (i = 0; i  fp-len; i += insn_count) {
insn_count = bpf_jit_insn(jit, fp, i);
if (insn_count  0)
-- 
2.3.8

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

[PATCH] samples: bpf: enable trace samples for s390x

[Michael Holzheu]

The trace bpf samples do not compile on s390x because they use x86
specific fields from the pt_regs structure.

Fix this and access the fields via new PT_REGS macros.

Signed-off-by: Michael Holzheu holz...@linux.vnet.ibm.com
---
 samples/bpf/bpf_helpers.h  | 25 +
 samples/bpf/tracex1_kern.c |  2 +-
 samples/bpf/tracex2_kern.c |  6 +++---
 samples/bpf/tracex3_kern.c |  4 ++--
 samples/bpf/tracex4_kern.c |  6 +++---
 samples/bpf/tracex5_kern.c |  6 +++---
 6 files changed, 37 insertions(+), 12 deletions(-)

diff --git a/samples/bpf/bpf_helpers.h b/samples/bpf/bpf_helpers.h
index bdf1c16..c77c872 100644
--- a/samples/bpf/bpf_helpers.h
+++ b/samples/bpf/bpf_helpers.h
@@ -60,4 +60,29 @@ static int (*bpf_l3_csum_replace)(void *ctx, int off, int 
from, int to, int flag
 static int (*bpf_l4_csum_replace)(void *ctx, int off, int from, int to, int 
flags) =
(void *) BPF_FUNC_l4_csum_replace;
 
+#if defined(__x86_64__)
+
+#define PT_REGS_PARM1(x) ((x)-di)
+#define PT_REGS_PARM2(x) ((x)-si)
+#define PT_REGS_PARM3(x) ((x)-dx)
+#define PT_REGS_PARM4(x) ((x)-cx)
+#define PT_REGS_PARM5(x) ((x)-r8)
+#define PT_REGS_RET(x) ((x)-sp)
+#define PT_REGS_FP(x) ((x)-bp)
+#define PT_REGS_RC(x) ((x)-ax)
+#define PT_REGS_SP(x) ((x)-sp)
+
+#elif defined(__s390x__)
+
+#define PT_REGS_PARM1(x) ((x)-gprs[2])
+#define PT_REGS_PARM2(x) ((x)-gprs[3])
+#define PT_REGS_PARM3(x) ((x)-gprs[4])
+#define PT_REGS_PARM4(x) ((x)-gprs[5])
+#define PT_REGS_PARM5(x) ((x)-gprs[6])
+#define PT_REGS_RET(x) ((x)-gprs[14])
+#define PT_REGS_FP(x) ((x)-gprs[11]) /* Works only with CONFIG_FRAME_POINTER 
*/
+#define PT_REGS_RC(x) ((x)-gprs[2])
+#define PT_REGS_SP(x) ((x)-gprs[15])
+
+#endif
 #endif
diff --git a/samples/bpf/tracex1_kern.c b/samples/bpf/tracex1_kern.c
index 3162046..3f450a8 100644
--- a/samples/bpf/tracex1_kern.c
+++ b/samples/bpf/tracex1_kern.c
@@ -29,7 +29,7 @@ int bpf_prog1(struct pt_regs *ctx)
int len;
 
/* non-portable! works for the given kernel only */
-   skb = (struct sk_buff *) ctx-di;
+   skb = (struct sk_buff *) PT_REGS_PARM1(ctx);
 
dev = _(skb-dev);
 
diff --git a/samples/bpf/tracex2_kern.c b/samples/bpf/tracex2_kern.c
index dc50f4f..b32367c 100644
--- a/samples/bpf/tracex2_kern.c
+++ b/samples/bpf/tracex2_kern.c
@@ -27,10 +27,10 @@ int bpf_prog2(struct pt_regs *ctx)
long init_val = 1;
long *value;
 
-   /* x64 specific: read ip of kfree_skb caller.
+   /* x64/s390x specific: read ip of kfree_skb caller.
 * non-portable version of __builtin_return_address(0)
 */
-   bpf_probe_read(loc, sizeof(loc), (void *)ctx-sp);
+   bpf_probe_read(loc, sizeof(loc), (void *)PT_REGS_RET(ctx));
 
value = bpf_map_lookup_elem(my_map, loc);
if (value)
@@ -79,7 +79,7 @@ struct bpf_map_def SEC(maps) my_hist_map = {
 SEC(kprobe/sys_write)
 int bpf_prog3(struct pt_regs *ctx)
 {
-   long write_size = ctx-dx; /* arg3 */
+   long write_size = PT_REGS_PARM3(ctx);
long init_val = 1;
long *value;
struct hist_key key = {};
diff --git a/samples/bpf/tracex3_kern.c b/samples/bpf/tracex3_kern.c
index 255ff27..bf337fb 100644
--- a/samples/bpf/tracex3_kern.c
+++ b/samples/bpf/tracex3_kern.c
@@ -23,7 +23,7 @@ struct bpf_map_def SEC(maps) my_map = {
 SEC(kprobe/blk_mq_start_request)
 int bpf_prog1(struct pt_regs *ctx)
 {
-   long rq = ctx-di;
+   long rq = PT_REGS_PARM1(ctx);
u64 val = bpf_ktime_get_ns();
 
bpf_map_update_elem(my_map, rq, val, BPF_ANY);
@@ -51,7 +51,7 @@ struct bpf_map_def SEC(maps) lat_map = {
 SEC(kprobe/blk_update_request)
 int bpf_prog2(struct pt_regs *ctx)
 {
-   long rq = ctx-di;
+   long rq = PT_REGS_PARM1(ctx);
u64 *value, l, base;
u32 index;
 
diff --git a/samples/bpf/tracex4_kern.c b/samples/bpf/tracex4_kern.c
index 126b805..ac46714 100644
--- a/samples/bpf/tracex4_kern.c
+++ b/samples/bpf/tracex4_kern.c
@@ -27,7 +27,7 @@ struct bpf_map_def SEC(maps) my_map = {
 SEC(kprobe/kmem_cache_free)
 int bpf_prog1(struct pt_regs *ctx)
 {
-   long ptr = ctx-si;
+   long ptr = PT_REGS_PARM2(ctx);
 
bpf_map_delete_elem(my_map, ptr);
return 0;
@@ -36,11 +36,11 @@ int bpf_prog1(struct pt_regs *ctx)
 SEC(kretprobe/kmem_cache_alloc_node)
 int bpf_prog2(struct pt_regs *ctx)
 {
-   long ptr = ctx-ax;
+   long ptr = PT_REGS_RC(ctx);
long ip = 0;
 
/* get ip address of kmem_cache_alloc_node() caller */
-   bpf_probe_read(ip, sizeof(ip), (void *)(ctx-bp + sizeof(ip)));
+   bpf_probe_read(ip, sizeof(ip), (void *)(PT_REGS_FP(ctx) + sizeof(ip)));
 
struct pair v = {
.val = bpf_ktime_get_ns(),
diff --git a/samples/bpf/tracex5_kern.c b/samples/bpf/tracex5_kern.c
index b71fe07..b3f4295 100644
--- a/samples/bpf/tracex5_kern.c
+++ b/samples/bpf/tracex5_kern.c
@@ -24,7 +24,7 @@ int bpf_prog1(struct pt_regs *ctx)
 {
struct seccomp_data sd = {};
 
-   bpf_probe_read(sd, sizeof(sd