[PATCH 0/3] ethdev: document special cases of port start and stop

[PATCH 0/3] ethdev: document special cases of port start and stop

[Dariusz Sosnowski]

This patch series attempts to address the special failure
cases of rte_eth_dev_stop() and rte_eth_dev_start().

In case of starting a port, If the port depends on another one
being started, PMDs might return (-EAGAIN) to notify
about such situation.

In case of stopping a port, If the port cannot be stopped,
because other port depends on its resources,
PMDs might return (-EBUSY) to notify about such situation.

These cases are addressed in ethdev API docs.
Testpmd is updated to allow users to manually retry start/stop operations.

Dariusz Sosnowski (3):
  net/mlx5: fix log level on failed transfer proxy stop
  doc: document E-Switch limitations with HWS in mlx5 PMD
  ethdev: document special cases of port start and stop

 app/test-pmd/testpmd.c          | 10 +++++++++-
 doc/guides/nics/mlx5.rst        | 13 +++++++++++++
 drivers/net/mlx5/mlx5_trigger.c |  6 +++---
 lib/ethdev/rte_ethdev.h         |  9 +++++++++
 4 files changed, 34 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH 1/3] net/mlx5: fix log level on failed transfer proxy stop

[Dariusz Sosnowski]

This patches increases log level for error reporting when stopping
the transfer proxy port failed. Stopping can fail with EBUSY when
related representor ports are still running.

Fixes: 483181f7b6dd ("net/mlx5: support device control of representor matching")

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5_trigger.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 4b821a1076..310a84729e 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -1361,9 +1361,9 @@ mlx5_hw_proxy_port_allowed_stop(struct rte_eth_dev *dev)
 			representor_started = true;
 	}
 	if (representor_started) {
-		DRV_LOG(INFO, "Failed to stop port %u: attached representor ports"
-			      " must be stopped before stopping transfer proxy port",
-			      dev->data->port_id);
+		DRV_LOG(ERR, "Failed to stop port %u: attached representor ports"
+			     " must be stopped before stopping transfer proxy port",
+			     dev->data->port_id);
 		rte_errno = EBUSY;
 		return -rte_errno;
 	}
-- 
2.25.1

[PATCH 2/3] doc: document E-Switch limitations with HWS in mlx5 PMD

[Dariusz Sosnowski]

This patch adds the following limitations to the mlx5 PMD guide:

- With HW Steering and E-Switch enabled, transfer proxy port must
  be started before any port representor.
- With HW Steering and E-Switch enabled, all representors
  must be stopped before transfer proxy port is stopped.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 4f0db21dde..7e39bb2ea5 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -161,6 +161,19 @@ Limitations
   - NIC ConnectX-5 and before are not supported.
   - Partial match with item template is not supported.
   - IPv6 5-tuple matching is not supported.
+  - With E-Switch enabled, ports which share the E-Switch domain
+    should be started and stopped in a specific order:
+
+    - When starting ports, the transfer proxy port should be started first
+      and port representors should follow.
+    - When stopping ports, all of the port representors
+      should be stopped before stopping the transfer proxy port.
+
+    If ports are started/stopped in an incorrect order,
+    ``rte_eth_dev_start()``/``rte_eth_dev_stop()`` will return an appropriate error code:
+
+    - ``-EAGAIN`` for ``rte_eth_dev_start()``.
+    - ``-EBUSY`` for ``rte_eth_dev_stop().
 
 - When using Verbs flow engine (``dv_flow_en`` = 0), flow pattern without any
   specific VLAN will match for VLAN packets as well:
-- 
2.25.1

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

[Dariusz Sosnowski]

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(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 7381dfd9e5..c9252031e8 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3158,6 +3158,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;
@@ -3207,9 +3208,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);
-- 
2.25.1

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

[Dariusz Sosnowski]

This patch series attempts to address the special failure
cases of rte_eth_dev_stop() and rte_eth_dev_start().

In case of starting a port, If the port depends on another one
being started, PMDs might return (-EAGAIN) to notify
about such situation.

In case of stopping a port, If the port cannot be stopped,
because other port depends on its resources,
PMDs might return (-EBUSY) to notify about such situation.

These cases are addressed in ethdev API docs.
Testpmd is updated to allow users to manually retry start/stop operations.

v2:
- Fixed documentation build.

