diff mbox

[dpdk-dev,v6,4/6] ethdev: adjust APIs removal error report

Message ID 1516274834-19755-5-git-send-email-matan@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Matan Azrad Jan. 18, 2018, 11:27 a.m. UTC 
  rte_eth_dev_is_removed API was added to detect a device removal
synchronously.

When a device removal occurs during control command execution, many
different errors can be reported to the user.

Adjust all ethdev APIs error reports to return -EIO in case of device
removal using rte_eth_dev_is_removed API.

Signed-off-by: Matan Azrad <matan@mellanox.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_ether/rte_ethdev.c | 192 +++++++++++++++++++++++++++---------------
 lib/librte_ether/rte_ethdev.h |  51 ++++++++++-
 2 files changed, 170 insertions(+), 73 deletions(-)

Comments

Ferruh Yigit Jan. 18, 2018, 5:31 p.m. UTC | #1 
On 1/18/2018 11:27 AM, Matan Azrad wrote:
> rte_eth_dev_is_removed API was added to detect a device removal
> synchronously.
> 
> When a device removal occurs during control command execution, many
> different errors can be reported to the user.
> 
> Adjust all ethdev APIs error reports to return -EIO in case of device
> removal using rte_eth_dev_is_removed API.
> 
> Signed-off-by: Matan Azrad <matan@mellanox.com>
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  lib/librte_ether/rte_ethdev.c | 192 +++++++++++++++++++++++++++---------------
>  lib/librte_ether/rte_ethdev.h |  51 ++++++++++-
>  2 files changed, 170 insertions(+), 73 deletions(-)
> 
> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> index c93cec1..7044159 100644
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -338,6 +338,16 @@ struct rte_eth_dev *
>  	return -ENODEV;
>  }
>  
> +static int
> +eth_err(uint16_t port_id, int ret)
> +{
> +	if (ret == 0)
> +		return 0;
> +	if (rte_eth_dev_is_removed(port_id))
> +		return -EIO;
> +	return ret;
> +}
> +
>  /* attach the new device, then store port_id of the device */
>  int
>  rte_eth_dev_attach(const char *devargs, uint16_t *port_id)
> @@ -492,7 +502,8 @@ struct rte_eth_dev *
>  		return 0;
>  	}
>  
> -	return dev->dev_ops->rx_queue_start(dev, rx_queue_id);
> +	return eth_err(port_id, dev->dev_ops->rx_queue_start(dev,
> +							     rx_queue_id));
>  
>  }

This patch updates *all* ethdev public APIs to add if device is removed check?
And each check goes to ethdev is_removed() dev_ops to ask if dev is removed.
These must be better way of doing this, am I missing something.

I definitely would like to see more comments for this patch.

Another question is what happens if device removed while or before dev_ops
called? There is no synchronizations in drivers for removal, right?

<...>
Matan Azrad Jan. 18, 2018, 6:10 p.m. UTC | #2 
Hi Ferruh

From: Ferruh Yigit, Thursday, January 18, 2018 7:31 PM

> On 1/18/2018 11:27 AM, Matan Azrad wrote:

> > rte_eth_dev_is_removed API was added to detect a device removal

> > synchronously.

> >

> > When a device removal occurs during control command execution, many

> > different errors can be reported to the user.

> >

> > Adjust all ethdev APIs error reports to return -EIO in case of device

> > removal using rte_eth_dev_is_removed API.

> >

> > Signed-off-by: Matan Azrad <matan@mellanox.com>

> > Acked-by: Thomas Monjalon <thomas@monjalon.net>

> > ---

> >  lib/librte_ether/rte_ethdev.c | 192

> > +++++++++++++++++++++++++++---------------

> >  lib/librte_ether/rte_ethdev.h |  51 ++++++++++-

> >  2 files changed, 170 insertions(+), 73 deletions(-)

> >

> > diff --git a/lib/librte_ether/rte_ethdev.c

> > b/lib/librte_ether/rte_ethdev.c index c93cec1..7044159 100644

> > --- a/lib/librte_ether/rte_ethdev.c

> > +++ b/lib/librte_ether/rte_ethdev.c

> > @@ -338,6 +338,16 @@ struct rte_eth_dev *

> >  	return -ENODEV;

> >  }

> >

> > +static int

> > +eth_err(uint16_t port_id, int ret)

> > +{

> > +	if (ret == 0)

> > +		return 0;

> > +	if (rte_eth_dev_is_removed(port_id))

> > +		return -EIO;

> > +	return ret;

> > +}

> > +

> >  /* attach the new device, then store port_id of the device */  int

> > rte_eth_dev_attach(const char *devargs, uint16_t *port_id) @@ -492,7

> > +502,8 @@ struct rte_eth_dev *

> >  		return 0;

> >  	}

> >

> > -	return dev->dev_ops->rx_queue_start(dev, rx_queue_id);

> > +	return eth_err(port_id, dev->dev_ops->rx_queue_start(dev,

> > +							     rx_queue_id));

> >

> >  }

> 

> This patch updates *all* ethdev public APIs to add if device is removed

> check?


Yes.

> And each check goes to ethdev is_removed() dev_ops to ask if dev is

> removed.

Probably, if the REMOVED state setted in will not call device is_remove.

> These must be better way of doing this, am I missing something.


Suggest.

This code will replace similar code in each PMD.

> I definitely would like to see more comments for this patch.

> 

> Another question is what happens if device removed while or before

> dev_ops called? There is no synchronizations in drivers for removal, right?

> 


Yes. You right, the device removal can be changed a moment after the call.
Actually the caller suspected in removal before call it(and want to validate it) - so it makes sense. 
From this reason the check in ethdev APIs is called generally in error flows.


> <...>
Ferruh Yigit Jan. 19, 2018, 4:19 p.m. UTC | #3 
On 1/18/2018 6:10 PM, Matan Azrad wrote:
> Hi Ferruh
> 
> From: Ferruh Yigit, Thursday, January 18, 2018 7:31 PM
>> On 1/18/2018 11:27 AM, Matan Azrad wrote:
>>> rte_eth_dev_is_removed API was added to detect a device removal
>>> synchronously.
>>>
>>> When a device removal occurs during control command execution, many
>>> different errors can be reported to the user.
>>>
>>> Adjust all ethdev APIs error reports to return -EIO in case of device
>>> removal using rte_eth_dev_is_removed API.
>>>
>>> Signed-off-by: Matan Azrad <matan@mellanox.com>
>>> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>>> ---
>>>  lib/librte_ether/rte_ethdev.c | 192
>>> +++++++++++++++++++++++++++---------------
>>>  lib/librte_ether/rte_ethdev.h |  51 ++++++++++-
>>>  2 files changed, 170 insertions(+), 73 deletions(-)
>>>
>>> diff --git a/lib/librte_ether/rte_ethdev.c
>>> b/lib/librte_ether/rte_ethdev.c index c93cec1..7044159 100644
>>> --- a/lib/librte_ether/rte_ethdev.c
>>> +++ b/lib/librte_ether/rte_ethdev.c
>>> @@ -338,6 +338,16 @@ struct rte_eth_dev *
>>>  	return -ENODEV;
>>>  }
>>>
>>> +static int
>>> +eth_err(uint16_t port_id, int ret)
>>> +{
>>> +	if (ret == 0)
>>> +		return 0;
>>> +	if (rte_eth_dev_is_removed(port_id))
>>> +		return -EIO;
>>> +	return ret;
>>> +}
>>> +
>>>  /* attach the new device, then store port_id of the device */  int
>>> rte_eth_dev_attach(const char *devargs, uint16_t *port_id) @@ -492,7
>>> +502,8 @@ struct rte_eth_dev *
>>>  		return 0;
>>>  	}
>>>
>>> -	return dev->dev_ops->rx_queue_start(dev, rx_queue_id);
>>> +	return eth_err(port_id, dev->dev_ops->rx_queue_start(dev,
>>> +							     rx_queue_id));
>>>
>>>  }
>>
>> This patch updates *all* ethdev public APIs to add if device is removed
>> check?
> 
> Yes.
> 
>> And each check goes to ethdev is_removed() dev_ops to ask if dev is
>> removed.
> Probably, if the REMOVED state setted in will not call device is_remove.
> 
>> These must be better way of doing this, am I missing something.
> 
> Suggest.

With a silly analogy, this is like a blind person asking each time if he is dead
before talking to other person.

At first glance I can think of a kind of watchdog timer can be implemented in
ethdev layer. It provides periodic checks and if device is dead it calls the
registered user callback function.

This method presented as synchronous method but not triggered from side where
event happens, I mean not triggered from PMD but from application.
So does application doing polling continuously if device is dead?
Or if application is relying this patch to add a check in each API, what happens
if device removed during data processing, will app rely on asynchronous method?

I am including a few consumers of the ethdev to the mail thread, clearly I am
not very supportive of this patch, but specially taking release is being close
to the account, if there is no objection than me I will take as consensus to get
the patch in.

