On Wed, Aug 17, 2022 at 12:22 PM Martin KaFai Lau <kafai@xxxxxx> wrote:
>
> On Tue, Aug 16, 2022 at 01:12:12PM -0700, Stanislav Fomichev wrote:
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index a627a02cf8ab..c302d2de073a 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -1948,6 +1948,10 @@ struct bpf_prog *bpf_prog_by_id(u32 id);
> > struct bpf_link *bpf_link_by_id(u32 id);
> >
> > const struct bpf_func_proto *bpf_base_func_proto(enum bpf_func_id func_id);
> > +const struct bpf_func_proto *
> > +cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog);
> > +const struct bpf_func_proto *
> > +cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog);
> > void bpf_task_storage_free(struct task_struct *task);
> > bool bpf_prog_has_kfunc_call(const struct bpf_prog *prog);
> > const struct btf_func_model *
> > @@ -2154,6 +2158,18 @@ bpf_base_func_proto(enum bpf_func_id func_id)
> > return NULL;
> > }
> >
> > +static inline const struct bpf_func_proto *
> > +cgroup_common_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > +{
> > + return NULL;
> > +}
> > +
> > +static inline const struct bpf_func_proto *
> > +cgroup_current_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> > +{
> > + return NULL;
> > +}
> > +
> There two new functions are implemented in cgroup.c which only compiles with
> CONFIG_CGROUP_BPF. I think the change in bpf.h here should be done in
> bpf-cgroup.h instead. Otherwise, the changes in filter.c in the next patch will
> have issue in resolving these functions when CONFIG_CGROUP_BPF is not set.
SG. Let me try. I think I have a config for CONFIG_CGROUPS=y
CONFIG_CGROUP_BPF=n, but maybe I don't, let me check.
> > -#define BPF_STRTOX_BASE_MASK 0x1F
> > -
> > -static int __bpf_strtoull(const char *buf, size_t buf_len, u64 flags,
> > - unsigned long long *res, bool *is_negative)
> > -{
> > - unsigned int base = flags & BPF_STRTOX_BASE_MASK;
> > - const char *cur_buf = buf;
> > - size_t cur_len = buf_len;
> > - unsigned int consumed;
> > - size_t val_len;
> > - char str[64];
> > -
> > - if (!buf || !buf_len || !res || !is_negative)
> > - return -EINVAL;
> > -
> > - if (base != 0 && base != 8 && base != 10 && base != 16)
> > - return -EINVAL;
> > -
> > - if (flags & ~BPF_STRTOX_BASE_MASK)
> > - return -EINVAL;
> > -
> > - while (cur_buf < buf + buf_len && isspace(*cur_buf))
> > - ++cur_buf;
> > -
> > - *is_negative = (cur_buf < buf + buf_len && *cur_buf == '-');
> > - if (*is_negative)
> > - ++cur_buf;
> > -
> > - consumed = cur_buf - buf;
> > - cur_len -= consumed;
> > - if (!cur_len)
> > - return -EINVAL;
> > -
> > - cur_len = min(cur_len, sizeof(str) - 1);
> > - memcpy(str, cur_buf, cur_len);
> > - str[cur_len] = '\0';
> > - cur_buf = str;
> > -
> > - cur_buf = _parse_integer_fixup_radix(cur_buf, &base);
> > - val_len = _parse_integer(cur_buf, base, res);
> > -
> > - if (val_len & KSTRTOX_OVERFLOW)
> > - return -ERANGE;
> > -
> > - if (val_len == 0)
> > - return -EINVAL;
> > -
> > - cur_buf += val_len;
> > - consumed += cur_buf - str;
> > -
> > - return consumed;
> > -}
> > -
> > -static int __bpf_strtoll(const char *buf, size_t buf_len, u64 flags,
> > - long long *res)
> > -{
> > - unsigned long long _res;
> > - bool is_negative;
> > - int err;
> > -
> > - err = __bpf_strtoull(buf, buf_len, flags, &_res, &is_negative);
> > - if (err < 0)
> > - return err;
> > - if (is_negative) {
> > - if ((long long)-_res > 0)
> > - return -ERANGE;
> > - *res = -_res;
> > - } else {
> > - if ((long long)_res < 0)
> > - return -ERANGE;
> > - *res = _res;
> > - }
> > - return err;
> > -}
> > -
> > -BPF_CALL_4(bpf_strtol, const char *, buf, size_t, buf_len, u64, flags,
> > - long *, res)
> > -{
> > - long long _res;
> > - int err;
> > -
> > - err = __bpf_strtoll(buf, buf_len, flags, &_res);
> > - if (err < 0)
> > - return err;
> > - if (_res != (long)_res)
> > - return -ERANGE;
> > - *res = _res;
> > - return err;
> > -}
> > -
> > -const struct bpf_func_proto bpf_strtol_proto = {
> > - .func = bpf_strtol,
> > - .gpl_only = false,
> > - .ret_type = RET_INTEGER,
> > - .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY,
> > - .arg2_type = ARG_CONST_SIZE,
> > - .arg3_type = ARG_ANYTHING,
> > - .arg4_type = ARG_PTR_TO_LONG,
> > -};
> > -
> > -BPF_CALL_4(bpf_strtoul, const char *, buf, size_t, buf_len, u64, flags,
> > - unsigned long *, res)
> > -{
> > - unsigned long long _res;
> > - bool is_negative;
> > - int err;
> > -
> > - err = __bpf_strtoull(buf, buf_len, flags, &_res, &is_negative);
> > - if (err < 0)
> > - return err;
> > - if (is_negative)
> > - return -EINVAL;
> > - if (_res != (unsigned long)_res)
> > - return -ERANGE;
> > - *res = _res;
> > - return err;
> > -}
> > -
> > -const struct bpf_func_proto bpf_strtoul_proto = {
> This should be useful in general other than cgroup bpf.
> It may end up moving back to helpers.c soon.
> How about take this chance to add it to bpf_base_func_proto()
> which already has another string helper bpf_strncmp_proto?
Sure, will do! Thanks!
> > - .func = bpf_strtoul,
> > - .gpl_only = false,
> > - .ret_type = RET_INTEGER,
> > - .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY,
> > - .arg2_type = ARG_CONST_SIZE,
> > - .arg3_type = ARG_ANYTHING,
> > - .arg4_type = ARG_PTR_TO_LONG,
> > -};
> > -#endif
