[v2,3/3] ethdev: document special cases of port start and stop

Message ID 20221109190639.886457-4-dsosnowski@nvidia.com (mailing list archive)
State Superseded, archived
Delegated to: Andrew Rybchenko
Headers
Series ethdev: document special cases of port start and stop |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/github-robot: build success github build: passed
ci/intel-Testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS

Commit Message

Dariusz Sosnowski Nov. 9, 2022, 7:06 p.m. UTC
  This patch clarifies the handling of following cases
in the ethdev API docs:

- If rte_eth_dev_start() returns (-EAGAIN) for some port,
  it cannot be started until other port is started.
- If rte_eth_dev_stop() returns (-EBUSY) for some port,
  it cannot be stopped until other port is stopped.

When stopping the port in testpmd fails due to (-EBUSY),
port's state is switched back to STARTED
to allow users to manually retry stopping the port.

No additional changes in testpmd are required to handle
failure to start port with (-EAGAIN).
If rte_eth_dev_start() fails, port's state is switched to STOPPED
and users are allowed to retry the operation.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 app/test-pmd/testpmd.c  | 10 +++++++++-
 lib/ethdev/rte_ethdev.h |  9 +++++++++
 2 files changed, 18 insertions(+), 1 deletion(-)
  

Comments

Ferruh Yigit Nov. 14, 2022, 2:07 p.m. UTC | #1
On 11/9/2022 7:06 PM, Dariusz Sosnowski wrote:
> This patch clarifies the handling of following cases
> in the ethdev API docs:
> 
> - If rte_eth_dev_start() returns (-EAGAIN) for some port,
>    it cannot be started until other port is started.
> - If rte_eth_dev_stop() returns (-EBUSY) for some port,
>    it cannot be stopped until other port is stopped.
> 

EAGAIN and EBUSY has kind of standard meaning, I am not sure if it is 
good idea to change this meaning to a specific "dependency to other 
port" use case.
Why not keep common generic meanings of the error codes?

> When stopping the port in testpmd fails due to (-EBUSY),
> port's state is switched back to STARTED
> to allow users to manually retry stopping the port.
> 
> No additional changes in testpmd are required to handle
> failure to start port with (-EAGAIN).
> If rte_eth_dev_start() fails, port's state is switched to STOPPED
> and users are allowed to retry the operation.
> 
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> ---
>   app/test-pmd/testpmd.c  | 10 +++++++++-
>   lib/ethdev/rte_ethdev.h |  9 +++++++++
>   2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index aa7ea29f15..5a69e3c77a 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -3118,6 +3118,7 @@ stop_port(portid_t pid)
>   	int need_check_link_status = 0;
>   	portid_t peer_pl[RTE_MAX_ETHPORTS];
>   	int peer_pi;
> +	int ret;
>   
>   	if (port_id_is_invalid(pid, ENABLED_WARN))
>   		return;
> @@ -3167,9 +3168,16 @@ stop_port(portid_t pid)
>   		if (port->flow_list)
>   			port_flow_flush(pi);
>   
> -		if (eth_dev_stop_mp(pi) != 0)
> +		ret = eth_dev_stop_mp(pi);
> +		if (ret != 0) {
>   			RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
>   				pi);
> +			if (ret == -EBUSY) {
> +				/* Allow to retry stopping the port. */
> +				port->port_status = RTE_PORT_STARTED;

If the stop() failed, isn't the current status should be STARTED 
independent from the error type?

> +				continue;
> +			}
> +		}
>   
>   		if (port->port_status == RTE_PORT_HANDLING)
>   			port->port_status = RTE_PORT_STOPPED;
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 13fe73d5a3..abf5a24f92 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -2701,10 +2701,14 @@ int rte_eth_dev_tx_queue_stop(uint16_t port_id, uint16_t tx_queue_id);
>    * On success, all basic functions exported by the Ethernet API (link status,
>    * receive/transmit, and so on) can be invoked.
>    *
> + * If the port depends on another one being started,
> + * PMDs might return (-EAGAIN) to notify about such requirement.
> + *
>    * @param port_id
>    *   The port identifier of the Ethernet device.
>    * @return
>    *   - 0: Success, Ethernet device started.
> + *   - -EAGAIN: If it depends on another port to be started first.
>    *   - <0: Error code of the driver device start function.
>    */
>   int rte_eth_dev_start(uint16_t port_id);
> @@ -2713,10 +2717,15 @@ int rte_eth_dev_start(uint16_t port_id);
>    * Stop an Ethernet device. The device can be restarted with a call to
>    * rte_eth_dev_start()
>    *
> + * If the port provides some resources for other ports
> + * and it cannot be stopped before them,
> + * PMDs might return (-EBUSY) to notify about such requirement.
> + *
>    * @param port_id
>    *   The port identifier of the Ethernet device.
>    * @return
>    *   - 0: Success, Ethernet device stopped.
> + *   - -EBUSY: If it depends on another port to be stopped first.
>    *   - <0: Error code of the driver device stop function.
>    */
>   int rte_eth_dev_stop(uint16_t port_id);
  
