From mboxrd@z Thu Jan  1 00:00:00 1970
From: Matan Azrad <matan@mellanox.com>
Subject: Re: [PATCH v3 6/6] net/failsafe: fix removed device
	handling
Date: Wed, 20 Dec 2017 10:58:29 +0000
Message-ID: <HE1PR0502MB3659A32940C84B489CB82FBCD20C0@HE1PR0502MB3659.eurprd05.prod.outlook.com>
References: <1513175370-16583-1-git-send-email-matan@mellanox.com>
 <1513703415-29145-1-git-send-email-matan@mellanox.com>
 <1513703415-29145-7-git-send-email-matan@mellanox.com>
 <20171219222131.plcfn5wqggyn5znw@bidouze.vm.6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>, Thomas Monjalon
 <thomas@monjalon.net>, "dev@dpdk.org" <dev@dpdk.org>
To: =?iso-8859-1?Q?Ga=EBtan_Rivet?= <gaetan.rivet@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from EUR03-DB5-obe.outbound.protection.outlook.com
 (mail-eopbgr40058.outbound.protection.outlook.com [40.107.4.58])
 by dpdk.org (Postfix) with ESMTP id 45B3C1B03F
 for <dev@dpdk.org>; Wed, 20 Dec 2017 11:58:30 +0100 (CET)
In-Reply-To: <20171219222131.plcfn5wqggyn5znw@bidouze.vm.6wind.com>
Content-Language: en-US
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Gaetan

> -----Original Message-----
> From: Ga=EBtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Wednesday, December 20, 2017 12:22 AM
> To: Matan Azrad <matan@mellanox.com>
> Cc: Adrien Mazarguil <adrien.mazarguil@6wind.com>; Thomas Monjalon
> <thomas@monjalon.net>; dev@dpdk.org
> Subject: Re: [PATCH v3 6/6] net/failsafe: fix removed device handling
>=20
> Hi Matan,
>=20
> 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 <matan@mellanox.com>
> > ---
>=20
> <snip>
>=20
> > +/*
> > + * 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 =3D=3D 0 || sdev->remove =3D=3D 1 || err =3D=3D -EIO)
> > +		return false;
> > +	return true;
> > +}
>=20
> This is better, thanks.
>=20
> 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.
>=20
> What do you think?
>=20

I think that there is a real different between eth_err function to fs_is_er=
ror:
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 awa=
re of device removal  -  this is the main idea that also causes me to chang=
e the name from fs_is_removed to fs_is_error.

> --
> Ga=EBtan Rivet
> 6WIND