netdev - Re: [RFC net-next v2 2/3] devlink: health: add remediation type

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210311084502.40c146d1@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Thu, 11 Mar 2021 08:45:02 -0800
From:   Jakub Kicinski <kuba@...nel.org>
To:     Eran Ben Elisha <eranbe@...dia.com>
Cc:     <netdev@...r.kernel.org>, <jiri@...nulli.us>, <saeedm@...dia.com>,
        <andrew.gospodarek@...adcom.com>, <jacob.e.keller@...el.com>,
        <guglielmo.morandin@...adcom.com>, <eugenem@...com>,
        <eranbe@...lanox.com>
Subject: Re: [RFC net-next v2 2/3] devlink: health: add remediation type

On Thu, 11 Mar 2021 16:32:44 +0200 Eran Ben Elisha wrote:
> > +/**
> > + * enum devlink_health_reporter_remedy - severity of remediation procedure
> > + * @DL_HEALTH_REMEDY_NONE: transient error, no remediation required
> > + * @DL_HEALTH_REMEDY_KICK: device stalled, processing will be re-triggered
> > + * @DL_HEALTH_REMEDY_COMP_RESET: associated device component (e.g. device queue)
> > + *			will be reset
> > + * @DL_HEALTH_REMEDY_RESET: full device reset, will result in temporary
> > + *			unavailability of the device, device configuration
> > + *			should not be lost
> > + * @DL_HEALTH_REMEDY_REINIT: device will be reinitialized and configuration lost
> > + *
> > + * Used in %DEVLINK_ATTR_HEALTH_REPORTER_REMEDY, categorizes the health reporter
> > + * by the severity of the remediation.
> > + */
> > +enum devlink_health_remedy {
> > +	DL_HEALTH_REMEDY_NONE = 1,  
> 
> What is the reason zero is skipped?
> 
> > +	DL_HEALTH_REMEDY_KICK,
> > +	DL_HEALTH_REMEDY_COMP_RESET,
> > +	DL_HEALTH_REMEDY_RESET,
> > +	DL_HEALTH_REMEDY_REINIT,
> > +};
> > +
> >   #endif /* _UAPI_LINUX_DEVLINK_H_ */
> > diff --git a/net/core/devlink.c b/net/core/devlink.c
> > index 8e4e4bd7bb36..09d77d43ff63 100644
> > --- a/net/core/devlink.c
> > +++ b/net/core/devlink.c
> > @@ -6095,7 +6095,8 @@ __devlink_health_reporter_create(struct devlink *devlink,
> >   {
> >   	struct devlink_health_reporter *reporter;
> >   
> > -	if (WARN_ON(graceful_period && !ops->recover))
> > +	if (WARN_ON(graceful_period && !ops->recover) ||
> > +	    WARN_ON(ops->recover && !ops->remedy))  
> 
> It allows drivers to set recover callback and report DL_HEALTH_REMEDY_NONE.
> Defining DL_HEALTH_REMEDY_NONE = 0  would make this if clause to catch it.

I was intending for "none" to mean no remediation from the driver side.
E.g. device sees bad descriptor and tosses it away. 

That's different from cases where remediation is fully manual.

I will improve the kdoc.