Dariusz Sosnowski Nov. 14, 2022, 4:12 p.m. UTC | #2
Hi Ferruh,

> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Monday, November 14, 2022 15:08
> To: Dariusz Sosnowski <dsosnowski@nvidia.com>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2 3/3] ethdev: document special cases of port start and
> stop
> 
> External email: Use caution opening links or attachments
> 
> 
> On 11/9/2022 7:06 PM, Dariusz Sosnowski wrote:
> > This patch clarifies the handling of following cases in the ethdev API
> > docs:
> >
> > - If rte_eth_dev_start() returns (-EAGAIN) for some port,
> >    it cannot be started until other port is started.
> > - If rte_eth_dev_stop() returns (-EBUSY) for some port,
> >    it cannot be stopped until other port is stopped.
> >
> 
> EAGAIN and EBUSY has kind of standard meaning, I am not sure if it is good
> idea to change this meaning to a specific "dependency to other port" use
> case.
> Why not keep common generic meanings of the error codes?

In my opinion, using standard error meanings for EAGAIN and EBUSY would make the returned errors too vague for the API user.
If we used the standard meanings, it's not clear why the port cannot be started or stopped and what the user should do about it.
Providing specific reasons in the API makes it more understandable. On top of that, they are "subcases" of standard errors:

- "Port cannot be stopped because another port depends on it" is a special case of "Device or resource is busy".
- "Port cannot be started because it depends on other port being started" is a special case of "Resource temporarily unavailable".

However, I see your concern, so maybe let's do the following. To not limit the API, we could for example:

- In the documentation of returned values - provide the generic meaning for the EAGAIN and EBUSY:
    - rte_eth_dev_stop(): EBUSY is returned if stopping the port is not allowed in the current state.
    - rte_eth_dev_start(): EAGAIN is returned if start operation must be retried.
- In the function description provide the specific use case of "dependency on other port" as an example
  of EBUSY/EAGAIN usage
    - Depending on the use cases emerging in the future, new examples can be added,
      if EBUSY/EAGAIN is suitable for the new use cases.

What do you think?

