Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

devlink: move flash end and begin to core devlink

When performing a flash update via devlink, device drivers may inform
user space of status updates via
devlink_flash_update_(begin|end|timeout|status)_notify functions.

It is expected that drivers do not send any status notifications unless
they send a begin and end message. If a driver sends a status
notification without sending the appropriate end notification upon
finishing (regardless of success or failure), the current implementation
of the devlink userspace program can get stuck endlessly waiting for the
end notification that will never come.

The current ice driver implementation may send such a status message
without the appropriate end notification in rare cases.

Fixing the ice driver is relatively simple: we just need to send the
begin_notify at the start of the function and always send an end_notify
no matter how the function exits.

Rather than assuming driver authors will always get this right in the
future, lets just fix the API so that it is not possible to get wrong.
Make devlink_flash_update_begin_notify and
devlink_flash_update_end_notify static, and call them in devlink.c core
code. Always send the begin_notify just before calling the driver's
flash_update routine. Always send the end_notify just after the routine
returns regardless of success or failure.

Doing this makes the status notification easier to use from the driver,
as it no longer needs to worry about catching failures and cleaning up
by calling devlink_flash_update_end_notify. It is now no longer possible
to do the wrong thing in this regard. We also save a couple of lines of
code in each driver.

Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Acked-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Jacob Keller and committed by
Jakub Kicinski
52cc5f3a b44cfd4f

