Re: [PATCH net-next v2 05/15] xdp: allow attaching programs loaded for specific device

[Daniel Borkmann <daniel@iogearbox.net>]

On 11/12/2017 10:00 AM, Jiri Pirko wrote:
> Fri, Nov 03, 2017 at 09:56:20PM CET, jakub.kicinski@netronome.com wrote:
>> Pass the netdev pointer to bpf_prog_get_type().  This way
>> BPF code can decide whether the device matches what the
>> code was loaded/translated for.
> 
> [...]
> 
>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>> index 3217c20ea91b..68f9123acd39 100644
>> --- a/kernel/bpf/syscall.c
>> +++ b/kernel/bpf/syscall.c
>> @@ -1057,7 +1057,22 @@ struct bpf_prog *bpf_prog_inc_not_zero(struct bpf_prog *prog)
>> }
>> EXPORT_SYMBOL_GPL(bpf_prog_inc_not_zero);
>>
>> -static struct bpf_prog *__bpf_prog_get(u32 ufd, enum bpf_prog_type *attach_type)
>> +static bool bpf_prog_can_attach(struct bpf_prog *prog,
>> +				enum bpf_prog_type *attach_type,
>> +				struct net_device *netdev)
>> +{
>> +	struct bpf_dev_offload *offload = prog->aux->offload;
>> +
>> +	if (prog->type != *attach_type)
>> +		return false;
>> +	if (offload && offload->netdev != netdev)
> 
> This means you return false in case netdev function arg is NULL. Is that
> intentional?
> 
> Seems wrong to me because for example in cls_bpf_prog_from_efd, you
> would get an error in case TCA_CLS_FLAGS_SKIP_SW is not set.

I think it's fine, the prog was originally loaded in the verifier pass
to be JITed via nfp JIT, so you'll have a valid prog->aux->offload, and
you're specifying TCA_CLS_FLAGS_SKIP_SW for fetching the qdisc dev where
we match devs above. So if that dev is not set (NULL) e.g. due to intention
of running the specified prog in sw, then you cannot use that bpf_prog
which was loaded as offloaded one instead. Looks correct to me.