[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20200821104907.00004607@intel.com>
Date: Fri, 21 Aug 2020 10:49:07 -0700
From: Jesse Brandeburg <jesse.brandeburg@...el.com>
To: Igor Russkikh <irusskikh@...vell.com>
Cc: <netdev@...r.kernel.org>, "David S . Miller" <davem@...emloft.net>,
Jakub Kicinski <kuba@...nel.org>,
Ariel Elior <aelior@...vell.com>,
Michal Kalderon <mkalderon@...vell.com>,
"Alexander Lobakin" <alobakin@...vell.com>,
Michal Kalderon <michal.kalderon@...vell.com>
Subject: Re: [PATCH net-next 07/11] qed: use devlink logic to report errors
Igor Russkikh wrote:
> Use devlink_health_report to push error indications.
> We implement this in qede via callback function to make it possible
> to reuse the same for other drivers sitting on top of qed in future.
>
> Signed-off-by: Igor Russkikh <irusskikh@...vell.com>
> Signed-off-by: Alexander Lobakin <alobakin@...vell.com>
> Signed-off-by: Michal Kalderon <michal.kalderon@...vell.com>
> ---
> drivers/net/ethernet/qlogic/qed/qed_devlink.c | 17 +++++++++++++++++
> drivers/net/ethernet/qlogic/qed/qed_devlink.h | 2 ++
> drivers/net/ethernet/qlogic/qed/qed_main.c | 1 +
> drivers/net/ethernet/qlogic/qede/qede.h | 1 +
> drivers/net/ethernet/qlogic/qede/qede_main.c | 5 ++++-
> include/linux/qed/qed_if.h | 3 +++
> 6 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.c b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
> index 843a35f14cca..ffe776a4f99a 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_devlink.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
> @@ -14,6 +14,23 @@ enum qed_devlink_param_id {
> QED_DEVLINK_PARAM_ID_IWARP_CMT,
> };
>
> +struct qed_fw_fatal_ctx {
> + enum qed_hw_err_type err_type;
> +};
> +
> +int qed_report_fatal_error(struct devlink *devlink, enum qed_hw_err_type err_type)
> +{
> + struct qed_devlink *qdl = devlink_priv(devlink);
> + struct qed_fw_fatal_ctx fw_fatal_ctx = {
> + .err_type = err_type,
> + };
> +
> + devlink_health_report(qdl->fw_reporter,
> + "Fatal error reported", &fw_fatal_ctx);
> +
> + return 0;
> +}
> +
> static const struct devlink_health_reporter_ops qed_fw_fatal_reporter_ops = {
> .name = "fw_fatal",
> };
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.h b/drivers/net/ethernet/qlogic/qed/qed_devlink.h
> index c68ecf778826..ccc7d1d1bfd4 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_devlink.h
> +++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.h
> @@ -15,4 +15,6 @@ void qed_devlink_unregister(struct devlink *devlink);
> void qed_fw_reporters_create(struct devlink *devlink);
> void qed_fw_reporters_destroy(struct devlink *devlink);
>
> +int qed_report_fatal_error(struct devlink *dl, enum qed_hw_err_type err_type);
> +
> #endif
> diff --git a/drivers/net/ethernet/qlogic/qed/qed_main.c b/drivers/net/ethernet/qlogic/qed/qed_main.c
> index d1a559ccf516..a64d594f9294 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_main.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_main.c
> @@ -3007,6 +3007,7 @@ const struct qed_common_ops qed_common_ops_pass = {
> .update_msglvl = &qed_init_dp,
> .devlink_register = qed_devlink_register,
> .devlink_unregister = qed_devlink_unregister,
> + .report_fatal_error = qed_report_fatal_error,
> .dbg_all_data = &qed_dbg_all_data,
> .dbg_all_data_size = &qed_dbg_all_data_size,
> .chain_alloc = &qed_chain_alloc,
> diff --git a/drivers/net/ethernet/qlogic/qede/qede.h b/drivers/net/ethernet/qlogic/qede/qede.h
> index 1f0e7505a973..3efc5899f656 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede.h
> +++ b/drivers/net/ethernet/qlogic/qede/qede.h
> @@ -264,6 +264,7 @@ struct qede_dev {
>
> struct bpf_prog *xdp_prog;
>
> + enum qed_hw_err_type last_err_type;
> unsigned long err_flags;
> #define QEDE_ERR_IS_HANDLED 31
> #define QEDE_ERR_ATTN_CLR_EN 0
> diff --git a/drivers/net/ethernet/qlogic/qede/qede_main.c b/drivers/net/ethernet/qlogic/qede/qede_main.c
> index 7c2d948b2035..df437c3f1fc9 100644
> --- a/drivers/net/ethernet/qlogic/qede/qede_main.c
> +++ b/drivers/net/ethernet/qlogic/qede/qede_main.c
> @@ -1181,7 +1181,6 @@ static int __qede_probe(struct pci_dev *pdev, u32 dp_module, u8 dp_level,
> }
> } else {
> struct net_device *ndev = pci_get_drvdata(pdev);
> -
should have left this blank line (there should always be a blank line
after declarations.)
> edev = netdev_priv(ndev);
>
> if (edev && edev->devlink) {
I think I mentioned this check in one of my other responses.
> @@ -2603,6 +2602,9 @@ static void qede_generic_hw_err_handler(struct qede_dev *edev)
> "Generic sleepable HW error handling started - err_flags 0x%lx\n",
> edev->err_flags);
>
> + if (edev->devlink)
> + edev->ops->common->report_fatal_error(edev->devlink, edev->last_err_type);
> +
> /* Trigger a recovery process.
> * This is placed in the sleep requiring section just to make
> * sure it is the last one, and that all the other operations
> @@ -2663,6 +2665,7 @@ static void qede_schedule_hw_err_handler(void *dev,
> return;
> }
>
> + edev->last_err_type = err_type;
> qede_set_hw_err_flags(edev, err_type);
> qede_atomic_hw_err_handler(edev);
> set_bit(QEDE_SP_HW_ERR, &edev->sp_flags);
> diff --git a/include/linux/qed/qed_if.h b/include/linux/qed/qed_if.h
> index 30fe06fe06a0..1297726f2b25 100644
> --- a/include/linux/qed/qed_if.h
> +++ b/include/linux/qed/qed_if.h
> @@ -906,6 +906,9 @@ struct qed_common_ops {
>
> int (*dbg_all_data_size) (struct qed_dev *cdev);
>
> + int (*report_fatal_error)(struct devlink *devlink,
way too many extra spaces here, doesn't even match the line above,
Please just do
\tint (*foo)(arg, arg, ...)
> + enum qed_hw_err_type err_type);
> +
> /**
> * @brief can_link_change - can the instance change the link or not
> *
Powered by blists - more mailing lists