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

[Ori Kam <orika@nvidia.com>]

Hi Ivan,

> -----Original Message-----
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> Sent: Thursday, 22 September 2022 15:43
> 
> Hi Ori,
> 
> On Thu, 22 Sep 2022, Ori Kam wrote:
> 
> > Hi Andrew,
> >
> >> -----Original Message-----
> >> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Sent: Thursday, 22 September 2022 10:39
> >>
> >> On 9/21/22 15:51, Morten Brørup wrote:
> >>>> From: Ori Kam [mailto:orika@nvidia.com]
> >>>> Sent: Wednesday, 21 September 2022 14.41
> >>>>
> >>>>> From: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >>>>>
> >>>>> On 9/21/22 12:40, Thomas Monjalon wrote:
> >>>>>> 21/09/2022 11:04, Ivan Malov:
> >>>>>>> Now it's clear to me that your intention is to match on exact
> >>>> ports,
> >>>>>>> as usual, but this time with a hint for the flow table. Got it.
> >>>>>>>
> >>>>>>> In your response, you say that matching on ALL vports is not what
> >>>>>>> the use case needs. OK, I understood. But please note that the
> >>>>>>> item name does not say "ALL", it says "ANY".
> >>>>>>>
> >>>>>>> OK. Say, "ANY" is also confusing. Let's then name it
> "VPORTS_ONLY"
> >>>>>>> and "PHY_PORTS_ONLY". This way, if user provides item
> >> VPORTS_ONLY
> >>>>>>> and then  provides item REPRESENTED_PORT, these two items do
> not
> >>>>>>> contradict each other. Item VPORTS_ONLY defines the scope of
> some
> >>>>>>> kind, then the following item, REPRESENTED_PORT, makes it
> >>>> narrower.
> >>>>>>>
> >>>>>>> And, in documentation, one can say clearly that the user *may*
> >>>>>>> omit item VPORTS_ONLY in the exact rule pattern provided that
> >>>>>>> they have already submitted this item as part of the template.
> >>>>>>
> >>>>>> I think the problem that Rongwei & Ori are trying to solve
> >>>>>> is to allocate resources for the templates table in the right
> >>>> place.
> >>>>>> A table can have multiple templates.
> >>>>>> If all rules/templates for this table are dedicated to virtual
> >>>> ports,
> >>>>>> then the table will be allocated in a place managing only virtual
> >>>> ports.
> >>>>>> This allocation decision must be taken at table creation,
> >>>>>> whereas rules will be created later.
> >>>>>> In order to do this specific table allocation for vports,
> >>>>>> we need to restrict all templates of the table to be "vports only".
> >>>>>>
> >>>>>> I hope it makes things clearer.
> >>>>>> Now the question is how to achieve this? Solutions are:
> >>>>>>
> >>>>>> 1/ give a hint to the table allocation
> >>>>>> 2/ insert a pattern item in all templates of the table
> >>>>>>
> >>>>>> I don't see any other solution. Please propose if there are more
> >>>> options.
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> See my mail
> >>>>>
> >>>>> 3/ use jump rule which ensures that all traffic meets out
> >>>>>      expectations
> >>>>>
> >>>>> It means that the table creation could be postponed. Or the
> >>>>> table could be per-configured at the point of creation and
> >>>>> finalized when we know that all traffic will be from wires
> >>>>> or from vports. Yes, it complicates internals to achieve
> >>>>> the optimization.
> >>>>
> >>>> Sorry Andrew your suggestion is not a valid one for the following
> >>>> reasons:
> >>>> 1. table creation can't be postponed this is a key idea of the rte_flow
> >>>> template API.
> >>
> >> I guess nobody cares if it delays insertion on the first rule
> >> only. Anyway, see below.
> >>
> >>>> 2. we can never know what rules will be inserted if the application
> >>>> doesn't tell us.
> >>>>       how can we know this is the last rule? What do we do with the
> >>>> first rule?
> >>>> 3. I don't see how jumping helps since it worsens the issue when you
> >>>> jump to a table,
> >>>>      how does the PMD know if this table should have only wire or only
> >>>> vports?
> >>
> >> Jump rules say so. PMD can analyze there rules.
> >> May be just need an attribute saying that all jump rules
> >> to the table are configured and further attempts to reconfigure
> >> will be rejected?
> >>
> >
> > The idea is the PMD will not analyze rules. That is why we have the table
> > and template.
> 
> PMDs will not analyze **rules**, yes. But that does not dismiss the
> need to analyze **tables** and **templates** when they are created.
> I.e. table/template creation is some sort of "cold"/"slow" path.
> The PMD sees the item in the pattern and translates it to the
> internal representation of the table. Just like it **would**
> do in case of the attribute approach. But when the rules
> are inserted (**hot** async path), the PMD should just
> collect exact "spec" values from the pattern without
> analyzing it, as per the previously learned template.
> 

