[v2,1/2] ethdev: clarify something about the new event

Message ID 20250116114034.9858-2-lihuisong@huawei.com (mailing list archive)
State Superseded
Delegated to: Stephen Hemminger
Headers
Series ethdev: clarify something about new event |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Huisong Li Jan. 16, 2025, 11:40 a.m. UTC
If application verify the validity of the port id or configure this port in
the new event callback, application may happen to the port id is invalid.

In case of similar confusion, this patch have to clarify something about
RTE_ETH_EVENT_NEW in code.

Signed-off-by: Huisong Li <lihuisong@huawei.com>
---
 lib/ethdev/rte_ethdev.h | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
  

Comments

Thomas Monjalon Jan. 16, 2025, 3:17 p.m. UTC | #1
16/01/2025 12:40, Huisong Li:
> If application verify the validity of the port id or configure this port in
> the new event callback, application may happen to the port id is invalid.
> 
> In case of similar confusion, this patch have to clarify something about
> RTE_ETH_EVENT_NEW in code.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
> -	RTE_ETH_EVENT_NEW,      /**< port is probed */
> +	/** The port is being probed, i.e. allocated and not yet available.
> +	 * It is too early to check validity, query infos, and configure
> +	 * the port.
> +	 */
> +	RTE_ETH_EVENT_NEW,

Acked-by: Thomas Monjalon <thomas@monjalon.net>
  
Stephen Hemminger Jan. 16, 2025, 6:31 p.m. UTC | #2
On Thu, 16 Jan 2025 19:40:33 +0800
Huisong Li <lihuisong@huawei.com> wrote:

> If application verify the validity of the port id or configure this port in
> the new event callback, application may happen to the port id is invalid.
> 
> In case of similar confusion, this patch have to clarify something about
> RTE_ETH_EVENT_NEW in code.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>  lib/ethdev/rte_ethdev.h | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 1f71cad244..ee7197aa97 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -4128,7 +4128,11 @@ enum rte_eth_event_type {
>  	RTE_ETH_EVENT_VF_MBOX,  /**< message from the VF received by PF */
>  	RTE_ETH_EVENT_MACSEC,   /**< MACsec offload related event */
>  	RTE_ETH_EVENT_INTR_RMV, /**< device removal event */
> -	RTE_ETH_EVENT_NEW,      /**< port is probed */
> +	/** The port is being probed, i.e. allocated and not yet available.
> +	 * It is too early to check validity, query infos, and configure
> +	 * the port.
> +	 */
> +	RTE_ETH_EVENT_NEW,
>  	RTE_ETH_EVENT_DESTROY,  /**< port is released */
>  	RTE_ETH_EVENT_IPSEC,    /**< IPsec offload related event */
>  	RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected */

All the comments in event_type need some editing to make them more
readable. It is good style when having a list to make sure that
each item in a list agrees in terms of wording, verb tense,
capitalization, description, etc.

This can be addressed by a later patch, ideally by looking
at the resulting API doc and fixing the source to match.
  
Huisong Li Jan. 17, 2025, 8:43 a.m. UTC | #3
在 2025/1/17 2:31, Stephen Hemminger 写道:
> On Thu, 16 Jan 2025 19:40:33 +0800
> Huisong Li <lihuisong@huawei.com> wrote:
>
>> If application verify the validity of the port id or configure this port in
>> the new event callback, application may happen to the port id is invalid.
>>
>> In case of similar confusion, this patch have to clarify something about
>> RTE_ETH_EVENT_NEW in code.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   lib/ethdev/rte_ethdev.h | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index 1f71cad244..ee7197aa97 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -4128,7 +4128,11 @@ enum rte_eth_event_type {
>>   	RTE_ETH_EVENT_VF_MBOX,  /**< message from the VF received by PF */
>>   	RTE_ETH_EVENT_MACSEC,   /**< MACsec offload related event */
>>   	RTE_ETH_EVENT_INTR_RMV, /**< device removal event */
>> -	RTE_ETH_EVENT_NEW,      /**< port is probed */
>> +	/** The port is being probed, i.e. allocated and not yet available.
>> +	 * It is too early to check validity, query infos, and configure
>> +	 * the port.
>> +	 */
>> +	RTE_ETH_EVENT_NEW,
>>   	RTE_ETH_EVENT_DESTROY,  /**< port is released */
>>   	RTE_ETH_EVENT_IPSEC,    /**< IPsec offload related event */
>>   	RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected */
> All the comments in event_type need some editing to make them more
> readable. It is good style when having a list to make sure that
> each item in a list agrees in terms of wording, verb tense,
> capitalization, description, etc.
I am not sure if it is appropriate just to modify the event type comments.
After all, it is common issue in doc.
>
> This can be addressed by a later patch, ideally by looking
> at the resulting API doc and fixing the source to match.
  

Patch

diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 1f71cad244..ee7197aa97 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -4128,7 +4128,11 @@  enum rte_eth_event_type {
 	RTE_ETH_EVENT_VF_MBOX,  /**< message from the VF received by PF */
 	RTE_ETH_EVENT_MACSEC,   /**< MACsec offload related event */
 	RTE_ETH_EVENT_INTR_RMV, /**< device removal event */
-	RTE_ETH_EVENT_NEW,      /**< port is probed */
+	/** The port is being probed, i.e. allocated and not yet available.
+	 * It is too early to check validity, query infos, and configure
+	 * the port.
+	 */
+	RTE_ETH_EVENT_NEW,
 	RTE_ETH_EVENT_DESTROY,  /**< port is released */
 	RTE_ETH_EVENT_IPSEC,    /**< IPsec offload related event */
 	RTE_ETH_EVENT_FLOW_AGED,/**< New aged-out flows is detected */