[dpdk-dev,v6,1/2] ethdev: introduce generic IP/UDP tunnel checksum and TSO

Message ID 20180418135852.27598-1-xuemingl@mellanox.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Xueming Li April 18, 2018, 1:58 p.m. UTC
  This patch introduce new TX offload flags for device that supports
IP or UDP tunneled packet L3/L4 checksum and TSO offload.
It will be used for non-standard tunnels.

The support from the device is for inner and outer checksums on
IPV4/TCP/UDP and TSO for *any packet with the following format*:

<some headers> / [optional IPv4/IPv6] / [optional TCP/UDP] / <some
headers> / [optional inner IPv4/IPv6] / [optional TCP/UDP]

For example the following packets can use this feature:

1. eth / ipv4 / udp / VXLAN / ip / tcp
2. eth / ipv4 / GRE / MPLS / ipv4 / udp

Please note that specific tunnel headers that contain payload length,
sequence id or checksum will not be updated.

The new flag PKT_TX_TUNNEL_IP is redundant with PKT_TX_OUTER_IP_CKSUM.
The old flag PKT_TX_OUTER_IP_CKSUM can be deprecated and removed in
later release.

Signed-off-by: Xueming Li <xuemingl@mellanox.com>
---
 lib/librte_ether/rte_ethdev.h | 12 ++++++++++++
 lib/librte_mbuf/rte_mbuf.c    |  6 ++++++
 lib/librte_mbuf/rte_mbuf.h    | 25 +++++++++++++++++++++++++
 3 files changed, 43 insertions(+)
  

Comments

Thomas Monjalon April 18, 2018, 2:28 p.m. UTC | #1
18/04/2018 15:58, Xueming Li:
> This patch introduce new TX offload flags for device that supports
> IP or UDP tunneled packet L3/L4 checksum and TSO offload.
> It will be used for non-standard tunnels.
> 
> The support from the device is for inner and outer checksums on
> IPV4/TCP/UDP and TSO for *any packet with the following format*:
> 
> <some headers> / [optional IPv4/IPv6] / [optional TCP/UDP] / <some
> headers> / [optional inner IPv4/IPv6] / [optional TCP/UDP]
> 
> For example the following packets can use this feature:
> 
> 1. eth / ipv4 / udp / VXLAN / ip / tcp
> 2. eth / ipv4 / GRE / MPLS / ipv4 / udp
> 
> Please note that specific tunnel headers that contain payload length,
> sequence id or checksum will not be updated.
> 
> The new flag PKT_TX_TUNNEL_IP is redundant with PKT_TX_OUTER_IP_CKSUM.
> The old flag PKT_TX_OUTER_IP_CKSUM can be deprecated and removed in
> later release.
> 
> Signed-off-by: Xueming Li <xuemingl@mellanox.com>

Except a small comment below, it looks OK.

Acked-by: Thomas Monjalon <thomas@monjalon.net>

Please send a deprecation notice for PKT_TX_OUTER_IP_CKSUM.


> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> +/**
> + * Generic IP encapsulated tunnel type, used for TSO and checksum offload.
> + * It can be used for tunnels which are not standards or listed above.
> + * It is preferred to use specific tunnel flags like PKT_TX_TUNNEL_VXLAN
> + * if possible.

PKT_TX_TUNNEL_GRE or PKT_TX_TUNNEL_IPIP may be a better example than
PKT_TX_TUNNEL_VXLAN in IP tunnel case.

> + * The ethdev must be configured with DEV_TX_OFFLOAD_IP_TNL_TSO.
> + * Outer and inner checksums are done according to the existing flags like
> + * PKT_TX_xxx_CKSUM.
> + * Specific tunnel headers that contain payload length, sequence id
> + * or checksum are not expected to be updated.
> + */
> +#define PKT_TX_TUNNEL_IP (0xDULL << 45)
> +/**
> + * Generic UDP encapsulated tunnel type, used for TSO and checksum offload.
> + * UDP tunnel type implies outer IP layer.
> + * It can be used for tunnels which are not standards or listed above.
> + * It is preferred to use specific tunnel flags like PKT_TX_TUNNEL_VXLAN
> + * if possible.
> + * The ethdev must be configured with DEV_TX_OFFLOAD_UDP_TNL_TSO.
> + * Outer and inner checksums are done according to the existing flags like
> + * PKT_TX_xxx_CKSUM.
> + * Specific tunnel headers that contain payload length, sequence id
> + * or checksum are not expected to be updated.
> + */
> +#define PKT_TX_TUNNEL_UDP (0xEULL << 45)
  
