Re: [RFC v2 4/4] ublk_drv: add ebpf support

[Xiaoguang Wang <xiaoguang.wang@linux.alibaba.com>]

hi,

> On Wed, Feb 22, 2023 at 5:29 AM Xiaoguang Wang
> <xiaoguang.wang@linux.alibaba.com> wrote:
>> Currenly only one bpf_ublk_queue_sqe() ebpf is added, ublksrv target
>> can use this helper to write ebpf prog to support ublk kernel & usersapce
>> zero copy, please see ublksrv test codes for more info.
>>
>>
>> +const struct bpf_func_proto ublk_bpf_queue_sqe_proto = {
>> +       .func = bpf_ublk_queue_sqe,
>> +       .gpl_only = false,
>> +       .ret_type = RET_INTEGER,
>> +       .arg1_type = ARG_ANYTHING,
>> +       .arg2_type = ARG_ANYTHING,
>> +       .arg3_type = ARG_ANYTHING,
>> +       .arg4_type = ARG_ANYTHING,
>> +};
> You know that the above is unsafe, right?
Yes, I know it's not safe, will improve it in next version.

>
>> +
>>  static const struct bpf_func_proto *
>>  ublk_bpf_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
>>  {
>> -       return bpf_base_func_proto(func_id);
>> +       switch (func_id) {
>> +       case BPF_FUNC_ublk_queue_sqe:
>> +               return &ublk_bpf_queue_sqe_proto;
>> +       default:
>> +               return bpf_base_func_proto(func_id);
>> +       }
>>  }
>>
>>  static bool ublk_bpf_is_valid_access(int off, int size,
>> @@ -200,6 +252,23 @@ static bool ublk_bpf_is_valid_access(int off, int size,
>>                         const struct bpf_prog *prog,
>>                         struct bpf_insn_access_aux *info)
>>  {
>> +       if (off < 0 || off >= sizeof(struct ublk_bpf_ctx))
>> +               return false;
>> +       if (off % size != 0)
>> +               return false;
>> +
>> +       switch (off) {
>> +       case offsetof(struct ublk_bpf_ctx, q_id):
>> +               return size == sizeof_field(struct ublk_bpf_ctx, q_id);
>> +       case offsetof(struct ublk_bpf_ctx, tag):
>> +               return size == sizeof_field(struct ublk_bpf_ctx, tag);
>> +       case offsetof(struct ublk_bpf_ctx, op):
>> +               return size == sizeof_field(struct ublk_bpf_ctx, op);
>> +       case offsetof(struct ublk_bpf_ctx, nr_sectors):
>> +               return size == sizeof_field(struct ublk_bpf_ctx, nr_sectors);
>> +       case offsetof(struct ublk_bpf_ctx, start_sector):
>> +               return size == sizeof_field(struct ublk_bpf_ctx, start_sector);
>> +       }
>>         return false;
> We don't introduce stable 'ctx' anymore.
> Please see how hid-bpf is doing things.
ok, will learn it, thanks.

Regards,
Xiaoguang Wang