DPDK patches and discussions
 help / color / mirror / Atom feed
From: Gregory Etelson <getelson@nvidia.com>
To: Andrew Rybchenko <arybchenko@solarflare.com>,
	"dev@dpdk.org" <dev@dpdk.org>
Cc: Matan Azrad <matan@nvidia.com>,
	Raslan Darawsheh <rasland@nvidia.com>,
	"Eli Britstein" <elibr@nvidia.com>, Ori Kam <orika@nvidia.com>
Subject: Re: [dpdk-dev] [PATCH 2/2] ethdev: tunnel offload model
Date: Wed, 19 Aug 2020 14:30:01 +0000	[thread overview]
Message-ID: <BY5PR12MB366850E5D62161EDF686C14BA55D0@BY5PR12MB3668.namprd12.prod.outlook.com> (raw)
In-Reply-To: <b87b6425-94bb-dcce-a214-f8d852489086@solarflare.com>

Hello Andrew,

Thank you for detailed review.
Sorry for the late response.
Please see my answers inline.

Regards,
Gregory

> -----Original Message-----
> From: Andrew Rybchenko <arybchenko@solarflare.com>
> Sent: Sunday, July 5, 2020 17:51
> To: Gregory Etelson <getelson@mellanox.com>; dev@dpdk.org
> Cc: Matan Azrad <matan@mellanox.com>; Raslan Darawsheh 
> <rasland@mellanox.com>; Eli Britstein <elibr@mellanox.com>; Ori Kam 
> <orika@mellanox.com>
> Subject: Re: [dpdk-dev] [PATCH 2/2] ethdev: tunnel offload model
> 
> Hi Gregory,
> 
> I'm sorry for the review with toooo many questions without any 
> suggestions on how to answer it. Please, see below.
> 
> On 6/25/20 7:03 PM, Gregory Etelson wrote:
> > From: Eli Britstein <elibr@mellanox.com>
> >
> > Hardware vendors implement tunneled traffic offload techniques 
> > differently. Although RTE flow API provides tools capable to offload 
> > all sorts of network stacks, software application must reference 
> > this hardware differences in flow rules compilation. As the result 
> > tunneled traffic flow rules that utilize hardware capabilities can 
> > be different for the same traffic.
> >
> > Tunnel port offload proposed in [1] provides software application 
> > with unified rules model for tunneled traffic regardless underlying 
> > hardware.
> >  - The model introduces a concept of a virtual tunnel port (VTP).
> >  - The model uses VTP to offload ingress tunneled network traffic
> >    with RTE flow rules.
> >  - The model is implemented as set of helper functions. Each PMD
> >    implements VTP offload according to underlying hardware offload
> >    capabilities.  Applications must query PMD for VTP flow
> >    items / actions before using in creation of a VTP flow rule.
> >
> > The model components:
> > - Virtual Tunnel Port (VTP) is a stateless software object that
> >   describes tunneled network traffic.  VTP object usually contains
> >   descriptions of outer headers, tunnel headers and inner headers.
> > - Tunnel Steering flow Rule (TSR) detects tunneled packets and
> >   delegates them to tunnel processing infrastructure, implemented
> >   in PMD for optimal hardware utilization, for further processing.
> > - Tunnel Matching flow Rule (TMR) verifies packet configuration and
> >   runs offload actions in case of a match.
> >
> > Application actions:
> > 1 Initialize VTP object according to tunnel
> >   network parameters.
> > 2 Create TSR flow rule:
> > 2.1 Query PMD for VTP actions: application can query for VTP actions
> >     more than once
> >     int
> >     rte_flow_tunnel_decap_set(uint16_t port_id,
> >                               struct rte_flow_tunnel *tunnel,
> >                               struct rte_flow_action **pmd_actions,
> >                               uint32_t *num_of_pmd_actions,
> >                               struct rte_flow_error *error);
> >
> > 2.2 Integrate PMD actions into TSR actions list.
> > 2.3 Create TSR flow rule:
> >     flow create <port> group 0
> >           match {tunnel items} / end
> >           actions {PMD actions} / {App actions} / end
> >
> > 3 Create TMR flow rule:
> > 3.1 Query PMD for VTP items: application can query for VTP items
> >     more than once
> >     int
> >     rte_flow_tunnel_match(uint16_t port_id,
> >                           struct rte_flow_tunnel *tunnel,
> >                           struct rte_flow_item **pmd_items,
> >                           uint32_t *num_of_pmd_items,
> >                           struct rte_flow_error *error);
> >
> > 3.2 Integrate PMD items into TMR items list:
> > 3.3 Create TMR flow rule
> >     flow create <port> group 0
> >           match {PMD items} / {APP items} / end
> >           actions {offload actions} / end
> >
> > The model provides helper function call to restore packets that miss 
> > tunnel TMR rules to its original state:
> > int
> > rte_flow_get_restore_info(uint16_t port_id,
> >                           struct rte_mbuf *mbuf,
> >                           struct rte_flow_restore_info *info,
> >                           struct rte_flow_error *error);
> >
> > rte_tunnel object filled by the call inside rte_flow_restore_info 
> > *info parameter can be used by the application to create new TMR 
> > rule for that tunnel.
> >
> > The model requirements:
> > Software application must initialize rte_tunnel object with tunnel 
> > parameters before calling
> > rte_flow_tunnel_decap_set() & rte_flow_tunnel_match().
> >
> > PMD actions array obtained in rte_flow_tunnel_decap_set() must be 
> > released by application with rte_flow_action_release() call.
> > Application can release the actionsfter TSR rule was created.
> >
> > PMD items array obtained with rte_flow_tunnel_match() must be 
> > released by application with rte_flow_item_release() call.
> > Application can release the items after rule was created. However, 
> > if the application needs to create additional TMR rule for the same 
> > tunnel it will need to obtain PMD items again.
> >
> > Application cannot destroy rte_tunnel object before it releases all 
> > PMD actions & PMD items referencing that tunnel.
> >
> > [1]
> > https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fma
> > il
> > s.dpdk.org%2Farchives%2Fdev%2F2020-
> June%2F169656.html&amp;data=02%7C01
> >
> %7Cgetelson%40mellanox.com%7Cb1dd5d535357492c727508d820f2d97b%7
> Ca65297
> >
> 1c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C637295574702858871&amp;sd
> ata=Ej%2
> >
> FtMUt2A%2FV3y70iRTXq5NmCRbW8nxCx4PLZQQFwb3o%3D&amp;reserved=
> 0
> >
> > Signed-off-by: Eli Britstein <elibr@mellanox.com>
> > Acked-by: Ori Kam <orika@mellanox.com>
> > ---
> >  doc/guides/prog_guide/rte_flow.rst       | 105 ++++++++++++
> >  lib/librte_ethdev/rte_ethdev_version.map |   5 +
> >  lib/librte_ethdev/rte_flow.c             | 112 +++++++++++++
> >  lib/librte_ethdev/rte_flow.h             | 196 +++++++++++++++++++++++
> >  lib/librte_ethdev/rte_flow_driver.h      |  32 ++++
> >  5 files changed, 450 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/rte_flow.rst
> > b/doc/guides/prog_guide/rte_flow.rst
> > index d5dd18ce99..cfd98c2e7d 100644
> > --- a/doc/guides/prog_guide/rte_flow.rst
> > +++ b/doc/guides/prog_guide/rte_flow.rst
> > @@ -3010,6 +3010,111 @@ operations include:
> >  - Duplication of a complete flow rule description.
> >  - Pattern item or action name retrieval.
> >
> > +Tunneled traffic offload
> > +~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +Provide software application with unified rules model for tunneled 
> > +traffic regardless underlying hardware.
> > +
> > + - The model introduces a concept of a virtual tunnel port (VTP).
> 
> It looks like it is absolutely abstract concept now, since it is not 
> mentioned/referenced in the header file. It makes it hard to put the 
> description and API together.
> 