> > When stopping the port in testpmd fails due to (-EBUSY), port's state
> > is switched back to STARTED to allow users to manually retry stopping
> > the port.
> >
> > No additional changes in testpmd are required to handle failure to
> > start port with (-EAGAIN).
> > If rte_eth_dev_start() fails, port's state is switched to STOPPED and
> > users are allowed to retry the operation.
> >
> > Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> > ---
> >   app/test-pmd/testpmd.c  | 10 +++++++++-
> >   lib/ethdev/rte_ethdev.h |  9 +++++++++
> >   2 files changed, 18 insertions(+), 1 deletion(-)
> >
> > diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> > aa7ea29f15..5a69e3c77a 100644
> > --- a/app/test-pmd/testpmd.c
> > +++ b/app/test-pmd/testpmd.c
> > @@ -3118,6 +3118,7 @@ stop_port(portid_t pid)
> >       int need_check_link_status = 0;
> >       portid_t peer_pl[RTE_MAX_ETHPORTS];
> >       int peer_pi;
> > +     int ret;
> >
> >       if (port_id_is_invalid(pid, ENABLED_WARN))
> >               return;
> > @@ -3167,9 +3168,16 @@ stop_port(portid_t pid)
> >               if (port->flow_list)
> >                       port_flow_flush(pi);
> >
> > -             if (eth_dev_stop_mp(pi) != 0)
> > +             ret = eth_dev_stop_mp(pi);
> > +             if (ret != 0) {
> >                       RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
> >                               pi);
> > +                     if (ret == -EBUSY) {
> > +                             /* Allow to retry stopping the port. */
> > +                             port->port_status = RTE_PORT_STARTED;
> 
> If the stop() failed, isn't the current status should be STARTED independent
> from the error type?

Correct. I'll update it in v3.

> > +                             continue;
> > +                     }
> > +             }
> >
> >               if (port->port_status == RTE_PORT_HANDLING)
> >                       port->port_status = RTE_PORT_STOPPED; diff --git
> > a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> > 13fe73d5a3..abf5a24f92 100644
> > --- a/lib/ethdev/rte_ethdev.h
> > +++ b/lib/ethdev/rte_ethdev.h
> > @@ -2701,10 +2701,14 @@ int rte_eth_dev_tx_queue_stop(uint16_t
> port_id, uint16_t tx_queue_id);
> >    * On success, all basic functions exported by the Ethernet API (link status,
> >    * receive/transmit, and so on) can be invoked.
> >    *
> > + * If the port depends on another one being started,
> > + * PMDs might return (-EAGAIN) to notify about such requirement.
> > + *
> >    * @param port_id
> >    *   The port identifier of the Ethernet device.
> >    * @return
> >    *   - 0: Success, Ethernet device started.
> > + *   - -EAGAIN: If it depends on another port to be started first.
> >    *   - <0: Error code of the driver device start function.
> >    */
> >   int rte_eth_dev_start(uint16_t port_id); @@ -2713,10 +2717,15 @@ int
> > rte_eth_dev_start(uint16_t port_id);
> >    * Stop an Ethernet device. The device can be restarted with a call to
> >    * rte_eth_dev_start()
> >    *
> > + * If the port provides some resources for other ports
> > + * and it cannot be stopped before them,
> > + * PMDs might return (-EBUSY) to notify about such requirement.
> > + *
> >    * @param port_id
> >    *   The port identifier of the Ethernet device.
> >    * @return
> >    *   - 0: Success, Ethernet device stopped.
> > + *   - -EBUSY: If it depends on another port to be stopped first.
> >    *   - <0: Error code of the driver device stop function.
> >    */
> >   int rte_eth_dev_stop(uint16_t port_id);

Best regards,
Dariusz Sosnowski
  
Ferruh Yigit Nov. 14, 2022, 5:10 p.m. UTC | #3
On 11/14/2022 4:12 PM, Dariusz Sosnowski wrote:
> Hi Ferruh,
> 
>> -----Original Message-----
>> From: Ferruh Yigit <ferruh.yigit@amd.com>
>> Sent: Monday, November 14, 2022 15:08
>> To: Dariusz Sosnowski <dsosnowski@nvidia.com>; Aman Singh
>> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
>> NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
>> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v2 3/3] ethdev: document special cases of port start and
>> stop
>>
>> External email: Use caution opening links or attachments
>>
>>
>> On 11/9/2022 7:06 PM, Dariusz Sosnowski wrote:
>>> This patch clarifies the handling of following cases in the ethdev API
>>> docs:
>>>
>>> - If rte_eth_dev_start() returns (-EAGAIN) for some port,
>>>     it cannot be started until other port is started.
>>> - If rte_eth_dev_stop() returns (-EBUSY) for some port,
>>>     it cannot be stopped until other port is stopped.
>>>
>>
>> EAGAIN and EBUSY has kind of standard meaning, I am not sure if it is good
>> idea to change this meaning to a specific "dependency to other port" use
>> case.
>> Why not keep common generic meanings of the error codes?
> 
> In my opinion, using standard error meanings for EAGAIN and EBUSY would make the returned errors too vague for the API user.
> If we used the standard meanings, it's not clear why the port cannot be started or stopped and what the user should do about it.
> Providing specific reasons in the API makes it more understandable. On top of that, they are "subcases" of standard errors:
> 

