RE: [PATCH v7] ethdev: add special flags when creating async transfer table

[Ori Kam <orika@nvidia.com>]

Hi 

> -----Original Message-----
> From: Thomas Monjalon <thomas@monjalon.net>
> Sent: Wednesday, 1 February 2023 12:59
> 
> 01/02/2023 11:17, Andrew Rybchenko:
> > On 1/18/23 19:18, Thomas Monjalon wrote:
> > > 18/01/2023 08:28, Andrew Rybchenko:
> > >> On 11/14/22 14:59, Rongwei Liu wrote:
> > >>> In case flow rules match only one kind of traffic in a flow table,
> > >>> then optimization can be done via allocation of this table.
> > >>> Such optimization is possible only if the application gives a hint
> > >>> about its usage of the table during initial configuration.
> > >>>
> > >>> The transfer domain rules may process traffic from wire or vport,
> > >>> which may correspond to two kinds of underlayer resources.
> > >>> That's why the first two hints introduced in this patch are about
> > >>> wire and vport traffic specialization.
> > >>> Wire means traffic arrives from the uplink port while vport means
> > >>> traffic initiated from VF/SF.
> > >>>
> > >>> There are two possible approaches for providing the hints.
> > >>> Using IPv4 as an example:
> > >>> 1. Use pattern item in both template table and flow rules.
> > >>>
> > >>>      pattern_template: pattern ANY_VPORT / eth / ipv4 is 1.1.1.1 / end
> > >>>      async flow create: pattern ANY_VPORT / eth / ipv4 is 1.1.1.2 / end
> > >>>
> > >>>      "ANY_VPORT" needs to be present in each flow rule even if it's
> > >>>      just a hint. No value to match because matching is already done by
> > >>>      IPv4 item.
> > >>>
> > >>> 2. Add special flags into table_attr.
> > >>>
> > >>>      template_table 0 create table_id 0 group 1 transfer vport_orig
> > >>>
> > >>> Approach 1 needs to specify the pattern in each flow rule which wastes
> > >>> memory and is not user friendly.
> > >>> This patch takes the 2nd approach and introduces one new member
> > >>> "specialize" into rte_flow_table_attr to indicate possible flow table
> > >>> optimization.
> > >>
> > >> The above description is misleading. It alternates options (1)
> > >> and (2), but in fact (2) requires (1) as well.
> > >
> > > Yes the above description may be misleading
> > > and it seems you are misleaded :)
> >
> > It is not my intention. If it is only my problem, I'm OK to
> > step back.
> 
> It's OK to explain and check everything is OK, no worries.
> Thanks for reviewing.
> 
> > > I will explain below why the option (2) doesn't require (1).
> > > I think we should apply the same example to both cases to make it clear:
> > >
> > > 1. Use pattern item in both template table and flow rules:
> > >
> > >     template table 3 = transfer pattern ANY_VPORT / eth / ipv4 src is
> 255.255.255.255 / end
> > >     flow rule = template_table 3 pattern ANY_VPORT / eth / ipv4 src is
> 1.1.1.1 / end
> > >
> > >     The pattern template 3 will be used only to match flows coming from
> vports.
> > >     ANY_VPORT needs to be present in each flow rule.
> >
> > It looks like I lost something here. Why do we need to specify
> > it in each flow rule if the matching is already fixed in
> > template table?
> 
> I think that's how template tables are designed.
> Ori, please could you point us to the relevant documentation?
> 

Two things:
1. we don't have at this point a way in the rte_flow template tables a way to set a fix match.
Meaning to say all rules should match ip src = x
When we create a template we are just saying on which fields/items we want to match
not the value.
So if ip src is masked it mean that we will want to match on the ip src but each rule
will specify the exact value.
Side note In the action we do have a way to set fixed action for this template.

2. I think in the above example, the idea is to show that the src marks the VM (vport)
This is why we need to match only on traffic coming from VM and not wire
since if the src is coming from uplink it doesn't have anything to do with the requested rule.
 
> > >     ANY_VPORT matching is redundant with IP src 1.1.1.1 because
> > >     the user knows 1.1.1.1 is the IP of a vport.
> >
> > What should happen if a packet with src IP 1.1.1.1 comes from
> > the wire? Almost anything could come from network.
> 
> It a packet comes from a wired port AND
> the PMD did an optimization based on this hint,
> then the packet could be not matched.

This is the idea the rule will not match on it. See my above comment.
> 
> > > 2. Add specialization flag into template table attribute:
> > >
> > >     template table 3 = transfer VPORT_ORIG pattern eth / ipv4 src is
> 255.255.255.255 / end
> > >     flow rule = template_table 3 pattern eth / ipv4 src is 1.1.1.1 / end
> > >
> > >     The pattern template 3 can be used only to match flows coming from
> vports.
> >
> > In this case it is interesting how it will behave on:
> > a NIC which does not support VPORT_ORIG and just ignores it
> 
> If the hint is ignored, all packets will hit the rule.
> 
+1
> > VS
> > a NIC which support VPORT_ORIG and takes it into account.
> 
> If the hint is taken into account,
> it is possible that wire packets do not hit this rule,
> but we don't really know, it is an internal driver optimization.
> 
> 
> > >> (2) is simply done on different level - much earlier, before
> > >> flow rules creation. Since resources allocation is assumed to
> > >> be done on table creation, we need to know the purpose of the
> > >> table in advance to optimize resources allocation.
> > >
> > > Actually in both cases we get the hint at template table creation.
> > > But in solution 2 we are not creating a redundant pattern matching,
> > > and we don't need to check it in flow rules, so it is more efficient.
> > >
> > >> Since (2) is *not a matching criteria*, but just a hint, (1)
> > >> flow rules must have matching criteria anyway.
> > >
> > > No we don't need the matching criteria ANY_VPORT with solution (2)
> > > because we are already matching on an IP src which is a vport.
> > >
> > >>> +Table Attribute: Specialize
> > >>> +^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > >>> +
> > >>> +Application can help optimizing underlayer resources and insertion
> rate
> > >>> +by specializing template table.
> > >>> +Specialization is done by providing hints
> > >>> +in the template table attribute ``specialize``.
> > >>> +
> > >>> +This attribute is not mandatory for each PMD to implement.
> > >>> +If a hint is not supported, it will be silently ignored,
> > >>> +and no special optimization is done.
> > >>> +
> > >>> +If a table is specialized, the application should make sure the rules
> > >>> +comply with the table attribute.
> > >>
> > >> If a table is specialized, the application must make sure that
> > >> all flow rules added to the table have pattern which implies
> > >> corresponding matching criteria. For example if a table is
> > >> specialized to be wire-origin only, pattern should have
> > >> represented port item with ethdev which corresponds to a
> > >> physical port (or any other item which matches packets
> > >> coming from wire only).
> > >
> > > No need of a matching criteria strictly mapping the hint.
> > > Here the hint is SPECIALIZE_TRANSFER_VPORT_ORIG
> > > and the rules can match on an IP src which is assigned to a vport.
> > > So there is no need to strictly match the vport itself in the rule.
> >
> > If so, the problem is that the same rules will behave in a different way
> > on different NICs.
> 
> Not exactly.
> If the assumption made by the application is wrong,
> yes there may be some differences for the unexpected packets.
> But it would be a user mistake somewhere.
> 
> In general, an application should have the same functional result,
> no matter the hint is used or not by the driver.
> 
> > > Hope it make thinks clear.
> > > We can improve the commit log as I wrote above.
> 
>