Hi Matan, On Wed, Jan 10, 2018 at 12:43:33PM +0000, Matan Azrad wrote: > Hi Gaetan > > > -----Original Message----- > > From: dev [mailto:dev-boun...@dpdk.org] On Behalf Of Matan Azrad > > Sent: Wednesday, January 10, 2018 2:31 PM > > To: Thomas Monjalon <tho...@monjalon.net>; Gaetan Rivet > > <gaetan.ri...@6wind.com> > > Cc: dev@dpdk.org > > Subject: [dpdk-dev] [PATCH v4 6/6] net/failsafe: fix removed device handling > > > > There is time between the physical removal of the device until sub-device > > PMDs get a RMV interrupt. At this time DPDK PMDs and applications still > > don't know about the removal and may call sub-device control operation > > which should return an error. > > > > In previous code this error is reported to the application contrary to > > fail-safe > > principle that the app should not be aware of device removal. > > > > Add an removal check in each relevant control command error flow and > > prevent an error report to application when the sub-device is removed. > > > > Signed-off-by: Matan Azrad <ma...@mellanox.com> > > --- > > drivers/net/failsafe/failsafe_flow.c | 18 ++++++++++------- > > drivers/net/failsafe/failsafe_ops.c | 35 ++++++++++++++++++++++-------- > > --- > > drivers/net/failsafe/failsafe_private.h | 11 +++++++++++ > > 3 files changed, 46 insertions(+), 18 deletions(-) > > > > diff --git a/drivers/net/failsafe/failsafe_flow.c > > b/drivers/net/failsafe/failsafe_flow.c > > index 153ceee..c072d1e 100644 > > --- a/drivers/net/failsafe/failsafe_flow.c > > +++ b/drivers/net/failsafe/failsafe_flow.c > > @@ -87,7 +87,7 @@ > > DEBUG("Calling rte_flow_validate on sub_device %d", i); > > ret = rte_flow_validate(PORT_ID(sdev), > > attr, patterns, actions, error); > > - if (ret) { > > + if ((ret = fs_err(sdev, ret))) { > This assignment in "if" statement causes to checkpatch error, I sent it as is > because you asked it like this. > If you think I need to change it, I see 2 options: > > 1. > ret = fs_err(sdev, ret); > if (ret ) {...} > > 2. > if (fs_err(sdev, &ret)) {..} > > what do you think? >
Yes I forgot that checkpatch was like this. Our mail crossed, but I acked this patch. I think this is acceptable at the driver level, or should be at the discretion of the driver maintainer. So personally, I'd say leave it this way. If someone or something shouts about this we will consider alternatives. Otherwise this is readable and easily understandable as-is. Regards, -- Gaëtan Rivet 6WIND
