On 10/7/2020 5:46 AM, Kalesh Anakkur Purayil wrote:
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 <ophi...@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.
+1 to renaming and documenting the event.
And agree to proceed as regular patch instead of RFC.
More comments below:
-----Original Message-----
From: dev <dev-boun...@dpdk.org> On Behalf Of Kalesh A P
Sent: Wednesday, September 30, 2020 3:33 PM
To: dev@dpdk.org
Subject: [dpdk-dev] [RFC PATCH v4 3/3] app/testpmd: handle device
recovery
event
From: Kalesh AP <kalesh-anakkur.pura...@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.pura...@broadcom.com>
Reviewed-by: Ajit Kumar Khaparde <ajit.khapa...@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
