Re: [PATCH net-next RFC v3 03/14] devlink: Add reload actions counters to dev get

[Moshe Shemesh]

On 8/31/2020 1:44 PM, Jiri Pirko wrote:
> Sun, Aug 30, 2020 at 05:27:23PM CEST, moshe@xxxxxxxxxxxx wrote:
> > Expose devlink reload actions counters to the user through devlink dev
> > get command.
> >
> > Examples:
> > $ devlink dev show
> > pci/0000:82:00.0:
> >   reload_actions_stats:
> >     driver_reinit 2
> >     fw_activate 1
> >     fw_activate_no_reset 0
> > pci/0000:82:00.1:
> >   reload_actions_stats:
> >     driver_reinit 1
> >     fw_activate 1
> >     fw_activate_no_reset 0
> >
> > $ devlink dev show -jp
> > {
> >     "dev": {
> >         "pci/0000:82:00.0": {
> >             "reload_actions_stats": [ {
> Perhaps "reload_action_stats" would be better.
OK.
> >                     "driver_reinit": 2
> >                 },{
> >                     "fw_activate": 1
> >                 },{
> >                     "fw_activate_no_reset": 0
> >                 } ]
> >         },
> >         "pci/0000:82:00.1": {
> >             "reload_actions_stats": [ {
> >                     "driver_reinit": 1
> >                 },{
> >                     "fw_activate": 1
> >                 },{
> >                     "fw_activate_no_reset": 0
> >                 } ]
> >         }
> >     }
> > }
> >
> > Signed-off-by: Moshe Shemesh <moshe@xxxxxxxxxxxx>
> > ---
> > v2 -> v3:
> > - Add reload actions counters instead of supported reload actions
> >   (reload actions counters are only for supported action so no need for
> >    both)
> > v1 -> v2:
> > - Removed DEVLINK_ATTR_RELOAD_DEFAULT_LEVEL
> > - Removed DEVLINK_ATTR_RELOAD_LEVELS_INFO
> > - Have actions instead of levels
> > ---
> > include/uapi/linux/devlink.h |  3 +++
> > net/core/devlink.c           | 37 +++++++++++++++++++++++++++++++-----
> > 2 files changed, 35 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/uapi/linux/devlink.h b/include/uapi/linux/devlink.h
> > index 0a438135c3cf..fd7667c78417 100644
> > --- a/include/uapi/linux/devlink.h
> > +++ b/include/uapi/linux/devlink.h
> > @@ -478,6 +478,9 @@ enum devlink_attr {
> >
> > 	DEVLINK_ATTR_RELOAD_ACTION,		/* u8 */
> > 	DEVLINK_ATTR_RELOAD_ACTIONS_DONE,	/* nested */
> > +	DEVLINK_ATTR_RELOAD_ACTION_CNT_VALUE,	/* u32 */
> > +	DEVLINK_ATTR_RELOAD_ACTION_CNT,		/* nested */
> > +	DEVLINK_ATTR_RELOAD_ACTIONS_CNTS,	/* nested */
> Be in-sync with the user outputs. Perhaps something like:
> 	DEVLINK_ATTR_RELOAD_ACTION_STATS
> 	DEVLINK_ATTR_RELOAD_ACTION_STAT
> 	DEVLINK_ATTR_RELOAD_ACTION_STAT_VALUE
> ?


I actually see it as counters of number of times action done, but 
generally counters and stats are the same, so I am fine with that too.

> > 	/* add new attributes above here, update the policy in devlink.c */
> [..]