> 
> This code will replace similar code in each PMD.
> 
>> I definitely would like to see more comments for this patch.
>>
>> Another question is what happens if device removed while or before
>> dev_ops called? There is no synchronizations in drivers for removal, right?
>>
> 
> Yes. You right, the device removal can be changed a moment after the call.
> Actually the caller suspected in removal before call it(and want to validate it) - so it makes sense. 
> From this reason the check in ethdev APIs is called generally in error flows.
> 
> 
>> <...>
Ananyev, Konstantin Jan. 19, 2018, 5:35 p.m. UTC | #4 
> -----Original Message-----

> From: Yigit, Ferruh

> Sent: Friday, January 19, 2018 4:19 PM

> To: Matan Azrad <matan@mellanox.com>; Adrien Mazarguil <adrien.mazarguil@6wind.com>; Gaetan Rivet <gaetan.rivet@6wind.com>

> Cc: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org; Andrew Rybchenko <arybchenko@solarflare.com>; Ananyev, Konstantin

> <konstantin.ananyev@intel.com>; Alejandro Lucero <alejandro.lucero@netronome.com>; Jerin Jacob

> <jerin.jacob@caviumnetworks.com>; Hemant Agrawal <hemant.agrawal@nxp.com>; Shahaf Shuler <shahafs@mellanox.com>; Adrien

> Mazarguil <adrien.mazarguil@6wind.com>; Olivier MATZ <olivier.matz@6wind.com>; Zhang, Helin <helin.zhang@intel.com>

> Subject: Re: [PATCH v6 4/6] ethdev: adjust APIs removal error report

> 

> On 1/18/2018 6:10 PM, Matan Azrad wrote:

> > Hi Ferruh

> >

> > From: Ferruh Yigit, Thursday, January 18, 2018 7:31 PM

> >> On 1/18/2018 11:27 AM, Matan Azrad wrote:

> >>> rte_eth_dev_is_removed API was added to detect a device removal

> >>> synchronously.

> >>>

> >>> When a device removal occurs during control command execution, many

> >>> different errors can be reported to the user.

> >>>

> >>> Adjust all ethdev APIs error reports to return -EIO in case of device

> >>> removal using rte_eth_dev_is_removed API.

> >>>

> >>> Signed-off-by: Matan Azrad <matan@mellanox.com>

> >>> Acked-by: Thomas Monjalon <thomas@monjalon.net>

> >>> ---

> >>>  lib/librte_ether/rte_ethdev.c | 192

> >>> +++++++++++++++++++++++++++---------------

> >>>  lib/librte_ether/rte_ethdev.h |  51 ++++++++++-

> >>>  2 files changed, 170 insertions(+), 73 deletions(-)

> >>>

> >>> diff --git a/lib/librte_ether/rte_ethdev.c

> >>> b/lib/librte_ether/rte_ethdev.c index c93cec1..7044159 100644

> >>> --- a/lib/librte_ether/rte_ethdev.c

> >>> +++ b/lib/librte_ether/rte_ethdev.c

> >>> @@ -338,6 +338,16 @@ struct rte_eth_dev *

> >>>  	return -ENODEV;

> >>>  }

> >>>

> >>> +static int

> >>> +eth_err(uint16_t port_id, int ret)

> >>> +{

> >>> +	if (ret == 0)

> >>> +		return 0;

> >>> +	if (rte_eth_dev_is_removed(port_id))

> >>> +		return -EIO;

> >>> +	return ret;

> >>> +}

> >>> +

> >>>  /* attach the new device, then store port_id of the device */  int

> >>> rte_eth_dev_attach(const char *devargs, uint16_t *port_id) @@ -492,7

> >>> +502,8 @@ struct rte_eth_dev *

> >>>  		return 0;

> >>>  	}

> >>>

> >>> -	return dev->dev_ops->rx_queue_start(dev, rx_queue_id);

> >>> +	return eth_err(port_id, dev->dev_ops->rx_queue_start(dev,

> >>> +							     rx_queue_id));

> >>>

> >>>  }

> >>

> >> This patch updates *all* ethdev public APIs to add if device is removed

> >> check?

> >

> > Yes.

> >

> >> And each check goes to ethdev is_removed() dev_ops to ask if dev is

> >> removed.

> > Probably, if the REMOVED state setted in will not call device is_remove.

> >

> >> These must be better way of doing this, am I missing something.

> >

> > Suggest.

> 

> With a silly analogy, this is like a blind person asking each time if he is dead

> before talking to other person.


I am agree with Ferruh that it looks a bit clumsy...
Though I don't have any bright ideas here too.
As I can see right now is_removed() is implemented only for mlx PMD.
Would I make sense to hide that check inside mlx implementation then?
BTW:


 int
+rte_eth_dev_is_removed(uint16_t port_id)
+{
+	struct rte_eth_dev *dev;
+	int ret;
+
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
+
+	dev = &rte_eth_devices[port_id];
+
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->is_removed, 0);

I'd says these 2 checks have to be swapped.
Konstantin

+
+	if (dev->state == RTE_ETH_DEV_REMOVED)
+		return 1;
+


> 

> At first glance I can think of a kind of watchdog timer can be implemented in

> ethdev layer. It provides periodic checks and if device is dead it calls the

> registered user callback function.

> 

> This method presented as synchronous method but not triggered from side where

> event happens, I mean not triggered from PMD but from application.

> So does application doing polling continuously if device is dead?

> Or if application is relying this patch to add a check in each API, what happens

> if device removed during data processing, will app rely on asynchronous method?

> 

> I am including a few consumers of the ethdev to the mail thread, clearly I am

> not very supportive of this patch, but specially taking release is being close

> to the account, if there is no objection than me I will take as consensus to get

> the patch in.

> 

> >

> > This code will replace similar code in each PMD.

> >

> >> I definitely would like to see more comments for this patch.

> >>

> >> Another question is what happens if device removed while or before

> >> dev_ops called? There is no synchronizations in drivers for removal, right?

> >>

> >

> > Yes. You right, the device removal can be changed a moment after the call.

> > Actually the caller suspected in removal before call it(and want to validate it) - so it makes sense.

> > From this reason the check in ethdev APIs is called generally in error flows.

> >

> >

> >> <...>
Thomas Monjalon Jan. 19, 2018, 5:54 p.m. UTC | #5 
19/01/2018 17:19, Ferruh Yigit:
> On 1/18/2018 6:10 PM, Matan Azrad wrote:
> > From: Ferruh Yigit, Thursday, January 18, 2018 7:31 PM
> >> This patch updates *all* ethdev public APIs to add if device is removed
> >> check?
> > 
> > Yes.
> > 
> >> And each check goes to ethdev is_removed() dev_ops to ask if dev is
> >> removed.
> > Probably, if the REMOVED state setted in will not call device is_remove.
> > 
> >> These must be better way of doing this, am I missing something.
> > 
> > Suggest.
> 
> With a silly analogy, this is like a blind person asking each time if he is dead
> before talking to other person.
> 
> At first glance I can think of a kind of watchdog timer can be implemented in
> ethdev layer. It provides periodic checks and if device is dead it calls the
> registered user callback function.
> 
> This method presented as synchronous method but not triggered from side where
> event happens, I mean not triggered from PMD but from application.
> So does application doing polling continuously if device is dead?
> Or if application is relying this patch to add a check in each API, what happens
> if device removed during data processing, will app rely on asynchronous method?

We cannot put a mutex on hardware removal :)
So we have to live with errors due to removal.
If we are trying to configure a removed device,
the error will be not related to the root cause.
This patch is just trying to improve the situation by returning
an appropriate error code if removal can be detected.
Note: the check is run only if there is an error
and if the removal is not already detected.

If I understand well, you prefer relying only on asynchronous
hotplug events? Even if they come really late?
Ferruh Yigit Jan. 19, 2018, 6:13 p.m. UTC | #6 
On 1/19/2018 5:54 PM, Thomas Monjalon wrote:
> 19/01/2018 17:19, Ferruh Yigit:
>> On 1/18/2018 6:10 PM, Matan Azrad wrote:
>>> From: Ferruh Yigit, Thursday, January 18, 2018 7:31 PM
>>>> This patch updates *all* ethdev public APIs to add if device is removed
>>>> check?
>>>
>>> Yes.
>>>
>>>> And each check goes to ethdev is_removed() dev_ops to ask if dev is
>>>> removed.
>>> Probably, if the REMOVED state setted in will not call device is_remove.
>>>
>>>> These must be better way of doing this, am I missing something.
>>>
>>> Suggest.
>>
>> With a silly analogy, this is like a blind person asking each time if he is dead
>> before talking to other person.
>>
>> At first glance I can think of a kind of watchdog timer can be implemented in
>> ethdev layer. It provides periodic checks and if device is dead it calls the
>> registered user callback function.
>>
>> This method presented as synchronous method but not triggered from side where
>> event happens, I mean not triggered from PMD but from application.
>> So does application doing polling continuously if device is dead?
>> Or if application is relying this patch to add a check in each API, what happens
>> if device removed during data processing, will app rely on asynchronous method?
> 
> We cannot put a mutex on hardware removal :)
> So we have to live with errors du to removal.
> If we are trying to configure a removed device,
> the error will be not related to the root cause.
> This patch is just trying to improve the situation by returning
> an appropriate error code if removal can be detected.
> Note: the check is run only if there is an error
> and if the removal is not already detected.
> 
> If I understand well, you prefer relying only on asynchronous
> hotplug events? Even if they come really late?

