[v11,2/5] ethdev: support proactive error handling mode

Message ID 20221009091009.38978-3-fengchengwen@huawei.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series support error handling mode |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

fengchengwen Oct. 9, 2022, 9:10 a.m. UTC
  From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>

Some PMDs (e.g. hns3) could detect hardware or firmware errors, and try
to recover from the errors. In this process, the PMD sets the data path
pointers to dummy functions (which will prevent the crash), and also
make sure the control path operations failed with retcode -EBUSY.

The above error handling mode is known as
RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE (proactive error handling mode).

In some service scenarios, application needs to be aware of the event
to determine whether to migrate services. So three events were
introduced:

1) RTE_ETH_EVENT_ERR_RECOVERING: used to notify the application that it
detected an error and the recovery is being started. Upon receiving the
event, the application should not invoke any control path APIs until
receiving RTE_ETH_EVENT_RECOVERY_SUCCESS or
RTE_ETH_EVENT_RECOVERY_FAILED event.

2) RTE_ETH_EVENT_RECOVERY_SUCCESS: used to notify the application that
it recovers successful from the error, the PMD already re-configures the
port, and the effect is the same as that of the restart operation.

3) RTE_ETH_EVENT_RECOVERY_FAILED: used to notify the application that it
recovers failed from the error, the port should not usable anymore. The
application should close the port.

Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
---
 app/test-pmd/config.c                   |  2 +
 doc/guides/prog_guide/poll_mode_drv.rst | 38 ++++++++++++++++
 doc/guides/rel_notes/release_22_11.rst  | 12 +++++
 lib/ethdev/rte_ethdev.h                 | 59 +++++++++++++++++++++++++
 4 files changed, 111 insertions(+)
  

Comments

Andrew Rybchenko Oct. 10, 2022, 8:47 a.m. UTC | #1
On 10/9/22 12:10, Chengwen Feng wrote:
> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> 
> Some PMDs (e.g. hns3) could detect hardware or firmware errors, and try
> to recover from the errors. In this process, the PMD sets the data path
> pointers to dummy functions (which will prevent the crash), and also
> make sure the control path operations failed with retcode -EBUSY.

Could you explain why passive mode is not good. Why is
proactive better? What are the benefits? IMHO, it would
be simpler to have just one error recovery mode.	
> 
> The above error handling mode is known as
> RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE (proactive error handling mode).
> 
> In some service scenarios, application needs to be aware of the event
> to determine whether to migrate services. So three events were
> introduced:
> 
> 1) RTE_ETH_EVENT_ERR_RECOVERING: used to notify the application that it
> detected an error and the recovery is being started. Upon receiving the
> event, the application should not invoke any control path APIs until
> receiving RTE_ETH_EVENT_RECOVERY_SUCCESS or
> RTE_ETH_EVENT_RECOVERY_FAILED event.
> 
> 2) RTE_ETH_EVENT_RECOVERY_SUCCESS: used to notify the application that
> it recovers successful from the error, the PMD already re-configures the
> port, and the effect is the same as that of the restart operation.
> 
> 3) RTE_ETH_EVENT_RECOVERY_FAILED: used to notify the application that it
> recovers failed from the error, the port should not usable anymore. The
> application should close the port.
> 
> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

The code itself LGTM. I just want to understand why we need it.
It should be proved in the description.
  
fengchengwen Oct. 11, 2022, 2:48 p.m. UTC | #2
Hi Andrew,

On 2022/10/10 16:47, Andrew Rybchenko wrote:
> On 10/9/22 12:10, Chengwen Feng wrote:
>> From: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
>>
>> Some PMDs (e.g. hns3) could detect hardware or firmware errors, and try
>> to recover from the errors. In this process, the PMD sets the data path
>> pointers to dummy functions (which will prevent the crash), and also
>> make sure the control path operations failed with retcode -EBUSY.
>
> Could you explain why passive mode is not good. Why is
> proactive better? What are the benefits? IMHO, it would
> be simpler to have just one error recovery mode.


I think the two modes are not good or bad. To a large extent, they are 
determined

by the hardware and software design of the network card chip. Here take 
the hns3

driver as an examples:

During the error recovery, multiple handshakes are required between the 
driver and

the firmware, in addition, the handshake timeout are required.

If chose passive mode, the application may not register the callback 
(and also we

found that only ovs-dpdk register the reset event in many DPDK-based 
opensource

software), so the recovery will failed.  Furthermore, even if registered 
the callback,

the recovery process involves multiple handshakes which may take too 
much time

to complete, imagine having multiple ports to report the reset time at 
the same time.

