netdev - Re: [PATCH v9 net-next 04/14] pds_core: add devlink health facilities

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]

Date:   Sun, 9 Apr 2023 14:54:14 +0300
From:   Leon Romanovsky <leon@...nel.org>
To:     Shannon Nelson <shannon.nelson@....com>
Cc:     brett.creeley@....com, davem@...emloft.net, netdev@...r.kernel.org,
        kuba@...nel.org, drivers@...sando.io, jiri@...nulli.us
Subject: Re: [PATCH v9 net-next 04/14] pds_core: add devlink health facilities

On Thu, Apr 06, 2023 at 04:41:33PM -0700, Shannon Nelson wrote:
> Add devlink health reporting on top of our fw watchdog.
> 
> Example:
>   # devlink health show pci/0000:2b:00.0 reporter fw
>   pci/0000:2b:00.0:
>     reporter fw
>       state healthy error 0 recover 0
>   # devlink health diagnose pci/0000:2b:00.0 reporter fw
>    Status: healthy State: 1 Generation: 0 Recoveries: 0
> 
> Signed-off-by: Shannon Nelson <shannon.nelson@....com>
> ---
>  .../device_drivers/ethernet/amd/pds_core.rst  | 12 ++++++
>  drivers/net/ethernet/amd/pds_core/Makefile    |  1 +
>  drivers/net/ethernet/amd/pds_core/core.c      |  6 +++
>  drivers/net/ethernet/amd/pds_core/core.h      |  6 +++
>  drivers/net/ethernet/amd/pds_core/devlink.c   | 37 +++++++++++++++++++
>  drivers/net/ethernet/amd/pds_core/main.c      | 22 +++++++++++
>  6 files changed, 84 insertions(+)
>  create mode 100644 drivers/net/ethernet/amd/pds_core/devlink.c

<...>

> +int pdsc_fw_reporter_diagnose(struct devlink_health_reporter *reporter,
> +			      struct devlink_fmsg *fmsg,
> +			      struct netlink_ext_ack *extack)
> +{
> +	struct pdsc *pdsc = devlink_health_reporter_priv(reporter);
> +	int err = 0;
> +
> +	if (test_bit(PDSC_S_FW_DEAD, &pdsc->state))

How is this check protected from race with your health workqueue added
in previous patch? 

> +		err = devlink_fmsg_string_pair_put(fmsg, "Status", "dead");
> +	else if (!pdsc_is_fw_good(pdsc))

Same question.

> +		err = devlink_fmsg_string_pair_put(fmsg, "Status", "unhealthy");
> +	else
> +		err = devlink_fmsg_string_pair_put(fmsg, "Status", "healthy");
> +	if (err)
> +		return err;

Thanks