I think asynchronous hotplug events are better approach, but if they are late I
understand you need a solution.

I assume issue is when device is removed, application can make API calls until
it detects the removal.

For that case what do you think instead of ethdev abstraction layer doing a
translation, define a specific error type and return it from PMDs to indicate
that error is related to the missing device and application will know device is
no more there? And consistently use that error type in all PMDs.

Or what do you think above suggested flexible watchdog timer implementation in
ethdev, so applications may be informed about missing device faster.
Thomas Monjalon Jan. 19, 2018, 6:16 p.m. UTC | #7 
19/01/2018 19:13, Ferruh Yigit:
> On 1/19/2018 5:54 PM, Thomas Monjalon wrote:
> > 19/01/2018 17:19, Ferruh Yigit:
> >> On 1/18/2018 6:10 PM, Matan Azrad wrote:
> >>> From: Ferruh Yigit, Thursday, January 18, 2018 7:31 PM
> >>>> This patch updates *all* ethdev public APIs to add if device is removed
> >>>> check?
> >>>
> >>> Yes.
> >>>
> >>>> And each check goes to ethdev is_removed() dev_ops to ask if dev is
> >>>> removed.
> >>> Probably, if the REMOVED state setted in will not call device is_remove.
> >>>
> >>>> These must be better way of doing this, am I missing something.
> >>>
> >>> Suggest.
> >>
> >> With a silly analogy, this is like a blind person asking each time if he is dead
> >> before talking to other person.
> >>
> >> At first glance I can think of a kind of watchdog timer can be implemented in
> >> ethdev layer. It provides periodic checks and if device is dead it calls the
> >> registered user callback function.
> >>
> >> This method presented as synchronous method but not triggered from side where
> >> event happens, I mean not triggered from PMD but from application.
> >> So does application doing polling continuously if device is dead?
> >> Or if application is relying this patch to add a check in each API, what happens
> >> if device removed during data processing, will app rely on asynchronous method?
> > 
> > We cannot put a mutex on hardware removal :)
> > So we have to live with errors du to removal.
> > If we are trying to configure a removed device,
> > the error will be not related to the root cause.
> > This patch is just trying to improve the situation by returning
> > an appropriate error code if removal can be detected.
> > Note: the check is run only if there is an error
> > and if the removal is not already detected.
> > 
> > If I understand well, you prefer relying only on asynchronous
> > hotplug events? Even if they come really late?
> 
> I think asynchronous hotplug events are better approach, but if they are late I
> understand you need a solution.
> 
> I assume issue is when device is removed, application can make API calls until
> it detects the removal.
> 
> For that case what do you think instead of ethdev abstraction layer doing a
> translation, define a specific error type and return it from PMDs to indicate
> that error is related to the missing device and application will know device is
> no more there? And consistently use that error type in all PMDs.

Yes it is also a good solution.
I think Matan proposed to integrate it in ethdev to avoid duplicating
the same mechanism in every PMDs.

> Or what do you think above suggested flexible watchdog timer implementation in
> ethdev, so applications may be informed about missing device faster.

I think the watchdog would compete with hotplug events.
So I prefer not integrating one more asynchronous mechanism
for the same purpose.
If we want polling, it can be an option to enable in EAL hotplug.
Matan Azrad Jan. 20, 2018, 7:04 p.m. UTC | #8 
Hi all

From: Thomas Monjalon, Friday, January 19, 2018 8:17 PM
> 19/01/2018 19:13, Ferruh Yigit:
> > On 1/19/2018 5:54 PM, Thomas Monjalon wrote:
> > > 19/01/2018 17:19, Ferruh Yigit:
> > >> On 1/18/2018 6:10 PM, Matan Azrad wrote:
> > >>> From: Ferruh Yigit, Thursday, January 18, 2018 7:31 PM
> > >>>> This patch updates *all* ethdev public APIs to add if device is
> > >>>> removed check?
> > >>>
> > >>> Yes.
> > >>>
> > >>>> And each check goes to ethdev is_removed() dev_ops to ask if dev
> > >>>> is removed.
> > >>> Probably, if the REMOVED state setted in will not call device
> is_remove.
> > >>>
> > >>>> These must be better way of doing this, am I missing something.
> > >>>
> > >>> Suggest.
> > >>
> > >> With a silly analogy, this is like a blind person asking each time
> > >> if he is dead before talking to other person.

Just to accurate the analogy:)
This is like a blind person(application,ethdev) using its guide dog(ethdev device), every time the dog refuses to take action (error occurred), the blind person asks if the dog can be a guide dog anymore(removal error). 

> > >> At first glance I can think of a kind of watchdog timer can be
> > >> implemented in ethdev layer. It provides periodic checks and if
> > >> device is dead it calls the registered user callback function.
> > >>
> > >> This method presented as synchronous method but not triggered from
> > >> side where event happens, I mean not triggered from PMD but from
> application.
> > >> So does application doing polling continuously if device is dead?
> > >> Or if application is relying this patch to add a check in each API,
> > >> what happens if device removed during data processing, will app rely on
> asynchronous method?
> > >
> > > We cannot put a mutex on hardware removal :) So we have to live with
> > > errors du to removal.
> > > If we are trying to configure a removed device, the error will be
> > > not related to the root cause.
> > > This patch is just trying to improve the situation by returning an
> > > appropriate error code if removal can be detected.
> > > Note: the check is run only if there is an error and if the removal
> > > is not already detected.
> > >
> > > If I understand well, you prefer relying only on asynchronous
> > > hotplug events? Even if they come really late?
> >
> > I think asynchronous hotplug events are better approach, but if they
> > are late I understand you need a solution.
> >
> > I assume issue is when device is removed, application can make API
> > calls until it detects the removal.
> >
> > For that case what do you think instead of ethdev abstraction layer
> > doing a translation, define a specific error type and return it from
> > PMDs to indicate that error is related to the missing device and
> > application will know device is no more there? And consistently use that
> error type in all PMDs.
> 
> Yes it is also a good solution.
> I think Matan proposed to integrate it in ethdev to avoid duplicating the
> same mechanism in every PMDs.
> 

Yes, as a lot of ethdev API code pieces do.

> > Or what do you think above suggested flexible watchdog timer
> > implementation in ethdev, so applications may be informed about missing
> device faster.
> 
> I think the watchdog would compete with hotplug events.
> So I prefer not integrating one more asynchronous mechanism for the same
> purpose.
> If we want polling, it can be an option to enable in EAL hotplug.

Konstantin wrote in another thread:
>+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
>+
>+	dev = &rte_eth_devices[port_id];
>+
>+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->is_removed, 0);

> I'd says these 2 checks have to be swapped.

Konstantin, Please explain why.
Thomas Monjalon Jan. 20, 2018, 8:28 p.m. UTC | #9 
20/01/2018 20:04, Matan Azrad:
> Konstantin wrote in another thread:
> >+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
> >+
> >+	dev = &rte_eth_devices[port_id];
> >+
> >+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->is_removed, 0);
> 
> > I'd says these 2 checks have to be swapped.
> 
> Konstantin, Please explain why.

I think he was talking about these 2 tests:

+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->is_removed, 0);
+	if (dev->state == RTE_ETH_DEV_REMOVED)
+		return 1;
Matan Azrad Jan. 20, 2018, 8:45 p.m. UTC | #10 
Hi Thomas
From: Thomas Monjalon, Saturday, January 20, 2018 10:29 PM
> 20/01/2018 20:04, Matan Azrad:
> > Konstantin wrote in another thread:
> > >+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, 0);
> > >+
> > >+	dev = &rte_eth_devices[port_id];
> > >+
> > >+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->is_removed, 0);
> >
> > > I'd says these 2 checks have to be swapped.
> >
> > Konstantin, Please explain why.
> 
> I think he was talking about these 2 tests:
> 
> +	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->is_removed, 0);
> +	if (dev->state == RTE_ETH_DEV_REMOVED)
> +		return 1;

Ahh yes, it makes sense, I will swap them.

