On 10/4/21 3:00 AM, Ivan Malov wrote:
> Hi Ori,
>
> On 04/10/2021 00:09, Ori Kam wrote:
>> Hi Ivan,
>>
>>> -----Original Message-----
>>> From: Ivan Malov <ivan.ma...@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.rybche...@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.ma...@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.ma...@oktetlabs.ru>
>>>>> Signed-off-by: Andrew Rybchenko <andrew.rybche...@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.
As far as I can see ethdev API uses 'port_id' everywhere to
refer to ethdev port by its number. So, I suggest
struct rte_flow_item_ethdev {
uint16_t port_id; /**< ethdev port ID */
};
Basically I agree with Ori, that just "id" is a bit confusing
even when it is a member of the _ethdev structure, but I'd
prepend "port_" a field name to sync with ethdev API which
uses port_id. So, we have ethdev->port_id.
Andrew.
