Re: [PATCH v1] ethdev: add direction info when creating the transfer table

From: Ivan Malov <ivan.malov@oktetlabs.ru>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: Rongwei Liu <rongweil@nvidia.com>, Matan Azrad <matan@nvidia.com>,
	 Slava Ovsiienko <viacheslavo@nvidia.com>,
	Ori Kam <orika@nvidia.com>,
	 Aman Singh <aman.deep.singh@intel.com>,
	 Yuying Zhang <yuying.zhang@intel.com>,
	 Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>,
	 "dev@dpdk.org" <dev@dpdk.org>,
	Raslan Darawsheh <rasland@nvidia.com>
Subject: Re: [PATCH v1] ethdev: add direction info when creating the transfer table
Date: Thu, 15 Sep 2022 12:42:41 +0300 (MSK)	[thread overview]
Message-ID: <476338da-52c2-442c-b539-c6ab31d4538c@oktetlabs.ru> (raw)
In-Reply-To: <2472631.4XsnlVU6TS@thomas>

Hi Thomas,

On Thu, 15 Sep 2022, Thomas Monjalon wrote:

> 15/09/2022 09:47, Ivan Malov:
>> As I said, match criteria belong in flow pattern. I recognise the
>> importance of the problem that you're looking to solve. It's very
>> good that you care to address it, but what this patch tries to do
>> is to add more match criteria in the form of new attributes with
>> rather questionable names... There's a room for improvement.
>>
>> When I say that new features should not confuse readers, I mean
>> a very basic thing: readers know that match criteria all sit
>> in the pattern. And they refer to the pattern item enum in
>> the code and in documentation to learn about criteria,
>> while "struct rte_flow_attr" is an unusual place from
>> which to learn about match criteria.
>>
>>> We should get a conclusion and reflect in the commit changes&logs, and it's easy for others to absorb.
>>
>> Yes, but before we get to that, perhaps it pays to hear
>> more feedback from other reviewers. Thomas? Ori? Andrew?
>
> Sorry I did not read all.

OK, I will attempt to summarise it to some extent
in my next response to Rongwei which is to follow.

> I think the main question is about the use of attributes.
> I refer to this commit of Ivan last year which was agreed:
>
>    ethdev: deprecate direction attributes in transfer flows
>
>    Attributes "ingress" and "egress" can only apply unambiguosly
>    to non-"transfer" flows. In "transfer" flows, the standpoint
>    is effectively shifted to the embedded switch. There can be
>    many different endpoints connected to the switch, so the
>    use of "ingress" / "egress" does not shed light on which
>    endpoints precisely can be considered as traffic sources.
>
>    Add relevant deprecation notices and suggest the use of precise
>    traffic source items (PORT_REPRESENTOR and REPRESENTED_PORT).
>
>    Signed-off-by: Ivan Malov <ivan.malov@oktetlabs.ru>
>    Acked-by: Ori Kam <orika@nvidia.com>
>    Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>    Acked-by: Viacheslav Ovsiienko <viacheslavo@nvidia.com>
>
> So +1 for using only pattern items as matching criteria.

Thank you.

>
>
>

Ivan