Virtual tunnel is a concept that allows PMD to restore packet outer header after decap.
Example below shows how this feature is used.

> > + - The model uses VTP to offload ingress tunneled network traffic
> > +   with RTE flow rules.
> > + - The model is implemented as set of helper functions. Each PMD
> > +   implements VTP offload according to underlying hardware offload
> > +   capabilities.  Applications must query PMD for VTP flow
> > +   items / actions before using in creation of a VTP flow rule.
> 
> For me it looks like "creation of a VTP flow rule" is not covered yet. 
> Flow rules examples mention it in pattern and actions, but there is no 
> corresponding pattern items and actions. May be I simply misunderstand the idea.
>

This feature does not introduce new RTE items or actions. 
The idea is that PMD will use private items and actions to implement internal logic 
that will allow packet restore after decap. There's a detailed example below.
Internal PMD elements referred as pmd_item / pmd_action.

> > +
> > +The model components:
> > +
> > +- Virtual Tunnel Port (VTP) is a stateless software object that
> > +  describes tunneled network traffic.  VTP object usually contains
> > +  descriptions of outer headers, tunnel headers and inner headers.
> 
> Are inner headers really a part of the tunnel description?
>

Inner headers are not part of tunnel description.

> > +- Tunnel Steering flow Rule (TSR) detects tunneled packets and
> > +  delegates them to tunnel processing infrastructure, implemented
> > +  in PMD for optimal hardware utilization, for further processing.
> > +- Tunnel Matching flow Rule (TMR) verifies packet configuration and
> > +  runs offload actions in case of a match.
> 
> Is it for fully offloaded tunnels with encap/decap or all tunnels 
> (detected, but partially offloaded, e.g. checksumming)?
> 

Tunnel API is designed for tunnel packets that will go through decap action.
Current tunnel API version focuses on VXLAN tunnel.

