On Sun, Jun 24, 2018 at 8:54 PM, Jakub Kicinski
<jakub.kicin...@netronome.com> wrote:
> From: Jiong Wang <jiong.w...@netronome.com>
>
> NFP doesn't have integer divide instruction, this patch use reciprocal
> algorithm (the basic one, reciprocal_div) to emulate it.
>
> For each u32 divide, we would need 11 instructions to finish the operation.
>
>   7 (for multiplication) + 4 (various ALUs) = 11
>
> Given NFP only supports multiplication no bigger than u32, we'd require
> divisor and dividend no bigger than that as well.
>
> Also eBPF doesn't support signed divide and has enforced this on C language
> level by failing compilation. However LLVM assembler hasn't enforced this,
> so it is possible for negative constant to leak in as a BPF_K operand
> through assembly code, we reject such cases as well.
>
> Signed-off-by: Jiong Wang <jiong.w...@netronome.com>
> Reviewed-by: Jakub Kicinski <jakub.kicin...@netronome.com>

Acked-by: Song Liu <songliubrav...@fb.com>

> ---
>  drivers/net/ethernet/netronome/nfp/bpf/jit.c  | 58 ++++++++++++++++++-
>  drivers/net/ethernet/netronome/nfp/bpf/main.h |  5 ++
>  .../net/ethernet/netronome/nfp/bpf/verifier.c | 31 ++++++++++
>  3 files changed, 93 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/netronome/nfp/bpf/jit.c 
> b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
> index 7d7061d93358..d732b6cfc356 100644
> --- a/drivers/net/ethernet/netronome/nfp/bpf/jit.c
> +++ b/drivers/net/ethernet/netronome/nfp/bpf/jit.c
> @@ -34,10 +34,11 @@
>  #define pr_fmt(fmt)    "NFP net bpf: " fmt
>
>  #include <linux/bug.h>
> -#include <linux/kernel.h>
>  #include <linux/bpf.h>
>  #include <linux/filter.h>
> +#include <linux/kernel.h>
>  #include <linux/pkt_cls.h>
> +#include <linux/reciprocal_div.h>
>  #include <linux/unistd.h>
>
>  #include "main.h"
> @@ -1493,6 +1494,32 @@ wrp_mul(struct nfp_prog *nfp_prog, struct 
> nfp_insn_meta *meta,
>         return 0;
>  }
>
> +static int wrp_div_imm(struct nfp_prog *nfp_prog, u8 dst, u64 imm)
> +{
> +       swreg tmp_both = imm_both(nfp_prog), dst_both = reg_both(dst);
> +       swreg dst_a = reg_a(dst), dst_b = reg_a(dst);
> +       struct reciprocal_value rvalue;
> +       swreg tmp_b = imm_b(nfp_prog);
> +       swreg magic;
> +
> +       if (imm > U32_MAX) {
> +               wrp_immed(nfp_prog, dst_both, 0);
> +               return 0;
> +       }
> +
> +       rvalue = reciprocal_value(imm);
> +       magic = re_load_imm_any(nfp_prog, rvalue.m, imm_b(nfp_prog));
> +       wrp_mul_u32(nfp_prog, tmp_both, tmp_both, dst_a, magic, true);
> +       emit_alu(nfp_prog, dst_both, dst_a, ALU_OP_SUB, tmp_b);
> +       emit_shf(nfp_prog, dst_both, reg_none(), SHF_OP_NONE, dst_b,
> +                SHF_SC_R_SHF, rvalue.sh1);
> +       emit_alu(nfp_prog, dst_both, dst_a, ALU_OP_ADD, tmp_b);
> +       emit_shf(nfp_prog, dst_both, reg_none(), SHF_OP_NONE, dst_b,
> +                SHF_SC_R_SHF, rvalue.sh2);
> +
> +       return 0;
> +}
> +
>  static int adjust_head(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
>  {
>         swreg tmp = imm_a(nfp_prog), tmp_len = imm_b(nfp_prog);
> @@ -1807,6 +1834,21 @@ static int mul_imm64(struct nfp_prog *nfp_prog, struct 
> nfp_insn_meta *meta)
>         return wrp_mul(nfp_prog, meta, true, false);
>  }
>
> +static int div_imm64(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
> +{
> +       const struct bpf_insn *insn = &meta->insn;
> +
> +       return wrp_div_imm(nfp_prog, insn->dst_reg * 2, insn->imm);
> +}
> +
> +static int div_reg64(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
> +{
> +       /* NOTE: verifier hook has rejected cases for which verifier doesn't
> +        * know whether the source operand is constant or not.
> +        */
> +       return wrp_div_imm(nfp_prog, meta->insn.dst_reg * 2, meta->umin_src);
> +}
> +
>  static int neg_reg64(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
>  {
>         const struct bpf_insn *insn = &meta->insn;
> @@ -2230,6 +2272,16 @@ static int mul_imm(struct nfp_prog *nfp_prog, struct 
> nfp_insn_meta *meta)
>         return wrp_mul(nfp_prog, meta, false, false);
>  }
>
> +static int div_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
> +{
> +       return div_reg64(nfp_prog, meta);
> +}
> +
> +static int div_imm(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
> +{
> +       return div_imm64(nfp_prog, meta);
> +}
> +
>  static int neg_reg(struct nfp_prog *nfp_prog, struct nfp_insn_meta *meta)
>  {
>         u8 dst = meta->insn.dst_reg * 2;
> @@ -2983,6 +3035,8 @@ static const instr_cb_t instr_cb[256] = {
>         [BPF_ALU64 | BPF_SUB | BPF_K] = sub_imm64,
>         [BPF_ALU64 | BPF_MUL | BPF_X] = mul_reg64,
>         [BPF_ALU64 | BPF_MUL | BPF_K] = mul_imm64,
> +       [BPF_ALU64 | BPF_DIV | BPF_X] = div_reg64,
> +       [BPF_ALU64 | BPF_DIV | BPF_K] = div_imm64,
>         [BPF_ALU64 | BPF_NEG] =         neg_reg64,
>         [BPF_ALU64 | BPF_LSH | BPF_X] = shl_reg64,
>         [BPF_ALU64 | BPF_LSH | BPF_K] = shl_imm64,
> @@ -3004,6 +3058,8 @@ static const instr_cb_t instr_cb[256] = {
>         [BPF_ALU | BPF_SUB | BPF_K] =   sub_imm,
>         [BPF_ALU | BPF_MUL | BPF_X] =   mul_reg,
>         [BPF_ALU | BPF_MUL | BPF_K] =   mul_imm,
> +       [BPF_ALU | BPF_DIV | BPF_X] =   div_reg,
> +       [BPF_ALU | BPF_DIV | BPF_K] =   div_imm,
>         [BPF_ALU | BPF_NEG] =           neg_reg,
>         [BPF_ALU | BPF_LSH | BPF_K] =   shl_imm,
>         [BPF_ALU | BPF_END | BPF_X] =   end_reg32,
> diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h 
> b/drivers/net/ethernet/netronome/nfp/bpf/main.h
> index c10079b1a312..9845c1a2d4c2 100644
> --- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
> +++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
> @@ -399,6 +399,11 @@ static inline bool is_mbpf_mul(const struct 
> nfp_insn_meta *meta)
>         return is_mbpf_alu(meta) && mbpf_op(meta) == BPF_MUL;
>  }
>
> +static inline bool is_mbpf_div(const struct nfp_insn_meta *meta)
> +{
> +       return is_mbpf_alu(meta) && mbpf_op(meta) == BPF_DIV;
> +}
> +
>  /**
>   * struct nfp_prog - nfp BPF program
>   * @bpf: backpointer to the bpf app priv structure
> diff --git a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c 
> b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
> index 30d4f1580693..f0f07e988c46 100644
> --- a/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
> +++ b/drivers/net/ethernet/netronome/nfp/bpf/verifier.c
> @@ -558,6 +558,37 @@ nfp_bpf_check_alu(struct nfp_prog *nfp_prog, struct 
> nfp_insn_meta *meta,
>                 }
>         }
>
> +       /* NFP doesn't have divide instructions, we support divide by constant
> +        * through reciprocal multiplication. Given NFP support multiplication
> +        * no bigger than u32, we'd require divisor and dividend no bigger 
> than
> +        * that as well.
> +        *
> +        * Also eBPF doesn't support signed divide and has enforced this on C
> +        * language level by failing compilation. However LLVM assembler 
> hasn't
> +        * enforced this, so it is possible for negative constant to leak in 
> as
> +        * a BPF_K operand through assembly code, we reject such cases as 
> well.
> +        */
> +       if (is_mbpf_div(meta)) {
> +               if (meta->umax_dst > U32_MAX) {
> +                       pr_vlog(env, "divisor is not within u32 value 
> range\n");
> +                       return -EINVAL;
> +               }
> +               if (mbpf_src(meta) == BPF_X) {
> +                       if (meta->umin_src != meta->umax_src) {
> +                               pr_vlog(env, "dividend is not constant\n");
> +                               return -EINVAL;
> +                       }
> +                       if (meta->umax_src > U32_MAX) {
> +                               pr_vlog(env, "dividend is not within u32 
> value range\n");
> +                               return -EINVAL;
> +                       }
> +               }
> +               if (mbpf_src(meta) == BPF_K && meta->insn.imm < 0) {
> +                       pr_vlog(env, "divide by negative constant is not 
> supported\n");
> +                       return -EINVAL;
> +               }
> +       }
> +
>         return 0;
>  }
>
> --
> 2.17.1
>

Reply via email to