Re: [PATCH v4] ethdev: add special flags when creating async transfer table

[Thomas Monjalon <thomas@monjalon.net>]

09/11/2022 10:36, Andrew Rybchenko:
> On 11/9/22 12:03, Thomas Monjalon wrote:
> > 09/11/2022 09:53, Andrew Rybchenko:
> >> On 11/8/22 18:25, Thomas Monjalon wrote:
> >>> 08/11/2022 15:38, Andrew Rybchenko:
> >>>> On 11/8/22 16:29, Thomas Monjalon wrote:
> >>>>> 08/11/2022 12:47, Andrew Rybchenko:
> >>>>>> On 11/8/22 14:39, Andrew Rybchenko wrote:
> >>>>>>> On 11/4/22 13:44, Rongwei Liu wrote:
> >>>>>>>> diff --git a/lib/ethdev/rte_flow.h b/lib/ethdev/rte_flow.h
> >>>>>>>> index 8858b56428..1eab12796f 100644
> >>>>>>>> --- a/lib/ethdev/rte_flow.h
> >>>>>>>> +++ b/lib/ethdev/rte_flow.h
> >>>>>>>> @@ -5186,6 +5186,34 @@ rte_flow_actions_template_destroy(uint16_t
> >>>>>>>> port_id,
> >>>>>>>>       */
> >>>>>>>>      struct rte_flow_template_table;
> >>>>>>>> +/**
> >>>>>>>> + * @warning
> >>>>>>>> + * @b EXPERIMENTAL: this API may change without prior notice.
> >>>>>>>> + *
> >>>>>>>> + * Special optional flags for template table attribute.
> >>>>>>>> + * Each bit stands for a table specialization
> >>>>>>>> + * offering a potential optimization at PMD layer.
> >>>>>>>> + * PMD can ignore the unsupported bits silently.
> >>>>>>>> + */
> >>>>>>>> +enum rte_flow_template_table_specialize {
> >>>>>>>> +    /**
> >>>>>>>> +     * Specialize table for transfer flows which come only from wire.
> >>>>>>>> +     * It allows PMD not to allocate resources for non-wire
> >>>>>>>> originated traffic.
> >>>>>>>> +     * This bit is not a matching criteria, just an optimization hint.
> >>>>>>>> +     * Flow rules which match non-wire originated traffic will be missed
> >>>>>>>> +     * if the hint is supported.
> >>>>>>
> >>>>>> Sorry, but if so, the hint changes behavior.
> >>>>>
> >>>>> Yes the hint may change behaviour.
> >>>>>
> >>>>>> Let's consider a rule which matches both VF originating and
> >>>>>> wire originating traffic. Will the rule be missed (ignored)
> >>>>>> regardless if the hint is supported or not?
> >>>>>
> >>>>> If the hint RTE_FLOW_TRANSFER_WIRE_ORIG is used,
> >>>>> the PMD may assume the table won't be used for traffic
> >>>>> which is not coming from wire ports.
> >>>>> As a consequence, the table may be implemented on the path
> >>>>> of wire traffic only.
> >>>>> In this case, the traffic coming from virtual ports
> >>>>> won't be affected by this table.
> >>>>> To answer the question, a rule matching both virtual and wire traffic
> >>>>> will be applied in a table affecting only wire traffic,
> >>>>> so it will still apply (not completely ignored).
> >>>>
> >>>> If so, it is not a hint. It becomes matching criteria
> >>>> which should be in pattern as we discussed.
> >>>
> >>> It is not a strict matching because the PMD is free to support it or not.
> >>
> >> It cannot be optional matching criteria. Matching criteria must
> >> be always mandatory. Otherwise application does not know what
> >> to expect and behaviour may legitimately vary on different
> >> vendors.
> > 
> > I think you take it in the wrong direction.
> > The idea is not to have it as a criteria.
> > Let me explain again:
> > 
> > If an application is using a flow table to manage flows
> > which *always* come from the same type of port (wire or virtual),
> 
> What does guarantee it? Is it used a jump-table and jump rule
> must guarantee it? Or has pattern corresponding unit?
> 
> It is very thin ice and I'm ready to bet money that finally
> it will be used as a matching criteria intentionally or not
> intentionally. Simply because it works as matching criteria
> on, for example, Mellanox. I.e. if rules from table with
> corresponding hint are programmed to HW which applies these
> rules on traffic from wire only - effectively it is a matching
> criteria. And it will be used this way. And it will be not
> portable to other HW which does not support the hint.
> So, we're making an API which is very easy to misuse if not
> to say more.

I completely understand your concern (I have same).
In other words, if the application misuse the hint,
it will become not portable.
That's why I made sure to highlight such misue consequence
in the API comments.

> You know better if it is OK or not to rely on liable users
> in the case of DPDK.

I do not rely on users, and I don't want to block innovation.
That's why I want to make sure all is explained and clear,
so freedom comes with responsibility.

> It would be much safer if we do not rely on application in this
> case, introduce a new pattern item to specify origin and
> require PMD to check that pattern has either a new pattern item
> or corresponding  REPRESENTED_PORT/PORT_REPRESENTOR pattern
> item.

Safer is not often compatible with fastest :)

> I realize that my concerns could be not valid and it is just
> a paranoia. Just add your ack and let's move forward.

Let's wait for other opinions.

> > then the application can give this information to the driver.
> > With this assumption coming from the application,
> > the driver may do some optimizations.
> > 
> > Now about what is explained above:
> > If the application gives such a hint
> > but does not respect its own assumption,
> > then confusion happens.