Ananyev, Konstantin April 18, 2018, 4:45 p.m. UTC | #2
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Wednesday, April 18, 2018 3:28 PM
> To: Xueming Li <xuemingl@mellanox.com>
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Yongseok Koh <yskoh@mellanox.com>; Olivier MATZ
> <olivier.matz@6wind.com>; Shahaf Shuler <shahafs@mellanox.com>; Iremonger, Bernard <bernard.iremonger@intel.com>; Yigit, Ferruh
> <ferruh.yigit@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 1/2] ethdev: introduce generic IP/UDP tunnel checksum and TSO
> 
> 18/04/2018 15:58, Xueming Li:
> > This patch introduce new TX offload flags for device that supports
> > IP or UDP tunneled packet L3/L4 checksum and TSO offload.
> > It will be used for non-standard tunnels.
> >
> > The support from the device is for inner and outer checksums on
> > IPV4/TCP/UDP and TSO for *any packet with the following format*:
> >
> > <some headers> / [optional IPv4/IPv6] / [optional TCP/UDP] / <some
> > headers> / [optional inner IPv4/IPv6] / [optional TCP/UDP]
> >
> > For example the following packets can use this feature:
> >
> > 1. eth / ipv4 / udp / VXLAN / ip / tcp
> > 2. eth / ipv4 / GRE / MPLS / ipv4 / udp
> >
> > Please note that specific tunnel headers that contain payload length,
> > sequence id or checksum will not be updated.
> >
> > The new flag PKT_TX_TUNNEL_IP is redundant with PKT_TX_OUTER_IP_CKSUM.
> > The old flag PKT_TX_OUTER_IP_CKSUM can be deprecated and removed in
> > later release.
> >
> > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> 
> Except a small comment below, it looks OK.
> 
> Acked-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Please send a deprecation notice for PKT_TX_OUTER_IP_CKSUM.

I probably missed something, but why PKT_TX_OUTER_IP_CKSUM
is supposed to be deprecated?
Konstantin

> 
> 
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > +/**
> > + * Generic IP encapsulated tunnel type, used for TSO and checksum offload.
> > + * It can be used for tunnels which are not standards or listed above.
> > + * It is preferred to use specific tunnel flags like PKT_TX_TUNNEL_VXLAN
> > + * if possible.
> 
> PKT_TX_TUNNEL_GRE or PKT_TX_TUNNEL_IPIP may be a better example than
> PKT_TX_TUNNEL_VXLAN in IP tunnel case.
> 
> > + * The ethdev must be configured with DEV_TX_OFFLOAD_IP_TNL_TSO.
> > + * Outer and inner checksums are done according to the existing flags like
> > + * PKT_TX_xxx_CKSUM.
> > + * Specific tunnel headers that contain payload length, sequence id
> > + * or checksum are not expected to be updated.
> > + */
> > +#define PKT_TX_TUNNEL_IP (0xDULL << 45)
> > +/**
> > + * Generic UDP encapsulated tunnel type, used for TSO and checksum offload.
> > + * UDP tunnel type implies outer IP layer.
> > + * It can be used for tunnels which are not standards or listed above.
> > + * It is preferred to use specific tunnel flags like PKT_TX_TUNNEL_VXLAN
> > + * if possible.
> > + * The ethdev must be configured with DEV_TX_OFFLOAD_UDP_TNL_TSO.
> > + * Outer and inner checksums are done according to the existing flags like
> > + * PKT_TX_xxx_CKSUM.
> > + * Specific tunnel headers that contain payload length, sequence id
> > + * or checksum are not expected to be updated.
> > + */
> > +#define PKT_TX_TUNNEL_UDP (0xEULL << 45)
> 
>
  
Thomas Monjalon April 18, 2018, 6:02 p.m. UTC | #3
18/04/2018 18:45, Ananyev, Konstantin:
> From: Thomas Monjalon
> > 18/04/2018 15:58, Xueming Li:
> > > The new flag PKT_TX_TUNNEL_IP is redundant with PKT_TX_OUTER_IP_CKSUM.
> > > The old flag PKT_TX_OUTER_IP_CKSUM can be deprecated and removed in
> > > later release.
> > >
> > > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > 
> > Except a small comment below, it looks OK.
> > 
> > Acked-by: Thomas Monjalon <thomas@monjalon.net>
> > 
> > Please send a deprecation notice for PKT_TX_OUTER_IP_CKSUM.
> 
> I probably missed something, but why PKT_TX_OUTER_IP_CKSUM
> is supposed to be deprecated?

