| Message ID | 20200121195414.3757563-1-kafai@fb.com |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: tcp: Add bpf_cubic example | expand |
On 1/21/20 8:54 PM, Martin KaFai Lau wrote: > This patch adds a helper to read the 64bit jiffies. It will be used > in a later patch to implement the bpf_cubic.c. > > The helper is inlined. "gen_inline" is added to "struct bpf_func_proto" > to do that. This helper is available to CAP_SYS_ADMIN. > > Signed-off-by: Martin KaFai Lau <kafai@fb.com> > --- > include/linux/bpf.h | 2 ++ > include/uapi/linux/bpf.h | 9 ++++++++- > kernel/bpf/core.c | 1 + > kernel/bpf/helpers.c | 27 +++++++++++++++++++++++++++ > kernel/bpf/verifier.c | 18 ++++++++++++++++++ > net/core/filter.c | 2 ++ > 6 files changed, 58 insertions(+), 1 deletion(-) > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > index 8e3b8f4ad183..3d85ef44b247 100644 > --- a/include/linux/bpf.h > +++ b/include/linux/bpf.h > @@ -258,6 +258,7 @@ enum bpf_return_type { > */ > struct bpf_func_proto { > u64 (*func)(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5); > + u32 (*gen_inline)(struct bpf_insn *insn_buf); > bool gpl_only; > bool pkt_access; > enum bpf_return_type ret_type; > @@ -1406,6 +1407,7 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto; > extern const struct bpf_func_proto bpf_strtol_proto; > extern const struct bpf_func_proto bpf_strtoul_proto; > extern const struct bpf_func_proto bpf_tcp_sock_proto; > +extern const struct bpf_func_proto bpf_jiffies64_proto; > > /* Shared helpers among cBPF and eBPF. */ > void bpf_user_rnd_init_once(void); > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > index 033d90a2282d..d17c6bcd50cd 100644 > --- a/include/uapi/linux/bpf.h > +++ b/include/uapi/linux/bpf.h > @@ -2885,6 +2885,12 @@ union bpf_attr { > * **-EPERM** if no permission to send the *sig*. > * > * **-EAGAIN** if bpf program can try again. > + * > + * u64 bpf_jiffies64(void) > + * Description > + * Obtain the 64bit jiffies > + * Return > + * The 64 bit jiffies > */ > #define __BPF_FUNC_MAPPER(FN) \ > FN(unspec), \ > @@ -3004,7 +3010,8 @@ union bpf_attr { > FN(probe_read_user_str), \ > FN(probe_read_kernel_str), \ > FN(tcp_send_ack), \ > - FN(send_signal_thread), > + FN(send_signal_thread), \ > + FN(jiffies64), > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper > * function eBPF program intends to call > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > index 29d47aae0dd1..973a20d49749 100644 > --- a/kernel/bpf/core.c > +++ b/kernel/bpf/core.c > @@ -2137,6 +2137,7 @@ const struct bpf_func_proto bpf_map_pop_elem_proto __weak; > const struct bpf_func_proto bpf_map_peek_elem_proto __weak; > const struct bpf_func_proto bpf_spin_lock_proto __weak; > const struct bpf_func_proto bpf_spin_unlock_proto __weak; > +const struct bpf_func_proto bpf_jiffies64_proto __weak; > > const struct bpf_func_proto bpf_get_prandom_u32_proto __weak; > const struct bpf_func_proto bpf_get_smp_processor_id_proto __weak; > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > index cada974c9f4e..b241cfd350c4 100644 > --- a/kernel/bpf/helpers.c > +++ b/kernel/bpf/helpers.c > @@ -11,6 +11,7 @@ > #include <linux/uidgid.h> > #include <linux/filter.h> > #include <linux/ctype.h> > +#include <linux/jiffies.h> > > #include "../../lib/kstrtox.h" > > @@ -312,6 +313,32 @@ void copy_map_value_locked(struct bpf_map *map, void *dst, void *src, > preempt_enable(); > } > > +static u32 bpf_jiffies64_gen_inline(struct bpf_insn *insn_buf) > +{ > + struct bpf_insn *insn = insn_buf; > +#if BITS_PER_LONG == 64 > + struct bpf_insn ld_jiffies_addr[2] = { > + BPF_LD_IMM64(BPF_REG_0, (unsigned long)&jiffies), > + }; > + > + BUILD_BUG_ON(sizeof(jiffies) != sizeof(unsigned long)); > + > + *insn++ = ld_jiffies_addr[0]; > + *insn++ = ld_jiffies_addr[1]; > + *insn++ = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 0); > +#else > + *insn++ = BPF_EMIT_CALL(BPF_CAST_CALL(get_jiffies_64)); > +#endif > + > + return insn - insn_buf; > +} > + > +const struct bpf_func_proto bpf_jiffies64_proto = { > + .gen_inline = bpf_jiffies64_gen_inline, > + .gpl_only = false, > + .ret_type = RET_INTEGER, > +}; > + > #ifdef CONFIG_CGROUPS > BPF_CALL_0(bpf_get_current_cgroup_id) > { > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index ca17dccc17ba..91818aad2f80 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -9447,6 +9447,24 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env) > > patch_call_imm: > fn = env->ops->get_func_proto(insn->imm, env->prog); > + if (fn->gen_inline) { > + cnt = fn->gen_inline(insn_buf); > + if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf)) { > + verbose(env, "bpf verifier is misconfigured\n"); > + return -EINVAL; > + } > + > + new_prog = bpf_patch_insn_data(env, i + delta, > + insn_buf, cnt); > + if (!new_prog) > + return -ENOMEM; > + > + delta += cnt - 1; > + env->prog = prog = new_prog; > + insn = new_prog->insnsi + i + delta; > + continue; > + } We do most of the inlining in fixup_bpf_calls() directly today, why breaking with that here with a special callback? (Agree it could probably be refactored in general, but such rework should also take the other helpers into account.) > /* all functions that have prototype and verifier allowed > * programs to call them, must be real in-kernel functions > */ > diff --git a/net/core/filter.c b/net/core/filter.c > index 17de6747d9e3..4bf3e4aa8a7a 100644 > --- a/net/core/filter.c > +++ b/net/core/filter.c > @@ -5923,6 +5923,8 @@ bpf_base_func_proto(enum bpf_func_id func_id) > return &bpf_spin_unlock_proto; > case BPF_FUNC_trace_printk: > return bpf_get_trace_printk_proto(); > + case BPF_FUNC_jiffies64: > + return &bpf_jiffies64_proto; > default: > return NULL; > } >
On Tue, Jan 21, 2020 at 11:41:06PM +0100, Daniel Borkmann wrote: > On 1/21/20 8:54 PM, Martin KaFai Lau wrote: > > This patch adds a helper to read the 64bit jiffies. It will be used > > in a later patch to implement the bpf_cubic.c. > > > > The helper is inlined. "gen_inline" is added to "struct bpf_func_proto" > > to do that. This helper is available to CAP_SYS_ADMIN. > > > > Signed-off-by: Martin KaFai Lau <kafai@fb.com> > > --- > > include/linux/bpf.h | 2 ++ > > include/uapi/linux/bpf.h | 9 ++++++++- > > kernel/bpf/core.c | 1 + > > kernel/bpf/helpers.c | 27 +++++++++++++++++++++++++++ > > kernel/bpf/verifier.c | 18 ++++++++++++++++++ > > net/core/filter.c | 2 ++ > > 6 files changed, 58 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/bpf.h b/include/linux/bpf.h > > index 8e3b8f4ad183..3d85ef44b247 100644 > > --- a/include/linux/bpf.h > > +++ b/include/linux/bpf.h > > @@ -258,6 +258,7 @@ enum bpf_return_type { > > */ > > struct bpf_func_proto { > > u64 (*func)(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5); > > + u32 (*gen_inline)(struct bpf_insn *insn_buf); > > bool gpl_only; > > bool pkt_access; > > enum bpf_return_type ret_type; > > @@ -1406,6 +1407,7 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto; > > extern const struct bpf_func_proto bpf_strtol_proto; > > extern const struct bpf_func_proto bpf_strtoul_proto; > > extern const struct bpf_func_proto bpf_tcp_sock_proto; > > +extern const struct bpf_func_proto bpf_jiffies64_proto; > > /* Shared helpers among cBPF and eBPF. */ > > void bpf_user_rnd_init_once(void); > > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h > > index 033d90a2282d..d17c6bcd50cd 100644 > > --- a/include/uapi/linux/bpf.h > > +++ b/include/uapi/linux/bpf.h > > @@ -2885,6 +2885,12 @@ union bpf_attr { > > * **-EPERM** if no permission to send the *sig*. > > * > > * **-EAGAIN** if bpf program can try again. > > + * > > + * u64 bpf_jiffies64(void) > > + * Description > > + * Obtain the 64bit jiffies > > + * Return > > + * The 64 bit jiffies > > */ > > #define __BPF_FUNC_MAPPER(FN) \ > > FN(unspec), \ > > @@ -3004,7 +3010,8 @@ union bpf_attr { > > FN(probe_read_user_str), \ > > FN(probe_read_kernel_str), \ > > FN(tcp_send_ack), \ > > - FN(send_signal_thread), > > + FN(send_signal_thread), \ > > + FN(jiffies64), > > /* integer value in 'imm' field of BPF_CALL instruction selects which helper > > * function eBPF program intends to call > > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > > index 29d47aae0dd1..973a20d49749 100644 > > --- a/kernel/bpf/core.c > > +++ b/kernel/bpf/core.c > > @@ -2137,6 +2137,7 @@ const struct bpf_func_proto bpf_map_pop_elem_proto __weak; > > const struct bpf_func_proto bpf_map_peek_elem_proto __weak; > > const struct bpf_func_proto bpf_spin_lock_proto __weak; > > const struct bpf_func_proto bpf_spin_unlock_proto __weak; > > +const struct bpf_func_proto bpf_jiffies64_proto __weak; > > const struct bpf_func_proto bpf_get_prandom_u32_proto __weak; > > const struct bpf_func_proto bpf_get_smp_processor_id_proto __weak; > > diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c > > index cada974c9f4e..b241cfd350c4 100644 > > --- a/kernel/bpf/helpers.c > > +++ b/kernel/bpf/helpers.c > > @@ -11,6 +11,7 @@ > > #include <linux/uidgid.h> > > #include <linux/filter.h> > > #include <linux/ctype.h> > > +#include <linux/jiffies.h> > > #include "../../lib/kstrtox.h" > > @@ -312,6 +313,32 @@ void copy_map_value_locked(struct bpf_map *map, void *dst, void *src, > > preempt_enable(); > > } > > +static u32 bpf_jiffies64_gen_inline(struct bpf_insn *insn_buf) > > +{ > > + struct bpf_insn *insn = insn_buf; > > +#if BITS_PER_LONG == 64 > > + struct bpf_insn ld_jiffies_addr[2] = { > > + BPF_LD_IMM64(BPF_REG_0, (unsigned long)&jiffies), > > + }; > > + > > + BUILD_BUG_ON(sizeof(jiffies) != sizeof(unsigned long)); > > + > > + *insn++ = ld_jiffies_addr[0]; > > + *insn++ = ld_jiffies_addr[1]; > > + *insn++ = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 0); > > +#else > > + *insn++ = BPF_EMIT_CALL(BPF_CAST_CALL(get_jiffies_64)); > > +#endif > > + > > + return insn - insn_buf; > > +} > > + > > +const struct bpf_func_proto bpf_jiffies64_proto = { > > + .gen_inline = bpf_jiffies64_gen_inline, > > + .gpl_only = false, > > + .ret_type = RET_INTEGER, > > +}; > > + > > #ifdef CONFIG_CGROUPS > > BPF_CALL_0(bpf_get_current_cgroup_id) > > { > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > index ca17dccc17ba..91818aad2f80 100644 > > --- a/kernel/bpf/verifier.c > > +++ b/kernel/bpf/verifier.c > > @@ -9447,6 +9447,24 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env) > > patch_call_imm: > > fn = env->ops->get_func_proto(insn->imm, env->prog); > > + if (fn->gen_inline) { > > + cnt = fn->gen_inline(insn_buf); > > + if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf)) { > > + verbose(env, "bpf verifier is misconfigured\n"); > > + return -EINVAL; > > + } > > + > > + new_prog = bpf_patch_insn_data(env, i + delta, > > + insn_buf, cnt); > > + if (!new_prog) > > + return -ENOMEM; > > + > > + delta += cnt - 1; > > + env->prog = prog = new_prog; > > + insn = new_prog->insnsi + i + delta; > > + continue; > > + } > > We do most of the inlining in fixup_bpf_calls() directly today, why > breaking with that here with a special callback? (Agree it could probably > be refactored in general, but such rework should also take the other > helpers into account.) Thanks for the review. I will remove bpf_jiffies64_gen_inline() and move those logic into fixup_bpf_calls().
diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 8e3b8f4ad183..3d85ef44b247 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -258,6 +258,7 @@ enum bpf_return_type { */ struct bpf_func_proto { u64 (*func)(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5); + u32 (*gen_inline)(struct bpf_insn *insn_buf); bool gpl_only; bool pkt_access; enum bpf_return_type ret_type; @@ -1406,6 +1407,7 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto; extern const struct bpf_func_proto bpf_strtol_proto; extern const struct bpf_func_proto bpf_strtoul_proto; extern const struct bpf_func_proto bpf_tcp_sock_proto; +extern const struct bpf_func_proto bpf_jiffies64_proto; /* Shared helpers among cBPF and eBPF. */ void bpf_user_rnd_init_once(void); diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h index 033d90a2282d..d17c6bcd50cd 100644 --- a/include/uapi/linux/bpf.h +++ b/include/uapi/linux/bpf.h @@ -2885,6 +2885,12 @@ union bpf_attr { * **-EPERM** if no permission to send the *sig*. * * **-EAGAIN** if bpf program can try again. + * + * u64 bpf_jiffies64(void) + * Description + * Obtain the 64bit jiffies + * Return + * The 64 bit jiffies */ #define __BPF_FUNC_MAPPER(FN) \ FN(unspec), \ @@ -3004,7 +3010,8 @@ union bpf_attr { FN(probe_read_user_str), \ FN(probe_read_kernel_str), \ FN(tcp_send_ack), \ - FN(send_signal_thread), + FN(send_signal_thread), \ + FN(jiffies64), /* integer value in 'imm' field of BPF_CALL instruction selects which helper * function eBPF program intends to call diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 29d47aae0dd1..973a20d49749 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2137,6 +2137,7 @@ const struct bpf_func_proto bpf_map_pop_elem_proto __weak; const struct bpf_func_proto bpf_map_peek_elem_proto __weak; const struct bpf_func_proto bpf_spin_lock_proto __weak; const struct bpf_func_proto bpf_spin_unlock_proto __weak; +const struct bpf_func_proto bpf_jiffies64_proto __weak; const struct bpf_func_proto bpf_get_prandom_u32_proto __weak; const struct bpf_func_proto bpf_get_smp_processor_id_proto __weak; diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index cada974c9f4e..b241cfd350c4 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -11,6 +11,7 @@ #include <linux/uidgid.h> #include <linux/filter.h> #include <linux/ctype.h> +#include <linux/jiffies.h> #include "../../lib/kstrtox.h" @@ -312,6 +313,32 @@ void copy_map_value_locked(struct bpf_map *map, void *dst, void *src, preempt_enable(); } +static u32 bpf_jiffies64_gen_inline(struct bpf_insn *insn_buf) +{ + struct bpf_insn *insn = insn_buf; +#if BITS_PER_LONG == 64 + struct bpf_insn ld_jiffies_addr[2] = { + BPF_LD_IMM64(BPF_REG_0, (unsigned long)&jiffies), + }; + + BUILD_BUG_ON(sizeof(jiffies) != sizeof(unsigned long)); + + *insn++ = ld_jiffies_addr[0]; + *insn++ = ld_jiffies_addr[1]; + *insn++ = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 0); +#else + *insn++ = BPF_EMIT_CALL(BPF_CAST_CALL(get_jiffies_64)); +#endif + + return insn - insn_buf; +} + +const struct bpf_func_proto bpf_jiffies64_proto = { + .gen_inline = bpf_jiffies64_gen_inline, + .gpl_only = false, + .ret_type = RET_INTEGER, +}; + #ifdef CONFIG_CGROUPS BPF_CALL_0(bpf_get_current_cgroup_id) { diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index ca17dccc17ba..91818aad2f80 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -9447,6 +9447,24 @@ static int fixup_bpf_calls(struct bpf_verifier_env *env) patch_call_imm: fn = env->ops->get_func_proto(insn->imm, env->prog); + if (fn->gen_inline) { + cnt = fn->gen_inline(insn_buf); + if (cnt == 0 || cnt >= ARRAY_SIZE(insn_buf)) { + verbose(env, "bpf verifier is misconfigured\n"); + return -EINVAL; + } + + new_prog = bpf_patch_insn_data(env, i + delta, + insn_buf, cnt); + if (!new_prog) + return -ENOMEM; + + delta += cnt - 1; + env->prog = prog = new_prog; + insn = new_prog->insnsi + i + delta; + continue; + } + /* all functions that have prototype and verifier allowed * programs to call them, must be real in-kernel functions */ diff --git a/net/core/filter.c b/net/core/filter.c index 17de6747d9e3..4bf3e4aa8a7a 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -5923,6 +5923,8 @@ bpf_base_func_proto(enum bpf_func_id func_id) return &bpf_spin_unlock_proto; case BPF_FUNC_trace_printk: return bpf_get_trace_printk_proto(); + case BPF_FUNC_jiffies64: + return &bpf_jiffies64_proto; default: return NULL; }
This patch adds a helper to read the 64bit jiffies. It will be used in a later patch to implement the bpf_cubic.c. The helper is inlined. "gen_inline" is added to "struct bpf_func_proto" to do that. This helper is available to CAP_SYS_ADMIN. Signed-off-by: Martin KaFai Lau <kafai@fb.com> --- include/linux/bpf.h | 2 ++ include/uapi/linux/bpf.h | 9 ++++++++- kernel/bpf/core.c | 1 + kernel/bpf/helpers.c | 27 +++++++++++++++++++++++++++ kernel/bpf/verifier.c | 18 ++++++++++++++++++ net/core/filter.c | 2 ++ 6 files changed, 58 insertions(+), 1 deletion(-)