lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 3 Oct 2020 11:05:42 +0200 From: Jiri Pirko <jiri@...nulli.us> To: Moshe Shemesh <moshe@...lanox.com> Cc: "David S. Miller" <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>, Jiri Pirko <jiri@...dia.com>, netdev@...r.kernel.org, linux-kernel@...r.kernel.org Subject: Re: [PATCH net-next 05/16] devlink: Add remote reload stats Thu, Oct 01, 2020 at 03:59:08PM CEST, moshe@...lanox.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@...lanox.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 >
Powered by blists - more mailing lists