[dpdk-dev] [PATCH v1 01/12] ethdev: add ethdev item to flow API

[Ivan Malov]

Hi Ori,

On 04/10/2021 00:09, Ori Kam wrote:
> Hi Ivan,
> 
>> -----Original Message-----
>> From: Ivan Malov <Ivan.Malov at oktetlabs.ru>
>> Subject: Re: [PATCH v1 01/12] ethdev: add ethdev item to flow API
>>
>> Hi Ori,
>>
>> On 03/10/2021 14:52, Ori Kam wrote:
>>> Hi Andrew and Ivan,
>>>
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
>>>> Sent: Friday, October 1, 2021 4:47 PM
>>>> Subject: [PATCH v1 01/12] ethdev: add ethdev item to flow API
>>>>
>>>> From: Ivan Malov <ivan.malov at oktetlabs.ru>
>>>>
>>>> For use with "transfer" flows. Supposed to match traffic transmitted
>>>> by the DPDK application via the specified ethdev, at e-switch level.
>>>>
>>>> Must not be combined with attributes "ingress" / "egress".
>>>>
>>>> Signed-off-by: Ivan Malov <ivan.malov at oktetlabs.ru>
>>>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko at oktetlabs.ru>
>>>> ---
>>>
>>> [Snip]
>>>
>>>>    /** Generate flow_action[] entry. */ diff --git
>>>> a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h index
>>>> 7b1ed7f110..880502098e 100644
>>>> --- a/lib/ethdev/rte_flow.h
>>>> +++ b/lib/ethdev/rte_flow.h
>>>> @@ -574,6 +574,15 @@ enum rte_flow_item_type {
>>>>    	 * @see struct rte_flow_item_conntrack.
>>>>    	 */
>>>>    	RTE_FLOW_ITEM_TYPE_CONNTRACK,
>>>> +
>>>> +	/**
>>>> +	 * [META]
>>>> +	 *
>>>> +	 * Matches traffic at e-switch going from (sent by) the given ethdev.
>>>> +	 *
>>>> +	 * @see struct rte_flow_item_ethdev
>>>> +	 */
>>>> +	RTE_FLOW_ITEM_TYPE_ETHDEV,
>>>>    };
>>>>
>>>>    /**
>>>> @@ -1799,6 +1808,24 @@ static const struct rte_flow_item_conntrack
>>>> rte_flow_item_conntrack_mask = {  };  #endif
>>>>
>>>> +/**
>>>> + * @warning
>>>> + * @b EXPERIMENTAL: this structure may change without prior notice
>>>> + *
>>>> + * Provides an ethdev ID for use with items which are as follows:
>>>> + * RTE_FLOW_ITEM_TYPE_ETHDEV.
>>>> + */
>>>> +struct rte_flow_item_ethdev {
>>>> +	uint16_t id; /**< Ethdev ID */
>>>
>>> True for all above uses,
>>> should this be renamed to port?
>>
>> I'd not rename it to "port". The very idea of this series is to disambiguate
>> things. This structure is shared between primitives ETHDEV and
>> ESWITCH_PORT. If we go for "port", then in conjunction with ESWITCH_PORT
>> the structure name may trick readers into thinking that the ID in question is
>> the own ID of the e-switch port itself. But in fact this ID is an ethdev ID which
>> is associated with the e-switch port.
>>
>> Should you wish to elaborate on your concerns with regard to naming, please
>> do so. I'm all ears.
>>
> Fully understand and agree that the idea is to clear the ambiguaty.
> My concern is that we don't use ethdev id, from application ethdev has only
> ports, so what is the id? (if we keep this, we should document that the id is the
> port)
> What about ETHDEV_PORT and ESWITCH_PORT?

I understand that, technically, the only ports which the application can 
really interface with are ethdevs. So, terms "ethdev" and "port" may 
appear synonymous to the application - you are right on that. But, given 
the fact that we have some primitives like PHY_PORT and the likes, which 
also have "PORT" in their names, I'd rather go for "ethdev" as more 
precise term.

But let me assure you: I'm not saying that my opinion should prevail. 
I'm giving more thoughts to this in the background. Maybe Andrew can 
join this conversation as well.

> 
> Best,
> Ori
>>>
>>>> +};
>>>> +
>>>> +/** Default mask for RTE_FLOW_ITEM_TYPE_ETHDEV */ #ifndef
>>>> __cplusplus
>>>> +static const struct rte_flow_item_ethdev rte_flow_item_ethdev_mask
>> = {
>>>> +	.id = 0xffff,
>>>> +};
>>>> +#endif
>>>> +
>>>>    /**
>>>>     * Matching pattern item definition.
>>>>     *
>>>> --
>>>> 2.30.2
>>
>> --
>> Ivan M

-- 
Ivan M