Subject: Re: [RFC PATCH iproute2-next] man: Add devlink health man page

From: "Tobin C. Harding" <[email-masked]>
Date: Fri, 14 Sep 2018 08:06:34 +1000

On Thu, Sep 13, 2018 at 02:58:52PM +0300, Eran Ben Elisha wrote:
> 
> 
> On 9/13/2018 1:27 PM, Tobin C. Harding wrote:
> > On Thu, Sep 13, 2018 at 11:18:16AM +0300, Eran Ben Elisha wrote:
> > > Add devlink-health man page. Devlink-health tool will control device
> > > health attributes, sensors, actions and logging.
> > > 
> > > Signed-off-by: Eran Ben Elisha <[email-masked]>
> > > 
> > > -------------------------------------------------------
> > > Copy paste man output to here for easier review process of the RFC.
> > > 
> > > DEVLINK-HEALTH(8)                                                                                               Linux                                                                                              DEVLINK-HEALTH(8)
> > > 
> > > NAME
> > >         devlink-health - devlink health configuration
> > > 
> > > SYNOPSIS
> > >         devlink [ OPTIONS ] health  { COMMAND | help }
> > > 
> > >         OPTIONS := { -V[ersion] | -n[no-nice-names] }
> > > 
> > >         devlink health show [ DEV ] [ sensor NAME ]
> > > 
> > >         devlink health sensor set DEV name NAME [ action NAME { active | inactive } ]"
> > > 
> > >         devlink health action set DEV name NAME period PERIOD count COUNT fail { ignore | down }
> > > 
> > >         devlink health action reinit DEV name NAME
> > > 
> > >         devlink health help
> > > 
> > > DESCRIPTION
> > >         devlink-health tool allows user to configure the way driver treats unexpected status. The tool allows configuration of the sensors that can trigger health activity. Set for each sensor the follow up operations, such as,
> > >         reset and dump of info. In addition, set the health activity termination action.
> > > 
> > >     devlink health show - Display devlink health sensors and actions attributes
> > >         DEV - Specifies the devlink device to show.  If this argument is omitted, all devices are listed.
> > > 
> > >             Format is:
> > >               BUS_NAME/BUS_ADDRESS
> > > 
> > >         sensor NAME - Specifies the devlink sensor to show.
> > > 
> > 
> > Perhaps the commands should include the optional arguments so when
> > reading the description one doesn't have to scroll to the top of the
> > page all the time
> > 
> > e.g
> >       devlink health show [ DEV ] [ sensor NAME ] - Display devlink health sensors and actions attributes
> > 
> 
> I followed the scheme presented in all other devlink man pages.
> see devlink-region, devlink-port, etc.

Oh ok, my mistake.  I'd stick with what you have then.  Thanks for
pointing this out.

> From my perspective, I am fine with adding it to devlink-health, need ack
> from the devlink maintainer to see if he likes it...
> 
> > >     devlink health sensor set - sets devlink health sensor attributes
> > >         DEV    Specifies the devlink device to show.
> > 
> > 	 		      	      	     	set
> > 
> > >         name NAME
> > >                Name of the sensor to set.
> > > 
> > >         action NAME { active | inactive }
> > >                    Specify which actions to activate and which to deactivate once a sensor was triggered. actions can be dump, reset, etc.
> > > 
> > >     devlink health action set - sets devlink action attributes
> > >         DEV    Specifies the devlink device to set.
> > > 
> > >         name NAME
> > >                Specifies the devlink action to set.
> > 
> > This is a little unclear to me?
> 
> what is not clear? the term 'action' or the naming? can you elaborate?

It wasn't immediately clear what 'name' referred to.  But following on
from discussion above this may be because I have not read any of the
other devlink man pages.

thanks,
Tobin.