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 for Android: free password hash cracker in your pocket

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

Message-ID: <YirRQWT7dtTV4fwG@unreal>
Date:   Fri, 11 Mar 2022 06:34:09 +0200
From:   Leon Romanovsky <leon@...rnel.org>
To:     Tony Nguyen <anthony.l.nguyen@...el.com>
Cc:     davem@...emloft.net, kuba@...nel.org, netdev@...r.kernel.org,
        sudheer.mogilappagari@...el.com, sridhar.samudrala@...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 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.

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.

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().

Thanks

> 
>  drivers/net/ethernet/intel/ice/ice.h          |  83 +++++
>  .../net/ethernet/intel/ice/ice_adminq_cmd.h   |  12 +
>  drivers/net/ethernet/intel/ice/ice_devlink.c  | 130 ++++++++
>  drivers/net/ethernet/intel/ice/ice_devlink.h  |   2 +
>  drivers/net/ethernet/intel/ice/ice_ethtool.c  |   1 +
>  drivers/net/ethernet/intel/ice/ice_fdir.c     |  25 +-
>  drivers/net/ethernet/intel/ice/ice_fdir.h     |   5 +
>  .../net/ethernet/intel/ice/ice_hw_autogen.h   |   1 +
>  .../net/ethernet/intel/ice/ice_lan_tx_rx.h    |   3 +
>  drivers/net/ethernet/intel/ice/ice_main.c     | 173 ++++++++++-
>  drivers/net/ethernet/intel/ice/ice_txrx.c     | 294 ++++++++++++++++++
>  drivers/net/ethernet/intel/ice/ice_txrx.h     |   8 +-
>  drivers/net/ethernet/intel/ice/ice_type.h     |   1 +
>  net/core/devlink.c                            |  14 +-
>  14 files changed, 734 insertions(+), 18 deletions(-)
> 
> -- 
> 2.31.1
>