On 10/9/22 12:10, Chengwen Feng wrote:
This patch support gets port's error handling mode by
rte_eth_dev_info_get() API.
Just: "Add error handling mode to device info."
Currently, the defined modes include:
1) NONE: it means no error handling modes are supported by this port.
2) PASSIVE: passive error handling, after the PMD detect that a reset
is required, the PMD reports RTE_ETH_EVENT_INTR_RESET event, and
application invoke rte_eth_dev_reset() to recover the port.
Signed-off-by: Chengwen Feng <[email protected]>
With review notes applied (may be except usage of reserved
fields):
Reviewed-by: Andrew Rybchenko <[email protected]>
diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index e8d1e1c658..3443bf20e1 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -1629,6 +1629,22 @@ enum rte_eth_representor_type {
RTE_ETH_REPRESENTOR_PF, /**< representor of Physical Function. */
};
+/**
+ * @warning
+ * @b EXPERIMENTAL: this enumeration may change without prior notice.
+ *
+ * Ethernet device error handling mode.
+ */
+enum rte_eth_err_handle_mode {
+ /** No error handling modes are supported. */
+ RTE_ETH_ERROR_HANDLE_MODE_NONE,
+ /** Passive error handling, after the PMD detect that a reset is
+ * required, the PMD reports @see RTE_ETH_EVENT_INTR_RESET event, and
+ * application invoke @see rte_eth_dev_reset to recover the port.
+ */
+ RTE_ETH_ERROR_HANDLE_MODE_PASSIVE,
+};
+
/**
* A structure used to retrieve the contextual information of
* an Ethernet device, such as the controlling driver of the
@@ -1689,8 +1705,12 @@ struct rte_eth_dev_info {
* embedded managed interconnect/switch.
*/
struct rte_eth_switch_info switch_info;
+ /** Supported error handling mode. @see enum rte_eth_err_handle_mode */
+ uint8_t err_handle_mode;
IMHO, it must be
enum rte_eth_err_handle_mode err_handle_mode;
Yes, it takes a bit more space, but it is a control path and
code clearness is more important here than few extra bytes.
- uint64_t reserved_64s[2]; /**< Reserved for future fields */
+ uint8_t reserved_8; /**< Reserved for future fields */
+ uint16_t reserved_16s[3]; /**< Reserved for future fields */
+ uint64_t reserved_64; /**< Reserved for future fields */
As far as I know it is done as per Stephen review notes, but
I'm not really sure why it is a right way in ABI breaking
release. I'd not touch it and just add a new field.
void *reserved_ptrs[2]; /**< Reserved for future fields */
};
