On Thu, Jan 5, 2023 at 4:57 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:
>
> On 1/4/23 1:59 PM, Stanislav Fomichev wrote:
> > -int bpf_prog_dev_bound_init(struct bpf_prog *prog, union bpf_attr *attr)
> > +static int __bpf_prog_dev_bound_init(struct bpf_prog *prog, struct net_device *netdev)
> > {
> > struct bpf_offload_netdev *ondev;
> > struct bpf_prog_offload *offload;
> > int err;
> >
> > - if (attr->prog_type != BPF_PROG_TYPE_SCHED_CLS &&
> > - attr->prog_type != BPF_PROG_TYPE_XDP)
> > - return -EINVAL;
> > -
> > - if (attr->prog_flags & ~BPF_F_XDP_DEV_BOUND_ONLY)
> > + if (!netdev)
>
> nit. I think this check is also unnecessary.
>
> [ ... ]
>
> > diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> > index 191a4312f4b7..2ec2f53eeff6 100644
> > --- a/kernel/bpf/syscall.c
> > +++ b/kernel/bpf/syscall.c
> > @@ -2605,6 +2605,13 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr)
> > goto free_prog_sec;
> > }
> >
> > + if (type == BPF_PROG_TYPE_EXT && dst_prog &&
> > + bpf_prog_is_dev_bound(dst_prog->aux)) {
> > + err = bpf_prog_dev_bound_inherit(prog, dst_prog);
> > + if (err)
> > + goto free_prog_sec;
> > + }
> > +
> > /* find program type: socket_filter vs tracing_filter */
> > err = find_prog_type(type, prog);
> > if (err < 0)
> > @@ -3021,6 +3028,12 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
> > goto out_put_prog;
> > }
> >
> > + if (bpf_prog_is_dev_bound(prog->aux) &&
> > + !bpf_prog_dev_bound_match(prog, tgt_prog)) {
> > + err = -EINVAL;
> > + goto out_put_prog;
> > + }
>
> This looks good. One minor comment...
True, fill drop, thanks!
> > +
> > key = bpf_trampoline_compute_key(tgt_prog, NULL, btf_id);
> > }
> >
> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 0d0a49a2c5fd..8c1b1259f30b 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -16531,11 +16531,6 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
> > if (tgt_prog) {
> > struct bpf_prog_aux *aux = tgt_prog->aux;
> >
> > - if (bpf_prog_is_dev_bound(tgt_prog->aux)) {
> > - bpf_log(log, "Replacing device-bound programs not supported\n");
> > - return -EINVAL;
> > - }
>
> ... can the above "bpf_prog_is_dev_bound(prog->aux) &&..." check in syscall.c be
> done in the bpf_check_attach_target() here? Mentally that seems to belong more
> to bpf_check_attach_target().
Sure, I don't see why not, will try!
> > -
> > for (i = 0; i < aux->func_info_cnt; i++)
> > if (aux->func_info[i].type_id == btf_id) {
> > subprog = i;
>