Dariusz Sosnowski (3):
  net/mlx5: fix log level on failed transfer proxy stop
  doc: document E-Switch limitations with HWS in mlx5 PMD
  ethdev: document special cases of port start and stop

 app/test-pmd/testpmd.c          | 10 +++++++++-
 doc/guides/nics/mlx5.rst        | 13 +++++++++++++
 drivers/net/mlx5/mlx5_trigger.c |  6 +++---
 lib/ethdev/rte_ethdev.h         |  9 +++++++++
 4 files changed, 34 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH v2 1/3] net/mlx5: fix log level on failed transfer proxy stop

[Dariusz Sosnowski]

This patches increases log level for error reporting when stopping
the transfer proxy port failed. Stopping can fail with EBUSY when
related representor ports are still running.

Fixes: 483181f7b6dd ("net/mlx5: support device control of representor matching")

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 drivers/net/mlx5/mlx5_trigger.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 4b821a1076..310a84729e 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -1361,9 +1361,9 @@ mlx5_hw_proxy_port_allowed_stop(struct rte_eth_dev *dev)
 			representor_started = true;
 	}
 	if (representor_started) {
-		DRV_LOG(INFO, "Failed to stop port %u: attached representor ports"
-			      " must be stopped before stopping transfer proxy port",
-			      dev->data->port_id);
+		DRV_LOG(ERR, "Failed to stop port %u: attached representor ports"
+			     " must be stopped before stopping transfer proxy port",
+			     dev->data->port_id);
 		rte_errno = EBUSY;
 		return -rte_errno;
 	}
-- 
2.25.1

RE: [PATCH v2 1/3] net/mlx5: fix log level on failed transfer proxy stop

[Slava Ovsiienko]

> -----Original Message-----
> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Sent: Wednesday, November 9, 2022 21:07
> To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v2 1/3] net/mlx5: fix log level on failed transfer proxy
> stop
> 
> This patches increases log level for error reporting when stopping the
> transfer proxy port failed. Stopping can fail with EBUSY when related
> representor ports are still running.
> 
> Fixes: 483181f7b6dd ("net/mlx5: support device control of representor
> matching")
> 
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

[PATCH v2 2/3] doc: document E-Switch limitations with HWS in mlx5 PMD

[Dariusz Sosnowski]

This patch adds the following limitations to the mlx5 PMD guide:

- With HW Steering and E-Switch enabled, transfer proxy port must
  be started before any port representor.
- With HW Steering and E-Switch enabled, all representors
  must be stopped before transfer proxy port is stopped.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index d5f9375a4e..ca555e7ca7 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -161,6 +161,19 @@ Limitations
   - NIC ConnectX-5 and before are not supported.
   - Partial match with item template is not supported.
   - IPv6 5-tuple matching is not supported.
+  - With E-Switch enabled, ports which share the E-Switch domain
+    should be started and stopped in a specific order:
+
+    - When starting ports, the transfer proxy port should be started first
+      and port representors should follow.
+    - When stopping ports, all of the port representors
+      should be stopped before stopping the transfer proxy port.
+
+    If ports are started/stopped in an incorrect order,
+    ``rte_eth_dev_start()``/``rte_eth_dev_stop()`` will return an appropriate error code:
+
+    - ``-EAGAIN`` for ``rte_eth_dev_start()``.
+    - ``-EBUSY`` for ``rte_eth_dev_stop()``.
 
 - When using Verbs flow engine (``dv_flow_en`` = 0), flow pattern without any
   specific VLAN will match for VLAN packets as well:
-- 
2.25.1

RE: [PATCH v2 2/3] doc: document E-Switch limitations with HWS in mlx5 PMD

[Slava Ovsiienko]

> -----Original Message-----
> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Sent: Wednesday, November 9, 2022 21:07
> To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v2 2/3] doc: document E-Switch limitations with HWS in mlx5
> PMD
> 
> This patch adds the following limitations to the mlx5 PMD guide:
> 
> - With HW Steering and E-Switch enabled, transfer proxy port must
>   be started before any port representor.
> - With HW Steering and E-Switch enabled, all representors
>   must be stopped before transfer proxy port is stopped.
> 
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

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

[Dariusz Sosnowski]

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(-)

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);
-- 
2.25.1

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

[Ferruh Yigit]

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);

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

[Dariusz Sosnowski]

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

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

[Ferruh Yigit]

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
> 

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

[Dariusz Sosnowski]

> -----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 v3 0/3] ethdev: document special cases of port start and stop

[Dariusz Sosnowski]

This patch series attempts to address the special failure
cases of rte_eth_dev_stop() and rte_eth_dev_start().