I think generic error meaning is not vague, although underlying reason is.

> - "Port cannot be stopped because another port depends on it" is a special case of "Device or resource is busy".
> - "Port cannot be started because it depends on other port being started" is a special case of "Resource temporarily unavailable".
> 
> However, I see your concern, so maybe let's do the following. To not limit the API, we could for example:
> 
> - In the documentation of returned values - provide the generic meaning for the EAGAIN and EBUSY:
>      - rte_eth_dev_stop(): EBUSY is returned if stopping the port is not allowed in the current state.
>      - rte_eth_dev_start(): EAGAIN is returned if start operation must be retried.
> - In the function description provide the specific use case of "dependency on other port" as an example
>    of EBUSY/EAGAIN usage
>      - Depending on the use cases emerging in the future, new examples can be added,
>        if EBUSY/EAGAIN is suitable for the new use cases.
> 
> What do you think?

OK to above generic error documentation.
And what do you think to document "dependency on other port" in the 
driver dev_ops function comment, since it will be an instance of the 
generic error message.

> 
>>> When stopping the port in testpmd fails due to (-EBUSY), port's state
>>> is switched back to STARTED to allow users to manually retry stopping
>>> the port.
>>>
>>> No additional changes in testpmd are required to handle failure to
>>> start port with (-EAGAIN).
>>> If rte_eth_dev_start() fails, port's state is switched to STOPPED and
>>> users are allowed to retry the operation.
>>>
>>> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
>>> ---
>>>    app/test-pmd/testpmd.c  | 10 +++++++++-
>>>    lib/ethdev/rte_ethdev.h |  9 +++++++++
>>>    2 files changed, 18 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
>>> aa7ea29f15..5a69e3c77a 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -3118,6 +3118,7 @@ stop_port(portid_t pid)
>>>        int need_check_link_status = 0;
>>>        portid_t peer_pl[RTE_MAX_ETHPORTS];
>>>        int peer_pi;
>>> +     int ret;
>>>
>>>        if (port_id_is_invalid(pid, ENABLED_WARN))
>>>                return;
>>> @@ -3167,9 +3168,16 @@ stop_port(portid_t pid)
>>>                if (port->flow_list)
>>>                        port_flow_flush(pi);
>>>
>>> -             if (eth_dev_stop_mp(pi) != 0)
>>> +             ret = eth_dev_stop_mp(pi);
>>> +             if (ret != 0) {
>>>                        RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
>>>                                pi);
>>> +                     if (ret == -EBUSY) {
>>> +                             /* Allow to retry stopping the port. */
>>> +                             port->port_status = RTE_PORT_STARTED;
>>
>> If the stop() failed, isn't the current status should be STARTED independent
>> from the error type?
> 
> Correct. I'll update it in v3.
> 
>>> +                             continue;
>>> +                     }
>>> +             }
>>>
>>>                if (port->port_status == RTE_PORT_HANDLING)
>>>                        port->port_status = RTE_PORT_STOPPED; diff --git
>>> a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
>>> 13fe73d5a3..abf5a24f92 100644
>>> --- a/lib/ethdev/rte_ethdev.h
>>> +++ b/lib/ethdev/rte_ethdev.h
>>> @@ -2701,10 +2701,14 @@ int rte_eth_dev_tx_queue_stop(uint16_t
>> port_id, uint16_t tx_queue_id);
>>>     * On success, all basic functions exported by the Ethernet API (link status,
>>>     * receive/transmit, and so on) can be invoked.
>>>     *
>>> + * If the port depends on another one being started,
>>> + * PMDs might return (-EAGAIN) to notify about such requirement.
>>> + *
>>>     * @param port_id
>>>     *   The port identifier of the Ethernet device.
>>>     * @return
>>>     *   - 0: Success, Ethernet device started.
>>> + *   - -EAGAIN: If it depends on another port to be started first.
>>>     *   - <0: Error code of the driver device start function.
>>>     */
>>>    int rte_eth_dev_start(uint16_t port_id); @@ -2713,10 +2717,15 @@ int
>>> rte_eth_dev_start(uint16_t port_id);
>>>     * Stop an Ethernet device. The device can be restarted with a call to
>>>     * rte_eth_dev_start()
>>>     *
>>> + * If the port provides some resources for other ports
>>> + * and it cannot be stopped before them,
>>> + * PMDs might return (-EBUSY) to notify about such requirement.
>>> + *
>>>     * @param port_id
>>>     *   The port identifier of the Ethernet device.
>>>     * @return
>>>     *   - 0: Success, Ethernet device stopped.
>>> + *   - -EBUSY: If it depends on another port to be stopped first.
>>>     *   - <0: Error code of the driver device stop function.
>>>     */
>>>    int rte_eth_dev_stop(uint16_t port_id);
> 
> Best regards,
> Dariusz Sosnowski
>
  
