From mboxrd@z Thu Jan  1 00:00:00 1970
From: Daniel Borkmann <daniel@iogearbox.net>
Subject: Re: [PATCH net-next v2 05/15] xdp: allow attaching programs loaded
 for specific device
Date: Sun, 12 Nov 2017 20:33:07 +0100
Message-ID: <935db219-863a-3164-6b60-b203eeafa5ae@iogearbox.net>
References: <20171103205630.1083-1-jakub.kicinski@netronome.com>
 <20171103205630.1083-6-jakub.kicinski@netronome.com>
 <20171112090041.GG1993@nanopsycho>
Mime-Version: 1.0
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: netdev@vger.kernel.org, oss-drivers@netronome.com,
        alexei.starovoitov@gmail.com
To: Jiri Pirko <jiri@resnulli.us>,
        Jakub Kicinski <jakub.kicinski@netronome.com>
Return-path: <netdev-owner@vger.kernel.org>
Received: from www62.your-server.de ([213.133.104.62]:43871 "EHLO
        www62.your-server.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1750910AbdKLTdK (ORCPT
        <rfc822;netdev@vger.kernel.org>); Sun, 12 Nov 2017 14:33:10 -0500
In-Reply-To: <20171112090041.GG1993@nanopsycho>
Content-Language: en-US
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>

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.