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.
> 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
>
>
--
Regards,
Kalesh A P