Dariusz Sosnowski Nov. 14, 2022, 6:22 p.m. UTC | #4
> -----Original Message-----
> From: Ferruh Yigit <ferruh.yigit@amd.com>
> Sent: Monday, November 14, 2022 18:10
> To: Dariusz Sosnowski <dsosnowski@nvidia.com>; Aman Singh
> <aman.deep.singh@intel.com>; Yuying Zhang <yuying.zhang@intel.com>;
> NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2 3/3] ethdev: document special cases of port start and
> stop
> 
> External email: Use caution opening links or attachments
> 
> 
> On 11/14/2022 4:12 PM, Dariusz Sosnowski wrote:
> > Hi Ferruh,
> >
> >> -----Original Message-----
> >> From: Ferruh Yigit <ferruh.yigit@amd.com>
> >> Sent: Monday, November 14, 2022 15:08
> >> To: Dariusz Sosnowski <dsosnowski@nvidia.com>; Aman Singh
> >> <aman.deep.singh@intel.com>; Yuying Zhang
> <yuying.zhang@intel.com>;
> >> NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> Andrew
> >> Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Cc: dev@dpdk.org
> >> Subject: Re: [PATCH v2 3/3] ethdev: document special cases of port
> >> start and stop
> >>
> >> External email: Use caution opening links or attachments
> >>
> >>
> >> On 11/9/2022 7:06 PM, Dariusz Sosnowski wrote:
> >>> This patch clarifies the handling of following cases in the ethdev
> >>> API
> >>> docs:
> >>>
> >>> - If rte_eth_dev_start() returns (-EAGAIN) for some port,
> >>>     it cannot be started until other port is started.
> >>> - If rte_eth_dev_stop() returns (-EBUSY) for some port,
> >>>     it cannot be stopped until other port is stopped.
> >>>
> >>
> >> EAGAIN and EBUSY has kind of standard meaning, I am not sure if it is
> >> good idea to change this meaning to a specific "dependency to other
> >> port" use case.
> >> Why not keep common generic meanings of the error codes?
> >
> > In my opinion, using standard error meanings for EAGAIN and EBUSY would
> make the returned errors too vague for the API user.
> > If we used the standard meanings, it's not clear why the port cannot be
> started or stopped and what the user should do about it.
> > Providing specific reasons in the API makes it more understandable. On top
> of that, they are "subcases" of standard errors:
> >
> 
> I think generic error meaning is not vague, although underlying reason is.
> 
> > - "Port cannot be stopped because another port depends on it" is a special
> case of "Device or resource is busy".
> > - "Port cannot be started because it depends on other port being started"
> is a special case of "Resource temporarily unavailable".
> >
> > However, I see your concern, so maybe let's do the following. To not limit
> the API, we could for example:
> >
> > - In the documentation of returned values - provide the generic meaning
> for the EAGAIN and EBUSY:
> >      - rte_eth_dev_stop(): EBUSY is returned if stopping the port is not
> allowed in the current state.
> >      - rte_eth_dev_start(): EAGAIN is returned if start operation must be
> retried.
> > - In the function description provide the specific use case of "dependency
> on other port" as an example
> >    of EBUSY/EAGAIN usage
> >      - Depending on the use cases emerging in the future, new examples can
> be added,
> >        if EBUSY/EAGAIN is suitable for the new use cases.
> >
> > What do you think?
> 
> OK to above generic error documentation.
> And what do you think to document "dependency on other port" in the
> driver dev_ops function comment, since it will be an instance of the generic
> error message.