Right so why should we force the application to give us what 
we both agree is just an hint for each rule, the PMD will not use it
so why give it?

> From the HW resource usage perspective (in your case),
> why isn't such design good enough?
> 

From my first reply I told you that both ways can work.
I think as SW developer (not as Nvidia guy) that since this is an attribute
for the table, just like you said the only place we use it is during table creation
the correct place for it is in the table and not as part of the pattern.
Pure SW design.

> > Sorry, I don't understand what attribute can be in jump? The jump is just
> > to table. It can't say anything about the table destination table.
> > This is all this patch adds the attribute to a table to say where this
> > table should be located.
> >
> >>>>
> >>>> I agree with Thomas, there are two valid options, I vote for the hint
> >>>> since this is the
> >>>> feature idea to tell the PMD where this resource should be allocated.
> >>>
> >>> This is an optimization; I agree with Ori that a hint is appropriate, like the
> >> MBUF_FAST_FREE hint on TX queues.
> >>>
> >>> No need to add more complexity by requiring the driver to recognize
> that
> >> the pattern is present in all templates. (And perhaps also remove that
> >> pattern when applying the templates.)
> >>
> >> What does the part of the matching criteria so special
> >> that it is allowed to have dedicated hint attribute?
> >>
> >> May be we can have really generic solution when any
> >> part of the matching criteria could provide such hints?
> >
> > That is the point I keep returning to, it is not matching!
> 
> Let's face it: these attributes are in fact matching, which,
> in the case of MLX5, is translated into resource properties.
> I.e., to MLX5 (internally!), these attributes are indeed
> not matching but separate resource allocation. Got it.
> 

Happy to hear.

> But what about other vendors? I guess, hardly can someone
> say for sure that others' internals work the same way...
> 

Maybe some please see my  answer to Andrew,
the idea is that we in DPDK want the best insertion for all vendors,
any vendor that thinks he can get a perf boost by getting a hint from 
the application will get my support. This is the idea of template API and
fast insertion.

> > This is on which HW resource the table should be allocated.
> > Think about ingress/egress/transfer why are they not in the  pattern?
> 
> - ingres/egress only applies to non-transfer rules
>    and serves to catch either incoming or outcoming
>    traffic of the single "door" (ethdev)
> 
>    (furthermore, these attributes had been defined
>     long before the transfer concept was added, so
>     even if we NOW realise these attributes **could**
>     have been expressed in the form of items, I'm
>     afraid it's no use crying over spilt milk)
> 
> - transfer is not in pattern because it is not
>    a match criterion; it is in fact the indication
>    of which **match engine** to use: either the
>    one of the embedded switch or the one of
>    the vNIC / ethdev
> 

This was just to show some point that there are cases that
even if something could be used as item maybe it is not the best
way.
In any case just like my reply about. From pure SW point of view
I think that it more correct to have it as an attribute.

> > They are where rules should be offloaded, they are different domain.
> 
> It's OK to say that generic concept of "embedded switch level",
> or "transfer domain", in the case of MLX5, is in turn split
> into two different HW domains, - it's vendor-specific
> internals, - but it's not OK to assume that the same
> separation is also valid for other vendors.
> 

Never said it is true to all vendors, I guess to some but for sure not all of them.
Just like I'm sure not all vendors will use other hints.

> > Like we have elsewhere for example in action create we can state on which
> > domain the action should be created. If the application selects a number of
> domains
> > it may mean that extra resources will be allocated.
> 
> Could you please expand on this / give an example?
> Just for me to check whether my point of view
> could be wrong based on the example or not.
>

Let's look at rte_flow_action_handle_create one of the conf parameters is ingress/egress/transfer
application may mark an action to be used only in ingress or in ingress+egress
if the application selects ingress+egress it is possible that insertion rate and PPS
maybe slower.
I hope this makes it clearer.

Best,
Ori
 
> >
> >
> >
> >
> >
> 
> Ivan