Because PKT_TX_TUNNEL_* flags sepersede it. I think we need to discuss it.

We use the offload flags PKT_TX_TUNNEL_* when we request some offloads
in the outer and the inner headers at the same time.
When setting a tunnel flag PKT_TX_TUNNEL_*, it can be expected that
the outer checksums will be offloaded, otherwise it is almost
impossible to request an offload in the inner header.
In the case of an UDP tunnel, any change in the inner packet will
require an UDP checksum update.
In the case of an IP tunnel, a change of the inner packet size
(like TSO) will require an IP checksum update.

If we really require PKT_TX_OUTER_IP_CKSUM to be set in addition to
PKT_TX_TUNNEL_* flags, then we should add more PKT_TX_OUTER_*_CKSUM,
like PKT_TX_OUTER_UDP_CKSUM which is missing.

Opinions?
  
Xueming Li April 20, 2018, 12:48 p.m. UTC | #4
V7:
- Removed PKT_TX_OUTER_IP_CKSUM deprecation suggestion from commit log

V6:
- Comments update according to maillist discussion.

V5:
- Removed duplicated testpmd patch in other pathset.
- More comments on PKT_TX_TUNNEL_IP and PKT_TX_TUNNEL_UDP

V4:
- Removed DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM and DEV_TX_OFFLOAD_GENERIC_TNL_TSO
- Replaced with DEV_TX_OFFLOAD_IP_TNL_TSO
- Removed PKT_TX_OUTER_UDP
- Splited PKT_TX_TUNNEL_UNKNOWN into PKT_TX_TUNNEL_IP and PKT_TX_TUNNEL_UDP

V3:
- Add VXLAN-GPE and GRE extention support to testpmd csum forwarding enginee
- Split DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM_TSO into DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM
  and DEV_TX_OFFLOAD_GENERIC_TNL_TSO
- Add PKT_TX_TUNNEL_UNKNOWN and PKT_TX_OUTER_UDP

  http://www.dpdk.org/dev/patchwork/patch/34655/

Xueming Li (2):
  ethdev: introduce generic IP/UDP tunnel checksum and TSO
  app/testpmd: testpmd support Tx generic tunnel offloads

 app/test-pmd/cmdline.c        | 14 ++++++++++++--
 app/test-pmd/config.c         | 17 +++++++++++++++++
 lib/librte_ether/rte_ethdev.h | 12 ++++++++++++
 lib/librte_mbuf/rte_mbuf.c    |  6 ++++++
 lib/librte_mbuf/rte_mbuf.h    | 25 +++++++++++++++++++++++++
 5 files changed, 72 insertions(+), 2 deletions(-)
  
Olivier Matz April 23, 2018, 9:55 a.m. UTC | #5
On Wed, Apr 18, 2018 at 08:02:35PM +0200, Thomas Monjalon wrote:
> 18/04/2018 18:45, Ananyev, Konstantin:
> > From: Thomas Monjalon
> > > 18/04/2018 15:58, Xueming Li:
> > > > The new flag PKT_TX_TUNNEL_IP is redundant with PKT_TX_OUTER_IP_CKSUM.
> > > > The old flag PKT_TX_OUTER_IP_CKSUM can be deprecated and removed in
> > > > later release.
> > > >
> > > > Signed-off-by: Xueming Li <xuemingl@mellanox.com>
> > > 
> > > Except a small comment below, it looks OK.
> > > 
> > > Acked-by: Thomas Monjalon <thomas@monjalon.net>
> > > 
> > > Please send a deprecation notice for PKT_TX_OUTER_IP_CKSUM.
> > 
> > I probably missed something, but why PKT_TX_OUTER_IP_CKSUM
> > is supposed to be deprecated?
> 
> Because PKT_TX_TUNNEL_* flags sepersede it. I think we need to discuss it.
> 
> We use the offload flags PKT_TX_TUNNEL_* when we request some offloads
> in the outer and the inner headers at the same time.
> When setting a tunnel flag PKT_TX_TUNNEL_*, it can be expected that
> the outer checksums will be offloaded, otherwise it is almost
> impossible to request an offload in the inner header.
> In the case of an UDP tunnel, any change in the inner packet will
> require an UDP checksum update.
> In the case of an IP tunnel, a change of the inner packet size
> (like TSO) will require an IP checksum update.
> 
> If we really require PKT_TX_OUTER_IP_CKSUM to be set in addition to
> PKT_TX_TUNNEL_* flags, then we should add more PKT_TX_OUTER_*_CKSUM,
> like PKT_TX_OUTER_UDP_CKSUM which is missing.