Sounds good to me. Updated in v3.

> >
> >>> When stopping the port in testpmd fails due to (-EBUSY), port's
> >>> state is switched back to STARTED to allow users to manually retry
> >>> stopping the port.
> >>>
> >>> No additional changes in testpmd are required to handle failure to
> >>> start port with (-EAGAIN).
> >>> If rte_eth_dev_start() fails, port's state is switched to STOPPED
> >>> and users are allowed to retry the operation.
> >>>
> >>> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
> >>> ---
> >>>    app/test-pmd/testpmd.c  | 10 +++++++++-
> >>>    lib/ethdev/rte_ethdev.h |  9 +++++++++
> >>>    2 files changed, 18 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c index
> >>> aa7ea29f15..5a69e3c77a 100644
> >>> --- a/app/test-pmd/testpmd.c
> >>> +++ b/app/test-pmd/testpmd.c
> >>> @@ -3118,6 +3118,7 @@ stop_port(portid_t pid)
> >>>        int need_check_link_status = 0;
> >>>        portid_t peer_pl[RTE_MAX_ETHPORTS];
> >>>        int peer_pi;
> >>> +     int ret;
> >>>
> >>>        if (port_id_is_invalid(pid, ENABLED_WARN))
> >>>                return;
> >>> @@ -3167,9 +3168,16 @@ stop_port(portid_t pid)
> >>>                if (port->flow_list)
> >>>                        port_flow_flush(pi);
> >>>
> >>> -             if (eth_dev_stop_mp(pi) != 0)
> >>> +             ret = eth_dev_stop_mp(pi);
> >>> +             if (ret != 0) {
> >>>                        RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
> >>>                                pi);
> >>> +                     if (ret == -EBUSY) {
> >>> +                             /* Allow to retry stopping the port. */
> >>> +                             port->port_status = RTE_PORT_STARTED;
> >>
> >> If the stop() failed, isn't the current status should be STARTED
> >> independent from the error type?
> >
> > Correct. I'll update it in v3.
> >
> >>> +                             continue;
> >>> +                     }
> >>> +             }
> >>>
> >>>                if (port->port_status == RTE_PORT_HANDLING)
> >>>                        port->port_status = RTE_PORT_STOPPED; diff
> >>> --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h index
> >>> 13fe73d5a3..abf5a24f92 100644
> >>> --- a/lib/ethdev/rte_ethdev.h
> >>> +++ b/lib/ethdev/rte_ethdev.h
> >>> @@ -2701,10 +2701,14 @@ int rte_eth_dev_tx_queue_stop(uint16_t
> >> port_id, uint16_t tx_queue_id);
> >>>     * On success, all basic functions exported by the Ethernet API (link
> status,
> >>>     * receive/transmit, and so on) can be invoked.
> >>>     *
> >>> + * If the port depends on another one being started,
> >>> + * PMDs might return (-EAGAIN) to notify about such requirement.
> >>> + *
> >>>     * @param port_id
> >>>     *   The port identifier of the Ethernet device.
> >>>     * @return
> >>>     *   - 0: Success, Ethernet device started.
> >>> + *   - -EAGAIN: If it depends on another port to be started first.
> >>>     *   - <0: Error code of the driver device start function.
> >>>     */
> >>>    int rte_eth_dev_start(uint16_t port_id); @@ -2713,10 +2717,15 @@
> >>> int rte_eth_dev_start(uint16_t port_id);
> >>>     * Stop an Ethernet device. The device can be restarted with a call to
> >>>     * rte_eth_dev_start()
> >>>     *
> >>> + * If the port provides some resources for other ports
> >>> + * and it cannot be stopped before them,
> >>> + * PMDs might return (-EBUSY) to notify about such requirement.
> >>> + *
> >>>     * @param port_id
> >>>     *   The port identifier of the Ethernet device.
> >>>     * @return
> >>>     *   - 0: Success, Ethernet device stopped.
> >>> + *   - -EBUSY: If it depends on another port to be stopped first.
> >>>     *   - <0: Error code of the driver device stop function.
> >>>     */
> >>>    int rte_eth_dev_stop(uint16_t port_id);
> >
> > Best regards,
> > Dariusz Sosnowski
> >
  

