Re: [dpdk-dev] [PATCH v6 0/4] additions to support tunnel encap/decap

[Michael Wildt <michael.wildt@broadcom.com>]

Hi Declan,

Thank you (and DPDK) for driving this proposal, much appreciated. Upon
quick review of the proposal one thing stood out.

In rte_flow.h the new tunnel action,
RLTE_FLOW_ACTION_VXLAN_ENCAP/DECAP, similar with NVGRE_ENCAP/DECAP, is
defined as:

/**
* Decapsulate outer most VXLAN tunnel from matched flow.
*
* If flow pattern does not define a valid VXLAN tunnel (as specified by
* RFC7348) then the PMD should return a RTE_FLOW_ERROR_TYPE_ACTION
* error.
*/
RTE_FLOW_ACTION_TYPE_VXLAN_DECAP,

and

 * RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP
 *
 * VXLAN tunnel end-point encapsulation data definition
 *
 * The tunnel definition is provided through the flow item pattern, the
 * provided pattern must conform to RFC7348 for the tunnel specified. The flow
 * definition must be provided in order from the RTE_FLOW_ITEM_TYPE_ETH
 * definition up the end item which is specified by RTE_FLOW_ITEM_TYPE_END.
 *
 * The mask field allows user to specify which fields in the flow item
 * definitions can be ignored and which have valid data and can be used
 * verbatim.
 *
 * Note: the last field is not used in the definition of a tunnel and can be
 * ignored.
 *
 * Valid flow definition for RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP include:
 *
 * - ETH / IPV4 / UDP / VXLAN / END
 * - ETH / IPV6 / UDP / VXLAN / END
 * - ETH / VLAN / IPV4 / UDP / VXLAN / END
 *
 */

Similar with doc/guides/prog_guide/rte_flow.rst.

With these specific RF7348 references it wouldn't be possible to
support customer private implementations for VXLAN, i.e. a non
compliant inner packet like eth/ipv4/udp/vxlan/no inner eth
header/ipv4. The proposed format though, by specifying the protocol
layer using the 'item' list does allow for listing of the complete
stack including the inner packet format. This could be used to trigger
the PMD to handle this flow differently than the RFC compliant VXLAN.

If the RFC reference is left in then one would have to create yet
another VXLAN action, i.e.
RTE_FLOW_ACTION_TYPE_VXLAN_CUSTOM_ENCAP/DECAP OR
RTE_FLOW_ACTION_TYPE_NONCOMPLIANT_VXLAN_ENCAP/DECAP, which I find
inappropriate even if its name describe the inner packet was missing
the ethernet header.

Would it be possible to accept that the RFC7348 compliance is removed
from these two comments as to allow for more flexibility so we do not,
potentially, drown in custom cases ?

Thanks,
Michael

