[v9,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 | 19 ++++++++++++++++++-
7 files changed, 30 insertions(+), 1 deletion(-)
Comments
On 9/22/2022 8:41 AM, 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>
<...>
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index de9e970d4d..930b0a2fff 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -1848,6 +1848,19 @@ enum rte_eth_representor_type {
> RTE_ETH_REPRESENTOR_PF, /**< representor of Physical Function. */
> };
>
> +/**
> + * Ethernet device error handling mode.
Needs to be experimental, if decides to keep.
> + */
> +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,
Hi Chengwen,
Is the intention of 'PASSIVE' / 'PROACTIVE' mode to let application
decide which event to register? Like some kind of capability?
If mode == RTE_ETH_ERROR_HANDLE_MODE_PASSIVE
register RTE_ETH_EVENT_INTR_RESET
if mode == RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE
register ERR_RECOVERING | RECOVERY_SUCCESS | RECOVERY_FAILED
Can't a PMD support both?
Or is application really needs to know this, what happens if it register
all events and implements related actions for it?
> +};
> +
> /**
> * A structure used to retrieve the contextual information of
> * an Ethernet device, such as the controlling driver of the
> @@ -1908,8 +1921,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;
>
I guess 'uint8_t' is used to save space, but 'enum' is mostly integer
(although as far as I remember compiler can select smaller type is cases
fit it), so I concern if it case any warning. If not agree to use
smaller type, since we know possible number of handler type is limited
and small.
> - 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 */
> };
>
Hi Ferruh,
On 2022/10/4 1:35, Ferruh Yigit wrote:
> On 9/22/2022 8:41 AM, 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>
>
> <...>
>
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index de9e970d4d..930b0a2fff 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -1848,6 +1848,19 @@ enum rte_eth_representor_type {
>> RTE_ETH_REPRESENTOR_PF, /**< representor of Physical
>> Function. */
>> };
>> +/**
>> + * Ethernet device error handling mode.
>
> Needs to be experimental, if decides to keep.
will fix in v10
>
>> + */
>> +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,
>
> Hi Chengwen,
>
> Is the intention of 'PASSIVE' / 'PROACTIVE' mode to let application
> decide which event to register? Like some kind of capability?
>
> If mode == RTE_ETH_ERROR_HANDLE_MODE_PASSIVE
> register RTE_ETH_EVENT_INTR_RESET
>
> if mode == RTE_ETH_ERROR_HANDLE_MODE_PROACTIVE
> register ERR_RECOVERING | RECOVERY_SUCCESS | RECOVERY_FAILED
>
It mainly to standardize the two error handling modes to avoid poor
perception.
In the concept space, the reset mode is separated, so that it is not
difficult to understand.
> Can't a PMD support both?
Currently, I find that no PMD support both.
If the new PMD supports both the two types, it can be extended well, for
example, it can be defined as bitmask,
and value should not change because PASSIVE correspond 1(1<<0), and
PROACTIVE correspond 2(1<<1)
>
> Or is application really needs to know this, what happens if it
> register all events and implements related actions for it?
For simpler, the application could register all events and do what the
framework requirements.
>
>
>> +};
>> +
>> /**
>> * A structure used to retrieve the contextual information of
>> * an Ethernet device, such as the controlling driver of the
>> @@ -1908,8 +1921,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;
>
> I guess 'uint8_t' is used to save space, but 'enum' is mostly integer
> (although as far as I remember compiler can select smaller type is
> cases fit it), so I concern if it case any warning. If not agree to
> use smaller type, since we know possible number of handler type is
> limited and small.
Yes, uint8_t is used to save space. It will depend on compiler if use
enum here, so I think it's OK to use deterministic type.
as for warning, I have not get such converting warning yeth.
>
>> - 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 */
>> };
>
@@ -922,6 +922,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;
}
@@ -1178,6 +1178,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;
}
@@ -1848,6 +1848,19 @@ enum rte_eth_representor_type {
RTE_ETH_REPRESENTOR_PF, /**< representor of Physical Function. */
};
+/**
+ * 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
@@ -1908,8 +1921,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 */
};