In case of starting a port, If the port cannot be started and
start operation must be retried, PMDs might return (-EAGAIN)
to notify about such situation.

In case of stopping a port, If the port cannot be stopped
in the current state, PMDs might return (-EBUSY)
to notify about such situation.

These cases are addressed in ethdev API docs.
Testpmd is updated to allow users to manually retry start/stop operations.

v3:
- Allow to retry port stop in testpmd for all errors.
- Made error descriptions in the API more generic and
  moved specific error causes descriptions to PMD.

v2:
- Fixed documentation build.

Dariusz Sosnowski (3):
  net/mlx5: fix log level on failed transfer proxy stop
  net/mlx5: document E-Switch limitations with HWS in mlx5 PMD
  ethdev: document special cases of port start and stop

 app/test-pmd/testpmd.c          |  8 +++++++-
 doc/guides/nics/mlx5.rst        | 13 +++++++++++++
 drivers/net/mlx5/mlx5_trigger.c | 17 ++++++++++++++---
 lib/ethdev/rte_ethdev.h         |  2 ++
 4 files changed, 36 insertions(+), 4 deletions(-)

-- 
2.25.1

[PATCH v3 1/3] net/mlx5: fix log level on failed transfer proxy stop

[Dariusz Sosnowski]

This patches increases log level for error reporting when stopping
the transfer proxy port failed. Stopping can fail with EBUSY when
related representor ports are still running.

Fixes: 483181f7b6dd ("net/mlx5: support device control of representor matching")

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
---
 drivers/net/mlx5/mlx5_trigger.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index 9854df828d..fe6359908a 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -1377,9 +1377,9 @@ mlx5_hw_proxy_port_allowed_stop(struct rte_eth_dev *dev)
 			representor_started = true;
 	}
 	if (representor_started) {
-		DRV_LOG(INFO, "Failed to stop port %u: attached representor ports"
-			      " must be stopped before stopping transfer proxy port",
-			      dev->data->port_id);
+		DRV_LOG(ERR, "Failed to stop port %u: attached representor ports"
+			     " must be stopped before stopping transfer proxy port",
+			     dev->data->port_id);
 		rte_errno = EBUSY;
 		return -rte_errno;
 	}
-- 
2.25.1

[PATCH v3 2/3] net/mlx5: document E-Switch limitations with HWS in mlx5 PMD

[Dariusz Sosnowski]

This patch adds the following limitations to the mlx5 PMD guide:

- With HW Steering and E-Switch enabled, transfer proxy port must
  be started before any port representor.
- With HW Steering and E-Switch enabled, all representors
  must be stopped before transfer proxy port is stopped.

Documentation of mlx5 PMD's implementations of
rte_eth_dev_start() and rte_eth_dev_stop() is updated accordingly:

- rte_eth_dev_start() returns (-EAGAIN) when transfer proxy port
  cannot be started.
- rte_eth_dev_stop() returns (-EBUSY) when port representor
  cannot be stopped.

Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
---
 doc/guides/nics/mlx5.rst        | 13 +++++++++++++
 drivers/net/mlx5/mlx5_trigger.c | 11 +++++++++++
 2 files changed, 24 insertions(+)

diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst
index 4f0db21dde..1df6ca9711 100644
--- a/doc/guides/nics/mlx5.rst
+++ b/doc/guides/nics/mlx5.rst
@@ -161,6 +161,19 @@ Limitations
   - NIC ConnectX-5 and before are not supported.
   - Partial match with item template is not supported.
   - IPv6 5-tuple matching is not supported.
+  - With E-Switch enabled, ports which share the E-Switch domain
+    should be started and stopped in a specific order:
+
+    - When starting ports, the transfer proxy port should be started first
+      and port representors should follow.
+    - When stopping ports, all of the port representors
+      should be stopped before stopping the transfer proxy port.
+
+    If ports are started/stopped in an incorrect order,
+    ``rte_eth_dev_start()``/``rte_eth_dev_stop()`` will return an appropriate error code:
+
+    - ``-EAGAIN`` for ``rte_eth_dev_start()``.
+    - ``-EBUSY`` for ``rte_eth_dev_stop()``.
 
 - When using Verbs flow engine (``dv_flow_en`` = 0), flow pattern without any
   specific VLAN will match for VLAN packets as well:
