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>

diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
index 4ebae8a236fdd3ce81299c9e0abd6139c92f067b..6b7b69ed62db08906588c10911841fcb670ddae9 100644 (file)
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_devlink.c
@@ -30,14 +30,12 @@ bnxt_dl_flash_update(struct devlink *dl,
                return -EPERM;
        }
 
-       devlink_flash_update_begin_notify(dl);
        devlink_flash_update_status_notify(dl, "Preparing to flash", NULL, 0, 0);
        rc = bnxt_flash_package_from_fw_obj(bp->dev, params->fw, 0);
        if (!rc)
                devlink_flash_update_status_notify(dl, "Flashing done", NULL, 0, 0);
        else
                devlink_flash_update_status_notify(dl, "Flashing failed", NULL, 0, 0);
-       devlink_flash_update_end_notify(dl);
        return rc;
 }
 

diff --git a/drivers/net/ethernet/intel/ice/ice_devlink.c b/drivers/net/ethernet/intel/ice/ice_devlink.c
index 0036d3e7df0bf513cfeaad582d4e1009902b3c01..29d6192b15f32a6b90ad0578023e23d19e04e534 100644 (file)
--- a/drivers/net/ethernet/intel/ice/ice_devlink.c
+++ b/drivers/net/ethernet/intel/ice/ice_devlink.c
@@ -275,12 +275,9 @@ ice_devlink_flash_update(struct devlink *devlink,
        if (err)
                return err;
 
-       devlink_flash_update_begin_notify(devlink);
        devlink_flash_update_status_notify(devlink, "Preparing to flash", NULL, 0, 0);
-       err = ice_flash_pldm_image(pf, params->fw, preservation, extack);
-       devlink_flash_update_end_notify(devlink);
 
-       return err;
+       return ice_flash_pldm_image(pf, params->fw, preservation, extack);
 }
 
 static const struct devlink_ops ice_devlink_ops = {

diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
index bcd166911d444b13dcbe7b390073368116a1ac34..46245e0b24623d669c064e976f911f1e0b54d7ed 100644 (file)
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
@@ -368,7 +368,6 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
        }
 
        mlxfw_info(mlxfw_dev, "Initialize firmware flash process\n");
-       devlink_flash_update_begin_notify(mlxfw_dev->devlink);
        mlxfw_status_notify(mlxfw_dev, "Initializing firmware flash process",
                            NULL, 0, 0);
        err = mlxfw_dev->ops->fsm_lock(mlxfw_dev, &fwhandle);
@@ -417,7 +416,6 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
        mlxfw_info(mlxfw_dev, "Firmware flash done\n");
        mlxfw_status_notify(mlxfw_dev, "Firmware flash done", NULL, 0, 0);
        mlxfw_mfa2_file_fini(mfa2_file);
-       devlink_flash_update_end_notify(mlxfw_dev->devlink);
        return 0;
 
 err_state_wait_activate_to_locked:
@@ -429,7 +427,6 @@ err_state_wait_idle_to_locked:
        mlxfw_dev->ops->fsm_release(mlxfw_dev, fwhandle);
 err_fsm_lock:
        mlxfw_mfa2_file_fini(mfa2_file);
-       devlink_flash_update_end_notify(mlxfw_dev->devlink);
        return err;
 }
 EXPORT_SYMBOL(mlxfw_firmware_flash);

diff --git a/drivers/net/ethernet/pensando/ionic/ionic_fw.c b/drivers/net/ethernet/pensando/ionic/ionic_fw.c
index 8922c316abe3a39b1aaf24e044f5955a2c7a5ad9..5f40324cd243fe2f2f79b924920951304d25df45 100644 (file)
--- a/drivers/net/ethernet/pensando/ionic/ionic_fw.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_fw.c
@@ -107,7 +107,6 @@ int ionic_firmware_update(struct ionic_lif *lif, const struct firmware *fw,
        netdev_info(netdev, "Installing firmware\n");
 
        dl = priv_to_devlink(ionic);
-       devlink_flash_update_begin_notify(dl);
        devlink_flash_update_status_notify(dl, "Preparing to flash", NULL, 0, 0);
 
        buf_sz = sizeof(idev->dev_cmd_regs->data);
@@ -193,6 +192,5 @@ err_out:
                devlink_flash_update_status_notify(dl, "Flash failed", NULL, 0, 0);
        else
                devlink_flash_update_status_notify(dl, "Flash done", NULL, 0, 0);
-       devlink_flash_update_end_notify(dl);
        return err;
 }

diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index fe48817b3985670c3c877911bfd0729fd1bfc1a8..816af1f55e2cd512cfb27b23386b48d99140a1bc 100644 (file)
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -766,7 +766,6 @@ static int nsim_dev_flash_update(struct devlink *devlink,
                return -EOPNOTSUPP;
 
        if (nsim_dev->fw_update_status) {
-               devlink_flash_update_begin_notify(devlink);
                devlink_flash_update_status_notify(devlink,
                                                   "Preparing to flash",
                                                   params->component, 0, 0);
@@ -790,7 +789,6 @@ static int nsim_dev_flash_update(struct devlink *devlink,
                                                    params->component, 81);
                devlink_flash_update_status_notify(devlink, "Flashing done",
                                                   params->component, 0, 0);
-               devlink_flash_update_end_notify(devlink);
        }
 
        return 0;

diff --git a/include/net/devlink.h b/include/net/devlink.h
index d1d125a3332211bcd927189aced413dcfa82a28c..457c537d0ef22f361dc08f1b33f12c7957920112 100644 (file)
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1577,8 +1577,6 @@ void devlink_remote_reload_actions_performed(struct devlink *devlink,
                                             enum devlink_reload_limit limit,
                                             u32 actions_performed);
 
-void devlink_flash_update_begin_notify(struct devlink *devlink);
-void devlink_flash_update_end_notify(struct devlink *devlink);
 void devlink_flash_update_status_notify(struct devlink *devlink,
                                        const char *status_msg,
                                        const char *component,

diff --git a/net/core/devlink.c b/net/core/devlink.c
index 7c5a5da8e338b60d4c3dc6fc10d868dc8b79312d..e6fb1fdedded85f622934f45f0ceab4c6996cb49 100644 (file)
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3372,7 +3372,7 @@ out_free_msg:
        nlmsg_free(msg);
 }
 
-void devlink_flash_update_begin_notify(struct devlink *devlink)
+static void devlink_flash_update_begin_notify(struct devlink *devlink)
 {
        struct devlink_flash_notify params = { 0 };
 
@@ -3380,9 +3380,8 @@ void devlink_flash_update_begin_notify(struct devlink *devlink)
                                      DEVLINK_CMD_FLASH_UPDATE,
                                      &params);
 }
-EXPORT_SYMBOL_GPL(devlink_flash_update_begin_notify);
 
-void devlink_flash_update_end_notify(struct devlink *devlink)
+static void devlink_flash_update_end_notify(struct devlink *devlink)
 {
        struct devlink_flash_notify params = { 0 };
 
@@ -3390,7 +3389,6 @@ void devlink_flash_update_end_notify(struct devlink *devlink)
                                      DEVLINK_CMD_FLASH_UPDATE_END,
                                      &params);
 }
-EXPORT_SYMBOL_GPL(devlink_flash_update_end_notify);
 
 void devlink_flash_update_status_notify(struct devlink *devlink,
                                        const char *status_msg,
@@ -3477,7 +3475,9 @@ static int devlink_nl_cmd_flash_update(struct sk_buff *skb,
                return ret;
        }
 
+       devlink_flash_update_begin_notify(devlink);
        ret = devlink->ops->flash_update(devlink, &params, info->extack);
+       devlink_flash_update_end_notify(devlink);
 
        release_firmware(params.fw);
 
@@ -10244,7 +10244,9 @@ int devlink_compat_flash_update(struct net_device *dev, const char *file_name)
                goto out;
 
        mutex_lock(&devlink->lock);
+       devlink_flash_update_begin_notify(devlink);
        ret = devlink->ops->flash_update(devlink, &params, NULL);
+       devlink_flash_update_end_notify(devlink);
        mutex_unlock(&devlink->lock);
 
        release_firmware(params.fw);