On Thu, Apr 26, 2018 at 8:08 AM, Declan Doherty
<declan.doherty@intel.com> wrote:
> his patchset contains the revised proposal to manage
> tunnel endpoints hardware accleration based on community
> feedback on RFC
> (http://dpdk.org/ml/archives/dev/2017-December/084676.html). This
> proposal is purely enabled through rte_flow APIs with the
> additions of some new features which were previously implemented
> by the proposed rte_tep APIs which were proposed in the original
> RFC. This patchset ultimately aims to enable the configuration
> of inline data path encapsulation and decapsulation of tunnel
> endpoint network overlays on accelerated IO devices.
>
> V2:
> - Split new functions into separate patches, and add additional
>   documentaiton.
>
> V3:
>
> - Extended the description of group counter in documentation.
> - Renamed VTEP to TUNNEL.
> - Fixed C99 syntax.
>
> V4:
>
> - Modify encap/decap actions to be protocol specific
> - rename group action type to jump
> - add mark flow item type in place of metadata flow/action types
> - add count action data structure
> - modify query API to accept rte_flow_action structure in place of
>   rte_flow_actio_type enumeration to support specification and
>   querying of multiple actions of the same type
>
> V5:
> - Documentation and comment updates
> - Mark new API structures as experimental
> - squash new function testpmd enablement into relevant patches.
>
> V6:
> - rebased to head of next-net
> - fixed whitespace issues add in previous revision
>
> The summary of the additions to the rte_flow are as follows:
>
> - Add new flow actions RTE_RTE_FLOW_ACTION_TYPE_[VXLAN/NVGRE]_ENCAP and
> RTE_FLOW_ACTION_TYPE_[VXLAN/NVGRE]_DECAP to rte_flow to support
> specfication of encapsulation and decapsulation of VXLAN and NVGRE
> tunnels in hardware.
> - Introduces support for the use of pipeline metadata in
> the flow pattern definition and the population of metadata fields
> from flow actions using the MARK flow and action items.
> - Add shared flag to counters to enable statistics to be kept on
> groups offlows such as all ingress/egress flows of a tunnel
> - Adds jump_action to allow a flows to be redirected to a group
> within the device.
>
> A high level summary of the proposed usage model is as follows:
>
> 1. Decapsulation
>
> 1.1. Decapsulation of tunnel outer headers and forward all traffic
>      to the same queue/s or port, would have the follow flows
>      paramteters, sudo code used here.
>
> struct rte_flow_attr attr = { .ingress = 1 };
>
> struct rte_flow_item pattern[] = {
>         { .type = RTE_FLOW_ITEM_TYPE_ETH,  .spec = &eth_item },
>         { .type = RTE_FLOW_ITEM_TYPE_IPV4, .spec = &ipv4_item },
>         { .type = RTE_FLOW_ITEM_TYPE_UDP, .spec = &udp_item },
>         { .type = RTE_FLOW_ITEM_TYPE_VxLAN, .spec = &vxlan_item },
>         { .type = RTE_FLOW_ITEM_TYPE_END }
> };
>
>
>
> struct rte_flow_action actions[] = {
>         { .type = RTE_FLOW_ACTION_TYPE_VXLAN_DECAP },
>         { .type = RTE_FLOW_ACTION_TYPE_VF, .conf = &vf_action  },
>         { .type = RTE_FLOW_ACTION_TYPE_END }
> }
>
> 1.2.
>
> Decapsulation of tunnel outer headers and matching on inner
> headers, and forwarding to the same queue/s or port.
>
> 1.2.1.
>
> The same scenario as above but either the application
> or hardware requires configuration as 2 logically independent
> operations (viewing it as 2 logical tables). The first stage
> being the flow rule to define the pattern to match the tunnel
> and the action to decapsulate the packet, and the second stage
> stage table matches the inner header and defines the actions,
> forward to port etc.
>
> flow rule for outer header on table 0
>
> struct rte_flow_attr attr = { .ingress = 1, .table = 0 };
>
> struct rte_flow_item pattern[] = {
>         { .type = RTE_FLOW_ITEM_TYPE_ETH,  .spec = &eth_item },
>         { .type = RTE_FLOW_ITEM_TYPE_IPV4, .spec = &ipv4_item },
>         { .type = RTE_FLOW_ITEM_TYPE_UDP, .spec = &udp_item },
>         { .type = RTE_FLOW_ITEM_TYPE_VxLAN, .spec = &vxlan_item },
>         { .type = RTE_FLOW_ITEM_TYPE_END }
> };
>
> struct rte_flow_item_count shared_couter = {
>         .shared = 1,
>         .id = {counter_id}
> }
>
> struct rte_flow_action actions[] = {
>         { .type = RTE_FLOW_ACTION_TYPE_COUNT, .conf = &shared_counter },
>         { .type = RTE_FLOW_ACTION_TYPE_MARK, .conf = &mark_action },
>         { .type = RTE_FLOW_ACTION_TYPE_VXLAN_DECAP },
>         {
>           .type = RTE_FLOW_ACTION_TYPE_JUMP,
>           .conf = { .group = 1 }
>         },
>         { .type = RTE_FLOW_ACTION_TYPE_END }
> }
>
> flow rule for inner header on table 1
>
> struct rte_flow_attr attr = { .ingress = 1, .group = 1 };
>
> struct rte_flow_item_mark mark_item = { id = {mark_id} };
>
> struct rte_flow_item pattern[] = {
>         { .type = RTE_FLOW_ITEM_TYPE_MARK,  .spec = &mark_item },
>         { .type = RTE_FLOW_ITEM_TYPE_ETH,  .spec = &eth_item },
>         { .type = RTE_FLOW_ITEM_TYPE_IPV4, .spec = &ipv4_item },
>         { .type = RTE_FLOW_ITEM_TYPE_TCP, .spec = &tcp_item },
>         { .type = RTE_FLOW_ITEM_TYPE_END }
> };
>
> struct rte_flow_action actions[] = {
>         {
>           .type = RTE_FLOW_ACTION_TYPE_PORT_ID,
>           .conf = &port_id_action = { port_id }
>         },
>         { .type = RTE_FLOW_ACTION_TYPE_END }
> }
>
> Note that the mark action in the flow rule in group 0 is generating
> the value in the pipeline which is then used in as part as the flow
> pattern in group 1 to specify the exact flow to match against. In the
> case where exact match rules are being provided by the application
> explicitly then the MARK item value can be provided by the application
> in the flow pattern for the flow rule in group 1 also.
>
> 2. Encapsulation
>
> Encapsulation of all traffic matching a specific flow pattern to a
> specified tunnel and egressing to a particular port.
>
> struct rte_flow_attr attr = { .egress = 1 };
>
> struct rte_flow_item pattern[] = {
>         { .type = RTE_FLOW_ITEM_TYPE_ETH, .spec = &eth_item },
>         { .type = RTE_FLOW_ITEM_TYPE_IPV4, .spec = &ipv4_item },
>         { .type = RTE_FLOW_ITEM_TYPE_TCP, .spec = &tcp_item },
>         { .type = RTE_FLOW_ITEM_TYPE_END }
> };
>
> struct rte_flow_action_tunnel_encap vxlan_encap_action = {
>         .definition = {
>                 { .type=eth, .spec={}, .mask={} },
>                 { .type=ipv4, .spec={}, .mask={} },
>                 { .type=udp, .spec={}, .mask={} },
>                 { .type=vxlan, .spec={}, .mask={} }
>                 { .type=end }
>         }
> };
>
> struct rte_flow_action actions[] = {
>         { .type = RTE_FLOW_ACTION_TYPE_COUNT, .conf = &count } },
>         { .type = RTE_FLOW_ACTION_TYPE_VXLAN_ENCAP, .conf = &vxlan_encap_action } },
>         {
>           .type = RTE_FLOW_ACTION_TYPE_PORT_ID,
>           .conf = &port_id_action = { port_id }
>         },
>         { .type = RTE_FLOW_ACTION_TYPE_END }
> };
>
> Declan Doherty (4):
>   ethdev: Add tunnel encap/decap actions
>   ethdev: Add group JUMP action
>   ethdev: add mark flow item to rte_flow_item_types
>   ethdev: add shared counter support to rte_flow
>
>  app/test-pmd/cmdline_flow.c                 |  51 +++++-
>  app/test-pmd/config.c                       |  15 +-
>  app/test-pmd/testpmd.h                      |   2 +-
>  doc/guides/prog_guide/rte_flow.rst          | 257 ++++++++++++++++++++++++----
>  doc/guides/testpmd_app_ug/testpmd_funcs.rst |   8 +
>  drivers/net/bonding/rte_eth_bond_flow.c     |   9 +-
>  drivers/net/failsafe/failsafe_flow.c        |   4 +-
>  lib/librte_ether/rte_flow.c                 |   2 +-
>  lib/librte_ether/rte_flow.h                 | 211 +++++++++++++++++++++--
>  lib/librte_ether/rte_flow_driver.h          |   2 +-
>  10 files changed, 500 insertions(+), 61 deletions(-)
>
> --
> 2.14.3
>