(This possibility exists. Consider that the PF is reset due to multiple 
VFs under the PF.)

In this case, many VFs report event, but the event callback is executed 
sequentially

(because there is only one interrupt thread). As a result, later VFs 
cannot be processed

in time, and the reset may fails.


In conclusion, the proactive mode is an available troubleshooting method in

engineering practice.


>>
>> The above error handling mode is known as
>> RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE (proactive error handling mode).
>>
>> In some service scenarios, application needs to be aware of the event
>> to determine whether to migrate services. So three events were
>> introduced:
>>
>> 1) RTE_ETH_EVENT_ERR_RECOVERING: used to notify the application that it
>> detected an error and the recovery is being started. Upon receiving the
>> event, the application should not invoke any control path APIs until
>> receiving RTE_ETH_EVENT_RECOVERY_SUCCESS or
>> RTE_ETH_EVENT_RECOVERY_FAILED event.
>>
>> 2) RTE_ETH_EVENT_RECOVERY_SUCCESS: used to notify the application that
>> it recovers successful from the error, the PMD already re-configures the
>> port, and the effect is the same as that of the restart operation.
>>
>> 3) RTE_ETH_EVENT_RECOVERY_FAILED: used to notify the application that it
>> recovers failed from the error, the port should not usable anymore. The
>> application should close the port.
>>
>> Signed-off-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
>> Signed-off-by: Somnath Kotur <somnath.kotur@broadcom.com>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
>
> The code itself LGTM. I just want to understand why we need it.
> It should be proved in the description.
>
  

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index bd7f2ba257..e53f2177b6 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -923,6 +923,8 @@  port_infos_display(portid_t port_id)
 	}
 	if (dev_info.err_handle_mode == RTE_ETH_ERROR_HANDLE_MODE_PASSIVE)
 		printf("Device error handling mode: passive\n");
+	else if (dev_info.err_handle_mode == RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE)
+		printf("Device error handling mode: proactive\n");
 }
 
 void
diff --git a/doc/guides/prog_guide/poll_mode_drv.rst b/doc/guides/prog_guide/poll_mode_drv.rst
index 9d081b1cba..73941a74bd 100644
--- a/doc/guides/prog_guide/poll_mode_drv.rst
+++ b/doc/guides/prog_guide/poll_mode_drv.rst
@@ -627,3 +627,41 @@  by application.
 The PMD itself should not call rte_eth_dev_reset(). The PMD can trigger
 the application to handle reset event. It is duty of application to
 handle all synchronization before it calls rte_eth_dev_reset().
+
+The above error handling mode is known as ``RTE_ETH_ERROR_HANDLE_MODE_PASSIVE``.
+
+Proactive Error Handling Mode
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+If PMD supports ``RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE``, it means once detect
+hardware or firmware errors, the PMD will try to recover from the errors. In
+this process, the PMD sets the data path pointers to dummy functions (which
+will prevent the crash), and also make sure the control path operations failed
+with retcode -EBUSY.
+
+Also in this process, from the perspective of application, services are
+affected. For example, the Rx/Tx bust APIs cannot receive and send packets,
+and the control plane API return failure.
+
+In some service scenarios, application needs to be aware of the event to
+determine whether to migrate services. So three events were introduced:
+
+* RTE_ETH_EVENT_ERR_RECOVERING: used to notify the application that it detected
+  an error and the recovery is being started. Upon receiving the event, the
+  application should not invoke any control path APIs until receiving
+  RTE_ETH_EVENT_RECOVERY_SUCCESS or RTE_ETH_EVENT_RECOVERY_FAILED event.
+
+* RTE_ETH_EVENT_RECOVERY_SUCCESS: used to notify the application that it
+  recovers successful from the error, the PMD already re-configures the port,
+  and the effect is the same as that of the restart operation.
+
+* RTE_ETH_EVENT_RECOVERY_FAILED: used to notify the application that it
+  recovers failed from the error, the port should not usable anymore. the
+  application should close the port.
+
+.. note::
+        * Before the PMD reports the recovery result, the PMD may report the
+          ``RTE_ETH_EVENT_ERR_RECOVERING`` event again, because a larger error
+          may occur during the recovery.
+        * The error handling mode supported by the PMD can be reported through
+          the ``rte_eth_dev_info_get`` API.
diff --git a/doc/guides/rel_notes/release_22_11.rst b/doc/guides/rel_notes/release_22_11.rst
index 10168ffece..b749f51405 100644
--- a/doc/guides/rel_notes/release_22_11.rst
+++ b/doc/guides/rel_notes/release_22_11.rst
@@ -111,6 +111,18 @@  New Features
   Added new flow action which allows application to re-route packets
   directly to the kernel without software involvement.
 
