linux-kernel - Re: [PATCH net-next v5 03/10] ethtool: Add an interface for flashing transceiver modules' firmware

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: <20240429191440.7b1182c3@kernel.org>
Date: Mon, 29 Apr 2024 19:14:40 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Danielle Ratson <danieller@...dia.com>
Cc: <netdev@...r.kernel.org>, <davem@...emloft.net>, <edumazet@...gle.com>,
 <pabeni@...hat.com>, <corbet@....net>, <linux@...linux.org.uk>,
 <sdf@...gle.com>, <kory.maincent@...tlin.com>,
 <maxime.chevallier@...tlin.com>, <vladimir.oltean@....com>,
 <przemyslaw.kitszel@...el.com>, <ahmed.zaki@...el.com>,
 <richardcochran@...il.com>, <shayagr@...zon.com>,
 <paul.greenwalt@...el.com>, <jiri@...nulli.us>,
 <linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
 <mlxsw@...dia.com>, <petrm@...dia.com>, <idosch@...dia.com>
Subject: Re: [PATCH net-next v5 03/10] ethtool: Add an interface for
 flashing transceiver modules' firmware

On Wed, 24 Apr 2024 16:30:16 +0300 Danielle Ratson wrote:
> +MODULE_FW_FLASH_ACT
> +===================
> +
> +Flashes transceiver module firmware.
> +
> +Request contents:
> +
> +  =======================================  ======  ===========================
> +  ``ETHTOOL_A_MODULE_FW_FLASH_HEADER``     nested  request header
> +  ``ETHTOOL_A_MODULE_FW_FLASH_FILE_NAME``  string  firmware image file name
> +  ``ETHTOOL_A_MODULE_FW_FLASH_PASSWORD``   u32     transceiver module password
> +  =======================================  ======  ===========================
> +
> +The firmware update process consists of three logical steps:
> +
> +1. Downloading a firmware image to the transceiver module and validating it.
> +2. Running the firmware image.
> +3. Committing the firmware image so that it is run upon reset.
> +
> +When flash command is given, those three steps are taken in that order.
> +
> +The ``ETHTOOL_A_MODULE_FW_FLASH_FILE_NAME`` attribute encodes the firmware
> +image file name. The firmware image is downloaded to the transceiver module,
> +validated, run and committed.
> +
> +The optional ``ETHTOOL_A_MODULE_FW_FLASH_PASSWORD`` attribute encodes a password
> +that might be required as part of the transceiver module firmware update
> +process.
> +
> +The firmware update process can take several minutes to complete. Therefore,
> +during the update process notifications are emitted from the kernel to user
> +space updating it about the status and progress.

We should spell out that the request only kicks off the process.
devlink flashing blocks the netlink socket send() until it's completed,
and the notifications have to be received on another socket.
So someone who has devlink experience may expect the same behavior here.

> +Notification contents:
> +
> + +---------------------------------------------------+--------+----------------+
> + | ``ETHTOOL_A_MODULE_FW_FLASH_HEADER``              | nested | reply header   |
> + +---------------------------------------------------+--------+----------------+
> + | ``ETHTOOL_A_MODULE_FW_FLASH_STATUS``              | u32    | status         |
> + +---------------------------------------------------+--------+----------------+
> + | ``ETHTOOL_A_MODULE_FW_FLASH_STATUS_MSG``          | string | status message |
> + +---------------------------------------------------+--------+----------------+
> + | ``ETHTOOL_A_MODULE_FW_FLASH_DONE``                | uint   | progress       |
> + +---------------------------------------------------+--------+----------------+
> + | ``ETHTOOL_A_MODULE_FW_FLASH_TOTAL``               | uint   | total          |
> + +---------------------------------------------------+--------+----------------+

> +enum {
> +	ETHTOOL_A_MODULE_FW_FLASH_UNSPEC,
> +	ETHTOOL_A_MODULE_FW_FLASH_HEADER,		/* nest - _A_HEADER_* */
> +	ETHTOOL_A_MODULE_FW_FLASH_FILE_NAME,		/* string */
> +	ETHTOOL_A_MODULE_FW_FLASH_PASSWORD,		/* u32 */
> +	ETHTOOL_A_MODULE_FW_FLASH_PAD,

uint doesn't need pad, and pad is not specified in YAML, so 
the following attribute IDs will be all off by one in YNL.

> +	ETHTOOL_A_MODULE_FW_FLASH_STATUS,		/* u32 */
> +	ETHTOOL_A_MODULE_FW_FLASH_STATUS_MSG,		/* string */
> +	ETHTOOL_A_MODULE_FW_FLASH_DONE,			/* uint */
> +	ETHTOOL_A_MODULE_FW_FLASH_TOTAL,		/* uint */
> +
> +	/* add new constants above here */
> +	__ETHTOOL_A_MODULE_FW_FLASH_CNT,
> +	ETHTOOL_A_MODULE_FW_FLASH_MAX = (__ETHTOOL_A_MODULE_FW_FLASH_CNT - 1)