Thanks.
Ferruh Yigit Jan. 21, 2018, 8:07 p.m. UTC | #11 
On 1/19/2018 4:19 PM, Ferruh Yigit wrote:
> On 1/18/2018 6:10 PM, Matan Azrad wrote:
>> Hi Ferruh
>>
>> From: Ferruh Yigit, Thursday, January 18, 2018 7:31 PM
>>> On 1/18/2018 11:27 AM, Matan Azrad wrote:
>>>> rte_eth_dev_is_removed API was added to detect a device removal
>>>> synchronously.
>>>>
>>>> When a device removal occurs during control command execution, many
>>>> different errors can be reported to the user.
>>>>
>>>> Adjust all ethdev APIs error reports to return -EIO in case of device
>>>> removal using rte_eth_dev_is_removed API.
>>>>
>>>> Signed-off-by: Matan Azrad <matan@mellanox.com>
>>>> Acked-by: Thomas Monjalon <thomas@monjalon.net>
>>>> ---
>>>>  lib/librte_ether/rte_ethdev.c | 192
>>>> +++++++++++++++++++++++++++---------------
>>>>  lib/librte_ether/rte_ethdev.h |  51 ++++++++++-
>>>>  2 files changed, 170 insertions(+), 73 deletions(-)
>>>>
>>>> diff --git a/lib/librte_ether/rte_ethdev.c
>>>> b/lib/librte_ether/rte_ethdev.c index c93cec1..7044159 100644
>>>> --- a/lib/librte_ether/rte_ethdev.c
>>>> +++ b/lib/librte_ether/rte_ethdev.c
>>>> @@ -338,6 +338,16 @@ struct rte_eth_dev *
>>>>  	return -ENODEV;
>>>>  }
>>>>
>>>> +static int
>>>> +eth_err(uint16_t port_id, int ret)
>>>> +{
>>>> +	if (ret == 0)
>>>> +		return 0;
>>>> +	if (rte_eth_dev_is_removed(port_id))
>>>> +		return -EIO;
>>>> +	return ret;
>>>> +}
>>>> +
>>>>  /* attach the new device, then store port_id of the device */  int
>>>> rte_eth_dev_attach(const char *devargs, uint16_t *port_id) @@ -492,7
>>>> +502,8 @@ struct rte_eth_dev *
>>>>  		return 0;
>>>>  	}
>>>>
>>>> -	return dev->dev_ops->rx_queue_start(dev, rx_queue_id);
>>>> +	return eth_err(port_id, dev->dev_ops->rx_queue_start(dev,
>>>> +							     rx_queue_id));
>>>>
>>>>  }
>>>
>>> This patch updates *all* ethdev public APIs to add if device is removed
>>> check?
>>
>> Yes.
>>
>>> And each check goes to ethdev is_removed() dev_ops to ask if dev is
>>> removed.
>> Probably, if the REMOVED state setted in will not call device is_remove.
>>
>>> These must be better way of doing this, am I missing something.
>>
>> Suggest.
> 
> With a silly analogy, this is like a blind person asking each time if he is dead
> before talking to other person.
> 
> At first glance I can think of a kind of watchdog timer can be implemented in
> ethdev layer. It provides periodic checks and if device is dead it calls the
> registered user callback function.
> 
> This method presented as synchronous method but not triggered from side where
> event happens, I mean not triggered from PMD but from application.
> So does application doing polling continuously if device is dead?
> Or if application is relying this patch to add a check in each API, what happens
> if device removed during data processing, will app rely on asynchronous method?
> 
> I am including a few consumers of the ethdev to the mail thread, clearly I am
> not very supportive of this patch, but specially taking release is being close
> to the account, if there is no objection than me I will take as consensus to get
> the patch in.

It looks like there is no objection to the patch and it is already acked, I will
get latest version to next-net.

> 
>>
>> This code will replace similar code in each PMD.
>>
>>> I definitely would like to see more comments for this patch.
>>>
>>> Another question is what happens if device removed while or before
>>> dev_ops called? There is no synchronizations in drivers for removal, right?
>>>
>>
>> Yes. You right, the device removal can be changed a moment after the call.
>> Actually the caller suspected in removal before call it(and want to validate it) - so it makes sense. 
>> From this reason the check in ethdev APIs is called generally in error flows.
>>
>>
>>> <...>
>
diff mbox

Patch

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index c93cec1..7044159 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -338,6 +338,16 @@  struct rte_eth_dev *
 	return -ENODEV;
 }
 
+static int
+eth_err(uint16_t port_id, int ret)
+{
+	if (ret == 0)
+		return 0;
+	if (rte_eth_dev_is_removed(port_id))
+		return -EIO;
+	return ret;
+}
+
 /* attach the new device, then store port_id of the device */
 int
 rte_eth_dev_attach(const char *devargs, uint16_t *port_id)
@@ -492,7 +502,8 @@  struct rte_eth_dev *
 		return 0;
 	}
 
-	return dev->dev_ops->rx_queue_start(dev, rx_queue_id);
+	return eth_err(port_id, dev->dev_ops->rx_queue_start(dev,
+							     rx_queue_id));
 
 }
 
@@ -518,7 +529,7 @@  struct rte_eth_dev *
 		return 0;
 	}
 
-	return dev->dev_ops->rx_queue_stop(dev, rx_queue_id);
+	return eth_err(port_id, dev->dev_ops->rx_queue_stop(dev, rx_queue_id));
 
 }
 
@@ -544,7 +555,8 @@  struct rte_eth_dev *
 		return 0;
 	}
 
-	return dev->dev_ops->tx_queue_start(dev, tx_queue_id);
+	return eth_err(port_id, dev->dev_ops->tx_queue_start(dev,
+							     tx_queue_id));
 
 }
 
@@ -570,7 +582,7 @@  struct rte_eth_dev *
 		return 0;
 	}
 
-	return dev->dev_ops->tx_queue_stop(dev, tx_queue_id);
+	return eth_err(port_id, dev->dev_ops->tx_queue_stop(dev, tx_queue_id));
 
 }
 
@@ -888,7 +900,7 @@  struct rte_eth_dev *
 				port_id, diag);
 		rte_eth_dev_rx_queue_config(dev, 0);
 		rte_eth_dev_tx_queue_config(dev, 0);
-		return diag;
+		return eth_err(port_id, diag);
 	}
 
 	/* Initialize Rx profiling if enabled at compilation time. */
@@ -898,7 +910,7 @@  struct rte_eth_dev *
 				port_id, diag);
 		rte_eth_dev_rx_queue_config(dev, 0);
 		rte_eth_dev_tx_queue_config(dev, 0);
-		return diag;
+		return eth_err(port_id, diag);
 	}
 
 	return 0;
@@ -998,7 +1010,7 @@  struct rte_eth_dev *
 	if (diag == 0)
 		dev->data->dev_started = 1;
 	else
-		return diag;
+		return eth_err(port_id, diag);
 
 	rte_eth_dev_config_restore(port_id);
 
@@ -1040,7 +1052,7 @@  struct rte_eth_dev *
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_set_link_up, -ENOTSUP);
-	return (*dev->dev_ops->dev_set_link_up)(dev);
+	return eth_err(port_id, (*dev->dev_ops->dev_set_link_up)(dev));
 }
 
 int
@@ -1053,7 +1065,7 @@  struct rte_eth_dev *
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_set_link_down, -ENOTSUP);
-	return (*dev->dev_ops->dev_set_link_down)(dev);
+	return eth_err(port_id, (*dev->dev_ops->dev_set_link_down)(dev));
 }
 
 void
@@ -1090,7 +1102,7 @@  struct rte_eth_dev *
 	rte_eth_dev_stop(port_id);
 	ret = dev->dev_ops->dev_reset(dev);
 
-	return ret;
+	return eth_err(port_id, ret);
 }
 
 int
@@ -1214,7 +1226,7 @@  struct rte_eth_dev *
 			dev->data->min_rx_buf_size = mbp_buf_size;
 	}
 
-	return ret;
+	return eth_err(port_id, ret);
 }
 
 /**
@@ -1333,8 +1345,8 @@  struct rte_eth_dev *
 					  &local_conf.offloads);
 	}
 
-	return (*dev->dev_ops->tx_queue_setup)(dev, tx_queue_id, nb_tx_desc,
-					       socket_id, &local_conf);
+	return eth_err(port_id, (*dev->dev_ops->tx_queue_setup)(dev,
+		       tx_queue_id, nb_tx_desc, socket_id, &local_conf));
 }
 
 void
@@ -1390,14 +1402,16 @@  struct rte_eth_dev *
 rte_eth_tx_done_cleanup(uint16_t port_id, uint16_t queue_id, uint32_t free_cnt)
 {
 	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+	int ret;
 
 	/* Validate Input Data. Bail if not valid or not supported. */
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_done_cleanup, -ENOTSUP);
 
 	/* Call driver to free pending mbufs. */
-	return (*dev->dev_ops->tx_done_cleanup)(dev->data->tx_queues[queue_id],
-			free_cnt);
+	ret = (*dev->dev_ops->tx_done_cleanup)(dev->data->tx_queues[queue_id],
+					       free_cnt);
+	return eth_err(port_id, ret);
 }
 
 void
