Re: [v7 1/1] net/af_xdp: fix multi interface support for K8s

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 1/11/2024 2:21 PM, Ferruh Yigit wrote:
> On 1/11/2024 12:21 PM, 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.
>>
> 
> Ack, this keeps backward compatibility,
> 
> BUT if 'use_cni' is already broken in v23.11 (that is what I understand
> from your above comment), means there is no user of it in LTS, and we
> can be more pragmatic and replace the dev_args, by backporting this
> patch, assuming LTS maintainer is also OK with it.
> 

Hi Maryam,

How do you want to continue with the patch, I think options we considered:

1. Fix 'use_cni' documentation (which we can backport to LTS) and
overload the argument for new purpose. This will enable new feature by
keeping backward compatibility. And requires new version of this patch.

2. If the 'use_cni' is completely broken in the 23.11 LTS, which means
there is no user or backward compatibility to worry about, we can merge
this patch and backport it to LTS.

3. Don't backport this fix to LTS, merge only to current release, which
means your new feature won't be available to some users as long as a few
years.


(1.) is most user friendly, but if 'use_cni' already broken in LTS we
can go with option (2.). What do you think?



btw, @Ciara, @Maryam, if (2.) is true, how we end up having a feature
('use_cni' dev_args) completely broken in an LTS release?



> 
>> 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?
>>
>>
>