> > +
> > +Application actions:
> > +
> > +1 Initialize VTP object according to tunnel network parameters.
> 
> I.e. fill in 'struct rte_flow_tunnel'. Is it correct?
>

Correct. Application initializes rte_flow_tunnel structure

> > +
> > +2 Create TSR flow rule.
> > +
> > +2.1 Query PMD for VTP actions. Application can query for VTP 
> > +actions
> more than once.
> > +
> > +  .. code-block:: c
> > +
> > +    int
> > +    rte_flow_tunnel_decap_set(uint16_t port_id,
> > +                              struct rte_flow_tunnel *tunnel,
> > +                              struct rte_flow_action **pmd_actions,
> > +                              uint32_t *num_of_pmd_actions,
> > +                              struct rte_flow_error *error);
> > +
> > +2.2 Integrate PMD actions into TSR actions list.
> > +
> > +2.3 Create TSR flow rule.
> > +
> > +    .. code-block:: console
> > +
> > +      flow create <port> group 0 match {tunnel items} / end actions 
> > + {PMD actions} / {App actions} / end
> 
> Are application actions strictly required?
> If no, it is better to make it clear.
> Do tunnel items correlate here somehow with tunnel specification in 
> 'struct rte_flow_tunnel'?
> Is it obtained using rte_flow_tunnel_match()?
>

Application actions are not required in steering rule.
{tunnel items} refer to rte flow items that identify tunnel. 
These items correlate to tunnel description in rte_flow_tunnel structure.

> > +
> > +3 Create TMR flow rule.
> > +
> > +3.1 Query PMD for VTP items. Application can query for VTP items 
> > +more
> than once.
> > +
> > +    .. code-block:: c
> > +
> > +      int
> > +      rte_flow_tunnel_match(uint16_t port_id,
> > +                            struct rte_flow_tunnel *tunnel,
> > +                            struct rte_flow_item **pmd_items,
> > +                            uint32_t *num_of_pmd_items,
> > +                            struct rte_flow_error *error);
> > +
> > +3.2 Integrate PMD items into TMR items list.
> > +
> > +3.3 Create TMR flow rule.
> > +
> > +    .. code-block:: console
> > +
> > +      flow create <port> group 0 match {PMD items} / {APP items} / 
> > + end actions {offload actions} / end
> > +
> > +The model provides helper function call to restore packets that 
> > +miss tunnel TMR rules to its original state:
> > +
> > +.. code-block:: c
> > +
> > +  int
> > +  rte_flow_get_restore_info(uint16_t port_id,
> > +                            struct rte_mbuf *mbuf,
> > +                            struct rte_flow_restore_info *info,
> > +                            struct rte_flow_error *error);
> > +
> > +rte_tunnel object filled by the call inside ``rte_flow_restore_info 
> > +*info parameter`` can be used by the application to create new TMR 
> > +rule for that tunnel.
> 
> I think an example, for example, for VXLAN over IPv4 tunnel case with 
> some concrete parameters would be very useful here for understanding.
> Could it be annotated with a description of the transformations 
> happening with a packet on different stages of the processing (including restore example).
>

VXLAN Code example:
Assume application needs to do inner NAT on VXLAN packet.
The first  rule in group 0:

flow create <port id> ingress group 0 pattern eth / ipv4 / udp dst is 4789 / vxlan / end
         actions {pmd actions} / jump group 3 / end

First VXLAN packet that arrives matches the rule in group 0 and jumps to group 3
In group 3 the packet will miss since there is no flow to match and will be uploaded
to application.
Application  will call rte_flow_get_restore_info() to get the packet outer header.
Application will insert a new rule in group 3 to match outer and inner headers:

flow create <port id> ingress group 3 pattern {pmd items} /  eth / ipv4 dst is 172.10.10.1 / udp dst 4789 / vxlan vni is 10 /
        ipv4 dst is 184.1.2.3 / end
         actions  set_ipv4_dst  186.1.1.1 / queue index 3 / end

Resulting of rules will be that VXLAN packet with vni=10, outer IPv4 dst =172.10.10.1 and inner IPv4 dst=184.1.2.3
will be received decaped on queue 3 with IPv4 dst=186.1.1.1

Note: Packet in group 3 is considered decaped. All actions in that group will be done on
header that was inner before decap. Application may specify outer header to be matched on. 
It's PMD responsibility to translate these items to outer metadata. 
 
API usage:
/**
 * 1. Initiate RTE flow tunnel object
 */
struct rte_flow_tunnel tunnel = {
  .type = RTE_FLOW_ITEM_TYPE_VXLAN,
  .tun_info = {
    .tun_id = rte_cpu_to_be_64(10),
  }
}

/**
 * 2. Obtain PMD tunnel actions
 *
 * pmd_actions is an intermediate variable application uses to
 * compile actions array
 */
struct rte_flow_action **pmd_actions;
rte_flow_tunnel_decap_and_set(&tunnel, &pmd_actions,
                              &num_pmd_actions, &error);

/**
 * 3. offload the first  rule
 * matching on VXLAN traffic and jumps to group 3
 * (implicitly decaps packet)
 */ 							  
app_actions  =   jump group 3
rule_items = app_items;  /* eth / ipv4 / udp / vxlan  */
rule_actions = { pmd_actions, app_actions };
attr.group = 0;
flow_1 = rte_flow_create(port_id, &attr,
                                               rule_items, rule_actions, &error);
/**
  * 4. after flow creation application does not need to keep tunnel
  * action resources.
  */
rte_flow_tunnel_action_release(port_id, pmd_actions,
                                                            num_pmd_actions);
							   
/**
  * 5. After partially offloaded packet miss because there was no
  * matching rule handle miss on group 3
  */
struct rte_flow_restore_info info;
rte_flow_get_restore_info(port_id, mbuf, &info, &error);

/**
 * 6. Offload NAT rule:
 */
app_items = eth / ipv4 dst is 172.10.10.1 / udp dst 4789 / vxlan vni is 10 / ipv4 dst is 184.1.2.3
app_actions = set_ipv4_dst  186.1.1.1 / queue index 3

rte_flow_tunnel_match(&info.tunnel, &pmd_items,
                                            &num_pmd_items,  &error); 
rule_items = {pmd_items, app_items}; 
rule_actions = app_actions; 
attr.group = info.group_id;
flow_2 = rte_flow_create(port_id, &attr,
                                               rule_items, rule_actions, &error);							
/**
 * 7. Release PMD items after rule creation  
 */
rte_flow_tunnel_item_release(port_id, pmd_items, num_pmd_items);

> > +
> > +The model requirements:
> > +
> > +Software application must initialize rte_tunnel object with tunnel 
> > +parameters before calling
> > +rte_flow_tunnel_decap_set() & rte_flow_tunnel_match().
> > +
> > +PMD actions array obtained in rte_flow_tunnel_decap_set() must be 
> > +released by application with rte_flow_action_release() call.
> > +Application can release the actionsfter TSR rule was created.
> 
> actionsfter ?
> 

Typo. Original text was
"Application can release the actions after TSR rule was created."