@@ -1534,7 +1548,7 @@  struct rte_eth_dev *
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->stats_get, -ENOTSUP);
 	stats->rx_nombuf = dev->data->rx_mbuf_alloc_failed;
-	return (*dev->dev_ops->stats_get)(dev, stats);
+	return eth_err(port_id, (*dev->dev_ops->stats_get)(dev, stats));
 }
 
 int
@@ -1580,12 +1594,12 @@  struct rte_eth_dev *
 		count = (*dev->dev_ops->xstats_get_names_by_id)(dev, NULL,
 				NULL, 0);
 		if (count < 0)
-			return count;
+			return eth_err(port_id, count);
 	}
 	if (dev->dev_ops->xstats_get_names != NULL) {
 		count = (*dev->dev_ops->xstats_get_names)(dev, NULL, 0);
 		if (count < 0)
-			return count;
+			return eth_err(port_id, count);
 	} else
 		count = 0;
 
@@ -1765,8 +1779,12 @@  struct rte_eth_dev *
 	if (ids && no_ext_stat_requested) {
 		rte_eth_basic_stats_get_names(dev, xstats_names_copy);
 	} else {
-		rte_eth_xstats_get_names(port_id, xstats_names_copy,
+		ret = rte_eth_xstats_get_names(port_id, xstats_names_copy,
 			expected_entries);
+		if (ret < 0) {
+			free(xstats_names_copy);
+			return ret;
+		}
 	}
 
 	/* Filter stats */
@@ -1813,7 +1831,7 @@  struct rte_eth_dev *
 			xstats_names + cnt_used_entries,
 			size - cnt_used_entries);
 		if (cnt_driver_entries < 0)
-			return cnt_driver_entries;
+			return eth_err(port_id, cnt_driver_entries);
 		cnt_used_entries += cnt_driver_entries;
 	}
 
@@ -1829,8 +1847,12 @@  struct rte_eth_dev *
 	unsigned int count = 0, i, q;
 	uint64_t val, *stats_ptr;
 	uint16_t nb_rxqs, nb_txqs;
+	int ret;
+
+	ret = rte_eth_stats_get(port_id, &eth_stats);
+	if (ret < 0)
+		return ret;
 
-	rte_eth_stats_get(port_id, &eth_stats);
 	dev = &rte_eth_devices[port_id];
 
 	nb_rxqs = RTE_MIN(dev->data->nb_rx_queues, RTE_ETHDEV_QUEUE_STAT_CNTRS);
@@ -1883,7 +1905,10 @@  struct rte_eth_dev *
 	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
-	expected_entries = get_xstats_count(port_id);
+	ret = get_xstats_count(port_id);
+	if (ret < 0)
+		return ret;
+	expected_entries = (uint16_t)ret;
 	struct rte_eth_xstat xstats[expected_entries];
 	dev = &rte_eth_devices[port_id];
 	basic_count = get_xstats_basic_count(dev);
@@ -1966,6 +1991,7 @@  struct rte_eth_dev *
 	unsigned int count = 0, i;
 	signed int xcount = 0;
 	uint16_t nb_rxqs, nb_txqs;
+	int ret;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
@@ -1988,14 +2014,17 @@  struct rte_eth_dev *
 				     (n > count) ? n - count : 0);
 
 		if (xcount < 0)
-			return xcount;
+			return eth_err(port_id, xcount);
 	}
 
 	if (n < count + xcount || xstats == NULL)
 		return count + xcount;
 
 	/* now fill the xstats structure */
-	count = rte_eth_basic_stats_get(port_id, xstats);
+	ret = rte_eth_basic_stats_get(port_id, xstats);
+	if (ret < 0)
+		return ret;
+	count = ret;
 
 	for (i = 0; i < count; i++)
 		xstats[i].id = i;
@@ -2045,8 +2074,8 @@  struct rte_eth_dev *
 rte_eth_dev_set_tx_queue_stats_mapping(uint16_t port_id, uint16_t tx_queue_id,
 		uint8_t stat_idx)
 {
-	return set_queue_stats_mapping(port_id, tx_queue_id, stat_idx,
-			STAT_QMAP_TX);
+	return eth_err(port_id, set_queue_stats_mapping(port_id, tx_queue_id,
+						stat_idx, STAT_QMAP_TX));
 }
 
 
@@ -2054,8 +2083,8 @@  struct rte_eth_dev *
 rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id, uint16_t rx_queue_id,
 		uint8_t stat_idx)
 {
-	return set_queue_stats_mapping(port_id, rx_queue_id, stat_idx,
-			STAT_QMAP_RX);
+	return eth_err(port_id, set_queue_stats_mapping(port_id, rx_queue_id,
+						stat_idx, STAT_QMAP_RX));
 }
 
 int
@@ -2067,7 +2096,8 @@  struct rte_eth_dev *
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->fw_version_get, -ENOTSUP);
-	return (*dev->dev_ops->fw_version_get)(dev, fw_version, fw_size);
+	return eth_err(port_id, (*dev->dev_ops->fw_version_get)(dev,
+							fw_version, fw_size));
 }
 
 void
@@ -2157,7 +2187,7 @@  struct rte_eth_dev *
 	if (!ret)
 		dev->data->mtu = mtu;
 
-	return ret;
+	return eth_err(port_id, ret);
 }
 
 int
@@ -2197,7 +2227,7 @@  struct rte_eth_dev *
 			vfc->ids[vidx] &= ~(UINT64_C(1) << vbit);
 	}
 
-	return ret;
+	return eth_err(port_id, ret);
 }
 
 int
@@ -2230,7 +2260,8 @@  struct rte_eth_dev *
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_tpid_set, -ENOTSUP);
 
-	return (*dev->dev_ops->vlan_tpid_set)(dev, vlan_type, tpid);
+	return eth_err(port_id, (*dev->dev_ops->vlan_tpid_set)(dev, vlan_type,
+							       tpid));
 }
 
 int
@@ -2308,7 +2339,7 @@  struct rte_eth_dev *
 					    &dev->data->dev_conf.rxmode);
 	}
 
-	return ret;
+	return eth_err(port_id, ret);
 }
 
 int
@@ -2343,9 +2374,8 @@  struct rte_eth_dev *
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->vlan_pvid_set, -ENOTSUP);
-	(*dev->dev_ops->vlan_pvid_set)(dev, pvid, on);
 
-	return 0;
+	return eth_err(port_id, (*dev->dev_ops->vlan_pvid_set)(dev, pvid, on));
 }
 
 int
@@ -2357,7 +2387,7 @@  struct rte_eth_dev *
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->flow_ctrl_get, -ENOTSUP);
 	memset(fc_conf, 0, sizeof(*fc_conf));
-	return (*dev->dev_ops->flow_ctrl_get)(dev, fc_conf);
+	return eth_err(port_id, (*dev->dev_ops->flow_ctrl_get)(dev, fc_conf));
 }
 
 int
@@ -2373,7 +2403,7 @@  struct rte_eth_dev *
 
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->flow_ctrl_set, -ENOTSUP);
-	return (*dev->dev_ops->flow_ctrl_set)(dev, fc_conf);
+	return eth_err(port_id, (*dev->dev_ops->flow_ctrl_set)(dev, fc_conf));
 }
 
 int
@@ -2391,7 +2421,8 @@  struct rte_eth_dev *
 	dev = &rte_eth_devices[port_id];
 	/* High water, low water validation are device specific */
 	if  (*dev->dev_ops->priority_flow_ctrl_set)
-		return (*dev->dev_ops->priority_flow_ctrl_set)(dev, pfc_conf);
+		return eth_err(port_id, (*dev->dev_ops->priority_flow_ctrl_set)
+					(dev, pfc_conf));
 	return -ENOTSUP;
 }
 
@@ -2466,7 +2497,8 @@  struct rte_eth_dev *
 		return ret;
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->reta_update, -ENOTSUP);
-	return (*dev->dev_ops->reta_update)(dev, reta_conf, reta_size);
+	return eth_err(port_id, (*dev->dev_ops->reta_update)(dev, reta_conf,
+							     reta_size));
 }
 
 int
@@ -2486,7 +2518,8 @@  struct rte_eth_dev *
 
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->reta_query, -ENOTSUP);
-	return (*dev->dev_ops->reta_query)(dev, reta_conf, reta_size);
+	return eth_err(port_id, (*dev->dev_ops->reta_query)(dev, reta_conf,
+							    reta_size));
 }
 
 int
@@ -2498,7 +2531,8 @@  struct rte_eth_dev *
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rss_hash_update, -ENOTSUP);
-	return (*dev->dev_ops->rss_hash_update)(dev, rss_conf);
+	return eth_err(port_id, (*dev->dev_ops->rss_hash_update)(dev,
+								 rss_conf));
 }
 
 int