Patch

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index aa7ea29f15..5a69e3c77a 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3118,6 +3118,7 @@  stop_port(portid_t pid)
 	int need_check_link_status = 0;
 	portid_t peer_pl[RTE_MAX_ETHPORTS];
 	int peer_pi;
+	int ret;
 
 	if (port_id_is_invalid(pid, ENABLED_WARN))
 		return;
@@ -3167,9 +3168,16 @@  stop_port(portid_t pid)
 		if (port->flow_list)
 			port_flow_flush(pi);
 
-		if (eth_dev_stop_mp(pi) != 0)
+		ret = eth_dev_stop_mp(pi);
+		if (ret != 0) {
 			RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
 				pi);
+			if (ret == -EBUSY) {
+				/* Allow to retry stopping the port. */
+				port->port_status = RTE_PORT_STARTED;
+				continue;
+			}
+		}
 
 		if (port->port_status == RTE_PORT_HANDLING)
 			port->port_status = RTE_PORT_STOPPED;
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 13fe73d5a3..abf5a24f92 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2701,10 +2701,14 @@  int rte_eth_dev_tx_queue_stop(uint16_t port_id, uint16_t tx_queue_id);
  * On success, all basic functions exported by the Ethernet API (link status,
  * receive/transmit, and so on) can be invoked.
  *
+ * If the port depends on another one being started,
+ * PMDs might return (-EAGAIN) to notify about such requirement.
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @return
  *   - 0: Success, Ethernet device started.
+ *   - -EAGAIN: If it depends on another port to be started first.
  *   - <0: Error code of the driver device start function.
  */
 int rte_eth_dev_start(uint16_t port_id);
@@ -2713,10 +2717,15 @@  int rte_eth_dev_start(uint16_t port_id);
  * Stop an Ethernet device. The device can be restarted with a call to
  * rte_eth_dev_start()
  *
+ * If the port provides some resources for other ports
+ * and it cannot be stopped before them,
+ * PMDs might return (-EBUSY) to notify about such requirement.
+ *
  * @param port_id
  *   The port identifier of the Ethernet device.
  * @return
  *   - 0: Success, Ethernet device stopped.
+ *   - -EBUSY: If it depends on another port to be stopped first.
  *   - <0: Error code of the driver device stop function.
  */
 int rte_eth_dev_stop(uint16_t port_id);