+7 -19
-2
drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
··· 30 30 return -EPERM; 31 31 } 32 32 33 - devlink_flash_update_begin_notify(dl); 34 33 devlink_flash_update_status_notify(dl, "Preparing to flash", NULL, 0, 0); 35 34 rc = bnxt_flash_package_from_fw_obj(bp->dev, params->fw, 0); 36 35 if (!rc) 37 36 devlink_flash_update_status_notify(dl, "Flashing done", NULL, 0, 0); 38 37 else 39 38 devlink_flash_update_status_notify(dl, "Flashing failed", NULL, 0, 0); 40 - devlink_flash_update_end_notify(dl); 41 39 return rc; 42 40 } 43 41
+1 -4
drivers/net/ethernet/intel/ice/ice_devlink.c
··· 275 275 if (err) 276 276 return err; 277 277 278 - devlink_flash_update_begin_notify(devlink); 279 278 devlink_flash_update_status_notify(devlink, "Preparing to flash", NULL, 0, 0); 280 - err = ice_flash_pldm_image(pf, params->fw, preservation, extack); 281 - devlink_flash_update_end_notify(devlink); 282 279 283 - return err; 280 + return ice_flash_pldm_image(pf, params->fw, preservation, extack); 284 281 } 285 282 286 283 static const struct devlink_ops ice_devlink_ops = {
-3
drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
··· 368 368 } 369 369 370 370 mlxfw_info(mlxfw_dev, "Initialize firmware flash process\n"); 371 - devlink_flash_update_begin_notify(mlxfw_dev->devlink); 372 371 mlxfw_status_notify(mlxfw_dev, "Initializing firmware flash process", 373 372 NULL, 0, 0); 374 373 err = mlxfw_dev->ops->fsm_lock(mlxfw_dev, &fwhandle); ··· 416 417 mlxfw_info(mlxfw_dev, "Firmware flash done\n"); 417 418 mlxfw_status_notify(mlxfw_dev, "Firmware flash done", NULL, 0, 0); 418 419 mlxfw_mfa2_file_fini(mfa2_file); 419 - devlink_flash_update_end_notify(mlxfw_dev->devlink); 420 420 return 0; 421 421 422 422 err_state_wait_activate_to_locked: ··· 427 429 mlxfw_dev->ops->fsm_release(mlxfw_dev, fwhandle); 428 430 err_fsm_lock: 429 431 mlxfw_mfa2_file_fini(mfa2_file); 430 - devlink_flash_update_end_notify(mlxfw_dev->devlink); 431 432 return err; 432 433 } 433 434 EXPORT_SYMBOL(mlxfw_firmware_flash);
-2
drivers/net/ethernet/pensando/ionic/ionic_fw.c
··· 107 107 netdev_info(netdev, "Installing firmware\n"); 108 108 109 109 dl = priv_to_devlink(ionic); 110 - devlink_flash_update_begin_notify(dl); 111 110 devlink_flash_update_status_notify(dl, "Preparing to flash", NULL, 0, 0); 112 111 113 112 buf_sz = sizeof(idev->dev_cmd_regs->data); ··· 192 193 devlink_flash_update_status_notify(dl, "Flash failed", NULL, 0, 0); 193 194 else 194 195 devlink_flash_update_status_notify(dl, "Flash done", NULL, 0, 0); 195 - devlink_flash_update_end_notify(dl); 196 196 return err; 197 197 }
-2
drivers/net/netdevsim/dev.c
··· 766 766 return -EOPNOTSUPP; 767 767 768 768 if (nsim_dev->fw_update_status) { 769 - devlink_flash_update_begin_notify(devlink); 770 769 devlink_flash_update_status_notify(devlink, 771 770 "Preparing to flash", 772 771 params->component, 0, 0); ··· 789 790 params->component, 81); 790 791 devlink_flash_update_status_notify(devlink, "Flashing done", 791 792 params->component, 0, 0); 792 - devlink_flash_update_end_notify(devlink); 793 793 } 794 794 795 795 return 0;
-2
include/net/devlink.h
··· 1577 1577 enum devlink_reload_limit limit, 1578 1578 u32 actions_performed); 1579 1579 1580 - void devlink_flash_update_begin_notify(struct devlink *devlink); 1581 - void devlink_flash_update_end_notify(struct devlink *devlink); 1582 1580 void devlink_flash_update_status_notify(struct devlink *devlink, 1583 1581 const char *status_msg, 1584 1582 const char *component,
+6 -4
net/core/devlink.c
··· 3372 3372 nlmsg_free(msg); 3373 3373 } 3374 3374 3375 - void devlink_flash_update_begin_notify(struct devlink *devlink) 3375 + static void devlink_flash_update_begin_notify(struct devlink *devlink) 3376 3376 { 3377 3377 struct devlink_flash_notify params = { 0 }; 3378 3378 ··· 3380 3380 DEVLINK_CMD_FLASH_UPDATE, 3381 3381 &params); 3382 3382 } 3383 - EXPORT_SYMBOL_GPL(devlink_flash_update_begin_notify); 3384 3383 3385 - void devlink_flash_update_end_notify(struct devlink *devlink) 3384 + static void devlink_flash_update_end_notify(struct devlink *devlink) 3386 3385 { 3387 3386 struct devlink_flash_notify params = { 0 }; 3388 3387 ··· 3389 3390 DEVLINK_CMD_FLASH_UPDATE_END, 3390 3391 &params); 3391 3392 } 3392 - EXPORT_SYMBOL_GPL(devlink_flash_update_end_notify); 3393 3393 3394 3394 void devlink_flash_update_status_notify(struct devlink *devlink, 3395 3395 const char *status_msg, ··· 3475 3477 return ret; 3476 3478 } 3477 3479 3480 + devlink_flash_update_begin_notify(devlink); 3478 3481 ret = devlink->ops->flash_update(devlink, &params, info->extack); 3482 + devlink_flash_update_end_notify(devlink); 3479 3483 3480 3484 release_firmware(params.fw); 3481 3485 ··· 10244 10244 goto out; 10245 10245 10246 10246 mutex_lock(&devlink->lock); 10247 + devlink_flash_update_begin_notify(devlink); 10247 10248 ret = devlink->ops->flash_update(devlink, &params, NULL); 10249 + devlink_flash_update_end_notify(devlink); 10248 10250 mutex_unlock(&devlink->lock); 10249 10251 10250 10252 release_firmware(params.fw);