netdev - Re: [PATCH net-next 1/2] devlink: add fw bank select parameter

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]

Message-ID: <20221208172422.37423144@kernel.org>
Date:   Thu, 8 Dec 2022 17:24:22 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Jacob Keller <jacob.e.keller@...el.com>
Cc:     Shannon Nelson <shannon.nelson@....com>, <netdev@...r.kernel.org>,
        <davem@...emloft.net>, <jiri@...dia.com>
Subject: Re: [PATCH net-next 1/2] devlink: add fw bank select parameter

On Thu, 8 Dec 2022 16:47:31 -0800 Jacob Keller wrote:
> This is what I was thinking of and looks good to me. As for how to add 
> attributes to get us from the current netlink API to this, I'm not 100% 
> sure.
> 
> I think we can mostly just add the bank ID and flags to indicate which 
> one is active and which one will be programmed next.

Why flags, tho?

The current nesting is:

  DEVLINK_ATTR_INFO_DRIVER_NAME		[str]
  DEVLINK_ATTR_INFO_SERIAL_NUMBER	[str]
  DEVLINK_ATTR_INFO_BOARD_SERIAL_NUMBER	[str]

  DEVLINK_ATTR_INFO_VERSION_FIXED	[nest] // multiple VERSION_* nests follow
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_FIXED	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_RUNNING	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_RUNNING	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]


Now we'd throw the bank into the nests, and add root attrs for the
current / flash / active as top level attrs:

  DEVLINK_ATTR_INFO_DRIVER_NAME		[str]
  DEVLINK_ATTR_INFO_SERIAL_NUMBER	[str]
  DEVLINK_ATTR_INFO_BOARD_SERIAL_NUMBER	[str]
  DEVLINK_ATTR_INFO_BANK_ACTIVE		[u32] // << optional
  DEVLINK_ATTR_INFO_BANK_UPDATE_TGT	[u32] // << optional

  DEVLINK_ATTR_INFO_VERSION_FIXED	[nest] // multiple VERSION_* nests follow
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_FIXED	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_RUNNING	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_RUNNING	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
  DEVLINK_ATTR_INFO_VERSION_STORED	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE	[str]
    DEVLINK_ATTR_INFO_VERSION_BANK	[u32] // << optional
  DEVLINK_ATTR_INFO_VERSION_STORED	[nest]
    DEVLINK_ATTR_INFO_VERSION_NAME	[str]
    DEVLINK_ATTR_INFO_VERSION_VALUE     [str]
    DEVLINK_ATTR_INFO_VERSION_BANK	[u32] // << optional

> I think we could also add a new attribute to both reload and flash which 
> specify which bank to use. For flash, this would be which bank to 
> program, and for update this would be which bank to load the firmware 
> from when doing a "fw_activate".

SG!

> Is that reasonable? Do you still need a permanent "use this bank by 
> default" parameter as well?

I hope we cover all cases, so no param needed?