Re: [PATCH bpf-next v4 06/15] bpf: Support consuming XDP HW metadata from fext programs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Stanislav Fomichev <sdf@xxxxxxxxxx> writes:

> On Wed, Dec 14, 2022 at 2:41 AM Toke Høiland-Jørgensen <toke@xxxxxxxxxx> wrote:
>>
>> Martin KaFai Lau <martin.lau@xxxxxxxxx> writes:
>>
>> > On 12/12/22 6:35 PM, Stanislav Fomichev wrote:
>> >> From: Toke Høiland-Jørgensen <toke@xxxxxxxxxx>
>> >>
>> >> Instead of rejecting the attaching of PROG_TYPE_EXT programs to XDP
>> >> programs that consume HW metadata, implement support for propagating the
>> >> offload information. The extension program doesn't need to set a flag or
>> >> ifindex, it these will just be propagated from the target by the verifier.
>> >
>> > s/it/because/ ... these will just be propagated....
>>
>> Yeah, or just drop 'it' :)
>>
>> >> We need to create a separate offload object for the extension program,
>> >> though, since it can be reattached to a different program later (which
>> >> means we can't just inhering the offload information from the target).
>> >
>> > hmm.... inheriting?
>>
>> Think I meant to write "we can't just inherit"
>>
>> >> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> >> index 11c558be4992..8686475f0dbe 100644
>> >> --- a/kernel/bpf/syscall.c
>> >> +++ b/kernel/bpf/syscall.c
>> >> @@ -3021,6 +3021,14 @@ static int bpf_tracing_prog_attach(struct bpf_prog *prog,
>> >>                      goto out_put_prog;
>> >>              }
>> >>
>> >> +            if (bpf_prog_is_dev_bound(prog->aux) &&
>> >
>> >
>> >> +                (bpf_prog_is_offloaded(tgt_prog->aux) ||
>> >> +                 !bpf_prog_is_dev_bound(tgt_prog->aux) ||
>> >> +                 !bpf_offload_dev_match(prog, tgt_prog->aux->offload->netdev))) {
>> >
>> > hmm... tgt_prog->aux->offload does not look safe without taking bpf_devs_lock.
>> > offload could be NULL, no?
>> >
>> > It probably needs a bpf_prog_dev_bound_match(prog, tgt_prog) which takes the lock.
>>
>> Hmm, right, I was kinda expecting that this would not go away while
>> tgt_prog was alive, but I see now that's not the case due to the
>> unregister hook. So yeah, needs locking (same below) :)
>
> Agreed, thanks! These seem easy enough to address on my side, so I'll
> take care of them (and will keep your attribution).

Awesome, thanks!

-Toke





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux