[v1,1/2] ethdev: clarify do not something in the new event

Message ID 20250115034110.15245-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. 15, 2025, 3:41 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.

Actually, when application receive a new event from one port, the port is
not fully probed and is just in allocated state. Application doesn't need
to verify the validity of the port id because it is definitely valid.
What's more, application shouldn't do something like configuring this port
or querying some information of this port by ethdev ops.

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

Comments

Thomas Monjalon Jan. 15, 2025, 11:31 a.m. UTC | #1
15/01/2025 04:41, 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.
> 
> Actually, when application receive a new event from one port, the port is
> not fully probed and is just in allocated state. Application doesn't need
> to verify the validity of the port id because it is definitely valid.
> What's more, application shouldn't do something like configuring this port
> or querying some information of this port by ethdev ops.
> 
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> ---
>  lib/ethdev/rte_ethdev.h | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> index 1f71cad244..e2021f0f12 100644
> --- a/lib/ethdev/rte_ethdev.h
> +++ b/lib/ethdev/rte_ethdev.h
> @@ -4128,7 +4128,14 @@ 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 */
> +	/** Port is probed and application's event callback will be called.

We are not going to say that the callback is called for each event :)

> +	 * In this moment, the port is not fully probed and is just in allocated
> +	 * state. When application receive this event, application doesn't need

It is not a real state.

> +	 * to verify the validity of the port id because it is definitely valid.
> +	 * What's more, application shouldn't do something like configuring this
> +	 * port or querying some information of this port by ethdev ops.
> +	 */

Let me try shorter:
"
The port is being probed, i.e. allocated and not yet available.
It is too early to check validity, infos, or configuring the port.
"

What do you think? Anything missing?
  
Huisong Li Jan. 16, 2025, 6:16 a.m. UTC | #2
在 2025/1/15 19:31, Thomas Monjalon 写道:
> 15/01/2025 04:41, 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.
>>
>> Actually, when application receive a new event from one port, the port is
>> not fully probed and is just in allocated state. Application doesn't need
>> to verify the validity of the port id because it is definitely valid.
>> What's more, application shouldn't do something like configuring this port
>> or querying some information of this port by ethdev ops.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   lib/ethdev/rte_ethdev.h | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>> index 1f71cad244..e2021f0f12 100644
>> --- a/lib/ethdev/rte_ethdev.h
>> +++ b/lib/ethdev/rte_ethdev.h
>> @@ -4128,7 +4128,14 @@ 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 */
>> +	/** Port is probed and application's event callback will be called.
> We are not going to say that the callback is called for each event :)
>
>> +	 * In this moment, the port is not fully probed and is just in allocated
>> +	 * state. When application receive this event, application doesn't need
> It is not a real state.
>
>> +	 * to verify the validity of the port id because it is definitely valid.
>> +	 * What's more, application shouldn't do something like configuring this
>> +	 * port or querying some information of this port by ethdev ops.
>> +	 */
> Let me try shorter:
> "
> The port is being probed, i.e. allocated and not yet available.
> It is too early to check validity, infos, or configuring the port.
> "
"
The port is being probed, i.e. allocated and not yet available.
It is too early to check validity, query infos, or configure the port.
But the socket id and owner id related to this port can be considered as 
valid.
"
How about use above comments?
>
>
> .
  
