| 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: Thu, 24 Sep 2020 22:31:35 +0300
From: Moshe Shemesh <moshe@...dia.com>
To: Jakub Kicinski <kuba@...nel.org>,
Moshe Shemesh <moshe@...lanox.com>
CC: "David S. Miller" <davem@...emloft.net>,
Jiri Pirko <jiri@...lanox.com>, <netdev@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next RFC v5 03/15] devlink: Add reload action stats
On 9/23/2020 9:42 PM, Jakub Kicinski wrote:
> External email: Use caution opening links or attachments
>
>
> On Fri, 18 Sep 2020 19:06:39 +0300 Moshe Shemesh wrote:
>> Add reload action stats to hold the history per reload action type and
>> limit level.
>>
>> 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.
>>
>> The function devlink_remote_reload_actions_performed() is exported to
>> enable also drivers update on reload actions performed, for example in
>> case firmware activation with reset finished successfully but was
>> initiated by remote host.
>>
>> Add devlink notification on stats update.
>>
>> Signed-off-by: Moshe Shemesh <moshe@...lanox.com>
> I'd split this and the next patch by stat type (remote vs local).
> Rather than split them by collect / report.
OK.
>> diff --git a/include/net/devlink.h b/include/net/devlink.h
>> index d8c62d605381..f09f55a47d09 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_ACTION_STATS_ARRAY_SIZE \
>> + (__DEVLINK_RELOAD_ACTION_LIMIT_LEVEL_MAX * __DEVLINK_RELOAD_ACTION_MAX)
>> +
>> struct devlink_ops;
>>
>> struct devlink {
>> @@ -38,6 +41,8 @@ struct devlink {
>> struct list_head trap_policer_list;
>> const struct devlink_ops *ops;
>> struct xarray snapshot_ids;
>> + u32 reload_action_stats[DEVLINK_RELOAD_ACTION_STATS_ARRAY_SIZE];
>> + u32 remote_reload_action_stats[DEVLINK_RELOAD_ACTION_STATS_ARRAY_SIZE];
>> struct device *dev;
>> possible_net_t _net;
>> struct mutex lock; /* Serializes access to devlink instance specific objects such as
>> @@ -1400,6 +1405,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_action_limit_level limit_level,
>> + 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/net/core/devlink.c b/net/core/devlink.c
>> index fee6fcc7dead..1509c2ffb98b 100644
>> --- a/net/core/devlink.c
>> +++ b/net/core/devlink.c
>> @@ -3007,16 +3007,74 @@ bool devlink_is_reload_failed(const struct devlink *devlink)
>> }
>> EXPORT_SYMBOL_GPL(devlink_is_reload_failed);
>>
>> +static void
>> +__devlink_reload_action_stats_update(struct devlink *devlink,
>> + u32 *reload_action_stats,
>> + enum devlink_reload_action_limit_level limit_level,
>> + unsigned long actions_performed)
>> +{
>> + int stat_idx;
>> + int action;
>> +
>> + if (!actions_performed)
>> + return;
>> +
>> + if (WARN_ON(limit_level > DEVLINK_RELOAD_ACTION_LIMIT_LEVEL_MAX))
>> + return;
>> + for (action = 0; action <= DEVLINK_RELOAD_ACTION_MAX; action++) {
>> + if (!test_bit(action, &actions_performed))
>> + continue;
>> + stat_idx = limit_level * __DEVLINK_RELOAD_ACTION_MAX + action;
>> + reload_action_stats[stat_idx]++;
>> + }
>> + devlink_notify(devlink, DEVLINK_CMD_NEW);
>> +}
>> +
>> +static void
>> +devlink_reload_action_stats_update(struct devlink *devlink,
>> + enum devlink_reload_action_limit_level limit_level,
>> + unsigned long actions_performed)
>> +{
>> + __devlink_reload_action_stats_update(devlink, devlink->reload_action_stats,
>> + limit_level, 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_level: reload action limit level
>> + * @actions_performed: bitmask of actions performed
>> + */
>> +void devlink_remote_reload_actions_performed(struct devlink *devlink,
>> + enum devlink_reload_action_limit_level limit_level,
>> + unsigned long actions_performed)
>> +{
>> + __devlink_reload_action_stats_update(devlink, devlink->remote_reload_action_stats,
>> + limit_level, 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_action_limit_level limit_level,
>> struct netlink_ext_ack *extack, unsigned long *actions_performed)
>> {
>> + u32 remote_reload_action_stats[DEVLINK_RELOAD_ACTION_STATS_ARRAY_SIZE];
>> int err;
>>
>> if (!devlink->reload_enabled)
>> return -EOPNOTSUPP;
>>
>> + memcpy(remote_reload_action_stats, devlink->remote_reload_action_stats,
>> + sizeof(remote_reload_action_stats));
>> err = devlink->ops->reload_down(devlink, !!dest_net, action, limit_level, extack);
>> if (err)
>> return err;
>> @@ -3030,6 +3088,10 @@ static int devlink_reload(struct devlink *devlink, struct net *dest_net,
>> return err;
>>
>> WARN_ON(!test_bit(action, actions_performed));
>> + /* protect from driver updating the remote action within devlink reload */
> s/protect from/catch/
Ack.
>> + WARN_ON(memcmp(remote_reload_action_stats, devlink->remote_reload_action_stats,
>> + sizeof(remote_reload_action_stats)));
>> + devlink_reload_action_stats_update(devlink, limit_level, *actions_performed);
>> return 0;
>> }
>>
Powered by blists - more mailing lists