netdev - Re: [PATCH net-next 0/2][pull request] 10GbE Intel Wired LAN Driver Updates 2022-03-10

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 11 Mar 2022 09:19:23 -0600
From:   "Samudrala, Sridhar" <sridhar.samudrala@...el.com>
To:     Leon Romanovsky <leon@...rnel.org>,
        Tony Nguyen <anthony.l.nguyen@...el.com>
CC:     <davem@...emloft.net>, <kuba@...nel.org>, <netdev@...r.kernel.org>,
        <sudheer.mogilappagari@...el.com>, <amritha.nambiar@...el.com>,
        <jiri@...dia.com>
Subject: Re: [PATCH net-next 0/2][pull request] 10GbE Intel Wired LAN Driver
 Updates 2022-03-10

On 3/10/2022 10:34 PM, Leon Romanovsky wrote:
> On Thu, Mar 10, 2022 at 03:12:33PM -0800, Tony Nguyen wrote:
>> Sudheer Mogilappagari says:
>>
>> Add support to enable inline flow director which allows uniform
>> distribution of flows among queues of a TC. This is configured
>> on a per TC basis using devlink interface.
>>
>> Devlink params are registered/unregistered during TC creation
>> at runtime. To allow that commit 7a690ad499e7 ("devlink: Clean
>> not-executed param notifications") needs to be reverted.
>>
>> The following are changes since commit 3126b731ceb168b3a780427873c417f2abdd5527:
>>    net: dsa: tag_rtl8_4: fix typo in modalias name
>> and are available in the git repository at:
>>    git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/next-queue 10GbE
>>
>> Kiran Patil (1):
>>    ice: Add inline flow director support for channels
>>
>> Sridhar Samudrala (1):
>>    devlink: Allow parameter registration/unregistration during runtime
> Sorry, NO to whole series.
>
> I don't see any explanation why it is good idea and must-to-be
> implemented one to configure global TC parameter during runtime.

This parameter is applicable only after splitting the netdevice queues into
queue groups(TCs) via tc mqprio command.
The queue groups can be created/destroyed during runtime.
So the patch is trying to register/unregister this parameter when TCs are
created and destroyed.

>
> You created TC with special tool, you should use that tool to configure
> TC and not devlink. Devlink parameters can be seen as better replacement
> of module parameters, which are global by nature. It means that this
> tc_inline_fd can be configured without relation if TC was created or
> not.

Extending tc qdisc mqprio to add this parameter is an option we could explore.
Not sure if it allows changing parameters without reloading the qdisc.

>
> I didn't look too deeply in revert patch, but from glance view it
> is not correct too as it doesn't have any protection from users
> who will try to configure params during devlink_params_unregister().

Is there any limitation that devlink params can be registered only during
probe time?
Would it be OK if we register this parameter during probe time, but allow
changing it only after TCs are created?

-Sridhar