* [PATCH net-next 01/16] devlink: Change devlink_reload_supported() param type
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 20:54 ` Jakub Kicinski
` (2 more replies)
2020-10-01 13:59 ` [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command Moshe Shemesh
` (14 subsequent siblings)
15 siblings, 3 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Change devlink_reload_supported() function to get devlink_ops pointer
param instead of devlink pointer param.
This change will be used in the next patch to check if devlink reload is
supported before devlink instance is allocated.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
---
RFCv5 -> v1:
- New patch
---
net/core/devlink.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 6f2863e717a9..722a9431ff60 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -2946,9 +2946,9 @@ static void devlink_reload_netns_change(struct devlink *devlink,
DEVLINK_CMD_PARAM_NEW);
}
-static bool devlink_reload_supported(const struct devlink *devlink)
+static bool devlink_reload_supported(const struct devlink_ops *ops)
{
- return devlink->ops->reload_down && devlink->ops->reload_up;
+ return ops->reload_down && ops->reload_up;
}
static void devlink_reload_failed_set(struct devlink *devlink,
@@ -2992,7 +2992,7 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
struct net *dest_net = NULL;
int err;
- if (!devlink_reload_supported(devlink))
+ if (!devlink_reload_supported(devlink->ops))
return -EOPNOTSUPP;
err = devlink_resources_validate(devlink, NULL, info);
@@ -7516,7 +7516,7 @@ EXPORT_SYMBOL_GPL(devlink_register);
void devlink_unregister(struct devlink *devlink)
{
mutex_lock(&devlink_mutex);
- WARN_ON(devlink_reload_supported(devlink) &&
+ WARN_ON(devlink_reload_supported(devlink->ops) &&
devlink->reload_enabled);
devlink_notify(devlink, DEVLINK_CMD_DEL);
list_del(&devlink->list);
@@ -8553,7 +8553,7 @@ __devlink_param_driverinit_value_set(struct devlink *devlink,
int devlink_param_driverinit_value_get(struct devlink *devlink, u32 param_id,
union devlink_param_value *init_val)
{
- if (!devlink_reload_supported(devlink))
+ if (!devlink_reload_supported(devlink->ops))
return -EOPNOTSUPP;
return __devlink_param_driverinit_value_get(&devlink->param_list,
@@ -8600,7 +8600,7 @@ int devlink_port_param_driverinit_value_get(struct devlink_port *devlink_port,
{
struct devlink *devlink = devlink_port->devlink;
- if (!devlink_reload_supported(devlink))
+ if (!devlink_reload_supported(devlink->ops))
return -EOPNOTSUPP;
return __devlink_param_driverinit_value_get(&devlink_port->param_list,
@@ -9733,7 +9733,7 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
mutex_lock(&devlink_mutex);
list_for_each_entry(devlink, &devlink_list, list) {
if (net_eq(devlink_net(devlink), net)) {
- if (WARN_ON(!devlink_reload_supported(devlink)))
+ if (WARN_ON(!devlink_reload_supported(devlink->ops)))
continue;
err = devlink_reload(devlink, &init_net, NULL);
if (err && err != -EOPNOTSUPP)
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH net-next 01/16] devlink: Change devlink_reload_supported() param type
2020-10-01 13:59 ` [PATCH net-next 01/16] devlink: Change devlink_reload_supported() param type Moshe Shemesh
@ 2020-10-01 20:54 ` Jakub Kicinski
2020-10-02 15:13 ` Jiri Pirko
2020-10-05 18:34 ` Jacob Keller
2 siblings, 0 replies; 49+ messages in thread
From: Jakub Kicinski @ 2020-10-01 20:54 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: David S. Miller, Jiri Pirko, netdev, linux-kernel
On Thu, 1 Oct 2020 16:59:04 +0300 Moshe Shemesh wrote:
> Change devlink_reload_supported() function to get devlink_ops pointer
> param instead of devlink pointer param.
> This change will be used in the next patch to check if devlink reload is
> supported before devlink instance is allocated.
>
> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH net-next 01/16] devlink: Change devlink_reload_supported() param type
2020-10-01 13:59 ` [PATCH net-next 01/16] devlink: Change devlink_reload_supported() param type Moshe Shemesh
2020-10-01 20:54 ` Jakub Kicinski
@ 2020-10-02 15:13 ` Jiri Pirko
2020-10-05 18:34 ` Jacob Keller
2 siblings, 0 replies; 49+ messages in thread
From: Jiri Pirko @ 2020-10-02 15:13 UTC (permalink / raw)
To: Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
Thu, Oct 01, 2020 at 03:59:04PM CEST, moshe@mellanox.com wrote:
>Change devlink_reload_supported() function to get devlink_ops pointer
>param instead of devlink pointer param.
>This change will be used in the next patch to check if devlink reload is
>supported before devlink instance is allocated.
>
>Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH net-next 01/16] devlink: Change devlink_reload_supported() param type
2020-10-01 13:59 ` [PATCH net-next 01/16] devlink: Change devlink_reload_supported() param type Moshe Shemesh
2020-10-01 20:54 ` Jakub Kicinski
2020-10-02 15:13 ` Jiri Pirko
@ 2020-10-05 18:34 ` Jacob Keller
2 siblings, 0 replies; 49+ messages in thread
From: Jacob Keller @ 2020-10-05 18:34 UTC (permalink / raw)
To: Moshe Shemesh, David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel
On 10/1/2020 6:59 AM, Moshe Shemesh wrote:
> Change devlink_reload_supported() function to get devlink_ops pointer
> param instead of devlink pointer param.
> This change will be used in the next patch to check if devlink reload is
> supported before devlink instance is allocated.
>
> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 01/16] devlink: Change devlink_reload_supported() param type Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 20:59 ` Jakub Kicinski
` (3 more replies)
2020-10-01 13:59 ` [PATCH net-next 03/16] devlink: Add devlink reload limit option Moshe Shemesh
` (13 subsequent siblings)
15 siblings, 4 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Add devlink reload action to allow the user to request a specific reload
action. The action parameter is optional, if not specified then devlink
driver re-init action is used (backward compatible).
Note that when required to do firmware activation some drivers may need
to reload the driver. On the other hand some drivers may need to reset
the firmware to reinitialize the driver entities. Therefore, the devlink
reload command returns the actions which were actually performed.
Reload actions supported are:
driver_reinit: driver entities re-initialization, applying devlink-param
and devlink-resource values.
fw_activate: firmware activate.
command examples:
$devlink dev reload pci/0000:82:00.0 action driver_reinit
reload_actions_performed:
driver_reinit
$devlink dev reload pci/0000:82:00.0 action fw_activate
reload_actions_performed:
driver_reinit fw_activate
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
---
RFCv5 -> v1:
- Rename supported_reload_actions to reload_actions.
- Rename devlink_nl_reload_actions_performed_fill() to
devlink_nl_reload_actions_performed_snd() and add genlmsg_reply() to it
- Actions_performed sent to user space as a mask
- Driver can initialize actions_performed before done as devlink ignores
in case of failure
- Use nla_poilcy range validation and remove the range check in
devlink_nl_cmd_reload
RFCv4 -> RFCv5:
- Always pass actions_performed to unload_up() instead of checking in
each driver
- Verify returned actions_performed includes the requested action
- Changed devlink_reload_actions_verify(devlink) to get ops
- Changed devlink_reload_actions_verify() to return bool and rename to
devlink_reload_actions_valid()
- Only generate the reply if request uses new attributes
RFCv3 -> RFCv4:
- Removed fw_activate_no_reset as an action (next patch adds limit
levels instead).
- Renamed actions_done to actions_performed
RFCv2 -> RFCv3:
- Replace fw_live_patch action by fw_activate_no_reset
- Devlink reload returns the actions done over netlink reply
RFCv1 -> RFCv2:
- Instead of reload levels driver,fw_reset,fw_live_patch have reload
actions driver_reinit,fw_activate,fw_live_patch
- Remove driver default level, the action driver_reinit is the default
action for all drivers
---
drivers/net/ethernet/mellanox/mlx4/main.c | 7 +-
.../net/ethernet/mellanox/mlx5/core/devlink.c | 7 +-
drivers/net/ethernet/mellanox/mlxsw/core.c | 11 +-
drivers/net/netdevsim/dev.c | 8 +-
include/net/devlink.h | 7 +-
include/uapi/linux/devlink.h | 18 ++++
net/core/devlink.c | 101 ++++++++++++++++--
7 files changed, 139 insertions(+), 20 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 70cf24ba71e4..a44d8b733db3 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3946,6 +3946,7 @@ static int mlx4_restart_one_up(struct pci_dev *pdev, bool reload,
struct devlink *devlink);
static int mlx4_devlink_reload_down(struct devlink *devlink, bool netns_change,
+ enum devlink_reload_action action,
struct netlink_ext_ack *extack)
{
struct mlx4_priv *priv = devlink_priv(devlink);
@@ -3962,14 +3963,15 @@ static int mlx4_devlink_reload_down(struct devlink *devlink, bool netns_change,
return 0;
}
-static int mlx4_devlink_reload_up(struct devlink *devlink,
- struct netlink_ext_ack *extack)
+static int mlx4_devlink_reload_up(struct devlink *devlink, enum devlink_reload_action action,
+ struct netlink_ext_ack *extack, unsigned long *actions_performed)
{
struct mlx4_priv *priv = devlink_priv(devlink);
struct mlx4_dev *dev = &priv->dev;
struct mlx4_dev_persistent *persist = dev->persist;
int err;
+ *actions_performed = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT);
err = mlx4_restart_one_up(persist->pdev, true, devlink);
if (err)
mlx4_err(persist->dev, "mlx4_restart_one_up failed, ret=%d\n",
@@ -3980,6 +3982,7 @@ static int mlx4_devlink_reload_up(struct devlink *devlink,
static const struct devlink_ops mlx4_devlink_ops = {
.port_type_set = mlx4_devlink_port_type_set,
+ .reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT),
.reload_down = mlx4_devlink_reload_down,
.reload_up = mlx4_devlink_reload_up,
};
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 9b14e3f805a2..35aceab79a50 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -85,6 +85,7 @@ mlx5_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
}
static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
+ enum devlink_reload_action action,
struct netlink_ext_ack *extack)
{
struct mlx5_core_dev *dev = devlink_priv(devlink);
@@ -93,11 +94,12 @@ static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
return 0;
}
-static int mlx5_devlink_reload_up(struct devlink *devlink,
- struct netlink_ext_ack *extack)
+static int mlx5_devlink_reload_up(struct devlink *devlink, enum devlink_reload_action action,
+ struct netlink_ext_ack *extack, unsigned long *actions_performed)
{
struct mlx5_core_dev *dev = devlink_priv(devlink);
+ *actions_performed = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT);
return mlx5_load_one(dev, false);
}
@@ -114,6 +116,7 @@ static const struct devlink_ops mlx5_devlink_ops = {
#endif
.flash_update = mlx5_devlink_flash_update,
.info_get = mlx5_devlink_info_get,
+ .reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT),
.reload_down = mlx5_devlink_reload_down,
.reload_up = mlx5_devlink_reload_up,
};
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index a21afa56e3f7..35b800dba6f5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1414,7 +1414,7 @@ mlxsw_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
static int
mlxsw_devlink_core_bus_device_reload_down(struct devlink *devlink,
- bool netns_change,
+ bool netns_change, enum devlink_reload_action action,
struct netlink_ext_ack *extack)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
@@ -1427,11 +1427,14 @@ mlxsw_devlink_core_bus_device_reload_down(struct devlink *devlink,
}
static int
-mlxsw_devlink_core_bus_device_reload_up(struct devlink *devlink,
- struct netlink_ext_ack *extack)
+mlxsw_devlink_core_bus_device_reload_up(struct devlink *devlink, enum devlink_reload_action action,
+ struct netlink_ext_ack *extack,
+ unsigned long *actions_performed)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
+ *actions_performed = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT) |
+ BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE);
return mlxsw_core_bus_device_register(mlxsw_core->bus_info,
mlxsw_core->bus,
mlxsw_core->bus_priv, true,
@@ -1564,6 +1567,8 @@ mlxsw_devlink_trap_policer_counter_get(struct devlink *devlink,
}
static const struct devlink_ops mlxsw_devlink_ops = {
+ .reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT) |
+ BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE),
.reload_down = mlxsw_devlink_core_bus_device_reload_down,
.reload_up = mlxsw_devlink_core_bus_device_reload_up,
.port_type_set = mlxsw_devlink_port_type_set,
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index 56213ba151f6..e1973d752148 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -701,7 +701,7 @@ static int nsim_dev_reload_create(struct nsim_dev *nsim_dev,
static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev);
static int nsim_dev_reload_down(struct devlink *devlink, bool netns_change,
- struct netlink_ext_ack *extack)
+ enum devlink_reload_action action, struct netlink_ext_ack *extack)
{
struct nsim_dev *nsim_dev = devlink_priv(devlink);
@@ -717,8 +717,8 @@ static int nsim_dev_reload_down(struct devlink *devlink, bool netns_change,
return 0;
}
-static int nsim_dev_reload_up(struct devlink *devlink,
- struct netlink_ext_ack *extack)
+static int nsim_dev_reload_up(struct devlink *devlink, enum devlink_reload_action action,
+ struct netlink_ext_ack *extack, unsigned long *actions_performed)
{
struct nsim_dev *nsim_dev = devlink_priv(devlink);
@@ -730,6 +730,7 @@ static int nsim_dev_reload_up(struct devlink *devlink,
return -EINVAL;
}
+ *actions_performed = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT);
return nsim_dev_reload_create(nsim_dev, extack);
}
@@ -886,6 +887,7 @@ nsim_dev_devlink_trap_policer_counter_get(struct devlink *devlink,
static const struct devlink_ops nsim_dev_devlink_ops = {
.supported_flash_update_params = DEVLINK_SUPPORT_FLASH_UPDATE_COMPONENT |
DEVLINK_SUPPORT_FLASH_UPDATE_OVERWRITE_MASK,
+ .reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT),
.reload_down = nsim_dev_reload_down,
.reload_up = nsim_dev_reload_up,
.info_get = nsim_dev_info_get,
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 1c286e9a3590..ddba63bce7ad 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1077,10 +1077,11 @@ struct devlink_ops {
* implemementation.
*/
u32 supported_flash_update_params;
+ unsigned long reload_actions;
int (*reload_down)(struct devlink *devlink, bool netns_change,
- struct netlink_ext_ack *extack);
- int (*reload_up)(struct devlink *devlink,
- struct netlink_ext_ack *extack);
+ enum devlink_reload_action action, struct netlink_ext_ack *extack);
+ int (*reload_up)(struct devlink *devlink, enum devlink_reload_action action,
+ struct netlink_ext_ack *extack, unsigned long *actions_performed);
int (*port_type_set)(struct devlink_port *devlink_port,
enum devlink_port_type port_type);
int (*port_split)(struct devlink *devlink, unsigned int port_index,
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index ba467dc07852..f1782eaf0be4 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -298,6 +298,21 @@ enum {
DEVLINK_ATTR_TRAP_METADATA_TYPE_FA_COOKIE,
};
+/**
+ * enum devlink_reload_action - Reload action.
+ * @DEVLINK_RELOAD_ACTION_DRIVER_REINIT: Driver entities re-instantiation.
+ * @DEVLINK_RELOAD_ACTION_FW_ACTIVATE: FW activate.
+ */
+enum devlink_reload_action {
+ DEVLINK_RELOAD_ACTION_UNSPEC,
+ DEVLINK_RELOAD_ACTION_DRIVER_REINIT,
+ DEVLINK_RELOAD_ACTION_FW_ACTIVATE,
+
+ /* Add new reload actions above */
+ __DEVLINK_RELOAD_ACTION_MAX,
+ DEVLINK_RELOAD_ACTION_MAX = __DEVLINK_RELOAD_ACTION_MAX - 1
+};
+
enum devlink_attr {
/* don't change the order or add anything between, this is ABI! */
DEVLINK_ATTR_UNSPEC,
@@ -490,6 +505,9 @@ enum devlink_attr {
DEVLINK_ATTR_FLASH_UPDATE_STATUS_TIMEOUT, /* u64 */
DEVLINK_ATTR_FLASH_UPDATE_OVERWRITE_MASK, /* bitfield32 */
+ DEVLINK_ATTR_RELOAD_ACTION, /* u8 */
+ DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, /* u64 */
+
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 722a9431ff60..6f39dc76370e 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -462,6 +462,12 @@ static int devlink_nl_put_handle(struct sk_buff *msg, struct devlink *devlink)
return 0;
}
+static bool
+devlink_reload_action_is_supported(struct devlink *devlink, enum devlink_reload_action action)
+{
+ return test_bit(action, &devlink->ops->reload_actions);
+}
+
static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
enum devlink_command cmd, u32 portid,
u32 seq, int flags)
@@ -2967,28 +2973,68 @@ bool devlink_is_reload_failed(const struct devlink *devlink)
EXPORT_SYMBOL_GPL(devlink_is_reload_failed);
static int devlink_reload(struct devlink *devlink, struct net *dest_net,
- struct netlink_ext_ack *extack)
+ enum devlink_reload_action action, struct netlink_ext_ack *extack,
+ unsigned long *actions_performed)
{
int err;
if (!devlink->reload_enabled)
return -EOPNOTSUPP;
- err = devlink->ops->reload_down(devlink, !!dest_net, extack);
+ err = devlink->ops->reload_down(devlink, !!dest_net, action, extack);
if (err)
return err;
if (dest_net && !net_eq(dest_net, devlink_net(devlink)))
devlink_reload_netns_change(devlink, dest_net);
- err = devlink->ops->reload_up(devlink, extack);
+ err = devlink->ops->reload_up(devlink, action, extack, actions_performed);
devlink_reload_failed_set(devlink, !!err);
- return err;
+ if (err)
+ return err;
+
+ WARN_ON(!test_bit(action, actions_performed));
+ return 0;
+}
+
+static int
+devlink_nl_reload_actions_performed_snd(struct devlink *devlink,
+ unsigned long actions_performed,
+ enum devlink_command cmd, struct genl_info *info)
+{
+ struct sk_buff *msg;
+ void *hdr;
+
+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
+ if (!msg)
+ return -ENOMEM;
+
+ hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq, &devlink_nl_family, 0, cmd);
+ if (!hdr)
+ goto free_msg;
+
+ if (devlink_nl_put_handle(msg, devlink))
+ goto nla_put_failure;
+
+ if (nla_put_u64_64bit(msg, DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, actions_performed,
+ DEVLINK_ATTR_PAD))
+ goto nla_put_failure;
+ genlmsg_end(msg, hdr);
+
+ return genlmsg_reply(msg, info);
+
+nla_put_failure:
+ genlmsg_cancel(msg, hdr);
+free_msg:
+ nlmsg_free(msg);
+ return -EMSGSIZE;
}
static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
{
struct devlink *devlink = info->user_ptr[0];
+ enum devlink_reload_action action;
+ unsigned long actions_performed;
struct net *dest_net = NULL;
int err;
@@ -3009,12 +3055,30 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
return PTR_ERR(dest_net);
}
- err = devlink_reload(devlink, dest_net, info->extack);
+ if (info->attrs[DEVLINK_ATTR_RELOAD_ACTION])
+ action = nla_get_u8(info->attrs[DEVLINK_ATTR_RELOAD_ACTION]);
+ else
+ action = DEVLINK_RELOAD_ACTION_DRIVER_REINIT;
+
+ if (!devlink_reload_action_is_supported(devlink, action)) {
+ NL_SET_ERR_MSG_MOD(info->extack,
+ "Requested reload action is not supported by the driver");
+ return -EOPNOTSUPP;
+ }
+
+ err = devlink_reload(devlink, dest_net, action, info->extack, &actions_performed);
if (dest_net)
put_net(dest_net);
- return err;
+ if (err)
+ return err;
+ /* For backward compatibility generate reply only if attributes used by user */
+ if (!info->attrs[DEVLINK_ATTR_RELOAD_ACTION])
+ return 0;
+
+ return devlink_nl_reload_actions_performed_snd(devlink, actions_performed,
+ DEVLINK_CMD_RELOAD, info);
}
static int devlink_nl_flash_update_fill(struct sk_buff *msg,
@@ -7119,6 +7183,8 @@ static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_TRAP_POLICER_RATE] = { .type = NLA_U64 },
[DEVLINK_ATTR_TRAP_POLICER_BURST] = { .type = NLA_U64 },
[DEVLINK_ATTR_PORT_FUNCTION] = { .type = NLA_NESTED },
+ [DEVLINK_ATTR_RELOAD_ACTION] = NLA_POLICY_RANGE(NLA_U8, DEVLINK_RELOAD_ACTION_DRIVER_REINIT,
+ DEVLINK_RELOAD_ACTION_MAX),
};
static const struct genl_ops devlink_nl_ops[] = {
@@ -7452,6 +7518,21 @@ static struct genl_family devlink_nl_family __ro_after_init = {
.n_mcgrps = ARRAY_SIZE(devlink_nl_mcgrps),
};
+static bool devlink_reload_actions_valid(const struct devlink_ops *ops)
+{
+ if (!devlink_reload_supported(ops)) {
+ if (WARN_ON(ops->reload_actions))
+ return false;
+ return true;
+ }
+
+ if (WARN_ON(!ops->reload_actions ||
+ ops->reload_actions & BIT(DEVLINK_RELOAD_ACTION_UNSPEC) ||
+ ops->reload_actions >= BIT(__DEVLINK_RELOAD_ACTION_MAX)))
+ return false;
+ return true;
+}
+
/**
* devlink_alloc - Allocate new devlink instance resources
*
@@ -7468,6 +7549,9 @@ struct devlink *devlink_alloc(const struct devlink_ops *ops, size_t priv_size)
if (WARN_ON(!ops))
return NULL;
+ if (!devlink_reload_actions_valid(ops))
+ return NULL;
+
devlink = kzalloc(sizeof(*devlink) + priv_size, GFP_KERNEL);
if (!devlink)
return NULL;
@@ -9724,6 +9808,7 @@ int devlink_compat_switch_id_get(struct net_device *dev,
static void __net_exit devlink_pernet_pre_exit(struct net *net)
{
+ unsigned long actions_performed;
struct devlink *devlink;
int err;
@@ -9735,7 +9820,9 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
if (net_eq(devlink_net(devlink), net)) {
if (WARN_ON(!devlink_reload_supported(devlink->ops)))
continue;
- err = devlink_reload(devlink, &init_net, NULL);
+ err = devlink_reload(devlink, &init_net,
+ DEVLINK_RELOAD_ACTION_DRIVER_REINIT,
+ NULL, &actions_performed);
if (err && err != -EOPNOTSUPP)
pr_warn("Failed to reload devlink instance into init_net\n");
}
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command
2020-10-01 13:59 ` [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command Moshe Shemesh
@ 2020-10-01 20:59 ` Jakub Kicinski
2020-10-02 15:19 ` Jiri Pirko
` (2 subsequent siblings)
3 siblings, 0 replies; 49+ messages in thread
From: Jakub Kicinski @ 2020-10-01 20:59 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: David S. Miller, Jiri Pirko, netdev, linux-kernel
On Thu, 1 Oct 2020 16:59:05 +0300 Moshe Shemesh wrote:
> Add devlink reload action to allow the user to request a specific reload
> action. The action parameter is optional, if not specified then devlink
> driver re-init action is used (backward compatible).
> Note that when required to do firmware activation some drivers may need
> to reload the driver. On the other hand some drivers may need to reset
> the firmware to reinitialize the driver entities. Therefore, the devlink
> reload command returns the actions which were actually performed.
> Reload actions supported are:
> driver_reinit: driver entities re-initialization, applying devlink-param
> and devlink-resource values.
> fw_activate: firmware activate.
>
> command examples:
> $devlink dev reload pci/0000:82:00.0 action driver_reinit
> reload_actions_performed:
> driver_reinit
>
> $devlink dev reload pci/0000:82:00.0 action fw_activate
> reload_actions_performed:
> driver_reinit fw_activate
>
> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command
2020-10-01 13:59 ` [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command Moshe Shemesh
2020-10-01 20:59 ` Jakub Kicinski
@ 2020-10-02 15:19 ` Jiri Pirko
2020-10-04 6:30 ` Moshe Shemesh
2020-10-03 7:52 ` Jiri Pirko
2020-10-05 18:39 ` Jacob Keller
3 siblings, 1 reply; 49+ messages in thread
From: Jiri Pirko @ 2020-10-02 15:19 UTC (permalink / raw)
To: Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
Thu, Oct 01, 2020 at 03:59:05PM CEST, moshe@mellanox.com wrote:
[...]
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 1c286e9a3590..ddba63bce7ad 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -1077,10 +1077,11 @@ struct devlink_ops {
> * implemementation.
> */
> u32 supported_flash_update_params;
>+ unsigned long reload_actions;
> int (*reload_down)(struct devlink *devlink, bool netns_change,
>- struct netlink_ext_ack *extack);
>- int (*reload_up)(struct devlink *devlink,
>- struct netlink_ext_ack *extack);
>+ enum devlink_reload_action action, struct netlink_ext_ack *extack);
>+ int (*reload_up)(struct devlink *devlink, enum devlink_reload_action action,
>+ struct netlink_ext_ack *extack, unsigned long *actions_performed);
Nit. Could you please push extack to be the last arg here? It is common
to have extack as the last arg + action and actions_performed are going
to be side by side.
Otherwise the patch looks fine.
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command
2020-10-02 15:19 ` Jiri Pirko
@ 2020-10-04 6:30 ` Moshe Shemesh
0 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-04 6:30 UTC (permalink / raw)
To: Jiri Pirko, Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
On 10/2/2020 6:19 PM, Jiri Pirko wrote:
> Thu, Oct 01, 2020 at 03:59:05PM CEST, moshe@mellanox.com wrote:
>
> [...]
>
>
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index 1c286e9a3590..ddba63bce7ad 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -1077,10 +1077,11 @@ struct devlink_ops {
>> * implemementation.
>> */
>> u32 supported_flash_update_params;
>> + unsigned long reload_actions;
>> int (*reload_down)(struct devlink *devlink, bool netns_change,
>> - struct netlink_ext_ack *extack);
>> - int (*reload_up)(struct devlink *devlink,
>> - struct netlink_ext_ack *extack);
>> + enum devlink_reload_action action, struct netlink_ext_ack *extack);
>> + int (*reload_up)(struct devlink *devlink, enum devlink_reload_action action,
>> + struct netlink_ext_ack *extack, unsigned long *actions_performed);
> Nit. Could you please push extack to be the last arg here? It is common
> to have extack as the last arg + action and actions_performed are going
> to be side by side.
Sure.
> Otherwise the patch looks fine.
> Reviewed-by: Jiri Pirko <jiri@nvidia.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command
2020-10-01 13:59 ` [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command Moshe Shemesh
2020-10-01 20:59 ` Jakub Kicinski
2020-10-02 15:19 ` Jiri Pirko
@ 2020-10-03 7:52 ` Jiri Pirko
2020-10-04 6:45 ` Moshe Shemesh
2020-10-05 18:39 ` Jacob Keller
3 siblings, 1 reply; 49+ messages in thread
From: Jiri Pirko @ 2020-10-03 7:52 UTC (permalink / raw)
To: Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
Thu, Oct 01, 2020 at 03:59:05PM CEST, moshe@mellanox.com wrote:
[...]
>+static int
>+devlink_nl_reload_actions_performed_snd(struct devlink *devlink,
>+ unsigned long actions_performed,
>+ enum devlink_command cmd, struct genl_info *info)
>+{
>+ struct sk_buff *msg;
>+ void *hdr;
>+
>+ msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>+ if (!msg)
>+ return -ENOMEM;
>+
>+ hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq, &devlink_nl_family, 0, cmd);
>+ if (!hdr)
>+ goto free_msg;
>+
>+ if (devlink_nl_put_handle(msg, devlink))
>+ goto nla_put_failure;
>+
>+ if (nla_put_u64_64bit(msg, DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, actions_performed,
This should be NLA_BITFIELD, I believe. We use it for other bitfields
too.
>+ DEVLINK_ATTR_PAD))
[...]
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command
2020-10-03 7:52 ` Jiri Pirko
@ 2020-10-04 6:45 ` Moshe Shemesh
0 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-04 6:45 UTC (permalink / raw)
To: Jiri Pirko, Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
On 10/3/2020 10:52 AM, Jiri Pirko wrote:
> Thu, Oct 01, 2020 at 03:59:05PM CEST, moshe@mellanox.com wrote:
>
> [...]
>
>> +static int
>> +devlink_nl_reload_actions_performed_snd(struct devlink *devlink,
>> + unsigned long actions_performed,
>> + enum devlink_command cmd, struct genl_info *info)
>> +{
>> + struct sk_buff *msg;
>> + void *hdr;
>> +
>> + msg = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_KERNEL);
>> + if (!msg)
>> + return -ENOMEM;
>> +
>> + hdr = genlmsg_put(msg, info->snd_portid, info->snd_seq, &devlink_nl_family, 0, cmd);
>> + if (!hdr)
>> + goto free_msg;
>> +
>> + if (devlink_nl_put_handle(msg, devlink))
>> + goto nla_put_failure;
>> +
>> + if (nla_put_u64_64bit(msg, DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, actions_performed,
> This should be NLA_BITFIELD, I believe. We use it for other bitfields
> too.
>
OK, I see it now, NLA_BITFIELD32.
>> + DEVLINK_ATTR_PAD))
> [...]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command
2020-10-01 13:59 ` [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command Moshe Shemesh
` (2 preceding siblings ...)
2020-10-03 7:52 ` Jiri Pirko
@ 2020-10-05 18:39 ` Jacob Keller
3 siblings, 0 replies; 49+ messages in thread
From: Jacob Keller @ 2020-10-05 18:39 UTC (permalink / raw)
To: Moshe Shemesh, David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel
On 10/1/2020 6:59 AM, Moshe Shemesh wrote:
> Add devlink reload action to allow the user to request a specific reload
> action. The action parameter is optional, if not specified then devlink
> driver re-init action is used (backward compatible).
> Note that when required to do firmware activation some drivers may need
> to reload the driver. On the other hand some drivers may need to reset
> the firmware to reinitialize the driver entities. Therefore, the devlink
> reload command returns the actions which were actually performed.
> Reload actions supported are:
> driver_reinit: driver entities re-initialization, applying devlink-param
> and devlink-resource values.
> fw_activate: firmware activate.
>
> command examples:
> $devlink dev reload pci/0000:82:00.0 action driver_reinit
> reload_actions_performed:
> driver_reinit
>
> $devlink dev reload pci/0000:82:00.0 action fw_activate
> reload_actions_performed:
> driver_reinit fw_activate
>
> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Looks straight forward.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
> ---
> RFCv5 -> v1:
> - Rename supported_reload_actions to reload_actions.
> - Rename devlink_nl_reload_actions_performed_fill() to
> devlink_nl_reload_actions_performed_snd() and add genlmsg_reply() to it
> - Actions_performed sent to user space as a mask
> - Driver can initialize actions_performed before done as devlink ignores
> in case of failure
> - Use nla_poilcy range validation and remove the range check in
> devlink_nl_cmd_reload
> RFCv4 -> RFCv5:
> - Always pass actions_performed to unload_up() instead of checking in
> each driver
> - Verify returned actions_performed includes the requested action
> - Changed devlink_reload_actions_verify(devlink) to get ops
> - Changed devlink_reload_actions_verify() to return bool and rename to
> devlink_reload_actions_valid()
> - Only generate the reply if request uses new attributes
> RFCv3 -> RFCv4:
> - Removed fw_activate_no_reset as an action (next patch adds limit
> levels instead).
> - Renamed actions_done to actions_performed
> RFCv2 -> RFCv3:
> - Replace fw_live_patch action by fw_activate_no_reset
> - Devlink reload returns the actions done over netlink reply
> RFCv1 -> RFCv2:
> - Instead of reload levels driver,fw_reset,fw_live_patch have reload
> actions driver_reinit,fw_activate,fw_live_patch
> - Remove driver default level, the action driver_reinit is the default
> action for all drivers
> ---
> drivers/net/ethernet/mellanox/mlx4/main.c | 7 +-
> .../net/ethernet/mellanox/mlx5/core/devlink.c | 7 +-
> drivers/net/ethernet/mellanox/mlxsw/core.c | 11 +-
> drivers/net/netdevsim/dev.c | 8 +-
> include/net/devlink.h | 7 +-
> include/uapi/linux/devlink.h | 18 ++++
> net/core/devlink.c | 101 ++++++++++++++++--
> 7 files changed, 139 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
> index 70cf24ba71e4..a44d8b733db3 100644
> --- a/drivers/net/ethernet/mellanox/mlx4/main.c
> +++ b/drivers/net/ethernet/mellanox/mlx4/main.c
> @@ -3946,6 +3946,7 @@ static int mlx4_restart_one_up(struct pci_dev *pdev, bool reload,
> struct devlink *devlink);
>
> static int mlx4_devlink_reload_down(struct devlink *devlink, bool netns_change,
> + enum devlink_reload_action action,
> struct netlink_ext_ack *extack)
> {
I might have opted to convert reload_down to take a structure of
parameters, given that we're about to add 2 parameters. Not really that
significant either way.
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH net-next 03/16] devlink: Add devlink reload limit option
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 01/16] devlink: Change devlink_reload_supported() param type Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 02/16] devlink: Add reload action option to devlink reload command Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 21:14 ` Jakub Kicinski
` (3 more replies)
2020-10-01 13:59 ` [PATCH net-next 04/16] devlink: Add reload stats Moshe Shemesh
` (12 subsequent siblings)
15 siblings, 4 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Add reload limit to demand restrictions on reload actions.
Reload limits supported:
no_reset: No reset allowed, no down time allowed, no link flap and no
configuration is lost.
By default reload limit is unspecified and so no constrains on reload
actions are required.
Some combinations of action and limit are invalid. For example, driver
can not reinitialize its entities without any downtime.
The no_reset reload limit will have usecase in this patchset to
implement restricted fw_activate on mlx5.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
---
RFCv5 -> v1:
- Renamed supported_reload_actions_limit_levels to reload_limits
- Renamed reload_action_limit_level to reload_limit
- Change RELOAD_LIMIT_NONE to unspecified RELOAD_LIMIT_UNSPEC,
drivers don't need to declare support limits if they support only
no limitation
- Use nla_poilcy range validation and remove the range check in
devlink_nl_cmd_reload
RFCv4 -> RFCv5:
- Remove check DEVLINK_RELOAD_ACTION_LIMIT_LEVEL_MAX
- Added list of invalid action-limit_level combinations and add check to
supported actions and levels and check user request
RFCv3 -> RFCv4:
- New patch
---
drivers/net/ethernet/mellanox/mlx4/main.c | 4 +-
.../net/ethernet/mellanox/mlx5/core/devlink.c | 2 +
drivers/net/ethernet/mellanox/mlxsw/core.c | 2 +
drivers/net/netdevsim/dev.c | 6 +-
include/net/devlink.h | 6 +-
include/uapi/linux/devlink.h | 18 +++++
net/core/devlink.c | 76 +++++++++++++++++--
7 files changed, 104 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index a44d8b733db3..f07287ebe473 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -3947,6 +3947,7 @@ static int mlx4_restart_one_up(struct pci_dev *pdev, bool reload,
static int mlx4_devlink_reload_down(struct devlink *devlink, bool netns_change,
enum devlink_reload_action action,
+ enum devlink_reload_limit limit,
struct netlink_ext_ack *extack)
{
struct mlx4_priv *priv = devlink_priv(devlink);
@@ -3964,7 +3965,8 @@ static int mlx4_devlink_reload_down(struct devlink *devlink, bool netns_change,
}
static int mlx4_devlink_reload_up(struct devlink *devlink, enum devlink_reload_action action,
- struct netlink_ext_ack *extack, unsigned long *actions_performed)
+ enum devlink_reload_limit limit, struct netlink_ext_ack *extack,
+ unsigned long *actions_performed)
{
struct mlx4_priv *priv = devlink_priv(devlink);
struct mlx4_dev *dev = &priv->dev;
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 35aceab79a50..47f3dbc80c7c 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -86,6 +86,7 @@ mlx5_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
enum devlink_reload_action action,
+ enum devlink_reload_limit limit,
struct netlink_ext_ack *extack)
{
struct mlx5_core_dev *dev = devlink_priv(devlink);
@@ -95,6 +96,7 @@ static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
}
static int mlx5_devlink_reload_up(struct devlink *devlink, enum devlink_reload_action action,
+ enum devlink_reload_limit limit,
struct netlink_ext_ack *extack, unsigned long *actions_performed)
{
struct mlx5_core_dev *dev = devlink_priv(devlink);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/core.c b/drivers/net/ethernet/mellanox/mlxsw/core.c
index 35b800dba6f5..6d9026ba0f5e 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/core.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/core.c
@@ -1415,6 +1415,7 @@ mlxsw_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
static int
mlxsw_devlink_core_bus_device_reload_down(struct devlink *devlink,
bool netns_change, enum devlink_reload_action action,
+ enum devlink_reload_limit limit,
struct netlink_ext_ack *extack)
{
struct mlxsw_core *mlxsw_core = devlink_priv(devlink);
@@ -1428,6 +1429,7 @@ mlxsw_devlink_core_bus_device_reload_down(struct devlink *devlink,
static int
mlxsw_devlink_core_bus_device_reload_up(struct devlink *devlink, enum devlink_reload_action action,
+ enum devlink_reload_limit limit,
struct netlink_ext_ack *extack,
unsigned long *actions_performed)
{
diff --git a/drivers/net/netdevsim/dev.c b/drivers/net/netdevsim/dev.c
index e1973d752148..c2113af47b61 100644
--- a/drivers/net/netdevsim/dev.c
+++ b/drivers/net/netdevsim/dev.c
@@ -701,7 +701,8 @@ static int nsim_dev_reload_create(struct nsim_dev *nsim_dev,
static void nsim_dev_reload_destroy(struct nsim_dev *nsim_dev);
static int nsim_dev_reload_down(struct devlink *devlink, bool netns_change,
- enum devlink_reload_action action, struct netlink_ext_ack *extack)
+ enum devlink_reload_action action, enum devlink_reload_limit limit,
+ struct netlink_ext_ack *extack)
{
struct nsim_dev *nsim_dev = devlink_priv(devlink);
@@ -718,7 +719,8 @@ static int nsim_dev_reload_down(struct devlink *devlink, bool netns_change,
}
static int nsim_dev_reload_up(struct devlink *devlink, enum devlink_reload_action action,
- struct netlink_ext_ack *extack, unsigned long *actions_performed)
+ enum devlink_reload_limit limit, struct netlink_ext_ack *extack,
+ unsigned long *actions_performed)
{
struct nsim_dev *nsim_dev = devlink_priv(devlink);
diff --git a/include/net/devlink.h b/include/net/devlink.h
index ddba63bce7ad..43dde69086e5 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -1078,9 +1078,13 @@ struct devlink_ops {
*/
u32 supported_flash_update_params;
unsigned long reload_actions;
+ unsigned long reload_limits;
int (*reload_down)(struct devlink *devlink, bool netns_change,
- enum devlink_reload_action action, struct netlink_ext_ack *extack);
+ enum devlink_reload_action action,
+ enum devlink_reload_limit limit,
+ struct netlink_ext_ack *extack);
int (*reload_up)(struct devlink *devlink, enum devlink_reload_action action,
+ enum devlink_reload_limit limit,
struct netlink_ext_ack *extack, unsigned long *actions_performed);
int (*port_type_set)(struct devlink_port *devlink_port,
enum devlink_port_type port_type);
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index f1782eaf0be4..cc5dc4c07b4a 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -313,6 +313,23 @@ enum devlink_reload_action {
DEVLINK_RELOAD_ACTION_MAX = __DEVLINK_RELOAD_ACTION_MAX - 1
};
+/**
+ * enum devlink_reload_limit - Reload limit.
+ * @DEVLINK_RELOAD_LIMIT_NO_RESET: No reset allowed, no down time allowed,
+ * no link flap and no configuration is lost.
+ *
+ * Note that by default reload limit is unspecified and so no constrains on
+ * reload action.
+ */
+enum devlink_reload_limit {
+ DEVLINK_RELOAD_LIMIT_UNSPEC,
+ DEVLINK_RELOAD_LIMIT_NO_RESET,
+
+ /* Add new reload limit above */
+ __DEVLINK_RELOAD_LIMIT_MAX,
+ DEVLINK_RELOAD_LIMIT_MAX = __DEVLINK_RELOAD_LIMIT_MAX - 1
+};
+
enum devlink_attr {
/* don't change the order or add anything between, this is ABI! */
DEVLINK_ATTR_UNSPEC,
@@ -507,6 +524,7 @@ enum devlink_attr {
DEVLINK_ATTR_RELOAD_ACTION, /* u8 */
DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, /* u64 */
+ DEVLINK_ATTR_RELOAD_LIMIT, /* u8 */
/* add new attributes above here, update the policy in devlink.c */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 6f39dc76370e..6de7d6aa6ed1 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -462,12 +462,44 @@ static int devlink_nl_put_handle(struct sk_buff *msg, struct devlink *devlink)
return 0;
}
+struct devlink_reload_combination {
+ enum devlink_reload_action action;
+ enum devlink_reload_limit limit;
+};
+
+static const struct devlink_reload_combination devlink_reload_invalid_combinations[] = {
+ {
+ /* can't reinitialize driver with no down time */
+ .action = DEVLINK_RELOAD_ACTION_DRIVER_REINIT,
+ .limit = DEVLINK_RELOAD_LIMIT_NO_RESET,
+ },
+};
+
+static bool
+devlink_reload_combination_is_invalid(enum devlink_reload_action action,
+ enum devlink_reload_limit limit)
+{
+ int i;
+
+ for (i = 0; i < ARRAY_SIZE(devlink_reload_invalid_combinations); i++)
+ if (devlink_reload_invalid_combinations[i].action == action &&
+ devlink_reload_invalid_combinations[i].limit == limit)
+ return true;
+ return false;
+}
+
static bool
devlink_reload_action_is_supported(struct devlink *devlink, enum devlink_reload_action action)
{
return test_bit(action, &devlink->ops->reload_actions);
}
+static bool
+devlink_reload_limit_is_supported(struct devlink *devlink, enum devlink_reload_limit limit)
+{
+ return test_bit(limit, &devlink->ops->reload_limits);
+}
+
static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
enum devlink_command cmd, u32 portid,
u32 seq, int flags)
@@ -2973,22 +3005,22 @@ bool devlink_is_reload_failed(const struct devlink *devlink)
EXPORT_SYMBOL_GPL(devlink_is_reload_failed);
static int devlink_reload(struct devlink *devlink, struct net *dest_net,
- enum devlink_reload_action action, struct netlink_ext_ack *extack,
- unsigned long *actions_performed)
+ enum devlink_reload_action action, enum devlink_reload_limit limit,
+ struct netlink_ext_ack *extack, unsigned long *actions_performed)
{
int err;
if (!devlink->reload_enabled)
return -EOPNOTSUPP;
- err = devlink->ops->reload_down(devlink, !!dest_net, action, extack);
+ err = devlink->ops->reload_down(devlink, !!dest_net, action, limit, extack);
if (err)
return err;
if (dest_net && !net_eq(dest_net, devlink_net(devlink)))
devlink_reload_netns_change(devlink, dest_net);
- err = devlink->ops->reload_up(devlink, action, extack, actions_performed);
+ err = devlink->ops->reload_up(devlink, action, limit, extack, actions_performed);
devlink_reload_failed_set(devlink, !!err);
if (err)
return err;
@@ -3032,6 +3064,7 @@ devlink_nl_reload_actions_performed_snd(struct devlink *devlink,
static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
{
+ enum devlink_reload_limit limit;
struct devlink *devlink = info->user_ptr[0];
enum devlink_reload_action action;
unsigned long actions_performed;
@@ -3066,7 +3099,21 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
return -EOPNOTSUPP;
}
- err = devlink_reload(devlink, dest_net, action, info->extack, &actions_performed);
+ limit = DEVLINK_RELOAD_LIMIT_UNSPEC;
+ if (info->attrs[DEVLINK_ATTR_RELOAD_LIMIT]) {
+ limit = nla_get_u8(info->attrs[DEVLINK_ATTR_RELOAD_LIMIT]);
+ if (!devlink_reload_limit_is_supported(devlink, limit)) {
+ NL_SET_ERR_MSG_MOD(info->extack,
+ "Requested limit is not supported by the driver");
+ return -EOPNOTSUPP;
+ }
+ if (devlink_reload_combination_is_invalid(action, limit)) {
+ NL_SET_ERR_MSG_MOD(info->extack,
+ "Requested limit is invalid for this action");
+ return -EINVAL;
+ }
+ }
+ err = devlink_reload(devlink, dest_net, action, limit, info->extack, &actions_performed);
if (dest_net)
put_net(dest_net);
@@ -3074,7 +3121,7 @@ static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
if (err)
return err;
/* For backward compatibility generate reply only if attributes used by user */
- if (!info->attrs[DEVLINK_ATTR_RELOAD_ACTION])
+ if (!info->attrs[DEVLINK_ATTR_RELOAD_ACTION] && !info->attrs[DEVLINK_ATTR_RELOAD_LIMIT])
return 0;
return devlink_nl_reload_actions_performed_snd(devlink, actions_performed,
@@ -7185,6 +7232,8 @@ static const struct nla_policy devlink_nl_policy[DEVLINK_ATTR_MAX + 1] = {
[DEVLINK_ATTR_PORT_FUNCTION] = { .type = NLA_NESTED },
[DEVLINK_ATTR_RELOAD_ACTION] = NLA_POLICY_RANGE(NLA_U8, DEVLINK_RELOAD_ACTION_DRIVER_REINIT,
DEVLINK_RELOAD_ACTION_MAX),
+ [DEVLINK_ATTR_RELOAD_LIMIT] = NLA_POLICY_RANGE(NLA_U8, DEVLINK_RELOAD_LIMIT_NO_RESET,
+ DEVLINK_RELOAD_LIMIT_MAX),
};
static const struct genl_ops devlink_nl_ops[] = {
@@ -7520,6 +7569,9 @@ static struct genl_family devlink_nl_family __ro_after_init = {
static bool devlink_reload_actions_valid(const struct devlink_ops *ops)
{
+ const struct devlink_reload_combination *comb;
+ int i;
+
if (!devlink_reload_supported(ops)) {
if (WARN_ON(ops->reload_actions))
return false;
@@ -7530,6 +7582,17 @@ static bool devlink_reload_actions_valid(const struct devlink_ops *ops)
ops->reload_actions & BIT(DEVLINK_RELOAD_ACTION_UNSPEC) ||
ops->reload_actions >= BIT(__DEVLINK_RELOAD_ACTION_MAX)))
return false;
+
+ if (WARN_ON(ops->reload_limits & BIT(DEVLINK_RELOAD_LIMIT_UNSPEC) ||
+ ops->reload_limits >= BIT(__DEVLINK_RELOAD_LIMIT_MAX)))
+ return false;
+
+ for (i = 0; i < ARRAY_SIZE(devlink_reload_invalid_combinations); i++) {
+ comb = &devlink_reload_invalid_combinations[i];
+ if (ops->reload_actions == BIT(comb->action) &&
+ ops->reload_limits == BIT(comb->limit))
+ return false;
+ }
return true;
}
@@ -9822,6 +9885,7 @@ static void __net_exit devlink_pernet_pre_exit(struct net *net)
continue;
err = devlink_reload(devlink, &init_net,
DEVLINK_RELOAD_ACTION_DRIVER_REINIT,
+ DEVLINK_RELOAD_LIMIT_UNSPEC,
NULL, &actions_performed);
if (err && err != -EOPNOTSUPP)
pr_warn("Failed to reload devlink instance into init_net\n");
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH net-next 03/16] devlink: Add devlink reload limit option
2020-10-01 13:59 ` [PATCH net-next 03/16] devlink: Add devlink reload limit option Moshe Shemesh
@ 2020-10-01 21:14 ` Jakub Kicinski
2020-10-02 15:01 ` Moshe Shemesh
2020-10-01 21:15 ` Jakub Kicinski
` (2 subsequent siblings)
3 siblings, 1 reply; 49+ messages in thread
From: Jakub Kicinski @ 2020-10-01 21:14 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: David S. Miller, Jiri Pirko, netdev, linux-kernel
On Thu, 1 Oct 2020 16:59:06 +0300 Moshe Shemesh wrote:
> @@ -3032,6 +3064,7 @@ devlink_nl_reload_actions_performed_snd(struct devlink *devlink,
>
> static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
> {
> + enum devlink_reload_limit limit;
> struct devlink *devlink = info->user_ptr[0];
> enum devlink_reload_action action;
> unsigned long actions_performed;
reverse xmas tree
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 03/16] devlink: Add devlink reload limit option
2020-10-01 21:14 ` Jakub Kicinski
@ 2020-10-02 15:01 ` Moshe Shemesh
0 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-02 15:01 UTC (permalink / raw)
To: Jakub Kicinski, Moshe Shemesh
Cc: David S. Miller, Jiri Pirko, netdev, linux-kernel
On 10/2/2020 12:14 AM, Jakub Kicinski wrote:
> External email: Use caution opening links or attachments
>
>
> On Thu, 1 Oct 2020 16:59:06 +0300 Moshe Shemesh wrote:
>> @@ -3032,6 +3064,7 @@ devlink_nl_reload_actions_performed_snd(struct devlink *devlink,
>>
>> static int devlink_nl_cmd_reload(struct sk_buff *skb, struct genl_info *info)
>> {
>> + enum devlink_reload_limit limit;
>> struct devlink *devlink = info->user_ptr[0];
>> enum devlink_reload_action action;
>> unsigned long actions_performed;
> reverse xmas tree
missed that, thanks.
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH net-next 03/16] devlink: Add devlink reload limit option
2020-10-01 13:59 ` [PATCH net-next 03/16] devlink: Add devlink reload limit option Moshe Shemesh
2020-10-01 21:14 ` Jakub Kicinski
@ 2020-10-01 21:15 ` Jakub Kicinski
2020-10-03 7:51 ` Jiri Pirko
2020-10-05 18:45 ` Jacob Keller
3 siblings, 0 replies; 49+ messages in thread
From: Jakub Kicinski @ 2020-10-01 21:15 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: David S. Miller, Jiri Pirko, netdev, linux-kernel
On Thu, 1 Oct 2020 16:59:06 +0300 Moshe Shemesh wrote:
> Add reload limit to demand restrictions on reload actions.
> Reload limits supported:
> no_reset: No reset allowed, no down time allowed, no link flap and no
> configuration is lost.
>
> By default reload limit is unspecified and so no constrains on reload
> actions are required.
>
> Some combinations of action and limit are invalid. For example, driver
> can not reinitialize its entities without any downtime.
>
> The no_reset reload limit will have usecase in this patchset to
> implement restricted fw_activate on mlx5.
>
> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Other than the nit:
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH net-next 03/16] devlink: Add devlink reload limit option
2020-10-01 13:59 ` [PATCH net-next 03/16] devlink: Add devlink reload limit option Moshe Shemesh
2020-10-01 21:14 ` Jakub Kicinski
2020-10-01 21:15 ` Jakub Kicinski
@ 2020-10-03 7:51 ` Jiri Pirko
2020-10-03 15:04 ` Jakub Kicinski
2020-10-04 6:42 ` Moshe Shemesh
2020-10-05 18:45 ` Jacob Keller
3 siblings, 2 replies; 49+ messages in thread
From: Jiri Pirko @ 2020-10-03 7:51 UTC (permalink / raw)
To: Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
Thu, Oct 01, 2020 at 03:59:06PM CEST, moshe@mellanox.com wrote:
[...]
> enum devlink_attr {
> /* don't change the order or add anything between, this is ABI! */
> DEVLINK_ATTR_UNSPEC,
>@@ -507,6 +524,7 @@ enum devlink_attr {
>
> DEVLINK_ATTR_RELOAD_ACTION, /* u8 */
> DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, /* u64 */
>+ DEVLINK_ATTR_RELOAD_LIMIT, /* u8 */
Hmm, why there could be specified only single "limit"? I believe this
should be a bitfield. Same for the internal api to the driver.
[...]
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 03/16] devlink: Add devlink reload limit option
2020-10-03 7:51 ` Jiri Pirko
@ 2020-10-03 15:04 ` Jakub Kicinski
2020-10-04 7:18 ` Moshe Shemesh
2020-10-05 18:53 ` Jacob Keller
2020-10-04 6:42 ` Moshe Shemesh
1 sibling, 2 replies; 49+ messages in thread
From: Jakub Kicinski @ 2020-10-03 15:04 UTC (permalink / raw)
To: Jiri Pirko
Cc: Moshe Shemesh, David S. Miller, Jiri Pirko, netdev, linux-kernel
On Sat, 3 Oct 2020 09:51:00 +0200 Jiri Pirko wrote:
> > enum devlink_attr {
> > /* don't change the order or add anything between, this is ABI! */
> > DEVLINK_ATTR_UNSPEC,
> >@@ -507,6 +524,7 @@ enum devlink_attr {
> >
> > DEVLINK_ATTR_RELOAD_ACTION, /* u8 */
> > DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, /* u64 */
> >+ DEVLINK_ATTR_RELOAD_LIMIT, /* u8 */
>
> Hmm, why there could be specified only single "limit"? I believe this
> should be a bitfield. Same for the internal api to the driver.
Hm I was expecting limits to be ordered (in maths sense) but you're
right perhaps that can't be always guaranteed.
Also - Moshe please double check that there will not be any kdoc
warnings here - I just learned that W=1 builds don't check headers
but I'll fix up my bot by the time you post v2.
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 03/16] devlink: Add devlink reload limit option
2020-10-03 15:04 ` Jakub Kicinski
@ 2020-10-04 7:18 ` Moshe Shemesh
2020-10-05 18:53 ` Jacob Keller
1 sibling, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-04 7:18 UTC (permalink / raw)
To: Jakub Kicinski, Jiri Pirko
Cc: Moshe Shemesh, David S. Miller, Jiri Pirko, netdev, linux-kernel
On 10/3/2020 6:04 PM, Jakub Kicinski wrote:
> External email: Use caution opening links or attachments
>
>
> On Sat, 3 Oct 2020 09:51:00 +0200 Jiri Pirko wrote:
>>> enum devlink_attr {
>>> /* don't change the order or add anything between, this is ABI! */
>>> DEVLINK_ATTR_UNSPEC,
>>> @@ -507,6 +524,7 @@ enum devlink_attr {
>>>
>>> DEVLINK_ATTR_RELOAD_ACTION, /* u8 */
>>> DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, /* u64 */
>>> + DEVLINK_ATTR_RELOAD_LIMIT, /* u8 */
>> Hmm, why there could be specified only single "limit"? I believe this
>> should be a bitfield. Same for the internal api to the driver.
> Hm I was expecting limits to be ordered (in maths sense) but you're
> right perhaps that can't be always guaranteed.
>
> Also - Moshe please double check that there will not be any kdoc
> warnings here - I just learned that W=1 builds don't check headers
> but I'll fix up my bot by the time you post v2.
Didn't know this tool, but I will. Thanks.
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 03/16] devlink: Add devlink reload limit option
2020-10-03 15:04 ` Jakub Kicinski
2020-10-04 7:18 ` Moshe Shemesh
@ 2020-10-05 18:53 ` Jacob Keller
1 sibling, 0 replies; 49+ messages in thread
From: Jacob Keller @ 2020-10-05 18:53 UTC (permalink / raw)
To: Jakub Kicinski, Jiri Pirko
Cc: Moshe Shemesh, David S. Miller, Jiri Pirko, netdev, linux-kernel
On 10/3/2020 8:04 AM, Jakub Kicinski wrote:
> On Sat, 3 Oct 2020 09:51:00 +0200 Jiri Pirko wrote:
>>> enum devlink_attr {
>>> /* don't change the order or add anything between, this is ABI! */
>>> DEVLINK_ATTR_UNSPEC,
>>> @@ -507,6 +524,7 @@ enum devlink_attr {
>>>
>>> DEVLINK_ATTR_RELOAD_ACTION, /* u8 */
>>> DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, /* u64 */
>>> + DEVLINK_ATTR_RELOAD_LIMIT, /* u8 */
>>
>> Hmm, why there could be specified only single "limit"? I believe this
>> should be a bitfield. Same for the internal api to the driver.
>
> Hm I was expecting limits to be ordered (in maths sense) but you're
> right perhaps that can't be always guaranteed.
>
> Also - Moshe please double check that there will not be any kdoc
> warnings here - I just learned that W=1 builds don't check headers
> but I'll fix up my bot by the time you post v2.
>
I think something like this got missed in one of my patches before...
I don't see anything obvious for this searching through the
Makefile.build... Mind sharing how you plan to fix your bot to check these?
Thanks,
Jake
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH net-next 03/16] devlink: Add devlink reload limit option
2020-10-03 7:51 ` Jiri Pirko
2020-10-03 15:04 ` Jakub Kicinski
@ 2020-10-04 6:42 ` Moshe Shemesh
2020-10-05 12:18 ` Jiri Pirko
1 sibling, 1 reply; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-04 6:42 UTC (permalink / raw)
To: Jiri Pirko, Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
On 10/3/2020 10:51 AM, Jiri Pirko wrote:
> Thu, Oct 01, 2020 at 03:59:06PM CEST, moshe@mellanox.com wrote:
>
> [...]
>
>> enum devlink_attr {
>> /* don't change the order or add anything between, this is ABI! */
>> DEVLINK_ATTR_UNSPEC,
>> @@ -507,6 +524,7 @@ enum devlink_attr {
>>
>> DEVLINK_ATTR_RELOAD_ACTION, /* u8 */
>> DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, /* u64 */
>> + DEVLINK_ATTR_RELOAD_LIMIT, /* u8 */
> Hmm, why there could be specified only single "limit"? I believe this
> should be a bitfield. Same for the internal api to the driver.
Why bitfield ? Either the user asks for a specific limit or he doesn't
ask for any (unspecified).
If the user doesn't need limitation he will not specify a limit.
> [...]
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 03/16] devlink: Add devlink reload limit option
2020-10-04 6:42 ` Moshe Shemesh
@ 2020-10-05 12:18 ` Jiri Pirko
0 siblings, 0 replies; 49+ messages in thread
From: Jiri Pirko @ 2020-10-05 12:18 UTC (permalink / raw)
To: Moshe Shemesh
Cc: Moshe Shemesh, David S. Miller, Jakub Kicinski, Jiri Pirko,
netdev, linux-kernel
Sun, Oct 04, 2020 at 08:42:47AM CEST, moshe@nvidia.com wrote:
>
>On 10/3/2020 10:51 AM, Jiri Pirko wrote:
>> Thu, Oct 01, 2020 at 03:59:06PM CEST, moshe@mellanox.com wrote:
>>
>> [...]
>>
>> > enum devlink_attr {
>> > /* don't change the order or add anything between, this is ABI! */
>> > DEVLINK_ATTR_UNSPEC,
>> > @@ -507,6 +524,7 @@ enum devlink_attr {
>> >
>> > DEVLINK_ATTR_RELOAD_ACTION, /* u8 */
>> > DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, /* u64 */
>> > + DEVLINK_ATTR_RELOAD_LIMIT, /* u8 */
>> Hmm, why there could be specified only single "limit"? I believe this
>> should be a bitfield. Same for the internal api to the driver.
>
>
>Why bitfield ? Either the user asks for a specific limit or he doesn't ask
>for any (unspecified).
He can ask for multiple limits: No_link_flag , no_something_else. Could
be totally unrelated limitations. Let's just have the UAPI ready for
this once we define it from scratch.
>
>If the user doesn't need limitation he will not specify a limit.
>
>> [...]
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH net-next 03/16] devlink: Add devlink reload limit option
2020-10-01 13:59 ` [PATCH net-next 03/16] devlink: Add devlink reload limit option Moshe Shemesh
` (2 preceding siblings ...)
2020-10-03 7:51 ` Jiri Pirko
@ 2020-10-05 18:45 ` Jacob Keller
3 siblings, 0 replies; 49+ messages in thread
From: Jacob Keller @ 2020-10-05 18:45 UTC (permalink / raw)
To: Moshe Shemesh, David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel
On 10/1/2020 6:59 AM, Moshe Shemesh wrote:
> Add reload limit to demand restrictions on reload actions.
> Reload limits supported:
> no_reset: No reset allowed, no down time allowed, no link flap and no
> configuration is lost.
>
> By default reload limit is unspecified and so no constrains on reload
> actions are required.
Nit: I think the spelling for the noun here would be "constraints"? Same
for a comment in the header file.
>
> Some combinations of action and limit are invalid. For example, driver
> can not reinitialize its entities without any downtime.
>
Good to see that checked in the core code.
> The no_reset reload limit will have usecase in this patchset to
> implement restricted fw_activate on mlx5.
>
> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
> ---
Other than the spelling hit and things pointed out by others, this looks
good to me.
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH net-next 04/16] devlink: Add reload stats
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (2 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 03/16] devlink: Add devlink reload limit option Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 21:25 ` Jakub Kicinski
2020-10-03 9:00 ` Jiri Pirko
2020-10-01 13:59 ` [PATCH net-next 05/16] devlink: Add remote " Moshe Shemesh
` (11 subsequent siblings)
15 siblings, 2 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Add reload stats to hold the history per reload action type and limit.
For example, the number of times fw_activate has been performed on this
device since the driver module was added or if the firmware activation
was performed with or without reset.
Add devlink notification on stats update.
Expose devlink reload stats to the user through devlink dev get command.
Examples:
$ devlink dev show
pci/0000:82:00.0:
stats:
reload_stats:
driver_reinit 2
fw_activate 1
fw_activate_no_reset 0
pci/0000:82:00.1:
stats:
reload_stats:
driver_reinit 1
fw_activate 0
fw_activate_no_reset 0
$ devlink dev show -jp
{
"dev": {
"pci/0000:82:00.0": {
"stats": {
"reload_stats": [ {
"driver_reinit": 2
},{
"fw_activate": 1
},{
"fw_activate_no_reset": 0
} ]
}
},
"pci/0000:82:00.1": {
"stats": {
"reload_stats": [ {
"driver_reinit": 1
},{
"fw_activate": 0
},{
"fw_activate_no_reset": 0
} ]
}
}
}
}
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
---
RFCv5 -> v1:
- Changed the stats output structure, have 2 stats, one for local and
one for remote
- Resplit this patch and the next one by remote/local reload stast
instead of set/get reload stats
- Add helper function devlink_reload_stats_put()
RFCv4 -> RFCv5:
- Add separate reload action stats for updating on remote actions
- Protect from updating remote actions stats during reload_down()/up()
RFCv3 -> RFCv4:
- Renamed reload_actions_cnts to reload_action_stats
- Add devlink notifications on stats update
- Renamed devlink_reload_actions_implicit_actions_performed() and add
function comment in code
RFCv2 -> RFCv3:
- New patch
---
include/net/devlink.h | 7 +++
include/uapi/linux/devlink.h | 5 ++
net/core/devlink.c | 97 ++++++++++++++++++++++++++++++++++++
3 files changed, 109 insertions(+)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 43dde69086e5..0f3bd23b6c04 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -20,6 +20,9 @@
#include <uapi/linux/devlink.h>
#include <linux/xarray.h>
+#define DEVLINK_RELOAD_STATS_ARRAY_SIZE \
+ (__DEVLINK_RELOAD_LIMIT_MAX * __DEVLINK_RELOAD_ACTION_MAX)
+
struct devlink_ops;
struct devlink {
@@ -38,6 +41,7 @@ struct devlink {
struct list_head trap_policer_list;
const struct devlink_ops *ops;
struct xarray snapshot_ids;
+ u32 reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
struct device *dev;
possible_net_t _net;
struct mutex lock; /* Serializes access to devlink instance specific objects such as
@@ -1470,6 +1474,9 @@ void
devlink_health_reporter_recovery_done(struct devlink_health_reporter *reporter);
bool devlink_is_reload_failed(const struct devlink *devlink);
+void devlink_remote_reload_actions_performed(struct devlink *devlink,
+ enum devlink_reload_limit limit,
+ unsigned long actions_performed);
void devlink_flash_update_begin_notify(struct devlink *devlink);
void devlink_flash_update_end_notify(struct devlink *devlink);
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index cc5dc4c07b4a..97e0137f6201 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -526,6 +526,11 @@ enum devlink_attr {
DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, /* u64 */
DEVLINK_ATTR_RELOAD_LIMIT, /* u8 */
+ DEVLINK_ATTR_DEV_STATS, /* nested */
+ DEVLINK_ATTR_RELOAD_STATS, /* nested */
+ DEVLINK_ATTR_RELOAD_STATS_ENTRY, /* nested */
+ DEVLINK_ATTR_RELOAD_STATS_VALUE, /* u32 */
+
/* add new attributes above here, update the policy in devlink.c */
__DEVLINK_ATTR_MAX,
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 6de7d6aa6ed1..05516f1e4c3e 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -500,10 +500,68 @@ devlink_reload_limit_is_supported(struct devlink *devlink, enum devlink_reload_l
return test_bit(limit, &devlink->ops->reload_limits);
}
+static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_action action,
+ enum devlink_reload_limit limit, u32 value)
+{
+ struct nlattr *reload_stats_entry;
+
+ reload_stats_entry = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS_ENTRY);
+ if (!reload_stats_entry)
+ return -EMSGSIZE;
+
+ if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_ACTION, action))
+ goto nla_put_failure;
+ if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_LIMIT, limit))
+ goto nla_put_failure;
+ if (nla_put_u32(msg, DEVLINK_ATTR_RELOAD_STATS_VALUE, value))
+ goto nla_put_failure;
+ nla_nest_end(msg, reload_stats_entry);
+ return 0;
+
+nla_put_failure:
+ nla_nest_cancel(msg, reload_stats_entry);
+ return -EMSGSIZE;
+}
+
+static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink)
+{
+ struct nlattr *reload_stats_attr;
+ int i, j, stat_idx;
+ u32 value;
+
+ reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
+
+ if (!reload_stats_attr)
+ return -EMSGSIZE;
+
+ for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) {
+ if (j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
+ !devlink_reload_limit_is_supported(devlink, j))
+ continue;
+ for (i = 0; i <= DEVLINK_RELOAD_ACTION_MAX; i++) {
+ if (!devlink_reload_action_is_supported(devlink, i) ||
+ devlink_reload_combination_is_invalid(i, j))
+ continue;
+
+ stat_idx = j * __DEVLINK_RELOAD_ACTION_MAX + i;
+ value = devlink->reload_stats[stat_idx];
+ if (devlink_reload_stat_put(msg, i, j, value))
+ goto nla_put_failure;
+ }
+ }
+ nla_nest_end(msg, reload_stats_attr);
+ return 0;
+
+nla_put_failure:
+ nla_nest_cancel(msg, reload_stats_attr);
+ return -EMSGSIZE;
+}
+
static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
enum devlink_command cmd, u32 portid,
u32 seq, int flags)
{
+ struct nlattr *dev_stats;
void *hdr;
hdr = genlmsg_put(msg, portid, seq, &devlink_nl_family, flags, cmd);
@@ -515,9 +573,19 @@ static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_FAILED, devlink->reload_failed))
goto nla_put_failure;
+ dev_stats = nla_nest_start(msg, DEVLINK_ATTR_DEV_STATS);
+ if (!dev_stats)
+ goto nla_put_failure;
+
+ if (devlink_reload_stats_put(msg, devlink))
+ goto dev_stats_nest_cancel;
+
+ nla_nest_end(msg, dev_stats);
genlmsg_end(msg, hdr);
return 0;
+dev_stats_nest_cancel:
+ nla_nest_cancel(msg, dev_stats);
nla_put_failure:
genlmsg_cancel(msg, hdr);
return -EMSGSIZE;
@@ -3004,6 +3072,34 @@ bool devlink_is_reload_failed(const struct devlink *devlink)
}
EXPORT_SYMBOL_GPL(devlink_is_reload_failed);
+static void
+__devlink_reload_stats_update(struct devlink *devlink, u32 *reload_stats,
+ enum devlink_reload_limit limit, unsigned long actions_performed)
+{
+ int stat_idx;
+ int action;
+
+ if (!actions_performed)
+ return;
+
+ if (WARN_ON(limit > DEVLINK_RELOAD_LIMIT_MAX))
+ return;
+ for (action = 0; action <= DEVLINK_RELOAD_ACTION_MAX; action++) {
+ if (!test_bit(action, &actions_performed))
+ continue;
+ stat_idx = limit * __DEVLINK_RELOAD_ACTION_MAX + action;
+ reload_stats[stat_idx]++;
+ }
+ devlink_notify(devlink, DEVLINK_CMD_NEW);
+}
+
+static void
+devlink_reload_stats_update(struct devlink *devlink, enum devlink_reload_limit limit,
+ unsigned long actions_performed)
+{
+ __devlink_reload_stats_update(devlink, devlink->reload_stats, limit, actions_performed);
+}
+
static int devlink_reload(struct devlink *devlink, struct net *dest_net,
enum devlink_reload_action action, enum devlink_reload_limit limit,
struct netlink_ext_ack *extack, unsigned long *actions_performed)
@@ -3026,6 +3122,7 @@ static int devlink_reload(struct devlink *devlink, struct net *dest_net,
return err;
WARN_ON(!test_bit(action, actions_performed));
+ devlink_reload_stats_update(devlink, limit, *actions_performed);
return 0;
}
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH net-next 04/16] devlink: Add reload stats
2020-10-01 13:59 ` [PATCH net-next 04/16] devlink: Add reload stats Moshe Shemesh
@ 2020-10-01 21:25 ` Jakub Kicinski
2020-10-02 15:07 ` Moshe Shemesh
2020-10-03 9:00 ` Jiri Pirko
1 sibling, 1 reply; 49+ messages in thread
From: Jakub Kicinski @ 2020-10-01 21:25 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: David S. Miller, Jiri Pirko, netdev, linux-kernel
On Thu, 1 Oct 2020 16:59:07 +0300 Moshe Shemesh wrote:
> Add reload stats to hold the history per reload action type and limit.
>
> For example, the number of times fw_activate has been performed on this
> device since the driver module was added or if the firmware activation
> was performed with or without reset.
>
> Add devlink notification on stats update.
>
> Expose devlink reload stats to the user through devlink dev get command.
>
> Examples:
> $ devlink dev show
> pci/0000:82:00.0:
> stats:
> reload_stats:
> driver_reinit 2
> fw_activate 1
> fw_activate_no_reset 0
> pci/0000:82:00.1:
> stats:
> reload_stats:
> driver_reinit 1
> fw_activate 0
> fw_activate_no_reset 0
>
> $ devlink dev show -jp
> {
> "dev": {
> "pci/0000:82:00.0": {
> "stats": {
> "reload_stats": [ {
> "driver_reinit": 2
> },{
> "fw_activate": 1
> },{
> "fw_activate_no_reset": 0
> } ]
> }
> },
> "pci/0000:82:00.1": {
> "stats": {
> "reload_stats": [ {
> "driver_reinit": 1
> },{
> "fw_activate": 0
> },{
> "fw_activate_no_reset": 0
> } ]
This will be a question to the user space part but IDK why every stat
is in a separate object?
Instead of doing an array of objects -> [ {}, {}, {} ]
make "reload_stats" itself an object.
> }
> }
> }
> }
>
> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Minor nits, looks good overall:
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
> diff --git a/net/core/devlink.c b/net/core/devlink.c
> index 6de7d6aa6ed1..05516f1e4c3e 100644
> --- a/net/core/devlink.c
> +++ b/net/core/devlink.c
> @@ -500,10 +500,68 @@ devlink_reload_limit_is_supported(struct devlink *devlink, enum devlink_reload_l
> return test_bit(limit, &devlink->ops->reload_limits);
> }
>
> +static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_action action,
> + enum devlink_reload_limit limit, u32 value)
> +{
> + struct nlattr *reload_stats_entry;
> +
> + reload_stats_entry = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS_ENTRY);
> + if (!reload_stats_entry)
> + return -EMSGSIZE;
> +
> + if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_ACTION, action))
> + goto nla_put_failure;
> + if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_LIMIT, limit))
> + goto nla_put_failure;
> + if (nla_put_u32(msg, DEVLINK_ATTR_RELOAD_STATS_VALUE, value))
> + goto nla_put_failure;
nit: it's common to combine such expressions into:
if (nla_put...() ||
nla_put...() ||
nla_put...())
goto ...;
> + nla_nest_end(msg, reload_stats_entry);
> + return 0;
> +
> +nla_put_failure:
> + nla_nest_cancel(msg, reload_stats_entry);
> + return -EMSGSIZE;
> +}
> +
> +static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink)
> +{
> + struct nlattr *reload_stats_attr;
> + int i, j, stat_idx;
> + u32 value;
> +
> + reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
> +
> + if (!reload_stats_attr)
> + return -EMSGSIZE;
> +
> + for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) {
> + if (j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
Why check this? It can't be supported.
> + !devlink_reload_limit_is_supported(devlink, j))
> + continue;
> + for (i = 0; i <= DEVLINK_RELOAD_ACTION_MAX; i++) {
> + if (!devlink_reload_action_is_supported(devlink, i) ||
> + devlink_reload_combination_is_invalid(i, j))
Again, devlink instance would not have been registered with invalid
combinations, right?
> + continue;
> +
> + stat_idx = j * __DEVLINK_RELOAD_ACTION_MAX + i;
> + value = devlink->reload_stats[stat_idx];
> + if (devlink_reload_stat_put(msg, i, j, value))
> + goto nla_put_failure;
> + }
> + }
> + nla_nest_end(msg, reload_stats_attr);
> + return 0;
> +
> +nla_put_failure:
> + nla_nest_cancel(msg, reload_stats_attr);
> + return -EMSGSIZE;
> +}
> @@ -3004,6 +3072,34 @@ bool devlink_is_reload_failed(const struct devlink *devlink)
> }
> EXPORT_SYMBOL_GPL(devlink_is_reload_failed);
>
> +static void
> +__devlink_reload_stats_update(struct devlink *devlink, u32 *reload_stats,
> + enum devlink_reload_limit limit, unsigned long actions_performed)
> + for (action = 0; action <= DEVLINK_RELOAD_ACTION_MAX; action++) {
nit: for_each_set_bit
> + if (!test_bit(action, &actions_performed))
> + continue;
> + stat_idx = limit * __DEVLINK_RELOAD_ACTION_MAX + action;
> + reload_stats[stat_idx]++;
> + }
> + devlink_notify(devlink, DEVLINK_CMD_NEW);
> +}
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 04/16] devlink: Add reload stats
2020-10-01 21:25 ` Jakub Kicinski
@ 2020-10-02 15:07 ` Moshe Shemesh
0 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-02 15:07 UTC (permalink / raw)
To: Jakub Kicinski, Moshe Shemesh
Cc: David S. Miller, Jiri Pirko, netdev, linux-kernel
On 10/2/2020 12:25 AM, Jakub Kicinski wrote:
> External email: Use caution opening links or attachments
>
>
> On Thu, 1 Oct 2020 16:59:07 +0300 Moshe Shemesh wrote:
>> Add reload stats to hold the history per reload action type and limit.
>>
>> For example, the number of times fw_activate has been performed on this
>> device since the driver module was added or if the firmware activation
>> was performed with or without reset.
>>
>> Add devlink notification on stats update.
>>
>> Expose devlink reload stats to the user through devlink dev get command.
>>
>> Examples:
>> $ devlink dev show
>> pci/0000:82:00.0:
>> stats:
>> reload_stats:
>> driver_reinit 2
>> fw_activate 1
>> fw_activate_no_reset 0
>> pci/0000:82:00.1:
>> stats:
>> reload_stats:
>> driver_reinit 1
>> fw_activate 0
>> fw_activate_no_reset 0
>>
>> $ devlink dev show -jp
>> {
>> "dev": {
>> "pci/0000:82:00.0": {
>> "stats": {
>> "reload_stats": [ {
>> "driver_reinit": 2
>> },{
>> "fw_activate": 1
>> },{
>> "fw_activate_no_reset": 0
>> } ]
>> }
>> },
>> "pci/0000:82:00.1": {
>> "stats": {
>> "reload_stats": [ {
>> "driver_reinit": 1
>> },{
>> "fw_activate": 0
>> },{
>> "fw_activate_no_reset": 0
>> } ]
> This will be a question to the user space part but IDK why every stat
> is in a separate object?
>
> Instead of doing an array of objects -> [ {}, {}, {} ]
> make "reload_stats" itself an object.
I first thought that there should be an array of stats, looking at it
again, as long as each stat contains just sting and value, object of
pairs will perfectly fit. I will change it.
>> }
>> }
>> }
>> }
>>
>> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
> Minor nits, looks good overall:
>
> Reviewed-by: Jakub Kicinski <kuba@kernel.org>
>
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 6de7d6aa6ed1..05516f1e4c3e 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -500,10 +500,68 @@ devlink_reload_limit_is_supported(struct devlink *devlink, enum devlink_reload_l
>> return test_bit(limit, &devlink->ops->reload_limits);
>> }
>>
>> +static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_action action,
>> + enum devlink_reload_limit limit, u32 value)
>> +{
>> + struct nlattr *reload_stats_entry;
>> +
>> + reload_stats_entry = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS_ENTRY);
>> + if (!reload_stats_entry)
>> + return -EMSGSIZE;
>> +
>> + if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_ACTION, action))
>> + goto nla_put_failure;
>> + if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_LIMIT, limit))
>> + goto nla_put_failure;
>> + if (nla_put_u32(msg, DEVLINK_ATTR_RELOAD_STATS_VALUE, value))
>> + goto nla_put_failure;
> nit: it's common to combine such expressions into:
>
> if (nla_put...() ||
> nla_put...() ||
> nla_put...())
> goto ...;
Ack.
>> + nla_nest_end(msg, reload_stats_entry);
>> + return 0;
>> +
>> +nla_put_failure:
>> + nla_nest_cancel(msg, reload_stats_entry);
>> + return -EMSGSIZE;
>> +}
>> +
>> +static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink)
>> +{
>> + struct nlattr *reload_stats_attr;
>> + int i, j, stat_idx;
>> + u32 value;
>> +
>> + reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
>> +
>> + if (!reload_stats_attr)
>> + return -EMSGSIZE;
>> +
>> + for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) {
>> + if (j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
> Why check this? It can't be supported.
Show stats of the actions with unspecified limit.
>> + !devlink_reload_limit_is_supported(devlink, j))
>> + continue;
>> + for (i = 0; i <= DEVLINK_RELOAD_ACTION_MAX; i++) {
>> + if (!devlink_reload_action_is_supported(devlink, i) ||
>> + devlink_reload_combination_is_invalid(i, j))
> Again, devlink instance would not have been registered with invalid
> combinations, right?
It does register actions and register limits, there can be invalid
combination, but devlink will block it before it gets to the driver.
>> + continue;
>> +
>> + stat_idx = j * __DEVLINK_RELOAD_ACTION_MAX + i;
>> + value = devlink->reload_stats[stat_idx];
>> + if (devlink_reload_stat_put(msg, i, j, value))
>> + goto nla_put_failure;
>> + }
>> + }
>> + nla_nest_end(msg, reload_stats_attr);
>> + return 0;
>> +
>> +nla_put_failure:
>> + nla_nest_cancel(msg, reload_stats_attr);
>> + return -EMSGSIZE;
>> +}
>
>> @@ -3004,6 +3072,34 @@ bool devlink_is_reload_failed(const struct devlink *devlink)
>> }
>> EXPORT_SYMBOL_GPL(devlink_is_reload_failed);
>>
>> +static void
>> +__devlink_reload_stats_update(struct devlink *devlink, u32 *reload_stats,
>> + enum devlink_reload_limit limit, unsigned long actions_performed)
>> + for (action = 0; action <= DEVLINK_RELOAD_ACTION_MAX; action++) {
> nit: for_each_set_bit
Ack.
>> + if (!test_bit(action, &actions_performed))
>> + continue;
>> + stat_idx = limit * __DEVLINK_RELOAD_ACTION_MAX + action;
>> + reload_stats[stat_idx]++;
>> + }
>> + devlink_notify(devlink, DEVLINK_CMD_NEW);
>> +}
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH net-next 04/16] devlink: Add reload stats
2020-10-01 13:59 ` [PATCH net-next 04/16] devlink: Add reload stats Moshe Shemesh
2020-10-01 21:25 ` Jakub Kicinski
@ 2020-10-03 9:00 ` Jiri Pirko
2020-10-04 6:59 ` Moshe Shemesh
1 sibling, 1 reply; 49+ messages in thread
From: Jiri Pirko @ 2020-10-03 9:00 UTC (permalink / raw)
To: Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
Thu, Oct 01, 2020 at 03:59:07PM CEST, moshe@mellanox.com wrote:
>Add reload stats to hold the history per reload action type and limit.
>
>For example, the number of times fw_activate has been performed on this
>device since the driver module was added or if the firmware activation
>was performed with or without reset.
>
>Add devlink notification on stats update.
>
>Expose devlink reload stats to the user through devlink dev get command.
>
>Examples:
>$ devlink dev show
>pci/0000:82:00.0:
> stats:
> reload_stats:
> driver_reinit 2
> fw_activate 1
> fw_activate_no_reset 0
>pci/0000:82:00.1:
> stats:
> reload_stats:
> driver_reinit 1
> fw_activate 0
> fw_activate_no_reset 0
>
>$ devlink dev show -jp
>{
> "dev": {
> "pci/0000:82:00.0": {
> "stats": {
> "reload_stats": [ {
Just "reload". No need to repeat "stats" here.
> "driver_reinit": 2
> },{
> "fw_activate": 1
> },{
> "fw_activate_no_reset": 0
> } ]
> }
> },
> "pci/0000:82:00.1": {
> "stats": {
> "reload_stats": [ {
> "driver_reinit": 1
> },{
> "fw_activate": 0
> },{
> "fw_activate_no_reset": 0
> } ]
> }
> }
> }
>}
>
>Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
>---
>RFCv5 -> v1:
>- Changed the stats output structure, have 2 stats, one for local and
>one for remote
>- Resplit this patch and the next one by remote/local reload stast
>instead of set/get reload stats
>- Add helper function devlink_reload_stats_put()
>RFCv4 -> RFCv5:
>- Add separate reload action stats for updating on remote actions
>- Protect from updating remote actions stats during reload_down()/up()
>RFCv3 -> RFCv4:
>- Renamed reload_actions_cnts to reload_action_stats
>- Add devlink notifications on stats update
>- Renamed devlink_reload_actions_implicit_actions_performed() and add
> function comment in code
>RFCv2 -> RFCv3:
>- New patch
>---
> include/net/devlink.h | 7 +++
> include/uapi/linux/devlink.h | 5 ++
> net/core/devlink.c | 97 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 109 insertions(+)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 43dde69086e5..0f3bd23b6c04 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -20,6 +20,9 @@
> #include <uapi/linux/devlink.h>
> #include <linux/xarray.h>
>
>+#define DEVLINK_RELOAD_STATS_ARRAY_SIZE \
>+ (__DEVLINK_RELOAD_LIMIT_MAX * __DEVLINK_RELOAD_ACTION_MAX)
>+
> struct devlink_ops;
>
> struct devlink {
>@@ -38,6 +41,7 @@ struct devlink {
> struct list_head trap_policer_list;
> const struct devlink_ops *ops;
> struct xarray snapshot_ids;
>+ u32 reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
> struct device *dev;
> possible_net_t _net;
> struct mutex lock; /* Serializes access to devlink instance specific objects such as
>@@ -1470,6 +1474,9 @@ void
> devlink_health_reporter_recovery_done(struct devlink_health_reporter *reporter);
>
> bool devlink_is_reload_failed(const struct devlink *devlink);
>+void devlink_remote_reload_actions_performed(struct devlink *devlink,
>+ enum devlink_reload_limit limit,
>+ unsigned long actions_performed);
Leftover, please remove/move.
>
> void devlink_flash_update_begin_notify(struct devlink *devlink);
> void devlink_flash_update_end_notify(struct devlink *devlink);
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index cc5dc4c07b4a..97e0137f6201 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -526,6 +526,11 @@ enum devlink_attr {
> DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, /* u64 */
> DEVLINK_ATTR_RELOAD_LIMIT, /* u8 */
>
>+ DEVLINK_ATTR_DEV_STATS, /* nested */
>+ DEVLINK_ATTR_RELOAD_STATS, /* nested */
>+ DEVLINK_ATTR_RELOAD_STATS_ENTRY, /* nested */
>+ DEVLINK_ATTR_RELOAD_STATS_VALUE, /* u32 */
>+
> /* add new attributes above here, update the policy in devlink.c */
>
> __DEVLINK_ATTR_MAX,
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index 6de7d6aa6ed1..05516f1e4c3e 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -500,10 +500,68 @@ devlink_reload_limit_is_supported(struct devlink *devlink, enum devlink_reload_l
> return test_bit(limit, &devlink->ops->reload_limits);
> }
>
>+static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_action action,
>+ enum devlink_reload_limit limit, u32 value)
>+{
>+ struct nlattr *reload_stats_entry;
>+
>+ reload_stats_entry = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS_ENTRY);
>+ if (!reload_stats_entry)
>+ return -EMSGSIZE;
>+
>+ if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_ACTION, action))
>+ goto nla_put_failure;
>+ if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_LIMIT, limit))
>+ goto nla_put_failure;
>+ if (nla_put_u32(msg, DEVLINK_ATTR_RELOAD_STATS_VALUE, value))
>+ goto nla_put_failure;
>+ nla_nest_end(msg, reload_stats_entry);
>+ return 0;
>+
>+nla_put_failure:
>+ nla_nest_cancel(msg, reload_stats_entry);
>+ return -EMSGSIZE;
>+}
>+
>+static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink)
>+{
>+ struct nlattr *reload_stats_attr;
>+ int i, j, stat_idx;
>+ u32 value;
>+
>+ reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
>+
>+ if (!reload_stats_attr)
>+ return -EMSGSIZE;
>+
>+ for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) {
>+ if (j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
You should check limit_unspec during driver register, not here.
>+ !devlink_reload_limit_is_supported(devlink, j))
>+ continue;
>+ for (i = 0; i <= DEVLINK_RELOAD_ACTION_MAX; i++) {
>+ if (!devlink_reload_action_is_supported(devlink, i) ||
>+ devlink_reload_combination_is_invalid(i, j))
>+ continue;
>+
>+ stat_idx = j * __DEVLINK_RELOAD_ACTION_MAX + i;
>+ value = devlink->reload_stats[stat_idx];
>+ if (devlink_reload_stat_put(msg, i, j, value))
>+ goto nla_put_failure;
>+ }
>+ }
>+ nla_nest_end(msg, reload_stats_attr);
>+ return 0;
>+
>+nla_put_failure:
>+ nla_nest_cancel(msg, reload_stats_attr);
>+ return -EMSGSIZE;
>+}
>+
> static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
> enum devlink_command cmd, u32 portid,
> u32 seq, int flags)
> {
>+ struct nlattr *dev_stats;
> void *hdr;
>
> hdr = genlmsg_put(msg, portid, seq, &devlink_nl_family, flags, cmd);
>@@ -515,9 +573,19 @@ static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
> if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_FAILED, devlink->reload_failed))
> goto nla_put_failure;
>
>+ dev_stats = nla_nest_start(msg, DEVLINK_ATTR_DEV_STATS);
Avoid the "DEV". Just "DEVLINK_ATTR_STATS" is enough.
>+ if (!dev_stats)
>+ goto nla_put_failure;
>+
>+ if (devlink_reload_stats_put(msg, devlink))
>+ goto dev_stats_nest_cancel;
>+
>+ nla_nest_end(msg, dev_stats);
> genlmsg_end(msg, hdr);
> return 0;
>
>+dev_stats_nest_cancel:
>+ nla_nest_cancel(msg, dev_stats);
> nla_put_failure:
> genlmsg_cancel(msg, hdr);
> return -EMSGSIZE;
>@@ -3004,6 +3072,34 @@ bool devlink_is_reload_failed(const struct devlink *devlink)
> }
> EXPORT_SYMBOL_GPL(devlink_is_reload_failed);
>
>+static void
>+__devlink_reload_stats_update(struct devlink *devlink, u32 *reload_stats,
>+ enum devlink_reload_limit limit, unsigned long actions_performed)
>+{
>+ int stat_idx;
>+ int action;
>+
>+ if (!actions_performed)
>+ return;
>+
>+ if (WARN_ON(limit > DEVLINK_RELOAD_LIMIT_MAX))
I don't understand the reason for this check and warn on. You should
sanitize this in the caller (I think you already do that).
>+ return;
>+ for (action = 0; action <= DEVLINK_RELOAD_ACTION_MAX; action++) {
>+ if (!test_bit(action, &actions_performed))
>+ continue;
>+ stat_idx = limit * __DEVLINK_RELOAD_ACTION_MAX + action;
>+ reload_stats[stat_idx]++;
>+ }
>+ devlink_notify(devlink, DEVLINK_CMD_NEW);
>+}
>+
>+static void
>+devlink_reload_stats_update(struct devlink *devlink, enum devlink_reload_limit limit,
>+ unsigned long actions_performed)
>+{
>+ __devlink_reload_stats_update(devlink, devlink->reload_stats, limit, actions_performed);
>+}
>+
> static int devlink_reload(struct devlink *devlink, struct net *dest_net,
> enum devlink_reload_action action, enum devlink_reload_limit limit,
> struct netlink_ext_ack *extack, unsigned long *actions_performed)
>@@ -3026,6 +3122,7 @@ static int devlink_reload(struct devlink *devlink, struct net *dest_net,
> return err;
>
> WARN_ON(!test_bit(action, actions_performed));
>+ devlink_reload_stats_update(devlink, limit, *actions_performed);
> return 0;
> }
>
>--
>2.18.2
>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 04/16] devlink: Add reload stats
2020-10-03 9:00 ` Jiri Pirko
@ 2020-10-04 6:59 ` Moshe Shemesh
0 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-04 6:59 UTC (permalink / raw)
To: Jiri Pirko, Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
On 10/3/2020 12:00 PM, Jiri Pirko wrote:
> Thu, Oct 01, 2020 at 03:59:07PM CEST, moshe@mellanox.com wrote:
>> Add reload stats to hold the history per reload action type and limit.
>>
>> For example, the number of times fw_activate has been performed on this
>> device since the driver module was added or if the firmware activation
>> was performed with or without reset.
>>
>> Add devlink notification on stats update.
>>
>> Expose devlink reload stats to the user through devlink dev get command.
>>
>> Examples:
>> $ devlink dev show
>> pci/0000:82:00.0:
>> stats:
>> reload_stats:
>> driver_reinit 2
>> fw_activate 1
>> fw_activate_no_reset 0
>> pci/0000:82:00.1:
>> stats:
>> reload_stats:
>> driver_reinit 1
>> fw_activate 0
>> fw_activate_no_reset 0
>>
>> $ devlink dev show -jp
>> {
>> "dev": {
>> "pci/0000:82:00.0": {
>> "stats": {
>> "reload_stats": [ {
> Just "reload". No need to repeat "stats" here.
Ack.
>
>> "driver_reinit": 2
>> },{
>> "fw_activate": 1
>> },{
>> "fw_activate_no_reset": 0
>> } ]
>> }
>> },
>> "pci/0000:82:00.1": {
>> "stats": {
>> "reload_stats": [ {
>> "driver_reinit": 1
>> },{
>> "fw_activate": 0
>> },{
>> "fw_activate_no_reset": 0
>> } ]
>> }
>> }
>> }
>> }
>>
>> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
>> ---
>> RFCv5 -> v1:
>> - Changed the stats output structure, have 2 stats, one for local and
>> one for remote
>> - Resplit this patch and the next one by remote/local reload stast
>> instead of set/get reload stats
>> - Add helper function devlink_reload_stats_put()
>> RFCv4 -> RFCv5:
>> - Add separate reload action stats for updating on remote actions
>> - Protect from updating remote actions stats during reload_down()/up()
>> RFCv3 -> RFCv4:
>> - Renamed reload_actions_cnts to reload_action_stats
>> - Add devlink notifications on stats update
>> - Renamed devlink_reload_actions_implicit_actions_performed() and add
>> function comment in code
>> RFCv2 -> RFCv3:
>> - New patch
>> ---
>> include/net/devlink.h | 7 +++
>> include/uapi/linux/devlink.h | 5 ++
>> net/core/devlink.c | 97 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 109 insertions(+)
>>
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index 43dde69086e5..0f3bd23b6c04 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -20,6 +20,9 @@
>> #include <uapi/linux/devlink.h>
>> #include <linux/xarray.h>
>>
>> +#define DEVLINK_RELOAD_STATS_ARRAY_SIZE \
>> + (__DEVLINK_RELOAD_LIMIT_MAX * __DEVLINK_RELOAD_ACTION_MAX)
>> +
>> struct devlink_ops;
>>
>> struct devlink {
>> @@ -38,6 +41,7 @@ struct devlink {
>> struct list_head trap_policer_list;
>> const struct devlink_ops *ops;
>> struct xarray snapshot_ids;
>> + u32 reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
>> struct device *dev;
>> possible_net_t _net;
>> struct mutex lock; /* Serializes access to devlink instance specific objects such as
>> @@ -1470,6 +1474,9 @@ void
>> devlink_health_reporter_recovery_done(struct devlink_health_reporter *reporter);
>>
>> bool devlink_is_reload_failed(const struct devlink *devlink);
>> +void devlink_remote_reload_actions_performed(struct devlink *devlink,
>> + enum devlink_reload_limit limit,
>> + unsigned long actions_performed);
> Leftover, please remove/move.
>
Yes, should be in the next patch, I missed it while re-splitting these
two patches.
>> void devlink_flash_update_begin_notify(struct devlink *devlink);
>> void devlink_flash_update_end_notify(struct devlink *devlink);
>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> index cc5dc4c07b4a..97e0137f6201 100644
>> --- a/include/uapi/linux/devlink.h
>> +++ b/include/uapi/linux/devlink.h
>> @@ -526,6 +526,11 @@ enum devlink_attr {
>> DEVLINK_ATTR_RELOAD_ACTIONS_PERFORMED, /* u64 */
>> DEVLINK_ATTR_RELOAD_LIMIT, /* u8 */
>>
>> + DEVLINK_ATTR_DEV_STATS, /* nested */
>> + DEVLINK_ATTR_RELOAD_STATS, /* nested */
>> + DEVLINK_ATTR_RELOAD_STATS_ENTRY, /* nested */
>> + DEVLINK_ATTR_RELOAD_STATS_VALUE, /* u32 */
>> +
>> /* add new attributes above here, update the policy in devlink.c */
>>
>> __DEVLINK_ATTR_MAX,
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 6de7d6aa6ed1..05516f1e4c3e 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -500,10 +500,68 @@ devlink_reload_limit_is_supported(struct devlink *devlink, enum devlink_reload_l
>> return test_bit(limit, &devlink->ops->reload_limits);
>> }
>>
>> +static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_action action,
>> + enum devlink_reload_limit limit, u32 value)
>> +{
>> + struct nlattr *reload_stats_entry;
>> +
>> + reload_stats_entry = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS_ENTRY);
>> + if (!reload_stats_entry)
>> + return -EMSGSIZE;
>> +
>> + if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_ACTION, action))
>> + goto nla_put_failure;
>> + if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_LIMIT, limit))
>> + goto nla_put_failure;
>> + if (nla_put_u32(msg, DEVLINK_ATTR_RELOAD_STATS_VALUE, value))
>> + goto nla_put_failure;
>> + nla_nest_end(msg, reload_stats_entry);
>> + return 0;
>> +
>> +nla_put_failure:
>> + nla_nest_cancel(msg, reload_stats_entry);
>> + return -EMSGSIZE;
>> +}
>> +
>> +static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink)
>> +{
>> + struct nlattr *reload_stats_attr;
>> + int i, j, stat_idx;
>> + u32 value;
>> +
>> + reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
>> +
>> + if (!reload_stats_attr)
>> + return -EMSGSIZE;
>> +
>> + for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) {
>> + if (j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
> You should check limit_unspec during driver register, not here.
The thing is that now (change from RFCv5 to v1, see patch 3 change log)
driver does not need to register supported limits if it only supports
actions without any limitation. So I want to show stats of actions with
unspecified limit, though I don't expect driver to register it.
>
>> + !devlink_reload_limit_is_supported(devlink, j))
>> + continue;
>> + for (i = 0; i <= DEVLINK_RELOAD_ACTION_MAX; i++) {
>> + if (!devlink_reload_action_is_supported(devlink, i) ||
>> + devlink_reload_combination_is_invalid(i, j))
>> + continue;
>> +
>> + stat_idx = j * __DEVLINK_RELOAD_ACTION_MAX + i;
>> + value = devlink->reload_stats[stat_idx];
>> + if (devlink_reload_stat_put(msg, i, j, value))
>> + goto nla_put_failure;
>> + }
>> + }
>> + nla_nest_end(msg, reload_stats_attr);
>> + return 0;
>> +
>> +nla_put_failure:
>> + nla_nest_cancel(msg, reload_stats_attr);
>> + return -EMSGSIZE;
>> +}
>> +
>> static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
>> enum devlink_command cmd, u32 portid,
>> u32 seq, int flags)
>> {
>> + struct nlattr *dev_stats;
>> void *hdr;
>>
>> hdr = genlmsg_put(msg, portid, seq, &devlink_nl_family, flags, cmd);
>> @@ -515,9 +573,19 @@ static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
>> if (nla_put_u8(msg, DEVLINK_ATTR_RELOAD_FAILED, devlink->reload_failed))
>> goto nla_put_failure;
>>
>> + dev_stats = nla_nest_start(msg, DEVLINK_ATTR_DEV_STATS);
> Avoid the "DEV". Just "DEVLINK_ATTR_STATS" is enough.
"DEVLINK_ATTR_STATS" is already used for something else (traps), I added
DEV as its dev get command stats.
>
>> + if (!dev_stats)
>> + goto nla_put_failure;
>> +
>> + if (devlink_reload_stats_put(msg, devlink))
>> + goto dev_stats_nest_cancel;
>> +
>> + nla_nest_end(msg, dev_stats);
>> genlmsg_end(msg, hdr);
>> return 0;
>>
>> +dev_stats_nest_cancel:
>> + nla_nest_cancel(msg, dev_stats);
>> nla_put_failure:
>> genlmsg_cancel(msg, hdr);
>> return -EMSGSIZE;
>> @@ -3004,6 +3072,34 @@ bool devlink_is_reload_failed(const struct devlink *devlink)
>> }
>> EXPORT_SYMBOL_GPL(devlink_is_reload_failed);
>>
>> +static void
>> +__devlink_reload_stats_update(struct devlink *devlink, u32 *reload_stats,
>> + enum devlink_reload_limit limit, unsigned long actions_performed)
>> +{
>> + int stat_idx;
>> + int action;
>> +
>> + if (!actions_performed)
>> + return;
>> +
>> + if (WARN_ON(limit > DEVLINK_RELOAD_LIMIT_MAX))
> I don't understand the reason for this check and warn on. You should
> sanitize this in the caller (I think you already do that).
That's because the next patch has a call to this function directly from
the driver. I should add this check on the caller in the next patch.
>> + return;
>> + for (action = 0; action <= DEVLINK_RELOAD_ACTION_MAX; action++) {
>> + if (!test_bit(action, &actions_performed))
>> + continue;
>> + stat_idx = limit * __DEVLINK_RELOAD_ACTION_MAX + action;
>> + reload_stats[stat_idx]++;
>> + }
>> + devlink_notify(devlink, DEVLINK_CMD_NEW);
>> +}
>> +
>> +static void
>> +devlink_reload_stats_update(struct devlink *devlink, enum devlink_reload_limit limit,
>> + unsigned long actions_performed)
>> +{
>> + __devlink_reload_stats_update(devlink, devlink->reload_stats, limit, actions_performed);
>> +}
>> +
>> static int devlink_reload(struct devlink *devlink, struct net *dest_net,
>> enum devlink_reload_action action, enum devlink_reload_limit limit,
>> struct netlink_ext_ack *extack, unsigned long *actions_performed)
>> @@ -3026,6 +3122,7 @@ static int devlink_reload(struct devlink *devlink, struct net *dest_net,
>> return err;
>>
>> WARN_ON(!test_bit(action, actions_performed));
>> + devlink_reload_stats_update(devlink, limit, *actions_performed);
>> return 0;
>> }
>>
>> --
>> 2.18.2
>>
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH net-next 05/16] devlink: Add remote reload stats
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (3 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 04/16] devlink: Add reload stats Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 21:48 ` Jakub Kicinski
2020-10-03 9:05 ` Jiri Pirko
2020-10-01 13:59 ` [PATCH net-next 06/16] net/mlx5: Add functions to set/query MFRL register Moshe Shemesh
` (10 subsequent siblings)
15 siblings, 2 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Add remote reload stats to hold the history of actions performed due
devlink reload commands initiated by remote host. For example, in case
firmware activation with reset finished successfully but was initiated
by remote host.
The function devlink_remote_reload_actions_performed() is exported to
enable drivers update on remote reload actions performed as it was not
initiated by their own devlink instance.
Expose devlink remote reload stats to the user through devlink dev get
command.
Examples:
$ devlink dev show
pci/0000:82:00.0:
stats:
reload_stats:
driver_reinit 2
fw_activate 1
fw_activate_no_reset 0
remote_reload_stats:
driver_reinit 0
fw_activate 0
fw_activate_no_reset 0
pci/0000:82:00.1:
stats:
reload_stats:
driver_reinit 1
fw_activate 0
fw_activate_no_reset 0
remote_reload_stats:
driver_reinit 1
fw_activate 1
fw_activate_no_reset 0
$ devlink dev show -jp
{
"dev": {
"pci/0000:82:00.0": {
"stats": {
"reload_stats": [ {
"driver_reinit": 2
},{
"fw_activate": 1
},{
"fw_activate_no_reset": 0
} ],
"remote_reload_stats": [ {
"driver_reinit": 0
},{
"fw_activate": 0
},{
"fw_activate_no_reset": 0
} ]
}
},
"pci/0000:82:00.1": {
"stats": {
"reload_stats": [ {
"driver_reinit": 1
},{
"fw_activate": 0
},{
"fw_activate_no_reset": 0
} ],
"remote_reload_stats": [ {
"driver_reinit": 1
},{
"fw_activate": 1
},{
"fw_activate_no_reset": 0
} ]
}
}
}
}
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
---
RFCv5 -> v1:
- Resplit this patch and the previous one by remote/local reload stats
instead of set/get reload stats
- Rename reload_action_stats to reload_stats
RFCv4 -> RFCv5:
- Add remote actions stats
- If devlink reload is not supported, show only remote_stats
RFCv3 -> RFCv4:
- Renamed DEVLINK_ATTR_RELOAD_ACTION_CNT to
DEVLINK_ATTR_RELOAD_ACTION_STAT
- Add stats per action per limit level
RFCv2 -> RFCv3:
- Add reload actions counters instead of supported reload actions
(reload actions counters are only for supported action so no need for
both)
RFCv1 -> RFCv2:
- Removed DEVLINK_ATTR_RELOAD_DEFAULT_LEVEL
- Removed DEVLINK_ATTR_RELOAD_LEVELS_INFO
- Have actions instead of levels
---
include/net/devlink.h | 1 +
include/uapi/linux/devlink.h | 1 +
net/core/devlink.c | 49 +++++++++++++++++++++++++++++++-----
3 files changed, 45 insertions(+), 6 deletions(-)
diff --git a/include/net/devlink.h b/include/net/devlink.h
index 0f3bd23b6c04..a4ccb83bbd2c 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -42,6 +42,7 @@ struct devlink {
const struct devlink_ops *ops;
struct xarray snapshot_ids;
u32 reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
+ u32 remote_reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
struct device *dev;
possible_net_t _net;
struct mutex lock; /* Serializes access to devlink instance specific objects such as
diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
index 97e0137f6201..f9887d8afdc7 100644
--- a/include/uapi/linux/devlink.h
+++ b/include/uapi/linux/devlink.h
@@ -530,6 +530,7 @@ enum devlink_attr {
DEVLINK_ATTR_RELOAD_STATS, /* nested */
DEVLINK_ATTR_RELOAD_STATS_ENTRY, /* nested */
DEVLINK_ATTR_RELOAD_STATS_VALUE, /* u32 */
+ DEVLINK_ATTR_REMOTE_RELOAD_STATS, /* nested */
/* add new attributes above here, update the policy in devlink.c */
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 05516f1e4c3e..3b6bd3b4d346 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -523,28 +523,35 @@ static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_acti
return -EMSGSIZE;
}
-static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink)
+static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink, bool is_remote)
{
struct nlattr *reload_stats_attr;
int i, j, stat_idx;
u32 value;
- reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
+ if (!is_remote)
+ reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
+ else
+ reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_REMOTE_RELOAD_STATS);
if (!reload_stats_attr)
return -EMSGSIZE;
for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) {
- if (j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
+ if (!is_remote && j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
!devlink_reload_limit_is_supported(devlink, j))
continue;
for (i = 0; i <= DEVLINK_RELOAD_ACTION_MAX; i++) {
- if (!devlink_reload_action_is_supported(devlink, i) ||
+ if ((!is_remote && !devlink_reload_action_is_supported(devlink, i)) ||
+ i == DEVLINK_RELOAD_ACTION_UNSPEC ||
devlink_reload_combination_is_invalid(i, j))
continue;
stat_idx = j * __DEVLINK_RELOAD_ACTION_MAX + i;
- value = devlink->reload_stats[stat_idx];
+ if (!is_remote)
+ value = devlink->reload_stats[stat_idx];
+ else
+ value = devlink->remote_reload_stats[stat_idx];
if (devlink_reload_stat_put(msg, i, j, value))
goto nla_put_failure;
}
@@ -577,7 +584,9 @@ static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
if (!dev_stats)
goto nla_put_failure;
- if (devlink_reload_stats_put(msg, devlink))
+ if (devlink_reload_stats_put(msg, devlink, false))
+ goto dev_stats_nest_cancel;
+ if (devlink_reload_stats_put(msg, devlink, true))
goto dev_stats_nest_cancel;
nla_nest_end(msg, dev_stats);
@@ -3100,15 +3109,40 @@ devlink_reload_stats_update(struct devlink *devlink, enum devlink_reload_limit l
__devlink_reload_stats_update(devlink, devlink->reload_stats, limit, actions_performed);
}
+/**
+ * devlink_remote_reload_actions_performed - Update devlink on reload actions
+ * performed which are not a direct result of devlink reload call.
+ *
+ * This should be called by a driver after performing reload actions in case it was not
+ * a result of devlink reload call. For example fw_activate was performed as a result
+ * of devlink reload triggered fw_activate on another host.
+ * The motivation for this function is to keep data on reload actions performed on this
+ * function whether it was done due to direct devlink reload call or not.
+ *
+ * @devlink: devlink
+ * @limit: reload limit
+ * @actions_performed: bitmask of actions performed
+ */
+void devlink_remote_reload_actions_performed(struct devlink *devlink,
+ enum devlink_reload_limit limit,
+ unsigned long actions_performed)
+{
+ __devlink_reload_stats_update(devlink, devlink->remote_reload_stats, limit,
+ actions_performed);
+}
+EXPORT_SYMBOL_GPL(devlink_remote_reload_actions_performed);
+
static int devlink_reload(struct devlink *devlink, struct net *dest_net,
enum devlink_reload_action action, enum devlink_reload_limit limit,
struct netlink_ext_ack *extack, unsigned long *actions_performed)
{
+ u32 remote_reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
int err;
if (!devlink->reload_enabled)
return -EOPNOTSUPP;
+ memcpy(remote_reload_stats, devlink->remote_reload_stats, sizeof(remote_reload_stats));
err = devlink->ops->reload_down(devlink, !!dest_net, action, limit, extack);
if (err)
return err;
@@ -3122,6 +3156,9 @@ static int devlink_reload(struct devlink *devlink, struct net *dest_net,
return err;
WARN_ON(!test_bit(action, actions_performed));
+ /* Catch driver on updating the remote action within devlink reload */
+ WARN_ON(memcmp(remote_reload_stats, devlink->remote_reload_stats,
+ sizeof(remote_reload_stats)));
devlink_reload_stats_update(devlink, limit, *actions_performed);
return 0;
}
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH net-next 05/16] devlink: Add remote reload stats
2020-10-01 13:59 ` [PATCH net-next 05/16] devlink: Add remote " Moshe Shemesh
@ 2020-10-01 21:48 ` Jakub Kicinski
2020-10-03 9:05 ` Jiri Pirko
1 sibling, 0 replies; 49+ messages in thread
From: Jakub Kicinski @ 2020-10-01 21:48 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: David S. Miller, Jiri Pirko, netdev, linux-kernel
On Thu, 1 Oct 2020 16:59:08 +0300 Moshe Shemesh wrote:
> Add remote reload stats to hold the history of actions performed due
> devlink reload commands initiated by remote host. For example, in case
> firmware activation with reset finished successfully but was initiated
> by remote host.
>
> The function devlink_remote_reload_actions_performed() is exported to
> enable drivers update on remote reload actions performed as it was not
> initiated by their own devlink instance.
>
> Expose devlink remote reload stats to the user through devlink dev get
> command.
Reviewed-by: Jakub Kicinski <kuba@kernel.org>
> for (i = 0; i <= DEVLINK_RELOAD_ACTION_MAX; i++) {
> - if (!devlink_reload_action_is_supported(devlink, i) ||
> + if ((!is_remote && !devlink_reload_action_is_supported(devlink, i))
I see the point of these checks now, I guess it would have been cleaner
if they were added in this patch, but no big deal.
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 05/16] devlink: Add remote reload stats
2020-10-01 13:59 ` [PATCH net-next 05/16] devlink: Add remote " Moshe Shemesh
2020-10-01 21:48 ` Jakub Kicinski
@ 2020-10-03 9:05 ` Jiri Pirko
2020-10-04 7:09 ` Moshe Shemesh
1 sibling, 1 reply; 49+ messages in thread
From: Jiri Pirko @ 2020-10-03 9:05 UTC (permalink / raw)
To: Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
Thu, Oct 01, 2020 at 03:59:08PM CEST, moshe@mellanox.com wrote:
>Add remote reload stats to hold the history of actions performed due
>devlink reload commands initiated by remote host. For example, in case
>firmware activation with reset finished successfully but was initiated
>by remote host.
>
>The function devlink_remote_reload_actions_performed() is exported to
>enable drivers update on remote reload actions performed as it was not
>initiated by their own devlink instance.
>
>Expose devlink remote reload stats to the user through devlink dev get
>command.
>
>Examples:
>$ devlink dev show
>pci/0000:82:00.0:
> stats:
> reload_stats:
> driver_reinit 2
> fw_activate 1
> fw_activate_no_reset 0
> remote_reload_stats:
> driver_reinit 0
> fw_activate 0
> fw_activate_no_reset 0
>pci/0000:82:00.1:
> stats:
> reload_stats:
> driver_reinit 1
> fw_activate 0
> fw_activate_no_reset 0
> remote_reload_stats:
> driver_reinit 1
> fw_activate 1
> fw_activate_no_reset 0
>
>$ devlink dev show -jp
>{
> "dev": {
> "pci/0000:82:00.0": {
> "stats": {
> "reload_stats": [ {
> "driver_reinit": 2
> },{
> "fw_activate": 1
> },{
> "fw_activate_no_reset": 0
> } ],
> "remote_reload_stats": [ {
> "driver_reinit": 0
> },{
> "fw_activate": 0
> },{
> "fw_activate_no_reset": 0
> } ]
> }
> },
> "pci/0000:82:00.1": {
> "stats": {
> "reload_stats": [ {
> "driver_reinit": 1
> },{
> "fw_activate": 0
> },{
> "fw_activate_no_reset": 0
> } ],
> "remote_reload_stats": [ {
> "driver_reinit": 1
> },{
> "fw_activate": 1
> },{
> "fw_activate_no_reset": 0
> } ]
> }
> }
> }
>}
>
>Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
>---
>RFCv5 -> v1:
>- Resplit this patch and the previous one by remote/local reload stats
>instead of set/get reload stats
>- Rename reload_action_stats to reload_stats
>RFCv4 -> RFCv5:
>- Add remote actions stats
>- If devlink reload is not supported, show only remote_stats
>RFCv3 -> RFCv4:
>- Renamed DEVLINK_ATTR_RELOAD_ACTION_CNT to
> DEVLINK_ATTR_RELOAD_ACTION_STAT
>- Add stats per action per limit level
>RFCv2 -> RFCv3:
>- Add reload actions counters instead of supported reload actions
> (reload actions counters are only for supported action so no need for
> both)
>RFCv1 -> RFCv2:
>- Removed DEVLINK_ATTR_RELOAD_DEFAULT_LEVEL
>- Removed DEVLINK_ATTR_RELOAD_LEVELS_INFO
>- Have actions instead of levels
>---
> include/net/devlink.h | 1 +
> include/uapi/linux/devlink.h | 1 +
> net/core/devlink.c | 49 +++++++++++++++++++++++++++++++-----
> 3 files changed, 45 insertions(+), 6 deletions(-)
>
>diff --git a/include/net/devlink.h b/include/net/devlink.h
>index 0f3bd23b6c04..a4ccb83bbd2c 100644
>--- a/include/net/devlink.h
>+++ b/include/net/devlink.h
>@@ -42,6 +42,7 @@ struct devlink {
> const struct devlink_ops *ops;
> struct xarray snapshot_ids;
> u32 reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
>+ u32 remote_reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
Perhaps a nested struct {} stats?
> struct device *dev;
> possible_net_t _net;
> struct mutex lock; /* Serializes access to devlink instance specific objects such as
>diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>index 97e0137f6201..f9887d8afdc7 100644
>--- a/include/uapi/linux/devlink.h
>+++ b/include/uapi/linux/devlink.h
>@@ -530,6 +530,7 @@ enum devlink_attr {
> DEVLINK_ATTR_RELOAD_STATS, /* nested */
> DEVLINK_ATTR_RELOAD_STATS_ENTRY, /* nested */
> DEVLINK_ATTR_RELOAD_STATS_VALUE, /* u32 */
>+ DEVLINK_ATTR_REMOTE_RELOAD_STATS, /* nested */
>
> /* add new attributes above here, update the policy in devlink.c */
>
>diff --git a/net/core/devlink.c b/net/core/devlink.c
>index 05516f1e4c3e..3b6bd3b4d346 100644
>--- a/net/core/devlink.c
>+++ b/net/core/devlink.c
>@@ -523,28 +523,35 @@ static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_acti
> return -EMSGSIZE;
> }
>
>-static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink)
>+static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink, bool is_remote)
> {
> struct nlattr *reload_stats_attr;
> int i, j, stat_idx;
> u32 value;
>
>- reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
>+ if (!is_remote)
>+ reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
>+ else
>+ reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_REMOTE_RELOAD_STATS);
>
> if (!reload_stats_attr)
> return -EMSGSIZE;
>
> for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) {
>- if (j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
>+ if (!is_remote && j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
I don't follow the check "!is_remote" here,
> !devlink_reload_limit_is_supported(devlink, j))
> continue;
> for (i = 0; i <= DEVLINK_RELOAD_ACTION_MAX; i++) {
>- if (!devlink_reload_action_is_supported(devlink, i) ||
>+ if ((!is_remote && !devlink_reload_action_is_supported(devlink, i)) ||
and here. Could you perhaps put in a comment to describe what are you
doing?
>+ i == DEVLINK_RELOAD_ACTION_UNSPEC ||
> devlink_reload_combination_is_invalid(i, j))
> continue;
>
> stat_idx = j * __DEVLINK_RELOAD_ACTION_MAX + i;
>- value = devlink->reload_stats[stat_idx];
>+ if (!is_remote)
>+ value = devlink->reload_stats[stat_idx];
>+ else
>+ value = devlink->remote_reload_stats[stat_idx];
> if (devlink_reload_stat_put(msg, i, j, value))
> goto nla_put_failure;
> }
>@@ -577,7 +584,9 @@ static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
> if (!dev_stats)
> goto nla_put_failure;
>
>- if (devlink_reload_stats_put(msg, devlink))
>+ if (devlink_reload_stats_put(msg, devlink, false))
>+ goto dev_stats_nest_cancel;
>+ if (devlink_reload_stats_put(msg, devlink, true))
> goto dev_stats_nest_cancel;
>
> nla_nest_end(msg, dev_stats);
>@@ -3100,15 +3109,40 @@ devlink_reload_stats_update(struct devlink *devlink, enum devlink_reload_limit l
> __devlink_reload_stats_update(devlink, devlink->reload_stats, limit, actions_performed);
> }
>
>+/**
>+ * devlink_remote_reload_actions_performed - Update devlink on reload actions
>+ * performed which are not a direct result of devlink reload call.
>+ *
>+ * This should be called by a driver after performing reload actions in case it was not
>+ * a result of devlink reload call. For example fw_activate was performed as a result
>+ * of devlink reload triggered fw_activate on another host.
>+ * The motivation for this function is to keep data on reload actions performed on this
>+ * function whether it was done due to direct devlink reload call or not.
>+ *
>+ * @devlink: devlink
>+ * @limit: reload limit
>+ * @actions_performed: bitmask of actions performed
>+ */
>+void devlink_remote_reload_actions_performed(struct devlink *devlink,
>+ enum devlink_reload_limit limit,
>+ unsigned long actions_performed)
>+{
>+ __devlink_reload_stats_update(devlink, devlink->remote_reload_stats, limit,
>+ actions_performed);
>+}
>+EXPORT_SYMBOL_GPL(devlink_remote_reload_actions_performed);
>+
> static int devlink_reload(struct devlink *devlink, struct net *dest_net,
> enum devlink_reload_action action, enum devlink_reload_limit limit,
> struct netlink_ext_ack *extack, unsigned long *actions_performed)
> {
>+ u32 remote_reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
> int err;
>
> if (!devlink->reload_enabled)
> return -EOPNOTSUPP;
>
>+ memcpy(remote_reload_stats, devlink->remote_reload_stats, sizeof(remote_reload_stats));
> err = devlink->ops->reload_down(devlink, !!dest_net, action, limit, extack);
> if (err)
> return err;
>@@ -3122,6 +3156,9 @@ static int devlink_reload(struct devlink *devlink, struct net *dest_net,
> return err;
>
> WARN_ON(!test_bit(action, actions_performed));
>+ /* Catch driver on updating the remote action within devlink reload */
>+ WARN_ON(memcmp(remote_reload_stats, devlink->remote_reload_stats,
>+ sizeof(remote_reload_stats)));
> devlink_reload_stats_update(devlink, limit, *actions_performed);
> return 0;
> }
>--
>2.18.2
>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 05/16] devlink: Add remote reload stats
2020-10-03 9:05 ` Jiri Pirko
@ 2020-10-04 7:09 ` Moshe Shemesh
2020-10-05 19:12 ` Jacob Keller
0 siblings, 1 reply; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-04 7:09 UTC (permalink / raw)
To: Jiri Pirko, Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
On 10/3/2020 12:05 PM, Jiri Pirko wrote:
> Thu, Oct 01, 2020 at 03:59:08PM CEST, moshe@mellanox.com wrote:
>> Add remote reload stats to hold the history of actions performed due
>> devlink reload commands initiated by remote host. For example, in case
>> firmware activation with reset finished successfully but was initiated
>> by remote host.
>>
>> The function devlink_remote_reload_actions_performed() is exported to
>> enable drivers update on remote reload actions performed as it was not
>> initiated by their own devlink instance.
>>
>> Expose devlink remote reload stats to the user through devlink dev get
>> command.
>>
>> Examples:
>> $ devlink dev show
>> pci/0000:82:00.0:
>> stats:
>> reload_stats:
>> driver_reinit 2
>> fw_activate 1
>> fw_activate_no_reset 0
>> remote_reload_stats:
>> driver_reinit 0
>> fw_activate 0
>> fw_activate_no_reset 0
>> pci/0000:82:00.1:
>> stats:
>> reload_stats:
>> driver_reinit 1
>> fw_activate 0
>> fw_activate_no_reset 0
>> remote_reload_stats:
>> driver_reinit 1
>> fw_activate 1
>> fw_activate_no_reset 0
>>
>> $ devlink dev show -jp
>> {
>> "dev": {
>> "pci/0000:82:00.0": {
>> "stats": {
>> "reload_stats": [ {
>> "driver_reinit": 2
>> },{
>> "fw_activate": 1
>> },{
>> "fw_activate_no_reset": 0
>> } ],
>> "remote_reload_stats": [ {
>> "driver_reinit": 0
>> },{
>> "fw_activate": 0
>> },{
>> "fw_activate_no_reset": 0
>> } ]
>> }
>> },
>> "pci/0000:82:00.1": {
>> "stats": {
>> "reload_stats": [ {
>> "driver_reinit": 1
>> },{
>> "fw_activate": 0
>> },{
>> "fw_activate_no_reset": 0
>> } ],
>> "remote_reload_stats": [ {
>> "driver_reinit": 1
>> },{
>> "fw_activate": 1
>> },{
>> "fw_activate_no_reset": 0
>> } ]
>> }
>> }
>> }
>> }
>>
>> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
>> ---
>> RFCv5 -> v1:
>> - Resplit this patch and the previous one by remote/local reload stats
>> instead of set/get reload stats
>> - Rename reload_action_stats to reload_stats
>> RFCv4 -> RFCv5:
>> - Add remote actions stats
>> - If devlink reload is not supported, show only remote_stats
>> RFCv3 -> RFCv4:
>> - Renamed DEVLINK_ATTR_RELOAD_ACTION_CNT to
>> DEVLINK_ATTR_RELOAD_ACTION_STAT
>> - Add stats per action per limit level
>> RFCv2 -> RFCv3:
>> - Add reload actions counters instead of supported reload actions
>> (reload actions counters are only for supported action so no need for
>> both)
>> RFCv1 -> RFCv2:
>> - Removed DEVLINK_ATTR_RELOAD_DEFAULT_LEVEL
>> - Removed DEVLINK_ATTR_RELOAD_LEVELS_INFO
>> - Have actions instead of levels
>> ---
>> include/net/devlink.h | 1 +
>> include/uapi/linux/devlink.h | 1 +
>> net/core/devlink.c | 49 +++++++++++++++++++++++++++++++-----
>> 3 files changed, 45 insertions(+), 6 deletions(-)
>>
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index 0f3bd23b6c04..a4ccb83bbd2c 100644
>> --- a/include/net/devlink.h
>> +++ b/include/net/devlink.h
>> @@ -42,6 +42,7 @@ struct devlink {
>> const struct devlink_ops *ops;
>> struct xarray snapshot_ids;
>> u32 reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
>> + u32 remote_reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
> Perhaps a nested struct {} stats?
I guess you mean struct that holds these two arrays.
>
>> struct device *dev;
>> possible_net_t _net;
>> struct mutex lock; /* Serializes access to devlink instance specific objects such as
>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>> index 97e0137f6201..f9887d8afdc7 100644
>> --- a/include/uapi/linux/devlink.h
>> +++ b/include/uapi/linux/devlink.h
>> @@ -530,6 +530,7 @@ enum devlink_attr {
>> DEVLINK_ATTR_RELOAD_STATS, /* nested */
>> DEVLINK_ATTR_RELOAD_STATS_ENTRY, /* nested */
>> DEVLINK_ATTR_RELOAD_STATS_VALUE, /* u32 */
>> + DEVLINK_ATTR_REMOTE_RELOAD_STATS, /* nested */
>>
>> /* add new attributes above here, update the policy in devlink.c */
>>
>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>> index 05516f1e4c3e..3b6bd3b4d346 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -523,28 +523,35 @@ static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_acti
>> return -EMSGSIZE;
>> }
>>
>> -static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink)
>> +static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink, bool is_remote)
>> {
>> struct nlattr *reload_stats_attr;
>> int i, j, stat_idx;
>> u32 value;
>>
>> - reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
>> + if (!is_remote)
>> + reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
>> + else
>> + reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_REMOTE_RELOAD_STATS);
>>
>> if (!reload_stats_attr)
>> return -EMSGSIZE;
>>
>> for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) {
>> - if (j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
>> + if (!is_remote && j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
> I don't follow the check "!is_remote" here,
We agreed that remote stats should be shown also for non supported
actions and limits, because its remote. So it makes this condition
different for remote stats. Rethinking about it, maybe that's wrong. I
mean if we had here reload actions as a result of remote driver, they
have common device, so it has to be the same type of driver and support
same actions/limits, right ?
>> !devlink_reload_limit_is_supported(devlink, j))
>> continue;
>> for (i = 0; i <= DEVLINK_RELOAD_ACTION_MAX; i++) {
>> - if (!devlink_reload_action_is_supported(devlink, i) ||
>> + if ((!is_remote && !devlink_reload_action_is_supported(devlink, i)) ||
> and here. Could you perhaps put in a comment to describe what are you
> doing?
>
Yes, similar.
>
>> + i == DEVLINK_RELOAD_ACTION_UNSPEC ||
>> devlink_reload_combination_is_invalid(i, j))
>> continue;
>>
>> stat_idx = j * __DEVLINK_RELOAD_ACTION_MAX + i;
>> - value = devlink->reload_stats[stat_idx];
>> + if (!is_remote)
>> + value = devlink->reload_stats[stat_idx];
>> + else
>> + value = devlink->remote_reload_stats[stat_idx];
>> if (devlink_reload_stat_put(msg, i, j, value))
>> goto nla_put_failure;
>> }
>> @@ -577,7 +584,9 @@ static int devlink_nl_fill(struct sk_buff *msg, struct devlink *devlink,
>> if (!dev_stats)
>> goto nla_put_failure;
>>
>> - if (devlink_reload_stats_put(msg, devlink))
>> + if (devlink_reload_stats_put(msg, devlink, false))
>> + goto dev_stats_nest_cancel;
>> + if (devlink_reload_stats_put(msg, devlink, true))
>> goto dev_stats_nest_cancel;
>>
>> nla_nest_end(msg, dev_stats);
>> @@ -3100,15 +3109,40 @@ devlink_reload_stats_update(struct devlink *devlink, enum devlink_reload_limit l
>> __devlink_reload_stats_update(devlink, devlink->reload_stats, limit, actions_performed);
>> }
>>
>> +/**
>> + * devlink_remote_reload_actions_performed - Update devlink on reload actions
>> + * performed which are not a direct result of devlink reload call.
>> + *
>> + * This should be called by a driver after performing reload actions in case it was not
>> + * a result of devlink reload call. For example fw_activate was performed as a result
>> + * of devlink reload triggered fw_activate on another host.
>> + * The motivation for this function is to keep data on reload actions performed on this
>> + * function whether it was done due to direct devlink reload call or not.
>> + *
>> + * @devlink: devlink
>> + * @limit: reload limit
>> + * @actions_performed: bitmask of actions performed
>> + */
>> +void devlink_remote_reload_actions_performed(struct devlink *devlink,
>> + enum devlink_reload_limit limit,
>> + unsigned long actions_performed)
>> +{
>> + __devlink_reload_stats_update(devlink, devlink->remote_reload_stats, limit,
>> + actions_performed);
>> +}
>> +EXPORT_SYMBOL_GPL(devlink_remote_reload_actions_performed);
>> +
>> static int devlink_reload(struct devlink *devlink, struct net *dest_net,
>> enum devlink_reload_action action, enum devlink_reload_limit limit,
>> struct netlink_ext_ack *extack, unsigned long *actions_performed)
>> {
>> + u32 remote_reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
>> int err;
>>
>> if (!devlink->reload_enabled)
>> return -EOPNOTSUPP;
>>
>> + memcpy(remote_reload_stats, devlink->remote_reload_stats, sizeof(remote_reload_stats));
>> err = devlink->ops->reload_down(devlink, !!dest_net, action, limit, extack);
>> if (err)
>> return err;
>> @@ -3122,6 +3156,9 @@ static int devlink_reload(struct devlink *devlink, struct net *dest_net,
>> return err;
>>
>> WARN_ON(!test_bit(action, actions_performed));
>> + /* Catch driver on updating the remote action within devlink reload */
>> + WARN_ON(memcmp(remote_reload_stats, devlink->remote_reload_stats,
>> + sizeof(remote_reload_stats)));
>> devlink_reload_stats_update(devlink, limit, *actions_performed);
>> return 0;
>> }
>> --
>> 2.18.2
>>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 05/16] devlink: Add remote reload stats
2020-10-04 7:09 ` Moshe Shemesh
@ 2020-10-05 19:12 ` Jacob Keller
2020-10-07 5:41 ` Moshe Shemesh
0 siblings, 1 reply; 49+ messages in thread
From: Jacob Keller @ 2020-10-05 19:12 UTC (permalink / raw)
To: Moshe Shemesh, Jiri Pirko, Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
On 10/4/2020 12:09 AM, Moshe Shemesh wrote:
>
> On 10/3/2020 12:05 PM, Jiri Pirko wrote:
>> Thu, Oct 01, 2020 at 03:59:08PM CEST, moshe@mellanox.com wrote:
>>> Add remote reload stats to hold the history of actions performed due
>>> devlink reload commands initiated by remote host. For example, in case
>>> firmware activation with reset finished successfully but was initiated
>>> by remote host.
>>>
>>> The function devlink_remote_reload_actions_performed() is exported to
>>> enable drivers update on remote reload actions performed as it was not
>>> initiated by their own devlink instance.
>>>
>>> Expose devlink remote reload stats to the user through devlink dev get
>>> command.
>>>
>>> Examples:
>>> $ devlink dev show
>>> pci/0000:82:00.0:
>>> stats:
>>> reload_stats:
>>> driver_reinit 2
>>> fw_activate 1
>>> fw_activate_no_reset 0
>>> remote_reload_stats:
>>> driver_reinit 0
>>> fw_activate 0
>>> fw_activate_no_reset 0
>>> pci/0000:82:00.1:
>>> stats:
>>> reload_stats:
>>> driver_reinit 1
>>> fw_activate 0
>>> fw_activate_no_reset 0
>>> remote_reload_stats:
>>> driver_reinit 1
>>> fw_activate 1
>>> fw_activate_no_reset 0
>>>
>>> $ devlink dev show -jp
>>> {
>>> "dev": {
>>> "pci/0000:82:00.0": {
>>> "stats": {
>>> "reload_stats": [ {
>>> "driver_reinit": 2
>>> },{
>>> "fw_activate": 1
>>> },{
>>> "fw_activate_no_reset": 0
>>> } ],
>>> "remote_reload_stats": [ {
>>> "driver_reinit": 0
>>> },{
>>> "fw_activate": 0
>>> },{
>>> "fw_activate_no_reset": 0
>>> } ]
>>> }
>>> },
>>> "pci/0000:82:00.1": {
>>> "stats": {
>>> "reload_stats": [ {
>>> "driver_reinit": 1
>>> },{
>>> "fw_activate": 0
>>> },{
>>> "fw_activate_no_reset": 0
>>> } ],
>>> "remote_reload_stats": [ {
>>> "driver_reinit": 1
>>> },{
>>> "fw_activate": 1
>>> },{
>>> "fw_activate_no_reset": 0
>>> } ]
>>> }
>>> }
>>> }
>>> }
>>>
>>> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
>>> ---
>>> RFCv5 -> v1:
>>> - Resplit this patch and the previous one by remote/local reload stats
>>> instead of set/get reload stats
>>> - Rename reload_action_stats to reload_stats
>>> RFCv4 -> RFCv5:
>>> - Add remote actions stats
>>> - If devlink reload is not supported, show only remote_stats
>>> RFCv3 -> RFCv4:
>>> - Renamed DEVLINK_ATTR_RELOAD_ACTION_CNT to
>>> DEVLINK_ATTR_RELOAD_ACTION_STAT
>>> - Add stats per action per limit level
>>> RFCv2 -> RFCv3:
>>> - Add reload actions counters instead of supported reload actions
>>> (reload actions counters are only for supported action so no need for
>>> both)
>>> RFCv1 -> RFCv2:
>>> - Removed DEVLINK_ATTR_RELOAD_DEFAULT_LEVEL
>>> - Removed DEVLINK_ATTR_RELOAD_LEVELS_INFO
>>> - Have actions instead of levels
>>> ---
>>> include/net/devlink.h | 1 +
>>> include/uapi/linux/devlink.h | 1 +
>>> net/core/devlink.c | 49 +++++++++++++++++++++++++++++++-----
>>> 3 files changed, 45 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>>> index 0f3bd23b6c04..a4ccb83bbd2c 100644
>>> --- a/include/net/devlink.h
>>> +++ b/include/net/devlink.h
>>> @@ -42,6 +42,7 @@ struct devlink {
>>> const struct devlink_ops *ops;
>>> struct xarray snapshot_ids;
>>> u32 reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
>>> + u32 remote_reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
>> Perhaps a nested struct {} stats?
> I guess you mean struct that holds these two arrays.
>>
>>> struct device *dev;
>>> possible_net_t _net;
>>> struct mutex lock; /* Serializes access to devlink instance specific objects such as
>>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>>> index 97e0137f6201..f9887d8afdc7 100644
>>> --- a/include/uapi/linux/devlink.h
>>> +++ b/include/uapi/linux/devlink.h
>>> @@ -530,6 +530,7 @@ enum devlink_attr {
>>> DEVLINK_ATTR_RELOAD_STATS, /* nested */
>>> DEVLINK_ATTR_RELOAD_STATS_ENTRY, /* nested */
>>> DEVLINK_ATTR_RELOAD_STATS_VALUE, /* u32 */
>>> + DEVLINK_ATTR_REMOTE_RELOAD_STATS, /* nested */
>>>
>>> /* add new attributes above here, update the policy in devlink.c */
>>>
>>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>>> index 05516f1e4c3e..3b6bd3b4d346 100644
>>> --- a/net/core/devlink.c
>>> +++ b/net/core/devlink.c
>>> @@ -523,28 +523,35 @@ static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_acti
>>> return -EMSGSIZE;
>>> }
>>>
>>> -static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink)
>>> +static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink, bool is_remote)
>>> {
>>> struct nlattr *reload_stats_attr;
>>> int i, j, stat_idx;
>>> u32 value;
>>>
>>> - reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
>>> + if (!is_remote)
>>> + reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
>>> + else
>>> + reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_REMOTE_RELOAD_STATS);
>>>
>>> if (!reload_stats_attr)
>>> return -EMSGSIZE;
>>>
>>> for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) {
>>> - if (j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
>>> + if (!is_remote && j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
>> I don't follow the check "!is_remote" here,
>
>
> We agreed that remote stats should be shown also for non supported
> actions and limits, because its remote. So it makes this condition
> different for remote stats. Rethinking about it, maybe that's wrong. I
> mean if we had here reload actions as a result of remote driver, they
> have common device, so it has to be the same type of driver and support
> same actions/limits, right ?
>
Obviously it runs the same device but.. technically, couldn't the remote
device be running a different version of the driver? i.e. what if it
supports some new mode that this host doesn't yet understand? (or does
understand but has a driver which doesn't yet?)
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 05/16] devlink: Add remote reload stats
2020-10-05 19:12 ` Jacob Keller
@ 2020-10-07 5:41 ` Moshe Shemesh
0 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-07 5:41 UTC (permalink / raw)
To: Jacob Keller, Jiri Pirko, Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
On 10/5/2020 10:12 PM, Jacob Keller wrote:
>
> On 10/4/2020 12:09 AM, Moshe Shemesh wrote:
>> On 10/3/2020 12:05 PM, Jiri Pirko wrote:
>>> Thu, Oct 01, 2020 at 03:59:08PM CEST, moshe@mellanox.com wrote:
>>>> Add remote reload stats to hold the history of actions performed due
>>>> devlink reload commands initiated by remote host. For example, in case
>>>> firmware activation with reset finished successfully but was initiated
>>>> by remote host.
>>>>
>>>> The function devlink_remote_reload_actions_performed() is exported to
>>>> enable drivers update on remote reload actions performed as it was not
>>>> initiated by their own devlink instance.
>>>>
>>>> Expose devlink remote reload stats to the user through devlink dev get
>>>> command.
>>>>
>>>> Examples:
>>>> $ devlink dev show
>>>> pci/0000:82:00.0:
>>>> stats:
>>>> reload_stats:
>>>> driver_reinit 2
>>>> fw_activate 1
>>>> fw_activate_no_reset 0
>>>> remote_reload_stats:
>>>> driver_reinit 0
>>>> fw_activate 0
>>>> fw_activate_no_reset 0
>>>> pci/0000:82:00.1:
>>>> stats:
>>>> reload_stats:
>>>> driver_reinit 1
>>>> fw_activate 0
>>>> fw_activate_no_reset 0
>>>> remote_reload_stats:
>>>> driver_reinit 1
>>>> fw_activate 1
>>>> fw_activate_no_reset 0
>>>>
>>>> $ devlink dev show -jp
>>>> {
>>>> "dev": {
>>>> "pci/0000:82:00.0": {
>>>> "stats": {
>>>> "reload_stats": [ {
>>>> "driver_reinit": 2
>>>> },{
>>>> "fw_activate": 1
>>>> },{
>>>> "fw_activate_no_reset": 0
>>>> } ],
>>>> "remote_reload_stats": [ {
>>>> "driver_reinit": 0
>>>> },{
>>>> "fw_activate": 0
>>>> },{
>>>> "fw_activate_no_reset": 0
>>>> } ]
>>>> }
>>>> },
>>>> "pci/0000:82:00.1": {
>>>> "stats": {
>>>> "reload_stats": [ {
>>>> "driver_reinit": 1
>>>> },{
>>>> "fw_activate": 0
>>>> },{
>>>> "fw_activate_no_reset": 0
>>>> } ],
>>>> "remote_reload_stats": [ {
>>>> "driver_reinit": 1
>>>> },{
>>>> "fw_activate": 1
>>>> },{
>>>> "fw_activate_no_reset": 0
>>>> } ]
>>>> }
>>>> }
>>>> }
>>>> }
>>>>
>>>> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
>>>> ---
>>>> RFCv5 -> v1:
>>>> - Resplit this patch and the previous one by remote/local reload stats
>>>> instead of set/get reload stats
>>>> - Rename reload_action_stats to reload_stats
>>>> RFCv4 -> RFCv5:
>>>> - Add remote actions stats
>>>> - If devlink reload is not supported, show only remote_stats
>>>> RFCv3 -> RFCv4:
>>>> - Renamed DEVLINK_ATTR_RELOAD_ACTION_CNT to
>>>> DEVLINK_ATTR_RELOAD_ACTION_STAT
>>>> - Add stats per action per limit level
>>>> RFCv2 -> RFCv3:
>>>> - Add reload actions counters instead of supported reload actions
>>>> (reload actions counters are only for supported action so no need for
>>>> both)
>>>> RFCv1 -> RFCv2:
>>>> - Removed DEVLINK_ATTR_RELOAD_DEFAULT_LEVEL
>>>> - Removed DEVLINK_ATTR_RELOAD_LEVELS_INFO
>>>> - Have actions instead of levels
>>>> ---
>>>> include/net/devlink.h | 1 +
>>>> include/uapi/linux/devlink.h | 1 +
>>>> net/core/devlink.c | 49 +++++++++++++++++++++++++++++++-----
>>>> 3 files changed, 45 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>>>> index 0f3bd23b6c04..a4ccb83bbd2c 100644
>>>> --- a/include/net/devlink.h
>>>> +++ b/include/net/devlink.h
>>>> @@ -42,6 +42,7 @@ struct devlink {
>>>> const struct devlink_ops *ops;
>>>> struct xarray snapshot_ids;
>>>> u32 reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
>>>> + u32 remote_reload_stats[DEVLINK_RELOAD_STATS_ARRAY_SIZE];
>>> Perhaps a nested struct {} stats?
>> I guess you mean struct that holds these two arrays.
>>>> struct device *dev;
>>>> possible_net_t _net;
>>>> struct mutex lock; /* Serializes access to devlink instance specific objects such as
>>>> diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
>>>> index 97e0137f6201..f9887d8afdc7 100644
>>>> --- a/include/uapi/linux/devlink.h
>>>> +++ b/include/uapi/linux/devlink.h
>>>> @@ -530,6 +530,7 @@ enum devlink_attr {
>>>> DEVLINK_ATTR_RELOAD_STATS, /* nested */
>>>> DEVLINK_ATTR_RELOAD_STATS_ENTRY, /* nested */
>>>> DEVLINK_ATTR_RELOAD_STATS_VALUE, /* u32 */
>>>> + DEVLINK_ATTR_REMOTE_RELOAD_STATS, /* nested */
>>>>
>>>> /* add new attributes above here, update the policy in devlink.c */
>>>>
>>>> diff --git a/net/core/devlink.c b/net/core/devlink.c
>>>> index 05516f1e4c3e..3b6bd3b4d346 100644
>>>> --- a/net/core/devlink.c
>>>> +++ b/net/core/devlink.c
>>>> @@ -523,28 +523,35 @@ static int devlink_reload_stat_put(struct sk_buff *msg, enum devlink_reload_acti
>>>> return -EMSGSIZE;
>>>> }
>>>>
>>>> -static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink)
>>>> +static int devlink_reload_stats_put(struct sk_buff *msg, struct devlink *devlink, bool is_remote)
>>>> {
>>>> struct nlattr *reload_stats_attr;
>>>> int i, j, stat_idx;
>>>> u32 value;
>>>>
>>>> - reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
>>>> + if (!is_remote)
>>>> + reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_RELOAD_STATS);
>>>> + else
>>>> + reload_stats_attr = nla_nest_start(msg, DEVLINK_ATTR_REMOTE_RELOAD_STATS);
>>>>
>>>> if (!reload_stats_attr)
>>>> return -EMSGSIZE;
>>>>
>>>> for (j = 0; j <= DEVLINK_RELOAD_LIMIT_MAX; j++) {
>>>> - if (j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
>>>> + if (!is_remote && j != DEVLINK_RELOAD_LIMIT_UNSPEC &&
>>> I don't follow the check "!is_remote" here,
>>
>> We agreed that remote stats should be shown also for non supported
>> actions and limits, because its remote. So it makes this condition
>> different for remote stats. Rethinking about it, maybe that's wrong. I
>> mean if we had here reload actions as a result of remote driver, they
>> have common device, so it has to be the same type of driver and support
>> same actions/limits, right ?
>>
> Obviously it runs the same device but.. technically, couldn't the remote
> device be running a different version of the driver? i.e. what if it
> supports some new mode that this host doesn't yet understand? (or does
> understand but has a driver which doesn't yet?)
Yes, also there is a possibility that one host function has privilege to
do an action that the other doesn't have. I see there are reasons to
keep this diff between remote stats and local. I will keep it. Thanks.
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH net-next 06/16] net/mlx5: Add functions to set/query MFRL register
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (4 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 05/16] devlink: Add remote " Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 07/16] net/mlx5: Set cap for pci sync for fw update event Moshe Shemesh
` (9 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Add functions to query and set the MFRL reset options supported by
firmware.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
---
RFCv5 -> v1:
- Renamed non-static functions to have module prefix
---
.../net/ethernet/mellanox/mlx5/core/Makefile | 2 +-
.../ethernet/mellanox/mlx5/core/fw_reset.c | 51 +++++++++++++++++++
.../ethernet/mellanox/mlx5/core/fw_reset.h | 13 +++++
3 files changed, 65 insertions(+), 1 deletion(-)
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
create mode 100644 drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/Makefile b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
index 9826a041e407..380a51747599 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/Makefile
+++ b/drivers/net/ethernet/mellanox/mlx5/core/Makefile
@@ -16,7 +16,7 @@ mlx5_core-y := main.o cmd.o debugfs.o fw.o eq.o uar.o pagealloc.o \
transobj.o vport.o sriov.o fs_cmd.o fs_core.o pci_irq.o \
fs_counters.o rl.o lag.o dev.o events.o wq.o lib/gid.o \
lib/devcom.o lib/pci_vsc.o lib/dm.o diag/fs_tracepoint.o \
- diag/fw_tracer.o diag/crdump.o devlink.o diag/rsc_dump.o
+ diag/fw_tracer.o diag/crdump.o devlink.o diag/rsc_dump.o fw_reset.o
#
# Netdev basic
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
new file mode 100644
index 000000000000..7feae827b4bc
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
@@ -0,0 +1,51 @@
+// SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB
+/* Copyright (c) 2020, Mellanox Technologies inc. All rights reserved. */
+
+#include "fw_reset.h"
+
+static int mlx5_reg_mfrl_set(struct mlx5_core_dev *dev, u8 reset_level,
+ u8 reset_type_sel, u8 sync_resp, bool sync_start)
+{
+ u32 out[MLX5_ST_SZ_DW(mfrl_reg)] = {};
+ u32 in[MLX5_ST_SZ_DW(mfrl_reg)] = {};
+
+ MLX5_SET(mfrl_reg, in, reset_level, reset_level);
+ MLX5_SET(mfrl_reg, in, rst_type_sel, reset_type_sel);
+ MLX5_SET(mfrl_reg, in, pci_sync_for_fw_update_resp, sync_resp);
+ MLX5_SET(mfrl_reg, in, pci_sync_for_fw_update_start, sync_start);
+
+ return mlx5_core_access_reg(dev, in, sizeof(in), out, sizeof(out), MLX5_REG_MFRL, 0, 1);
+}
+
+static int mlx5_reg_mfrl_query(struct mlx5_core_dev *dev, u8 *reset_level, u8 *reset_type)
+{
+ u32 out[MLX5_ST_SZ_DW(mfrl_reg)] = {};
+ u32 in[MLX5_ST_SZ_DW(mfrl_reg)] = {};
+ int err;
+
+ err = mlx5_core_access_reg(dev, in, sizeof(in), out, sizeof(out), MLX5_REG_MFRL, 0, 0);
+ if (err)
+ return err;
+
+ if (reset_level)
+ *reset_level = MLX5_GET(mfrl_reg, out, reset_level);
+ if (reset_type)
+ *reset_type = MLX5_GET(mfrl_reg, out, reset_type);
+
+ return 0;
+}
+
+int mlx5_fw_reset_query(struct mlx5_core_dev *dev, u8 *reset_level, u8 *reset_type)
+{
+ return mlx5_reg_mfrl_query(dev, reset_level, reset_type);
+}
+
+int mlx5_fw_reset_set_reset_sync(struct mlx5_core_dev *dev, u8 reset_type_sel)
+{
+ return mlx5_reg_mfrl_set(dev, MLX5_MFRL_REG_RESET_LEVEL3, reset_type_sel, 0, true);
+}
+
+int mlx5_fw_reset_set_live_patch(struct mlx5_core_dev *dev)
+{
+ return mlx5_reg_mfrl_set(dev, MLX5_MFRL_REG_RESET_LEVEL0, 0, 0, false);
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
new file mode 100644
index 000000000000..10b5f108cc8b
--- /dev/null
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 OR Linux-OpenIB */
+/* Copyright (c) 2020, Mellanox Technologies inc. All rights reserved. */
+
+#ifndef __MLX5_FW_RESET_H
+#define __MLX5_FW_RESET_H
+
+#include "mlx5_core.h"
+
+int mlx5_fw_reset_query(struct mlx5_core_dev *dev, u8 *reset_level, u8 *reset_type);
+int mlx5_fw_reset_set_reset_sync(struct mlx5_core_dev *dev, u8 reset_type_sel);
+int mlx5_fw_reset_set_live_patch(struct mlx5_core_dev *dev);
+
+#endif
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH net-next 07/16] net/mlx5: Set cap for pci sync for fw update event
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (5 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 06/16] net/mlx5: Add functions to set/query MFRL register Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 08/16] net/mlx5: Handle sync reset request event Moshe Shemesh
` (8 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Set capability to notify the firmware that this host driver is capable
of handling pci sync for firmware update events.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
---
drivers/net/ethernet/mellanox/mlx5/core/main.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index ce43e3feccd9..871d28b09f8a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -548,6 +548,9 @@ static int handle_hca_cap(struct mlx5_core_dev *dev, void *set_ctx)
if (MLX5_CAP_GEN_MAX(dev, dct))
MLX5_SET(cmd_hca_cap, set_hca_cap, dct, 1);
+ if (MLX5_CAP_GEN_MAX(dev, pci_sync_for_fw_update_event))
+ MLX5_SET(cmd_hca_cap, set_hca_cap, pci_sync_for_fw_update_event, 1);
+
if (MLX5_CAP_GEN_MAX(dev, num_vhca_ports))
MLX5_SET(cmd_hca_cap,
set_hca_cap,
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH net-next 08/16] net/mlx5: Handle sync reset request event
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (6 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 07/16] net/mlx5: Set cap for pci sync for fw update event Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 09/16] net/mlx5: Handle sync reset now event Moshe Shemesh
` (7 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Once the driver gets sync_reset_request from firmware it prepares for the
coming reset and sends acknowledge.
After getting this event the driver expects device reset, either it will
trigger PCI reset on sync_reset_now event or such PCI reset will be
triggered by another PF of the same device. So it moves to reset
requested mode and if it gets PCI reset triggered by the other PF it
detect the reset and reloads.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
---
RFCv1 -> RFCv2:
- Moved handling of sync reset recovery from health to fw_reset
---
.../ethernet/mellanox/mlx5/core/fw_reset.c | 178 ++++++++++++++++++
.../ethernet/mellanox/mlx5/core/fw_reset.h | 5 +
.../net/ethernet/mellanox/mlx5/core/health.c | 35 ++--
.../net/ethernet/mellanox/mlx5/core/main.c | 13 ++
.../ethernet/mellanox/mlx5/core/mlx5_core.h | 2 +
include/linux/mlx5/driver.h | 2 +
6 files changed, 220 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
index 7feae827b4bc..cd1b4e1b56ba 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
@@ -3,6 +3,20 @@
#include "fw_reset.h"
+enum {
+ MLX5_FW_RESET_FLAGS_RESET_REQUESTED,
+};
+
+struct mlx5_fw_reset {
+ struct mlx5_core_dev *dev;
+ struct mlx5_nb nb;
+ struct workqueue_struct *wq;
+ struct work_struct reset_request_work;
+ struct work_struct reset_reload_work;
+ unsigned long reset_flags;
+ struct timer_list timer;
+};
+
static int mlx5_reg_mfrl_set(struct mlx5_core_dev *dev, u8 reset_level,
u8 reset_type_sel, u8 sync_resp, bool sync_start)
{
@@ -49,3 +63,167 @@ int mlx5_fw_reset_set_live_patch(struct mlx5_core_dev *dev)
{
return mlx5_reg_mfrl_set(dev, MLX5_MFRL_REG_RESET_LEVEL0, 0, 0, false);
}
+
+static void mlx5_sync_reset_reload_work(struct work_struct *work)
+{
+ struct mlx5_fw_reset *fw_reset = container_of(work, struct mlx5_fw_reset,
+ reset_reload_work);
+ struct mlx5_core_dev *dev = fw_reset->dev;
+
+ mlx5_enter_error_state(dev, true);
+ mlx5_unload_one(dev, false);
+ if (mlx5_health_wait_pci_up(dev)) {
+ mlx5_core_err(dev, "reset reload flow aborted, PCI reads still not working\n");
+ return;
+ }
+ mlx5_load_one(dev, false);
+}
+
+static void mlx5_stop_sync_reset_poll(struct mlx5_core_dev *dev)
+{
+ struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
+
+ del_timer(&fw_reset->timer);
+}
+
+static void mlx5_sync_reset_clear_reset_requested(struct mlx5_core_dev *dev, bool poll_health)
+{
+ struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
+
+ mlx5_stop_sync_reset_poll(dev);
+ clear_bit(MLX5_FW_RESET_FLAGS_RESET_REQUESTED, &fw_reset->reset_flags);
+ if (poll_health)
+ mlx5_start_health_poll(dev);
+}
+
+#define MLX5_RESET_POLL_INTERVAL (HZ / 10)
+static void poll_sync_reset(struct timer_list *t)
+{
+ struct mlx5_fw_reset *fw_reset = from_timer(fw_reset, t, timer);
+ struct mlx5_core_dev *dev = fw_reset->dev;
+ u32 fatal_error;
+
+ if (!test_bit(MLX5_FW_RESET_FLAGS_RESET_REQUESTED, &fw_reset->reset_flags))
+ return;
+
+ fatal_error = mlx5_health_check_fatal_sensors(dev);
+
+ if (fatal_error) {
+ mlx5_core_warn(dev, "Got Device Reset\n");
+ mlx5_sync_reset_clear_reset_requested(dev, false);
+ queue_work(fw_reset->wq, &fw_reset->reset_reload_work);
+ return;
+ }
+
+ mod_timer(&fw_reset->timer, round_jiffies(jiffies + MLX5_RESET_POLL_INTERVAL));
+}
+
+static void mlx5_start_sync_reset_poll(struct mlx5_core_dev *dev)
+{
+ struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
+
+ timer_setup(&fw_reset->timer, poll_sync_reset, 0);
+ fw_reset->timer.expires = round_jiffies(jiffies + MLX5_RESET_POLL_INTERVAL);
+ add_timer(&fw_reset->timer);
+}
+
+static int mlx5_fw_reset_set_reset_sync_ack(struct mlx5_core_dev *dev)
+{
+ return mlx5_reg_mfrl_set(dev, MLX5_MFRL_REG_RESET_LEVEL3, 0, 1, false);
+}
+
+static void mlx5_sync_reset_set_reset_requested(struct mlx5_core_dev *dev)
+{
+ struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
+
+ mlx5_stop_health_poll(dev, true);
+ set_bit(MLX5_FW_RESET_FLAGS_RESET_REQUESTED, &fw_reset->reset_flags);
+ mlx5_start_sync_reset_poll(dev);
+}
+
+static void mlx5_sync_reset_request_event(struct work_struct *work)
+{
+ struct mlx5_fw_reset *fw_reset = container_of(work, struct mlx5_fw_reset,
+ reset_request_work);
+ struct mlx5_core_dev *dev = fw_reset->dev;
+ int err;
+
+ mlx5_sync_reset_set_reset_requested(dev);
+ err = mlx5_fw_reset_set_reset_sync_ack(dev);
+ if (err)
+ mlx5_core_warn(dev, "PCI Sync FW Update Reset Ack Failed. Error code: %d\n", err);
+ else
+ mlx5_core_warn(dev, "PCI Sync FW Update Reset Ack. Device reset is expected.\n");
+}
+
+static void mlx5_sync_reset_events_handle(struct mlx5_fw_reset *fw_reset, struct mlx5_eqe *eqe)
+{
+ struct mlx5_eqe_sync_fw_update *sync_fw_update_eqe;
+ u8 sync_event_rst_type;
+
+ sync_fw_update_eqe = &eqe->data.sync_fw_update;
+ sync_event_rst_type = sync_fw_update_eqe->sync_rst_state & SYNC_RST_STATE_MASK;
+ switch (sync_event_rst_type) {
+ case MLX5_SYNC_RST_STATE_RESET_REQUEST:
+ queue_work(fw_reset->wq, &fw_reset->reset_request_work);
+ break;
+ }
+}
+
+static int fw_reset_event_notifier(struct notifier_block *nb, unsigned long action, void *data)
+{
+ struct mlx5_fw_reset *fw_reset = mlx5_nb_cof(nb, struct mlx5_fw_reset, nb);
+ struct mlx5_eqe *eqe = data;
+
+ switch (eqe->sub_type) {
+ case MLX5_GENERAL_SUBTYPE_PCI_SYNC_FOR_FW_UPDATE_EVENT:
+ mlx5_sync_reset_events_handle(fw_reset, eqe);
+ break;
+ default:
+ return NOTIFY_DONE;
+ }
+
+ return NOTIFY_OK;
+}
+
+void mlx5_fw_reset_events_start(struct mlx5_core_dev *dev)
+{
+ struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
+
+ MLX5_NB_INIT(&fw_reset->nb, fw_reset_event_notifier, GENERAL_EVENT);
+ mlx5_eq_notifier_register(dev, &fw_reset->nb);
+}
+
+void mlx5_fw_reset_events_stop(struct mlx5_core_dev *dev)
+{
+ mlx5_eq_notifier_unregister(dev, &dev->priv.fw_reset->nb);
+}
+
+int mlx5_fw_reset_init(struct mlx5_core_dev *dev)
+{
+ struct mlx5_fw_reset *fw_reset = kzalloc(sizeof(*fw_reset), GFP_KERNEL);
+
+ if (!fw_reset)
+ return -ENOMEM;
+ fw_reset->wq = create_singlethread_workqueue("mlx5_fw_reset_events");
+ if (!fw_reset->wq) {
+ kfree(fw_reset);
+ return -ENOMEM;
+ }
+
+ fw_reset->dev = dev;
+ dev->priv.fw_reset = fw_reset;
+
+ INIT_WORK(&fw_reset->reset_request_work, mlx5_sync_reset_request_event);
+ INIT_WORK(&fw_reset->reset_reload_work, mlx5_sync_reset_reload_work);
+
+ return 0;
+}
+
+void mlx5_fw_reset_cleanup(struct mlx5_core_dev *dev)
+{
+ struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
+
+ destroy_workqueue(fw_reset->wq);
+ kfree(dev->priv.fw_reset);
+}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
index 10b5f108cc8b..a231f7848a8f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
@@ -10,4 +10,9 @@ int mlx5_fw_reset_query(struct mlx5_core_dev *dev, u8 *reset_level, u8 *reset_ty
int mlx5_fw_reset_set_reset_sync(struct mlx5_core_dev *dev, u8 reset_type_sel);
int mlx5_fw_reset_set_live_patch(struct mlx5_core_dev *dev);
+void mlx5_fw_reset_events_start(struct mlx5_core_dev *dev);
+void mlx5_fw_reset_events_stop(struct mlx5_core_dev *dev);
+int mlx5_fw_reset_init(struct mlx5_core_dev *dev);
+void mlx5_fw_reset_cleanup(struct mlx5_core_dev *dev);
+
#endif
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/health.c b/drivers/net/ethernet/mellanox/mlx5/core/health.c
index b31f769d2df9..54523bed16cd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/health.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/health.c
@@ -110,7 +110,7 @@ static bool sensor_fw_synd_rfr(struct mlx5_core_dev *dev)
return rfr && synd;
}
-static u32 check_fatal_sensors(struct mlx5_core_dev *dev)
+u32 mlx5_health_check_fatal_sensors(struct mlx5_core_dev *dev)
{
if (sensor_pci_not_working(dev))
return MLX5_SENSOR_PCI_COMM_ERR;
@@ -173,7 +173,7 @@ static bool reset_fw_if_needed(struct mlx5_core_dev *dev)
* Check again to avoid a redundant 2nd reset. If the fatal erros was
* PCI related a reset won't help.
*/
- fatal_error = check_fatal_sensors(dev);
+ fatal_error = mlx5_health_check_fatal_sensors(dev);
if (fatal_error == MLX5_SENSOR_PCI_COMM_ERR ||
fatal_error == MLX5_SENSOR_NIC_DISABLED ||
fatal_error == MLX5_SENSOR_NIC_SW_RESET) {
@@ -195,7 +195,7 @@ void mlx5_enter_error_state(struct mlx5_core_dev *dev, bool force)
bool err_detected = false;
/* Mark the device as fatal in order to abort FW commands */
- if ((check_fatal_sensors(dev) || force) &&
+ if ((mlx5_health_check_fatal_sensors(dev) || force) &&
dev->state == MLX5_DEVICE_STATE_UP) {
dev->state = MLX5_DEVICE_STATE_INTERNAL_ERROR;
err_detected = true;
@@ -208,7 +208,7 @@ void mlx5_enter_error_state(struct mlx5_core_dev *dev, bool force)
goto unlock;
}
- if (check_fatal_sensors(dev) || force) { /* protected state setting */
+ if (mlx5_health_check_fatal_sensors(dev) || force) { /* protected state setting */
dev->state = MLX5_DEVICE_STATE_INTERNAL_ERROR;
mlx5_cmd_flush(dev);
}
@@ -231,7 +231,7 @@ void mlx5_error_sw_reset(struct mlx5_core_dev *dev)
mlx5_core_err(dev, "start\n");
- if (check_fatal_sensors(dev) == MLX5_SENSOR_FW_SYND_RFR) {
+ if (mlx5_health_check_fatal_sensors(dev) == MLX5_SENSOR_FW_SYND_RFR) {
/* Get cr-dump and reset FW semaphore */
lock = lock_sem_sw_reset(dev, true);
@@ -308,26 +308,31 @@ static void mlx5_handle_bad_state(struct mlx5_core_dev *dev)
/* How much time to wait until health resetting the driver (in msecs) */
#define MLX5_RECOVERY_WAIT_MSECS 60000
-static int mlx5_health_try_recover(struct mlx5_core_dev *dev)
+int mlx5_health_wait_pci_up(struct mlx5_core_dev *dev)
{
unsigned long end;
- mlx5_core_warn(dev, "handling bad device here\n");
- mlx5_handle_bad_state(dev);
end = jiffies + msecs_to_jiffies(MLX5_RECOVERY_WAIT_MSECS);
while (sensor_pci_not_working(dev)) {
- if (time_after(jiffies, end)) {
- mlx5_core_err(dev,
- "health recovery flow aborted, PCI reads still not working\n");
- return -EIO;
- }
+ if (time_after(jiffies, end))
+ return -ETIMEDOUT;
msleep(100);
}
+ return 0;
+}
+static int mlx5_health_try_recover(struct mlx5_core_dev *dev)
+{
+ mlx5_core_warn(dev, "handling bad device here\n");
+ mlx5_handle_bad_state(dev);
+ if (mlx5_health_wait_pci_up(dev)) {
+ mlx5_core_err(dev, "health recovery flow aborted, PCI reads still not working\n");
+ return -EIO;
+ }
mlx5_core_err(dev, "starting health recovery flow\n");
mlx5_recover_device(dev);
if (!test_bit(MLX5_INTERFACE_STATE_UP, &dev->intf_state) ||
- check_fatal_sensors(dev)) {
+ mlx5_health_check_fatal_sensors(dev)) {
mlx5_core_err(dev, "health recovery failed\n");
return -EIO;
}
@@ -696,7 +701,7 @@ static void poll_health(struct timer_list *t)
if (dev->state == MLX5_DEVICE_STATE_INTERNAL_ERROR)
goto out;
- fatal_error = check_fatal_sensors(dev);
+ fatal_error = mlx5_health_check_fatal_sensors(dev);
if (fatal_error && !health->fatal_error) {
mlx5_core_err(dev, "Fatal error %u detected\n", fatal_error);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 871d28b09f8a..c22c96f26ab5 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -57,6 +57,7 @@
#include "lib/mpfs.h"
#include "eswitch.h"
#include "devlink.h"
+#include "fw_reset.h"
#include "lib/mlx5.h"
#include "fpga/core.h"
#include "fpga/ipsec.h"
@@ -835,6 +836,12 @@ static int mlx5_init_once(struct mlx5_core_dev *dev)
goto err_eq_cleanup;
}
+ err = mlx5_fw_reset_init(dev);
+ if (err) {
+ mlx5_core_err(dev, "failed to initialize fw reset events\n");
+ goto err_events_cleanup;
+ }
+
mlx5_cq_debugfs_init(dev);
mlx5_init_reserved_gids(dev);
@@ -896,6 +903,8 @@ static int mlx5_init_once(struct mlx5_core_dev *dev)
mlx5_geneve_destroy(dev->geneve);
mlx5_vxlan_destroy(dev->vxlan);
mlx5_cq_debugfs_cleanup(dev);
+ mlx5_fw_reset_cleanup(dev);
+err_events_cleanup:
mlx5_events_cleanup(dev);
err_eq_cleanup:
mlx5_eq_table_cleanup(dev);
@@ -923,6 +932,7 @@ static void mlx5_cleanup_once(struct mlx5_core_dev *dev)
mlx5_cleanup_clock(dev);
mlx5_cleanup_reserved_gids(dev);
mlx5_cq_debugfs_cleanup(dev);
+ mlx5_fw_reset_cleanup(dev);
mlx5_events_cleanup(dev);
mlx5_eq_table_cleanup(dev);
mlx5_irq_table_cleanup(dev);
@@ -1081,6 +1091,7 @@ static int mlx5_load(struct mlx5_core_dev *dev)
goto err_fw_tracer;
}
+ mlx5_fw_reset_events_start(dev);
mlx5_hv_vhca_init(dev->hv_vhca);
err = mlx5_rsc_dump_init(dev);
@@ -1142,6 +1153,7 @@ static int mlx5_load(struct mlx5_core_dev *dev)
mlx5_rsc_dump_cleanup(dev);
err_rsc_dump:
mlx5_hv_vhca_cleanup(dev->hv_vhca);
+ mlx5_fw_reset_events_stop(dev);
mlx5_fw_tracer_cleanup(dev->tracer);
err_fw_tracer:
mlx5_eq_table_destroy(dev);
@@ -1164,6 +1176,7 @@ static void mlx5_unload(struct mlx5_core_dev *dev)
mlx5_fpga_device_stop(dev);
mlx5_rsc_dump_cleanup(dev);
mlx5_hv_vhca_cleanup(dev->hv_vhca);
+ mlx5_fw_reset_events_stop(dev);
mlx5_fw_tracer_cleanup(dev->tracer);
mlx5_eq_table_destroy(dev);
mlx5_irq_table_destroy(dev);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
index fc1649dac11b..d07a32165792 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/mlx5_core.h
@@ -123,6 +123,8 @@ int mlx5_cmd_force_teardown_hca(struct mlx5_core_dev *dev);
int mlx5_cmd_fast_teardown_hca(struct mlx5_core_dev *dev);
void mlx5_enter_error_state(struct mlx5_core_dev *dev, bool force);
void mlx5_error_sw_reset(struct mlx5_core_dev *dev);
+u32 mlx5_health_check_fatal_sensors(struct mlx5_core_dev *dev);
+int mlx5_health_wait_pci_up(struct mlx5_core_dev *dev);
void mlx5_disable_device(struct mlx5_core_dev *dev);
void mlx5_recover_device(struct mlx5_core_dev *dev);
int mlx5_sriov_init(struct mlx5_core_dev *dev);
diff --git a/include/linux/mlx5/driver.h b/include/linux/mlx5/driver.h
index 8dc3da6e6480..e1f192a63df3 100644
--- a/include/linux/mlx5/driver.h
+++ b/include/linux/mlx5/driver.h
@@ -501,6 +501,7 @@ struct mlx5_mpfs;
struct mlx5_eswitch;
struct mlx5_lag;
struct mlx5_devcom;
+struct mlx5_fw_reset;
struct mlx5_eq_table;
struct mlx5_irq_table;
@@ -578,6 +579,7 @@ struct mlx5_priv {
struct mlx5_core_sriov sriov;
struct mlx5_lag *lag;
struct mlx5_devcom *devcom;
+ struct mlx5_fw_reset *fw_reset;
struct mlx5_core_roce roce;
struct mlx5_fc_stats fc_stats;
struct mlx5_rl_table rl_table;
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH net-next 09/16] net/mlx5: Handle sync reset now event
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (7 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 08/16] net/mlx5: Handle sync reset request event Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 10/16] net/mlx5: Handle sync reset abort event Moshe Shemesh
` (6 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
On sync_reset_now event the driver does reload and PCI link toggle to
activate firmware upgrade reset. When the firmware sends this event it
syncs the event on all PFs, so all PFs will do PCI link toggle at once.
To do PCI link toggle, the driver ensures that no other device ID under
the same bridge by checking that all the PF functions under the same PCI
bridge have same device ID. If no other device it uses PCI bridge link
control to turn link down and up.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
---
.../ethernet/mellanox/mlx5/core/fw_reset.c | 119 ++++++++++++++++++
1 file changed, 119 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
index cd1b4e1b56ba..26caf65e9f5f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
@@ -13,6 +13,7 @@ struct mlx5_fw_reset {
struct workqueue_struct *wq;
struct work_struct reset_request_work;
struct work_struct reset_reload_work;
+ struct work_struct reset_now_work;
unsigned long reset_flags;
struct timer_list timer;
};
@@ -156,6 +157,120 @@ static void mlx5_sync_reset_request_event(struct work_struct *work)
mlx5_core_warn(dev, "PCI Sync FW Update Reset Ack. Device reset is expected.\n");
}
+#define MLX5_PCI_LINK_UP_TIMEOUT 2000
+
+static int mlx5_pci_link_toggle(struct mlx5_core_dev *dev)
+{
+ struct pci_bus *bridge_bus = dev->pdev->bus;
+ struct pci_dev *bridge = bridge_bus->self;
+ u16 reg16, dev_id, sdev_id;
+ unsigned long timeout;
+ struct pci_dev *sdev;
+ int cap, err;
+ u32 reg32;
+
+ /* Check that all functions under the pci bridge are PFs of
+ * this device otherwise fail this function.
+ */
+ err = pci_read_config_word(dev->pdev, PCI_DEVICE_ID, &dev_id);
+ if (err)
+ return err;
+ list_for_each_entry(sdev, &bridge_bus->devices, bus_list) {
+ err = pci_read_config_word(sdev, PCI_DEVICE_ID, &sdev_id);
+ if (err)
+ return err;
+ if (sdev_id != dev_id)
+ return -EPERM;
+ }
+
+ cap = pci_find_capability(bridge, PCI_CAP_ID_EXP);
+ if (!cap)
+ return -EOPNOTSUPP;
+
+ list_for_each_entry(sdev, &bridge_bus->devices, bus_list) {
+ pci_save_state(sdev);
+ pci_cfg_access_lock(sdev);
+ }
+ /* PCI link toggle */
+ err = pci_read_config_word(bridge, cap + PCI_EXP_LNKCTL, ®16);
+ if (err)
+ return err;
+ reg16 |= PCI_EXP_LNKCTL_LD;
+ err = pci_write_config_word(bridge, cap + PCI_EXP_LNKCTL, reg16);
+ if (err)
+ return err;
+ msleep(500);
+ reg16 &= ~PCI_EXP_LNKCTL_LD;
+ err = pci_write_config_word(bridge, cap + PCI_EXP_LNKCTL, reg16);
+ if (err)
+ return err;
+
+ /* Check link */
+ err = pci_read_config_dword(bridge, cap + PCI_EXP_LNKCAP, ®32);
+ if (err)
+ return err;
+ if (!(reg32 & PCI_EXP_LNKCAP_DLLLARC)) {
+ mlx5_core_warn(dev, "No PCI link reporting capability (0x%08x)\n", reg32);
+ msleep(1000);
+ goto restore;
+ }
+
+ timeout = jiffies + msecs_to_jiffies(MLX5_PCI_LINK_UP_TIMEOUT);
+ do {
+ err = pci_read_config_word(bridge, cap + PCI_EXP_LNKSTA, ®16);
+ if (err)
+ return err;
+ if (reg16 & PCI_EXP_LNKSTA_DLLLA)
+ break;
+ msleep(20);
+ } while (!time_after(jiffies, timeout));
+
+ if (reg16 & PCI_EXP_LNKSTA_DLLLA) {
+ mlx5_core_info(dev, "PCI Link up\n");
+ } else {
+ mlx5_core_err(dev, "PCI link not ready (0x%04x) after %d ms\n",
+ reg16, MLX5_PCI_LINK_UP_TIMEOUT);
+ err = -ETIMEDOUT;
+ }
+
+restore:
+ list_for_each_entry(sdev, &bridge_bus->devices, bus_list) {
+ pci_cfg_access_unlock(sdev);
+ pci_restore_state(sdev);
+ }
+
+ return err;
+}
+
+static void mlx5_sync_reset_now_event(struct work_struct *work)
+{
+ struct mlx5_fw_reset *fw_reset = container_of(work, struct mlx5_fw_reset,
+ reset_now_work);
+ struct mlx5_core_dev *dev = fw_reset->dev;
+ int err;
+
+ mlx5_sync_reset_clear_reset_requested(dev, false);
+
+ mlx5_core_warn(dev, "Sync Reset now. Device is going to reset.\n");
+
+ err = mlx5_cmd_fast_teardown_hca(dev);
+ if (err) {
+ mlx5_core_warn(dev, "Fast teardown failed, no reset done, err %d\n", err);
+ goto done;
+ }
+
+ err = mlx5_pci_link_toggle(dev);
+ if (err) {
+ mlx5_core_warn(dev, "mlx5_pci_link_toggle failed, no reset done, err %d\n", err);
+ goto done;
+ }
+
+ mlx5_enter_error_state(dev, true);
+ mlx5_unload_one(dev, false);
+done:
+ mlx5_load_one(dev, false);
+}
+
static void mlx5_sync_reset_events_handle(struct mlx5_fw_reset *fw_reset, struct mlx5_eqe *eqe)
{
struct mlx5_eqe_sync_fw_update *sync_fw_update_eqe;
@@ -167,6 +282,9 @@ static void mlx5_sync_reset_events_handle(struct mlx5_fw_reset *fw_reset, struct
case MLX5_SYNC_RST_STATE_RESET_REQUEST:
queue_work(fw_reset->wq, &fw_reset->reset_request_work);
break;
+ case MLX5_SYNC_RST_STATE_RESET_NOW:
+ queue_work(fw_reset->wq, &fw_reset->reset_now_work);
+ break;
}
}
@@ -216,6 +334,7 @@ int mlx5_fw_reset_init(struct mlx5_core_dev *dev)
INIT_WORK(&fw_reset->reset_request_work, mlx5_sync_reset_request_event);
INIT_WORK(&fw_reset->reset_reload_work, mlx5_sync_reset_reload_work);
+ INIT_WORK(&fw_reset->reset_now_work, mlx5_sync_reset_now_event);
return 0;
}
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH net-next 10/16] net/mlx5: Handle sync reset abort event
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (8 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 09/16] net/mlx5: Handle sync reset now event Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 11/16] net/mlx5: Add support for devlink reload action fw activate Moshe Shemesh
` (5 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
If firmware sends sync_reset_abort to driver the driver should clear the
reset requested mode as reset is not expected any more.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
---
.../net/ethernet/mellanox/mlx5/core/fw_reset.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
index 26caf65e9f5f..56ae72e016f1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
@@ -14,6 +14,7 @@ struct mlx5_fw_reset {
struct work_struct reset_request_work;
struct work_struct reset_reload_work;
struct work_struct reset_now_work;
+ struct work_struct reset_abort_work;
unsigned long reset_flags;
struct timer_list timer;
};
@@ -271,6 +272,16 @@ static void mlx5_sync_reset_now_event(struct work_struct *work)
mlx5_load_one(dev, false);
}
+static void mlx5_sync_reset_abort_event(struct work_struct *work)
+{
+ struct mlx5_fw_reset *fw_reset = container_of(work, struct mlx5_fw_reset,
+ reset_abort_work);
+ struct mlx5_core_dev *dev = fw_reset->dev;
+
+ mlx5_sync_reset_clear_reset_requested(dev, true);
+ mlx5_core_warn(dev, "PCI Sync FW Update Reset Aborted.\n");
+}
+
static void mlx5_sync_reset_events_handle(struct mlx5_fw_reset *fw_reset, struct mlx5_eqe *eqe)
{
struct mlx5_eqe_sync_fw_update *sync_fw_update_eqe;
@@ -285,6 +296,9 @@ static void mlx5_sync_reset_events_handle(struct mlx5_fw_reset *fw_reset, struct
case MLX5_SYNC_RST_STATE_RESET_NOW:
queue_work(fw_reset->wq, &fw_reset->reset_now_work);
break;
+ case MLX5_SYNC_RST_STATE_RESET_ABORT:
+ queue_work(fw_reset->wq, &fw_reset->reset_abort_work);
+ break;
}
}
@@ -335,6 +349,7 @@ int mlx5_fw_reset_init(struct mlx5_core_dev *dev)
INIT_WORK(&fw_reset->reset_request_work, mlx5_sync_reset_request_event);
INIT_WORK(&fw_reset->reset_reload_work, mlx5_sync_reset_reload_work);
INIT_WORK(&fw_reset->reset_now_work, mlx5_sync_reset_now_event);
+ INIT_WORK(&fw_reset->reset_abort_work, mlx5_sync_reset_abort_event);
return 0;
}
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH net-next 11/16] net/mlx5: Add support for devlink reload action fw activate
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (9 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 10/16] net/mlx5: Handle sync reset abort event Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 12/16] devlink: Add enable_remote_dev_reset generic parameter Moshe Shemesh
` (4 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Add support for devlink reload action fw_activate. To activate firmware
image the mlx5 driver resets the firmware and reloads it from flash. If
a new image was stored on flash it will be loaded. Once this reload
command is executed the driver initiates fw sync reset flow, where the
firmware synchronizes all PFs on coming reset and driver reload.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
---
RFCv4 -> RFCv5:
- Add remote actions stats
- If devlink reload is not supported, show only remote_stats
RFCv3 -> RFCv4:
- Renamed DEVLINK_ATTR_RELOAD_ACTION_CNT to
DEVLINK_ATTR_RELOAD_ACTION_STAT
- Add stats per action per limit level
RFCv2 -> RFCv3:
- Add reload actions counters instead of supported reload actions
(reload actions counters are only for supported action so no need for
both)
RFCv1 -> RFCv2:
- Removed DEVLINK_ATTR_RELOAD_DEFAULT_LEVEL
- Removed DEVLINK_ATTR_RELOAD_LEVELS_INFO
- Have actions instead of levels
---
.../net/ethernet/mellanox/mlx5/core/devlink.c | 59 +++++++++++++++++--
.../ethernet/mellanox/mlx5/core/fw_reset.c | 59 +++++++++++++++++--
.../ethernet/mellanox/mlx5/core/fw_reset.h | 1 +
3 files changed, 108 insertions(+), 11 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 47f3dbc80c7c..13153454cc05 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -4,6 +4,7 @@
#include <devlink.h>
#include "mlx5_core.h"
+#include "fw_reset.h"
#include "fs_core.h"
#include "eswitch.h"
@@ -84,6 +85,32 @@ mlx5_devlink_info_get(struct devlink *devlink, struct devlink_info_req *req,
return 0;
}
+static int mlx5_devlink_reload_fw_activate(struct devlink *devlink, struct netlink_ext_ack *extack)
+{
+ struct mlx5_core_dev *dev = devlink_priv(devlink);
+ u8 reset_level, reset_type, net_port_alive;
+ int err;
+
+ err = mlx5_fw_reset_query(dev, &reset_level, &reset_type);
+ if (err)
+ return err;
+ if (!(reset_level & MLX5_MFRL_REG_RESET_LEVEL3)) {
+ NL_SET_ERR_MSG_MOD(extack, "FW activate requires reboot");
+ return -EINVAL;
+ }
+
+ net_port_alive = !!(reset_type & MLX5_MFRL_REG_RESET_TYPE_NET_PORT_ALIVE);
+ err = mlx5_fw_reset_set_reset_sync(dev, net_port_alive);
+ if (err)
+ goto out;
+
+ err = mlx5_fw_reset_wait_reset_done(dev);
+out:
+ if (err)
+ NL_SET_ERR_MSG_MOD(extack, "FW activate command failed");
+ return err;
+}
+
static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
enum devlink_reload_action action,
enum devlink_reload_limit limit,
@@ -91,8 +118,17 @@ static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
{
struct mlx5_core_dev *dev = devlink_priv(devlink);
- mlx5_unload_one(dev, false);
- return 0;
+ switch (action) {
+ case DEVLINK_RELOAD_ACTION_DRIVER_REINIT:
+ mlx5_unload_one(dev, false);
+ return 0;
+ case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
+ return mlx5_devlink_reload_fw_activate(devlink, extack);
+ default:
+ /* Unsupported action should not get to this function */
+ WARN_ON(1);
+ return -EOPNOTSUPP;
+ }
}
static int mlx5_devlink_reload_up(struct devlink *devlink, enum devlink_reload_action action,
@@ -101,8 +137,20 @@ static int mlx5_devlink_reload_up(struct devlink *devlink, enum devlink_reload_a
{
struct mlx5_core_dev *dev = devlink_priv(devlink);
- *actions_performed = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT);
- return mlx5_load_one(dev, false);
+ *actions_performed = BIT(action);
+ switch (action) {
+ case DEVLINK_RELOAD_ACTION_DRIVER_REINIT:
+ case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
+ /* On fw_activate action, also driver is reloaded and reinit performed */
+ *actions_performed |= BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT);
+ return mlx5_load_one(dev, false);
+ default:
+ /* Unsupported action should not get to this function */
+ WARN_ON(1);
+ return -EOPNOTSUPP;
+ }
+
+ return 0;
}
static const struct devlink_ops mlx5_devlink_ops = {
@@ -118,7 +166,8 @@ static const struct devlink_ops mlx5_devlink_ops = {
#endif
.flash_update = mlx5_devlink_flash_update,
.info_get = mlx5_devlink_info_get,
- .reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT),
+ .reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT) |
+ BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE),
.reload_down = mlx5_devlink_reload_down,
.reload_up = mlx5_devlink_reload_up,
};
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
index 56ae72e016f1..f5ffb6fc55c3 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
@@ -5,6 +5,7 @@
enum {
MLX5_FW_RESET_FLAGS_RESET_REQUESTED,
+ MLX5_FW_RESET_FLAGS_PENDING_COMP
};
struct mlx5_fw_reset {
@@ -17,6 +18,8 @@ struct mlx5_fw_reset {
struct work_struct reset_abort_work;
unsigned long reset_flags;
struct timer_list timer;
+ struct completion done;
+ int ret;
};
static int mlx5_reg_mfrl_set(struct mlx5_core_dev *dev, u8 reset_level,
@@ -58,7 +61,14 @@ int mlx5_fw_reset_query(struct mlx5_core_dev *dev, u8 *reset_level, u8 *reset_ty
int mlx5_fw_reset_set_reset_sync(struct mlx5_core_dev *dev, u8 reset_type_sel)
{
- return mlx5_reg_mfrl_set(dev, MLX5_MFRL_REG_RESET_LEVEL3, reset_type_sel, 0, true);
+ struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
+ int err;
+
+ set_bit(MLX5_FW_RESET_FLAGS_PENDING_COMP, &fw_reset->reset_flags);
+ err = mlx5_reg_mfrl_set(dev, MLX5_MFRL_REG_RESET_LEVEL3, reset_type_sel, 0, true);
+ if (err)
+ clear_bit(MLX5_FW_RESET_FLAGS_PENDING_COMP, &fw_reset->reset_flags);
+ return err;
}
int mlx5_fw_reset_set_live_patch(struct mlx5_core_dev *dev)
@@ -66,19 +76,35 @@ int mlx5_fw_reset_set_live_patch(struct mlx5_core_dev *dev)
return mlx5_reg_mfrl_set(dev, MLX5_MFRL_REG_RESET_LEVEL0, 0, 0, false);
}
+static void mlx5_fw_reset_complete_reload(struct mlx5_core_dev *dev)
+{
+ struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
+
+ /* if this is the driver that initiated the fw reset, devlink completed the reload */
+ if (test_bit(MLX5_FW_RESET_FLAGS_PENDING_COMP, &fw_reset->reset_flags)) {
+ complete(&fw_reset->done);
+ } else {
+ mlx5_load_one(dev, false);
+ devlink_remote_reload_actions_performed(priv_to_devlink(dev), 0,
+ BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT) |
+ BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE));
+ }
+}
+
static void mlx5_sync_reset_reload_work(struct work_struct *work)
{
struct mlx5_fw_reset *fw_reset = container_of(work, struct mlx5_fw_reset,
reset_reload_work);
struct mlx5_core_dev *dev = fw_reset->dev;
+ int err;
mlx5_enter_error_state(dev, true);
mlx5_unload_one(dev, false);
- if (mlx5_health_wait_pci_up(dev)) {
+ err = mlx5_health_wait_pci_up(dev);
+ if (err)
mlx5_core_err(dev, "reset reload flow aborted, PCI reads still not working\n");
- return;
- }
- mlx5_load_one(dev, false);
+ fw_reset->ret = err;
+ mlx5_fw_reset_complete_reload(dev);
}
static void mlx5_stop_sync_reset_poll(struct mlx5_core_dev *dev)
@@ -269,7 +295,8 @@ static void mlx5_sync_reset_now_event(struct work_struct *work)
mlx5_enter_error_state(dev, true);
mlx5_unload_one(dev, false);
done:
- mlx5_load_one(dev, false);
+ fw_reset->ret = err;
+ mlx5_fw_reset_complete_reload(dev);
}
static void mlx5_sync_reset_abort_event(struct work_struct *work)
@@ -318,6 +345,25 @@ static int fw_reset_event_notifier(struct notifier_block *nb, unsigned long acti
return NOTIFY_OK;
}
+#define MLX5_FW_RESET_TIMEOUT_MSEC 5000
+int mlx5_fw_reset_wait_reset_done(struct mlx5_core_dev *dev)
+{
+ unsigned long timeout = msecs_to_jiffies(MLX5_FW_RESET_TIMEOUT_MSEC);
+ struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
+ int err;
+
+ if (!wait_for_completion_timeout(&fw_reset->done, timeout)) {
+ mlx5_core_warn(dev, "FW sync reset timeout after %d seconds\n",
+ MLX5_FW_RESET_TIMEOUT_MSEC / 1000);
+ err = -ETIMEDOUT;
+ goto out;
+ }
+ err = fw_reset->ret;
+out:
+ clear_bit(MLX5_FW_RESET_FLAGS_PENDING_COMP, &fw_reset->reset_flags);
+ return err;
+}
+
void mlx5_fw_reset_events_start(struct mlx5_core_dev *dev)
{
struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
@@ -351,6 +397,7 @@ int mlx5_fw_reset_init(struct mlx5_core_dev *dev)
INIT_WORK(&fw_reset->reset_now_work, mlx5_sync_reset_now_event);
INIT_WORK(&fw_reset->reset_abort_work, mlx5_sync_reset_abort_event);
+ init_completion(&fw_reset->done);
return 0;
}
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
index a231f7848a8f..e7937447ce1d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
@@ -10,6 +10,7 @@ int mlx5_fw_reset_query(struct mlx5_core_dev *dev, u8 *reset_level, u8 *reset_ty
int mlx5_fw_reset_set_reset_sync(struct mlx5_core_dev *dev, u8 reset_type_sel);
int mlx5_fw_reset_set_live_patch(struct mlx5_core_dev *dev);
+int mlx5_fw_reset_wait_reset_done(struct mlx5_core_dev *dev);
void mlx5_fw_reset_events_start(struct mlx5_core_dev *dev);
void mlx5_fw_reset_events_stop(struct mlx5_core_dev *dev);
int mlx5_fw_reset_init(struct mlx5_core_dev *dev);
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH net-next 12/16] devlink: Add enable_remote_dev_reset generic parameter
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (10 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 11/16] net/mlx5: Add support for devlink reload action fw activate Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 13/16] net/mlx5: Add devlink param enable_remote_dev_reset support Moshe Shemesh
` (3 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
The enable_remote_dev_reset devlink param flags that the host admin
allows device resets that can be initiated by other hosts. This
parameter is useful for setups where a device is shared by different
hosts, such as multi-host setup. Once the user set this parameter to
false, the driver should NACK any attempt to reset the device while the
driver is loaded.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Jiri Pirko <jiri@nvidia.com>
---
Documentation/networking/devlink/devlink-params.rst | 6 ++++++
include/net/devlink.h | 4 ++++
net/core/devlink.c | 5 +++++
3 files changed, 15 insertions(+)
diff --git a/Documentation/networking/devlink/devlink-params.rst b/Documentation/networking/devlink/devlink-params.rst
index d075fd090b3d..54c9f107c4b0 100644
--- a/Documentation/networking/devlink/devlink-params.rst
+++ b/Documentation/networking/devlink/devlink-params.rst
@@ -108,3 +108,9 @@ own name.
* - ``region_snapshot_enable``
- Boolean
- Enable capture of ``devlink-region`` snapshots.
+ * - ``enable_remote_dev_reset``
+ - Boolean
+ - Enable device reset by remote host. When cleared, the device driver
+ will NACK any attempt of other host to reset the device. This parameter
+ is useful for setups where a device is shared by different hosts, such
+ as multi-host setup.
diff --git a/include/net/devlink.h b/include/net/devlink.h
index a4ccb83bbd2c..0a1c88805627 100644
--- a/include/net/devlink.h
+++ b/include/net/devlink.h
@@ -464,6 +464,7 @@ enum devlink_param_generic_id {
DEVLINK_PARAM_GENERIC_ID_FW_LOAD_POLICY,
DEVLINK_PARAM_GENERIC_ID_RESET_DEV_ON_DRV_PROBE,
DEVLINK_PARAM_GENERIC_ID_ENABLE_ROCE,
+ DEVLINK_PARAM_GENERIC_ID_ENABLE_REMOTE_DEV_RESET,
/* add new param generic ids above here*/
__DEVLINK_PARAM_GENERIC_ID_MAX,
@@ -501,6 +502,9 @@ enum devlink_param_generic_id {
#define DEVLINK_PARAM_GENERIC_ENABLE_ROCE_NAME "enable_roce"
#define DEVLINK_PARAM_GENERIC_ENABLE_ROCE_TYPE DEVLINK_PARAM_TYPE_BOOL
+#define DEVLINK_PARAM_GENERIC_ENABLE_REMOTE_DEV_RESET_NAME "enable_remote_dev_reset"
+#define DEVLINK_PARAM_GENERIC_ENABLE_REMOTE_DEV_RESET_TYPE DEVLINK_PARAM_TYPE_BOOL
+
#define DEVLINK_PARAM_GENERIC(_id, _cmodes, _get, _set, _validate) \
{ \
.id = DEVLINK_PARAM_GENERIC_ID_##_id, \
diff --git a/net/core/devlink.c b/net/core/devlink.c
index 3b6bd3b4d346..4f074c81a5e4 100644
--- a/net/core/devlink.c
+++ b/net/core/devlink.c
@@ -3484,6 +3484,11 @@ static const struct devlink_param devlink_param_generic[] = {
.name = DEVLINK_PARAM_GENERIC_ENABLE_ROCE_NAME,
.type = DEVLINK_PARAM_GENERIC_ENABLE_ROCE_TYPE,
},
+ {
+ .id = DEVLINK_PARAM_GENERIC_ID_ENABLE_REMOTE_DEV_RESET,
+ .name = DEVLINK_PARAM_GENERIC_ENABLE_REMOTE_DEV_RESET_NAME,
+ .type = DEVLINK_PARAM_GENERIC_ENABLE_REMOTE_DEV_RESET_TYPE,
+ },
};
static int devlink_param_generic_verify(const struct devlink_param *param)
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH net-next 13/16] net/mlx5: Add devlink param enable_remote_dev_reset support
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (11 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 12/16] devlink: Add enable_remote_dev_reset generic parameter Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 14/16] net/mlx5: Add support for fw live patch event Moshe Shemesh
` (2 subsequent siblings)
15 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
The enable_remote_dev_reset devlink param flags that the host admin
allows resets by other hosts. In case it is cleared mlx5 host PF driver
will send NACK on pci sync for firmware update reset request and the
command will fail.
By default enable_remote_dev_reset parameter is true, so pci sync for
firmware update reset is enabled.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
---
RFCv1 -> RFCv2:
- Have MLX5_FW_RESET_FLAGS_NACK_RESET_REQUEST instead of
MLX5_HEALTH_RESET_FLAGS_NACK_RESET_REQUEST
---
.../net/ethernet/mellanox/mlx5/core/devlink.c | 21 ++++++++++++++
.../ethernet/mellanox/mlx5/core/fw_reset.c | 29 +++++++++++++++++++
.../ethernet/mellanox/mlx5/core/fw_reset.h | 2 ++
3 files changed, 52 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 13153454cc05..7b304227ad57 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -278,6 +278,24 @@ static int mlx5_devlink_large_group_num_validate(struct devlink *devlink, u32 id
}
#endif
+static int mlx5_devlink_enable_remote_dev_reset_set(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct mlx5_core_dev *dev = devlink_priv(devlink);
+
+ mlx5_fw_reset_enable_remote_dev_reset_set(dev, ctx->val.vbool);
+ return 0;
+}
+
+static int mlx5_devlink_enable_remote_dev_reset_get(struct devlink *devlink, u32 id,
+ struct devlink_param_gset_ctx *ctx)
+{
+ struct mlx5_core_dev *dev = devlink_priv(devlink);
+
+ ctx->val.vbool = mlx5_fw_reset_enable_remote_dev_reset_get(dev);
+ return 0;
+}
+
static const struct devlink_param mlx5_devlink_params[] = {
DEVLINK_PARAM_DRIVER(MLX5_DEVLINK_PARAM_ID_FLOW_STEERING_MODE,
"flow_steering_mode", DEVLINK_PARAM_TYPE_STRING,
@@ -293,6 +311,9 @@ static const struct devlink_param mlx5_devlink_params[] = {
NULL, NULL,
mlx5_devlink_large_group_num_validate),
#endif
+ DEVLINK_PARAM_GENERIC(ENABLE_REMOTE_DEV_RESET, BIT(DEVLINK_PARAM_CMODE_RUNTIME),
+ mlx5_devlink_enable_remote_dev_reset_get,
+ mlx5_devlink_enable_remote_dev_reset_set, NULL),
};
static void mlx5_devlink_set_params_init_values(struct devlink *devlink)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
index f5ffb6fc55c3..b2aaff8d4fcd 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
@@ -5,6 +5,7 @@
enum {
MLX5_FW_RESET_FLAGS_RESET_REQUESTED,
+ MLX5_FW_RESET_FLAGS_NACK_RESET_REQUEST,
MLX5_FW_RESET_FLAGS_PENDING_COMP
};
@@ -22,6 +23,23 @@ struct mlx5_fw_reset {
int ret;
};
+void mlx5_fw_reset_enable_remote_dev_reset_set(struct mlx5_core_dev *dev, bool enable)
+{
+ struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
+
+ if (enable)
+ clear_bit(MLX5_FW_RESET_FLAGS_NACK_RESET_REQUEST, &fw_reset->reset_flags);
+ else
+ set_bit(MLX5_FW_RESET_FLAGS_NACK_RESET_REQUEST, &fw_reset->reset_flags);
+}
+
+bool mlx5_fw_reset_enable_remote_dev_reset_get(struct mlx5_core_dev *dev)
+{
+ struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
+
+ return !test_bit(MLX5_FW_RESET_FLAGS_NACK_RESET_REQUEST, &fw_reset->reset_flags);
+}
+
static int mlx5_reg_mfrl_set(struct mlx5_core_dev *dev, u8 reset_level,
u8 reset_type_sel, u8 sync_resp, bool sync_start)
{
@@ -160,6 +178,11 @@ static int mlx5_fw_reset_set_reset_sync_ack(struct mlx5_core_dev *dev)
return mlx5_reg_mfrl_set(dev, MLX5_MFRL_REG_RESET_LEVEL3, 0, 1, false);
}
+static int mlx5_fw_reset_set_reset_sync_nack(struct mlx5_core_dev *dev)
+{
+ return mlx5_reg_mfrl_set(dev, MLX5_MFRL_REG_RESET_LEVEL3, 0, 2, false);
+}
+
static void mlx5_sync_reset_set_reset_requested(struct mlx5_core_dev *dev)
{
struct mlx5_fw_reset *fw_reset = dev->priv.fw_reset;
@@ -176,6 +199,12 @@ static void mlx5_sync_reset_request_event(struct work_struct *work)
struct mlx5_core_dev *dev = fw_reset->dev;
int err;
+ if (test_bit(MLX5_FW_RESET_FLAGS_NACK_RESET_REQUEST, &fw_reset->reset_flags)) {
+ err = mlx5_fw_reset_set_reset_sync_nack(dev);
+ mlx5_core_warn(dev, "PCI Sync FW Update Reset Nack %s",
+ err ? "Failed" : "Sent");
+ return;
+ }
mlx5_sync_reset_set_reset_requested(dev);
err = mlx5_fw_reset_set_reset_sync_ack(dev);
if (err)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
index e7937447ce1d..7761ee5fc7d0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.h
@@ -6,6 +6,8 @@
#include "mlx5_core.h"
+void mlx5_fw_reset_enable_remote_dev_reset_set(struct mlx5_core_dev *dev, bool enable);
+bool mlx5_fw_reset_enable_remote_dev_reset_get(struct mlx5_core_dev *dev);
int mlx5_fw_reset_query(struct mlx5_core_dev *dev, u8 *reset_level, u8 *reset_type);
int mlx5_fw_reset_set_reset_sync(struct mlx5_core_dev *dev, u8 reset_type_sel);
int mlx5_fw_reset_set_live_patch(struct mlx5_core_dev *dev);
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH net-next 14/16] net/mlx5: Add support for fw live patch event
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (12 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 13/16] net/mlx5: Add devlink param enable_remote_dev_reset support Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 15/16] net/mlx5: Add support for devlink reload limit no reset Moshe Shemesh
2020-10-01 13:59 ` [PATCH net-next 16/16] devlink: Add Documentation/networking/devlink/devlink-reload.rst Moshe Shemesh
15 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Firmware live patch event notifies the driver that the firmware was just
updated using live patch. In such case the driver should not reload or
re-initiate entities, part to updating the firmware version and
re-initiate the firmware tracer which can be updated by live patch with
new strings database to help debugging an issue.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
---
.../mellanox/mlx5/core/diag/fw_tracer.c | 52 +++++++++++++++++++
.../mellanox/mlx5/core/diag/fw_tracer.h | 1 +
.../ethernet/mellanox/mlx5/core/fw_reset.c | 24 +++++++++
include/linux/mlx5/device.h | 1 +
4 files changed, 78 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
index ad3594c4afcb..a02fed9040d0 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.c
@@ -1064,6 +1064,58 @@ void mlx5_fw_tracer_destroy(struct mlx5_fw_tracer *tracer)
kvfree(tracer);
}
+static int mlx5_fw_tracer_recreate_strings_db(struct mlx5_fw_tracer *tracer)
+{
+ struct mlx5_core_dev *dev;
+ int err;
+
+ cancel_work_sync(&tracer->read_fw_strings_work);
+ mlx5_fw_tracer_clean_ready_list(tracer);
+ mlx5_fw_tracer_clean_print_hash(tracer);
+ mlx5_fw_tracer_clean_saved_traces_array(tracer);
+ mlx5_fw_tracer_free_strings_db(tracer);
+
+ dev = tracer->dev;
+ err = mlx5_query_mtrc_caps(tracer);
+ if (err) {
+ mlx5_core_dbg(dev, "FWTracer: Failed to query capabilities %d\n", err);
+ return err;
+ }
+
+ err = mlx5_fw_tracer_allocate_strings_db(tracer);
+ if (err) {
+ mlx5_core_warn(dev, "FWTracer: Allocate strings DB failed %d\n", err);
+ return err;
+ }
+ mlx5_fw_tracer_init_saved_traces_array(tracer);
+
+ return 0;
+}
+
+int mlx5_fw_tracer_reload(struct mlx5_fw_tracer *tracer)
+{
+ struct mlx5_core_dev *dev;
+ int err;
+
+ if (IS_ERR_OR_NULL(tracer))
+ return -EINVAL;
+
+ dev = tracer->dev;
+ mlx5_fw_tracer_cleanup(tracer);
+ err = mlx5_fw_tracer_recreate_strings_db(tracer);
+ if (err) {
+ mlx5_core_warn(dev, "Failed to recreate FW tracer strings DB\n");
+ return err;
+ }
+ err = mlx5_fw_tracer_init(tracer);
+ if (err) {
+ mlx5_core_warn(dev, "Failed to re-initialize FW tracer\n");
+ return err;
+ }
+
+ return 0;
+}
+
static int fw_tracer_event(struct notifier_block *nb, unsigned long action, void *data)
{
struct mlx5_fw_tracer *tracer = mlx5_nb_cof(nb, struct mlx5_fw_tracer, nb);
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.h b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.h
index 40601fba80ba..97252a85d65e 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/diag/fw_tracer.h
@@ -191,5 +191,6 @@ void mlx5_fw_tracer_destroy(struct mlx5_fw_tracer *tracer);
int mlx5_fw_tracer_trigger_core_dump_general(struct mlx5_core_dev *dev);
int mlx5_fw_tracer_get_saved_traces_objects(struct mlx5_fw_tracer *tracer,
struct devlink_fmsg *fmsg);
+int mlx5_fw_tracer_reload(struct mlx5_fw_tracer *tracer);
#endif
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
index b2aaff8d4fcd..f9042e147c7f 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/fw_reset.c
@@ -2,6 +2,7 @@
/* Copyright (c) 2020, Mellanox Technologies inc. All rights reserved. */
#include "fw_reset.h"
+#include "diag/fw_tracer.h"
enum {
MLX5_FW_RESET_FLAGS_RESET_REQUESTED,
@@ -13,6 +14,7 @@ struct mlx5_fw_reset {
struct mlx5_core_dev *dev;
struct mlx5_nb nb;
struct workqueue_struct *wq;
+ struct work_struct fw_live_patch_work;
struct work_struct reset_request_work;
struct work_struct reset_reload_work;
struct work_struct reset_now_work;
@@ -192,6 +194,24 @@ static void mlx5_sync_reset_set_reset_requested(struct mlx5_core_dev *dev)
mlx5_start_sync_reset_poll(dev);
}
+static void mlx5_fw_live_patch_event(struct work_struct *work)
+{
+ struct mlx5_fw_reset *fw_reset = container_of(work, struct mlx5_fw_reset,
+ fw_live_patch_work);
+ struct mlx5_core_dev *dev = fw_reset->dev;
+ struct mlx5_fw_tracer *tracer;
+
+ mlx5_core_info(dev, "Live patch updated firmware version: %d.%d.%d\n", fw_rev_maj(dev),
+ fw_rev_min(dev), fw_rev_sub(dev));
+
+ tracer = dev->tracer;
+ if (IS_ERR_OR_NULL(tracer))
+ return;
+
+ if (mlx5_fw_tracer_reload(tracer))
+ mlx5_core_err(dev, "Failed to reload FW tracer\n");
+}
+
static void mlx5_sync_reset_request_event(struct work_struct *work)
{
struct mlx5_fw_reset *fw_reset = container_of(work, struct mlx5_fw_reset,
@@ -364,6 +384,9 @@ static int fw_reset_event_notifier(struct notifier_block *nb, unsigned long acti
struct mlx5_eqe *eqe = data;
switch (eqe->sub_type) {
+ case MLX5_GENERAL_SUBTYPE_FW_LIVE_PATCH_EVENT:
+ queue_work(fw_reset->wq, &fw_reset->fw_live_patch_work);
+ break;
case MLX5_GENERAL_SUBTYPE_PCI_SYNC_FOR_FW_UPDATE_EVENT:
mlx5_sync_reset_events_handle(fw_reset, eqe);
break;
@@ -421,6 +444,7 @@ int mlx5_fw_reset_init(struct mlx5_core_dev *dev)
fw_reset->dev = dev;
dev->priv.fw_reset = fw_reset;
+ INIT_WORK(&fw_reset->fw_live_patch_work, mlx5_fw_live_patch_event);
INIT_WORK(&fw_reset->reset_request_work, mlx5_sync_reset_request_event);
INIT_WORK(&fw_reset->reset_reload_work, mlx5_sync_reset_reload_work);
INIT_WORK(&fw_reset->reset_now_work, mlx5_sync_reset_now_event);
diff --git a/include/linux/mlx5/device.h b/include/linux/mlx5/device.h
index 81ca5989009b..cf824366a7d1 100644
--- a/include/linux/mlx5/device.h
+++ b/include/linux/mlx5/device.h
@@ -366,6 +366,7 @@ enum {
enum {
MLX5_GENERAL_SUBTYPE_DELAY_DROP_TIMEOUT = 0x1,
MLX5_GENERAL_SUBTYPE_PCI_POWER_CHANGE_EVENT = 0x5,
+ MLX5_GENERAL_SUBTYPE_FW_LIVE_PATCH_EVENT = 0x7,
MLX5_GENERAL_SUBTYPE_PCI_SYNC_FOR_FW_UPDATE_EVENT = 0x8,
};
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* [PATCH net-next 15/16] net/mlx5: Add support for devlink reload limit no reset
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (13 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 14/16] net/mlx5: Add support for fw live patch event Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-01 21:52 ` Jakub Kicinski
2020-10-01 13:59 ` [PATCH net-next 16/16] devlink: Add Documentation/networking/devlink/devlink-reload.rst Moshe Shemesh
15 siblings, 1 reply; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Add support for devlink reload action fw_activate with reload limit
no_reset which does firmware live patching, updating the firmware image
without reset, no downtime and no configuration lose. The driver checks
if the firmware is capable of handling the pending firmware changes as a
live patch. If it is then it triggers firmware live patching flow.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
Reviewed-by: Saeed Mahameed <saeedm@nvidia.com>
---
RFCv5 -> v1:
- Renamed reload_action_limit_level to reload_limit
RFCv3 -> RFCv4:
- Have action fw_activate with limit level no_reset instead of action
fw_activate_no_reset
RFCv2 -> RFCv3:
- Replace fw_live_patch action by fw_activate_no_reset
RFCv1 -> RFCv2:
- Have fw_live_patch action instead of level
---
.../net/ethernet/mellanox/mlx5/core/devlink.c | 29 +++++++++++++++++++
1 file changed, 29 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
index 7b304227ad57..db59933a7fe1 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/devlink.c
@@ -111,6 +111,29 @@ static int mlx5_devlink_reload_fw_activate(struct devlink *devlink, struct netli
return err;
}
+static int mlx5_devlink_trigger_fw_live_patch(struct devlink *devlink,
+ struct netlink_ext_ack *extack)
+{
+ struct mlx5_core_dev *dev = devlink_priv(devlink);
+ u8 reset_level;
+ int err;
+
+ err = mlx5_fw_reset_query(dev, &reset_level, NULL);
+ if (err)
+ return err;
+ if (!(reset_level & MLX5_MFRL_REG_RESET_LEVEL0)) {
+ NL_SET_ERR_MSG_MOD(extack,
+ "FW upgrade to the stored FW can't be done by FW live patching");
+ return -EINVAL;
+ }
+
+ err = mlx5_fw_reset_set_live_patch(dev);
+ if (err)
+ return err;
+
+ return 0;
+}
+
static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
enum devlink_reload_action action,
enum devlink_reload_limit limit,
@@ -123,6 +146,8 @@ static int mlx5_devlink_reload_down(struct devlink *devlink, bool netns_change,
mlx5_unload_one(dev, false);
return 0;
case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
+ if (limit == DEVLINK_RELOAD_LIMIT_NO_RESET)
+ return mlx5_devlink_trigger_fw_live_patch(devlink, extack);
return mlx5_devlink_reload_fw_activate(devlink, extack);
default:
/* Unsupported action should not get to this function */
@@ -140,7 +165,10 @@ static int mlx5_devlink_reload_up(struct devlink *devlink, enum devlink_reload_a
*actions_performed = BIT(action);
switch (action) {
case DEVLINK_RELOAD_ACTION_DRIVER_REINIT:
+ return mlx5_load_one(dev, false);
case DEVLINK_RELOAD_ACTION_FW_ACTIVATE:
+ if (limit == DEVLINK_RELOAD_LIMIT_NO_RESET)
+ break;
/* On fw_activate action, also driver is reloaded and reinit performed */
*actions_performed |= BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT);
return mlx5_load_one(dev, false);
@@ -168,6 +196,7 @@ static const struct devlink_ops mlx5_devlink_ops = {
.info_get = mlx5_devlink_info_get,
.reload_actions = BIT(DEVLINK_RELOAD_ACTION_DRIVER_REINIT) |
BIT(DEVLINK_RELOAD_ACTION_FW_ACTIVATE),
+ .reload_limits = BIT(DEVLINK_RELOAD_LIMIT_NO_RESET),
.reload_down = mlx5_devlink_reload_down,
.reload_up = mlx5_devlink_reload_up,
};
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH net-next 15/16] net/mlx5: Add support for devlink reload limit no reset
2020-10-01 13:59 ` [PATCH net-next 15/16] net/mlx5: Add support for devlink reload limit no reset Moshe Shemesh
@ 2020-10-01 21:52 ` Jakub Kicinski
2020-10-02 15:08 ` Moshe Shemesh
0 siblings, 1 reply; 49+ messages in thread
From: Jakub Kicinski @ 2020-10-01 21:52 UTC (permalink / raw)
To: Moshe Shemesh; +Cc: David S. Miller, Jiri Pirko, netdev, linux-kernel
On Thu, 1 Oct 2020 16:59:18 +0300 Moshe Shemesh wrote:
> + err = mlx5_fw_reset_set_live_patch(dev);
> + if (err)
> + return err;
> +
> + return 0;
nit return mlx...
^ permalink raw reply [flat|nested] 49+ messages in thread
* Re: [PATCH net-next 15/16] net/mlx5: Add support for devlink reload limit no reset
2020-10-01 21:52 ` Jakub Kicinski
@ 2020-10-02 15:08 ` Moshe Shemesh
0 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-02 15:08 UTC (permalink / raw)
To: Jakub Kicinski, Moshe Shemesh
Cc: David S. Miller, Jiri Pirko, netdev, linux-kernel
On 10/2/2020 12:52 AM, Jakub Kicinski wrote:
> External email: Use caution opening links or attachments
>
>
> On Thu, 1 Oct 2020 16:59:18 +0300 Moshe Shemesh wrote:
>> + err = mlx5_fw_reset_set_live_patch(dev);
>> + if (err)
>> + return err;
>> +
>> + return 0;
> nit return mlx...
Right, will fix. Thanks.
^ permalink raw reply [flat|nested] 49+ messages in thread
* [PATCH net-next 16/16] devlink: Add Documentation/networking/devlink/devlink-reload.rst
2020-10-01 13:59 [PATCH net-next 00/16] Add devlink reload action and limit options Moshe Shemesh
` (14 preceding siblings ...)
2020-10-01 13:59 ` [PATCH net-next 15/16] net/mlx5: Add support for devlink reload limit no reset Moshe Shemesh
@ 2020-10-01 13:59 ` Moshe Shemesh
2020-10-03 9:14 ` Jiri Pirko
15 siblings, 1 reply; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-01 13:59 UTC (permalink / raw)
To: David S. Miller, Jakub Kicinski, Jiri Pirko
Cc: netdev, linux-kernel, Moshe Shemesh
Add devlink reload rst documentation file.
Update index file to include it.
Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
---
RFCv5 -> v1:
- Rename reload_action_limit_level to reload_limit
RFCv4 -> RFCv5:
- Rephrase namespace chnage section
- Rephrase note on actions performed
RFCv3 -> RFCv4:
- Remove reload action fw_activate_no_reset
- Add reload actions limit levels and document the no_reset limit level
constrains
RFCv2 -> RFCv3:
- Devlink reload returns the actions done
- Replace fw_live_patch action by fw_activate_no_reset
- Explain fw_activate meaning
RFCv1 -> RFCv2:
- Instead of reload levels driver,fw_reset,fw_live_patch have reload
actions driver_reinit,fw_activate,fw_live_patch
---
.../networking/devlink/devlink-reload.rst | 81 +++++++++++++++++++
Documentation/networking/devlink/index.rst | 1 +
2 files changed, 82 insertions(+)
create mode 100644 Documentation/networking/devlink/devlink-reload.rst
diff --git a/Documentation/networking/devlink/devlink-reload.rst b/Documentation/networking/devlink/devlink-reload.rst
new file mode 100644
index 000000000000..5abc5c2c75fd
--- /dev/null
+++ b/Documentation/networking/devlink/devlink-reload.rst
@@ -0,0 +1,81 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+==============
+Devlink Reload
+==============
+
+``devlink-reload`` provides mechanism to either reinit driver entities,
+applying ``devlink-params`` and ``devlink-resources`` new values or firmware
+activation depends on reload action selected.
+
+Reload actions
+==============
+
+User may select a reload action.
+By default ``driver_reinit`` action is selected.
+
+.. list-table:: Possible reload actions
+ :widths: 5 90
+
+ * - Name
+ - Description
+ * - ``driver-reinit``
+ - Devlink driver entities re-initialization, including applying
+ new values to devlink entities which are used during driver
+ load such as ``devlink-params`` in configuration mode
+ ``driverinit`` or ``devlink-resources``
+ * - ``fw_activate``
+ - Firmware activate. Activates new firmware if such image is stored and
+ pending activation. If no limitation specified this action may involve
+ firmware reset. If no new image pending this action will reload current
+ firmware image.
+
+Note that even though user asks for a specific action, the driver
+implementation might require to perform another action alongside with
+it. For example, some driver do not support driver reinitialization
+being performed without fw activation. Therefore, the devlink reload
+command returns the list of actions which were actrually performed.
+
+Reload limits
+=============
+
+By default reload actions are not limited and driver implementation may
+include reset or downtime as needed to perform the actions.
+
+However, some drivers support action limits, which limit the action
+implementation to specific constrains.
+
+.. list-table:: Possible reload limits
+ :widths: 5 90
+
+ * - Name
+ - Description
+ * - ``no_reset``
+ - No reset allowed, no down time allowed, no link flap and no
+ configuration is lost.
+
+Change namespace
+================
+
+The netns option allow user to be able to move devlink instances into
+namespaces during devlink reload operation.
+By default all devlink instances are created in init_net and stay there.
+
+example usage
+-------------
+
+.. code:: shell
+
+ $ devlink dev reload help
+ $ devlink dev reload DEV [ netns { PID | NAME | ID } ] [ action { driver_reinit | fw_activate } ] [ limit no_reset ]
+
+ # Run reload command for devlink driver entities re-initialization:
+ $ devlink dev reload pci/0000:82:00.0 action driver_reinit
+ reload_actions_performed:
+ driver_reinit
+
+ # Run reload command to activate firmware:
+ # Note that mlx5 driver reloads the driver while activating firmware
+ $ devlink dev reload pci/0000:82:00.0 action fw_activate
+ reload_actions_performed:
+ driver_reinit fw_activate
diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
index 7684ae5c4a4a..d82874760ae2 100644
--- a/Documentation/networking/devlink/index.rst
+++ b/Documentation/networking/devlink/index.rst
@@ -20,6 +20,7 @@ general.
devlink-params
devlink-region
devlink-resource
+ devlink-reload
devlink-trap
Driver-specific documentation
--
2.18.2
^ permalink raw reply related [flat|nested] 49+ messages in thread* Re: [PATCH net-next 16/16] devlink: Add Documentation/networking/devlink/devlink-reload.rst
2020-10-01 13:59 ` [PATCH net-next 16/16] devlink: Add Documentation/networking/devlink/devlink-reload.rst Moshe Shemesh
@ 2020-10-03 9:14 ` Jiri Pirko
2020-10-04 7:15 ` Moshe Shemesh
0 siblings, 1 reply; 49+ messages in thread
From: Jiri Pirko @ 2020-10-03 9:14 UTC (permalink / raw)
To: Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
Thu, Oct 01, 2020 at 03:59:19PM CEST, moshe@mellanox.com wrote:
>Add devlink reload rst documentation file.
>Update index file to include it.
>
>Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
>---
>RFCv5 -> v1:
>- Rename reload_action_limit_level to reload_limit
>RFCv4 -> RFCv5:
>- Rephrase namespace chnage section
>- Rephrase note on actions performed
>RFCv3 -> RFCv4:
>- Remove reload action fw_activate_no_reset
>- Add reload actions limit levels and document the no_reset limit level
> constrains
>RFCv2 -> RFCv3:
>- Devlink reload returns the actions done
>- Replace fw_live_patch action by fw_activate_no_reset
>- Explain fw_activate meaning
>RFCv1 -> RFCv2:
>- Instead of reload levels driver,fw_reset,fw_live_patch have reload
> actions driver_reinit,fw_activate,fw_live_patch
>---
> .../networking/devlink/devlink-reload.rst | 81 +++++++++++++++++++
> Documentation/networking/devlink/index.rst | 1 +
> 2 files changed, 82 insertions(+)
> create mode 100644 Documentation/networking/devlink/devlink-reload.rst
>
>diff --git a/Documentation/networking/devlink/devlink-reload.rst b/Documentation/networking/devlink/devlink-reload.rst
>new file mode 100644
>index 000000000000..5abc5c2c75fd
>--- /dev/null
>+++ b/Documentation/networking/devlink/devlink-reload.rst
>@@ -0,0 +1,81 @@
>+.. SPDX-License-Identifier: GPL-2.0
>+
>+==============
>+Devlink Reload
No reason for capital "R".
>+==============
>+
>+``devlink-reload`` provides mechanism to either reinit driver entities,
>+applying ``devlink-params`` and ``devlink-resources`` new values or firmware
>+activation depends on reload action selected.
Could you perhaps split the sentense? It is hard to read.
>+
>+Reload actions
>+==============
>+
>+User may select a reload action.
>+By default ``driver_reinit`` action is selected.
>+
>+.. list-table:: Possible reload actions
>+ :widths: 5 90
>+
>+ * - Name
>+ - Description
>+ * - ``driver-reinit``
>+ - Devlink driver entities re-initialization, including applying
>+ new values to devlink entities which are used during driver
>+ load such as ``devlink-params`` in configuration mode
>+ ``driverinit`` or ``devlink-resources``
>+ * - ``fw_activate``
>+ - Firmware activate. Activates new firmware if such image is stored and
>+ pending activation. If no limitation specified this action may involve
>+ firmware reset. If no new image pending this action will reload current
>+ firmware image.
>+
>+Note that even though user asks for a specific action, the driver
>+implementation might require to perform another action alongside with
>+it. For example, some driver do not support driver reinitialization
>+being performed without fw activation. Therefore, the devlink reload
>+command returns the list of actions which were actrually performed.
>+
>+Reload limits
>+=============
>+
>+By default reload actions are not limited and driver implementation may
>+include reset or downtime as needed to perform the actions.
>+
>+However, some drivers support action limits, which limit the action
>+implementation to specific constrains.
>+
>+.. list-table:: Possible reload limits
>+ :widths: 5 90
>+
>+ * - Name
>+ - Description
>+ * - ``no_reset``
>+ - No reset allowed, no down time allowed, no link flap and no
>+ configuration is lost.
>+
>+Change namespace
>+================
>+
>+The netns option allow user to be able to move devlink instances into
"allows"
>+namespaces during devlink reload operation.
>+By default all devlink instances are created in init_net and stay there.
>+
>+example usage
>+-------------
>+
>+.. code:: shell
>+
>+ $ devlink dev reload help
>+ $ devlink dev reload DEV [ netns { PID | NAME | ID } ] [ action { driver_reinit | fw_activate } ] [ limit no_reset ]
>+
>+ # Run reload command for devlink driver entities re-initialization:
>+ $ devlink dev reload pci/0000:82:00.0 action driver_reinit
>+ reload_actions_performed:
>+ driver_reinit
>+
>+ # Run reload command to activate firmware:
>+ # Note that mlx5 driver reloads the driver while activating firmware
>+ $ devlink dev reload pci/0000:82:00.0 action fw_activate
>+ reload_actions_performed:
>+ driver_reinit fw_activate
>diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
>index 7684ae5c4a4a..d82874760ae2 100644
>--- a/Documentation/networking/devlink/index.rst
>+++ b/Documentation/networking/devlink/index.rst
>@@ -20,6 +20,7 @@ general.
> devlink-params
> devlink-region
> devlink-resource
>+ devlink-reload
> devlink-trap
>
> Driver-specific documentation
>--
>2.18.2
>
^ permalink raw reply [flat|nested] 49+ messages in thread* Re: [PATCH net-next 16/16] devlink: Add Documentation/networking/devlink/devlink-reload.rst
2020-10-03 9:14 ` Jiri Pirko
@ 2020-10-04 7:15 ` Moshe Shemesh
0 siblings, 0 replies; 49+ messages in thread
From: Moshe Shemesh @ 2020-10-04 7:15 UTC (permalink / raw)
To: Jiri Pirko, Moshe Shemesh
Cc: David S. Miller, Jakub Kicinski, Jiri Pirko, netdev, linux-kernel
On 10/3/2020 12:14 PM, Jiri Pirko wrote:
> Thu, Oct 01, 2020 at 03:59:19PM CEST, moshe@mellanox.com wrote:
>> Add devlink reload rst documentation file.
>> Update index file to include it.
>>
>> Signed-off-by: Moshe Shemesh <moshe@mellanox.com>
>> ---
>> RFCv5 -> v1:
>> - Rename reload_action_limit_level to reload_limit
>> RFCv4 -> RFCv5:
>> - Rephrase namespace chnage section
>> - Rephrase note on actions performed
>> RFCv3 -> RFCv4:
>> - Remove reload action fw_activate_no_reset
>> - Add reload actions limit levels and document the no_reset limit level
>> constrains
>> RFCv2 -> RFCv3:
>> - Devlink reload returns the actions done
>> - Replace fw_live_patch action by fw_activate_no_reset
>> - Explain fw_activate meaning
>> RFCv1 -> RFCv2:
>> - Instead of reload levels driver,fw_reset,fw_live_patch have reload
>> actions driver_reinit,fw_activate,fw_live_patch
>> ---
>> .../networking/devlink/devlink-reload.rst | 81 +++++++++++++++++++
>> Documentation/networking/devlink/index.rst | 1 +
>> 2 files changed, 82 insertions(+)
>> create mode 100644 Documentation/networking/devlink/devlink-reload.rst
>>
>> diff --git a/Documentation/networking/devlink/devlink-reload.rst b/Documentation/networking/devlink/devlink-reload.rst
>> new file mode 100644
>> index 000000000000..5abc5c2c75fd
>> --- /dev/null
>> +++ b/Documentation/networking/devlink/devlink-reload.rst
>> @@ -0,0 +1,81 @@
>> +.. SPDX-License-Identifier: GPL-2.0
>> +
>> +==============
>> +Devlink Reload
> No reason for capital "R".
It looks as the convention here for rst headers, as in
devlink-region.rst, devlink-trap.rst, devlink-resource.rst
>
>> +==============
>> +
>> +``devlink-reload`` provides mechanism to either reinit driver entities,
>> +applying ``devlink-params`` and ``devlink-resources`` new values or firmware
>> +activation depends on reload action selected.
> Could you perhaps split the sentense? It is hard to read.
Yes.
>
>> +
>> +Reload actions
>> +==============
>> +
>> +User may select a reload action.
>> +By default ``driver_reinit`` action is selected.
>> +
>> +.. list-table:: Possible reload actions
>> + :widths: 5 90
>> +
>> + * - Name
>> + - Description
>> + * - ``driver-reinit``
>> + - Devlink driver entities re-initialization, including applying
>> + new values to devlink entities which are used during driver
>> + load such as ``devlink-params`` in configuration mode
>> + ``driverinit`` or ``devlink-resources``
>> + * - ``fw_activate``
>> + - Firmware activate. Activates new firmware if such image is stored and
>> + pending activation. If no limitation specified this action may involve
>> + firmware reset. If no new image pending this action will reload current
>> + firmware image.
>> +
>> +Note that even though user asks for a specific action, the driver
>> +implementation might require to perform another action alongside with
>> +it. For example, some driver do not support driver reinitialization
>> +being performed without fw activation. Therefore, the devlink reload
>> +command returns the list of actions which were actrually performed.
>> +
>> +Reload limits
>> +=============
>> +
>> +By default reload actions are not limited and driver implementation may
>> +include reset or downtime as needed to perform the actions.
>> +
>> +However, some drivers support action limits, which limit the action
>> +implementation to specific constrains.
>> +
>> +.. list-table:: Possible reload limits
>> + :widths: 5 90
>> +
>> + * - Name
>> + - Description
>> + * - ``no_reset``
>> + - No reset allowed, no down time allowed, no link flap and no
>> + configuration is lost.
>> +
>> +Change namespace
>> +================
>> +
>> +The netns option allow user to be able to move devlink instances into
> "allows"
Ack.
>
>> +namespaces during devlink reload operation.
>> +By default all devlink instances are created in init_net and stay there.
>> +
>> +example usage
>> +-------------
>> +
>> +.. code:: shell
>> +
>> + $ devlink dev reload help
>> + $ devlink dev reload DEV [ netns { PID | NAME | ID } ] [ action { driver_reinit | fw_activate } ] [ limit no_reset ]
>> +
>> + # Run reload command for devlink driver entities re-initialization:
>> + $ devlink dev reload pci/0000:82:00.0 action driver_reinit
>> + reload_actions_performed:
>> + driver_reinit
>> +
>> + # Run reload command to activate firmware:
>> + # Note that mlx5 driver reloads the driver while activating firmware
>> + $ devlink dev reload pci/0000:82:00.0 action fw_activate
>> + reload_actions_performed:
>> + driver_reinit fw_activate
>> diff --git a/Documentation/networking/devlink/index.rst b/Documentation/networking/devlink/index.rst
>> index 7684ae5c4a4a..d82874760ae2 100644
>> --- a/Documentation/networking/devlink/index.rst
>> +++ b/Documentation/networking/devlink/index.rst
>> @@ -20,6 +20,7 @@ general.
>> devlink-params
>> devlink-region
>> devlink-resource
>> + devlink-reload
>> devlink-trap
>>
>> Driver-specific documentation
>> --
>> 2.18.2
>>
^ permalink raw reply [flat|nested] 49+ messages in thread