Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <f804f5f2-1114-4aeb-2199-4dda597e1f5e@isovalent.com>
Date:   Mon, 15 May 2023 17:55:42 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.1
Subject: Re: [PATCH bpf-next] bpftool: specify XDP Hints ifname when loading
 program
Content-Language: en-GB
To:     =?UTF-8?Q?Niklas_S=c3=b6derlund?= <niklas.soderlund@corigine.com>,
        Larysa Zaremba <larysa.zaremba@intel.com>
Cc:     Stanislav Fomichev <sdf@google.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Andrii Nakryiko <andrii@kernel.org>,
        Martin KaFai Lau <martin.lau@linux.dev>,
        Song Liu <song@kernel.org>, Yonghong Song <yhs@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        KP Singh <kpsingh@kernel.org>, Hao Luo <haoluo@google.com>,
        Jiri Olsa <jolsa@kernel.org>, bpf@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20230511151345.7529-1-larysa.zaremba@intel.com>
 <dd7a4bec-c0d0-4ffe-3bb8-e4d7ab4a01b8@isovalent.com>
 <ZF5A752Z4eu8FAw9@lincoln> <ZGJePGCkfuCITRxn@oden.dyn.berto.se>
From:   Quentin Monnet <quentin@isovalent.com>
In-Reply-To: <ZGJePGCkfuCITRxn@oden.dyn.berto.se>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

2023-05-15 18:30 UTC+0200 ~ Niklas Söderlund <niklas.soderlund@corigine.com>
> On 2023-05-12 15:36:47 +0200, Larysa Zaremba wrote:
>> [You don't often get email from larysa.zaremba@intel.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> On Fri, May 12, 2023 at 11:23:00AM +0100, Quentin Monnet wrote:
>>> 2023-05-11 17:13 UTC+0200 ~ Larysa Zaremba <larysa.zaremba@intel.com>
>>>> Add ability to specify a network interface used to resolve
>>>> XDP Hints kfuncs when loading program through bpftool.
>>>>
>>>> Usage:
>>>> bpftool prog load <bpf_obj_path> <pin_path> dev xdpmeta <ifname>
>>>
>>> Thanks for this patch!
>>>
>>> Regarding the command-line syntax, I'm not a big fan of the optional
>>> sub-keyword for the device for XDP hints. I must admit I had not
>>> anticipated other another use for the "dev" keyword. Instead, have you
>>> considered one of the following:
>>>
>>> 1) Adding a different keyword ("xdpmeta_dev"?) and making it
>>> incompatible with "dev"
>>>
>>> 2) Another alternative would be adding a sub-keyword for offload too:
>>>
>>>     bpftool p l [...] dev <[offload <ifname> | xdpmeta <ifname>]>
>>>
>>> If the ifname is provided with no sub-keyword, we would consider it for
>>> offload for legacy support, possibly warn that the syntax is deprecated.
>>>
>>> What do you think?
>>>
>>
>> I think first option would look a little bit nicer, but I like the idea to
>> deprecate "dev <ifname>". In my current version, forgetting to add "xdpmeta"
>> resulted in not very descriptive errors, this may confuse new users. So what
>> about:
> 
> I agree the first option looks a little bit nicer, but I think both 
> options would work.
> 
>>
>> bpftool prog load [...] xdpmeta_dev/offload_dev <ifname>
>>
>> "dev <ifname>" syntax would still work, but with a big warning, like this:
>>
>>   'bpftool prog [...] dev <ifname>' syntax is deprecated. Going further, please
>>   use 'offload_dev <ifname>' to offload program to device. For XDP hints
>>   applications, use 'xdpmeta_dev <ifname>'.

OK let's go with this

Thanks,
Quentin