From: Kevin Traynor <ktraynor@redhat.com>
To: Maryam Tahhan <mtahhan@redhat.com>,
Ferruh Yigit <ferruh.yigit@amd.com>,
stephen@networkplumber.org, lihuisong@huawei.com,
fengchengwen@huawei.com, liuyonglong@huawei.com,
david.marchand@redhat.com
Cc: dev@dpdk.org, Ciara Loftus <ciara.loftus@intel.com>,
Shibin Koikkara Reeny <shibin.koikkara.reeny@intel.com>,
Luca Boccassi <bluca@debian.org>
Subject: Re: [v7 1/1] net/af_xdp: fix multi interface support for K8s
Date: Thu, 11 Jan 2024 13:28:40 +0000 [thread overview]
Message-ID: <3e1ac663-f86c-476b-836a-9db820ad9852@redhat.com> (raw)
In-Reply-To: <89fd40d1-cc91-4390-9bc6-c60f24129ebb@redhat.com>
On 11/01/2024 12:21, Maryam Tahhan wrote:
> On 11/01/2024 11:35, Ferruh Yigit wrote:
>> Devarg is user interface, changing it impacts the user.
>>
>> Assume that user of '22.11.3' using 'use_cni' dev_arg, it will be broken
>> when user upgrades DPDK to '22.11.4', which is not expected.
>>
>> dev_arg is not API/ABI but as it impacts the user, it is in the gray
>> area to backport to the LTS release.
> Fair enough
>>
>>
>> Current patch doesn't have Fixes tag or stable tag, so it doesn't
>> request to be backported to LTS release. I took this as an improvement,
>> more than a fix.
>
> This was overlooked by me apologies. It's been a while since I've
> contributed to DPDK and I must've missed this detail in the contribution
> guide.
>> As far as I understand existing code (that use 'use_cni' dev_arg)
>> supports only single netdev, this patch adds support for multiple netdevs.
>
> The use_cni implementation will no longer work with the AF_XDP DP as the
> use_cni was originally implemented as it has hard coded what's now an
> incorrect path for the UDS.
>
>>
>> So what do you think keep LTS with 'use_cni' dev_arg, is there a
>> requirement to update LTS release?
>> If so, can it be an option to keep 'use_cni' for backward compatibility
>> but add only add 'uds_path' and remove 'use_cni' in next LTS?
>
>
> Yeah we can go back to the version of the patch that had the 'use_cni'
> flag that was used in combination with the path argument. We can add
> better documentation re the "use_cni" misnomer... What we can then do is
> if no path argument is set by the user assume their intent and and
> generate the path internally in the AF_XDP PMD (which was suggested by
> Shibin at some stage). That way there should be no surprises to the End
> User.
>
That plan sounds like it wouldn't impact a user upgrading from 23.11.0
to 23.11.X which is always the main concern.
> Long term I would like to keep a (renamed) path argument (in case the
> path does ever change from the AF_XDP DP POV) and use it also in
> combination with another (maybe boolean) param for passing pinned bpf
> maps rather than another separate path.
>
> WDYT? Would this work for the LTS release?
>
>
Is it needed on LTS branch? If the current limitation was an oversight
or it's not really usable as intended, then I'd say yes. If not, then
please consider the questions in
http://doc.dpdk.org/guides/contributing/stable.html#what-changes-should-be-backported
Seems like it's for
Fixes: 7fc6ae50369d ("net/af_xdp: support CNI Integration")
so it would be only relevant for 23.11 only, so 23.11 maintainer (TBC)
should ack.
I suggest if you feel it should be backported, add the Fixes: and stable
tags, and 23.11 maintainer can review at some point before backport.
thanks,
Kevin.
next prev parent reply other threads:[~2024-01-11 13:28 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-22 11:04 Maryam Tahhan
2024-01-10 14:58 ` Maryam Tahhan
2024-01-10 15:21 ` Ferruh Yigit
2024-01-11 9:01 ` Maryam Tahhan
2024-01-11 11:35 ` Ferruh Yigit
2024-01-11 12:21 ` Maryam Tahhan
2024-01-11 13:28 ` Kevin Traynor [this message]
2024-01-11 14:21 ` Ferruh Yigit
2024-02-07 23:24 ` Ferruh Yigit
2024-02-09 12:40 ` Loftus, Ciara
2024-02-09 16:52 ` Maryam Tahhan
2024-02-09 18:07 ` Ferruh Yigit
2024-02-09 17:02 ` Maryam Tahhan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=3e1ac663-f86c-476b-836a-9db820ad9852@redhat.com \
--to=ktraynor@redhat.com \
--cc=bluca@debian.org \
--cc=ciara.loftus@intel.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=fengchengwen@huawei.com \
--cc=ferruh.yigit@amd.com \
--cc=lihuisong@huawei.com \
--cc=liuyonglong@huawei.com \
--cc=mtahhan@redhat.com \
--cc=shibin.koikkara.reeny@intel.com \
--cc=stephen@networkplumber.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).