+* **Added proactive error handling mode for ethdev.**
+
+  Added proactive error handling mode for ethdev, and three event were
+  introduced:
+
+  * Added new event: ``RTE_ETH_EVENT_ERR_RECOVERING`` for the PMD to report
+    that the port is recovering from an error.
+  * Added new event: ``RTE_ETH_EVENT_RECOVER_SUCCESS`` for the PMD to report
+    that the port recover successful from an error.
+  * Added new event: ``RTE_ETH_EVENT_RECOVER_FAILED`` for the PMD to report
+    that the prot recover failed from an error.
+
 * **Updated AF_XDP driver.**
 
   * Made compatible with libbpf v0.8.0 (when used with libxdp).
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 3443bf20e1..ccdd6c8655 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -1643,6 +1643,12 @@  enum rte_eth_err_handle_mode {
 	 * application invoke @see rte_eth_dev_reset to recover the port.
 	 */
 	RTE_ETH_ERROR_HANDLE_MODE_PASSIVE,
+	/** Proactive error handling, after the PMD detect that a reset is
+	 * required, the PMD reports @see RTE_ETH_EVENT_ERR_RECOVERING event,
+	 * and do recovery internally, finally, reports the recovery result
+	 * event (@see RTE_ETH_EVENT_RECOVERY_*).
+	 */
+	RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE,
 };
 
 /**
@@ -3824,6 +3830,59 @@  enum rte_eth_event_type {
 	 * @see rte_eth_rx_avail_thresh_set()
 	 */
 	RTE_ETH_EVENT_RX_AVAIL_THRESH,
+	/** Port recovering from a hardware or firmware error.
+	 * If PMD supports proactive error recovery, it should trigger this
+	 * event to notify application that it detected an error and the
+	 * recovery is being started. Upon receiving the event, the application
+	 * should not invoke any control path APIs (such as
+	 * rte_eth_dev_configure/rte_eth_dev_stop...) until receiving
+	 * RTE_ETH_EVENT_RECOVERY_SUCCESS or RTE_ETH_EVENT_RECOVERY_FAILED
+	 * event.
+	 * The PMD will set the data path pointers to dummy functions, and
+	 * re-set the data patch pointers to non-dummy functions before reports
+	 * RTE_ETH_EVENT_RECOVERY_SUCCESS event. It means that the application
+	 * cannot send or receive any packets during this period.
+	 * @note Before the PMD reports the recovery result, the PMD may report
+	 * the RTE_ETH_EVENT_ERR_RECOVERING event again, because a larger error
+	 * may occur during the recovery.
+	 */
+	RTE_ETH_EVENT_ERR_RECOVERING,
+	/** Port recovers successful from the error.
+	 * The PMD already re-configures the port, and the effect is the same as
+	 * that of the restart operation.
+	 * a) the following operation will be retained: (alphabetically)
+	 *    - DCB configuration
+	 *    - FEC configuration
+	 *    - Flow control configuration
+	 *    - LRO configuration
+	 *    - LSC configuration
+	 *    - MTU
+	 *    - Mac address (default and those supplied by MAC address array)
+	 *    - Promiscuous and allmulticast mode
+	 *    - PTP configuration
+	 *    - Queue (Rx/Tx) settings
+	 *    - Queue statistics mappings
+	 *    - RSS configuration by rte_eth_dev_rss_xxx() family
+	 *    - Rx checksum configuration
+	 *    - Rx interrupt settings
+	 *    - Traffic management configuration
+	 *    - VLAN configuration (including filtering, tpid, strip, pvid)
+	 *    - VMDq configuration
+	 * b) the following configuration maybe retained or not depending on the
+	 *    device capabilities:
+	 *    - flow rules
+	 *      @see RTE_ETH_DEV_CAPA_FLOW_RULE_KEEP
+	 *    - shared flow objects
+	 *      @see RTE_ETH_DEV_CAPA_FLOW_SHARED_OBJECT_KEEP
+	 * c) the other configuration will not be stored and will need to be
+	 *    re-configured.
+	 */
+	RTE_ETH_EVENT_RECOVERY_SUCCESS,
+	/** Port recovers failed from the error.
+	 * It means that the port should not usable anymore. The application
+	 * should close the port.
+	 */
+	RTE_ETH_EVENT_RECOVERY_FAILED,
 	RTE_ETH_EVENT_MAX       /**< max value of this enum */
 };