I agree, I think PKT_TX_TUNNEL_* implies PKT_TX_OUTER_IP_CKSUM.
And I don't see use-case where we only need PKT_TX_OUTER_IP_CKSUM,
because in that case we can use PKT_TX_IP_CKSUM.

But this can be discussed in a second step, for now I think it's safer
to keep it as is.

Olivier
  
Xueming Li April 23, 2018, 11:36 a.m. UTC | #6
V8:
- Fixed comment of PKT_TX_TUNNEL_UDP
V7:
- Removed PKT_TX_OUTER_IP_CKSUM deprecation suggestion from commit log

V6:
- Comments update according to maillist discussion.

V5:
- Removed duplicated testpmd patch in other pathset.
- More comments on PKT_TX_TUNNEL_IP and PKT_TX_TUNNEL_UDP

V4:
- Removed DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM and DEV_TX_OFFLOAD_GENERIC_TNL_TSO
- Replaced with DEV_TX_OFFLOAD_IP_TNL_TSO
- Removed PKT_TX_OUTER_UDP
- Splited PKT_TX_TUNNEL_UNKNOWN into PKT_TX_TUNNEL_IP and PKT_TX_TUNNEL_UDP

V3:
- Add VXLAN-GPE and GRE extention support to testpmd csum forwarding enginee
- Split DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM_TSO into DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM
  and DEV_TX_OFFLOAD_GENERIC_TNL_TSO
- Add PKT_TX_TUNNEL_UNKNOWN and PKT_TX_OUTER_UDP

  http://www.dpdk.org/dev/patchwork/patch/34655/



Xueming Li (2):
  ethdev: introduce generic IP/UDP tunnel checksum and TSO
  app/testpmd: testpmd support Tx generic tunnel offloads

 app/test-pmd/cmdline.c        | 14 ++++++++++++--
 app/test-pmd/config.c         | 17 +++++++++++++++++
 lib/librte_ether/rte_ethdev.h | 12 ++++++++++++
 lib/librte_mbuf/rte_mbuf.c    |  6 ++++++
 lib/librte_mbuf/rte_mbuf.h    | 25 +++++++++++++++++++++++++
 5 files changed, 72 insertions(+), 2 deletions(-)
  
Ferruh Yigit April 23, 2018, 4:17 p.m. UTC | #7
On 4/23/2018 12:36 PM, Xueming Li wrote:
> V8:
> - Fixed comment of PKT_TX_TUNNEL_UDP
> V7:
> - Removed PKT_TX_OUTER_IP_CKSUM deprecation suggestion from commit log
> 
> V6:
> - Comments update according to maillist discussion.
> 
> V5:
> - Removed duplicated testpmd patch in other pathset.
> - More comments on PKT_TX_TUNNEL_IP and PKT_TX_TUNNEL_UDP
> 
> V4:
> - Removed DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM and DEV_TX_OFFLOAD_GENERIC_TNL_TSO
> - Replaced with DEV_TX_OFFLOAD_IP_TNL_TSO
> - Removed PKT_TX_OUTER_UDP
> - Splited PKT_TX_TUNNEL_UNKNOWN into PKT_TX_TUNNEL_IP and PKT_TX_TUNNEL_UDP
> 
> V3:
> - Add VXLAN-GPE and GRE extention support to testpmd csum forwarding enginee
> - Split DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM_TSO into DEV_TX_OFFLOAD_GENERIC_TNL_CKSUM
>   and DEV_TX_OFFLOAD_GENERIC_TNL_TSO
> - Add PKT_TX_TUNNEL_UNKNOWN and PKT_TX_OUTER_UDP
> 
>   http://www.dpdk.org/dev/patchwork/patch/34655/
> 
> 
> 
> Xueming Li (2):
>   ethdev: introduce generic IP/UDP tunnel checksum and TSO
>   app/testpmd: testpmd support Tx generic tunnel offloads

Series applied to dpdk-next-net/master, thanks.
  

