Re: [dpdk-dev] [PATCH v2 1/4] ethdev: allow negative values in flow rule types

[Andrew Rybchenko <arybchenko@solarflare.com>]

Hi Ori,

On 9/17/20 10:47 AM, Ori Kam wrote:
> Hi Andrew,
> 
>> -----Original Message-----
>> From: Andrew Rybchenko <arybchenko@solarflare.com>
>> Sent: Thursday, September 17, 2020 9:50 AM
>>
>> On 9/16/20 8:21 PM, Gregory Etelson wrote:
>>> From: Gregory Etelson
>>> Sent: Tuesday, September 15, 2020 13:27
>>> To: Andrew Rybchenko <arybchenko@solarflare.com>; Ajit Khaparde
>> <ajit.khaparde@broadcom.com>
>>> Cc: dpdk-dev <dev@dpdk.org>; Matan Azrad <matan@nvidia.com>; Raslan
>> Darawsheh <rasland@nvidia.com>; Ori Kam <orika@nvidia.com>; Gregory
>> Etelson <getelson@mellanox.com>; Ori Kam <orika@mellanox.com>; NBU-
>> Contact-Thomas Monjalon <thomas@monjalon.net>; Ferruh Yigit
>> <ferruh.yigit@intel.com>
>>> Subject: RE: [dpdk-dev] [PATCH v2 1/4] ethdev: allow negative values in flow
>> rule types
>>>
>>> Subject: Re: [dpdk-dev] [PATCH v2 1/4] ethdev: allow negative values in flow
>> rule types
>>> On 9/15/20 7:36 AM, Ajit Khaparde wrote:
>>> On Tue, Sep 8, 2020 at 1:16 PM Gregory Etelson
>> <mailto:getelson@nvidia.com> wrote:
>>> From: Gregory Etelson <mailto:getelson@mellanox.com>
>>>
>>> RTE flow items & actions use positive values in item & action type.
>>> Negative values are reserved for PMD private types. PMD
>>> items & actions usually are not exposed to application and are not
>>> used to create RTE flows.
>>>
>>> The patch allows applications with access to PMD flow
>>> items & actions ability to integrate RTE and PMD items & actions
>>> and use them to create flow rule.
>>> While we are reviewing this, some quick comment/questions..
>>>
>>> Doesn't this go against the above "PMD
>>> items & actions usually are not exposed to application and are not
>>> used to create RTE flows."?
>>> Why would an application try to use PMD specific private types?
>>> Isn't this contrary to having a standard API?
>>>
>>> +1
>>>
>>> I would like to clarify the purpose and use of private elements patch.
>>> That patch is prerequisite for  [PATCH v2 2/4] ethdev: tunnel offload model
>> patch.
>>> The tunnel offload API provides unified hardware independent model to
>> offload tunneled packets,
>>> match on packet headers in hardware and to restore outer headers of
>> partially offloaded packets.
>>> The model implementation depends on hardware capabilities. For example,
>> if hardware supports inner nat,
>>> it can do nat first and postpone decap to the end, while other hardware that
>> cannot do inner nat must decap first
>>> and run nat actions afterwards. Such hardware has to save outer header in
>> some hardware context,
>>> register or memory, for application to restore a packet later, if needed. Also,
>> in this case the exact solution
>>> depends on PMD because of limited number of hardware contexts.
>>> Although application working with DKDK can implement all these
>> requirements with existing flow rules API,
>>> it will have to address each hardware specifications separately.
>>> To solve this limitation we selected design where application quires PMD for
>> actions, or items,
>>> that are optimal for a hardware that PMD represents. Result can be a
>> mixture of RTE and PMD private elements -
>>> it's up to PMD implementation. Application passes these elements back to
>> PMD as a flow rule recipe
>>> that's already optimal for underlying hardware.
>>> If PMD has private elements in such rule items or actions, these private
>> elements must not be rejected by RTE layer.
>>>
>>> I hope it helps to understand what this model is trying to achieve.
>>> Did that clarify your concerns ?
>>
>> There is a very simple question which I can't answer after
>> reading it.
>> Why these PMD specific actions and items do not bind
>> application to a specific vendor. If it binds, it should
>> be clearly stated in the description. If no, I'd like to
>> understand why since opaque actions/items are not really
>> well defined and hardly portable across vendors.
> 
> You are correct, when looking at this patch as a stand a lone
> patch using such action / items does bind the application to specific PMD.
> first sometimes it is required, for example one vendor may introduce private action
> to support some key costumer, or enable feature that is not supported using standard rte flow API.

Good. So I understand it correctly.

> The main reason for this patch is the tunnel API[1] as stated in the reply
> from Gregory, the tunnel API exposes a public function that returns a list of
> actions / items. The list is generated by the PMD, so using the API is not binding
> since it is generic, but the action / items returned are private but the application is not aware of those actions / items, from it's point of view it called a generic function
> and got actions that are configured to do the requested job. All the application needs to do is send the actions / item as actions / item when calling flow create.
> 
> Does this answer your question?

Yes, many thanks. I'm still trying to put the feature design in my head
and your explanations helped a lot. May be it is just the same words as
before, but in a bit different order and highlights.

> [1] https://patches.dpdk.org/patch/76931/
>