@@ -2510,7 +2544,8 @@  struct rte_eth_dev *
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rss_hash_conf_get, -ENOTSUP);
-	return (*dev->dev_ops->rss_hash_conf_get)(dev, rss_conf);
+	return eth_err(port_id, (*dev->dev_ops->rss_hash_conf_get)(dev,
+								   rss_conf));
 }
 
 int
@@ -2532,7 +2567,8 @@  struct rte_eth_dev *
 
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->udp_tunnel_port_add, -ENOTSUP);
-	return (*dev->dev_ops->udp_tunnel_port_add)(dev, udp_tunnel);
+	return eth_err(port_id, (*dev->dev_ops->udp_tunnel_port_add)(dev,
+								udp_tunnel));
 }
 
 int
@@ -2555,7 +2591,8 @@  struct rte_eth_dev *
 	}
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->udp_tunnel_port_del, -ENOTSUP);
-	return (*dev->dev_ops->udp_tunnel_port_del)(dev, udp_tunnel);
+	return eth_err(port_id, (*dev->dev_ops->udp_tunnel_port_del)(dev,
+								udp_tunnel));
 }
 
 int
@@ -2566,7 +2603,7 @@  struct rte_eth_dev *
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_led_on, -ENOTSUP);
-	return (*dev->dev_ops->dev_led_on)(dev);
+	return eth_err(port_id, (*dev->dev_ops->dev_led_on)(dev));
 }
 
 int
@@ -2577,7 +2614,7 @@  struct rte_eth_dev *
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->dev_led_off, -ENOTSUP);
-	return (*dev->dev_ops->dev_led_off)(dev);
+	return eth_err(port_id, (*dev->dev_ops->dev_led_off)(dev));
 }
 
 /*
@@ -2653,7 +2690,7 @@  struct rte_eth_dev *
 		dev->data->mac_pool_sel[index] |= (1ULL << pool);
 	}
 
-	return ret;
+	return eth_err(port_id, ret);
 }
 
 int
@@ -2779,7 +2816,7 @@  struct rte_eth_dev *
 					&dev->data->hash_mac_addrs[index]);
 	}
 
-	return ret;
+	return eth_err(port_id, ret);
 }
 
 int
@@ -2792,7 +2829,8 @@  struct rte_eth_dev *
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->uc_all_hash_table_set, -ENOTSUP);
-	return (*dev->dev_ops->uc_all_hash_table_set)(dev, on);
+	return eth_err(port_id, (*dev->dev_ops->uc_all_hash_table_set)(dev,
+								       on));
 }
 
 int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
@@ -2822,7 +2860,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	}
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_queue_rate_limit, -ENOTSUP);
-	return (*dev->dev_ops->set_queue_rate_limit)(dev, queue_idx, tx_rate);
+	return eth_err(port_id, (*dev->dev_ops->set_queue_rate_limit)(dev,
+							queue_idx, tx_rate));
 }
 
 int
@@ -2860,7 +2899,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_set, -ENOTSUP);
 
-	return (*dev->dev_ops->mirror_rule_set)(dev, mirror_conf, rule_id, on);
+	return eth_err(port_id, (*dev->dev_ops->mirror_rule_set)(dev,
+						mirror_conf, rule_id, on));
 }
 
 int
@@ -2873,7 +2913,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->mirror_rule_reset, -ENOTSUP);
 
-	return (*dev->dev_ops->mirror_rule_reset)(dev, rule_id);
+	return eth_err(port_id, (*dev->dev_ops->mirror_rule_reset)(dev,
+								   rule_id));
 }
 
 RTE_INIT(eth_dev_init_cb_lists)
@@ -3137,7 +3178,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_intr_enable, -ENOTSUP);
-	return (*dev->dev_ops->rx_queue_intr_enable)(dev, queue_id);
+	return eth_err(port_id, (*dev->dev_ops->rx_queue_intr_enable)(dev,
+								queue_id));
 }
 
 int
@@ -3151,7 +3193,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_queue_intr_disable, -ENOTSUP);
-	return (*dev->dev_ops->rx_queue_intr_disable)(dev, queue_id);
+	return eth_err(port_id, (*dev->dev_ops->rx_queue_intr_disable)(dev,
+								queue_id));
 }
 
 
@@ -3179,7 +3222,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->filter_ctrl, -ENOTSUP);
-	return (*dev->dev_ops->filter_ctrl)(dev, filter_type, filter_op, arg);
+	return eth_err(port_id, (*dev->dev_ops->filter_ctrl)(dev, filter_type,
+							     filter_op, arg));
 }
 
 void *
@@ -3429,7 +3473,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_mc_addr_list, -ENOTSUP);
-	return dev->dev_ops->set_mc_addr_list(dev, mc_addr_set, nb_mc_addr);
+	return eth_err(port_id, dev->dev_ops->set_mc_addr_list(dev,
+						mc_addr_set, nb_mc_addr));
 }
 
 int
@@ -3441,7 +3486,7 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_enable, -ENOTSUP);
-	return (*dev->dev_ops->timesync_enable)(dev);
+	return eth_err(port_id, (*dev->dev_ops->timesync_enable)(dev));
 }
 
 int
@@ -3453,7 +3498,7 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_disable, -ENOTSUP);
-	return (*dev->dev_ops->timesync_disable)(dev);
+	return eth_err(port_id, (*dev->dev_ops->timesync_disable)(dev));
 }
 
 int
@@ -3466,7 +3511,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_rx_timestamp, -ENOTSUP);
-	return (*dev->dev_ops->timesync_read_rx_timestamp)(dev, timestamp, flags);
+	return eth_err(port_id, (*dev->dev_ops->timesync_read_rx_timestamp)
+				(dev, timestamp, flags));
 }
 
 int
@@ -3479,7 +3525,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_tx_timestamp, -ENOTSUP);
-	return (*dev->dev_ops->timesync_read_tx_timestamp)(dev, timestamp);
+	return eth_err(port_id, (*dev->dev_ops->timesync_read_tx_timestamp)
+				(dev, timestamp));
 }
 
 int
@@ -3491,7 +3538,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_adjust_time, -ENOTSUP);
-	return (*dev->dev_ops->timesync_adjust_time)(dev, delta);
+	return eth_err(port_id, (*dev->dev_ops->timesync_adjust_time)(dev,
+								      delta));
 }
 
 int
@@ -3503,7 +3551,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_read_time, -ENOTSUP);
-	return (*dev->dev_ops->timesync_read_time)(dev, timestamp);
+	return eth_err(port_id, (*dev->dev_ops->timesync_read_time)(dev,
+								timestamp));
 }
 
 int
@@ -3515,7 +3564,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->timesync_write_time, -ENOTSUP);
-	return (*dev->dev_ops->timesync_write_time)(dev, timestamp);
+	return eth_err(port_id, (*dev->dev_ops->timesync_write_time)(dev,
+								timestamp));
 }
 
 int
@@ -3527,7 +3577,7 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_reg, -ENOTSUP);
-	return (*dev->dev_ops->get_reg)(dev, info);
+	return eth_err(port_id, (*dev->dev_ops->get_reg)(dev, info));
 }
 
 int
@@ -3539,7 +3589,7 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_eeprom_length, -ENOTSUP);
-	return (*dev->dev_ops->get_eeprom_length)(dev);
+	return eth_err(port_id, (*dev->dev_ops->get_eeprom_length)(dev));
 }
 
 int
@@ -3551,7 +3601,7 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_eeprom, -ENOTSUP);
-	return (*dev->dev_ops->get_eeprom)(dev, info);
+	return eth_err(port_id, (*dev->dev_ops->get_eeprom)(dev, info));
 }
 
 int
@@ -3563,7 +3613,7 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->set_eeprom, -ENOTSUP);
-	return (*dev->dev_ops->set_eeprom)(dev, info);
+	return eth_err(port_id, (*dev->dev_ops->set_eeprom)(dev, info));
 }
 
 int
@@ -3578,7 +3628,7 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	memset(dcb_info, 0, sizeof(struct rte_eth_dcb_info));
 
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->get_dcb_info, -ENOTSUP);
-	return (*dev->dev_ops->get_dcb_info)(dev, dcb_info);
+	return eth_err(port_id, (*dev->dev_ops->get_dcb_info)(dev, dcb_info));
 }
 
 int
@@ -3601,7 +3651,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->l2_tunnel_eth_type_conf,
 				-ENOTSUP);
-	return (*dev->dev_ops->l2_tunnel_eth_type_conf)(dev, l2_tunnel);
+	return eth_err(port_id, (*dev->dev_ops->l2_tunnel_eth_type_conf)(dev,
+								l2_tunnel));
 }
 
 int
@@ -3632,7 +3683,8 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
 	dev = &rte_eth_devices[port_id];
 	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->l2_tunnel_offload_set,
 				-ENOTSUP);
-	return (*dev->dev_ops->l2_tunnel_offload_set)(dev, l2_tunnel, mask, en);
+	return eth_err(port_id, (*dev->dev_ops->l2_tunnel_offload_set)(dev,
+							l2_tunnel, mask, en));
 }
 
 static void
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 18c14e9..cf4defb 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -2023,6 +2023,7 @@  int rte_eth_dev_configure(uint16_t port_id, uint16_t nb_rx_queue,
  *   memory buffers to populate each descriptor of the receive ring.
  * @return
  *   - 0: Success, receive queue correctly set up.
+ *   - -EIO: if device is removed.
  *   - -EINVAL: The size of network buffers which can be allocated from the
  *      memory pool does not fit the various buffer sizes allowed by the
  *      device controller.
@@ -2123,6 +2124,7 @@  int rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
  * @return
  *   - 0: Success, the receive queue is started.
  *   - -EINVAL: The port_id or the queue_id out of range.
+ *   - -EIO: if device is removed.
  *   - -ENOTSUP: The function not supported in PMD driver.
  */
 int rte_eth_dev_rx_queue_start(uint16_t port_id, uint16_t rx_queue_id);
