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.ma...@oktetlabs.ru>
   Acked-by: Ori Kam <or...@nvidia.com>
   Acked-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
   Acked-by: Viacheslav Ovsiienko <viachesl...@nvidia.com>

So +1 for using only pattern items as matching criteria.

Thank you.





Ivan

Reply via email to