* [patch net-next 1/7] mlxsw: Move firmware flash implementation to devlink
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
@ 2019-05-23 9:45 ` Jiri Pirko
2019-05-23 9:45 ` [patch net-next 2/7] mlx5: " Jiri Pirko
` (10 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Jiri Pirko @ 2019-05-23 9:45 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
From: Jiri Pirko <jiri@mellanox.com>
Benefit from the devlink flash update implementation and ethtool
fallback to it and move firmware flash implementation there.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/core.c | 15 ++++++
drivers/net/ethernet/mellanox/mlxsw/core.h | 3 ++
.../net/ethernet/mellanox/mlxsw/spectrum.c | 49 +++++++++----------
3 files changed, 41 insertions(+), 26 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 6ee6de7f0160..a992a7c69b45 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1003,6 +1003,20 @@ static int mlxsw_devlink_core_bus_device_reload(struct devlink *devlink,
return err;
}
+static int mlxsw_devlink_flash_update(struct devlink *devlink,
+ const char *file_name,
+ const char *component,
+ struct netlink_ext_ack *extack)
+{
+ struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+ struct mlxsw_driver *mlxsw_driver = mlxsw_core->driver;
+
+ if (!mlxsw_driver->flash_update)
+ return -EOPNOTSUPP;
+ return mlxsw_driver->flash_update(mlxsw_core, file_name,
+ component, extack);
+}
+
static const struct devlink_ops mlxsw_devlink_ops = {
.reload = mlxsw_devlink_core_bus_device_reload,
.port_type_set = mlxsw_devlink_port_type_set,
@@ -1019,6 +1033,7 @@ static const struct devlink_ops mlxsw_devlink_ops = {
.sb_occ_port_pool_get = mlxsw_devlink_sb_occ_port_pool_get,
.sb_occ_tc_port_bind_get = mlxsw_devlink_sb_occ_tc_port_bind_get,
.info_get = mlxsw_devlink_info_get,
+ .flash_update = mlxsw_devlink_flash_update,
};
static int
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.h b/drivers/net/ethernet/mellanox/mlxsw/core.h
index e3832cb5bdda..a44ad0fb9477 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.h
@@ -284,6 +284,9 @@ struct mlxsw_driver {
unsigned int sb_index, u16 tc_index,
enum devlink_sb_pool_type pool_type,
u32 *p_cur, u32 *p_max);
+ int (*flash_update)(struct mlxsw_core *mlxsw_core,
+ const char *file_name, const char *component,
+ struct netlink_ext_ack *extack);
void (*txhdr_construct)(struct sk_buff *skb,
const struct mlxsw_tx_info *tx_info);
int (*resources_register)(struct mlxsw_core *mlxsw_core);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index dbb425717f5e..861a77538859 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -388,6 +388,27 @@ static int mlxsw_sp_fw_rev_validate(struct mlxsw_sp *mlxsw_sp)
return 0;
}
+static int mlxsw_sp_flash_update(struct mlxsw_core *mlxsw_core,
+ const char *file_name, const char *component,
+ struct netlink_ext_ack *extack)
+{
+ struct mlxsw_sp *mlxsw_sp = mlxsw_core_driver_priv(mlxsw_core);
+ const struct firmware *firmware;
+ int err;
+
+ if (component)
+ return -EOPNOTSUPP;
+
+ err = request_firmware_direct(&firmware, file_name,
+ mlxsw_sp->bus_info->dev);
+ if (err)
+ return err;
+ err = mlxsw_sp_firmware_flash(mlxsw_sp, firmware);
+ release_firmware(firmware);
+
+ return err;
+}
+
int mlxsw_sp_flow_counter_get(struct mlxsw_sp *mlxsw_sp,
unsigned int counter_index, u64 *packets,
u64 *bytes)
@@ -3155,31 +3176,6 @@ mlxsw_sp_port_set_link_ksettings(struct net_device *dev,
return 0;
}
-static int mlxsw_sp_flash_device(struct net_device *dev,
- struct ethtool_flash *flash)
-{
- struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(dev);
- struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
- const struct firmware *firmware;
- int err;
-
- if (flash->region != ETHTOOL_FLASH_ALL_REGIONS)
- return -EOPNOTSUPP;
-
- dev_hold(dev);
- rtnl_unlock();
-
- err = request_firmware_direct(&firmware, flash->data, &dev->dev);
- if (err)
- goto out;
- err = mlxsw_sp_firmware_flash(mlxsw_sp, firmware);
- release_firmware(firmware);
-out:
- rtnl_lock();
- dev_put(dev);
- return err;
-}
-
static int mlxsw_sp_get_module_info(struct net_device *netdev,
struct ethtool_modinfo *modinfo)
{
@@ -3220,7 +3216,6 @@ static const struct ethtool_ops mlxsw_sp_port_ethtool_ops = {
.get_sset_count = mlxsw_sp_port_get_sset_count,
.get_link_ksettings = mlxsw_sp_port_get_link_ksettings,
.set_link_ksettings = mlxsw_sp_port_set_link_ksettings,
- .flash_device = mlxsw_sp_flash_device,
.get_module_info = mlxsw_sp_get_module_info,
.get_module_eeprom = mlxsw_sp_get_module_eeprom,
};
@@ -4885,6 +4880,7 @@ static struct mlxsw_driver mlxsw_sp1_driver = {
.sb_occ_max_clear = mlxsw_sp_sb_occ_max_clear,
.sb_occ_port_pool_get = mlxsw_sp_sb_occ_port_pool_get,
.sb_occ_tc_port_bind_get = mlxsw_sp_sb_occ_tc_port_bind_get,
+ .flash_update = mlxsw_sp_flash_update,
.txhdr_construct = mlxsw_sp_txhdr_construct,
.resources_register = mlxsw_sp1_resources_register,
.kvd_sizes_get = mlxsw_sp_kvd_sizes_get,
@@ -4913,6 +4909,7 @@ static struct mlxsw_driver mlxsw_sp2_driver = {
.sb_occ_max_clear = mlxsw_sp_sb_occ_max_clear,
.sb_occ_port_pool_get = mlxsw_sp_sb_occ_port_pool_get,
.sb_occ_tc_port_bind_get = mlxsw_sp_sb_occ_tc_port_bind_get,
+ .flash_update = mlxsw_sp_flash_update,
.txhdr_construct = mlxsw_sp_txhdr_construct,
.resources_register = mlxsw_sp2_resources_register,
.params_register = mlxsw_sp2_params_register,
--
2.17.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [patch net-next 2/7] mlx5: Move firmware flash implementation to devlink
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
2019-05-23 9:45 ` [patch net-next 1/7] mlxsw: Move firmware flash implementation to devlink Jiri Pirko
@ 2019-05-23 9:45 ` Jiri Pirko
2019-05-23 9:45 ` [patch net-next 3/7] mlxfw: Propagate error messages through extack Jiri Pirko
` (9 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Jiri Pirko @ 2019-05-23 9:45 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
From: Jiri Pirko <jiri@mellanox.com>
Benefit from the devlink flash update implementation and ethtool
fallback to it and move firmware flash implementation there.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/en.h | 2 --
.../ethernet/mellanox/mlx5/core/en_ethtool.c | 35 -------------------
.../mellanox/mlx5/core/ipoib/ethtool.c | 9 -----
.../net/ethernet/mellanox/mlx5/core/main.c | 20 +++++++++++
4 files changed, 20 insertions(+), 46 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en.h b/drivers/net/ethernet/mellanox/mlx5/core/en.h
index 3a183d690e23..4e417dfe4ee5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en.h
@@ -1074,8 +1074,6 @@ u32 mlx5e_ethtool_get_rxfh_key_size(struct mlx5e_priv *priv);
u32 mlx5e_ethtool_get_rxfh_indir_size(struct mlx5e_priv *priv);
int mlx5e_ethtool_get_ts_info(struct mlx5e_priv *priv,
struct ethtool_ts_info *info);
-int mlx5e_ethtool_flash_device(struct mlx5e_priv *priv,
- struct ethtool_flash *flash);
void mlx5e_ethtool_get_pauseparam(struct mlx5e_priv *priv,
struct ethtool_pauseparam *pauseparam);
int mlx5e_ethtool_set_pauseparam(struct mlx5e_priv *priv,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
index dd764e0471f2..ea59097dd4f8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c
@@ -1867,40 +1867,6 @@ static u32 mlx5e_get_priv_flags(struct net_device *netdev)
return priv->channels.params.pflags;
}
-int mlx5e_ethtool_flash_device(struct mlx5e_priv *priv,
- struct ethtool_flash *flash)
-{
- struct mlx5_core_dev *mdev = priv->mdev;
- struct net_device *dev = priv->netdev;
- const struct firmware *fw;
- int err;
-
- if (flash->region != ETHTOOL_FLASH_ALL_REGIONS)
- return -EOPNOTSUPP;
-
- err = request_firmware_direct(&fw, flash->data, &dev->dev);
- if (err)
- return err;
-
- dev_hold(dev);
- rtnl_unlock();
-
- err = mlx5_firmware_flash(mdev, fw);
- release_firmware(fw);
-
- rtnl_lock();
- dev_put(dev);
- return err;
-}
-
-static int mlx5e_flash_device(struct net_device *dev,
- struct ethtool_flash *flash)
-{
- struct mlx5e_priv *priv = netdev_priv(dev);
-
- return mlx5e_ethtool_flash_device(priv, flash);
-}
-
#ifndef CONFIG_MLX5_EN_RXNFC
/* When CONFIG_MLX5_EN_RXNFC=n we only support ETHTOOL_GRXRINGS
* otherwise this function will be defined from en_fs_ethtool.c
@@ -1939,7 +1905,6 @@ const struct ethtool_ops mlx5e_ethtool_ops = {
#ifdef CONFIG_MLX5_EN_RXNFC
.set_rxnfc = mlx5e_set_rxnfc,
#endif
- .flash_device = mlx5e_flash_device,
.get_tunable = mlx5e_get_tunable,
.set_tunable = mlx5e_set_tunable,
.get_pauseparam = mlx5e_get_pauseparam,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c
index 90cb50fe17fd..ebd81f6b556e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/ipoib/ethtool.c
@@ -122,14 +122,6 @@ static int mlx5i_get_ts_info(struct net_device *netdev,
return mlx5e_ethtool_get_ts_info(priv, info);
}
-static int mlx5i_flash_device(struct net_device *netdev,
- struct ethtool_flash *flash)
-{
- struct mlx5e_priv *priv = mlx5i_epriv(netdev);
-
- return mlx5e_ethtool_flash_device(priv, flash);
-}
-
enum mlx5_ptys_width {
MLX5_PTYS_WIDTH_1X = 1 << 0,
MLX5_PTYS_WIDTH_2X = 1 << 1,
@@ -241,7 +233,6 @@ const struct ethtool_ops mlx5i_ethtool_ops = {
.get_ethtool_stats = mlx5i_get_ethtool_stats,
.get_ringparam = mlx5i_get_ringparam,
.set_ringparam = mlx5i_set_ringparam,
- .flash_device = mlx5i_flash_device,
.get_channels = mlx5i_get_channels,
.set_channels = mlx5i_set_channels,
.get_coalesce = mlx5i_get_coalesce,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 61fa1d162d28..36acd0267a13 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1210,6 +1210,25 @@ static int mlx5_unload_one(struct mlx5_core_dev *dev, bool cleanup)
return err;
}
+static int mlx5_devlink_flash_update(struct devlink *devlink,
+ const char *file_name,
+ const char *component,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_core_dev *dev = devlink_priv(devlink);
+ const struct firmware *fw;
+ int err;
+
+ if (component)
+ return -EOPNOTSUPP;
+
+ err = request_firmware_direct(&fw, file_name, &dev->pdev->dev);
+ if (err)
+ return err;
+
+ return mlx5_firmware_flash(dev, fw);
+}
+
static const struct devlink_ops mlx5_devlink_ops = {
#ifdef CONFIG_MLX5_ESWITCH
.eswitch_mode_set = mlx5_devlink_eswitch_mode_set,
@@ -1219,6 +1238,7 @@ static const struct devlink_ops mlx5_devlink_ops = {
.eswitch_encap_mode_set = mlx5_devlink_eswitch_encap_mode_set,
.eswitch_encap_mode_get = mlx5_devlink_eswitch_encap_mode_get,
#endif
+ .flash_update = mlx5_devlink_flash_update,
};
static int mlx5_mdev_init(struct mlx5_core_dev *dev, int profile_idx)
--
2.17.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [patch net-next 3/7] mlxfw: Propagate error messages through extack
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
2019-05-23 9:45 ` [patch net-next 1/7] mlxsw: Move firmware flash implementation to devlink Jiri Pirko
2019-05-23 9:45 ` [patch net-next 2/7] mlx5: " Jiri Pirko
@ 2019-05-23 9:45 ` Jiri Pirko
2019-05-23 15:19 ` David Ahern
2019-05-23 9:45 ` [patch net-next 4/7] devlink: allow driver to update progress of flash update Jiri Pirko
` (8 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Jiri Pirko @ 2019-05-23 9:45 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
From: Jiri Pirko <jiri@mellanox.com>
Currently the error messages are printed to dmesg. Propagate them also
to directly to user doing the flashing through extack.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlx5/core/fw.c | 6 ++--
.../net/ethernet/mellanox/mlx5/core/main.c | 2 +-
.../ethernet/mellanox/mlx5/core/mlx5_core.h | 3 +-
drivers/net/ethernet/mellanox/mlxfw/mlxfw.h | 7 ++--
.../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c | 33 +++++++++++++------
.../net/ethernet/mellanox/mlxsw/spectrum.c | 10 +++---
6 files changed, 41 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw.c b/drivers/net/ethernet/mellanox/mlx5/core/fw.c
index 1ab6f7e3bec6..e8fedb307b2c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw.c
@@ -552,7 +552,8 @@ static const struct mlxfw_dev_ops mlx5_mlxfw_dev_ops = {
};
int mlx5_firmware_flash(struct mlx5_core_dev *dev,
- const struct firmware *firmware)
+ const struct firmware *firmware,
+ struct netlink_ext_ack *extack)
{
struct mlx5_mlxfw_dev mlx5_mlxfw_dev = {
.mlxfw_dev = {
@@ -571,5 +572,6 @@ int mlx5_firmware_flash(struct mlx5_core_dev *dev,
return -EOPNOTSUPP;
}
- return mlxfw_firmware_flash(&mlx5_mlxfw_dev.mlxfw_dev, firmware);
+ return mlxfw_firmware_flash(&mlx5_mlxfw_dev.mlxfw_dev,
+ firmware, extack);
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 36acd0267a13..52fcfc17fcc6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1226,7 +1226,7 @@ static int mlx5_devlink_flash_update(struct devlink *devlink,
if (err)
return err;
- return mlx5_firmware_flash(dev, fw);
+ return mlx5_firmware_flash(dev, fw, extack);
}
static const struct devlink_ops mlx5_devlink_ops = {
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
index 22e69d4813e4..d4dd8c1ae55c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
@@ -184,7 +184,8 @@ int mlx5_set_mtppse(struct mlx5_core_dev *mdev, u8 pin, u8 arm, u8 mode);
MLX5_CAP_MCAM_FEATURE((mdev), mtpps_fs) && \
MLX5_CAP_MCAM_FEATURE((mdev), mtpps_enh_out_per_adj))
-int mlx5_firmware_flash(struct mlx5_core_dev *dev, const struct firmware *fw);
+int mlx5_firmware_flash(struct mlx5_core_dev *dev, const struct firmware *fw,
+ struct netlink_ext_ack *extack);
void mlx5e_init(void);
void mlx5e_cleanup(void);
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h b/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
index 14c0c62f8e73..83286b90593f 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
@@ -5,6 +5,7 @@
#define _MLXFW_H
#include <linux/firmware.h>
+#include <linux/netlink.h>
enum mlxfw_fsm_state {
MLXFW_FSM_STATE_IDLE,
@@ -67,11 +68,13 @@ struct mlxfw_dev {
#if IS_REACHABLE(CONFIG_MLXFW)
int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
- const struct firmware *firmware);
+ const struct firmware *firmware,
+ struct netlink_ext_ack *extack);
#else
static inline
int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
- const struct firmware *firmware)
+ const struct firmware *firmware,
+ struct netlink_ext_ack *extack)
{
return -EOPNOTSUPP;
}
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
index 240c027e5f07..588d9a9c08c9 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
@@ -40,7 +40,8 @@ static const char * const mlxfw_fsm_state_err_str[] = {
};
static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
- enum mlxfw_fsm_state fsm_state)
+ enum mlxfw_fsm_state fsm_state,
+ struct netlink_ext_ack *extack)
{
enum mlxfw_fsm_state_err fsm_state_err;
enum mlxfw_fsm_state curr_fsm_state;
@@ -57,11 +58,13 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK) {
pr_err("Firmware flash failed: %s\n",
mlxfw_fsm_state_err_str[fsm_state_err]);
+ NL_SET_ERR_MSG_MOD(extack, "Firmware flash failed");
return -EINVAL;
}
if (curr_fsm_state != fsm_state) {
if (--times == 0) {
pr_err("Timeout reached on FSM state change");
+ NL_SET_ERR_MSG_MOD(extack, "Timeout reached on FSM state change");
return -ETIMEDOUT;
}
msleep(MLXFW_FSM_STATE_WAIT_CYCLE_MS);
@@ -76,7 +79,8 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
u32 fwhandle,
- struct mlxfw_mfa2_component *comp)
+ struct mlxfw_mfa2_component *comp,
+ struct netlink_ext_ack *extack)
{
u16 comp_max_write_size;
u8 comp_align_bits;
@@ -96,6 +100,7 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
if (comp->data_size > comp_max_size) {
pr_err("Component %d is of size %d which is bigger than limit %d\n",
comp->index, comp->data_size, comp_max_size);
+ NL_SET_ERR_MSG_MOD(extack, "Component is which is bigger than limit");
return -EINVAL;
}
@@ -110,7 +115,7 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
return err;
err = mlxfw_fsm_state_wait(mlxfw_dev, fwhandle,
- MLXFW_FSM_STATE_DOWNLOAD);
+ MLXFW_FSM_STATE_DOWNLOAD, extack);
if (err)
goto err_out;
@@ -134,7 +139,8 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
if (err)
goto err_out;
- err = mlxfw_fsm_state_wait(mlxfw_dev, fwhandle, MLXFW_FSM_STATE_LOCKED);
+ err = mlxfw_fsm_state_wait(mlxfw_dev, fwhandle,
+ MLXFW_FSM_STATE_LOCKED, extack);
if (err)
goto err_out;
return 0;
@@ -145,7 +151,8 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
}
static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
- struct mlxfw_mfa2_file *mfa2_file)
+ struct mlxfw_mfa2_file *mfa2_file,
+ struct netlink_ext_ack *extack)
{
u32 component_count;
int err;
@@ -156,6 +163,7 @@ static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
&component_count);
if (err) {
pr_err("Could not find device PSID in MFA2 file\n");
+ NL_SET_ERR_MSG_MOD(extack, "Could not find device PSID in MFA2 file");
return err;
}
@@ -168,7 +176,7 @@ static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
return PTR_ERR(comp);
pr_info("Flashing component type %d\n", comp->index);
- err = mlxfw_flash_component(mlxfw_dev, fwhandle, comp);
+ err = mlxfw_flash_component(mlxfw_dev, fwhandle, comp, extack);
mlxfw_mfa2_file_component_put(comp);
if (err)
return err;
@@ -177,7 +185,8 @@ static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
}
int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
- const struct firmware *firmware)
+ const struct firmware *firmware,
+ struct netlink_ext_ack *extack)
{
struct mlxfw_mfa2_file *mfa2_file;
u32 fwhandle;
@@ -185,6 +194,7 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
if (!mlxfw_mfa2_check(firmware)) {
pr_err("Firmware file is not MFA2\n");
+ NL_SET_ERR_MSG_MOD(extack, "Firmware file is not MFA2");
return -EINVAL;
}
@@ -196,15 +206,16 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
err = mlxfw_dev->ops->fsm_lock(mlxfw_dev, &fwhandle);
if (err) {
pr_err("Could not lock the firmware FSM\n");
+ NL_SET_ERR_MSG_MOD(extack, "Could not lock the firmware FSM");
goto err_fsm_lock;
}
err = mlxfw_fsm_state_wait(mlxfw_dev, fwhandle,
- MLXFW_FSM_STATE_LOCKED);
+ MLXFW_FSM_STATE_LOCKED, extack);
if (err)
goto err_state_wait_idle_to_locked;
- err = mlxfw_flash_components(mlxfw_dev, fwhandle, mfa2_file);
+ err = mlxfw_flash_components(mlxfw_dev, fwhandle, mfa2_file, extack);
if (err)
goto err_flash_components;
@@ -212,10 +223,12 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
err = mlxfw_dev->ops->fsm_activate(mlxfw_dev, fwhandle);
if (err) {
pr_err("Could not activate the downloaded image\n");
+ NL_SET_ERR_MSG_MOD(extack, "Could not activate the downloaded image");
goto err_fsm_activate;
}
- err = mlxfw_fsm_state_wait(mlxfw_dev, fwhandle, MLXFW_FSM_STATE_LOCKED);
+ err = mlxfw_fsm_state_wait(mlxfw_dev, fwhandle,
+ MLXFW_FSM_STATE_LOCKED, extack);
if (err)
goto err_state_wait_activate_to_locked;
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 861a77538859..639bb5778ff3 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -307,7 +307,8 @@ static const struct mlxfw_dev_ops mlxsw_sp_mlxfw_dev_ops = {
};
static int mlxsw_sp_firmware_flash(struct mlxsw_sp *mlxsw_sp,
- const struct firmware *firmware)
+ const struct firmware *firmware,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_mlxfw_dev mlxsw_sp_mlxfw_dev = {
.mlxfw_dev = {
@@ -320,7 +321,8 @@ static int mlxsw_sp_firmware_flash(struct mlxsw_sp *mlxsw_sp,
int err;
mlxsw_core_fw_flash_start(mlxsw_sp->core);
- err = mlxfw_firmware_flash(&mlxsw_sp_mlxfw_dev.mlxfw_dev, firmware);
+ err = mlxfw_firmware_flash(&mlxsw_sp_mlxfw_dev.mlxfw_dev,
+ firmware, extack);
mlxsw_core_fw_flash_end(mlxsw_sp->core);
return err;
@@ -374,7 +376,7 @@ static int mlxsw_sp_fw_rev_validate(struct mlxsw_sp *mlxsw_sp)
return err;
}
- err = mlxsw_sp_firmware_flash(mlxsw_sp, firmware);
+ err = mlxsw_sp_firmware_flash(mlxsw_sp, firmware, NULL);
release_firmware(firmware);
if (err)
dev_err(mlxsw_sp->bus_info->dev, "Could not upgrade firmware\n");
@@ -403,7 +405,7 @@ static int mlxsw_sp_flash_update(struct mlxsw_core *mlxsw_core,
mlxsw_sp->bus_info->dev);
if (err)
return err;
- err = mlxsw_sp_firmware_flash(mlxsw_sp, firmware);
+ err = mlxsw_sp_firmware_flash(mlxsw_sp, firmware, extack);
release_firmware(firmware);
return err;
--
2.17.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [patch net-next 3/7] mlxfw: Propagate error messages through extack
2019-05-23 9:45 ` [patch net-next 3/7] mlxfw: Propagate error messages through extack Jiri Pirko
@ 2019-05-23 15:19 ` David Ahern
2019-05-23 18:30 ` Saeed Mahameed
2019-05-24 8:11 ` Jiri Pirko
0 siblings, 2 replies; 29+ messages in thread
From: David Ahern @ 2019-05-23 15:19 UTC (permalink / raw)
To: Jiri Pirko, netdev; +Cc: davem, mlxsw, jakub.kicinski, sthemmin, saeedm, leon
On 5/23/19 3:45 AM, Jiri Pirko wrote:
> @@ -57,11 +58,13 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
> if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK) {
> pr_err("Firmware flash failed: %s\n",
> mlxfw_fsm_state_err_str[fsm_state_err]);
> + NL_SET_ERR_MSG_MOD(extack, "Firmware flash failed");
> return -EINVAL;
> }
> if (curr_fsm_state != fsm_state) {
> if (--times == 0) {
> pr_err("Timeout reached on FSM state change");
> + NL_SET_ERR_MSG_MOD(extack, "Timeout reached on FSM state change");
FSM? Is the meaning obvious to users?
> return -ETIMEDOUT;
> }
> msleep(MLXFW_FSM_STATE_WAIT_CYCLE_MS);
> @@ -76,7 +79,8 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
>
> static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
> u32 fwhandle,
> - struct mlxfw_mfa2_component *comp)
> + struct mlxfw_mfa2_component *comp,
> + struct netlink_ext_ack *extack)
> {
> u16 comp_max_write_size;
> u8 comp_align_bits;
> @@ -96,6 +100,7 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
> if (comp->data_size > comp_max_size) {
> pr_err("Component %d is of size %d which is bigger than limit %d\n",
> comp->index, comp->data_size, comp_max_size);
> + NL_SET_ERR_MSG_MOD(extack, "Component is which is bigger than limit");
Need to drop 'is which'.
...
> @@ -156,6 +163,7 @@ static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
> &component_count);
> if (err) {
> pr_err("Could not find device PSID in MFA2 file\n");
> + NL_SET_ERR_MSG_MOD(extack, "Could not find device PSID in MFA2 file");
same here, is PSID understood by user?
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [patch net-next 3/7] mlxfw: Propagate error messages through extack
2019-05-23 15:19 ` David Ahern
@ 2019-05-23 18:30 ` Saeed Mahameed
2019-05-24 8:11 ` Jiri Pirko
1 sibling, 0 replies; 29+ messages in thread
From: Saeed Mahameed @ 2019-05-23 18:30 UTC (permalink / raw)
To: dsahern, jiri, netdev; +Cc: sthemmin, davem, mlxsw, jakub.kicinski, leon
On Thu, 2019-05-23 at 09:19 -0600, David Ahern wrote:
> On 5/23/19 3:45 AM, Jiri Pirko wrote:
> > @@ -57,11 +58,13 @@ static int mlxfw_fsm_state_wait(struct
> > mlxfw_dev *mlxfw_dev, u32 fwhandle,
> > if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK) {
> > pr_err("Firmware flash failed: %s\n",
> > mlxfw_fsm_state_err_str[fsm_state_err]);
> > + NL_SET_ERR_MSG_MOD(extack, "Firmware flash failed");
> > return -EINVAL;
> > }
> > if (curr_fsm_state != fsm_state) {
> > if (--times == 0) {
> > pr_err("Timeout reached on FSM state change");
> > + NL_SET_ERR_MSG_MOD(extack, "Timeout reached on
> > FSM state change");
>
> FSM? Is the meaning obvious to users?
These messages are vendor driver generated, how can we make them user
friendly, yet expose vendor specific information that only the vendor
can understand .. ? I think it is legit to have vendor specific terms
in extack which is generated by drivers..
>
> > return -ETIMEDOUT;
> > }
> > msleep(MLXFW_FSM_STATE_WAIT_CYCLE_MS);
> > @@ -76,7 +79,8 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev
> > *mlxfw_dev, u32 fwhandle,
> >
> > static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
> > u32 fwhandle,
> > - struct mlxfw_mfa2_component *comp)
> > + struct mlxfw_mfa2_component *comp,
> > + struct netlink_ext_ack *extack)
> > {
> > u16 comp_max_write_size;
> > u8 comp_align_bits;
> > @@ -96,6 +100,7 @@ static int mlxfw_flash_component(struct
> > mlxfw_dev *mlxfw_dev,
> > if (comp->data_size > comp_max_size) {
> > pr_err("Component %d is of size %d which is bigger than
> > limit %d\n",
> > comp->index, comp->data_size, comp_max_size);
> > + NL_SET_ERR_MSG_MOD(extack, "Component is which is
> > bigger than limit");
>
> Need to drop 'is which'.
>
>
> ...
>
> > @@ -156,6 +163,7 @@ static int mlxfw_flash_components(struct
> > mlxfw_dev *mlxfw_dev, u32 fwhandle,
> > &component_count);
> > if (err) {
> > pr_err("Could not find device PSID in MFA2 file\n");
> > + NL_SET_ERR_MSG_MOD(extack, "Could not find device PSID
> > in MFA2 file");
>
> same here, is PSID understood by user?
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [patch net-next 3/7] mlxfw: Propagate error messages through extack
2019-05-23 15:19 ` David Ahern
2019-05-23 18:30 ` Saeed Mahameed
@ 2019-05-24 8:11 ` Jiri Pirko
2019-05-24 15:54 ` Jakub Kicinski
1 sibling, 1 reply; 29+ messages in thread
From: Jiri Pirko @ 2019-05-24 8:11 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, davem, mlxsw, jakub.kicinski, sthemmin, saeedm, leon
Thu, May 23, 2019 at 05:19:46PM CEST, dsahern@gmail.com wrote:
>On 5/23/19 3:45 AM, Jiri Pirko wrote:
>> @@ -57,11 +58,13 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
>> if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK) {
>> pr_err("Firmware flash failed: %s\n",
>> mlxfw_fsm_state_err_str[fsm_state_err]);
>> + NL_SET_ERR_MSG_MOD(extack, "Firmware flash failed");
>> return -EINVAL;
>> }
>> if (curr_fsm_state != fsm_state) {
>> if (--times == 0) {
>> pr_err("Timeout reached on FSM state change");
>> + NL_SET_ERR_MSG_MOD(extack, "Timeout reached on FSM state change");
>
>FSM? Is the meaning obvious to users?
It is specific to mlx drivers. But I think it is valuable to have
driver-specific terms in driver speficic extack messages.
>
>> return -ETIMEDOUT;
>> }
>> msleep(MLXFW_FSM_STATE_WAIT_CYCLE_MS);
>> @@ -76,7 +79,8 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
>>
>> static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
>> u32 fwhandle,
>> - struct mlxfw_mfa2_component *comp)
>> + struct mlxfw_mfa2_component *comp,
>> + struct netlink_ext_ack *extack)
>> {
>> u16 comp_max_write_size;
>> u8 comp_align_bits;
>> @@ -96,6 +100,7 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
>> if (comp->data_size > comp_max_size) {
>> pr_err("Component %d is of size %d which is bigger than limit %d\n",
>> comp->index, comp->data_size, comp_max_size);
>> + NL_SET_ERR_MSG_MOD(extack, "Component is which is bigger than limit");
>
>Need to drop 'is which'.
Will do.
>
>
>...
>
>> @@ -156,6 +163,7 @@ static int mlxfw_flash_components(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
>> &component_count);
>> if (err) {
>> pr_err("Could not find device PSID in MFA2 file\n");
>> + NL_SET_ERR_MSG_MOD(extack, "Could not find device PSID in MFA2 file");
>
>same here, is PSID understood by user?
PSID is actually exposed in "devlink dev info" for mlxsw.
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [patch net-next 3/7] mlxfw: Propagate error messages through extack
2019-05-24 8:11 ` Jiri Pirko
@ 2019-05-24 15:54 ` Jakub Kicinski
2019-05-24 22:26 ` Jiri Pirko
0 siblings, 1 reply; 29+ messages in thread
From: Jakub Kicinski @ 2019-05-24 15:54 UTC (permalink / raw)
To: Jiri Pirko; +Cc: David Ahern, netdev, davem, mlxsw, sthemmin, saeedm, leon
On Fri, 24 May 2019 10:11:10 +0200, Jiri Pirko wrote:
> Thu, May 23, 2019 at 05:19:46PM CEST, dsahern@gmail.com wrote:
> >On 5/23/19 3:45 AM, Jiri Pirko wrote:
> >> @@ -57,11 +58,13 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
> >> if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK) {
> >> pr_err("Firmware flash failed: %s\n",
> >> mlxfw_fsm_state_err_str[fsm_state_err]);
> >> + NL_SET_ERR_MSG_MOD(extack, "Firmware flash failed");
> >> return -EINVAL;
> >> }
> >> if (curr_fsm_state != fsm_state) {
> >> if (--times == 0) {
> >> pr_err("Timeout reached on FSM state change");
> >> + NL_SET_ERR_MSG_MOD(extack, "Timeout reached on FSM state change");
> >
> >FSM? Is the meaning obvious to users?
>
> It is specific to mlx drivers.
What does it stand for? Isn't it just Finite State Machine?
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [patch net-next 3/7] mlxfw: Propagate error messages through extack
2019-05-24 15:54 ` Jakub Kicinski
@ 2019-05-24 22:26 ` Jiri Pirko
2019-05-25 0:08 ` Jakub Kicinski
0 siblings, 1 reply; 29+ messages in thread
From: Jiri Pirko @ 2019-05-24 22:26 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: David Ahern, netdev, davem, mlxsw, sthemmin, saeedm, leon
Fri, May 24, 2019 at 05:54:46PM CEST, jakub.kicinski@netronome.com wrote:
>On Fri, 24 May 2019 10:11:10 +0200, Jiri Pirko wrote:
>> Thu, May 23, 2019 at 05:19:46PM CEST, dsahern@gmail.com wrote:
>> >On 5/23/19 3:45 AM, Jiri Pirko wrote:
>> >> @@ -57,11 +58,13 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
>> >> if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK) {
>> >> pr_err("Firmware flash failed: %s\n",
>> >> mlxfw_fsm_state_err_str[fsm_state_err]);
>> >> + NL_SET_ERR_MSG_MOD(extack, "Firmware flash failed");
>> >> return -EINVAL;
>> >> }
>> >> if (curr_fsm_state != fsm_state) {
>> >> if (--times == 0) {
>> >> pr_err("Timeout reached on FSM state change");
>> >> + NL_SET_ERR_MSG_MOD(extack, "Timeout reached on FSM state change");
>> >
>> >FSM? Is the meaning obvious to users?
>>
>> It is specific to mlx drivers.
>
>What does it stand for? Isn't it just Finite State Machine?
I believe so.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [patch net-next 3/7] mlxfw: Propagate error messages through extack
2019-05-24 22:26 ` Jiri Pirko
@ 2019-05-25 0:08 ` Jakub Kicinski
2019-05-27 13:33 ` Jiri Pirko
0 siblings, 1 reply; 29+ messages in thread
From: Jakub Kicinski @ 2019-05-25 0:08 UTC (permalink / raw)
To: Jiri Pirko, David Ahern; +Cc: netdev, davem, mlxsw, sthemmin, saeedm, leon
On Sat, 25 May 2019 00:26:35 +0200, Jiri Pirko wrote:
> Fri, May 24, 2019 at 05:54:46PM CEST, jakub.kicinski@netronome.com wrote:
> >On Fri, 24 May 2019 10:11:10 +0200, Jiri Pirko wrote:
> >> Thu, May 23, 2019 at 05:19:46PM CEST, dsahern@gmail.com wrote:
> >> >On 5/23/19 3:45 AM, Jiri Pirko wrote:
> >> >> @@ -57,11 +58,13 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
> >> >> if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK) {
> >> >> pr_err("Firmware flash failed: %s\n",
> >> >> mlxfw_fsm_state_err_str[fsm_state_err]);
> >> >> + NL_SET_ERR_MSG_MOD(extack, "Firmware flash failed");
> >> >> return -EINVAL;
> >> >> }
> >> >> if (curr_fsm_state != fsm_state) {
> >> >> if (--times == 0) {
> >> >> pr_err("Timeout reached on FSM state change");
> >> >> + NL_SET_ERR_MSG_MOD(extack, "Timeout reached on FSM state change");
> >> >
> >> >FSM? Is the meaning obvious to users?
> >>
> >> It is specific to mlx drivers.
> >
> >What does it stand for? Isn't it just Finite State Machine?
>
> I believe so.
In which case it doesn't really add much, no? I second David's request
to make the messages as easy to understand as possible.
PSID for better or worse I have previously capitulated on, so I guess
the ship has indeed sailed there :)
$ grep -A4 psid -- Documentation/networking/devlink-info-versions.rst
fw.psid
=======
Unique identifier of the firmware parameter set.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [patch net-next 3/7] mlxfw: Propagate error messages through extack
2019-05-25 0:08 ` Jakub Kicinski
@ 2019-05-27 13:33 ` Jiri Pirko
0 siblings, 0 replies; 29+ messages in thread
From: Jiri Pirko @ 2019-05-27 13:33 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: David Ahern, netdev, davem, mlxsw, sthemmin, saeedm, leon
Sat, May 25, 2019 at 02:08:52AM CEST, jakub.kicinski@netronome.com wrote:
>On Sat, 25 May 2019 00:26:35 +0200, Jiri Pirko wrote:
>> Fri, May 24, 2019 at 05:54:46PM CEST, jakub.kicinski@netronome.com wrote:
>> >On Fri, 24 May 2019 10:11:10 +0200, Jiri Pirko wrote:
>> >> Thu, May 23, 2019 at 05:19:46PM CEST, dsahern@gmail.com wrote:
>> >> >On 5/23/19 3:45 AM, Jiri Pirko wrote:
>> >> >> @@ -57,11 +58,13 @@ static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
>> >> >> if (fsm_state_err != MLXFW_FSM_STATE_ERR_OK) {
>> >> >> pr_err("Firmware flash failed: %s\n",
>> >> >> mlxfw_fsm_state_err_str[fsm_state_err]);
>> >> >> + NL_SET_ERR_MSG_MOD(extack, "Firmware flash failed");
>> >> >> return -EINVAL;
>> >> >> }
>> >> >> if (curr_fsm_state != fsm_state) {
>> >> >> if (--times == 0) {
>> >> >> pr_err("Timeout reached on FSM state change");
>> >> >> + NL_SET_ERR_MSG_MOD(extack, "Timeout reached on FSM state change");
>> >> >
>> >> >FSM? Is the meaning obvious to users?
>> >>
>> >> It is specific to mlx drivers.
>> >
>> >What does it stand for? Isn't it just Finite State Machine?
>>
>> I believe so.
>
>In which case it doesn't really add much, no? I second David's request
>to make the messages as easy to understand as possible.
Well, FSM is something that is used in the code and known. I would
change it to "finite state machine" (which I'm still not sure it
really is) but I don't believe that would bring more info to the user.
Well, nothing. On contrary, a MLX engineer might get confused if customer
sends him the message, because he is used to "FSM" :)
Same with "MFA2" in the other message. I only know it is the format
of the binary, have no clue what it actually stands for (other than
it is version 2).
>
>PSID for better or worse I have previously capitulated on, so I guess
>the ship has indeed sailed there :)
>
>$ grep -A4 psid -- Documentation/networking/devlink-info-versions.rst
>fw.psid
>=======
>
>Unique identifier of the firmware parameter set.
^ permalink raw reply [flat|nested] 29+ messages in thread
* [patch net-next 4/7] devlink: allow driver to update progress of flash update
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
` (2 preceding siblings ...)
2019-05-23 9:45 ` [patch net-next 3/7] mlxfw: Propagate error messages through extack Jiri Pirko
@ 2019-05-23 9:45 ` Jiri Pirko
2019-05-23 17:39 ` Jakub Kicinski
2019-05-23 9:45 ` [patch net-next 5/7] mlxfw: Introduce status_notify op and call it to notify about the status Jiri Pirko
` (7 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Jiri Pirko @ 2019-05-23 9:45 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
From: Jiri Pirko <jiri@mellanox.com>
Introduce a function to be called from drivers during flash. It sends
notification to userspace about flash update progress.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/devlink.h | 8 +++
include/uapi/linux/devlink.h | 5 ++
net/core/devlink.c | 102 +++++++++++++++++++++++++++++++++++
3 files changed, 115 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 151eb930d329..8f65356132be 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -741,6 +741,14 @@ void
devlink_health_reporter_state_update(struct devlink_health_reporter *reporter,
enum devlink_health_reporter_state state);
+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,
+ unsigned long done,
+ unsigned long total);
+
#if IS_ENABLED(CONFIG_NET_DEVLINK)
void devlink_compat_running_version(struct net_device *dev,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 5bb4ea67d84f..5287b42c181f 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -104,6 +104,8 @@ enum devlink_command {
DEVLINK_CMD_HEALTH_REPORTER_DUMP_CLEAR,
DEVLINK_CMD_FLASH_UPDATE,
+ DEVLINK_CMD_FLASH_UPDATE_END, /* notification only */
+ DEVLINK_CMD_FLASH_UPDATE_STATUS, /* notification only */
/* add new commands above here */
__DEVLINK_CMD_MAX,
@@ -331,6 +333,9 @@ enum devlink_attr {
DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME, /* string */
DEVLINK_ATTR_FLASH_UPDATE_COMPONENT, /* string */
+ DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG, /* string */
+ DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE, /* u64 */
+ DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL, /* u64 */
/* add new attributes above here, update the policy in devlink.c */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 9716a7f382cb..963178d32dda 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2673,6 +2673,108 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
return devlink->ops->reload(devlink, info->extack);
}
+static int devlink_nl_flash_update_fill(struct sk_buff *msg,
+ struct devlink *devlink,
+ enum devlink_command cmd,
+ const char *status_msg,
+ const char *component,
+ unsigned long done, unsigned long total)
+{
+ void *hdr;
+
+ hdr = genlmsg_put(msg, 0, 0, &devlink_nl_family, 0, cmd);
+ if (!hdr)
+ return -EMSGSIZE;
+
+ if (devlink_nl_put_handle(msg, devlink))
+ goto nla_put_failure;
+
+ if (cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS)
+ goto out;
+
+ if (status_msg &&
+ nla_put_string(msg, DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG,
+ status_msg))
+ goto nla_put_failure;
+ if (component &&
+ nla_put_string(msg, DEVLINK_ATTR_FLASH_UPDATE_COMPONENT,
+ component))
+ goto nla_put_failure;
+ if (nla_put_u64_64bit(msg, DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE,
+ done, DEVLINK_ATTR_PAD))
+ goto nla_put_failure;
+ if (nla_put_u64_64bit(msg, DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL,
+ total, DEVLINK_ATTR_PAD))
+ goto nla_put_failure;
+
+out:
+ genlmsg_end(msg, hdr);
+ return 0;
+
+nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+ return -EMSGSIZE;
+}
+
+static void __devlink_flash_update_notify(struct devlink *devlink,
+ enum devlink_command cmd,
+ const char *status_msg,
+ const char *component,
+ unsigned long done,
+ unsigned long total)
+{
+ struct sk_buff *msg;
+ int err;
+
+ WARN_ON(cmd != DEVLINK_CMD_FLASH_UPDATE &&
+ cmd != DEVLINK_CMD_FLASH_UPDATE_END &&
+ cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS);
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return;
+
+ err = devlink_nl_flash_update_fill(msg, devlink, cmd, status_msg,
+ component, done, total);
+ if (err)
+ goto out_free_msg;
+
+ genlmsg_multicast_netns(&devlink_nl_family, devlink_net(devlink),
+ msg, 0, DEVLINK_MCGRP_CONFIG, GFP_KERNEL);
+ return;
+
+out_free_msg:
+ nlmsg_free(msg);
+}
+
+void devlink_flash_update_begin_notify(struct devlink *devlink)
+{
+ __devlink_flash_update_notify(devlink,
+ DEVLINK_CMD_FLASH_UPDATE,
+ NULL, NULL, 0, 0);
+}
+EXPORT_SYMBOL_GPL(devlink_flash_update_begin_notify);
+
+void devlink_flash_update_end_notify(struct devlink *devlink)
+{
+ __devlink_flash_update_notify(devlink,
+ DEVLINK_CMD_FLASH_UPDATE_END,
+ NULL, NULL, 0, 0);
+}
+EXPORT_SYMBOL_GPL(devlink_flash_update_end_notify);
+
+void devlink_flash_update_status_notify(struct devlink *devlink,
+ const char *status_msg,
+ const char *component,
+ unsigned long done,
+ unsigned long total)
+{
+ __devlink_flash_update_notify(devlink,
+ DEVLINK_CMD_FLASH_UPDATE_STATUS,
+ status_msg, component, done, total);
+}
+EXPORT_SYMBOL_GPL(devlink_flash_update_status_notify);
+
static int devlink_nl_cmd_flash_update(struct sk_buff *skb,
struct genl_info *info)
{
--
2.17.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [patch net-next 5/7] mlxfw: Introduce status_notify op and call it to notify about the status
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
` (3 preceding siblings ...)
2019-05-23 9:45 ` [patch net-next 4/7] devlink: allow driver to update progress of flash update Jiri Pirko
@ 2019-05-23 9:45 ` Jiri Pirko
2019-05-23 9:45 ` [patch net-next 6/7] mlxsw: Implement flash update status notifications Jiri Pirko
` (6 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Jiri Pirko @ 2019-05-23 9:45 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
From: Jiri Pirko <jiri@mellanox.com>
Add new op status_notify which is called to update the user about
flashing status.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxfw/mlxfw.h | 4 ++++
.../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c | 24 +++++++++++++++++++
2 files changed, 28 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h b/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
index 83286b90593f..c50e74ab02c4 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw.h
@@ -58,6 +58,10 @@ struct mlxfw_dev_ops {
void (*fsm_cancel)(struct mlxfw_dev *mlxfw_dev, u32 fwhandle);
void (*fsm_release)(struct mlxfw_dev *mlxfw_dev, u32 fwhandle);
+
+ void (*status_notify)(struct mlxfw_dev *mlxfw_dev,
+ const char *msg, const char *comp_name,
+ u32 done_bytes, u32 total_bytes);
};
struct mlxfw_dev {
diff --git a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
index 588d9a9c08c9..6a18ec05181a 100644
--- a/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
+++ b/drivers/net/ethernet/mellanox/mlxfw/mlxfw_fsm.c
@@ -39,6 +39,16 @@ static const char * const mlxfw_fsm_state_err_str[] = {
"unknown error"
};
+static void mlxfw_status_notify(struct mlxfw_dev *mlxfw_dev,
+ const char *msg, const char *comp_name,
+ u32 done_bytes, u32 total_bytes)
+{
+ if (!mlxfw_dev->ops->status_notify)
+ return;
+ mlxfw_dev->ops->status_notify(mlxfw_dev, msg, comp_name,
+ done_bytes, total_bytes);
+}
+
static int mlxfw_fsm_state_wait(struct mlxfw_dev *mlxfw_dev, u32 fwhandle,
enum mlxfw_fsm_state fsm_state,
struct netlink_ext_ack *extack)
@@ -85,11 +95,14 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
u16 comp_max_write_size;
u8 comp_align_bits;
u32 comp_max_size;
+ char comp_name[8];
u16 block_size;
u8 *block_ptr;
u32 offset;
int err;
+ sprintf(comp_name, "%u", comp->index);
+
err = mlxfw_dev->ops->component_query(mlxfw_dev, comp->index,
&comp_max_size, &comp_align_bits,
&comp_max_write_size);
@@ -108,6 +121,7 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
comp_align_bits);
pr_debug("Component update\n");
+ mlxfw_status_notify(mlxfw_dev, "Updating component", comp_name, 0, 0);
err = mlxfw_dev->ops->fsm_component_update(mlxfw_dev, fwhandle,
comp->index,
comp->data_size);
@@ -120,6 +134,8 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
goto err_out;
pr_debug("Component download\n");
+ mlxfw_status_notify(mlxfw_dev, "Downloading component",
+ comp_name, 0, comp->data_size);
for (offset = 0;
offset < MLXFW_ALIGN_UP(comp->data_size, comp_align_bits);
offset += comp_max_write_size) {
@@ -131,9 +147,13 @@ static int mlxfw_flash_component(struct mlxfw_dev *mlxfw_dev,
offset);
if (err)
goto err_out;
+ mlxfw_status_notify(mlxfw_dev, "Downloading component",
+ comp_name, offset + block_size,
+ comp->data_size);
}
pr_debug("Component verify\n");
+ mlxfw_status_notify(mlxfw_dev, "Verifying component", comp_name, 0, 0);
err = mlxfw_dev->ops->fsm_component_verify(mlxfw_dev, fwhandle,
comp->index);
if (err)
@@ -203,6 +223,8 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
return PTR_ERR(mfa2_file);
pr_info("Initialize firmware flash process\n");
+ mlxfw_status_notify(mlxfw_dev, "Initializing firmware flash process",
+ NULL, 0, 0);
err = mlxfw_dev->ops->fsm_lock(mlxfw_dev, &fwhandle);
if (err) {
pr_err("Could not lock the firmware FSM\n");
@@ -220,6 +242,7 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
goto err_flash_components;
pr_debug("Activate image\n");
+ mlxfw_status_notify(mlxfw_dev, "Activating image", NULL, 0, 0);
err = mlxfw_dev->ops->fsm_activate(mlxfw_dev, fwhandle);
if (err) {
pr_err("Could not activate the downloaded image\n");
@@ -236,6 +259,7 @@ int mlxfw_firmware_flash(struct mlxfw_dev *mlxfw_dev,
mlxfw_dev->ops->fsm_release(mlxfw_dev, fwhandle);
pr_info("Firmware flash done.\n");
+ mlxfw_status_notify(mlxfw_dev, "Firmware flash done", NULL, 0, 0);
mlxfw_mfa2_file_fini(mfa2_file);
return 0;
--
2.17.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [patch net-next 6/7] mlxsw: Implement flash update status notifications
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
` (4 preceding siblings ...)
2019-05-23 9:45 ` [patch net-next 5/7] mlxfw: Introduce status_notify op and call it to notify about the status Jiri Pirko
@ 2019-05-23 9:45 ` Jiri Pirko
2019-05-23 9:45 ` [patch net-next 7/7] netdevsim: implement fake flash updating with notifications Jiri Pirko
` (5 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Jiri Pirko @ 2019-05-23 9:45 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
From: Jiri Pirko <jiri@mellanox.com>
Implement mlxfw status_notify op by passing notification down to
devlink. Also notify about flash update begin and end.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 18 +++++++++++++++++-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 639bb5778ff3..5ac893d9dd12 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -294,6 +294,19 @@ static void mlxsw_sp_fsm_release(struct mlxfw_dev *mlxfw_dev, u32 fwhandle)
mlxsw_reg_write(mlxsw_sp->core, MLXSW_REG(mcc), mcc_pl);
}
+static void mlxsw_sp_status_notify(struct mlxfw_dev *mlxfw_dev,
+ const char *msg, const char *comp_name,
+ u32 done_bytes, u32 total_bytes)
+{
+ struct mlxsw_sp_mlxfw_dev *mlxsw_sp_mlxfw_dev =
+ container_of(mlxfw_dev, struct mlxsw_sp_mlxfw_dev, mlxfw_dev);
+ struct mlxsw_sp *mlxsw_sp = mlxsw_sp_mlxfw_dev->mlxsw_sp;
+
+ devlink_flash_update_status_notify(priv_to_devlink(mlxsw_sp->core),
+ msg, comp_name,
+ done_bytes, total_bytes);
+}
+
static const struct mlxfw_dev_ops mlxsw_sp_mlxfw_dev_ops = {
.component_query = mlxsw_sp_component_query,
.fsm_lock = mlxsw_sp_fsm_lock,
@@ -303,7 +316,8 @@ static const struct mlxfw_dev_ops mlxsw_sp_mlxfw_dev_ops = {
.fsm_activate = mlxsw_sp_fsm_activate,
.fsm_query_state = mlxsw_sp_fsm_query_state,
.fsm_cancel = mlxsw_sp_fsm_cancel,
- .fsm_release = mlxsw_sp_fsm_release
+ .fsm_release = mlxsw_sp_fsm_release,
+ .status_notify = mlxsw_sp_status_notify,
};
static int mlxsw_sp_firmware_flash(struct mlxsw_sp *mlxsw_sp,
@@ -321,8 +335,10 @@ static int mlxsw_sp_firmware_flash(struct mlxsw_sp *mlxsw_sp,
int err;
mlxsw_core_fw_flash_start(mlxsw_sp->core);
+ devlink_flash_update_begin_notify(priv_to_devlink(mlxsw_sp->core));
err = mlxfw_firmware_flash(&mlxsw_sp_mlxfw_dev.mlxfw_dev,
firmware, extack);
+ devlink_flash_update_end_notify(priv_to_devlink(mlxsw_sp->core));
mlxsw_core_fw_flash_end(mlxsw_sp->core);
return err;
--
2.17.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [patch net-next 7/7] netdevsim: implement fake flash updating with notifications
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
` (5 preceding siblings ...)
2019-05-23 9:45 ` [patch net-next 6/7] mlxsw: Implement flash update status notifications Jiri Pirko
@ 2019-05-23 9:45 ` Jiri Pirko
2019-05-23 17:47 ` Jakub Kicinski
2019-05-23 9:47 ` [patch iproute2 1/3] header update Jiri Pirko
` (4 subsequent siblings)
11 siblings, 1 reply; 29+ messages in thread
From: Jiri Pirko @ 2019-05-23 9:45 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
From: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
drivers/net/netdevsim/dev.c | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index b509b941d5ca..c15b86f9cd2b 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -220,8 +220,43 @@ static int nsim_dev_reload(struct devlink *devlink,
return 0;
}
+#define NSIM_DEV_FLASH_SIZE 500000
+#define NSIM_DEV_FLASH_CHUNK_SIZE 1000
+#define NSIM_DEV_FLASH_CHUNK_TIME_MS 10
+
+static int nsim_dev_flash_update(struct devlink *devlink, const char *file_name,
+ const char *component,
+ struct netlink_ext_ack *extack)
+{
+ int i;
+
+ devlink_flash_update_begin_notify(devlink);
+
+ devlink_flash_update_status_notify(devlink, "Preparing to flash",
+ component, 0, 0);
+ for (i = 0; i < NSIM_DEV_FLASH_SIZE / NSIM_DEV_FLASH_CHUNK_SIZE; i++) {
+ devlink_flash_update_status_notify(devlink, "Flashing",
+ component,
+ i * NSIM_DEV_FLASH_CHUNK_SIZE,
+ NSIM_DEV_FLASH_SIZE);
+ msleep(NSIM_DEV_FLASH_CHUNK_TIME_MS);
+ }
+ devlink_flash_update_status_notify(devlink, "Flashing",
+ component,
+ NSIM_DEV_FLASH_SIZE,
+ NSIM_DEV_FLASH_SIZE);
+
+ devlink_flash_update_status_notify(devlink, "Flashing done",
+ component, 0, 0);
+
+ devlink_flash_update_end_notify(devlink);
+
+ return 0;
+}
+
static const struct devlink_ops nsim_dev_devlink_ops = {
.reload = nsim_dev_reload,
+ .flash_update = nsim_dev_flash_update,
};
static struct nsim_dev *
--
2.17.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [patch net-next 7/7] netdevsim: implement fake flash updating with notifications
2019-05-23 9:45 ` [patch net-next 7/7] netdevsim: implement fake flash updating with notifications Jiri Pirko
@ 2019-05-23 17:47 ` Jakub Kicinski
2019-05-24 8:17 ` Jiri Pirko
0 siblings, 1 reply; 29+ messages in thread
From: Jakub Kicinski @ 2019-05-23 17:47 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, mlxsw, sthemmin, dsahern, saeedm, leon
On Thu, 23 May 2019 11:45:10 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
> drivers/net/netdevsim/dev.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
> index b509b941d5ca..c15b86f9cd2b 100644
> --- a/drivers/net/netdevsim/dev.c
> +++ b/drivers/net/netdevsim/dev.c
> @@ -220,8 +220,43 @@ static int nsim_dev_reload(struct devlink *devlink,
> return 0;
> }
>
> +#define NSIM_DEV_FLASH_SIZE 500000
> +#define NSIM_DEV_FLASH_CHUNK_SIZE 1000
> +#define NSIM_DEV_FLASH_CHUNK_TIME_MS 10
> +
> +static int nsim_dev_flash_update(struct devlink *devlink, const char *file_name,
> + const char *component,
> + struct netlink_ext_ack *extack)
> +{
> + int i;
> +
> + devlink_flash_update_begin_notify(devlink);
Now I wonder if it would be good for the core to send those. Is it
down to the driver to send the begin/end notifications because it would
be wasteful to always send them, or is it some ABI thing?
Also I wonder if it'd be useful for netdevsim to have a mode which
doesn't send notifications, to test both cases.
> + devlink_flash_update_status_notify(devlink, "Preparing to flash",
> + component, 0, 0);
> + for (i = 0; i < NSIM_DEV_FLASH_SIZE / NSIM_DEV_FLASH_CHUNK_SIZE; i++) {
> + devlink_flash_update_status_notify(devlink, "Flashing",
> + component,
> + i * NSIM_DEV_FLASH_CHUNK_SIZE,
> + NSIM_DEV_FLASH_SIZE);
> + msleep(NSIM_DEV_FLASH_CHUNK_TIME_MS);
> + }
> + devlink_flash_update_status_notify(devlink, "Flashing",
> + component,
> + NSIM_DEV_FLASH_SIZE,
> + NSIM_DEV_FLASH_SIZE);
> +
> + devlink_flash_update_status_notify(devlink, "Flashing done",
> + component, 0, 0);
> +
> + devlink_flash_update_end_notify(devlink);
> +
> + return 0;
> +}
> +
> static const struct devlink_ops nsim_dev_devlink_ops = {
> .reload = nsim_dev_reload,
> + .flash_update = nsim_dev_flash_update,
> };
>
> static struct nsim_dev *
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [patch net-next 7/7] netdevsim: implement fake flash updating with notifications
2019-05-23 17:47 ` Jakub Kicinski
@ 2019-05-24 8:17 ` Jiri Pirko
0 siblings, 0 replies; 29+ messages in thread
From: Jiri Pirko @ 2019-05-24 8:17 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, mlxsw, sthemmin, dsahern, saeedm, leon
Thu, May 23, 2019 at 07:47:54PM CEST, jakub.kicinski@netronome.com wrote:
>On Thu, 23 May 2019 11:45:10 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> ---
>> drivers/net/netdevsim/dev.c | 35 +++++++++++++++++++++++++++++++++++
>> 1 file changed, 35 insertions(+)
>>
>> diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
>> index b509b941d5ca..c15b86f9cd2b 100644
>> --- a/drivers/net/netdevsim/dev.c
>> +++ b/drivers/net/netdevsim/dev.c
>> @@ -220,8 +220,43 @@ static int nsim_dev_reload(struct devlink *devlink,
>> return 0;
>> }
>>
>> +#define NSIM_DEV_FLASH_SIZE 500000
>> +#define NSIM_DEV_FLASH_CHUNK_SIZE 1000
>> +#define NSIM_DEV_FLASH_CHUNK_TIME_MS 10
>> +
>> +static int nsim_dev_flash_update(struct devlink *devlink, const char *file_name,
>> + const char *component,
>> + struct netlink_ext_ack *extack)
>> +{
>> + int i;
>> +
>> + devlink_flash_update_begin_notify(devlink);
>
>Now I wonder if it would be good for the core to send those. Is it
>down to the driver to send the begin/end notifications because it would
>be wasteful to always send them, or is it some ABI thing?
The thing is the driver update could be triggered by driver itself. For
example in mlxsw, during init the fw is flashed to the supported
version. And we still want notification that it happened.
>
>Also I wonder if it'd be useful for netdevsim to have a mode which
>doesn't send notifications, to test both cases.
Okay. I'll look into adding a debugfs toggle for this.
>
>> + devlink_flash_update_status_notify(devlink, "Preparing to flash",
>> + component, 0, 0);
>> + for (i = 0; i < NSIM_DEV_FLASH_SIZE / NSIM_DEV_FLASH_CHUNK_SIZE; i++) {
>> + devlink_flash_update_status_notify(devlink, "Flashing",
>> + component,
>> + i * NSIM_DEV_FLASH_CHUNK_SIZE,
>> + NSIM_DEV_FLASH_SIZE);
>> + msleep(NSIM_DEV_FLASH_CHUNK_TIME_MS);
>> + }
>> + devlink_flash_update_status_notify(devlink, "Flashing",
>> + component,
>> + NSIM_DEV_FLASH_SIZE,
>> + NSIM_DEV_FLASH_SIZE);
>> +
>> + devlink_flash_update_status_notify(devlink, "Flashing done",
>> + component, 0, 0);
>> +
>> + devlink_flash_update_end_notify(devlink);
>> +
>> + return 0;
>> +}
>> +
>> static const struct devlink_ops nsim_dev_devlink_ops = {
>> .reload = nsim_dev_reload,
>> + .flash_update = nsim_dev_flash_update,
>> };
>>
>> static struct nsim_dev *
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* [patch iproute2 1/3] header update
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
` (6 preceding siblings ...)
2019-05-23 9:45 ` [patch net-next 7/7] netdevsim: implement fake flash updating with notifications Jiri Pirko
@ 2019-05-23 9:47 ` Jiri Pirko
2019-05-23 9:47 ` [patch iproute2 2/3] devlink: implement flash update status monitoring Jiri Pirko
` (3 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Jiri Pirko @ 2019-05-23 9:47 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
From: Jiri Pirko <jiri@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/uapi/linux/devlink.h | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 3b6a9e6be3ac..6544824a0b97 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -104,6 +104,8 @@ enum devlink_command {
DEVLINK_CMD_HEALTH_REPORTER_DUMP_CLEAR,
DEVLINK_CMD_FLASH_UPDATE,
+ DEVLINK_CMD_FLASH_UPDATE_END, /* notification only */
+ DEVLINK_CMD_FLASH_UPDATE_STATUS, /* notification only */
/* add new commands above here */
__DEVLINK_CMD_MAX,
@@ -331,6 +333,9 @@ enum devlink_attr {
DEVLINK_ATTR_FLASH_UPDATE_FILE_NAME, /* string */
DEVLINK_ATTR_FLASH_UPDATE_COMPONENT, /* string */
+ DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG, /* string */
+ DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE, /* u64 */
+ DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL, /* u64 */
/* add new attributes above here, update the policy in devlink.c */
--
2.17.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [patch iproute2 2/3] devlink: implement flash update status monitoring
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
` (7 preceding siblings ...)
2019-05-23 9:47 ` [patch iproute2 1/3] header update Jiri Pirko
@ 2019-05-23 9:47 ` Jiri Pirko
2019-05-23 9:47 ` [patch iproute2 3/3] devlink: implement flash " Jiri Pirko
` (2 subsequent siblings)
11 siblings, 0 replies; 29+ messages in thread
From: Jiri Pirko @ 2019-05-23 9:47 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
From: Jiri Pirko <jiri@mellanox.com>
Kernel sends notifications about flash update status, so implement these
messages for monitoring.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 43 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 436935f88bda..55cbc01189db 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -414,6 +414,10 @@ static const enum mnl_attr_data_type devlink_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_HEALTH_REPORTER_RECOVER_COUNT] = MNL_TYPE_U64,
[DEVLINK_ATTR_HEALTH_REPORTER_DUMP_TS] = MNL_TYPE_U64,
[DEVLINK_ATTR_HEALTH_REPORTER_GRACEFUL_PERIOD] = MNL_TYPE_U64,
+ [DEVLINK_ATTR_FLASH_UPDATE_COMPONENT] = MNL_TYPE_STRING,
+ [DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG] = MNL_TYPE_STRING,
+ [DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE] = MNL_TYPE_U64,
+ [DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL] = MNL_TYPE_U64,
};
static int attr_cb(const struct nlattr *attr, void *data)
@@ -3712,6 +3716,9 @@ static const char *cmd_name(uint8_t cmd)
case DEVLINK_CMD_REGION_SET: return "set";
case DEVLINK_CMD_REGION_NEW: return "new";
case DEVLINK_CMD_REGION_DEL: return "del";
+ case DEVLINK_CMD_FLASH_UPDATE: return "begin";
+ case DEVLINK_CMD_FLASH_UPDATE_END: return "end";
+ case DEVLINK_CMD_FLASH_UPDATE_STATUS: return "status";
default: return "<unknown cmd>";
}
}
@@ -3740,6 +3747,10 @@ static const char *cmd_obj(uint8_t cmd)
case DEVLINK_CMD_REGION_NEW:
case DEVLINK_CMD_REGION_DEL:
return "region";
+ case DEVLINK_CMD_FLASH_UPDATE:
+ case DEVLINK_CMD_FLASH_UPDATE_END:
+ case DEVLINK_CMD_FLASH_UPDATE_STATUS:
+ return "flash";
default: return "<unknown obj>";
}
}
@@ -3764,6 +3775,29 @@ static bool cmd_filter_check(struct dl *dl, uint8_t cmd)
return false;
}
+static void pr_out_flash_update(struct dl *dl, struct nlattr **tb)
+{
+ __pr_out_handle_start(dl, tb, true, false);
+
+ if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG])
+ pr_out_str(dl, "msg",
+ mnl_attr_get_str(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG]));
+
+ if (tb[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT])
+ pr_out_str(dl, "component",
+ mnl_attr_get_str(tb[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT]));
+
+ if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE])
+ pr_out_u64(dl, "done",
+ mnl_attr_get_u64(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE]));
+
+ if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL])
+ pr_out_u64(dl, "total",
+ mnl_attr_get_u64(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL]));
+
+ pr_out_handle_end(dl);
+}
+
static void pr_out_region(struct dl *dl, struct nlattr **tb);
static int cmd_mon_show_cb(const struct nlmsghdr *nlh, void *data)
@@ -3820,6 +3854,15 @@ static int cmd_mon_show_cb(const struct nlmsghdr *nlh, void *data)
pr_out_mon_header(genl->cmd);
pr_out_region(dl, tb);
break;
+ case DEVLINK_CMD_FLASH_UPDATE: /* fall through */
+ case DEVLINK_CMD_FLASH_UPDATE_END: /* fall through */
+ case DEVLINK_CMD_FLASH_UPDATE_STATUS:
+ mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
+ if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME])
+ return MNL_CB_ERROR;
+ pr_out_mon_header(genl->cmd);
+ pr_out_flash_update(dl, tb);
+ break;
}
return MNL_CB_OK;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* [patch iproute2 3/3] devlink: implement flash status monitoring
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
` (8 preceding siblings ...)
2019-05-23 9:47 ` [patch iproute2 2/3] devlink: implement flash update status monitoring Jiri Pirko
@ 2019-05-23 9:47 ` Jiri Pirko
2019-05-23 17:57 ` Jakub Kicinski
2019-05-23 18:37 ` [patch net-next 0/7] expose flash update status to user Saeed Mahameed
2019-05-27 18:14 ` Florian Fainelli
11 siblings, 1 reply; 29+ messages in thread
From: Jiri Pirko @ 2019-05-23 9:47 UTC (permalink / raw)
To: netdev; +Cc: davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
From: Jiri Pirko <jiri@mellanox.com>
Listen to status notifications coming from kernel during flashing and
put them on stdout to inform user about the status.
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
devlink/devlink.c | 209 +++++++++++++++++++++++++++++++++++++++++++++-
devlink/mnlg.c | 5 ++
devlink/mnlg.h | 1 +
3 files changed, 211 insertions(+), 4 deletions(-)
diff --git a/devlink/devlink.c b/devlink/devlink.c
index 55cbc01189db..8078e8801e98 100644
--- a/devlink/devlink.c
+++ b/devlink/devlink.c
@@ -24,6 +24,7 @@
#include <netinet/ether.h>
#include <sys/sysinfo.h>
#include <sys/queue.h>
+#include <sys/types.h>
#include "SNAPSHOT.h"
#include "list.h"
@@ -68,6 +69,12 @@ static int g_new_line_count;
g_new_line_count = 0; \
} while (0)
+#define pr_out_tty(args...) \
+ do { \
+ if (isatty(STDOUT_FILENO)) \
+ fprintf(stdout, ##args); \
+ } while (0)
+
static int g_indent_level;
static bool g_indent_newline;
#define INDENT_STR_STEP 2
@@ -113,9 +120,8 @@ static int _mnlg_socket_recv_run(struct mnlg_socket *nlg,
return 0;
}
-static int _mnlg_socket_sndrcv(struct mnlg_socket *nlg,
- const struct nlmsghdr *nlh,
- mnl_cb_t data_cb, void *data)
+static int _mnlg_socket_send(struct mnlg_socket *nlg,
+ const struct nlmsghdr *nlh)
{
int err;
@@ -124,6 +130,18 @@ static int _mnlg_socket_sndrcv(struct mnlg_socket *nlg,
pr_err("Failed to call mnlg_socket_send\n");
return -errno;
}
+ return 0;
+}
+
+static int _mnlg_socket_sndrcv(struct mnlg_socket *nlg,
+ const struct nlmsghdr *nlh,
+ mnl_cb_t data_cb, void *data)
+{
+ int err;
+
+ err = _mnlg_socket_send(nlg, nlh);
+ if (err)
+ return err;
return _mnlg_socket_recv_run(nlg, data_cb, data);
}
@@ -2697,9 +2715,151 @@ static void cmd_dev_flash_help(void)
pr_err("Usage: devlink dev flash DEV file PATH [ component NAME ]\n");
}
+
+struct cmd_dev_flash_status_ctx {
+ struct dl *dl;
+ char *last_msg;
+ char *last_component;
+ uint8_t not_first:1,
+ last_pc:1,
+ received_end:1,
+ flash_done:1;
+};
+
+static int nullstrcmp(const char *str1, const char *str2)
+{
+ if (str1 && str2)
+ return strcmp(str1, str2);
+ if (!str1 && !str2)
+ return 0;
+ return str1 ? 1 : -1;
+}
+
+static int cmd_dev_flash_status_cb(const struct nlmsghdr *nlh, void *data)
+{
+ struct cmd_dev_flash_status_ctx *ctx = data;
+ struct dl_opts *opts = &ctx->dl->opts;
+ struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
+ struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
+ const char *component = NULL;
+ uint64_t done = 0, total = 0;
+ const char *msg = NULL;
+ const char *bus_name;
+ const char *dev_name;
+
+ if (genl->cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS &&
+ genl->cmd != DEVLINK_CMD_FLASH_UPDATE_END)
+ return MNL_CB_STOP;
+
+ mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
+ if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME])
+ return MNL_CB_ERROR;
+ bus_name = mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]);
+ dev_name = mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]);
+ if (strcmp(bus_name, opts->bus_name) ||
+ strcmp(dev_name, opts->dev_name))
+ return MNL_CB_ERROR;
+
+ if (genl->cmd == DEVLINK_CMD_FLASH_UPDATE_END && ctx->not_first) {
+ pr_out("\n");
+ free(ctx->last_msg);
+ free(ctx->last_component);
+ ctx->received_end = 1;
+ return MNL_CB_STOP;
+ }
+
+ if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG])
+ msg = mnl_attr_get_str(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_MSG]);
+ if (tb[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT])
+ component = mnl_attr_get_str(tb[DEVLINK_ATTR_FLASH_UPDATE_COMPONENT]);
+ if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE])
+ done = mnl_attr_get_u64(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_DONE]);
+ if (tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL])
+ total = mnl_attr_get_u64(tb[DEVLINK_ATTR_FLASH_UPDATE_STATUS_TOTAL]);
+
+ if (!nullstrcmp(msg, ctx->last_msg) &&
+ !nullstrcmp(component, ctx->last_component) &&
+ ctx->last_pc && ctx->not_first) {
+ pr_out_tty("\b\b\b\b\b"); /* clean percentage */
+ } else {
+ if (ctx->not_first)
+ pr_out("\n");
+ if (component) {
+ pr_out("[%s] ", component);
+ free(ctx->last_component);
+ ctx->last_component = strdup(component);
+ }
+ if (msg) {
+ pr_out("%s", msg);
+ free(ctx->last_msg);
+ ctx->last_msg = strdup(msg);
+ }
+ }
+ if (total) {
+ pr_out_tty(" %3lu%%", (done * 100) / total);
+ ctx->last_pc = 1;
+ } else {
+ ctx->last_pc = 0;
+ }
+ fflush(stdout);
+ ctx->not_first = 1;
+
+ return MNL_CB_STOP;
+}
+
+static int cmd_dev_flash_fds_process(struct cmd_dev_flash_status_ctx *ctx,
+ struct mnlg_socket *nlg_ntf,
+ int pipe_r)
+{
+ int nlfd = mnlg_socket_get_fd(nlg_ntf);
+ fd_set fds[3];
+ int fdmax;
+ int i;
+ int err;
+ int err2;
+
+ for (i = 0; i < 3; i++)
+ FD_ZERO(&fds[i]);
+ FD_SET(pipe_r, &fds[0]);
+ fdmax = pipe_r + 1;
+ FD_SET(nlfd, &fds[0]);
+ if (nlfd >= fdmax)
+ fdmax = nlfd + 1;
+
+ while (select(fdmax, &fds[0], &fds[1], &fds[2], NULL) < 0) {
+ if (errno == EINTR)
+ continue;
+ pr_err("select() failed\n");
+ return -errno;
+ }
+ if (FD_ISSET(nlfd, &fds[0])) {
+ err = _mnlg_socket_recv_run(nlg_ntf,
+ cmd_dev_flash_status_cb, ctx);
+ if (err)
+ return err;
+ }
+ if (FD_ISSET(pipe_r, &fds[0])) {
+ err = read(pipe_r, &err2, sizeof(err2));
+ if (err == -1) {
+ pr_err("Failed to read pipe\n");
+ return -errno;
+ }
+ if (err2)
+ return err2;
+ ctx->flash_done = 1;
+ }
+ return 0;
+}
+
+
static int cmd_dev_flash(struct dl *dl)
{
+ struct cmd_dev_flash_status_ctx ctx = {.dl = dl,};
+ struct mnlg_socket *nlg_ntf;
struct nlmsghdr *nlh;
+ int pipe_r, pipe_w;
+ int pipe_fds[2];
+ pid_t pid;
int err;
if (dl_argv_match(dl, "help") || dl_no_arg(dl)) {
@@ -2715,7 +2875,48 @@ static int cmd_dev_flash(struct dl *dl)
if (err)
return err;
- return _mnlg_socket_sndrcv(dl->nlg, nlh, NULL, NULL);
+ nlg_ntf = mnlg_socket_open(DEVLINK_GENL_NAME, DEVLINK_GENL_VERSION);
+ if (!nlg_ntf)
+ return err;
+
+ err = _mnlg_socket_group_add(nlg_ntf, DEVLINK_GENL_MCGRP_CONFIG_NAME);
+ if (err)
+ return err;
+
+ err = pipe(pipe_fds);
+ if (err == -1)
+ return -errno;
+ pipe_r = pipe_fds[0];
+ pipe_w = pipe_fds[1];
+
+ pid = fork();
+ if (pid == -1) {
+ close(pipe_r);
+ close(pipe_w);
+ return -errno;
+ } else if (!pid) {
+ /* In child, just execute the flash and pass returned
+ * value through pipe once it is done.
+ */
+ close(pipe_r);
+ err = _mnlg_socket_send(dl->nlg, nlh);
+ write(pipe_w, &err, sizeof(err));
+ close(pipe_w);
+ exit(0);
+ }
+ close(pipe_w);
+
+ do {
+ err = cmd_dev_flash_fds_process(&ctx, nlg_ntf, pipe_r);
+ if (err)
+ goto out;
+ } while (!ctx.flash_done || !ctx.received_end);
+
+ err = _mnlg_socket_recv_run(dl->nlg, NULL, NULL);
+out:
+ close(pipe_r);
+ mnlg_socket_close(nlg_ntf);
+ return err;
}
static int cmd_dev(struct dl *dl)
diff --git a/devlink/mnlg.c b/devlink/mnlg.c
index 37cc25ddf490..f5a5dbe7f64f 100644
--- a/devlink/mnlg.c
+++ b/devlink/mnlg.c
@@ -313,3 +313,8 @@ void mnlg_socket_close(struct mnlg_socket *nlg)
free(nlg->buf);
free(nlg);
}
+
+int mnlg_socket_get_fd(struct mnlg_socket *nlg)
+{
+ return mnl_socket_get_fd(nlg->nl);
+}
diff --git a/devlink/mnlg.h b/devlink/mnlg.h
index 4d1babf3b4c2..61bc5a3f31aa 100644
--- a/devlink/mnlg.h
+++ b/devlink/mnlg.h
@@ -23,5 +23,6 @@ int mnlg_socket_recv_run(struct mnlg_socket *nlg, mnl_cb_t data_cb, void *data);
int mnlg_socket_group_add(struct mnlg_socket *nlg, const char *group_name);
struct mnlg_socket *mnlg_socket_open(const char *family_name, uint8_t version);
void mnlg_socket_close(struct mnlg_socket *nlg);
+int mnlg_socket_get_fd(struct mnlg_socket *nlg);
#endif /* _MNLG_H_ */
--
2.17.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [patch iproute2 3/3] devlink: implement flash status monitoring
2019-05-23 9:47 ` [patch iproute2 3/3] devlink: implement flash " Jiri Pirko
@ 2019-05-23 17:57 ` Jakub Kicinski
2019-05-24 8:14 ` Jiri Pirko
0 siblings, 1 reply; 29+ messages in thread
From: Jakub Kicinski @ 2019-05-23 17:57 UTC (permalink / raw)
To: Jiri Pirko; +Cc: netdev, davem, mlxsw, sthemmin, dsahern, saeedm, leon
On Thu, 23 May 2019 11:47:10 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> Listen to status notifications coming from kernel during flashing and
> put them on stdout to inform user about the status.
>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> +static int cmd_dev_flash_status_cb(const struct nlmsghdr *nlh, void *data)
> +{
> + struct cmd_dev_flash_status_ctx *ctx = data;
> + struct dl_opts *opts = &ctx->dl->opts;
> + struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
> + struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
> + const char *component = NULL;
> + uint64_t done = 0, total = 0;
> + const char *msg = NULL;
> + const char *bus_name;
> + const char *dev_name;
> +
> + if (genl->cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS &&
> + genl->cmd != DEVLINK_CMD_FLASH_UPDATE_END)
> + return MNL_CB_STOP;
> +
> + mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
> + if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME])
> + return MNL_CB_ERROR;
> + bus_name = mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]);
> + dev_name = mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]);
> + if (strcmp(bus_name, opts->bus_name) ||
> + strcmp(dev_name, opts->dev_name))
> + return MNL_CB_ERROR;
> +
> + if (genl->cmd == DEVLINK_CMD_FLASH_UPDATE_END && ctx->not_first) {
> + pr_out("\n");
> + free(ctx->last_msg);
> + free(ctx->last_component);
> + ctx->received_end = 1;
> + return MNL_CB_STOP;
> + }
> + pid = fork();
> + if (pid == -1) {
> + close(pipe_r);
> + close(pipe_w);
> + return -errno;
> + } else if (!pid) {
> + /* In child, just execute the flash and pass returned
> + * value through pipe once it is done.
> + */
> + close(pipe_r);
> + err = _mnlg_socket_send(dl->nlg, nlh);
> + write(pipe_w, &err, sizeof(err));
> + close(pipe_w);
> + exit(0);
> + }
> + close(pipe_w);
> +
> + do {
> + err = cmd_dev_flash_fds_process(&ctx, nlg_ntf, pipe_r);
> + if (err)
> + goto out;
> + } while (!ctx.flash_done || !ctx.received_end);
Won't this loop forever if driver never sends
DEVLINK_CMD_FLASH_UPDATE_END IOW doesn't implement status updates?
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [patch iproute2 3/3] devlink: implement flash status monitoring
2019-05-23 17:57 ` Jakub Kicinski
@ 2019-05-24 8:14 ` Jiri Pirko
0 siblings, 0 replies; 29+ messages in thread
From: Jiri Pirko @ 2019-05-24 8:14 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, davem, mlxsw, sthemmin, dsahern, saeedm, leon
Thu, May 23, 2019 at 07:57:03PM CEST, jakub.kicinski@netronome.com wrote:
>On Thu, 23 May 2019 11:47:10 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Listen to status notifications coming from kernel during flashing and
>> put them on stdout to inform user about the status.
>>
>> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>
>> +static int cmd_dev_flash_status_cb(const struct nlmsghdr *nlh, void *data)
>> +{
>> + struct cmd_dev_flash_status_ctx *ctx = data;
>> + struct dl_opts *opts = &ctx->dl->opts;
>> + struct genlmsghdr *genl = mnl_nlmsg_get_payload(nlh);
>> + struct nlattr *tb[DEVLINK_ATTR_MAX + 1] = {};
>> + const char *component = NULL;
>> + uint64_t done = 0, total = 0;
>> + const char *msg = NULL;
>> + const char *bus_name;
>> + const char *dev_name;
>> +
>> + if (genl->cmd != DEVLINK_CMD_FLASH_UPDATE_STATUS &&
>> + genl->cmd != DEVLINK_CMD_FLASH_UPDATE_END)
>> + return MNL_CB_STOP;
>> +
>> + mnl_attr_parse(nlh, sizeof(*genl), attr_cb, tb);
>> + if (!tb[DEVLINK_ATTR_BUS_NAME] || !tb[DEVLINK_ATTR_DEV_NAME])
>> + return MNL_CB_ERROR;
>> + bus_name = mnl_attr_get_str(tb[DEVLINK_ATTR_BUS_NAME]);
>> + dev_name = mnl_attr_get_str(tb[DEVLINK_ATTR_DEV_NAME]);
>> + if (strcmp(bus_name, opts->bus_name) ||
>> + strcmp(dev_name, opts->dev_name))
>> + return MNL_CB_ERROR;
>> +
>> + if (genl->cmd == DEVLINK_CMD_FLASH_UPDATE_END && ctx->not_first) {
>> + pr_out("\n");
>> + free(ctx->last_msg);
>> + free(ctx->last_component);
>> + ctx->received_end = 1;
>> + return MNL_CB_STOP;
>> + }
>
>> + pid = fork();
>> + if (pid == -1) {
>> + close(pipe_r);
>> + close(pipe_w);
>> + return -errno;
>> + } else if (!pid) {
>> + /* In child, just execute the flash and pass returned
>> + * value through pipe once it is done.
>> + */
>> + close(pipe_r);
>> + err = _mnlg_socket_send(dl->nlg, nlh);
>> + write(pipe_w, &err, sizeof(err));
>> + close(pipe_w);
>> + exit(0);
>> + }
>> + close(pipe_w);
>> +
>> + do {
>> + err = cmd_dev_flash_fds_process(&ctx, nlg_ntf, pipe_r);
>> + if (err)
>> + goto out;
>> + } while (!ctx.flash_done || !ctx.received_end);
>
>Won't this loop forever if driver never sends
>DEVLINK_CMD_FLASH_UPDATE_END IOW doesn't implement status updates?
Hmm, right. Will look into this and fix.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [patch net-next 0/7] expose flash update status to user
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
` (9 preceding siblings ...)
2019-05-23 9:47 ` [patch iproute2 3/3] devlink: implement flash " Jiri Pirko
@ 2019-05-23 18:37 ` Saeed Mahameed
2019-05-24 8:15 ` Jiri Pirko
2019-05-28 11:29 ` Jiri Pirko
2019-05-27 18:14 ` Florian Fainelli
11 siblings, 2 replies; 29+ messages in thread
From: Saeed Mahameed @ 2019-05-23 18:37 UTC (permalink / raw)
To: jiri, netdev; +Cc: sthemmin, dsahern, davem, mlxsw, jakub.kicinski, leon
On Thu, 2019-05-23 at 11:45 +0200, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> When user is flashing device using devlink, he currenly does not see
> any
> information about what is going on, percentages, etc.
> Drivers, for example mlxsw and mlx5, have notion about the progress
> and what is happening. This patchset exposes this progress
> information to userspace.
>
Very cool stuff, \let's update devlink docs with the new potential
output of the fw flash commands, and show us some output example here
or on one of the commit messages, it would really help getting an idea
of what this cool patchset provides.
> See this console recording which shows flashing FW on a Mellanox
> Spectrum device:
> https://asciinema.org/a/247926
>
> Jiri Pirko (7):
> mlxsw: Move firmware flash implementation to devlink
> mlx5: Move firmware flash implementation to devlink
> mlxfw: Propagate error messages through extack
> devlink: allow driver to update progress of flash update
> mlxfw: Introduce status_notify op and call it to notify about the
> status
> mlxsw: Implement flash update status notifications
> netdevsim: implement fake flash updating with notifications
>
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 2 -
> .../ethernet/mellanox/mlx5/core/en_ethtool.c | 35 ------
> drivers/net/ethernet/mellanox/mlx5/core/fw.c | 6 +-
> .../mellanox/mlx5/core/ipoib/ethtool.c | 9 --
> .../net/ethernet/mellanox/mlx5/core/main.c | 20 ++++
> .../ethernet/mellanox/mlx5/core/mlx5_core.h | 3 +-
> drivers/net/ethernet/mellanox/mlxfw/mlxfw.h | 11 +-
> .../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c | 57 ++++++++--
> drivers/net/ethernet/mellanox/mlxsw/core.c | 15 +++
> drivers/net/ethernet/mellanox/mlxsw/core.h | 3 +
> .../net/ethernet/mellanox/mlxsw/spectrum.c | 75 +++++++------
> drivers/net/netdevsim/dev.c | 35 ++++++
> include/net/devlink.h | 8 ++
> include/uapi/linux/devlink.h | 5 +
> net/core/devlink.c | 102
> ++++++++++++++++++
> 15 files changed, 295 insertions(+), 91 deletions(-)
>
Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [patch net-next 0/7] expose flash update status to user
2019-05-23 18:37 ` [patch net-next 0/7] expose flash update status to user Saeed Mahameed
@ 2019-05-24 8:15 ` Jiri Pirko
2019-05-24 15:04 ` David Ahern
2019-05-28 11:29 ` Jiri Pirko
1 sibling, 1 reply; 29+ messages in thread
From: Jiri Pirko @ 2019-05-24 8:15 UTC (permalink / raw)
To: Saeed Mahameed
Cc: netdev, sthemmin, dsahern, davem, mlxsw, jakub.kicinski, leon
Thu, May 23, 2019 at 08:37:28PM CEST, saeedm@mellanox.com wrote:
>On Thu, 2019-05-23 at 11:45 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> When user is flashing device using devlink, he currenly does not see
>> any
>> information about what is going on, percentages, etc.
>> Drivers, for example mlxsw and mlx5, have notion about the progress
>> and what is happening. This patchset exposes this progress
>> information to userspace.
>>
>
>Very cool stuff, \let's update devlink docs with the new potential
>output of the fw flash commands, and show us some output example here
>or on one of the commit messages, it would really help getting an idea
>of what this cool patchset provides.
You mean in man? I can put some example there.
>
>> See this console recording which shows flashing FW on a Mellanox
>> Spectrum device:
>> https://asciinema.org/a/247926
>>
>> Jiri Pirko (7):
>> mlxsw: Move firmware flash implementation to devlink
>> mlx5: Move firmware flash implementation to devlink
>> mlxfw: Propagate error messages through extack
>> devlink: allow driver to update progress of flash update
>> mlxfw: Introduce status_notify op and call it to notify about the
>> status
>> mlxsw: Implement flash update status notifications
>> netdevsim: implement fake flash updating with notifications
>>
>> drivers/net/ethernet/mellanox/mlx5/core/en.h | 2 -
>> .../ethernet/mellanox/mlx5/core/en_ethtool.c | 35 ------
>> drivers/net/ethernet/mellanox/mlx5/core/fw.c | 6 +-
>> .../mellanox/mlx5/core/ipoib/ethtool.c | 9 --
>> .../net/ethernet/mellanox/mlx5/core/main.c | 20 ++++
>> .../ethernet/mellanox/mlx5/core/mlx5_core.h | 3 +-
>> drivers/net/ethernet/mellanox/mlxfw/mlxfw.h | 11 +-
>> .../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c | 57 ++++++++--
>> drivers/net/ethernet/mellanox/mlxsw/core.c | 15 +++
>> drivers/net/ethernet/mellanox/mlxsw/core.h | 3 +
>> .../net/ethernet/mellanox/mlxsw/spectrum.c | 75 +++++++------
>> drivers/net/netdevsim/dev.c | 35 ++++++
>> include/net/devlink.h | 8 ++
>> include/uapi/linux/devlink.h | 5 +
>> net/core/devlink.c | 102
>> ++++++++++++++++++
>> 15 files changed, 295 insertions(+), 91 deletions(-)
>>
>
>Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [patch net-next 0/7] expose flash update status to user
2019-05-24 8:15 ` Jiri Pirko
@ 2019-05-24 15:04 ` David Ahern
0 siblings, 0 replies; 29+ messages in thread
From: David Ahern @ 2019-05-24 15:04 UTC (permalink / raw)
To: Jiri Pirko, Saeed Mahameed
Cc: netdev, sthemmin, davem, mlxsw, jakub.kicinski, leon
On 5/24/19 2:15 AM, Jiri Pirko wrote:
>> Very cool stuff, \let's update devlink docs with the new potential
>> output of the fw flash commands, and show us some output example here
>> or on one of the commit messages, it would really help getting an idea
>> of what this cool patchset provides.
> You mean in man? I can put some example there.
>
>
and commit log messages.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [patch net-next 0/7] expose flash update status to user
2019-05-23 18:37 ` [patch net-next 0/7] expose flash update status to user Saeed Mahameed
2019-05-24 8:15 ` Jiri Pirko
@ 2019-05-28 11:29 ` Jiri Pirko
1 sibling, 0 replies; 29+ messages in thread
From: Jiri Pirko @ 2019-05-28 11:29 UTC (permalink / raw)
To: Saeed Mahameed
Cc: netdev, sthemmin, dsahern, davem, mlxsw, jakub.kicinski, leon
Thu, May 23, 2019 at 08:37:28PM CEST, saeedm@mellanox.com wrote:
>On Thu, 2019-05-23 at 11:45 +0200, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> When user is flashing device using devlink, he currenly does not see
>> any
>> information about what is going on, percentages, etc.
>> Drivers, for example mlxsw and mlx5, have notion about the progress
>> and what is happening. This patchset exposes this progress
>> information to userspace.
>>
>
>Very cool stuff, \let's update devlink docs with the new potential
>output of the fw flash commands, and show us some output example here
>or on one of the commit messages, it would really help getting an idea
>of what this cool patchset provides.
>
>> See this console recording which shows flashing FW on a Mellanox
>> Spectrum device:
>> https://asciinema.org/a/247926
This should give you the idea :) It's not easy to add it in static
text...
>>
>> Jiri Pirko (7):
>> mlxsw: Move firmware flash implementation to devlink
>> mlx5: Move firmware flash implementation to devlink
>> mlxfw: Propagate error messages through extack
>> devlink: allow driver to update progress of flash update
>> mlxfw: Introduce status_notify op and call it to notify about the
>> status
>> mlxsw: Implement flash update status notifications
>> netdevsim: implement fake flash updating with notifications
>>
>> drivers/net/ethernet/mellanox/mlx5/core/en.h | 2 -
>> .../ethernet/mellanox/mlx5/core/en_ethtool.c | 35 ------
>> drivers/net/ethernet/mellanox/mlx5/core/fw.c | 6 +-
>> .../mellanox/mlx5/core/ipoib/ethtool.c | 9 --
>> .../net/ethernet/mellanox/mlx5/core/main.c | 20 ++++
>> .../ethernet/mellanox/mlx5/core/mlx5_core.h | 3 +-
>> drivers/net/ethernet/mellanox/mlxfw/mlxfw.h | 11 +-
>> .../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c | 57 ++++++++--
>> drivers/net/ethernet/mellanox/mlxsw/core.c | 15 +++
>> drivers/net/ethernet/mellanox/mlxsw/core.h | 3 +
>> .../net/ethernet/mellanox/mlxsw/spectrum.c | 75 +++++++------
>> drivers/net/netdevsim/dev.c | 35 ++++++
>> include/net/devlink.h | 8 ++
>> include/uapi/linux/devlink.h | 5 +
>> net/core/devlink.c | 102
>> ++++++++++++++++++
>> 15 files changed, 295 insertions(+), 91 deletions(-)
>>
>
>Reviewed-by: Saeed Mahameed <saeedm@mellanox.com>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [patch net-next 0/7] expose flash update status to user
2019-05-23 9:45 [patch net-next 0/7] expose flash update status to user Jiri Pirko
` (10 preceding siblings ...)
2019-05-23 18:37 ` [patch net-next 0/7] expose flash update status to user Saeed Mahameed
@ 2019-05-27 18:14 ` Florian Fainelli
2019-05-28 8:55 ` Jiri Pirko
11 siblings, 1 reply; 29+ messages in thread
From: Florian Fainelli @ 2019-05-27 18:14 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
On 5/23/2019 2:45 AM, Jiri Pirko wrote:
> From: Jiri Pirko <jiri@mellanox.com>
>
> When user is flashing device using devlink, he currenly does not see any
> information about what is going on, percentages, etc.
> Drivers, for example mlxsw and mlx5, have notion about the progress
> and what is happening. This patchset exposes this progress
> information to userspace.
>
> See this console recording which shows flashing FW on a Mellanox
> Spectrum device:
> https://asciinema.org/a/247926
It would be great to explain why you went that route instead of
implementing a MTD device (like what sfc) which would have presumably
allowed you to more or less the same thing using a standard device
driver model that is establish with flash devices.
>
> Jiri Pirko (7):
> mlxsw: Move firmware flash implementation to devlink
> mlx5: Move firmware flash implementation to devlink
> mlxfw: Propagate error messages through extack
> devlink: allow driver to update progress of flash update
> mlxfw: Introduce status_notify op and call it to notify about the
> status
> mlxsw: Implement flash update status notifications
> netdevsim: implement fake flash updating with notifications
>
> drivers/net/ethernet/mellanox/mlx5/core/en.h | 2 -
> .../ethernet/mellanox/mlx5/core/en_ethtool.c | 35 ------
> drivers/net/ethernet/mellanox/mlx5/core/fw.c | 6 +-
> .../mellanox/mlx5/core/ipoib/ethtool.c | 9 --
> .../net/ethernet/mellanox/mlx5/core/main.c | 20 ++++
> .../ethernet/mellanox/mlx5/core/mlx5_core.h | 3 +-
> drivers/net/ethernet/mellanox/mlxfw/mlxfw.h | 11 +-
> .../net/ethernet/mellanox/mlxfw/mlxfw_fsm.c | 57 ++++++++--
> drivers/net/ethernet/mellanox/mlxsw/core.c | 15 +++
> drivers/net/ethernet/mellanox/mlxsw/core.h | 3 +
> .../net/ethernet/mellanox/mlxsw/spectrum.c | 75 +++++++------
> drivers/net/netdevsim/dev.c | 35 ++++++
> include/net/devlink.h | 8 ++
> include/uapi/linux/devlink.h | 5 +
> net/core/devlink.c | 102 ++++++++++++++++++
> 15 files changed, 295 insertions(+), 91 deletions(-)
>
--
Florian
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [patch net-next 0/7] expose flash update status to user
2019-05-27 18:14 ` Florian Fainelli
@ 2019-05-28 8:55 ` Jiri Pirko
0 siblings, 0 replies; 29+ messages in thread
From: Jiri Pirko @ 2019-05-28 8:55 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev, davem, mlxsw, jakub.kicinski, sthemmin, dsahern, saeedm, leon
Mon, May 27, 2019 at 08:14:32PM CEST, f.fainelli@gmail.com wrote:
>
>
>On 5/23/2019 2:45 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> When user is flashing device using devlink, he currenly does not see any
>> information about what is going on, percentages, etc.
>> Drivers, for example mlxsw and mlx5, have notion about the progress
>> and what is happening. This patchset exposes this progress
>> information to userspace.
>>
>> See this console recording which shows flashing FW on a Mellanox
>> Spectrum device:
>> https://asciinema.org/a/247926
>
>It would be great to explain why you went that route instead of
>implementing a MTD device (like what sfc) which would have presumably
>allowed you to more or less the same thing using a standard device
>driver model that is establish with flash devices.
This patch is not adding flash functionality. It only adds the
visibility into already implemented flash process.
Also, re MTD, let me refer you to the discussion 2 years ago:
Subject: Re: [patch net-next 0/9] mlxsw: Support firmware flash
From: Yotam Gigi <yotamg@mellanox.com>
Message-ID: <0daa5c5a-377c-767f-ea19-26c1f22bd30c@mellanox.com>
Date: Sun, 28 May 2017 10:26:49 +0300
^ permalink raw reply [flat|nested] 29+ messages in thread