[dpdk-dev] [RFC PATCH v4 3/3] app/testpmd: handle device recovery event
Ophir Munk
ophirmu at nvidia.com
Wed Oct 7 10:36:13 CEST 2020
Adding Ferruh and Ajit
From: Kalesh Anakkur Purayil <kalesh-anakkur.purayil at broadcom.com>
Sent: Wednesday, October 7, 2020 7:47 AM
To: Ophir Munk <ophirmu at nvidia.com>
Cc: dev at dpdk.org; NBU-Contact-Thomas Monjalon <thomas at monjalon.net>
Subject: Re: [dpdk-dev] [RFC PATCH v4 3/3] app/testpmd: handle device recovery event
Hi Ophir,
Thank you for the comments. I will address them in the next version.
I will push these changes as Patches next time and not as an RFC. Hope that is OK.
Regards,
Kalesh
On Tue, Oct 6, 2020 at 10:55 PM Ophir Munk <ophirmu at nvidia.com<mailto:ophirmu at nvidia.com>> wrote:
Hi Kalesh,
Please find a few comments.
The name you gave to the event (EVENT_RESET) is very close to an already existing one: "EVENT_INTR_RESET".
But they are different.
EVENT_INTR_RESET originates from a port reset. It requires application reaction. It is widely used. It is documented in *.rst files.
EVENT_RESET originates from FW error (or maybe any error). It requires no application reaction (PMD manages by itself). It is not documented.
I therefore suggest renaming it (maybe EVENT_ERR_RECOVERING) and please document it in *.rst files.
More comments below:
> -----Original Message-----
> From: dev <dev-bounces at dpdk.org<mailto:dev-bounces at dpdk.org>> On Behalf Of Kalesh A P
> Sent: Wednesday, September 30, 2020 3:33 PM
> To: dev at dpdk.org<mailto:dev at dpdk.org>
> Subject: [dpdk-dev] [RFC PATCH v4 3/3] app/testpmd: handle device recovery
> event
>
> From: Kalesh AP <kalesh-anakkur.purayil at broadcom.com<mailto:kalesh-anakkur.purayil at broadcom.com>>
>
> Added code to handle device reset and recovery event in testpmd.
> This is an indication from the PMD that device has reset and recovered error
> condition.
>
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil at broadcom.com<mailto:kalesh-anakkur.purayil at broadcom.com>>
> Reviewed-by: Ajit Kumar Khaparde <ajit.khaparde at broadcom.com<mailto:ajit.khaparde at broadcom.com>>
> ---
> app/test-pmd/testpmd.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> fe6450c..1c8fb46 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -380,6 +380,8 @@ static const char * const eth_event_desc[] = {
> [RTE_ETH_EVENT_NEW] = "device probed",
> [RTE_ETH_EVENT_DESTROY] = "device released",
> [RTE_ETH_EVENT_FLOW_AGED] = "flow aged",
> + [RTE_ETH_EVENT_RESET] = "device reset",
"device reset" is similar to the existing "reset" string. Can you suggest a different one? Maybe "error under recovery" ?
> + [RTE_ETH_EVENT_RECOVERED] = "device recovery",
Wouldn't you prefer "device recovered" ?
> [RTE_ETH_EVENT_MAX] = NULL,
> };
>
> @@ -394,7 +396,9 @@ uint32_t event_print_mask = (UINT32_C(1) <<
> RTE_ETH_EVENT_UNKNOWN) |
> (UINT32_C(1) << RTE_ETH_EVENT_IPSEC) |
> (UINT32_C(1) << RTE_ETH_EVENT_MACSEC) |
> (UINT32_C(1) << RTE_ETH_EVENT_INTR_RMV) |
> - (UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED);
> + (UINT32_C(1) << RTE_ETH_EVENT_FLOW_AGED) |
> + (UINT32_C(1) << RTE_ETH_EVENT_RESET) |
> + (UINT32_C(1) << RTE_ETH_EVENT_RECOVERED);
> /*
> * Decide if all memory are locked for performance.
> */
> --
> 2.10.1
--
Regards,
Kalesh A P
More information about the dev
mailing list