diff --git a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c
index fe6359908a..f54443ed1a 100644
--- a/drivers/net/mlx5/mlx5_trigger.c
+++ b/drivers/net/mlx5/mlx5_trigger.c
@@ -1138,6 +1138,10 @@ mlx5_hw_representor_port_allowed_start(struct rte_eth_dev *dev)
  *
  * @return
  *   0 on success, a negative errno value otherwise and rte_errno is set.
+ *   The following error values are defined:
+ *
+ *   - -EAGAIN: If port representor cannot be started,
+ *     because transfer proxy port is not started.
  */
 int
 mlx5_dev_start(struct rte_eth_dev *dev)
@@ -1394,6 +1398,13 @@ mlx5_hw_proxy_port_allowed_stop(struct rte_eth_dev *dev)
  *
  * @param dev
  *   Pointer to Ethernet device structure.
+ *
+ * @return
+ *   0 on success, a negative errno value otherwise and rte_errno is set.
+ *   The following error values are defined:
+ *
+ *   - -EBUSY: If transfer proxy port cannot be stopped,
+ *     because other port representors are still running.
  */
 int
 mlx5_dev_stop(struct rte_eth_dev *dev)
-- 
2.25.1

RE: [PATCH v3 2/3] net/mlx5: document E-Switch limitations with HWS in mlx5 PMD

[Slava Ovsiienko]

> -----Original Message-----
> From: Dariusz Sosnowski <dsosnowski@nvidia.com>
> Sent: Monday, November 14, 2022 20:20
> To: Matan Azrad <matan@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org
> Subject: [PATCH v3 2/3] net/mlx5: document E-Switch limitations with HWS in
> mlx5 PMD
> 
> This patch adds the following limitations to the mlx5 PMD guide:
> 
> - With HW Steering and E-Switch enabled, transfer proxy port must
>   be started before any port representor.
> - With HW Steering and E-Switch enabled, all representors
>   must be stopped before transfer proxy port is stopped.
> 
> Documentation of mlx5 PMD's implementations of
> rte_eth_dev_start() and rte_eth_dev_stop() is updated accordingly:
> 
> - rte_eth_dev_start() returns (-EAGAIN) when transfer proxy port
>   cannot be started.
> - rte_eth_dev_stop() returns (-EBUSY) when port representor
>   cannot be stopped.
> 
> Signed-off-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>

[PATCH v3 3/3] ethdev: document special cases of port start and stop

[Dariusz Sosnowski]

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 right now and start operation
  must be retried.
- If rte_eth_dev_stop() returns (-EBUSY) for some port,
  it cannot be stopped in the current state.

When stopping the port in testpmd fails,
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
failures to start the port.
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  | 8 +++++++-
 lib/ethdev/rte_ethdev.h | 2 ++
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 4e25f77c6a..a0b4ede48b 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -3182,6 +3182,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;
@@ -3231,9 +3232,14 @@ 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);
+			/* 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..c129ca1eaf 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -2705,6 +2705,7 @@ int rte_eth_dev_tx_queue_stop(uint16_t port_id, uint16_t tx_queue_id);
  *   The port identifier of the Ethernet device.
  * @return
  *   - 0: Success, Ethernet device started.
+ *   - -EAGAIN: If start operation must be retried.
  *   - <0: Error code of the driver device start function.
  */
 int rte_eth_dev_start(uint16_t port_id);
@@ -2717,6 +2718,7 @@ int rte_eth_dev_start(uint16_t port_id);
  *   The port identifier of the Ethernet device.
  * @return
  *   - 0: Success, Ethernet device stopped.
+ *   - -EBUSY: If stopping the port is not allowed in current state.
  *   - <0: Error code of the driver device stop function.
  */
 int rte_eth_dev_stop(uint16_t port_id);
-- 
2.25.1

Re: [PATCH v3 3/3] ethdev: document special cases of port start and stop

[Ferruh Yigit]

On 11/14/2022 6:19 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 right now and start operation
>    must be retried.
> - If rte_eth_dev_stop() returns (-EBUSY) for some port,
>    it cannot be stopped in the current state.
> 
> When stopping the port in testpmd fails,
> 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
> failures to start the port.
> 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>

Acked-by: Ferruh Yigit <ferruh.yigit@amd.com>

Re: [PATCH v3 0/3] ethdev: document special cases of port start and stop

[Thomas Monjalon]

> Dariusz Sosnowski (3):
>   net/mlx5: fix log level on failed transfer proxy stop
>   net/mlx5: document E-Switch limitations with HWS in mlx5 PMD
>   ethdev: document special cases of port start and stop

There is a change in testpmd but it looks simple enough for -rc4.
Applied, thanks.