From: Jakub Kicinski <kuba@xxxxxxxxxx>
Sent: Wednesday, April 9, 2025 4:31 AM
>On Mon, 7 Apr 2025 14:51:14 -0700 Tony Nguyen wrote:
>> From: Jedrzej Jagielski <jedrzej.jagielski@xxxxxxxxx>
>>
>> Add functions reading inactive versions from the inactive flash
>> bank.
>
>Just to be crystal clear -- could you share the outputs for dev info:
> - before update
> - after update, before reboot/reload
> - after update, after activation (/reboot/reload)
>?
OK so this looks this way:
[osdadmin@os-delivery ~]$ devlink dev info pci/0000:63:00.0
pci/0000:63:00.0:
driver ixgbe
serial_number 00-00-00-ff-ff-00-01-00
versions:
fixed:
board.id N43235-000
running:
fw.undi 0.12552.1
fw.bundle_id 0x8000c459
fw.mgmt.api 1.7.11
fw.mgmt.build 0x3a458c89
fw.mgmt.srev 1
fw.undi.srev 1
fw.psid.api 1.00
fw.netlist 0.0.0-0.22.0
fw.netlist.build 0xe1aaca1d
[osdadmin@os-delivery ~]$
[root@os-delivery lanconf]# devlink dev flash pci/0000:63:00.0 file nvm/file.bin
Preparing to flash
[...]
[root@os-delivery lanconf]# devlink dev info pci/0000:63:00.0
pci/0000:63:00.0:
driver ixgbe
serial_number 00-00-00-ff-ff-00-01-00
versions:
fixed:
board.id N43235-000
running:
fw.undi 0.12552.1
fw.bundle_id 0x8000c459
fw.mgmt.api 1.7.11
fw.mgmt.build 0x3a458c89
fw.mgmt.srev 1
fw.undi.srev 1
fw.psid.api 1.00
fw.netlist 0.0.0-0.22.0
fw.netlist.build 0xe1aaca1d
stored:
fw.mgmt.srev 1
fw.bundle_id 0x8000d993
fw.psid.api 1.01
fw.undi 0.12552.1
fw.undi.srev 1
[root@os-delivery lanconf]#
[root@os-delivery lanconf]# devlink dev reload pci/0000:63:00.0 action fw_activate
reload_actions_performed:
fw_activate
[root@os-delivery lanconf]# devlink dev info pci/0000:63:00.0
pci/0000:63:00.0:
driver ixgbe
serial_number 00-00-00-ff-ff-00-01-00
versions:
fixed:
board.id N43235-000
running:
fw.undi 0.12552.1
fw.bundle_id 0x8000d993
fw.mgmt.api 1.7.11
fw.mgmt.build 0xbc86d939
fw.mgmt.srev 1
fw.undi.srev 1
fw.psid.api 1.01
fw.netlist 0.0.0-0.22.0
fw.netlist.build 0xe1aaca1d
[root@os-delivery lanconf]#
>
>AFAICT the code is fine but talking about the "inactive versions"
>is not exactly in line with the uAPI expectations.
>
>> +static int ixgbe_set_ctx_dev_caps(struct ixgbe_hw *hw,
>> + struct ixgbe_info_ctx *ctx,
>> + struct netlink_ext_ack *extack)
>> +{
>> + int err = ixgbe_discover_dev_caps(hw, &ctx->dev_caps);
>> +
>> + if (err) {
>
>Don't call functions which need error checking as part of variable init.
>
>> + NL_SET_ERR_MSG_MOD(extack,
>> + "Unable to discover device capabilities");
>> + return err;
>> + }
>> +
>> + if (ctx->dev_caps.common_cap.nvm_update_pending_orom) {
>> + err = ixgbe_get_inactive_orom_ver(hw, &ctx->pending_orom);
>> + if (err)
>> + ctx->dev_caps.common_cap.nvm_update_pending_orom =
>> + false;
>
>This function would benefit from having ctx->dev_caps.common_cap
>stored to a local variable with a shorter name :S
yeah, agree
>
>> + }
>> +
>> + if (ctx->dev_caps.common_cap.nvm_update_pending_nvm) {
>> + err = ixgbe_get_inactive_nvm_ver(hw, &ctx->pending_nvm);
>> + if (err)
>> + ctx->dev_caps.common_cap.nvm_update_pending_nvm = false;
>> + }
>> +
>> + if (ctx->dev_caps.common_cap.nvm_update_pending_netlist) {
>> + err = ixgbe_get_inactive_netlist_ver(hw, &ctx->pending_netlist);
>> + if (err)
>> + ctx->dev_caps.common_cap.nvm_update_pending_netlist =
>> + false;
>> + }
>--
>pw-bot: cr
