[v11,1/5] ethdev: support get port error handling mode
Checks
Commit Message
This patch support gets port's error handling mode by
rte_eth_dev_info_get() API.
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 <fengchengwen@huawei.com>
---
app/test-pmd/config.c | 2 ++
drivers/net/e1000/igb_ethdev.c | 2 ++
drivers/net/ena/ena_ethdev.c | 2 ++
drivers/net/iavf/iavf_ethdev.c | 2 ++
drivers/net/ixgbe/ixgbe_ethdev.c | 2 ++
drivers/net/txgbe/txgbe_ethdev_vf.c | 2 ++
lib/ethdev/rte_ethdev.h | 22 +++++++++++++++++++++-
7 files changed, 33 insertions(+), 1 deletion(-)
Comments
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 <fengchengwen@huawei.com>
With review notes applied (may be except usage of reserved
fields):
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> 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 */
> };
>
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.
>
> 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 <fengchengwen@huawei.com>
in fact one more point below
> ---
> app/test-pmd/config.c | 2 ++
> drivers/net/e1000/igb_ethdev.c | 2 ++
> drivers/net/ena/ena_ethdev.c | 2 ++
> drivers/net/iavf/iavf_ethdev.c | 2 ++
> drivers/net/ixgbe/ixgbe_ethdev.c | 2 ++
> drivers/net/txgbe/txgbe_ethdev_vf.c | 2 ++
> lib/ethdev/rte_ethdev.h | 22 +++++++++++++++++++++-
> 7 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
> index 841e8efe78..bd7f2ba257 100644
> --- a/app/test-pmd/config.c
> +++ b/app/test-pmd/config.c
> @@ -921,6 +921,8 @@ port_infos_display(portid_t port_id)
> printf("Switch Rx domain: %u\n",
> dev_info.switch_info.rx_domain);
> }
> + if (dev_info.err_handle_mode == RTE_ETH_ERROR_HANDLE_MODE_PASSIVE)
> + printf("Device error handling mode: passive\n");
It should be done using switch/case instead of if/elseif.
Also I'd say that none should be handled as well.
> }
>
> void
@@ -921,6 +921,8 @@ port_infos_display(portid_t port_id)
printf("Switch Rx domain: %u\n",
dev_info.switch_info.rx_domain);
}
+ if (dev_info.err_handle_mode == RTE_ETH_ERROR_HANDLE_MODE_PASSIVE)
+ printf("Device error handling mode: passive\n");
}
void
@@ -2341,6 +2341,8 @@ eth_igbvf_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
dev_info->rx_desc_lim = rx_desc_lim;
dev_info->tx_desc_lim = tx_desc_lim;
+ dev_info->err_handle_mode = RTE_ETH_ERROR_HANDLE_MODE_PASSIVE;
+
return 0;
}
@@ -2482,6 +2482,8 @@ static int ena_infos_get(struct rte_eth_dev *dev,
dev_info->default_rxportconf.ring_size = ENA_DEFAULT_RING_SIZE;
dev_info->default_txportconf.ring_size = ENA_DEFAULT_RING_SIZE;
+ dev_info->err_handle_mode = RTE_ETH_ERROR_HANDLE_MODE_PASSIVE;
+
return 0;
}
@@ -1179,6 +1179,8 @@ iavf_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
.nb_align = IAVF_ALIGN_RING_DESC,
};
+ dev_info->err_handle_mode = RTE_ETH_ERROR_HANDLE_MODE_PASSIVE;
+
return 0;
}
@@ -4056,6 +4056,8 @@ ixgbevf_dev_info_get(struct rte_eth_dev *dev,
dev_info->rx_desc_lim = rx_desc_lim;
dev_info->tx_desc_lim = tx_desc_lim;
+ dev_info->err_handle_mode = RTE_ETH_ERROR_HANDLE_MODE_PASSIVE;
+
return 0;
}
@@ -521,6 +521,8 @@ txgbevf_dev_info_get(struct rte_eth_dev *dev,
dev_info->rx_desc_lim = rx_desc_lim;
dev_info->tx_desc_lim = tx_desc_lim;
+ dev_info->err_handle_mode = RTE_ETH_ERROR_HANDLE_MODE_PASSIVE;
+
return 0;
}
@@ -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;
- 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 */
void *reserved_ptrs[2]; /**< Reserved for future fields */
};