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

["Toke Høiland-Jørgensen" <toke@redhat.com>]

Martin KaFai Lau <martin.lau@linux.dev> writes:

> On 12/12/22 6:35 PM, Stanislav Fomichev wrote:
>> From: Toke Høiland-Jørgensen <toke@redhat.com>
>> 
>> 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) :)

-Toke