@@ -2139,6 +2141,7 @@  int rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
  * @return
  *   - 0: Success, the receive queue is stopped.
  *   - -EINVAL: The port_id or the queue_id out of range.
+ *   - -EIO: if device is removed.
  *   - -ENOTSUP: The function not supported in PMD driver.
  */
 int rte_eth_dev_rx_queue_stop(uint16_t port_id, uint16_t rx_queue_id);
@@ -2156,6 +2159,7 @@  int rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
  * @return
  *   - 0: Success, the transmit queue is started.
  *   - -EINVAL: The port_id or the queue_id out of range.
+ *   - -EIO: if device is removed.
  *   - -ENOTSUP: The function not supported in PMD driver.
  */
 int rte_eth_dev_tx_queue_start(uint16_t port_id, uint16_t tx_queue_id);
@@ -2172,6 +2176,7 @@  int rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
  * @return
  *   - 0: Success, the transmit queue is stopped.
  *   - -EINVAL: The port_id or the queue_id out of range.
+ *   - -EIO: if device is removed.
  *   - -ENOTSUP: The function not supported in PMD driver.
  */
 int rte_eth_dev_tx_queue_stop(uint16_t port_id, uint16_t tx_queue_id);
@@ -2273,7 +2278,7 @@  int rte_eth_tx_queue_setup(uint16_t port_id, uint16_t tx_queue_id,
  *   - (-EINVAL) if port identifier is invalid.
  *   - (-ENOTSUP) if hardware doesn't support this function.
  *   - (-EPERM) if not ran from the primary process.
- *   - (-EIO) if re-initialisation failed.
+ *   - (-EIO) if re-initialisation failed or device is removed.
  *   - (-ENOMEM) if the reset failed due to OOM.
  *   - (-EAGAIN) if the reset temporarily failed and should be retried later.
  */
@@ -2509,6 +2514,7 @@  int rte_eth_xstats_get_by_id(uint16_t port_id, const uint64_t *ids,
  * @return
  *    0 on success
  *    -ENODEV for invalid port_id,
+ *    -EIO if device is removed,
  *    -EINVAL if the xstat_name doesn't exist in port_id
  */
 int rte_eth_xstats_get_id_by_name(uint16_t port_id, const char *xstat_name,
@@ -2600,6 +2606,7 @@  int rte_eth_dev_set_rx_queue_stats_mapping(uint16_t port_id,
  *   - (0) if successful.
  *   - (-ENOTSUP) if operation is not supported.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - (>0) if *fw_size* is not enough to store firmware version, return
  *          the size of the non truncated string.
  */
@@ -2671,6 +2678,7 @@  int rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
  *   - (0) if successful.
  *   - (-ENOTSUP) if operation is not supported.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - (-EINVAL) if *mtu* invalid.
  *   - (-EBUSY) if operation is not allowed when the port is running
  */
@@ -2691,6 +2699,7 @@  int rte_eth_dev_get_supported_ptypes(uint16_t port_id, uint32_t ptype_mask,
  *   - (0) if successful.
  *   - (-ENOSUP) if hardware-assisted VLAN filtering not configured.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - (-ENOSYS) if VLAN filtering on *port_id* disabled.
  *   - (-EINVAL) if *vlan_id* > 4095.
  */
@@ -2733,6 +2742,7 @@  int rte_eth_dev_set_vlan_strip_on_queue(uint16_t port_id, uint16_t rx_queue_id,
  *   - (0) if successful.
  *   - (-ENOSUP) if hardware-assisted VLAN TPID setup is not supported.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  */
 int rte_eth_dev_set_vlan_ether_type(uint16_t port_id,
 				    enum rte_vlan_type vlan_type,
@@ -2757,6 +2767,7 @@  int rte_eth_dev_set_vlan_ether_type(uint16_t port_id,
  *   - (0) if successful.
  *   - (-ENOSUP) if hardware-assisted VLAN filtering not configured.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  */
 int rte_eth_dev_set_vlan_offload(uint16_t port_id, int offload_mask);
 
@@ -3498,6 +3509,7 @@  struct rte_eth_dev_tx_buffer {
  * @return
  *   Failure: < 0
  *     -ENODEV: Invalid interface
+ *     -EIO: device is removed
  *     -ENOTSUP: Driver does not support function
  *   Success: >= 0
  *     0-n: Number of packets freed. More packets may still remain in ring that
@@ -3612,6 +3624,7 @@  int _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
  *   - (-ENOTSUP) if underlying hardware OR driver doesn't support
  *     that operation.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  */
 int rte_eth_dev_rx_intr_enable(uint16_t port_id, uint16_t queue_id);
 
@@ -3633,6 +3646,7 @@  int _rte_eth_dev_callback_process(struct rte_eth_dev *dev,
  *   - (-ENOTSUP) if underlying hardware OR driver doesn't support
  *     that operation.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  */
 int rte_eth_dev_rx_intr_disable(uint16_t port_id, uint16_t queue_id);
 
@@ -3690,6 +3704,7 @@  int rte_eth_dev_rx_intr_ctl_q(uint16_t port_id, uint16_t queue_id,
  *   - (-ENOTSUP) if underlying hardware OR driver doesn't support
  *     that operation.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  */
 int  rte_eth_led_on(uint16_t port_id);
 
@@ -3704,6 +3719,7 @@  int rte_eth_dev_rx_intr_ctl_q(uint16_t port_id, uint16_t queue_id,
  *   - (-ENOTSUP) if underlying hardware OR driver doesn't support
  *     that operation.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  */
 int  rte_eth_led_off(uint16_t port_id);
 
@@ -3718,6 +3734,7 @@  int rte_eth_dev_rx_intr_ctl_q(uint16_t port_id, uint16_t queue_id,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support flow control.
  *   - (-ENODEV)  if *port_id* invalid.
+ *   - (-EIO)  if device is removed.
  */
 int rte_eth_dev_flow_ctrl_get(uint16_t port_id,
 			      struct rte_eth_fc_conf *fc_conf);
@@ -3734,7 +3751,7 @@  int rte_eth_dev_flow_ctrl_get(uint16_t port_id,
  *   - (-ENOTSUP) if hardware doesn't support flow control mode.
  *   - (-ENODEV)  if *port_id* invalid.
  *   - (-EINVAL)  if bad parameter
- *   - (-EIO)     if flow control setup failure
+ *   - (-EIO)     if flow control setup failure or device is removed.
  */
 int rte_eth_dev_flow_ctrl_set(uint16_t port_id,
 			      struct rte_eth_fc_conf *fc_conf);
@@ -3752,7 +3769,7 @@  int rte_eth_dev_flow_ctrl_set(uint16_t port_id,
  *   - (-ENOTSUP) if hardware doesn't support priority flow control mode.
  *   - (-ENODEV)  if *port_id* invalid.
  *   - (-EINVAL)  if bad parameter
- *   - (-EIO)     if flow control setup failure
+ *   - (-EIO)     if flow control setup failure or device is removed.
  */
 int rte_eth_dev_priority_flow_ctrl_set(uint16_t port_id,
 				struct rte_eth_pfc_conf *pfc_conf);
@@ -3772,6 +3789,7 @@  int rte_eth_dev_priority_flow_ctrl_set(uint16_t port_id,
  *   - (0) if successfully added or *mac_addr" was already added.
  *   - (-ENOTSUP) if hardware doesn't support this feature.
  *   - (-ENODEV) if *port* is invalid.
+ *   - (-EIO) if device is removed.
  *   - (-ENOSPC) if no more MAC addresses can be added.
  *   - (-EINVAL) if MAC address is invalid.
  */
@@ -3823,6 +3841,7 @@  int rte_eth_dev_default_mac_addr_set(uint16_t port,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support.
  *   - (-EINVAL) if bad parameter.
+ *   - (-EIO) if device is removed.
  */
 int rte_eth_dev_rss_reta_update(uint16_t port,
 				struct rte_eth_rss_reta_entry64 *reta_conf,
@@ -3842,6 +3861,7 @@  int rte_eth_dev_rss_reta_update(uint16_t port,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support.
  *   - (-EINVAL) if bad parameter.
+ *   - (-EIO) if device is removed.
  */
 int rte_eth_dev_rss_reta_query(uint16_t port,
 			       struct rte_eth_rss_reta_entry64 *reta_conf,
@@ -3863,6 +3883,7 @@  int rte_eth_dev_rss_reta_query(uint16_t port,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support.
   *  - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_dev_uc_hash_table_set(uint16_t port, struct ether_addr *addr,
@@ -3883,6 +3904,7 @@  int rte_eth_dev_uc_hash_table_set(uint16_t port, struct ether_addr *addr,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support.
   *  - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_dev_uc_all_hash_table_set(uint16_t port, uint8_t on);
@@ -3906,6 +3928,7 @@  int rte_eth_dev_uc_hash_table_set(uint16_t port, struct ether_addr *addr,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support this feature.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - (-EINVAL) if the mr_conf information is not correct.
  */
 int rte_eth_mirror_rule_set(uint16_t port_id,
@@ -3924,6 +3947,7 @@  int rte_eth_mirror_rule_set(uint16_t port_id,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support this feature.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_mirror_rule_reset(uint16_t port_id,
@@ -3942,6 +3966,7 @@  int rte_eth_mirror_rule_reset(uint16_t port_id,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support this feature.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - (-EINVAL) if bad parameter.
  */
 int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
@@ -3957,6 +3982,7 @@  int rte_eth_set_queue_rate_limit(uint16_t port_id, uint16_t queue_idx,
  * @return
  *   - (0) if successful.
  *   - (-ENODEV) if port identifier is invalid.
+ *   - (-EIO) if device is removed.
  *   - (-ENOTSUP) if hardware doesn't support.
  *   - (-EINVAL) if bad parameter.
  */
@@ -3974,6 +4000,7 @@  int rte_eth_dev_rss_hash_update(uint16_t port_id,
  * @return
  *   - (0) if successful.
  *   - (-ENODEV) if port identifier is invalid.
+ *   - (-EIO) if device is removed.
  *   - (-ENOTSUP) if hardware doesn't support RSS.
  */
 int
@@ -3995,6 +4022,7 @@  int rte_eth_dev_rss_hash_update(uint16_t port_id,
  * @return
  *   - (0) if successful.
  *   - (-ENODEV) if port identifier is invalid.
+ *   - (-EIO) if device is removed.
  *   - (-ENOTSUP) if hardware doesn't support tunnel type.
  */
 int
@@ -4017,6 +4045,7 @@  int rte_eth_dev_rss_hash_update(uint16_t port_id,
  * @return
  *   - (0) if successful.
  *   - (-ENODEV) if port identifier is invalid.
+ *   - (-EIO) if device is removed.
  *   - (-ENOTSUP) if hardware doesn't support tunnel type.
  */
 int
@@ -4035,6 +4064,7 @@  int rte_eth_dev_rss_hash_update(uint16_t port_id,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support this filter type.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  */
 int rte_eth_dev_filter_supported(uint16_t port_id,
 		enum rte_filter_type filter_type);
@@ -4055,6 +4085,7 @@  int rte_eth_dev_filter_supported(uint16_t port_id,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - others depends on the specific operations implementation.
  */
 int rte_eth_dev_filter_ctrl(uint16_t port_id, enum rte_filter_type filter_type,
@@ -4070,6 +4101,7 @@  int rte_eth_dev_filter_ctrl(uint16_t port_id, enum rte_filter_type filter_type,
  * @return
  *   - (0) if successful.
  *   - (-ENODEV) if port identifier is invalid.
+ *   - (-EIO) if device is removed.
  *   - (-ENOTSUP) if hardware doesn't support.
  */
 int rte_eth_dev_get_dcb_info(uint16_t port_id,
@@ -4277,6 +4309,7 @@  int rte_eth_tx_queue_info_get(uint16_t port_id, uint16_t queue_id,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - others depends on the specific operations implementation.
  */
 int rte_eth_dev_get_reg_info(uint16_t port_id, struct rte_dev_reg_info *info);
@@ -4290,6 +4323,7 @@  int rte_eth_tx_queue_info_get(uint16_t port_id, uint16_t queue_id,
  *   - (>=0) EEPROM size if successful.
  *   - (-ENOTSUP) if hardware doesn't support.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - others depends on the specific operations implementation.
  */
 int rte_eth_dev_get_eeprom_length(uint16_t port_id);
@@ -4306,6 +4340,7 @@  int rte_eth_tx_queue_info_get(uint16_t port_id, uint16_t queue_id,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - others depends on the specific operations implementation.
  */
 int rte_eth_dev_get_eeprom(uint16_t port_id, struct rte_dev_eeprom_info *info);
@@ -4322,6 +4357,7 @@  int rte_eth_tx_queue_info_get(uint16_t port_id, uint16_t queue_id,
  *   - (0) if successful.
  *   - (-ENOTSUP) if hardware doesn't support.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - others depends on the specific operations implementation.
  */
 int rte_eth_dev_set_eeprom(uint16_t port_id, struct rte_dev_eeprom_info *info);
@@ -4340,6 +4376,7 @@  int rte_eth_tx_queue_info_get(uint16_t port_id, uint16_t queue_id,
  * @return
  *   - (0) if successful.
  *   - (-ENODEV) if *port_id* invalid.
+ *   - (-EIO) if device is removed.
  *   - (-ENOTSUP) if PMD of *port_id* doesn't support multicast filtering.
  *   - (-ENOSPC) if *port_id* has not enough multicast filtering resources.
  */
@@ -4356,6 +4393,7 @@  int rte_eth_dev_set_mc_addr_list(uint16_t port_id,
  * @return
  *   - 0: Success.
  *   - -ENODEV: The port ID is invalid.
+ *   - -EIO: if device is removed.
  *   - -ENOTSUP: The function is not supported by the Ethernet driver.
  */
 int rte_eth_timesync_enable(uint16_t port_id);
@@ -4369,6 +4407,7 @@  int rte_eth_dev_set_mc_addr_list(uint16_t port_id,
  * @return
  *   - 0: Success.
  *   - -ENODEV: The port ID is invalid.
+ *   - -EIO: if device is removed.
  *   - -ENOTSUP: The function is not supported by the Ethernet driver.
  */
 int rte_eth_timesync_disable(uint16_t port_id);
@@ -4388,6 +4427,7 @@  int rte_eth_dev_set_mc_addr_list(uint16_t port_id,
  *   - 0: Success.
  *   - -EINVAL: No timestamp is available.
  *   - -ENODEV: The port ID is invalid.
+ *   - -EIO: if device is removed.
  *   - -ENOTSUP: The function is not supported by the Ethernet driver.
  */
 int rte_eth_timesync_read_rx_timestamp(uint16_t port_id,
@@ -4405,6 +4445,7 @@  int rte_eth_timesync_read_rx_timestamp(uint16_t port_id,
  *   - 0: Success.
  *   - -EINVAL: No timestamp is available.
  *   - -ENODEV: The port ID is invalid.
+ *   - -EIO: if device is removed.
  *   - -ENOTSUP: The function is not supported by the Ethernet driver.
  */
 int rte_eth_timesync_read_tx_timestamp(uint16_t port_id,
@@ -4424,6 +4465,7 @@  int rte_eth_timesync_read_tx_timestamp(uint16_t port_id,
  * @return
  *   - 0: Success.
  *   - -ENODEV: The port ID is invalid.
+ *   - -EIO: if device is removed.
  *   - -ENOTSUP: The function is not supported by the Ethernet driver.
  */
 int rte_eth_timesync_adjust_time(uint16_t port_id, int64_t delta);
@@ -4459,6 +4501,7 @@  int rte_eth_timesync_read_tx_timestamp(uint16_t port_id,
  *   - 0: Success.
  *   - -EINVAL: No timestamp is available.
  *   - -ENODEV: The port ID is invalid.
+ *   - -EIO: if device is removed.
  *   - -ENOTSUP: The function is not supported by the Ethernet driver.
  */
 int rte_eth_timesync_write_time(uint16_t port_id, const struct timespec *time);
@@ -4499,6 +4542,7 @@  int rte_eth_timesync_read_tx_timestamp(uint16_t port_id,
  * @return
  *   - (0) if successful.
  *   - (-ENODEV) if port identifier is invalid.
+ *   - (-EIO) if device is removed.
  *   - (-ENOTSUP) if hardware doesn't support tunnel type.
  */
 int
@@ -4526,6 +4570,7 @@  int rte_eth_timesync_read_tx_timestamp(uint16_t port_id,
  * @return
  *   - (0) if successful.
  *   - (-ENODEV) if port identifier is invalid.
+ *   - (-EIO) if device is removed.
  *   - (-ENOTSUP) if hardware doesn't support tunnel type.
  */
 int