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 <era...@mellanox.com>

-------------------------------------------------------
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.

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?


        period PERIOD
               The period on which we limit the amount of performed actions, 
measured in seconds.

        count COUNT
               The maximum amount of actions performed in a limit time frame.

Perhaps                                 
                 The maximum number of actions performed in a limited time 
frame.

ack


        fail   { ignore | down }
                   Specify the behavior once count limit was reached.

                   ignore - Ignore errors without execution of any action.

                   down - Driver will remain in nonoperational state.

    devlink health action reinit - reset devlink action attributes (period, 
count, fail, etc)
        DEV    Specifies the devlink device to set.

        name NAME
               Specifies the devlink action to set.

Perhaps s/set/reinitialise/g for the above two descriptions.

ack


Hope this helps,
Tobin.

thanks

Reply via email to