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

[Ivan Malov <Ivan.Malov@oktetlabs.ru>]

Hi Ori,

On 03/10/2021 14:52, Ori Kam wrote:
> Hi Andrew and Ivan,
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <andrew.rybchenko@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@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@oktetlabs.ru>
>> Signed-off-by: Andrew Rybchenko <andrew.rybchenko@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.

> 
>> +};
>> +
>> +/** 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