Adding Ferruh and Ajit From: Kalesh Anakkur Purayil <kalesh-anakkur.pura...@broadcom.com> Sent: Wednesday, October 7, 2020 7:47 AM To: Ophir Munk <ophi...@nvidia.com> Cc: dev@dpdk.org; NBU-Contact-Thomas Monjalon <tho...@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 <ophi...@nvidia.com<mailto: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. More comments below: > -----Original Message----- > From: dev <dev-boun...@dpdk.org<mailto:dev-boun...@dpdk.org>> On Behalf Of > Kalesh A P > Sent: Wednesday, September 30, 2020 3:33 PM > To: dev@dpdk.org<mailto: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<mailto: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<mailto:kalesh-anakkur.pura...@broadcom.com>> > Reviewed-by: Ajit Kumar Khaparde > <ajit.khapa...@broadcom.com<mailto: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 -- Regards, Kalesh A P
