Re: [dpdk-dev] [PATCH v4 1/3] ethdev: add raw encapsulation action

[Andrew Rybchenko <arybchenko@solarflare.com>]

On 10/22/18 4:19 PM, Ori Kam wrote:
>> -----Original Message-----
>> From: Andrew Rybchenko <arybchenko@solarflare.com>
>> Sent: Monday, October 22, 2018 4:06 PM
>> To: Ori Kam <orika@mellanox.com>; wenzhuo.lu@intel.com;
>> jingjing.wu@intel.com; bernard.iremonger@intel.com; ferruh.yigit@intel.com;
>> stephen@networkplumber.org; Adrien Mazarguil
>> <adrien.mazarguil@6wind.com>
>> Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>; Thomas Monjalon
>> <thomas@monjalon.net>; Nélio Laranjeiro <nelio.laranjeiro@6wind.com>;
>> Yongseok Koh <yskoh@mellanox.com>; Shahaf Shuler
>> <shahafs@mellanox.com>
>> Subject: Re: [dpdk-dev] [PATCH v4 1/3] ethdev: add raw encapsulation action
>>
>> On 10/17/18 11:43 AM, Ori Kam wrote:
>>>> -----Original Message-----
>>>> From: Andrew Rybchenko <arybchenko@solarflare.com>
>>>> Sent: Wednesday, October 17, 2018 10:56 AM
>>>> To: Ori Kam <orika@mellanox.com>; wenzhuo.lu@intel.com;
>>>> jingjing.wu@intel.com; bernard.iremonger@intel.com;
>> ferruh.yigit@intel.com;
>>>> stephen@networkplumber.org; Adrien Mazarguil
>>>> <adrien.mazarguil@6wind.com>
>>>> Cc: dev@dpdk.org; Dekel Peled <dekelp@mellanox.com>; Thomas Monjalon
>>>> <thomas@monjalon.net>; Nélio Laranjeiro <nelio.laranjeiro@6wind.com>;
>>>> Yongseok Koh <yskoh@mellanox.com>; Shahaf Shuler
>>>> <shahafs@mellanox.com>
>>>> Subject: Re: [dpdk-dev] [PATCH v4 1/3] ethdev: add raw encapsulation action
>>>>
>>>> On 10/17/18 12:41 AM, Ori Kam wrote:
>>>> Currenlty the encap/decap actions only support encapsulation
>>>> of VXLAN and NVGRE L2 packets (L2 encapsulation is where
>>>> the inner packet has a valid Ethernet header, while L3 encapsulation
>>>> is where the inner packet doesn't have the Ethernet header).
>>>> In addtion the parameter to to the encap action is a list of rte items,
>>>> this results in 2 extra translation, between the application to the
>>>> actioni and from the action to the NIC. This results in negetive impact
>>>> on the insertion performance.
>>>>
>>>> Looking forward there are going to be a need to support many more tunnel
>>>> encapsulations. For example MPLSoGRE, MPLSoUDP.
>>>> Adding the new encapsulation will result in duplication of code.
>>>> For example the code for handling NVGRE and VXLAN are exactly the same,
>>>> and each new tunnel will have the same exact structure.
>>>>
>>>> This patch introduce a raw encapsulation that can support L2 tunnel types
>>>> and L3 tunnel types. In addtion the new
>>>> encapsulations commands are using raw buffer inorder to save the
>>>> converstion time, both for the application and the PMD.
>>>>
>>>> In order to encapsulate L3 tunnel type there is a need to use both
>>>> actions in the same rule: The decap to remove the L2 of the original
>>>> packet, and then encap command to encapsulate the packet with the
>>>> tunnel.
>>>> For decap L3 there is also a need to use both commands in the same flow
>>>> first the decap command to remove the outer tunnel header and then encap
>>>> to add the L2 header.
>>>>
>>>> Signed-off-by: Ori Kam mailto:orika@mellanox.com
>> [...]
>>
>>>> +
>>>> +This action modifies the payload of matched flows. The data supplied must
>>>> +be a valid header, either holding layer 2 data in case of removing layer 2
>>>> +before incapsulation of layer 3 tunnel (for example MPLSoGRE) or
>> complete
>>>> +tunnel definition starting from layer 2 and moving to the tunel item itself.
>>>> +When applied to the original packet the resulting packet must be a
>>>> +valid packet.
>>>> +
>>>> +.. _table_rte_flow_action_raw_decap:
>>>> +
>>>> +.. table:: RAW_DECAP
>>>> +
>>>> +   +----------------+----------------------------------------+
>>>> +   | Field          | Value                                  |
>>>> +   +================+========================================+
>>>> +   | ``data``       | Decapsulation data                     |
>>>>
>>>> Sorry, I've missed the point why it is here. Is it used for matching?
>>>> Why is the size insufficient?
>>>>
>>> No it is not used for matching this is only for the encapsulation data.
>>> The data is for PMD that needs to validate that they can decapsulate
>>> The packet, and on some PMD there might need the specify which headers
>>> to remove and not just number of bytes.
>> Sorry, but I still don't understand. How should PMD or HW use it?
>> I guess the main problem here is that it is a generic action.
>> If it is VXLAN_DECAP, it would not be a problem and neither
>> size nor data would be required.
>>
> The data is buffer of the encap/decap headers, so the PMD can parse the this data
> and check the validity and if the HW supports it.
> Some NICs will not use this others can check if the tunnel request is valid.
> For example let's assume that some tunnel encapsulation is supported on some FW version
> and not supported in other version so the PMD can check the encapsulation data to see
> what is the requested tunnel type and the FW capabilities to return success or fail.

OK, I see. Could you improve the action description to make it clear.
Right now the description says nothing about it.