On 4/5/18 2:13 PM, Jiri Pirko wrote:
> From: Jiri Pirko <[email protected]>
>
> This resolves race during initialization where the resources with
> ops are registered before driver and the structures used by occ_get
> op is initialized. So keep occ_get callbacks registered only when
> all structs are initialized.
>
> The example flows, as it is in mlxsw:
> 1) driver load/asic probe:
> mlxsw_core
> -> mlxsw_sp_resources_register
> -> mlxsw_sp_kvdl_resources_register
> -> devlink_resource_register IDX
> mlxsw_spectrum
> -> mlxsw_sp_kvdl_init
> -> mlxsw_sp_kvdl_parts_init
> -> mlxsw_sp_kvdl_part_init
> -> devlink_resource_size_get IDX (to get the current setup
> size from devlink)
> -> devlink_resource_occ_get_register IDX (register current
> occupancy getter)
> 2) reload triggered by devlink command:
> -> mlxsw_devlink_core_bus_device_reload
> -> mlxsw_sp_fini
> -> mlxsw_sp_kvdl_fini
> -> devlink_resource_occ_get_unregister IDX
> (struct mlxsw_sp *mlxsw_sp is freed at this point, call to occ get
> which is using mlxsw_sp would cause use-after free)
> -> mlxsw_sp_init
> -> mlxsw_sp_kvdl_init
> -> mlxsw_sp_kvdl_parts_init
> -> mlxsw_sp_kvdl_part_init
> -> devlink_resource_size_get IDX (to get the current setup
> size from devlink)
> -> devlink_resource_occ_get_register IDX (register current
> occupancy getter)
>
> Fixes: d9f9b9a4d05f ("devlink: Add support for resource abstraction")
> Signed-off-by: Jiri Pirko <[email protected]>
> ---
Why won't something like the attached work as opposed to playing
register / unregister games?
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c
b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 93ea56620a24..dcded613faa6 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -113,6 +113,7 @@ struct mlxsw_core {
struct mlxsw_thermal *thermal;
struct mlxsw_core_port *ports;
unsigned int max_ports;
+ bool reload_in_progress;
bool reload_fail;
unsigned long driver_priv[0];
/* driver_priv has to be always the last item */
@@ -154,6 +155,12 @@ void *mlxsw_core_driver_priv(struct mlxsw_core *mlxsw_core)
}
EXPORT_SYMBOL(mlxsw_core_driver_priv);
+bool mlxsw_core_reload_in_progress(struct mlxsw_core *mlxsw_core)
+{
+ return mlxsw_core->mlxsw_core_driver_priv;
+}
+EXPORT_SYMBOL(mlxsw_core_reload_in_progress);
+
struct mlxsw_rx_listener_item {
struct list_head list;
struct mlxsw_rx_listener rxl;
@@ -972,12 +979,16 @@ static int mlxsw_devlink_core_bus_device_reload(struct
devlink *devlink)
if (!mlxsw_bus->reset)
return -EOPNOTSUPP;
+ mlxsw_core->reload_in_progress = true;
+
mlxsw_core_bus_device_unregister(mlxsw_core, true);
mlxsw_bus->reset(mlxsw_core->bus_priv);
err = mlxsw_core_bus_device_register(mlxsw_core->bus_info,
mlxsw_core->bus,
mlxsw_core->bus_priv, true,
devlink);
+ mlxsw_core->reload_in_progress = false;
+
if (err)
mlxsw_core->reload_fail = true;
return err;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h
b/drivers/net/ethernet/mellanox/mlxsw/core.h
index 092d39399f3c..ffa1db154757 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -60,6 +60,7 @@ struct mlxsw_bus_info;
unsigned int mlxsw_core_max_ports(const struct mlxsw_core *mlxsw_core);
void *mlxsw_core_driver_priv(struct mlxsw_core *mlxsw_core);
+bool mlxsw_core_reload_in_progress(struct mlxsw_core *mlxsw_core);
int mlxsw_core_driver_register(struct mlxsw_driver *mlxsw_driver);
void mlxsw_core_driver_unregister(struct mlxsw_driver *mlxsw_driver);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 53fffd09d133..09b89af37d8a 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -3808,8 +3808,12 @@ static const struct mlxsw_config_profile
mlxsw_sp_config_profile = {
static u64 mlxsw_sp_resource_kvd_linear_occ_get(struct devlink *devlink)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
+ struct mlxsw_sp *mlxsw_sp;
+
+ if (mlxsw_core_reload_in_progress(mlxsw_core))
+ return 0;
+ mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
return mlxsw_sp_kvdl_occ_get(mlxsw_sp);
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
index 8796db44dcc3..dd66285bafb5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_kvdl.c
@@ -329,9 +329,13 @@ u64 mlxsw_sp_kvdl_occ_get(const struct mlxsw_sp *mlxsw_sp)
static u64 mlxsw_sp_kvdl_single_occ_get(struct devlink *devlink)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
struct mlxsw_sp_kvdl_part *part;
+ struct mlxsw_sp *mlxsw_sp;
+ if (mlxsw_core_reload_in_progress(mlxsw_core))
+ return 0;
+
+ mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_SINGLE];
return mlxsw_sp_kvdl_part_occ(part);
}
@@ -339,8 +343,13 @@ static u64 mlxsw_sp_kvdl_single_occ_get(struct devlink
*devlink)
static u64 mlxsw_sp_kvdl_chunks_occ_get(struct devlink *devlink)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
struct mlxsw_sp_kvdl_part *part;
+ struct mlxsw_sp *mlxsw_sp;
+
+ if (mlxsw_core_reload_in_progress(mlxsw_core))
+ return 0;
+
+ mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_CHUNKS];
return mlxsw_sp_kvdl_part_occ(part);
@@ -349,9 +358,13 @@ static u64 mlxsw_sp_kvdl_chunks_occ_get(struct devlink
*devlink)
static u64 mlxsw_sp_kvdl_large_chunks_occ_get(struct devlink *devlink)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
- struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
struct mlxsw_sp_kvdl_part *part;
+ struct mlxsw_sp *mlxsw_sp;
+
+ if (mlxsw_core_reload_in_progress(mlxsw_core))
+ return 0;
+ mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
part = mlxsw_sp->kvdl->parts[MLXSW_SP_KVDL_PART_ID_LARGE_CHUNKS];
return mlxsw_sp_kvdl_part_occ(part);
}