Patch

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 4f417f573..ef152dec6 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -980,6 +980,18 @@  struct rte_eth_conf {
  *   the same mempool and has refcnt = 1.
  */
 #define DEV_TX_OFFLOAD_SECURITY         0x00020000
+/**
+ * Device supports generic UDP tunneled packet TSO.
+ * Application must set PKT_TX_TUNNEL_UDP and other mbuf fields required
+ * for tunnel TSO.
+ */
+#define DEV_TX_OFFLOAD_UDP_TNL_TSO      0x00040000
+/**
+ * Device supports generic IP tunneled packet TSO.
+ * Application must set PKT_TX_TUNNEL_IP and other mbuf fields required
+ * for tunnel TSO.
+ */
+#define DEV_TX_OFFLOAD_IP_TNL_TSO       0x00080000
 
 /*
  * If new Tx offload capabilities are defined, they also must be
diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 3f4c83305..64e960d4c 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -390,6 +390,8 @@  const char *rte_get_tx_ol_flag_name(uint64_t mask)
 	case PKT_TX_TUNNEL_IPIP: return "PKT_TX_TUNNEL_IPIP";
 	case PKT_TX_TUNNEL_GENEVE: return "PKT_TX_TUNNEL_GENEVE";
 	case PKT_TX_TUNNEL_MPLSINUDP: return "PKT_TX_TUNNEL_MPLSINUDP";
+	case PKT_TX_TUNNEL_IP: return "PKT_TX_TUNNEL_IP";
+	case PKT_TX_TUNNEL_UDP: return "PKT_TX_TUNNEL_UDP";
 	case PKT_TX_MACSEC: return "PKT_TX_MACSEC";
 	case PKT_TX_SEC_OFFLOAD: return "PKT_TX_SEC_OFFLOAD";
 	default: return NULL;
@@ -424,6 +426,10 @@  rte_get_tx_ol_flag_list(uint64_t mask, char *buf, size_t buflen)
 		  "PKT_TX_TUNNEL_NONE" },
 		{ PKT_TX_TUNNEL_MPLSINUDP, PKT_TX_TUNNEL_MASK,
 		  "PKT_TX_TUNNEL_NONE" },
+		{ PKT_TX_TUNNEL_IP, PKT_TX_TUNNEL_MASK,
+		  "PKT_TX_TUNNEL_NONE" },
+		{ PKT_TX_TUNNEL_UDP, PKT_TX_TUNNEL_MASK,
+		  "PKT_TX_TUNNEL_NONE" },
 		{ PKT_TX_MACSEC, PKT_TX_MACSEC, NULL },
 		{ PKT_TX_SEC_OFFLOAD, PKT_TX_SEC_OFFLOAD, NULL },
 	};
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 06eceba37..a1ee1c3a7 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -210,6 +210,31 @@  extern "C" {
 #define PKT_TX_TUNNEL_GENEVE  (0x4ULL << 45)
 /**< TX packet with MPLS-in-UDP RFC 7510 header. */
 #define PKT_TX_TUNNEL_MPLSINUDP (0x5ULL << 45)
+/**
+ * Generic IP encapsulated tunnel type, used for TSO and checksum offload.
+ * It can be used for tunnels which are not standards or listed above.
+ * It is preferred to use specific tunnel flags like PKT_TX_TUNNEL_VXLAN
+ * if possible.
+ * The ethdev must be configured with DEV_TX_OFFLOAD_IP_TNL_TSO.
+ * Outer and inner checksums are done according to the existing flags like
+ * PKT_TX_xxx_CKSUM.
+ * Specific tunnel headers that contain payload length, sequence id
+ * or checksum are not expected to be updated.
+ */
+#define PKT_TX_TUNNEL_IP (0xDULL << 45)
+/**
+ * Generic UDP encapsulated tunnel type, used for TSO and checksum offload.
+ * UDP tunnel type implies outer IP layer.
+ * It can be used for tunnels which are not standards or listed above.
+ * It is preferred to use specific tunnel flags like PKT_TX_TUNNEL_VXLAN
+ * if possible.
+ * The ethdev must be configured with DEV_TX_OFFLOAD_UDP_TNL_TSO.
+ * Outer and inner checksums are done according to the existing flags like
+ * PKT_TX_xxx_CKSUM.
+ * Specific tunnel headers that contain payload length, sequence id
+ * or checksum are not expected to be updated.
+ */
+#define PKT_TX_TUNNEL_UDP (0xEULL << 45)
 /* add new TX TUNNEL type here */
 #define PKT_TX_TUNNEL_MASK    (0xFULL << 45)