Re: [PATCH v5 03/10] ethdev: bring in async queue-based flow rules operations

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 2/17/22 11:18, Thomas Monjalon wrote:
> 16/02/2022 23:49, Alexander Kozyrev:
>> On Sat, Feb 12, 2022 4:25 Thomas Monjalon <thomas@monjalon.net> wrote:
>>> 12/02/2022 03:19, Alexander Kozyrev:
>>>> On Fri, Feb 11, 2022 7:42 Andrew Rybchenko
>>> <andrew.rybchenko@oktetlabs.ru>:
>>>>> On 2/11/22 05:26, Alexander Kozyrev wrote:
>>>>>> +__rte_experimental
>>>>>> +struct rte_flow *
>>>>>> +rte_flow_q_flow_create(uint16_t port_id,
>>>>>
>>>>> flow_q_flow does not sound like a good nameing, consider:
>>>>> rte_flow_q_rule_create() is
>>> <subsystem>_<subtype>_<object>_<action>
>>>>
>>>> More like:
>>>> <subsystem>_<subtype>_<object>_<action>
>>>>   <rte>_<flow>_<rule_create_operation>_<queue>
>>>> Which is pretty lengthy name as for me.
>>>
>>> Naming :)
>>> This one may be improved I think.
>>> What is the problem with replacing "flow" with "rule"?
>>> Is it the right meaning?
>>
>> I've got a better naming for all the functions. What do you think about this?
>> Asynchronous rte_flow_async_create and rte_flow_async_destroy functions
>> as an extension of synchronous rte_flow_create/ rte_flow_destroy API.
>> The same is true for asynchronous API for indirect actions:
>> 	rte_flow_async_action_handle_create;
>> 	rte_flow_async_action_handle_destroy;
>> 	rte_flow_async_action_handle_update;
>> And rte_flow_push/rte_flow_pull without "_q_" part to make them clearer.
>> And yes, I'm still thinking pull is better than poll since we are actually retrieving
>> something, not just checking if it has something we can retrieve.
>> Let me know if we can agree on this scheme? Look pretty close to existing one.
> 
> I like the "async" word.
> 
> In summary, you propose this change for the functions of this patch:
> 
> 	rte_flow_q_flow_create           -> rte_flow_async_create
> 	rte_flow_q_flow_destroy          -> rte_flow_async_destroy
> 	rte_flow_q_action_handle_create  -> rte_flow_async_action_handle_create
> 	rte_flow_q_action_handle_destroy -> rte_flow_async_action_handle_destroy
> 	rte_flow_q_action_handle_update  -> rte_flow_async_action_handle_update
> 	rte_flow_q_push                  -> rte_flow_push
> 	rte_flow_q_pull                  -> rte_flow_pull
> 
> They are close to the exisiting synchronous function names:
> 
> 	rte_flow_create
> 	rte_flow_destroy
> 	rte_flow_action_handle_create
> 	rte_flow_action_handle_destroy
> 	rte_flow_action_handle_update
> 
> I think it is a good naming scheme.

+1