Thomas Monjalon Jan. 16, 2025, 9:15 a.m. UTC | #3
16/01/2025 07:16, lihuisong (C):
> 
> 在 2025/1/15 19:31, Thomas Monjalon 写道:
> > 15/01/2025 04:41, 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.
> >>
> >> Actually, when application receive a new event from one port, the port is
> >> not fully probed and is just in allocated state. Application doesn't need
> >> to verify the validity of the port id because it is definitely valid.
> >> What's more, application shouldn't do something like configuring this port
> >> or querying some information of this port by ethdev ops.
> >>
> >> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> >> ---
> >>   lib/ethdev/rte_ethdev.h | 9 ++++++++-
> >>   1 file changed, 8 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
> >> index 1f71cad244..e2021f0f12 100644
> >> --- a/lib/ethdev/rte_ethdev.h
> >> +++ b/lib/ethdev/rte_ethdev.h
> >> @@ -4128,7 +4128,14 @@ 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 */
> >> +	/** Port is probed and application's event callback will be called.
> > We are not going to say that the callback is called for each event :)
> >
> >> +	 * In this moment, the port is not fully probed and is just in allocated
> >> +	 * state. When application receive this event, application doesn't need
> > It is not a real state.
> >
> >> +	 * to verify the validity of the port id because it is definitely valid.
> >> +	 * What's more, application shouldn't do something like configuring this
> >> +	 * port or querying some information of this port by ethdev ops.
> >> +	 */
> > Let me try shorter:
> > "
> > The port is being probed, i.e. allocated and not yet available.
> > It is too early to check validity, infos, or configuring the port.
> > "
> "
> The port is being probed, i.e. allocated and not yet available.
> It is too early to check validity, query infos, or configure the port.
> But the socket id and owner id related to this port can be considered as 
> valid.
> "
> How about use above comments?

The last sentence should be in patch 2.
And I am not sure about saying they are valid.
It is more accurate to say that the functions for socket id and owner id are available.
  
Huisong Li Jan. 16, 2025, 9:35 a.m. UTC | #4
在 2025/1/16 17:15, Thomas Monjalon 写道:
> 16/01/2025 07:16, lihuisong (C):
>> 在 2025/1/15 19:31, Thomas Monjalon 写道:
>>> 15/01/2025 04:41, 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.
>>>>
>>>> Actually, when application receive a new event from one port, the port is
>>>> not fully probed and is just in allocated state. Application doesn't need
>>>> to verify the validity of the port id because it is definitely valid.
>>>> What's more, application shouldn't do something like configuring this port
>>>> or querying some information of this port by ethdev ops.
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> ---
>>>>    lib/ethdev/rte_ethdev.h | 9 ++++++++-
>>>>    1 file changed, 8 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
>>>> index 1f71cad244..e2021f0f12 100644
>>>> --- a/lib/ethdev/rte_ethdev.h
>>>> +++ b/lib/ethdev/rte_ethdev.h
>>>> @@ -4128,7 +4128,14 @@ 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 */
>>>> +	/** Port is probed and application's event callback will be called.
>>> We are not going to say that the callback is called for each event :)
>>>
>>>> +	 * In this moment, the port is not fully probed and is just in allocated
>>>> +	 * state. When application receive this event, application doesn't need
>>> It is not a real state.
>>>
>>>> +	 * to verify the validity of the port id because it is definitely valid.
>>>> +	 * What's more, application shouldn't do something like configuring this
>>>> +	 * port or querying some information of this port by ethdev ops.
>>>> +	 */
>>> Let me try shorter:
>>> "
>>> The port is being probed, i.e. allocated and not yet available.
>>> It is too early to check validity, infos, or configuring the port.
>>> "
>> "
>> The port is being probed, i.e. allocated and not yet available.
>> It is too early to check validity, query infos, or configure the port.
>> But the socket id and owner id related to this port can be considered as
>> valid.
>> "
>> How about use above comments?
> The last sentence should be in patch 2.
> And I am not sure about saying they are valid.
> It is more accurate to say that the functions for socket id and owner id are available.
Ack
>
>
> .
  

Patch

diff --git a/lib/ethdev/rte_ethdev.h b/lib/ethdev/rte_ethdev.h
index 1f71cad244..e2021f0f12 100644
--- a/lib/ethdev/rte_ethdev.h
+++ b/lib/ethdev/rte_ethdev.h
@@ -4128,7 +4128,14 @@  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 */
+	/** Port is probed and application's event callback will be called.
+	 * In this moment, the port is not fully probed and is just in allocated
+	 * state. When application receive this event, application doesn't need
+	 * to verify the validity of the port id because it is definitely valid.
+	 * What's more, application shouldn't do something like configuring this
+	 * port or querying some information of this port by ethdev ops.
+	 */
+	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 */