Hi Gaetan > -----Original Message----- > From: Gaëtan Rivet [mailto:gaetan.ri...@6wind.com] > Sent: Wednesday, December 20, 2017 12:22 AM > To: Matan Azrad <ma...@mellanox.com> > Cc: Adrien Mazarguil <adrien.mazarg...@6wind.com>; Thomas Monjalon > <tho...@monjalon.net>; dev@dpdk.org > Subject: Re: [PATCH v3 6/6] net/failsafe: fix removed device handling > > Hi Matan, > > On Tue, Dec 19, 2017 at 05:10:15PM +0000, Matan Azrad wrote: > > 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. > > > > Fixes: a46f8d5 ("net/failsafe: add fail-safe PMD") > > Fixes: b737a1e ("net/failsafe: support flow API") > > > > Signed-off-by: Matan Azrad <ma...@mellanox.com> > > --- > > <snip> > > > +/* > > + * Check if error should be reported to the user. > > + */ > > +static inline bool > > +fs_is_error(struct sub_device *sdev, int err) { > > + /* A device removal shouldn't be reported as an error. */ > > + if (err == 0 || sdev->remove == 1 || err == -EIO) > > + return false; > > + return true; > > +} > > This is better, thanks. > > However is there a reason you did not follow the same pattern as ethdev > with eth_err? I see the two functions as similar in their intent, making them > close to each other would be clearer to a reader being familiar with the > ethdev API and that would be interested in fail-safe. > > What do you think? >
I think that there is a real different between eth_err function to fs_is_error: ethdev uses eth_err function to adjust removal return value to be -EIO. fail-safe uses fs_is_error function to check if an error should be reported to the user to save the fail-safe principle that the app should not be aware of device removal - this is the main idea that also causes me to change the name from fs_is_removed to fs_is_error. > -- > Gaëtan Rivet > 6WIND