> > +
> > +PMD items array obtained with rte_flow_tunnel_match() must be 
> > +released by application with rte_flow_item_release() call.
> > +Application can release the items after rule was created. However, 
> > +if the application needs to create additional TMR rule for the same 
> > +tunnel it will need to obtain PMD items again.
> > +
> > +Application cannot destroy rte_tunnel object before it releases all 
> > +PMD actions & PMD items referencing that tunnel.
> > +
> >  Caveats
> >  -------
> >
> 
> [snip]
> 
> > diff --git a/lib/librte_ethdev/rte_flow.h 
> > b/lib/librte_ethdev/rte_flow.h index b0e4199192..1374b6e5a7 100644
> > --- a/lib/librte_ethdev/rte_flow.h
> > +++ b/lib/librte_ethdev/rte_flow.h
> > @@ -3324,6 +3324,202 @@ int
> >  rte_flow_get_aged_flows(uint16_t port_id, void **contexts,
> >  			uint32_t nb_contexts, struct rte_flow_error *error);
> >
> > +/* Tunnel information. */
> > +__rte_experimental
> > +struct rte_flow_ip_tunnel_key {
> > +	rte_be64_t tun_id; /**< Tunnel identification. */
> 
> What is it? Why is it big-endian? Why is it in IP tunnel key?
> I.e. why is it not in a generic structure?
>

tun_id is tunnel identification key, vni value in VXLAN case.
I'll change it to CPU order. 
Also, I will merge tunnel_key structure into rte_tunnel in next version.

> > +	union {
> > +		struct {
> > +			rte_be32_t src_addr; /**< IPv4 source address. */
> > +			rte_be32_t dst_addr; /**< IPv4 destination address.
> */
> > +		} ipv4;
> > +		struct {
> > +			uint8_t src_addr[16]; /**< IPv6 source address. */
> > +			uint8_t dst_addr[16]; /**< IPv6 destination address.
> */
> > +		} ipv6;
> > +	} u;
> > +	bool       is_ipv6; /**< True for valid IPv6 fields. Otherwise IPv4. */
> > +	rte_be16_t tun_flags; /**< Tunnel flags. */
> 
> Which flags? Where are these flags defined?
> Why is it big-endian?
> 

Tunnel flags value, as it appeared in incoming packet.
Since this structure may be used for different tunnel types,
exact definitions are different in each type.
I'll change is to CPU order.

> > +	uint8_t    tos; /**< TOS for IPv4, TC for IPv6. */
> > +	uint8_t    ttl; /**< TTL for IPv4, HL for IPv6. */
> 
> If combine, I'd stick to IPv6 terminology since it is a bit better 
> (well-thought, especially current tendencies in (re)naming in software).
> 
> > +	rte_be32_t label; /**< Flow Label for IPv6. */
> 
> What about IPv6 tunnels with extension headers? How to extend?
> 

These tunnels are not supported at this point.
Structure can be expended in future.

> > +	rte_be16_t tp_src; /**< Tunnel port source. */
> > +	rte_be16_t tp_dst; /**< Tunnel port destination. */
> 
> What about IP-in-IP tunnels? Is it applicable?
> 

No support for IP-in-IP tunnels in current version.
Future releases can have that feature, based on request.

> > +};
> > +
> > +
> > +/* Tunnel has a type and the key information. */ __rte_experimental 
> > +struct rte_flow_tunnel {
> > +	/**
> > +	 * Tunnel type, for example RTE_FLOW_ITEM_TYPE_VXLAN,
> > +	 * RTE_FLOW_ITEM_TYPE_NVGRE etc.
> > +	 */
> > +	enum rte_flow_item_type		type;
> > +	struct rte_flow_ip_tunnel_key	tun_info; /**< Tunnel key info. */
> 
> How to extended for non-IP tunnels? MPLS?
> Or tunnels with more protocols? E.g. MPLS-over-UDP?
>

Regarding extensions to new tunnels
Basically this structure is a software representor. Each tunnel format can add its own
fields. In the current implementation we target VXLAN tunnel type.
Additional tunnel types can be added in the future per demand with addition of relevant members to 
base structure.
 
> > +};
> > +
> > +/**
> > + * Indicate that the packet has a tunnel.
> > + */
> > +#define RTE_FLOW_RESTORE_INFO_TUNNEL  (1ULL << 0)
> > +
> > +/**
> > + * Indicate that the packet has a non decapsulated tunnel header.
> > + */
> > +#define RTE_FLOW_RESTORE_INFO_ENCAPSULATED  (1ULL << 1)
> > +
> > +/**
> > + * Indicate that the packet has a group_id.
> > + */
> > +#define RTE_FLOW_RESTORE_INFO_GROUP_ID  (1ULL << 2)
> > +
> > +/**
> > + * Restore information structure to communicate the current packet 
> > +processing
> > + * state when some of the processing pipeline is done in hardware 
> > +and should
> > + * continue in software.
> > + */
> > +__rte_experimental
> > +struct rte_flow_restore_info {
> > +	/**
> > +	 * Bitwise flags (RTE_FLOW_RESTORE_INFO_*) to indicate validation
> of
> > +	 * other fields in struct rte_flow_restore_info.
> > +	 */
> > +	uint64_t flags;
> > +	uint32_t group_id; /**< Group ID. */
> 
> What is the group ID here?
>

Flow group ID where partially offloaded packed missed.
 
> > +	struct rte_flow_tunnel tunnel; /**< Tunnel information. */ };
> > +
> > +/**
> > + * Allocate an array of actions to be used in rte_flow_create, to 
> > +implement
> > + * tunnel-decap-set for the given tunnel.
> > + * Sample usage:
> > + *   actions vxlan_decap / tunnel-decap-set(tunnel properties) /
> > + *            jump group 0 / end
> 
> Why is jump to group used in example above? Is it mandatory?
>

JUMP action is not strictly required.  Application may use any terminating action.
The most common case however is  jump.

> > + *
> > + * @param port_id
> > + *   Port identifier of Ethernet device.
> > + * @param[in] tunnel
> > + *   Tunnel properties.
> > + * @param[out] actions
> > + *   Array of actions to be allocated by the PMD. This array should be
> > + *   concatenated with the actions array provided to rte_flow_create.
> 
> Please, specify concatenation order explicitly.
> 

PMD actions precede application actions.
I'll update that spec in next version.

> > + * @param[out] num_of_actions
> > + *   Number of actions allocated.
> > + * @param[out] error
> > + *   Perform verbose error reporting if not NULL. PMDs initialize this
> > + *   structure in case of error only.
> > + *
> > + * @return
> > + *   0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_tunnel_decap_set(uint16_t port_id,
> > +			  struct rte_flow_tunnel *tunnel,
> > +			  struct rte_flow_action **actions,
> > +			  uint32_t *num_of_actions,
> 
> Why does approach to specify actions differ here?
> I.e. array of specified size vs END-terminated array?
> Must the actions array be END-terminated here?
> It must be a strong reason to do it and it should be explained.
> 

PMD actions returned in rte_flow_tunnel_decap_set() concatenated with application actions. 
Actions array produced by concatenation is passed to rte_flow_create() and must be END terminated.
PMD actions array is intermediate parameter and does not require END termination.

> > +			  struct rte_flow_error *error);
> > +
> > +/**
> > + * Allocate an array of items to be used in rte_flow_create, to 
> > +implement
> > + * tunnel-match for the given tunnel.
> > + * Sample usage:
> > + *   pattern tunnel-match(tunnel properties) / outer-header-matches /
> > + *           inner-header-matches / end
> > + *
> > + * @param port_id
> > + *   Port identifier of Ethernet device.
> > + * @param[in] tunnel
> > + *   Tunnel properties.
> > + * @param[out] items
> > + *   Array of items to be allocated by the PMD. This array should be
> > + *   concatenated with the items array provided to rte_flow_create.
> 
> Concatenation order/rules should be described.
> Since it is an output which entity does the concatenation.
> Is it allowed to refine PMD rules in application rule specification?
> 

PMD items precede application items.
I'll update this section too.
I'm not sure about the last question about rules refine. Please elaborate.

> > + * @param[out] num_of_items
> > + *   Number of items allocated.
> > + * @param[out] error
> > + *   Perform verbose error reporting if not NULL. PMDs initialize this
> > + *   structure in case of error only.
> > + *
> > + * @return
> > + *   0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_tunnel_match(uint16_t port_id,
> > +		      struct rte_flow_tunnel *tunnel,
> > +		      struct rte_flow_item **items,
> > +		      uint32_t *num_of_items,
> 
> Same as above for actions.
> 

PMD items returned in rte_flow_tunnel_match () concatenated with application items.
Items array produced by concatenation is passed to rte_flow_create() and must be END terminated. 
PMD items array is intermediate parameter and does not require END termination.

> > +		      struct rte_flow_error *error);
> > +
> > +/**
> > + * Populate the current packet processing state, if exists, for the 
> > +given
> mbuf.
> > + *
> > + * @param port_id
> > + *   Port identifier of Ethernet device.
> > + * @param[in] m
> > + *   Mbuf struct.
> > + * @param[out] info
> > + *   Restore information. Upon success contains the HW state.
> > + * @param[out] error
> > + *   Perform verbose error reporting if not NULL. PMDs initialize this
> > + *   structure in case of error only.
> > + *
> > + * @return
> > + *   0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_tunnel_get_restore_info(uint16_t port_id,
> > +				 struct rte_mbuf *m,
> > +				 struct rte_flow_restore_info *info,
> 
> Is it suggesting to make a copy of the restore info for each mbuf? It 
> sounds very expensive. Could you share your thoughts about it.
> 

Restore info may be different for each packet. For example,
if application declared tunnel info with tunnel type only,
restore info will have outer header data as well.

> > +				 struct rte_flow_error *error);
> > +
> > +/**
> > + * Release the action array as allocated by rte_flow_tunnel_decap_set.
> > + *
> > + * @param port_id
> > + *   Port identifier of Ethernet device.
> > + * @param[in] actions
> > + *   Array of actions to be released.
> > + * @param[in] num_of_actions
> > + *   Number of elements in actions array.
> > + * @param[out] error
> > + *   Perform verbose error reporting if not NULL. PMDs initialize this
> > + *   structure in case of error only.
> > + *
> > + * @return
> > + *   0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_tunnel_action_decap_release(uint16_t port_id,
> > +				     struct rte_flow_action *actions,
> > +				     uint32_t num_of_actions,
> 
> Same question as above for actions and items specification approach.
> 

PMD items and actions are intermediate parameters used by application
to create arguments for rte_flow_create() call.

> > +				     struct rte_flow_error *error);
> > +
> > +/**
> > + * Release the item array as allocated by rte_flow_tunnel_match.
> > + *
> > + * @param port_id
> > + *   Port identifier of Ethernet device.
> > + * @param[in] items
> > + *   Array of items to be released.
> > + * @param[in] num_of_items
> > + *   Number of elements in item array.
> > + * @param[out] error
> > + *   Perform verbose error reporting if not NULL. PMDs initialize this
> > + *   structure in case of error only.
> > + *
> > + * @return
> > + *   0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +__rte_experimental
> > +int
> > +rte_flow_tunnel_item_release(uint16_t port_id,
> > +			     struct rte_flow_item *items,
> > +			     uint32_t num_of_items,
> 
> Same question as above for actions and items specification approach.
> 

PMD items and actions are intermediate parameters used by application
to create arguments for rte_flow_create() call.

> > +			     struct rte_flow_error *error);
> >  #ifdef __cplusplus
> >  }
> >  #endif
> 
> [snip]
> 
> Andrew.
> 
> (Right now it is hard to fully imagine how to deal with it.
> And it looks like a shim to vendor-specific API. May be I'm wrong. 
> Hopefully the next version will have PMD implementation example and it 
> will shed a bit more light on it.)

The main idea of this API is to hide the internal logic and best practices of specific vendors.
For example, one vendor may use internal registers to save outer header while other vendor
will  only decap packet at the final stage.

I hope that examples above give you better understanding of the API.

PMD implementation is in progress.
However, we are in contact with other vendors that also interested implementing this API.
Since we don't want to block their progress it's important to reach conclusion on general API parts.
In any case, I'll be more than happy to get your review on PMD.


  reply	other threads:[~2020-08-19 14:30 UTC|newest]

Thread overview: 95+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-25 16:03 [dpdk-dev] [PATCH 0/2] " Gregory Etelson
2020-06-25 16:03 ` [dpdk-dev] [PATCH 1/2] ethdev: allow negative values in flow rule types Gregory Etelson
2020-07-05 13:34   ` Andrew Rybchenko
2020-08-19 14:33     ` Gregory Etelson
2020-06-25 16:03 ` [dpdk-dev] [PATCH 2/2] ethdev: tunnel offload model Gregory Etelson
     [not found]   ` <DB8PR05MB6761ED02BCD188771BDCDE64A86F0@DB8PR05MB6761.eurprd05.prod.outlook.com>
     [not found]     ` <38d3513f-1261-0fbc-7c56-f83ced61f97a@ashroe.eu>
2020-07-01  6:52       ` Gregory Etelson
2020-07-13  8:21         ` Thomas Monjalon
2020-07-13 13:23           ` Gregory Etelson
2020-07-05 14:50   ` Andrew Rybchenko
2020-08-19 14:30     ` Gregory Etelson [this message]
2020-07-05 13:39 ` [dpdk-dev] [PATCH 0/2] " Andrew Rybchenko
2020-09-08 20:15 ` [dpdk-dev] [PATCH v2 0/4] Tunnel Offload API Gregory Etelson
2020-09-08 20:15   ` [dpdk-dev] [PATCH v2 1/4] ethdev: allow negative values in flow rule types Gregory Etelson
2020-09-15  4:36     ` Ajit Khaparde
2020-09-15  8:46       ` Andrew Rybchenko
2020-09-15 10:27         ` Gregory Etelson
2020-09-16 17:21           ` Gregory Etelson
2020-09-17  6:49             ` Andrew Rybchenko
2020-09-17  7:47               ` Ori Kam
2020-09-17 15:15                 ` Andrew Rybchenko
2020-09-17  7:56               ` Gregory Etelson
2020-09-17 15:18                 ` Andrew Rybchenko
2020-09-15  8:45     ` Andrew Rybchenko
2020-09-15 16:17       ` Gregory Etelson
2020-09-08 20:15   ` [dpdk-dev] [PATCH v2 2/4] ethdev: tunnel offload model Gregory Etelson
2020-09-08 20:15   ` [dpdk-dev] [PATCH v2 3/4] net/mlx5: implement tunnel offload API Gregory Etelson
2020-09-08 20:15   ` [dpdk-dev] [PATCH v2 4/4] app/testpmd: support " Gregory Etelson
2020-09-15  4:47     ` Ajit Khaparde
2020-09-15 10:44       ` Gregory Etelson
2020-09-30  9:18 ` [dpdk-dev] [PATCH v3 0/4] Tunnel Offload API Gregory Etelson
2020-09-30  9:18   ` [dpdk-dev] [PATCH v3 1/4] ethdev: allow negative values in flow rule types Gregory Etelson
2020-10-04  5:40     ` Ajit Khaparde
2020-10-04  9:24       ` Gregory Etelson
2020-09-30  9:18   ` [dpdk-dev] [PATCH v3 2/4] ethdev: tunnel offload model Gregory Etelson
2020-09-30  9:18   ` [dpdk-dev] [PATCH v3 3/4] net/mlx5: implement tunnel offload API Gregory Etelson
2020-09-30  9:18   ` [dpdk-dev] [PATCH v3 4/4] app/testpmd: add commands for " Gregory Etelson
2020-10-01  5:32     ` Ajit Khaparde
2020-10-01  9:05       ` Gregory Etelson
2020-10-04  5:40         ` Ajit Khaparde
2020-10-04  9:29           ` Gregory Etelson
2020-10-04 13:50 ` [dpdk-dev] [PATCH v4 0/4] Tunnel Offload API Gregory Etelson
2020-10-04 13:50   ` [dpdk-dev] [PATCH v4 1/4] ethdev: allow negative values in flow rule types Gregory Etelson
2020-10-14 23:40     ` Thomas Monjalon
2020-10-04 13:50   ` [dpdk-dev] [PATCH v4 2/4] ethdev: tunnel offload model Gregory Etelson
2020-10-06  9:47     ` Sriharsha Basavapatna
2020-10-07 12:36       ` Gregory Etelson
2020-10-14 17:23         ` Ferruh Yigit
2020-10-16  9:15           ` Gregory Etelson
2020-10-14 23:55     ` Thomas Monjalon
2020-10-04 13:50   ` [dpdk-dev] [PATCH v4 3/4] net/mlx5: implement tunnel offload API Gregory Etelson
2020-10-04 13:50   ` [dpdk-dev] [PATCH v4 4/4] app/testpmd: add commands for " Gregory Etelson
2020-10-04 13:59     ` Ori Kam
2020-10-14 17:25   ` [dpdk-dev] [PATCH v4 0/4] Tunnel Offload API Ferruh Yigit
2020-10-15 12:41 ` [dpdk-dev] [PATCH v5 0/3] " Gregory Etelson
2020-10-15 12:41   ` [dpdk-dev] [PATCH v5 1/3] ethdev: allow negative values in flow rule types Gregory Etelson
2020-10-15 12:41   ` [dpdk-dev] [PATCH v5 2/3] ethdev: tunnel offload model Gregory Etelson
2020-10-15 12:41   ` [dpdk-dev] [PATCH v5 3/3] app/testpmd: add commands for tunnel offload API Gregory Etelson
2020-10-15 22:47   ` [dpdk-dev] [PATCH v5 0/3] Tunnel Offload API Ferruh Yigit
2020-10-16  8:55 ` [dpdk-dev] [PATCH v6 " Gregory Etelson
2020-10-16  8:55   ` [dpdk-dev] [PATCH v6 1/3] ethdev: allow negative values in flow rule types Gregory Etelson
2020-10-16  8:55   ` [dpdk-dev] [PATCH v6 2/3] ethdev: tunnel offload model Gregory Etelson
2020-10-16  8:55   ` [dpdk-dev] [PATCH v6 3/3] app/testpmd: add commands for tunnel offload API Gregory Etelson
2020-10-16 10:33 ` [dpdk-dev] [PATCH v7 0/3] Tunnel Offload API Gregory Etelson
2020-10-16 10:33   ` [dpdk-dev] [PATCH v7 1/3] ethdev: allow negative values in flow rule types Gregory Etelson
2020-10-16 10:33   ` [dpdk-dev] [PATCH v7 2/3] ethdev: tunnel offload model Gregory Etelson
2020-10-16 10:34   ` [dpdk-dev] [PATCH v7 3/3] app/testpmd: add commands for tunnel offload API Gregory Etelson
2020-10-16 12:10   ` [dpdk-dev] [PATCH v7 0/3] Tunnel Offload API Ferruh Yigit
2020-10-16 12:51 ` [dpdk-dev] [PATCH v8 " Gregory Etelson
2020-10-16 12:51   ` [dpdk-dev] [PATCH v8 1/3] ethdev: allow negative values in flow rule types Gregory Etelson
2020-10-16 12:51   ` [dpdk-dev] [PATCH v8 2/3] ethdev: tunnel offload model Gregory Etelson
2020-10-16 15:41     ` Kinsella, Ray
2021-03-02  9:22     ` Ivan Malov
2021-03-02  9:42       ` Thomas Monjalon
2021-03-03 14:03         ` Ivan Malov
2021-03-04  6:35           ` Eli Britstein
2021-03-08 14:01       ` Gregory Etelson
2020-10-16 12:51   ` [dpdk-dev] [PATCH v8 3/3] app/testpmd: add commands for tunnel offload API Gregory Etelson
2020-10-16 13:19   ` [dpdk-dev] [PATCH v8 0/3] Tunnel Offload API Ferruh Yigit
2020-10-16 14:20     ` Ferruh Yigit
2020-10-18 12:15 ` [dpdk-dev] [PATCH] ethdev: rename tunnel offload callbacks Gregory Etelson
2020-10-19  8:31   ` Ferruh Yigit
2020-10-19  9:56     ` Kinsella, Ray
2020-10-19 21:29       ` Thomas Monjalon
2020-10-21  9:22 ` [dpdk-dev] [PATCH] net/mlx5: implement tunnel offload API Gregory Etelson
2020-10-22 16:00 ` [dpdk-dev] [PATCH v2] " Gregory Etelson
2020-10-23 13:49 ` [dpdk-dev] [PATCH v3] " Gregory Etelson
2020-10-23 13:57 ` [dpdk-dev] [PATCH v4] " Gregory Etelson
2020-10-25 14:08 ` [dpdk-dev] [PATCH v5] " Gregory Etelson
2020-10-25 15:01   ` Raslan Darawsheh
2020-10-27 16:12 ` [dpdk-dev] [PATCH] net/mlx5: tunnel offload code cleanup Gregory Etelson
2020-10-27 16:29   ` Slava Ovsiienko
2020-10-27 17:16   ` Raslan Darawsheh
2020-10-28 12:33     ` Andrew Rybchenko
2020-10-28  4:58 ` [dpdk-dev] [PATCH] net/mlx5: fix tunnel flow destroy Gregory Etelson
2020-11-02 16:27   ` Raslan Darawsheh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=BY5PR12MB366850E5D62161EDF686C14BA55D0@BY5PR12MB3668.namprd12.prod.outlook.com \
    --to=getelson@nvidia.com \
    --cc=arybchenko@solarflare.com \
    --cc=dev@dpdk.org \
    --cc=elibr@nvidia.com \
    --cc=matan@nvidia.com \
    --cc=orika@nvidia.com \
    --cc=rasland@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).