lkml.org 
[lkml]   [2021]   [Mar]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH bpf-next v2] bpf: Simplify expression for identify bpf mem type
From
Date
On 3/18/21 7:36 AM, Jianlin Lv wrote:
> Added BPF_LD_ST_SIZE_MASK macro as mask of size modifier that help to
> reduce the evaluation of expressions in if statements,
> and remove BPF_SIZE_MASK in netronome driver.
>
> Signed-off-by: Jianlin Lv <Jianlin.Lv@arm.com>
> ---
> v2: Move the bpf_LD_ST_SIZE_MASK macro definition to include/linux/bpf.h
> ---
> drivers/net/ethernet/netronome/nfp/bpf/main.h | 8 +++-----
> include/linux/bpf.h | 1 +
> kernel/bpf/verifier.c | 12 ++++--------
> 3 files changed, 8 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/net/ethernet/netronome/nfp/bpf/main.h b/drivers/net/ethernet/netronome/nfp/bpf/main.h
> index d0e17eebddd9..e90981e69763 100644
> --- a/drivers/net/ethernet/netronome/nfp/bpf/main.h
> +++ b/drivers/net/ethernet/netronome/nfp/bpf/main.h
> @@ -346,8 +346,6 @@ struct nfp_insn_meta {
> struct list_head l;
> };
>
> -#define BPF_SIZE_MASK 0x18
> -
> static inline u8 mbpf_class(const struct nfp_insn_meta *meta)
> {
> return BPF_CLASS(meta->insn.code);
> @@ -375,7 +373,7 @@ static inline bool is_mbpf_alu(const struct nfp_insn_meta *meta)
>
> static inline bool is_mbpf_load(const struct nfp_insn_meta *meta)
> {
> - return (meta->insn.code & ~BPF_SIZE_MASK) == (BPF_LDX | BPF_MEM);
> + return (meta->insn.code & ~BPF_LD_ST_SIZE_MASK) == (BPF_LDX | BPF_MEM);
> }
>
> static inline bool is_mbpf_jmp32(const struct nfp_insn_meta *meta)
> @@ -395,7 +393,7 @@ static inline bool is_mbpf_jmp(const struct nfp_insn_meta *meta)
>
> static inline bool is_mbpf_store(const struct nfp_insn_meta *meta)
> {
> - return (meta->insn.code & ~BPF_SIZE_MASK) == (BPF_STX | BPF_MEM);
> + return (meta->insn.code & ~BPF_LD_ST_SIZE_MASK) == (BPF_STX | BPF_MEM);
> }
>
> static inline bool is_mbpf_load_pkt(const struct nfp_insn_meta *meta)
> @@ -430,7 +428,7 @@ static inline bool is_mbpf_classic_store_pkt(const struct nfp_insn_meta *meta)
>
> static inline bool is_mbpf_atomic(const struct nfp_insn_meta *meta)
> {
> - return (meta->insn.code & ~BPF_SIZE_MASK) == (BPF_STX | BPF_ATOMIC);
> + return (meta->insn.code & ~BPF_LD_ST_SIZE_MASK) == (BPF_STX | BPF_ATOMIC);
> }
>
> static inline bool is_mbpf_mul(const struct nfp_insn_meta *meta)
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index a25730eaa148..e85924719c65 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -995,6 +995,7 @@ struct bpf_array {
> BPF_F_RDONLY_PROG | \
> BPF_F_WRONLY | \
> BPF_F_WRONLY_PROG)
> +#define BPF_LD_ST_SIZE_MASK 0x18 /* mask of size modifier */
>
> #define BPF_MAP_CAN_READ BIT(0)
> #define BPF_MAP_CAN_WRITE BIT(1)
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index f9096b049cd6..29fdfdb8abfa 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -11384,15 +11384,11 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> for (i = 0; i < insn_cnt; i++, insn++) {
> bpf_convert_ctx_access_t convert_ctx_access;
>
> - if (insn->code == (BPF_LDX | BPF_MEM | BPF_B) ||
> - insn->code == (BPF_LDX | BPF_MEM | BPF_H) ||
> - insn->code == (BPF_LDX | BPF_MEM | BPF_W) ||
> - insn->code == (BPF_LDX | BPF_MEM | BPF_DW))
> + /* opcode: BPF_MEM | <size> | BPF_LDX */
> + if ((insn->code & ~BPF_LD_ST_SIZE_MASK) == (BPF_LDX | BPF_MEM))
> type = BPF_READ;
> - else if (insn->code == (BPF_STX | BPF_MEM | BPF_B) ||
> - insn->code == (BPF_STX | BPF_MEM | BPF_H) ||
> - insn->code == (BPF_STX | BPF_MEM | BPF_W) ||
> - insn->code == (BPF_STX | BPF_MEM | BPF_DW))
> + /* opcode: BPF_MEM | <size> | BPF_STX */
> + else if ((insn->code & ~BPF_LD_ST_SIZE_MASK) == (BPF_STX | BPF_MEM))
> type = BPF_WRITE;
> else
> continue;
>

To me this cleanup makes the code harder to read, in particular on verfier side,
I don't think it's worth it, especially given it's not in (highly) performance
critical code.

Thanks,
Daniel

\
 
 \ /
  Last update: 2021-03-18 16:59    [W:0.178 / U:0.320 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site