[PATCH v1] i40: fix the VXLAN TSO issue

[PATCH v1] i40: fix the VXLAN TSO issue

[Zhe Tao]

Problem:
when using the TSO + VXLAM feature in i40e, the outer UDP len will
sometimes be a invalid value for the multiple UDP segements which
are TSOed by the i40e for the inner TCP.

Fix this problem by add the tunnel type field in the i40e descriptor
which is missed before.

Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")

Signed-off-by: Zhe Tao <zhe.tao@intel.com>
---
 app/test-pmd/csumonly.c      | 26 +++++++++++++++++++-------
 drivers/net/i40e/i40e_rxtx.c | 10 ++++++++--
 lib/librte_mbuf/rte_mbuf.h   | 11 +++++++++++
 3 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index ac4bd8f..d423c20 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -204,7 +204,8 @@ parse_ethernet(struct ether_hdr *eth_hdr, struct testpmd_offload_info *info)
 static void
 parse_vxlan(struct udp_hdr *udp_hdr,
 	    struct testpmd_offload_info *info,
-	    uint32_t pkt_type)
+	    uint32_t pkt_type,
+	    uint64_t *ol_flags)
 {
 	struct ether_hdr *eth_hdr;
 
@@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
 		RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
 		return;
 
+	*ol_flags |= PKT_TX_TUNNEL_VXLAN;
 	info->is_tunnel = 1;
 	info->outer_ethertype = info->ethertype;
 	info->outer_l2_len = info->l2_len;
@@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
 
 /* Parse a gre header */
 static void
-parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
+parse_gre(struct simple_gre_hdr *gre_hdr,
+	  struct testpmd_offload_info *info,
+	  uint64_t *ol_flags)
 {
 	struct ether_hdr *eth_hdr;
 	struct ipv4_hdr *ipv4_hdr;
@@ -242,6 +246,8 @@ parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
 	if ((gre_hdr->flags & _htons(~GRE_SUPPORTED_FIELDS)) != 0)
 		return;
 
+	*ol_flags |= PKT_TX_TUNNEL_GRE;
+
 	gre_len += sizeof(struct simple_gre_hdr);
 
 	if (gre_hdr->flags & _htons(GRE_KEY_PRESENT))
@@ -417,7 +423,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
  * packet */
 static uint64_t
 process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
-	uint16_t testpmd_ol_flags)
+	uint16_t testpmd_ol_flags, uint64_t orig_ol_flags)
 {
 	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
 	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr;
@@ -442,6 +448,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	 * hardware supporting it today, and no API for it. */
 
 	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
+	if ((orig_ol_flags & PKT_TX_TCP_SEG) &&
+	    ((orig_ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN))
+		udp_hdr->dgram_cksum = 0;
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
@@ -705,15 +714,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 			if (info.l4_proto == IPPROTO_UDP) {
 				struct udp_hdr *udp_hdr;
 				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
-					info.l3_len);
-				parse_vxlan(udp_hdr, &info, m->packet_type);
+					   info.l3_len);
+				parse_vxlan(udp_hdr, &info, m->packet_type,
+					    &ol_flags);
 			} else if (info.l4_proto == IPPROTO_GRE) {
 				struct simple_gre_hdr *gre_hdr;
 				gre_hdr = (struct simple_gre_hdr *)
 					((char *)l3_hdr + info.l3_len);
-				parse_gre(gre_hdr, &info);
+				parse_gre(gre_hdr, &info, &ol_flags);
 			} else if (info.l4_proto == IPPROTO_IPIP) {
 				void *encap_ip_hdr;
+
+				ol_flags |= PKT_TX_TUNNEL_IPIP;
 				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
 				parse_encap_ip(encap_ip_hdr, &info);
 			}
@@ -745,7 +757,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		 * processed in hardware. */
 		if (info.is_tunnel == 1) {
 			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
-				testpmd_ol_flags);
+				testpmd_ol_flags, ol_flags);
 		}
 
 		/* step 4: fill the mbuf meta data (flags and header lengths) */
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 049a813..272b04c 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
 			union i40e_tx_offload tx_offload,
 			uint32_t *cd_tunneling)
 {
+	/* Tx pkts tunnel type*/
+	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
+		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
+	else if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE)
+		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
+
 	/* UDP tunneling packet TX checksum offload */
 	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
 
@@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
 
 /* set i40e TSO context descriptor */
 static inline uint64_t
-i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
+i40e_set_tso_ctx(struct rte_mbuf *mbuf,
+		 union i40e_tx_offload tx_offload)
 {
 	uint64_t ctx_desc = 0;
 	uint32_t cd_cmd, hdr_len, cd_tso_len;
@@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
 		 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
 		((uint64_t)mbuf->tso_segsz <<
 		 I40E_TXD_CTX_QW1_MSS_SHIFT);
-
 	return ctx_desc;
 }
 
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 15e3a10..847767c 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -133,6 +133,17 @@ extern "C" {
 /* add new TX flags here */
 
 /**
+ * Bits 45:48 used for the tunnel type.
+ * When doing Tx offload like TSO or checksum, the HW needs to configure the
+ * tunnel type into the HW descriptors.
+ */
+#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
+#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
+#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
+/* add new TX TUNNEL type here */
+#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
+
+/**
  * Second VLAN insertion (QinQ) flag.
  */
 #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
-- 
2.1.4

Re: [PATCH v1] i40: fix the VXLAN TSO issue

[Wu, Jingjing]

> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
>  			union i40e_tx_offload tx_offload,
>  			uint32_t *cd_tunneling)
>  {
> +	/* Tx pkts tunnel type*/
> +	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
> +		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
> +	else if ((ol_flags & PKT_TX_TUNNEL_MASK) ==
> PKT_TX_TUNNEL_GRE)
> +		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
> +
>  	/* UDP tunneling packet TX checksum offload */
>  	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
> 
Besides the tunnel type programming, please also check the L4TUNLEN in tunnel parameters.

> @@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
> 
>  /* set i40e TSO context descriptor */
>  static inline uint64_t
> -i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> +i40e_set_tso_ctx(struct rte_mbuf *mbuf,
> +		 union i40e_tx_offload tx_offload)
>  {
>  	uint64_t ctx_desc = 0;
>  	uint32_t cd_cmd, hdr_len, cd_tso_len;
> @@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union
> i40e_tx_offload tx_offload)

Please have a look at the calculation of tso length here. Only tunnel type added may not be enogh.
 
>  		 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
>  		((uint64_t)mbuf->tso_segsz <<
>  		 I40E_TXD_CTX_QW1_MSS_SHIFT);
> -
>  	return ctx_desc;
>  }
> 

This patch contains changes on testpmd, driver and mbuf definition. It's better to split them and sent to the maintainers.

[PATCH v2] i40: fix the VXLAN TSO issue

[Zhe Tao]

Problem:
When using the TSO + VXLAN feature in i40e, the outer UDP length fields in
the multiple UDP segments which are TSOed by the i40e will have the
wrong value.

Fix this problem by adding the tunnel type field in the i40e descriptor
which was missed before.

Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")

Signed-off-by: Zhe Tao <zhe.tao@intel.com>
---
V2: Edited some comments for mbuf structure and i40e driver.

 app/test-pmd/csumonly.c      | 26 +++++++++++++++++++-------
 drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
 lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
 3 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index ac4bd8f..d423c20 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -204,7 +204,8 @@ parse_ethernet(struct ether_hdr *eth_hdr, struct testpmd_offload_info *info)
 static void
 parse_vxlan(struct udp_hdr *udp_hdr,
 	    struct testpmd_offload_info *info,
-	    uint32_t pkt_type)
+	    uint32_t pkt_type,
+	    uint64_t *ol_flags)
 {
 	struct ether_hdr *eth_hdr;
 
@@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
 		RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
 		return;
 
+	*ol_flags |= PKT_TX_TUNNEL_VXLAN;
 	info->is_tunnel = 1;
 	info->outer_ethertype = info->ethertype;
 	info->outer_l2_len = info->l2_len;
@@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
 
 /* Parse a gre header */
 static void
-parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
+parse_gre(struct simple_gre_hdr *gre_hdr,
+	  struct testpmd_offload_info *info,
+	  uint64_t *ol_flags)
 {
 	struct ether_hdr *eth_hdr;
 	struct ipv4_hdr *ipv4_hdr;
@@ -242,6 +246,8 @@ parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
 	if ((gre_hdr->flags & _htons(~GRE_SUPPORTED_FIELDS)) != 0)
 		return;
 
+	*ol_flags |= PKT_TX_TUNNEL_GRE;
+
 	gre_len += sizeof(struct simple_gre_hdr);
 
 	if (gre_hdr->flags & _htons(GRE_KEY_PRESENT))
@@ -417,7 +423,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
  * packet */
 static uint64_t
 process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
-	uint16_t testpmd_ol_flags)
+	uint16_t testpmd_ol_flags, uint64_t orig_ol_flags)
 {
 	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
 	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr;
@@ -442,6 +448,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	 * hardware supporting it today, and no API for it. */
 
 	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
+	if ((orig_ol_flags & PKT_TX_TCP_SEG) &&
+	    ((orig_ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN))
+		udp_hdr->dgram_cksum = 0;
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
@@ -705,15 +714,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 			if (info.l4_proto == IPPROTO_UDP) {
 				struct udp_hdr *udp_hdr;
 				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
-					info.l3_len);
-				parse_vxlan(udp_hdr, &info, m->packet_type);
+					   info.l3_len);
+				parse_vxlan(udp_hdr, &info, m->packet_type,
+					    &ol_flags);
 			} else if (info.l4_proto == IPPROTO_GRE) {
 				struct simple_gre_hdr *gre_hdr;
 				gre_hdr = (struct simple_gre_hdr *)
 					((char *)l3_hdr + info.l3_len);
-				parse_gre(gre_hdr, &info);
+				parse_gre(gre_hdr, &info, &ol_flags);
 			} else if (info.l4_proto == IPPROTO_IPIP) {
 				void *encap_ip_hdr;
+
+				ol_flags |= PKT_TX_TUNNEL_IPIP;
 				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
 				parse_encap_ip(encap_ip_hdr, &info);
 			}
@@ -745,7 +757,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		 * processed in hardware. */
 		if (info.is_tunnel == 1) {
 			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
-				testpmd_ol_flags);
+				testpmd_ol_flags, ol_flags);
 		}
 
 		/* step 4: fill the mbuf meta data (flags and header lengths) */
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 049a813..4c987f2 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
 			union i40e_tx_offload tx_offload,
 			uint32_t *cd_tunneling)
 {
+	/* Tx pkts tunnel type*/
+	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
+		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
+	else if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE)
+		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
+
 	/* UDP tunneling packet TX checksum offload */
 	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
 
@@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
 
 /* set i40e TSO context descriptor */
 static inline uint64_t
-i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
+i40e_set_tso_ctx(struct rte_mbuf *mbuf,
+		 union i40e_tx_offload tx_offload)
 {
 	uint64_t ctx_desc = 0;
 	uint32_t cd_cmd, hdr_len, cd_tso_len;
@@ -1521,7 +1528,7 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
 	}
 
 	/**
-	 * in case of tunneling packet, the outer_l2_len and
+	 * in case of non tunneling packet, the outer_l2_len and
 	 * outer_l3_len must be 0.
 	 */
 	hdr_len = tx_offload.outer_l2_len +
@@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
 		 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
 		((uint64_t)mbuf->tso_segsz <<
 		 I40E_TXD_CTX_QW1_MSS_SHIFT);
-
 	return ctx_desc;
 }
 
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 15e3a10..8eb0d33 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -133,6 +133,17 @@ extern "C" {
 /* add new TX flags here */
 
 /**
+ * Bits 45:48 used for the tunnel type.
+ * When doing Tx offload like TSO or checksum, the HW needs to configure the
+ * tunnel type into the HW descriptors.
+ */
+#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
+#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
+#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
+/* add new TX TUNNEL type here */
+#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
+
+/**
  * Second VLAN insertion (QinQ) flag.
  */
 #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
@@ -867,7 +878,10 @@ struct rte_mbuf {
 	union {
 		uint64_t tx_offload;       /**< combined for easy fetch */
 		struct {
-			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
+			/* L2 (MAC) Header Length if it is not a tunneling pkt.
+			 * for tunnel it is outer L4 len+tunnel len+inner L2 len
+			 */
+			uint64_t l2_len:7;
 			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
 			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
 			uint64_t tso_segsz:16; /**< TCP TSO segment size */
-- 
2.1.4

Re: [PATCH v2] i40: fix the VXLAN TSO issue

[Ananyev, Konstantin]

Hi Tao,

> 
> Problem:
> When using the TSO + VXLAN feature in i40e, the outer UDP length fields in
> the multiple UDP segments which are TSOed by the i40e will have the
> wrong value.
> 
> Fix this problem by adding the tunnel type field in the i40e descriptor
> which was missed before.
> 
> Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")
> 
> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> ---
> V2: Edited some comments for mbuf structure and i40e driver.
> 
>  app/test-pmd/csumonly.c      | 26 +++++++++++++++++++-------
>  drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
>  lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
>  3 files changed, 43 insertions(+), 11 deletions(-)
> 
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> index ac4bd8f..d423c20 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -204,7 +204,8 @@ parse_ethernet(struct ether_hdr *eth_hdr, struct testpmd_offload_info *info)
>  static void
>  parse_vxlan(struct udp_hdr *udp_hdr,
>  	    struct testpmd_offload_info *info,
> -	    uint32_t pkt_type)
> +	    uint32_t pkt_type,
> +	    uint64_t *ol_flags)
>  {
>  	struct ether_hdr *eth_hdr;
> 
> @@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
>  		RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
>  		return;
> 
> +	*ol_flags |= PKT_TX_TUNNEL_VXLAN;
>  	info->is_tunnel = 1;
>  	info->outer_ethertype = info->ethertype;
>  	info->outer_l2_len = info->l2_len;
> @@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> 
>  /* Parse a gre header */
>  static void
> -parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
> +parse_gre(struct simple_gre_hdr *gre_hdr,
> +	  struct testpmd_offload_info *info,
> +	  uint64_t *ol_flags)
>  {
>  	struct ether_hdr *eth_hdr;
>  	struct ipv4_hdr *ipv4_hdr;
> @@ -242,6 +246,8 @@ parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
>  	if ((gre_hdr->flags & _htons(~GRE_SUPPORTED_FIELDS)) != 0)
>  		return;
> 
> +	*ol_flags |= PKT_TX_TUNNEL_GRE;
> +
>  	gre_len += sizeof(struct simple_gre_hdr);
> 
>  	if (gre_hdr->flags & _htons(GRE_KEY_PRESENT))
> @@ -417,7 +423,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
>   * packet */
>  static uint64_t
>  process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
> -	uint16_t testpmd_ol_flags)
> +	uint16_t testpmd_ol_flags, uint64_t orig_ol_flags)
>  {
>  	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
>  	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr;
> @@ -442,6 +448,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
>  	 * hardware supporting it today, and no API for it. */
> 
>  	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
> +	if ((orig_ol_flags & PKT_TX_TCP_SEG) &&
> +	    ((orig_ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN))
> +		udp_hdr->dgram_cksum = 0;
>  	/* do not recalculate udp cksum if it was 0 */
>  	if (udp_hdr->dgram_cksum != 0) {
>  		udp_hdr->dgram_cksum = 0;
> @@ -705,15 +714,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>  			if (info.l4_proto == IPPROTO_UDP) {
>  				struct udp_hdr *udp_hdr;
>  				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
> -					info.l3_len);
> -				parse_vxlan(udp_hdr, &info, m->packet_type);
> +					   info.l3_len);
> +				parse_vxlan(udp_hdr, &info, m->packet_type,
> +					    &ol_flags);
>  			} else if (info.l4_proto == IPPROTO_GRE) {
>  				struct simple_gre_hdr *gre_hdr;
>  				gre_hdr = (struct simple_gre_hdr *)
>  					((char *)l3_hdr + info.l3_len);
> -				parse_gre(gre_hdr, &info);
> +				parse_gre(gre_hdr, &info, &ol_flags);
>  			} else if (info.l4_proto == IPPROTO_IPIP) {
>  				void *encap_ip_hdr;
> +
> +				ol_flags |= PKT_TX_TUNNEL_IPIP;
>  				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
>  				parse_encap_ip(encap_ip_hdr, &info);
>  			}
> @@ -745,7 +757,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>  		 * processed in hardware. */
>  		if (info.is_tunnel == 1) {
>  			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
> -				testpmd_ol_flags);
> +				testpmd_ol_flags, ol_flags);
>  		}
> 
>  		/* step 4: fill the mbuf meta data (flags and header lengths) */
> diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> index 049a813..4c987f2 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
>  			union i40e_tx_offload tx_offload,
>  			uint32_t *cd_tunneling)
>  {
> +	/* Tx pkts tunnel type*/
> +	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
> +		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
> +	else if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE)
> +		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
> +
>  	/* UDP tunneling packet TX checksum offload */
>  	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
> 
> @@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
> 
>  /* set i40e TSO context descriptor */
>  static inline uint64_t
> -i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> +i40e_set_tso_ctx(struct rte_mbuf *mbuf,
> +		 union i40e_tx_offload tx_offload)
>  {
>  	uint64_t ctx_desc = 0;
>  	uint32_t cd_cmd, hdr_len, cd_tso_len;
> @@ -1521,7 +1528,7 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
>  	}
> 
>  	/**
> -	 * in case of tunneling packet, the outer_l2_len and
> +	 * in case of non tunneling packet, the outer_l2_len and
>  	 * outer_l3_len must be 0.
>  	 */
>  	hdr_len = tx_offload.outer_l2_len +
> @@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
>  		 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
>  		((uint64_t)mbuf->tso_segsz <<
>  		 I40E_TXD_CTX_QW1_MSS_SHIFT);
> -
>  	return ctx_desc;
>  }
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 15e3a10..8eb0d33 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -133,6 +133,17 @@ extern "C" {
>  /* add new TX flags here */
> 
>  /**
> + * Bits 45:48 used for the tunnel type.
> + * When doing Tx offload like TSO or checksum, the HW needs to configure the
> + * tunnel type into the HW descriptors.
> + */
> +#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
> +#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
> +#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
> +/* add new TX TUNNEL type here */
> +#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
> +
> +/**
>   * Second VLAN insertion (QinQ) flag.
>   */
>  #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
> @@ -867,7 +878,10 @@ struct rte_mbuf {
>  	union {
>  		uint64_t tx_offload;       /**< combined for easy fetch */
>  		struct {
> -			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
> +			/* L2 (MAC) Header Length if it is not a tunneling pkt.
> +			 * for tunnel it is outer L4 len+tunnel len+inner L2 len
> +			 */
> +			uint64_t l2_len:7;
>  			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
>  			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
>  			uint64_t tso_segsz:16; /**< TCP TSO segment size */
> --
> 2.1.4

Re: [PATCH v2] i40: fix the VXLAN TSO issue

[Ananyev, Konstantin]

 
Hi Tao,

Sorry hit send button too early by accident :)
 
> >
> > Problem:
> > When using the TSO + VXLAN feature in i40e, the outer UDP length fields in
> > the multiple UDP segments which are TSOed by the i40e will have the
> > wrong value.
> >
> > Fix this problem by adding the tunnel type field in the i40e descriptor
> > which was missed before.
> >
> > Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")
> >
> > Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> > ---
> > V2: Edited some comments for mbuf structure and i40e driver.
> >
> >  app/test-pmd/csumonly.c      | 26 +++++++++++++++++++-------
> >  drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
> >  lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
> >  3 files changed, 43 insertions(+), 11 deletions(-)
> >
> > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> > index ac4bd8f..d423c20 100644
> > --- a/app/test-pmd/csumonly.c
> > +++ b/app/test-pmd/csumonly.c
> > @@ -204,7 +204,8 @@ parse_ethernet(struct ether_hdr *eth_hdr, struct testpmd_offload_info *info)
> >  static void
> >  parse_vxlan(struct udp_hdr *udp_hdr,
> >  	    struct testpmd_offload_info *info,
> > -	    uint32_t pkt_type)
> > +	    uint32_t pkt_type,
> > +	    uint64_t *ol_flags)
> >  {
> >  	struct ether_hdr *eth_hdr;
> >
> > @@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> >  		RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
> >  		return;
> >
> > +	*ol_flags |= PKT_TX_TUNNEL_VXLAN;
> >  	info->is_tunnel = 1;
> >  	info->outer_ethertype = info->ethertype;
> >  	info->outer_l2_len = info->l2_len;
> > @@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> >
> >  /* Parse a gre header */
> >  static void
> > -parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
> > +parse_gre(struct simple_gre_hdr *gre_hdr,
> > +	  struct testpmd_offload_info *info,
> > +	  uint64_t *ol_flags)
> >  {
> >  	struct ether_hdr *eth_hdr;
> >  	struct ipv4_hdr *ipv4_hdr;
> > @@ -242,6 +246,8 @@ parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
> >  	if ((gre_hdr->flags & _htons(~GRE_SUPPORTED_FIELDS)) != 0)
> >  		return;
> >
> > +	*ol_flags |= PKT_TX_TUNNEL_GRE;
> > +
> >  	gre_len += sizeof(struct simple_gre_hdr);
> >
> >  	if (gre_hdr->flags & _htons(GRE_KEY_PRESENT))
> > @@ -417,7 +423,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
> >   * packet */
> >  static uint64_t
> >  process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
> > -	uint16_t testpmd_ol_flags)
> > +	uint16_t testpmd_ol_flags, uint64_t orig_ol_flags)
> >  {
> >  	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
> >  	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr;
> > @@ -442,6 +448,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
> >  	 * hardware supporting it today, and no API for it. */
> >
> >  	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
> > +	if ((orig_ol_flags & PKT_TX_TCP_SEG) &&
> > +	    ((orig_ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN))
> > +		udp_hdr->dgram_cksum = 0;
> >  	/* do not recalculate udp cksum if it was 0 */
> >  	if (udp_hdr->dgram_cksum != 0) {
> >  		udp_hdr->dgram_cksum = 0;
> > @@ -705,15 +714,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> >  			if (info.l4_proto == IPPROTO_UDP) {
> >  				struct udp_hdr *udp_hdr;
> >  				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
> > -					info.l3_len);
> > -				parse_vxlan(udp_hdr, &info, m->packet_type);
> > +					   info.l3_len);
> > +				parse_vxlan(udp_hdr, &info, m->packet_type,
> > +					    &ol_flags);
> >  			} else if (info.l4_proto == IPPROTO_GRE) {
> >  				struct simple_gre_hdr *gre_hdr;
> >  				gre_hdr = (struct simple_gre_hdr *)
> >  					((char *)l3_hdr + info.l3_len);
> > -				parse_gre(gre_hdr, &info);
> > +				parse_gre(gre_hdr, &info, &ol_flags);
> >  			} else if (info.l4_proto == IPPROTO_IPIP) {
> >  				void *encap_ip_hdr;
> > +
> > +				ol_flags |= PKT_TX_TUNNEL_IPIP;
> >  				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
> >  				parse_encap_ip(encap_ip_hdr, &info);
> >  			}
> > @@ -745,7 +757,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> >  		 * processed in hardware. */
> >  		if (info.is_tunnel == 1) {
> >  			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
> > -				testpmd_ol_flags);
> > +				testpmd_ol_flags, ol_flags);
> >  		}
> >
> >  		/* step 4: fill the mbuf meta data (flags and header lengths) */
> > diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> > index 049a813..4c987f2 100644
> > --- a/drivers/net/i40e/i40e_rxtx.c
> > +++ b/drivers/net/i40e/i40e_rxtx.c
> > @@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
> >  			union i40e_tx_offload tx_offload,
> >  			uint32_t *cd_tunneling)
> >  {
> > +	/* Tx pkts tunnel type*/
> > +	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
> > +		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
> > +	else if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE)
> > +		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
> > +

As I understand that fix is needed to enable TSO for tunnelling packets, correct?
For that case, should we setup EIPLEN also, no matter is PKT_TX_OUTER_IP_CKSUM
is on/off?

> >  	/* UDP tunneling packet TX checksum offload */
> >  	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
> >
> > @@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
> >
> >  /* set i40e TSO context descriptor */
> >  static inline uint64_t
> > -i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> > +i40e_set_tso_ctx(struct rte_mbuf *mbuf,
> > +		 union i40e_tx_offload tx_offload)
> >  {
> >  	uint64_t ctx_desc = 0;
> >  	uint32_t cd_cmd, hdr_len, cd_tso_len;
> > @@ -1521,7 +1528,7 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> >  	}
> >
> >  	/**
> > -	 * in case of tunneling packet, the outer_l2_len and
> > +	 * in case of non tunneling packet, the outer_l2_len and
> >  	 * outer_l3_len must be 0.
> >  	 */
> >  	hdr_len = tx_offload.outer_l2_len +
> > @@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> >  		 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
> >  		((uint64_t)mbuf->tso_segsz <<
> >  		 I40E_TXD_CTX_QW1_MSS_SHIFT);
> > -
> >  	return ctx_desc;
> >  }
> >
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index 15e3a10..8eb0d33 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -133,6 +133,17 @@ extern "C" {
> >  /* add new TX flags here */
> >
> >  /**
> > + * Bits 45:48 used for the tunnel type.
> > + * When doing Tx offload like TSO or checksum, the HW needs to configure the
> > + * tunnel type into the HW descriptors.
> > + */
> > +#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
> > +#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
> > +#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
> > +/* add new TX TUNNEL type here */
> > +#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
> > +
> > +/**
> >   * Second VLAN insertion (QinQ) flag.
> >   */
> >  #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
> > @@ -867,7 +878,10 @@ struct rte_mbuf {
> >  	union {
> >  		uint64_t tx_offload;       /**< combined for easy fetch */
> >  		struct {
> > -			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
> > +			/* L2 (MAC) Header Length if it is not a tunneling pkt.
> > +			 * for tunnel it is outer L4 len+tunnel len+inner L2 len
> > +			 */

As a nit: that doesn't look like doxygen style comment to me.
Konstantin

> > +			uint64_t l2_len:7;
> >  			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
> >  			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
> >  			uint64_t tso_segsz:16; /**< TCP TSO segment size */
> > --
> > 2.1.4

Re: [PATCH v2] i40: fix the VXLAN TSO issue

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin
> Sent: Thursday, July 07, 2016 11:51 AM
> To: Tao, Zhe; dev@dpdk.org
> Cc: Tao, Zhe; Wu, Jingjing
> Subject: Re: [dpdk-dev] [PATCH v2] i40: fix the VXLAN TSO issue
> 
> 
> Hi Tao,
> 
> Sorry hit send button too early by accident :)
> 
> > >
> > > Problem:
> > > When using the TSO + VXLAN feature in i40e, the outer UDP length fields in
> > > the multiple UDP segments which are TSOed by the i40e will have the
> > > wrong value.
> > >
> > > Fix this problem by adding the tunnel type field in the i40e descriptor
> > > which was missed before.
> > >
> > > Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")
> > >
> > > Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> > > ---
> > > V2: Edited some comments for mbuf structure and i40e driver.
> > >
> > >  app/test-pmd/csumonly.c      | 26 +++++++++++++++++++-------
> > >  drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
> > >  lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
> > >  3 files changed, 43 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> > > index ac4bd8f..d423c20 100644
> > > --- a/app/test-pmd/csumonly.c
> > > +++ b/app/test-pmd/csumonly.c
> > > @@ -204,7 +204,8 @@ parse_ethernet(struct ether_hdr *eth_hdr, struct testpmd_offload_info *info)
> > >  static void
> > >  parse_vxlan(struct udp_hdr *udp_hdr,
> > >  	    struct testpmd_offload_info *info,
> > > -	    uint32_t pkt_type)
> > > +	    uint32_t pkt_type,
> > > +	    uint64_t *ol_flags)
> > >  {
> > >  	struct ether_hdr *eth_hdr;
> > >
> > > @@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> > >  		RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
> > >  		return;
> > >
> > > +	*ol_flags |= PKT_TX_TUNNEL_VXLAN;

Do we always have to setup tunnelling flags here?
Obviously it would mean an extra CTD per packet and might slowdown things.
In fact, I think current patch wouldn't work correctly if
TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM is not set.
So, can we do it only when TSO is enabled or outer IP checksum is enabled?

> > >  	info->is_tunnel = 1;
> > >  	info->outer_ethertype = info->ethertype;
> > >  	info->outer_l2_len = info->l2_len;
> > > @@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> > >
> > >  /* Parse a gre header */
> > >  static void
> > > -parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
> > > +parse_gre(struct simple_gre_hdr *gre_hdr,
> > > +	  struct testpmd_offload_info *info,
> > > +	  uint64_t *ol_flags)
> > >  {
> > >  	struct ether_hdr *eth_hdr;
> > >  	struct ipv4_hdr *ipv4_hdr;
> > > @@ -242,6 +246,8 @@ parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
> > >  	if ((gre_hdr->flags & _htons(~GRE_SUPPORTED_FIELDS)) != 0)
> > >  		return;
> > >
> > > +	*ol_flags |= PKT_TX_TUNNEL_GRE;
> > > +
> > >  	gre_len += sizeof(struct simple_gre_hdr);
> > >
> > >  	if (gre_hdr->flags & _htons(GRE_KEY_PRESENT))
> > > @@ -417,7 +423,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
> > >   * packet */
> > >  static uint64_t
> > >  process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
> > > -	uint16_t testpmd_ol_flags)
> > > +	uint16_t testpmd_ol_flags, uint64_t orig_ol_flags)
> > >  {
> > >  	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
> > >  	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr;
> > > @@ -442,6 +448,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
> > >  	 * hardware supporting it today, and no API for it. */
> > >
> > >  	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
> > > +	if ((orig_ol_flags & PKT_TX_TCP_SEG) &&
> > > +	    ((orig_ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN))
> > > +		udp_hdr->dgram_cksum = 0;
> > >  	/* do not recalculate udp cksum if it was 0 */
> > >  	if (udp_hdr->dgram_cksum != 0) {
> > >  		udp_hdr->dgram_cksum = 0;
> > > @@ -705,15 +714,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> > >  			if (info.l4_proto == IPPROTO_UDP) {
> > >  				struct udp_hdr *udp_hdr;
> > >  				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
> > > -					info.l3_len);
> > > -				parse_vxlan(udp_hdr, &info, m->packet_type);
> > > +					   info.l3_len);
> > > +				parse_vxlan(udp_hdr, &info, m->packet_type,
> > > +					    &ol_flags);
> > >  			} else if (info.l4_proto == IPPROTO_GRE) {
> > >  				struct simple_gre_hdr *gre_hdr;
> > >  				gre_hdr = (struct simple_gre_hdr *)
> > >  					((char *)l3_hdr + info.l3_len);
> > > -				parse_gre(gre_hdr, &info);
> > > +				parse_gre(gre_hdr, &info, &ol_flags);
> > >  			} else if (info.l4_proto == IPPROTO_IPIP) {
> > >  				void *encap_ip_hdr;
> > > +
> > > +				ol_flags |= PKT_TX_TUNNEL_IPIP;
> > >  				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
> > >  				parse_encap_ip(encap_ip_hdr, &info);
> > >  			}
> > > @@ -745,7 +757,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> > >  		 * processed in hardware. */
> > >  		if (info.is_tunnel == 1) {
> > >  			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
> > > -				testpmd_ol_flags);
> > > +				testpmd_ol_flags, ol_flags);
> > >  		}
> > >
> > >  		/* step 4: fill the mbuf meta data (flags and header lengths) */
> > > diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> > > index 049a813..4c987f2 100644
> > > --- a/drivers/net/i40e/i40e_rxtx.c
> > > +++ b/drivers/net/i40e/i40e_rxtx.c
> > > @@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
> > >  			union i40e_tx_offload tx_offload,
> > >  			uint32_t *cd_tunneling)
> > >  {
> > > +	/* Tx pkts tunnel type*/
> > > +	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
> > > +		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
> > > +	else if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE)
> > > +		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
> > > +
> 
> As I understand that fix is needed to enable TSO for tunnelling packets, correct?
> For that case, should we setup EIPLEN also, no matter is PKT_TX_OUTER_IP_CKSUM
> is on/off?

In fact, it seems we have always to setup all 3: EIPLEN, MACLEN and L4TUNLEN,
when L4TUNT != 0.

> 
> > >  	/* UDP tunneling packet TX checksum offload */
> > >  	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
> > >
> > > @@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
> > >
> > >  /* set i40e TSO context descriptor */
> > >  static inline uint64_t
> > > -i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> > > +i40e_set_tso_ctx(struct rte_mbuf *mbuf,
> > > +		 union i40e_tx_offload tx_offload)
> > >  {
> > >  	uint64_t ctx_desc = 0;
> > >  	uint32_t cd_cmd, hdr_len, cd_tso_len;
> > > @@ -1521,7 +1528,7 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> > >  	}
> > >
> > >  	/**
> > > -	 * in case of tunneling packet, the outer_l2_len and
> > > +	 * in case of non tunneling packet, the outer_l2_len and
> > >  	 * outer_l3_len must be 0.
> > >  	 */
> > >  	hdr_len = tx_offload.outer_l2_len +
> > > @@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> > >  		 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
> > >  		((uint64_t)mbuf->tso_segsz <<
> > >  		 I40E_TXD_CTX_QW1_MSS_SHIFT);
> > > -
> > >  	return ctx_desc;
> > >  }
> > >
> > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > > index 15e3a10..8eb0d33 100644
> > > --- a/lib/librte_mbuf/rte_mbuf.h
> > > +++ b/lib/librte_mbuf/rte_mbuf.h
> > > @@ -133,6 +133,17 @@ extern "C" {
> > >  /* add new TX flags here */
> > >
> > >  /**
> > > + * Bits 45:48 used for the tunnel type.
> > > + * When doing Tx offload like TSO or checksum, the HW needs to configure the
> > > + * tunnel type into the HW descriptors.
> > > + */
> > > +#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
> > > +#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
> > > +#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
> > > +/* add new TX TUNNEL type here */
> > > +#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
> > > +
> > > +/**
> > >   * Second VLAN insertion (QinQ) flag.
> > >   */
> > >  #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
> > > @@ -867,7 +878,10 @@ struct rte_mbuf {
> > >  	union {
> > >  		uint64_t tx_offload;       /**< combined for easy fetch */
> > >  		struct {
> > > -			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
> > > +			/* L2 (MAC) Header Length if it is not a tunneling pkt.
> > > +			 * for tunnel it is outer L4 len+tunnel len+inner L2 len
> > > +			 */
> 
> As a nit: that doesn't look like doxygen style comment to me.
> Konstantin
> 
> > > +			uint64_t l2_len:7;
> > >  			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
> > >  			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
> > >  			uint64_t tso_segsz:16; /**< TCP TSO segment size */
> > > --
> > > 2.1.4

Re: [PATCH v2] i40: fix the VXLAN TSO issue

[Bruce Richardson]

Since we are now heading to RC3 for 16.07 and there are quite a number of
open comments on this patch unresolved, it's going to be deferred till
16.11, when it can have more review and discussion.

/Bruce

On Thu, Jul 07, 2016 at 12:24:43PM +0000, Ananyev, Konstantin wrote:
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin
> > Sent: Thursday, July 07, 2016 11:51 AM
> > To: Tao, Zhe; dev@dpdk.org
> > Cc: Tao, Zhe; Wu, Jingjing
> > Subject: Re: [dpdk-dev] [PATCH v2] i40: fix the VXLAN TSO issue
> > 
> > 
> > Hi Tao,
> > 
> > Sorry hit send button too early by accident :)
> > 
> > > >
> > > > Problem:
> > > > When using the TSO + VXLAN feature in i40e, the outer UDP length fields in
> > > > the multiple UDP segments which are TSOed by the i40e will have the
> > > > wrong value.
> > > >
> > > > Fix this problem by adding the tunnel type field in the i40e descriptor
> > > > which was missed before.
> > > >
> > > > Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")
> > > >
> > > > Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> > > > ---
> > > > V2: Edited some comments for mbuf structure and i40e driver.
> > > >
> > > >  app/test-pmd/csumonly.c      | 26 +++++++++++++++++++-------
> > > >  drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
> > > >  lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
> > > >  3 files changed, 43 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
> > > > index ac4bd8f..d423c20 100644
> > > > --- a/app/test-pmd/csumonly.c
> > > > +++ b/app/test-pmd/csumonly.c
> > > > @@ -204,7 +204,8 @@ parse_ethernet(struct ether_hdr *eth_hdr, struct testpmd_offload_info *info)
> > > >  static void
> > > >  parse_vxlan(struct udp_hdr *udp_hdr,
> > > >  	    struct testpmd_offload_info *info,
> > > > -	    uint32_t pkt_type)
> > > > +	    uint32_t pkt_type,
> > > > +	    uint64_t *ol_flags)
> > > >  {
> > > >  	struct ether_hdr *eth_hdr;
> > > >
> > > > @@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> > > >  		RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
> > > >  		return;
> > > >
> > > > +	*ol_flags |= PKT_TX_TUNNEL_VXLAN;
> 
> Do we always have to setup tunnelling flags here?
> Obviously it would mean an extra CTD per packet and might slowdown things.
> In fact, I think current patch wouldn't work correctly if
> TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM is not set.
> So, can we do it only when TSO is enabled or outer IP checksum is enabled?
> 
> > > >  	info->is_tunnel = 1;
> > > >  	info->outer_ethertype = info->ethertype;
> > > >  	info->outer_l2_len = info->l2_len;
> > > > @@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> > > >
> > > >  /* Parse a gre header */
> > > >  static void
> > > > -parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
> > > > +parse_gre(struct simple_gre_hdr *gre_hdr,
> > > > +	  struct testpmd_offload_info *info,
> > > > +	  uint64_t *ol_flags)
> > > >  {
> > > >  	struct ether_hdr *eth_hdr;
> > > >  	struct ipv4_hdr *ipv4_hdr;
> > > > @@ -242,6 +246,8 @@ parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
> > > >  	if ((gre_hdr->flags & _htons(~GRE_SUPPORTED_FIELDS)) != 0)
> > > >  		return;
> > > >
> > > > +	*ol_flags |= PKT_TX_TUNNEL_GRE;
> > > > +
> > > >  	gre_len += sizeof(struct simple_gre_hdr);
> > > >
> > > >  	if (gre_hdr->flags & _htons(GRE_KEY_PRESENT))
> > > > @@ -417,7 +423,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
> > > >   * packet */
> > > >  static uint64_t
> > > >  process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
> > > > -	uint16_t testpmd_ol_flags)
> > > > +	uint16_t testpmd_ol_flags, uint64_t orig_ol_flags)
> > > >  {
> > > >  	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
> > > >  	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr;
> > > > @@ -442,6 +448,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
> > > >  	 * hardware supporting it today, and no API for it. */
> > > >
> > > >  	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
> > > > +	if ((orig_ol_flags & PKT_TX_TCP_SEG) &&
> > > > +	    ((orig_ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN))
> > > > +		udp_hdr->dgram_cksum = 0;
> > > >  	/* do not recalculate udp cksum if it was 0 */
> > > >  	if (udp_hdr->dgram_cksum != 0) {
> > > >  		udp_hdr->dgram_cksum = 0;
> > > > @@ -705,15 +714,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> > > >  			if (info.l4_proto == IPPROTO_UDP) {
> > > >  				struct udp_hdr *udp_hdr;
> > > >  				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
> > > > -					info.l3_len);
> > > > -				parse_vxlan(udp_hdr, &info, m->packet_type);
> > > > +					   info.l3_len);
> > > > +				parse_vxlan(udp_hdr, &info, m->packet_type,
> > > > +					    &ol_flags);
> > > >  			} else if (info.l4_proto == IPPROTO_GRE) {
> > > >  				struct simple_gre_hdr *gre_hdr;
> > > >  				gre_hdr = (struct simple_gre_hdr *)
> > > >  					((char *)l3_hdr + info.l3_len);
> > > > -				parse_gre(gre_hdr, &info);
> > > > +				parse_gre(gre_hdr, &info, &ol_flags);
> > > >  			} else if (info.l4_proto == IPPROTO_IPIP) {
> > > >  				void *encap_ip_hdr;
> > > > +
> > > > +				ol_flags |= PKT_TX_TUNNEL_IPIP;
> > > >  				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
> > > >  				parse_encap_ip(encap_ip_hdr, &info);
> > > >  			}
> > > > @@ -745,7 +757,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> > > >  		 * processed in hardware. */
> > > >  		if (info.is_tunnel == 1) {
> > > >  			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
> > > > -				testpmd_ol_flags);
> > > > +				testpmd_ol_flags, ol_flags);
> > > >  		}
> > > >
> > > >  		/* step 4: fill the mbuf meta data (flags and header lengths) */
> > > > diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> > > > index 049a813..4c987f2 100644
> > > > --- a/drivers/net/i40e/i40e_rxtx.c
> > > > +++ b/drivers/net/i40e/i40e_rxtx.c
> > > > @@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
> > > >  			union i40e_tx_offload tx_offload,
> > > >  			uint32_t *cd_tunneling)
> > > >  {
> > > > +	/* Tx pkts tunnel type*/
> > > > +	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
> > > > +		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
> > > > +	else if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE)
> > > > +		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
> > > > +
> > 
> > As I understand that fix is needed to enable TSO for tunnelling packets, correct?
> > For that case, should we setup EIPLEN also, no matter is PKT_TX_OUTER_IP_CKSUM
> > is on/off?
> 
> In fact, it seems we have always to setup all 3: EIPLEN, MACLEN and L4TUNLEN,
> when L4TUNT != 0.
> 
> > 
> > > >  	/* UDP tunneling packet TX checksum offload */
> > > >  	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
> > > >
> > > > @@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
> > > >
> > > >  /* set i40e TSO context descriptor */
> > > >  static inline uint64_t
> > > > -i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> > > > +i40e_set_tso_ctx(struct rte_mbuf *mbuf,
> > > > +		 union i40e_tx_offload tx_offload)
> > > >  {
> > > >  	uint64_t ctx_desc = 0;
> > > >  	uint32_t cd_cmd, hdr_len, cd_tso_len;
> > > > @@ -1521,7 +1528,7 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> > > >  	}
> > > >
> > > >  	/**
> > > > -	 * in case of tunneling packet, the outer_l2_len and
> > > > +	 * in case of non tunneling packet, the outer_l2_len and
> > > >  	 * outer_l3_len must be 0.
> > > >  	 */
> > > >  	hdr_len = tx_offload.outer_l2_len +
> > > > @@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> > > >  		 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
> > > >  		((uint64_t)mbuf->tso_segsz <<
> > > >  		 I40E_TXD_CTX_QW1_MSS_SHIFT);
> > > > -
> > > >  	return ctx_desc;
> > > >  }
> > > >
> > > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > > > index 15e3a10..8eb0d33 100644
> > > > --- a/lib/librte_mbuf/rte_mbuf.h
> > > > +++ b/lib/librte_mbuf/rte_mbuf.h
> > > > @@ -133,6 +133,17 @@ extern "C" {
> > > >  /* add new TX flags here */
> > > >
> > > >  /**
> > > > + * Bits 45:48 used for the tunnel type.
> > > > + * When doing Tx offload like TSO or checksum, the HW needs to configure the
> > > > + * tunnel type into the HW descriptors.
> > > > + */
> > > > +#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
> > > > +#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
> > > > +#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
> > > > +/* add new TX TUNNEL type here */
> > > > +#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
> > > > +
> > > > +/**
> > > >   * Second VLAN insertion (QinQ) flag.
> > > >   */
> > > >  #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
> > > > @@ -867,7 +878,10 @@ struct rte_mbuf {
> > > >  	union {
> > > >  		uint64_t tx_offload;       /**< combined for easy fetch */
> > > >  		struct {
> > > > -			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
> > > > +			/* L2 (MAC) Header Length if it is not a tunneling pkt.
> > > > +			 * for tunnel it is outer L4 len+tunnel len+inner L2 len
> > > > +			 */
> > 
> > As a nit: that doesn't look like doxygen style comment to me.
> > Konstantin
> > 
> > > > +			uint64_t l2_len:7;
> > > >  			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
> > > >  			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
> > > >  			uint64_t tso_segsz:16; /**< TCP TSO segment size */
> > > > --
> > > > 2.1.4
> 

Re: [PATCH v2] i40: fix the VXLAN TSO issue

[Zhe Tao]

On Thu, Jul 07, 2016 at 08:24:43PM +0800, Ananyev, Konstantin wrote:
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, Konstantin
> > Sent: Thursday, July 07, 2016 11:51 AM
> > To: Tao, Zhe; dev@dpdk.org
> > Cc: Tao, Zhe; Wu, Jingjing
> > Subject: Re: [dpdk-dev] [PATCH v2] i40: fix the VXLAN TSO issue
> > 
> > 
> > > > @@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> > > >  		RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
> > > >  		return;
> > > >
> > > > +	*ol_flags |= PKT_TX_TUNNEL_VXLAN;
> 
> Do we always have to setup tunnelling flags here?
> Obviously it would mean an extra CTD per packet and might slowdown things.
this flag is for tunnelling type, and CTD is based on whether we need to do the
external ip offload and TSO.so this flag will not cause one extra CTD.
> In fact, I think current patch wouldn't work correctly if
> TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM is not set.
> So, can we do it only when TSO is enabled or outer IP checksum is enabled?
good comments and I aggree with you to let the APP to make sure all the flags driver needed
is set.
> 
> > > >  	info->is_tunnel = 1;
> > > >  	info->outer_ethertype = info->ethertype;
> > > >  	info->outer_l2_len = info->l2_len;
> > > > @@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> > > >
> > > > +				testpmd_ol_flags, ol_flags);
> > > >  		}
> > > >
> > > >  		/* step 4: fill the mbuf meta data (flags and header lengths) */
> > > > diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> > > > index 049a813..4c987f2 100644
> > > > --- a/drivers/net/i40e/i40e_rxtx.c
> > > > +++ b/drivers/net/i40e/i40e_rxtx.c
> > > > @@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
> > > >  			union i40e_tx_offload tx_offload,
> > > >  			uint32_t *cd_tunneling)
> > > >  {
> > > > +	/* Tx pkts tunnel type*/
> > > > +	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
> > > > +		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
> > > > +	else if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE)
> > > > +		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
> > > > +
> > 
> > As I understand that fix is needed to enable TSO for tunnelling packets, correct?
> > For that case, should we setup EIPLEN also, no matter is PKT_TX_OUTER_IP_CKSUM
> > is on/off?
> 
> In fact, it seems we have always to setup all 3: EIPLEN, MACLEN and L4TUNLEN,
> when L4TUNT != 0.
L4TUNT only means the tunnel type, but if we don't want to do the checksum,
these fields will not be set
> 
> > 
> > > >  	/* UDP tunneling packet TX checksum offload */
> > > >  	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
> > > >
> > > > @@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
thanks again for the commets,
I will resend the patch
Zhe 

[PATCH v3] i40: fix the VXLAN TSO issue

[Zhe Tao]

Problem:
When using the TSO + VXLAN feature in i40e, the outer UDP length fields in
the multiple UDP segments which are TSOed by the i40e will have a
wrong value.

Fix this problem by adding the tunnel type field in the i40e descriptor
which was missed before.

Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")

Signed-off-by: Zhe Tao <zhe.tao@intel.com>
---
v2: edited the comments
v3: added external IP offload flag when TSO is enabled for tunnelling packets

 app/test-pmd/csumonly.c      | 29 +++++++++++++++++++++--------
 drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
 lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
 3 files changed, 45 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index ac4bd8f..aaa006f 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -204,7 +204,8 @@ parse_ethernet(struct ether_hdr *eth_hdr, struct testpmd_offload_info *info)
 static void
 parse_vxlan(struct udp_hdr *udp_hdr,
 	    struct testpmd_offload_info *info,
-	    uint32_t pkt_type)
+	    uint32_t pkt_type,
+	    uint64_t *ol_flags)
 {
 	struct ether_hdr *eth_hdr;
 
@@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
 		RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
 		return;
 
+	*ol_flags |= PKT_TX_TUNNEL_VXLAN;
 	info->is_tunnel = 1;
 	info->outer_ethertype = info->ethertype;
 	info->outer_l2_len = info->l2_len;
@@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
 
 /* Parse a gre header */
 static void
-parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
+parse_gre(struct simple_gre_hdr *gre_hdr,
+	  struct testpmd_offload_info *info,
+	  uint64_t *ol_flags)
 {
 	struct ether_hdr *eth_hdr;
 	struct ipv4_hdr *ipv4_hdr;
@@ -242,6 +246,8 @@ parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
 	if ((gre_hdr->flags & _htons(~GRE_SUPPORTED_FIELDS)) != 0)
 		return;
 
+	*ol_flags |= PKT_TX_TUNNEL_GRE;
+
 	gre_len += sizeof(struct simple_gre_hdr);
 
 	if (gre_hdr->flags & _htons(GRE_KEY_PRESENT))
@@ -417,7 +423,7 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
  * packet */
 static uint64_t
 process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
-	uint16_t testpmd_ol_flags)
+	uint16_t testpmd_ol_flags, uint64_t orig_ol_flags)
 {
 	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
 	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr;
@@ -428,7 +434,8 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 		ipv4_hdr->hdr_checksum = 0;
 		ol_flags |= PKT_TX_OUTER_IPV4;
 
-		if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM)
+		if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) ||
+		    (info->tso_segsz != 0))
 			ol_flags |= PKT_TX_OUTER_IP_CKSUM;
 		else
 			ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
@@ -442,6 +449,9 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	 * hardware supporting it today, and no API for it. */
 
 	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
+	if ((orig_ol_flags & PKT_TX_TCP_SEG) &&
+	    ((orig_ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN))
+		udp_hdr->dgram_cksum = 0;
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
@@ -705,15 +715,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 			if (info.l4_proto == IPPROTO_UDP) {
 				struct udp_hdr *udp_hdr;
 				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
-					info.l3_len);
-				parse_vxlan(udp_hdr, &info, m->packet_type);
+					   info.l3_len);
+				parse_vxlan(udp_hdr, &info, m->packet_type,
+					    &ol_flags);
 			} else if (info.l4_proto == IPPROTO_GRE) {
 				struct simple_gre_hdr *gre_hdr;
 				gre_hdr = (struct simple_gre_hdr *)
 					((char *)l3_hdr + info.l3_len);
-				parse_gre(gre_hdr, &info);
+				parse_gre(gre_hdr, &info, &ol_flags);
 			} else if (info.l4_proto == IPPROTO_IPIP) {
 				void *encap_ip_hdr;
+
+				ol_flags |= PKT_TX_TUNNEL_IPIP;
 				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
 				parse_encap_ip(encap_ip_hdr, &info);
 			}
@@ -745,7 +758,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		 * processed in hardware. */
 		if (info.is_tunnel == 1) {
 			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
-				testpmd_ol_flags);
+				testpmd_ol_flags, ol_flags);
 		}
 
 		/* step 4: fill the mbuf meta data (flags and header lengths) */
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 049a813..4c987f2 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
 			union i40e_tx_offload tx_offload,
 			uint32_t *cd_tunneling)
 {
+	/* Tx pkts tunnel type*/
+	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
+		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
+	else if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE)
+		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
+
 	/* UDP tunneling packet TX checksum offload */
 	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
 
@@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
 
 /* set i40e TSO context descriptor */
 static inline uint64_t
-i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
+i40e_set_tso_ctx(struct rte_mbuf *mbuf,
+		 union i40e_tx_offload tx_offload)
 {
 	uint64_t ctx_desc = 0;
 	uint32_t cd_cmd, hdr_len, cd_tso_len;
@@ -1521,7 +1528,7 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
 	}
 
 	/**
-	 * in case of tunneling packet, the outer_l2_len and
+	 * in case of non tunneling packet, the outer_l2_len and
 	 * outer_l3_len must be 0.
 	 */
 	hdr_len = tx_offload.outer_l2_len +
@@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
 		 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
 		((uint64_t)mbuf->tso_segsz <<
 		 I40E_TXD_CTX_QW1_MSS_SHIFT);
-
 	return ctx_desc;
 }
 
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 15e3a10..90812ea 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -133,6 +133,17 @@ extern "C" {
 /* add new TX flags here */
 
 /**
+ * Bits 45:48 used for the tunnel type.
+ * When doing Tx offload like TSO or checksum, the HW needs to configure the
+ * tunnel type into the HW descriptors.
+ */
+#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
+#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
+#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
+/* add new TX TUNNEL type here */
+#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
+
+/**
  * Second VLAN insertion (QinQ) flag.
  */
 #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
@@ -867,7 +878,10 @@ struct rte_mbuf {
 	union {
 		uint64_t tx_offload;       /**< combined for easy fetch */
 		struct {
-			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
+			uint64_t l2_len:7;
+			/**< L2 (MAC) Header Length if it isn't a tunneling pkt.
+			 * for tunnel it is outer L4 len+tunnel len+inner L2 len
+			 */
 			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
 			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
 			uint64_t tso_segsz:16; /**< TCP TSO segment size */
-- 
2.1.4

Re: [PATCH v3] i40: fix the VXLAN TSO issue

[Ananyev, Konstantin]

> 
> Problem:
> When using the TSO + VXLAN feature in i40e, the outer UDP length fields in the multiple UDP segments which are TSOed by the i40e will
> have a wrong value.
> 
> Fix this problem by adding the tunnel type field in the i40e descriptor which was missed before.
> 
> Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")
> 
> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> ---
> v2: edited the comments
> v3: added external IP offload flag when TSO is enabled for tunnelling packets
> 
>  app/test-pmd/csumonly.c      | 29 +++++++++++++++++++++--------
>  drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
>  lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
>  3 files changed, 45 insertions(+), 12 deletions(-)
> 
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index ac4bd8f..aaa006f 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -204,7 +204,8 @@ parse_ethernet(struct ether_hdr *eth_hdr, struct testpmd_offload_info *info)  static void  parse_vxlan(struct
> udp_hdr *udp_hdr,
>  	    struct testpmd_offload_info *info,
> -	    uint32_t pkt_type)
> +	    uint32_t pkt_type,
> +	    uint64_t *ol_flags)
>  {
>  	struct ether_hdr *eth_hdr;
> 
> @@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
>  		RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
>  		return;
> 
> +	*ol_flags |= PKT_TX_TUNNEL_VXLAN;


Hmm, I don't actually see much difference between that version and the previous one.
 Regarding your comment on V2:
" this flag is for tunnelling type, and CTD is based on whether we need to do the
external ip offload and TSO.so this flag will not cause one extra CTD."
I think CTD selection should be based not only on is EIP cksum is enabled or not.
You can have tunneled packet with TSO on over IPv6, right?
I think for i40e we need CTD each time PKT_TX_TUNNEL_ is on.


>  	info->is_tunnel = 1;
>  	info->outer_ethertype = info->ethertype;
>  	info->outer_l2_len = info->l2_len;
> @@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> 
>  /* Parse a gre header */
>  static void
> -parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
> +parse_gre(struct simple_gre_hdr *gre_hdr,
> +	  struct testpmd_offload_info *info,
> +	  uint64_t *ol_flags)
>  {
>  	struct ether_hdr *eth_hdr;
>  	struct ipv4_hdr *ipv4_hdr;
> @@ -242,6 +246,8 @@ parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
>  	if ((gre_hdr->flags & _htons(~GRE_SUPPORTED_FIELDS)) != 0)
>  		return;
> 
> +	*ol_flags |= PKT_TX_TUNNEL_GRE;
> +
>  	gre_len += sizeof(struct simple_gre_hdr);
> 
>  	if (gre_hdr->flags & _htons(GRE_KEY_PRESENT)) @@ -417,7 +423,7 @@ process_inner_cksums(void *l3_hdr, const struct
> testpmd_offload_info *info,
>   * packet */
>  static uint64_t
>  process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
> -	uint16_t testpmd_ol_flags)
> +	uint16_t testpmd_ol_flags, uint64_t orig_ol_flags)
>  {
>  	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
>  	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr; @@ -428,7 +434,8 @@ process_outer_cksums(void *outer_l3_hdr, struct
> testpmd_offload_info *info,
>  		ipv4_hdr->hdr_checksum = 0;
>  		ol_flags |= PKT_TX_OUTER_IPV4;
> 
> -		if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM)
> +		if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) ||
> +		    (info->tso_segsz != 0))
>  			ol_flags |= PKT_TX_OUTER_IP_CKSUM;

Why do you need to always raise OUTER_IP_CKSUM when TSO is enabled?

>  		else
>  			ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr); @@ -442,6 +449,9 @@ process_outer_cksums(void
> *outer_l3_hdr, struct testpmd_offload_info *info,
>  	 * hardware supporting it today, and no API for it. */
> 
>  	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
> +	if ((orig_ol_flags & PKT_TX_TCP_SEG) &&
> +	    ((orig_ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN))
> +		udp_hdr->dgram_cksum = 0;
>  	/* do not recalculate udp cksum if it was 0 */
>  	if (udp_hdr->dgram_cksum != 0) {
>  		udp_hdr->dgram_cksum = 0;
> @@ -705,15 +715,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>  			if (info.l4_proto == IPPROTO_UDP) {
>  				struct udp_hdr *udp_hdr;
>  				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
> -					info.l3_len);
> -				parse_vxlan(udp_hdr, &info, m->packet_type);
> +					   info.l3_len);
> +				parse_vxlan(udp_hdr, &info, m->packet_type,
> +					    &ol_flags);
>  			} else if (info.l4_proto == IPPROTO_GRE) {
>  				struct simple_gre_hdr *gre_hdr;
>  				gre_hdr = (struct simple_gre_hdr *)
>  					((char *)l3_hdr + info.l3_len);
> -				parse_gre(gre_hdr, &info);
> +				parse_gre(gre_hdr, &info, &ol_flags);
>  			} else if (info.l4_proto == IPPROTO_IPIP) {
>  				void *encap_ip_hdr;
> +
> +				ol_flags |= PKT_TX_TUNNEL_IPIP;
>  				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
>  				parse_encap_ip(encap_ip_hdr, &info);
>  			}
> @@ -745,7 +758,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>  		 * processed in hardware. */
>  		if (info.is_tunnel == 1) {
>  			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
> -				testpmd_ol_flags);
> +				testpmd_ol_flags, ol_flags);
>  		}
> 
>  		/* step 4: fill the mbuf meta data (flags and header lengths) */ diff --git a/drivers/net/i40e/i40e_rxtx.c
> b/drivers/net/i40e/i40e_rxtx.c index 049a813..4c987f2 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
>  			union i40e_tx_offload tx_offload,
>  			uint32_t *cd_tunneling)
>  {
> +	/* Tx pkts tunnel type*/
> +	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
> +		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
> +	else if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE)
> +		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
> +
>  	/* UDP tunneling packet TX checksum offload */
>  	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {


I believe the problem is still there: you setup EIPLEN and NATLEN only when
PKT_TX_OUTER_IP_CKSUM is on.
Same story with MACLEN, you setup it with tx_offload.outer_l2_len,
only when PKT_TX_OUTER_IP_CKSUM is on:

if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {

                *td_offset |= (tx_offload.outer_l2_len >> 1)
                                << I40E_TX_DESC_LENGTH_MACLEN_SHIFT;

                if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
                        *cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4;
                else if (ol_flags & PKT_TX_OUTER_IPV4)
                        *cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4_NO_CSUM;
                else if (ol_flags & PKT_TX_OUTER_IPV6)
                        *cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;

                /* Now set the ctx descriptor fields */
                *cd_tunneling |= (tx_offload.outer_l3_len >> 2) <<
                                I40E_TXD_CTX_QW0_EXT_IPLEN_SHIFT |
                                (tx_offload.l2_len >> 1) <<
                                I40E_TXD_CTX_QW0_NATLEN_SHIFT;

        } else
                *td_offset |= (tx_offload.l2_len >> 1)
                        << I40E_TX_DESC_LENGTH_MACLEN_SHIFT;

So if user would like to enable TSO for tunneled packets over outer IPv6 packets,
I suppose it wouldn't work, right? 
Again people can choose to setup PKT_TX_TUNNEL_VXLAN for non-tso packets,
and don't ask for OUTER_IP_CHECKSUM.
 
I think it needs to be something like that:

if (ol_flags & PKT_TX_OUTER_IP_CKSUM) ||
	(ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN || 
	ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE) {
	...
}

Also, I think to modify i40e_calc_context_desc(), so it return 1,
when tunneling flags (VXLAN, GRE) is on.

Another thing, if we introduce new ol_flags PKT_TX_TUNNEL_*,
don't we need to update dev_info.tx_offload_capa, so user can
query does device support that or not?

Konstantin

> 
> @@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
> 
>  /* set i40e TSO context descriptor */
>  static inline uint64_t
> -i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> +i40e_set_tso_ctx(struct rte_mbuf *mbuf,
> +		 union i40e_tx_offload tx_offload)
>  {
>  	uint64_t ctx_desc = 0;
>  	uint32_t cd_cmd, hdr_len, cd_tso_len;
> @@ -1521,7 +1528,7 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
>  	}
> 
>  	/**
> -	 * in case of tunneling packet, the outer_l2_len and
> +	 * in case of non tunneling packet, the outer_l2_len and
>  	 * outer_l3_len must be 0.
>  	 */
>  	hdr_len = tx_offload.outer_l2_len +
> @@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
>  		 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
>  		((uint64_t)mbuf->tso_segsz <<
>  		 I40E_TXD_CTX_QW1_MSS_SHIFT);
> -
>  	return ctx_desc;
>  }
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index 15e3a10..90812ea 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -133,6 +133,17 @@ extern "C" {
>  /* add new TX flags here */
> 
>  /**
> + * Bits 45:48 used for the tunnel type.
> + * When doing Tx offload like TSO or checksum, the HW needs to
> +configure the
> + * tunnel type into the HW descriptors.
> + */
> +#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
> +#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
> +#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
> +/* add new TX TUNNEL type here */
> +#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
> +
> +/**
>   * Second VLAN insertion (QinQ) flag.
>   */
>  #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
> @@ -867,7 +878,10 @@ struct rte_mbuf {
>  	union {
>  		uint64_t tx_offload;       /**< combined for easy fetch */
>  		struct {
> -			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
> +			uint64_t l2_len:7;
> +			/**< L2 (MAC) Header Length if it isn't a tunneling pkt.
> +			 * for tunnel it is outer L4 len+tunnel len+inner L2 len
> +			 */
>  			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
>  			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
>  			uint64_t tso_segsz:16; /**< TCP TSO segment size */
> --
> 2.1.4

Re: [PATCH v3] i40: fix the VXLAN TSO issue

[Tan, Jianfeng]

Hi Konstantin,


On 7/19/2016 6:29 PM, Ananyev, Konstantin wrote:
>
>> Problem:
>> When using the TSO + VXLAN feature in i40e, the outer UDP length fields in the multiple UDP segments which are TSOed by the i40e will
>> have a wrong value.
>>
>> Fix this problem by adding the tunnel type field in the i40e descriptor which was missed before.
>>
>> Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")
>>
>> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
>> ---
>> v2: edited the comments
>> v3: added external IP offload flag when TSO is enabled for tunnelling packets
>>
>>   app/test-pmd/csumonly.c      | 29 +++++++++++++++++++++--------
>>   drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
>>   lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
>>   3 files changed, 45 insertions(+), 12 deletions(-)
>>
>> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index ac4bd8f..aaa006f 100644
>> --- a/app/test-pmd/csumonly.c
>> +++ b/app/test-pmd/csumonly.c
>> @@ -204,7 +204,8 @@ parse_ethernet(struct ether_hdr *eth_hdr, struct testpmd_offload_info *info)  static void  parse_vxlan(struct
>> udp_hdr *udp_hdr,
>>   	    struct testpmd_offload_info *info,
>> -	    uint32_t pkt_type)
>> +	    uint32_t pkt_type,
>> +	    uint64_t *ol_flags)
>>   {
>>   	struct ether_hdr *eth_hdr;
>>
>> @@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
>>   		RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
>>   		return;
>>
>> +	*ol_flags |= PKT_TX_TUNNEL_VXLAN;
>
> Hmm, I don't actually see much difference between that version and the previous one.
>   Regarding your comment on V2:
> " this flag is for tunnelling type, and CTD is based on whether we need to do the
> external ip offload and TSO.so this flag will not cause one extra CTD."
> I think CTD selection should be based not only on is EIP cksum is enabled or not.
> You can have tunneled packet with TSO on over IPv6, right?
> I think for i40e we need CTD each time PKT_TX_TUNNEL_ is on.

Yes, I agree. Depending on whether EIP cksum is enabled to fill 
tunneling parameters is not correct under the case: EIP cksum is not 
needed, tso is needed (tunneling parameter will not be filled). And app 
should not set PKT_TX_TUNNEL_ if there's no TSO and no checksum offload 
to avoid extra filling of tunneling parameters.

Another corner case is that, EIP cksum not needed, TSO not needed, but 
inner L3/L4 checksum needed, so:
(1) App to set PKT_TX_TUNNEL_ on, but in this case, no extra CTD will be 
used according to i40e_calc_context_desc(), so it cannot work right.
(2) App to unset PKT_TX_TUNNEL_, will not work either.

How do you think?
(Sorry, the answer is in your following comments. We need to change 
i40e_calc_context_desc() to return 1 if PKT_TX_TUNNEL_ is on.)


>
>
>>   	info->is_tunnel = 1;
>>   	info->outer_ethertype = info->ethertype;
>>   	info->outer_l2_len = info->l2_len;
>> @@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
>>
>>   /* Parse a gre header */
>>   static void
>> -parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
>> +parse_gre(struct simple_gre_hdr *gre_hdr,
>> +	  struct testpmd_offload_info *info,
>> +	  uint64_t *ol_flags)
>>   {
>>   	struct ether_hdr *eth_hdr;
>>   	struct ipv4_hdr *ipv4_hdr;
>> @@ -242,6 +246,8 @@ parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info *info)
>>   	if ((gre_hdr->flags & _htons(~GRE_SUPPORTED_FIELDS)) != 0)
>>   		return;
>>
>> +	*ol_flags |= PKT_TX_TUNNEL_GRE;
>> +
>>   	gre_len += sizeof(struct simple_gre_hdr);
>>
>>   	if (gre_hdr->flags & _htons(GRE_KEY_PRESENT)) @@ -417,7 +423,7 @@ process_inner_cksums(void *l3_hdr, const struct
>> testpmd_offload_info *info,
>>    * packet */
>>   static uint64_t
>>   process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
>> -	uint16_t testpmd_ol_flags)
>> +	uint16_t testpmd_ol_flags, uint64_t orig_ol_flags)
>>   {
>>   	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
>>   	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr; @@ -428,7 +434,8 @@ process_outer_cksums(void *outer_l3_hdr, struct
>> testpmd_offload_info *info,
>>   		ipv4_hdr->hdr_checksum = 0;
>>   		ol_flags |= PKT_TX_OUTER_IPV4;
>>
>> -		if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM)
>> +		if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) ||
>> +		    (info->tso_segsz != 0))
>>   			ol_flags |= PKT_TX_OUTER_IP_CKSUM;
> Why do you need to always raise OUTER_IP_CKSUM when TSO is enabled?

I believe it's not necessary if we fill tunneling parameters according 
to PKT_TX_TUNNEL_ as you suggest above.

>
>>   		else
>>   			ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr); @@ -442,6 +449,9 @@ process_outer_cksums(void
>> *outer_l3_hdr, struct testpmd_offload_info *info,
>>   	 * hardware supporting it today, and no API for it. */
>>
>>   	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
>> +	if ((orig_ol_flags & PKT_TX_TCP_SEG) &&
>> +	    ((orig_ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN))
>> +		udp_hdr->dgram_cksum = 0;
>>   	/* do not recalculate udp cksum if it was 0 */
>>   	if (udp_hdr->dgram_cksum != 0) {
>>   		udp_hdr->dgram_cksum = 0;
>> @@ -705,15 +715,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>>   			if (info.l4_proto == IPPROTO_UDP) {
>>   				struct udp_hdr *udp_hdr;
>>   				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
>> -					info.l3_len);
>> -				parse_vxlan(udp_hdr, &info, m->packet_type);
>> +					   info.l3_len);
>> +				parse_vxlan(udp_hdr, &info, m->packet_type,
>> +					    &ol_flags);
>>   			} else if (info.l4_proto == IPPROTO_GRE) {
>>   				struct simple_gre_hdr *gre_hdr;
>>   				gre_hdr = (struct simple_gre_hdr *)
>>   					((char *)l3_hdr + info.l3_len);
>> -				parse_gre(gre_hdr, &info);
>> +				parse_gre(gre_hdr, &info, &ol_flags);
>>   			} else if (info.l4_proto == IPPROTO_IPIP) {
>>   				void *encap_ip_hdr;
>> +
>> +				ol_flags |= PKT_TX_TUNNEL_IPIP;
>>   				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
>>   				parse_encap_ip(encap_ip_hdr, &info);
>>   			}
>> @@ -745,7 +758,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>>   		 * processed in hardware. */
>>   		if (info.is_tunnel == 1) {
>>   			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
>> -				testpmd_ol_flags);
>> +				testpmd_ol_flags, ol_flags);
>>   		}
>>
>>   		/* step 4: fill the mbuf meta data (flags and header lengths) */ diff --git a/drivers/net/i40e/i40e_rxtx.c
>> b/drivers/net/i40e/i40e_rxtx.c index 049a813..4c987f2 100644
>> --- a/drivers/net/i40e/i40e_rxtx.c
>> +++ b/drivers/net/i40e/i40e_rxtx.c
>> @@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
>>   			union i40e_tx_offload tx_offload,
>>   			uint32_t *cd_tunneling)
>>   {
>> +	/* Tx pkts tunnel type*/
>> +	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
>> +		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
>> +	else if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE)
>> +		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
>> +
>>   	/* UDP tunneling packet TX checksum offload */
>>   	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
>
> I believe the problem is still there: you setup EIPLEN and NATLEN only when
> PKT_TX_OUTER_IP_CKSUM is on.
> Same story with MACLEN, you setup it with tx_offload.outer_l2_len,
> only when PKT_TX_OUTER_IP_CKSUM is on:
>
> if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
>
>                  *td_offset |= (tx_offload.outer_l2_len >> 1)
>                                  << I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
>
>                  if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
>                          *cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4;
>                  else if (ol_flags & PKT_TX_OUTER_IPV4)
>                          *cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4_NO_CSUM;
>                  else if (ol_flags & PKT_TX_OUTER_IPV6)
>                          *cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;
>
>                  /* Now set the ctx descriptor fields */
>                  *cd_tunneling |= (tx_offload.outer_l3_len >> 2) <<
>                                  I40E_TXD_CTX_QW0_EXT_IPLEN_SHIFT |
>                                  (tx_offload.l2_len >> 1) <<
>                                  I40E_TXD_CTX_QW0_NATLEN_SHIFT;
>
>          } else
>                  *td_offset |= (tx_offload.l2_len >> 1)
>                          << I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
>
> So if user would like to enable TSO for tunneled packets over outer IPv6 packets,
> I suppose it wouldn't work, right?
> Again people can choose to setup PKT_TX_TUNNEL_VXLAN for non-tso packets,
> and don't ask for OUTER_IP_CHECKSUM.
>   
> I think it needs to be something like that:
>
> if (ol_flags & PKT_TX_OUTER_IP_CKSUM) ||
> 	(ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN ||
> 	ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE) {
> 	...
> }

Is "ol_flags & PKT_TX_OUTER_IP_CKSUM" necessary? Tunneling type is 
indispensable to make PKT_TX_OUTER_IP_CKSUM work. I mean something like 
this:

    if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN ||
         (ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_GRE) {
         ...
    }

>
> Also, I think to modify i40e_calc_context_desc(), so it return 1,
> when tunneling flags (VXLAN, GRE) is on.

Agreed.

>
> Another thing, if we introduce new ol_flags PKT_TX_TUNNEL_*,
> don't we need to update dev_info.tx_offload_capa, so user can
> query does device support that or not?

Agreed.

Thanks,
Jianfeng

>
> Konstantin
>
>> @@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
>>
>>   /* set i40e TSO context descriptor */
>>   static inline uint64_t
>> -i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
>> +i40e_set_tso_ctx(struct rte_mbuf *mbuf,
>> +		 union i40e_tx_offload tx_offload)
>>   {
>>   	uint64_t ctx_desc = 0;
>>   	uint32_t cd_cmd, hdr_len, cd_tso_len;
>> @@ -1521,7 +1528,7 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
>>   	}
>>
>>   	/**
>> -	 * in case of tunneling packet, the outer_l2_len and
>> +	 * in case of non tunneling packet, the outer_l2_len and
>>   	 * outer_l3_len must be 0.
>>   	 */
>>   	hdr_len = tx_offload.outer_l2_len +
>> @@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
>>   		 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
>>   		((uint64_t)mbuf->tso_segsz <<
>>   		 I40E_TXD_CTX_QW1_MSS_SHIFT);
>> -
>>   	return ctx_desc;
>>   }
>>
>> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index 15e3a10..90812ea 100644
>> --- a/lib/librte_mbuf/rte_mbuf.h
>> +++ b/lib/librte_mbuf/rte_mbuf.h
>> @@ -133,6 +133,17 @@ extern "C" {
>>   /* add new TX flags here */
>>
>>   /**
>> + * Bits 45:48 used for the tunnel type.
>> + * When doing Tx offload like TSO or checksum, the HW needs to
>> +configure the
>> + * tunnel type into the HW descriptors.
>> + */
>> +#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
>> +#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
>> +#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
>> +/* add new TX TUNNEL type here */
>> +#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
>> +
>> +/**
>>    * Second VLAN insertion (QinQ) flag.
>>    */
>>   #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
>> @@ -867,7 +878,10 @@ struct rte_mbuf {
>>   	union {
>>   		uint64_t tx_offload;       /**< combined for easy fetch */
>>   		struct {
>> -			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
>> +			uint64_t l2_len:7;
>> +			/**< L2 (MAC) Header Length if it isn't a tunneling pkt.
>> +			 * for tunnel it is outer L4 len+tunnel len+inner L2 len
>> +			 */
>>   			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
>>   			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
>>   			uint64_t tso_segsz:16; /**< TCP TSO segment size */
>> --
>> 2.1.4

Re: [PATCH v3] i40: fix the VXLAN TSO issue

[Tan, Jianfeng]

Hi,

On 7/18/2016 7:56 PM, Zhe Tao wrote:
> Problem:
> When using the TSO + VXLAN feature in i40e, the outer UDP length fields in
> the multiple UDP segments which are TSOed by the i40e will have a
> wrong value.
>
> Fix this problem by adding the tunnel type field in the i40e descriptor
> which was missed before.
>
> Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")
>
> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> ---
> v2: edited the comments
> v3: added external IP offload flag when TSO is enabled for tunnelling packets
>
>   app/test-pmd/csumonly.c      | 29 +++++++++++++++++++++--------
>   drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
>   lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
>   3 files changed, 45 insertions(+), 12 deletions(-)
>
...
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> index 15e3a10..90812ea 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -133,6 +133,17 @@ extern "C" {
>   /* add new TX flags here */
>   
>   /**
> + * Bits 45:48 used for the tunnel type.
> + * When doing Tx offload like TSO or checksum, the HW needs to configure the
> + * tunnel type into the HW descriptors.
> + */
> +#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
> +#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
> +#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
> +/* add new TX TUNNEL type here */
> +#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
> +

Above flag bits are added so that i40e driver can tell tunnel type of 
this packet (udp or gre or ipip), just interested to know how about just 
do a simple analysis like below without introducing these flags?

if outer_ether.proto == ipv4
     l4_proto = ipv4_hdr->next_proto;
else outer_ether.proto == ipv6
     l4_proto = ipv6_hdr->next_proto;

switch (l4_proto)
     case ipv4:
     case ipv6:
         tunnel_type = ipip;
         break;
     case udp:
         tunnel_type = udp;
         break;
     case gre:
         tunnel_type = gre;
         break;
     default:
          error;

Thanks,
Jianfeng


> +/**
>    * Second VLAN insertion (QinQ) flag.
>    */
>   #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
> @@ -867,7 +878,10 @@ struct rte_mbuf {
>   	union {
>   		uint64_t tx_offload;       /**< combined for easy fetch */
>   		struct {
> -			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
> +			uint64_t l2_len:7;
> +			/**< L2 (MAC) Header Length if it isn't a tunneling pkt.
> +			 * for tunnel it is outer L4 len+tunnel len+inner L2 len
> +			 */
>   			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
>   			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
>   			uint64_t tso_segsz:16; /**< TCP TSO segment size */

Re: [PATCH v3] i40: fix the VXLAN TSO issue

[Ananyev, Konstantin]

Hi Jianfeng,

> 
> Hi,
> 
> On 7/18/2016 7:56 PM, Zhe Tao wrote:
> > Problem:
> > When using the TSO + VXLAN feature in i40e, the outer UDP length
> > fields in the multiple UDP segments which are TSOed by the i40e will
> > have a wrong value.
> >
> > Fix this problem by adding the tunnel type field in the i40e
> > descriptor which was missed before.
> >
> > Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")
> >
> > Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> > ---
> > v2: edited the comments
> > v3: added external IP offload flag when TSO is enabled for tunnelling
> > packets
> >
> >   app/test-pmd/csumonly.c      | 29 +++++++++++++++++++++--------
> >   drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
> >   lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
> >   3 files changed, 45 insertions(+), 12 deletions(-)
> >
> ...
> > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > index 15e3a10..90812ea 100644
> > --- a/lib/librte_mbuf/rte_mbuf.h
> > +++ b/lib/librte_mbuf/rte_mbuf.h
> > @@ -133,6 +133,17 @@ extern "C" {
> >   /* add new TX flags here */
> >
> >   /**
> > + * Bits 45:48 used for the tunnel type.
> > + * When doing Tx offload like TSO or checksum, the HW needs to
> > +configure the
> > + * tunnel type into the HW descriptors.
> > + */
> > +#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
> > +#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
> > +#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
> > +/* add new TX TUNNEL type here */
> > +#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
> > +
> 
> Above flag bits are added so that i40e driver can tell tunnel type of this packet (udp or gre or ipip), just interested to know how about just do
> a simple analysis like below without introducing these flags?
> 
> if outer_ether.proto == ipv4
>      l4_proto = ipv4_hdr->next_proto;
> else outer_ether.proto == ipv6
>      l4_proto = ipv6_hdr->next_proto;
> 
> switch (l4_proto)
>      case ipv4:
>      case ipv6:
>          tunnel_type = ipip;
>          break;
>      case udp:
>          tunnel_type = udp;
>          break;
>      case gre:
>          tunnel_type = gre;
>          break;
>      default:
>           error;

Right now none of our PMDs reads/writes actual packet data,
and I think it is better to keep it that way.
It is an upper layer responsibility to specify which offload
needs to be enabled and provide necessary information. 
Konstantin 

> 
> Thanks,
> Jianfeng
> 
> 
> > +/**
> >    * Second VLAN insertion (QinQ) flag.
> >    */
> >   #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
> > @@ -867,7 +878,10 @@ struct rte_mbuf {
> >   	union {
> >   		uint64_t tx_offload;       /**< combined for easy fetch */
> >   		struct {
> > -			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
> > +			uint64_t l2_len:7;
> > +			/**< L2 (MAC) Header Length if it isn't a tunneling pkt.
> > +			 * for tunnel it is outer L4 len+tunnel len+inner L2 len
> > +			 */
> >   			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
> >   			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
> >   			uint64_t tso_segsz:16; /**< TCP TSO segment size */

Re: [PATCH v3] i40: fix the VXLAN TSO issue

[Tan, Jianfeng]

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Friday, July 29, 2016 4:46 PM
> To: Tan, Jianfeng; dev@dpdk.org
> Cc: Wu, Jingjing
> Subject: RE: [dpdk-dev] [PATCH v3] i40: fix the VXLAN TSO issue
> 
> Hi Jianfeng,
> 
> >
> > Hi,
> >
> > On 7/18/2016 7:56 PM, Zhe Tao wrote:
> > > Problem:
> > > When using the TSO + VXLAN feature in i40e, the outer UDP length
> > > fields in the multiple UDP segments which are TSOed by the i40e will
> > > have a wrong value.
> > >
> > > Fix this problem by adding the tunnel type field in the i40e
> > > descriptor which was missed before.
> > >
> > > Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")
> > >
> > > Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> > > ---
> > > v2: edited the comments
> > > v3: added external IP offload flag when TSO is enabled for tunnelling
> > > packets
> > >
> > >   app/test-pmd/csumonly.c      | 29 +++++++++++++++++++++--------
> > >   drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
> > >   lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
> > >   3 files changed, 45 insertions(+), 12 deletions(-)
> > >
> > ...
> > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
> > > index 15e3a10..90812ea 100644
> > > --- a/lib/librte_mbuf/rte_mbuf.h
> > > +++ b/lib/librte_mbuf/rte_mbuf.h
> > > @@ -133,6 +133,17 @@ extern "C" {
> > >   /* add new TX flags here */
> > >
> > >   /**
> > > + * Bits 45:48 used for the tunnel type.
> > > + * When doing Tx offload like TSO or checksum, the HW needs to
> > > +configure the
> > > + * tunnel type into the HW descriptors.
> > > + */
> > > +#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
> > > +#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
> > > +#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
> > > +/* add new TX TUNNEL type here */
> > > +#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
> > > +
> >
> > Above flag bits are added so that i40e driver can tell tunnel type of this
> packet (udp or gre or ipip), just interested to know how about just do
> > a simple analysis like below without introducing these flags?
> >
> > if outer_ether.proto == ipv4
> >      l4_proto = ipv4_hdr->next_proto;
> > else outer_ether.proto == ipv6
> >      l4_proto = ipv6_hdr->next_proto;
> >
> > switch (l4_proto)
> >      case ipv4:
> >      case ipv6:
> >          tunnel_type = ipip;
> >          break;
> >      case udp:
> >          tunnel_type = udp;
> >          break;
> >      case gre:
> >          tunnel_type = gre;
> >          break;
> >      default:
> >           error;
> 
> Right now none of our PMDs reads/writes actual packet data,
> and I think it is better to keep it that way.
> It is an upper layer responsibility to specify which offload
> needs to be enabled and provide necessary information.
> Konstantin

Make sense. I'll send out v4 as the original way later.

Thanks,
Jianfeng

[PATCH v4 0/3] Add TSO on tunneling packet

[Jianfeng Tan]

Patch 1: mbuf: add Tx side tunneling type
Patch 2: net/i40e: add TSO support on tunneling packet
Patch 3: app/testpmd: fix Tx offload on tunneling packet

v4:
  - According to tunnel type flag to parse tunneling parameters.
  - Add new capabilities to indicate support of TSO on tunneling packets.
  - Add check to see if TSO on tunneling packets are supported for the
    specified NIC.
  - Add support for geneve (as i40e does not differentiate UDP tunneling.
  - Split into three patches.

v3:
  - added external IP offload flag when TSO is enabled for tunnelling packets
v2:
  - edited the comments

Signed-off-by: Zhe Tao <zhe.tao@intel.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>

Jianfeng Tan (3):
  mbuf: add Tx side tunneling type
  net/i40e: add TSO support on tunneling packet
  app/testpmd: fix Tx offload on tunneling packet

 app/test-pmd/cmdline.c         | 42 +++++++++++++++++---
 app/test-pmd/csumonly.c        | 37 +++++++++++++----
 drivers/net/i40e/i40e_ethdev.c |  6 ++-
 drivers/net/i40e/i40e_rxtx.c   | 90 +++++++++++++++++++++++++++++-------------
 lib/librte_ether/rte_ethdev.h  |  4 ++
 lib/librte_mbuf/rte_mbuf.c     |  4 ++
 lib/librte_mbuf/rte_mbuf.h     | 17 +++++++-
 7 files changed, 157 insertions(+), 43 deletions(-)

-- 
2.7.4

[PATCH v4 1/3] mbuf: add Tx side tunneling type

[Jianfeng Tan]

To support tunneling packet offload capabilities on Tx side, PMDs
(e.g., i40e) need to know what kind of tunneling type of this packet.
Instead of analyzing the packet itself, we depend on applications to
correctly set the tunneling type. These flags are defined inside
rte_mbuf.ol_flags.

Signed-off-by: Zhe Tao <zhe.tao@intel.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 lib/librte_mbuf/rte_mbuf.c |  4 ++++
 lib/librte_mbuf/rte_mbuf.h | 17 ++++++++++++++++-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/lib/librte_mbuf/rte_mbuf.c b/lib/librte_mbuf/rte_mbuf.c
index 4846b89..4505abb 100644
--- a/lib/librte_mbuf/rte_mbuf.c
+++ b/lib/librte_mbuf/rte_mbuf.c
@@ -302,6 +302,10 @@ const char *rte_get_tx_ol_flag_name(uint64_t mask)
 	case PKT_TX_OUTER_IP_CKSUM: return "PKT_TX_OUTER_IP_CKSUM";
 	case PKT_TX_OUTER_IPV4: return "PKT_TX_OUTER_IPV4";
 	case PKT_TX_OUTER_IPV6: return "PKT_TX_OUTER_IPV6";
+	case PKT_TX_TUNNEL_VXLAN: return "PKT_TX_TUNNEL_VXLAN";
+	case PKT_TX_TUNNEL_GRE: return "PKT_TX_TUNNEL_GRE";
+	case PKT_TX_TUNNEL_IPIP: return "PKT_TX_TUNNEL_IPIP";
+	case PKT_TX_TUNNEL_GENEVE: return "PKT_TX_TUNNEL_GENEVE";
 	default: return NULL;
 	}
 }
diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 101485f..0eec112 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -129,6 +129,18 @@ extern "C" {
 /* add new TX flags here */
 
 /**
+ * Bits 45:48 used for the tunnel type.
+ * When doing Tx offload like TSO or checksum, the HW needs to configure the
+ * tunnel type into the HW descriptors.
+ */
+#define PKT_TX_TUNNEL_VXLAN   (0x1ULL << 45)
+#define PKT_TX_TUNNEL_GRE     (0x2ULL << 45)
+#define PKT_TX_TUNNEL_IPIP    (0x3ULL << 45)
+#define PKT_TX_TUNNEL_GENEVE  (0x4ULL << 45)
+/* add new TX TUNNEL type here */
+#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
+
+/**
  * Second VLAN insertion (QinQ) flag.
  */
 #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double VLAN inserted. */
@@ -863,7 +875,10 @@ struct rte_mbuf {
 	union {
 		uint64_t tx_offload;       /**< combined for easy fetch */
 		struct {
-			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
+			uint64_t l2_len:7;
+			/**< L2 (MAC) Header Length for non-tunneling pkt.
+			 * Outer_L4_len + ... + Inner_L2_len for tunneling pkt.
+			 */
 			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
 			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length. */
 			uint64_t tso_segsz:16; /**< TCP TSO segment size */
-- 
2.7.4

[PATCH v4 2/3] net/i40e: add TSO support on tunneling packet

[Jianfeng Tan]

To enable Tx side offload on tunneling packet, driver should set
correct tunneling parameters: (1) EIPT, External IP header type;
(2) EIPLEN, External IP; (3) L4TUNT; (4) L4TUNLEN. This parsing
behavior is based on (ol_flag & PKT_TX_TUNNEL_MASK). And when
it's a tunneling packet, MACLEN defines the outer L2 header.

Also, we define TSO on each kind of tunneling type as a capabilities.
Now only i40e declares to support them.

Signed-off-by: Zhe Tao <zhe.tao@intel.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c |  6 ++-
 drivers/net/i40e/i40e_rxtx.c   | 90 +++++++++++++++++++++++++++++-------------
 lib/librte_ether/rte_ethdev.h  |  4 ++
 3 files changed, 72 insertions(+), 28 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index d0aeb70..64ba570 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2576,7 +2576,11 @@ i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 		DEV_TX_OFFLOAD_TCP_CKSUM |
 		DEV_TX_OFFLOAD_SCTP_CKSUM |
 		DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM |
-		DEV_TX_OFFLOAD_TCP_TSO;
+		DEV_TX_OFFLOAD_TCP_TSO |
+		DEV_TX_OFFLOAD_VXLAN_TNL_TSO |
+		DEV_TX_OFFLOAD_GRE_TNL_TSO |
+		DEV_TX_OFFLOAD_IPIP_TNL_TSO |
+		DEV_TX_OFFLOAD_GENEVE_TNL_TSO;
 	dev_info->hash_key_size = (I40E_PFQF_HKEY_MAX_INDEX + 1) *
 						sizeof(uint32_t);
 	dev_info->reta_size = pf->hash_lut_size;
diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
index 554d167..4eac713 100644
--- a/drivers/net/i40e/i40e_rxtx.c
+++ b/drivers/net/i40e/i40e_rxtx.c
@@ -779,33 +779,65 @@ i40e_rxd_build_fdir(volatile union i40e_rx_desc *rxdp, struct rte_mbuf *mb)
 #endif
 	return flags;
 }
+
+static inline void
+i40e_parse_tunneling_params(uint64_t ol_flags,
+			    union i40e_tx_offload tx_offload,
+			    uint32_t *cd_tunneling)
+{
+	/* EIPT: External (outer) IP header type */
+	if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
+		*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4;
+	else if (ol_flags & PKT_TX_OUTER_IPV4)
+		*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4_NO_CSUM;
+	else if (ol_flags & PKT_TX_OUTER_IPV6)
+		*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;
+
+	/* EIPLEN: External (outer) IP header length, in DWords */
+	*cd_tunneling |= (tx_offload.outer_l3_len >> 2) <<
+		I40E_TXD_CTX_QW0_EXT_IPLEN_SHIFT;
+
+	/* L4TUNT: L4 Tunneling Type */
+	switch (ol_flags & PKT_TX_TUNNEL_MASK) {
+	case PKT_TX_TUNNEL_IPIP:
+		/* for non UDP / GRE tunneling, set to 00b */
+		break;
+	case PKT_TX_TUNNEL_VXLAN:
+	case PKT_TX_TUNNEL_GENEVE:
+		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
+		break;
+	case PKT_TX_TUNNEL_GRE:
+		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
+		break;
+	default:
+		PMD_TX_LOG(ERR, "Tunnel type not supported\n");
+		return;
+	}
+
+	/* L4TUNLEN: L4 Tunneling Length, in Words
+	 *
+	 * We depend on app to set rte_mbuf.l2_len correctly.
+	 * For IP in GRE it should be set to the length of the GRE
+	 * header;
+	 * for MAC in GRE or MAC in UDP it should be set to the length
+	 * of the GRE or UDP headers plus the inner MAC up to including
+	 * its last Ethertype.
+	 */
+	*cd_tunneling |= (tx_offload.l2_len >> 1) <<
+		I40E_TXD_CTX_QW0_NATLEN_SHIFT;
+}
+
 static inline void
 i40e_txd_enable_checksum(uint64_t ol_flags,
 			uint32_t *td_cmd,
 			uint32_t *td_offset,
-			union i40e_tx_offload tx_offload,
-			uint32_t *cd_tunneling)
+			union i40e_tx_offload tx_offload)
 {
-	/* UDP tunneling packet TX checksum offload */
-	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
-
+	/* Set MACLEN */
+	if (ol_flags & PKT_TX_TUNNEL_MASK)
 		*td_offset |= (tx_offload.outer_l2_len >> 1)
 				<< I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
-
-		if (ol_flags & PKT_TX_OUTER_IP_CKSUM)
-			*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4;
-		else if (ol_flags & PKT_TX_OUTER_IPV4)
-			*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4_NO_CSUM;
-		else if (ol_flags & PKT_TX_OUTER_IPV6)
-			*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;
-
-		/* Now set the ctx descriptor fields */
-		*cd_tunneling |= (tx_offload.outer_l3_len >> 2) <<
-				I40E_TXD_CTX_QW0_EXT_IPLEN_SHIFT |
-				(tx_offload.l2_len >> 1) <<
-				I40E_TXD_CTX_QW0_NATLEN_SHIFT;
-
-	} else
+	else
 		*td_offset |= (tx_offload.l2_len >> 1)
 			<< I40E_TX_DESC_LENGTH_MACLEN_SHIFT;
 
@@ -1484,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
 {
 	static uint64_t mask = PKT_TX_OUTER_IP_CKSUM |
 		PKT_TX_TCP_SEG |
-		PKT_TX_QINQ_PKT;
+		PKT_TX_QINQ_PKT |
+		PKT_TX_TUNNEL_MASK;
 
 #ifdef RTE_LIBRTE_IEEE1588
 	mask |= PKT_TX_IEEE1588_TMST;
@@ -1506,7 +1539,7 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
 	}
 
 	/**
-	 * in case of tunneling packet, the outer_l2_len and
+	 * in case of non tunneling packet, the outer_l2_len and
 	 * outer_l3_len must be 0.
 	 */
 	hdr_len = tx_offload.outer_l2_len +
@@ -1623,12 +1656,15 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 		/* Always enable CRC offload insertion */
 		td_cmd |= I40E_TX_DESC_CMD_ICRC;
 
-		/* Enable checksum offloading */
+		/* Fill in tunneling parameters if necessary */
 		cd_tunneling_params = 0;
-		if (ol_flags & I40E_TX_CKSUM_OFFLOAD_MASK) {
-			i40e_txd_enable_checksum(ol_flags, &td_cmd, &td_offset,
-				tx_offload, &cd_tunneling_params);
-		}
+		if (ol_flags & PKT_TX_TUNNEL_MASK)
+			i40e_parse_tunneling_params(ol_flags, tx_offload,
+						    &cd_tunneling_params);
+		/* Enable checksum offloading */
+		if (ol_flags & I40E_TX_CKSUM_OFFLOAD_MASK)
+			i40e_txd_enable_checksum(ol_flags, &td_cmd,
+						 &td_offset, tx_offload);
 
 		if (nb_ctx) {
 			/* Setup TX context descriptor if required */
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index b0fe033..7bf0cc4 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -864,6 +864,10 @@ struct rte_eth_conf {
 #define DEV_TX_OFFLOAD_UDP_TSO     0x00000040
 #define DEV_TX_OFFLOAD_OUTER_IPV4_CKSUM 0x00000080 /**< Used for tunneling packet. */
 #define DEV_TX_OFFLOAD_QINQ_INSERT 0x00000100
+#define DEV_TX_OFFLOAD_VXLAN_TNL_TSO    0x00000200    /**< Used for tunneling packet. */
+#define DEV_TX_OFFLOAD_GRE_TNL_TSO      0x00000400    /**< Used for tunneling packet. */
+#define DEV_TX_OFFLOAD_IPIP_TNL_TSO     0x00000800    /**< Used for tunneling packet. */
+#define DEV_TX_OFFLOAD_GENEVE_TNL_TSO   0x00001000    /**< Used for tunneling packet. */
 
 /**
  * Ethernet device information
-- 
2.7.4

[PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet

[Jianfeng Tan]

Tx offload on tunneling packet now requires applications to correctly
set tunneling type. Without setting it, i40e driver does not parse
tunneling parameters. Besides that, add a check to see if NIC supports
TSO on tunneling packet when executing "csum parse_tunnel on _port"
after "tso set _size _port" or the other way around.

Fixes: b51c47536a9e ("app/testpmd: support TSO in checksum forward engine")

Signed-off-by: Zhe Tao <zhe.tao@intel.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
 app/test-pmd/cmdline.c  | 42 ++++++++++++++++++++++++++++++++++++------
 app/test-pmd/csumonly.c | 37 +++++++++++++++++++++++++++++--------
 2 files changed, 65 insertions(+), 14 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index f90befc..561839f 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -3426,6 +3426,26 @@ struct cmd_csum_tunnel_result {
 };
 
 static void
+check_tunnel_tso_support(uint8_t port_id)
+{
+	struct rte_eth_dev_info dev_info;
+
+	rte_eth_dev_info_get(port_id, &dev_info);
+	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_VXLAN_TNL_TSO))
+		printf("Warning: TSO enabled but VXLAN TUNNEL TSO not "
+		       "supported by port %d\n", port_id);
+	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_GRE_TNL_TSO))
+		printf("Warning: TSO enabled but GRE TUNNEL TSO not "
+			"supported by port %d\n", port_id);
+	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_IPIP_TNL_TSO))
+		printf("Warning: TSO enabled but IPIP TUNNEL TSO not "
+		       "supported by port %d\n", port_id);
+	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_GENEVE_TNL_TSO))
+		printf("Warning: TSO enabled but GENEVE TUNNEL TSO not "
+		       "supported by port %d\n", port_id);
+}
+
+static void
 cmd_csum_tunnel_parsed(void *parsed_result,
 		       __attribute__((unused)) struct cmdline *cl,
 		       __attribute__((unused)) void *data)
@@ -3435,10 +3455,13 @@ cmd_csum_tunnel_parsed(void *parsed_result,
 	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
 		return;
 
-	if (!strcmp(res->onoff, "on"))
+	if (!strcmp(res->onoff, "on")) {
 		ports[res->port_id].tx_ol_flags |=
 			TESTPMD_TX_OFFLOAD_PARSE_TUNNEL;
-	else
+
+		if (ports[res->port_id].tso_segsz != 0)
+			check_tunnel_tso_support(res->port_id);
+	} else
 		ports[res->port_id].tx_ol_flags &=
 			(~TESTPMD_TX_OFFLOAD_PARSE_TUNNEL);
 
@@ -3502,10 +3525,17 @@ cmd_tso_set_parsed(void *parsed_result,
 
 	/* display warnings if configuration is not supported by the NIC */
 	rte_eth_dev_info_get(res->port_id, &dev_info);
-	if ((ports[res->port_id].tso_segsz != 0) &&
-		(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_TCP_TSO) == 0) {
-		printf("Warning: TSO enabled but not "
-			"supported by port %d\n", res->port_id);
+	if (ports[res->port_id].tso_segsz != 0) {
+		if (ports[res->port_id].tx_ol_flags &
+		    TESTPMD_TX_OFFLOAD_PARSE_TUNNEL)
+			check_tunnel_tso_support(res->port_id);
+		/* For packets,
+		 * (1) when tnl parse is disabled;
+		 * (2) when tnl parse is enabled but not deemed as tnl pkts
+		 */
+		if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_TCP_TSO))
+			printf("Warning: TSO enabled but not "
+			       "supported by port %d\n", res->port_id);
 	}
 }
 
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index ac4bd8f..0a1f95d 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -412,12 +412,10 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 	return ol_flags;
 }
 
-/* Calculate the checksum of outer header (only vxlan is supported,
- * meaning IP + UDP). The caller already checked that it's a vxlan
- * packet */
+/* Calculate the checksum of outer header */
 static uint64_t
 process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
-	uint16_t testpmd_ol_flags)
+	uint16_t testpmd_ol_flags, int tso_enabled)
 {
 	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
 	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr;
@@ -438,10 +436,20 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	if (info->outer_l4_proto != IPPROTO_UDP)
 		return ol_flags;
 
-	/* outer UDP checksum is always done in software as we have no
-	 * hardware supporting it today, and no API for it. */
-
 	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
+
+	/* outer UDP checksum is done in software as we have no hardware
+	 * supporting it today, and no API for it. In the other side, for
+	 * UDP tunneling, like VXLAN or Geneve, outer UDP checksum can be
+	 * set to zero.
+	 *
+	 * If a packet will be TSOed into small packets by NIC, we cannot
+	 * set/calculate a non-zero checksum, because it will be a wrong
+	 * value after the packet be split into several small packets.
+	 */
+	if (tso_enabled)
+		udp_hdr->dgram_cksum = 0;
+
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
@@ -704,18 +712,27 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) {
 			if (info.l4_proto == IPPROTO_UDP) {
 				struct udp_hdr *udp_hdr;
+
 				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
 					info.l3_len);
 				parse_vxlan(udp_hdr, &info, m->packet_type);
+				if (info.is_tunnel)
+					ol_flags |= PKT_TX_TUNNEL_VXLAN;
 			} else if (info.l4_proto == IPPROTO_GRE) {
 				struct simple_gre_hdr *gre_hdr;
+
 				gre_hdr = (struct simple_gre_hdr *)
 					((char *)l3_hdr + info.l3_len);
 				parse_gre(gre_hdr, &info);
+				if (info.is_tunnel)
+					ol_flags |= PKT_TX_TUNNEL_GRE;
 			} else if (info.l4_proto == IPPROTO_IPIP) {
 				void *encap_ip_hdr;
+
 				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
 				parse_encap_ip(encap_ip_hdr, &info);
+				if (info.is_tunnel)
+					ol_flags |= PKT_TX_TUNNEL_IPIP;
 			}
 		}
 
@@ -745,7 +762,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		 * processed in hardware. */
 		if (info.is_tunnel == 1) {
 			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
-				testpmd_ol_flags);
+				testpmd_ol_flags, ol_flags & PKT_TX_TCP_SEG);
 		}
 
 		/* step 4: fill the mbuf meta data (flags and header lengths) */
@@ -806,6 +823,10 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 				{ PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IPV4 },
 				{ PKT_TX_OUTER_IPV6, PKT_TX_OUTER_IPV6 },
 				{ PKT_TX_TCP_SEG, PKT_TX_TCP_SEG },
+				{ PKT_TX_TUNNEL_VXLAN, PKT_TX_TUNNEL_MASK },
+				{ PKT_TX_TUNNEL_GRE, PKT_TX_TUNNEL_MASK },
+				{ PKT_TX_TUNNEL_IPIP, PKT_TX_TUNNEL_MASK },
+				{ PKT_TX_TUNNEL_GENEVE, PKT_TX_TUNNEL_MASK },
 			};
 			unsigned j;
 			const char *name;
-- 
2.7.4

Re: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet

[Ananyev, Konstantin]

Hi Jainfeng,

> -----Original Message-----
> From: Tan, Jianfeng
> Sent: Monday, August 1, 2016 4:57 AM
> To: dev@dpdk.org
> Cc: thomas.monjalon@6wind.com; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>; Tan, Jianfeng
> <jianfeng.tan@intel.com>; Tao, Zhe <zhe.tao@intel.com>
> Subject: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet
> 
> Tx offload on tunneling packet now requires applications to correctly set tunneling type. Without setting it, i40e driver does not parse
> tunneling parameters. Besides that, add a check to see if NIC supports TSO on tunneling packet when executing "csum parse_tunnel on
> _port"
> after "tso set _size _port" or the other way around.
> 
> Fixes: b51c47536a9e ("app/testpmd: support TSO in checksum forward engine")
> 
> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
>  app/test-pmd/cmdline.c  | 42 ++++++++++++++++++++++++++++++++++++------
>  app/test-pmd/csumonly.c | 37 +++++++++++++++++++++++++++++--------
>  2 files changed, 65 insertions(+), 14 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index f90befc..561839f 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -3426,6 +3426,26 @@ struct cmd_csum_tunnel_result {  };
> 
>  static void
> +check_tunnel_tso_support(uint8_t port_id) {
> +	struct rte_eth_dev_info dev_info;
> +
> +	rte_eth_dev_info_get(port_id, &dev_info);
> +	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_VXLAN_TNL_TSO))
> +		printf("Warning: TSO enabled but VXLAN TUNNEL TSO not "
> +		       "supported by port %d\n", port_id);
> +	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_GRE_TNL_TSO))
> +		printf("Warning: TSO enabled but GRE TUNNEL TSO not "
> +			"supported by port %d\n", port_id);
> +	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_IPIP_TNL_TSO))
> +		printf("Warning: TSO enabled but IPIP TUNNEL TSO not "
> +		       "supported by port %d\n", port_id);
> +	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_GENEVE_TNL_TSO))
> +		printf("Warning: TSO enabled but GENEVE TUNNEL TSO not "
> +		       "supported by port %d\n", port_id); }
> +
> +static void
>  cmd_csum_tunnel_parsed(void *parsed_result,
>  		       __attribute__((unused)) struct cmdline *cl,
>  		       __attribute__((unused)) void *data) @@ -3435,10 +3455,13 @@ cmd_csum_tunnel_parsed(void *parsed_result,
>  	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
>  		return;
> 
> -	if (!strcmp(res->onoff, "on"))
> +	if (!strcmp(res->onoff, "on")) {
>  		ports[res->port_id].tx_ol_flags |=
>  			TESTPMD_TX_OFFLOAD_PARSE_TUNNEL;
> -	else
> +
> +		if (ports[res->port_id].tso_segsz != 0)
> +			check_tunnel_tso_support(res->port_id);
> +	} else
>  		ports[res->port_id].tx_ol_flags &=
>  			(~TESTPMD_TX_OFFLOAD_PARSE_TUNNEL);
> 
> @@ -3502,10 +3525,17 @@ cmd_tso_set_parsed(void *parsed_result,
> 
>  	/* display warnings if configuration is not supported by the NIC */
>  	rte_eth_dev_info_get(res->port_id, &dev_info);
> -	if ((ports[res->port_id].tso_segsz != 0) &&
> -		(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_TCP_TSO) == 0) {
> -		printf("Warning: TSO enabled but not "
> -			"supported by port %d\n", res->port_id);
> +	if (ports[res->port_id].tso_segsz != 0) {
> +		if (ports[res->port_id].tx_ol_flags &
> +		    TESTPMD_TX_OFFLOAD_PARSE_TUNNEL)
> +			check_tunnel_tso_support(res->port_id);
> +		/* For packets,
> +		 * (1) when tnl parse is disabled;
> +		 * (2) when tnl parse is enabled but not deemed as tnl pkts
> +		 */
> +		if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_TCP_TSO))
> +			printf("Warning: TSO enabled but not "
> +			       "supported by port %d\n", res->port_id);
>  	}
>  }
> 
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index ac4bd8f..0a1f95d 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -412,12 +412,10 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
>  	return ol_flags;
>  }
> 
> -/* Calculate the checksum of outer header (only vxlan is supported,
> - * meaning IP + UDP). The caller already checked that it's a vxlan
> - * packet */
> +/* Calculate the checksum of outer header */
>  static uint64_t
>  process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
> -	uint16_t testpmd_ol_flags)
> +	uint16_t testpmd_ol_flags, int tso_enabled)
>  {
>  	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
>  	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr; @@ -438,10 +436,20 @@ process_outer_cksums(void *outer_l3_hdr, struct
> testpmd_offload_info *info,
>  	if (info->outer_l4_proto != IPPROTO_UDP)
>  		return ol_flags;
> 
> -	/* outer UDP checksum is always done in software as we have no
> -	 * hardware supporting it today, and no API for it. */
> -
>  	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
> +
> +	/* outer UDP checksum is done in software as we have no hardware
> +	 * supporting it today, and no API for it. In the other side, for
> +	 * UDP tunneling, like VXLAN or Geneve, outer UDP checksum can be
> +	 * set to zero.
> +	 *
> +	 * If a packet will be TSOed into small packets by NIC, we cannot
> +	 * set/calculate a non-zero checksum, because it will be a wrong
> +	 * value after the packet be split into several small packets.
> +	 */
> +	if (tso_enabled)
> +		udp_hdr->dgram_cksum = 0;
> +
>  	/* do not recalculate udp cksum if it was 0 */
>  	if (udp_hdr->dgram_cksum != 0) {
>  		udp_hdr->dgram_cksum = 0;
> @@ -704,18 +712,27 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>  		if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) {
>  			if (info.l4_proto == IPPROTO_UDP) {
>  				struct udp_hdr *udp_hdr;
> +
>  				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
>  					info.l3_len);
>  				parse_vxlan(udp_hdr, &info, m->packet_type);
> +				if (info.is_tunnel)
> +					ol_flags |= PKT_TX_TUNNEL_VXLAN;
>  			} else if (info.l4_proto == IPPROTO_GRE) {
>  				struct simple_gre_hdr *gre_hdr;
> +
>  				gre_hdr = (struct simple_gre_hdr *)
>  					((char *)l3_hdr + info.l3_len);
>  				parse_gre(gre_hdr, &info);
> +				if (info.is_tunnel)
> +					ol_flags |= PKT_TX_TUNNEL_GRE;
>  			} else if (info.l4_proto == IPPROTO_IPIP) {
>  				void *encap_ip_hdr;
> +
>  				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
>  				parse_encap_ip(encap_ip_hdr, &info);
> +				if (info.is_tunnel)
> +					ol_flags |= PKT_TX_TUNNEL_IPIP;
>  			}
>  		}
> 
> @@ -745,7 +762,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>  		 * processed in hardware. */
>  		if (info.is_tunnel == 1) {
>  			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
> -				testpmd_ol_flags);
> +				testpmd_ol_flags, ol_flags & PKT_TX_TCP_SEG);
>  		}
> 
>  		/* step 4: fill the mbuf meta data (flags and header lengths) */ @@ -806,6 +823,10 @@


It was a while since I looked a t it closely, but shouldn't you also update step 4 below:

if (info.is_tunnel == 1) {
                        if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) {
                                m->outer_l2_len = info.outer_l2_len;
                                m->outer_l3_len = info.outer_l3_len;
                                m->l2_len = info.l2_len;
                                m->l3_len = info.l3_len;
                                m->l4_len = info.l4_len;
                        }
                        else {
                                /* if there is a outer UDP cksum
                                   processed in sw and the inner in hw,
                                   the outer checksum will be wrong as
                                   the payload will be modified by the
                                   hardware */
                                m->l2_len = info.outer_l2_len +
                                        info.outer_l3_len + info.l2_len;
                                m->l3_len = info.l3_len;
                                m->l4_len = info.l4_len;
                        }


?

In particular shouldn't it be something like:
if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) != 0 ||
      ((testmpd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) != 0 && info.tso_segsz != 0)) {
....
?

Another thought, might be it is worth to introduce new flag: TESTPMD_TX_OFFLOAD_TSO_TUNNEL,
and new command in cmdline.c, that would set/clear that flag.
Instead of trying to make assumptions does 
user wants tso for tunneled packets based on 2 different things:
- enable/disable tso
- enable/disable tunneled packets parsing
?

Konstantin

> pkt_burst_checksum_forward(struct fwd_stream *fs)
>  				{ PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IPV4 },
>  				{ PKT_TX_OUTER_IPV6, PKT_TX_OUTER_IPV6 },
>  				{ PKT_TX_TCP_SEG, PKT_TX_TCP_SEG },
> +				{ PKT_TX_TUNNEL_VXLAN, PKT_TX_TUNNEL_MASK },
> +				{ PKT_TX_TUNNEL_GRE, PKT_TX_TUNNEL_MASK },
> +				{ PKT_TX_TUNNEL_IPIP, PKT_TX_TUNNEL_MASK },
> +				{ PKT_TX_TUNNEL_GENEVE, PKT_TX_TUNNEL_MASK },
>  			};
>  			unsigned j;
>  			const char *name;
> --
> 2.7.4

Re: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet

[Tan, Jianfeng]

Hi Konstantin,


On 9/19/2016 8:09 PM, Ananyev, Konstantin wrote:
> Hi Jainfeng,
>
>> -----Original Message-----
>> From: Tan, Jianfeng
>> Sent: Monday, August 1, 2016 4:57 AM
>> To: dev@dpdk.org
>> Cc: thomas.monjalon@6wind.com; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>; Tan, Jianfeng
>> <jianfeng.tan@intel.com>; Tao, Zhe <zhe.tao@intel.com>
>> Subject: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet
>>
>> Tx offload on tunneling packet now requires applications to correctly set tunneling type. Without setting it, i40e driver does not parse
>> tunneling parameters. Besides that, add a check to see if NIC supports TSO on tunneling packet when executing "csum parse_tunnel on
>> _port"
>> after "tso set _size _port" or the other way around.
>>
>> Fixes: b51c47536a9e ("app/testpmd: support TSO in checksum forward engine")
>>
>> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>> ---
>>   app/test-pmd/cmdline.c  | 42 ++++++++++++++++++++++++++++++++++++------
>>   app/test-pmd/csumonly.c | 37 +++++++++++++++++++++++++++++--------
>>   2 files changed, 65 insertions(+), 14 deletions(-)
>>
>> [...]
>>
>> @@ -745,7 +762,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>>   		 * processed in hardware. */
>>   		if (info.is_tunnel == 1) {
>>   			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
>> -				testpmd_ol_flags);
>> +				testpmd_ol_flags, ol_flags & PKT_TX_TCP_SEG);
>>   		}
>>
>>   		/* step 4: fill the mbuf meta data (flags and header lengths) */ @@ -806,6 +823,10 @@
>
> It was a while since I looked a t it closely, but shouldn't you also update step 4 below:
>
> if (info.is_tunnel == 1) {
>                          if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) {
>                                  m->outer_l2_len = info.outer_l2_len;
>                                  m->outer_l3_len = info.outer_l3_len;
>                                  m->l2_len = info.l2_len;
>                                  m->l3_len = info.l3_len;
>                                  m->l4_len = info.l4_len;
>                          }
>                          else {
>                                  /* if there is a outer UDP cksum
>                                     processed in sw and the inner in hw,
>                                     the outer checksum will be wrong as
>                                     the payload will be modified by the
>                                     hardware */
>                                  m->l2_len = info.outer_l2_len +
>                                          info.outer_l3_len + info.l2_len;
>                                  m->l3_len = info.l3_len;
>                                  m->l4_len = info.l4_len;
>                          }
>
>
> ?
>
> In particular shouldn't it be something like:
> if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) != 0 ||
>        ((testmpd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) != 0 && info.tso_segsz != 0)) {
> ....
> ?

Sorry for late response, because I also take some time to refresh 
memory. And, you are right, I missed this corner case. After applying 
your way above, it works!

The case below settings in testpmd:
$ set fwd csum
$ csum parse_tunnel on 0
$ tso set 800 0
<keep outer-ip checksum offload is sw>

And unfortunately, our previous verification is based on "outer-ip 
checksum offload is hw".

>
> Another thought, might be it is worth to introduce new flag: TESTPMD_TX_OFFLOAD_TSO_TUNNEL,
> and new command in cmdline.c, that would set/clear that flag.
> Instead of trying to make assumptions does
> user wants tso for tunneled packets based on 2 different things:
> - enable/disable tso
> - enable/disable tunneled packets parsing
> ?

Currently, if we do parse_tunnel is based on the command "csum 
parse_tunnel on/off <port>".
If we add a command like "tso_tunnel set <length> <port>", it's a little 
duplicated with "tso set <length> <port>", and there is too much info to 
just set a flag like TESTPMD_TX_OFFLOAD_TSO_TUNNEL;
If we add a command like "csum tunnel_tso on <port>", it also depends on 
"csum parse_tunnel on <port>" so that tunnel packets are parsed.

As far as I can see, the new command will always have semantic 
overlapping with existing commands, because it indeed depends on the two 
different things.

Thanks,
Jianfeng

>
> Konstantin
>

Re: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet

[Ananyev, Konstantin]

Hi Jianfeng,

> 
> Hi Konstantin,
> 
> 
> On 9/19/2016 8:09 PM, Ananyev, Konstantin wrote:
> > Hi Jainfeng,
> >
> >> -----Original Message-----
> >> From: Tan, Jianfeng
> >> Sent: Monday, August 1, 2016 4:57 AM
> >> To: dev@dpdk.org
> >> Cc: thomas.monjalon@6wind.com; De Lara Guarch, Pablo
> >> <pablo.de.lara.guarch@intel.com>; Ananyev, Konstantin
> >> <konstantin.ananyev@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> >> Zhang, Helin <helin.zhang@intel.com>; Tan, Jianfeng
> >> <jianfeng.tan@intel.com>; Tao, Zhe <zhe.tao@intel.com>
> >> Subject: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling
> >> packet
> >>
> >> Tx offload on tunneling packet now requires applications to correctly
> >> set tunneling type. Without setting it, i40e driver does not parse
> >> tunneling parameters. Besides that, add a check to see if NIC supports TSO on tunneling packet when executing "csum
> parse_tunnel on _port"
> >> after "tso set _size _port" or the other way around.
> >>
> >> Fixes: b51c47536a9e ("app/testpmd: support TSO in checksum forward
> >> engine")
> >>
> >> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> >> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> >> ---
> >>   app/test-pmd/cmdline.c  | 42 ++++++++++++++++++++++++++++++++++++------
> >>   app/test-pmd/csumonly.c | 37 +++++++++++++++++++++++++++++--------
> >>   2 files changed, 65 insertions(+), 14 deletions(-)
> >>
> >> [...]
> >>
> >> @@ -745,7 +762,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> >>   		 * processed in hardware. */
> >>   		if (info.is_tunnel == 1) {
> >>   			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
> >> -				testpmd_ol_flags);
> >> +				testpmd_ol_flags, ol_flags & PKT_TX_TCP_SEG);
> >>   		}
> >>
> >>   		/* step 4: fill the mbuf meta data (flags and header lengths) */
> >> @@ -806,6 +823,10 @@
> >
> > It was a while since I looked a t it closely, but shouldn't you also update step 4 below:
> >
> > if (info.is_tunnel == 1) {
> >                          if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) {
> >                                  m->outer_l2_len = info.outer_l2_len;
> >                                  m->outer_l3_len = info.outer_l3_len;
> >                                  m->l2_len = info.l2_len;
> >                                  m->l3_len = info.l3_len;
> >                                  m->l4_len = info.l4_len;
> >                          }
> >                          else {
> >                                  /* if there is a outer UDP cksum
> >                                     processed in sw and the inner in hw,
> >                                     the outer checksum will be wrong as
> >                                     the payload will be modified by the
> >                                     hardware */
> >                                  m->l2_len = info.outer_l2_len +
> >                                          info.outer_l3_len + info.l2_len;
> >                                  m->l3_len = info.l3_len;
> >                                  m->l4_len = info.l4_len;
> >                          }
> >
> >
> > ?
> >
> > In particular shouldn't it be something like:
> > if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) != 0 ||
> >        ((testmpd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) != 0 &&
> > info.tso_segsz != 0)) { ....
> > ?
> 
> Sorry for late response, because I also take some time to refresh memory. And, you are right, I missed this corner case. After applying
> your way above, it works!
> 
> The case below settings in testpmd:
> $ set fwd csum
> $ csum parse_tunnel on 0
> $ tso set 800 0
> <keep outer-ip checksum offload is sw>

Great :)

> 
> And unfortunately, our previous verification is based on "outer-ip checksum offload is hw".
> 
> >
> > Another thought, might be it is worth to introduce new flag:
> > TESTPMD_TX_OFFLOAD_TSO_TUNNEL, and new command in cmdline.c, that would set/clear that flag.
> > Instead of trying to make assumptions does user wants tso for tunneled
> > packets based on 2 different things:
> > - enable/disable tso
> > - enable/disable tunneled packets parsing ?
> 
> Currently, if we do parse_tunnel is based on the command "csum parse_tunnel on/off <port>".
> If we add a command like "tso_tunnel set <length> <port>", it's a little duplicated with "tso set <length> <port>", and there is too
> much info to just set a flag like TESTPMD_TX_OFFLOAD_TSO_TUNNEL; If we add a command like "csum tunnel_tso on <port>", it also
> depends on "csum parse_tunnel on <port>" so that tunnel packets are parsed.

But I thought in some cases user might want to enable tunnel parsing, but do tso for non-tunneled packets only.
I.E.
 - enable tunnel parsing
- for non-tunneled packets do tso
- for tunneled packets don't do tso
My understanding that with current set commands/flags this is not possible, correct? 
Konstantin

> 
> As far as I can see, the new command will always have semantic overlapping with existing commands, because it indeed depends on
> the two different things.
> 
> Thanks,
> Jianfeng
> 
> >
> > Konstantin
> >

Re: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet

[Tan, Jianfeng]

Hi Konstantin,


On 9/21/2016 11:47 PM, Ananyev, Konstantin wrote:
> Hi Jianfeng,
>
>> Hi Konstantin,
>>
>>
>> On 9/19/2016 8:09 PM, Ananyev, Konstantin wrote:
>>> Hi Jainfeng,
>>>
>>>> -----Original Message-----
>>>> From: Tan, Jianfeng
>>>> Sent: Monday, August 1, 2016 4:57 AM
>>>> To: dev@dpdk.org
>>>> Cc: thomas.monjalon@6wind.com; De Lara Guarch, Pablo
>>>> <pablo.de.lara.guarch@intel.com>; Ananyev, Konstantin
>>>> <konstantin.ananyev@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>;
>>>> Zhang, Helin <helin.zhang@intel.com>; Tan, Jianfeng
>>>> <jianfeng.tan@intel.com>; Tao, Zhe <zhe.tao@intel.com>
>>>> Subject: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling
>>>> packet
>>>>
>>>> Tx offload on tunneling packet now requires applications to correctly
>>>> set tunneling type. Without setting it, i40e driver does not parse
>>>> tunneling parameters. Besides that, add a check to see if NIC supports TSO on tunneling packet when executing "csum
>> parse_tunnel on _port"
>>>> after "tso set _size _port" or the other way around.
>>>>
>>>> Fixes: b51c47536a9e ("app/testpmd: support TSO in checksum forward
>>>> engine")
>>>>
>>>> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
>>>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
>>>> ---
>>>>    app/test-pmd/cmdline.c  | 42 ++++++++++++++++++++++++++++++++++++------
>>>>    app/test-pmd/csumonly.c | 37 +++++++++++++++++++++++++++++--------
>>>>    2 files changed, 65 insertions(+), 14 deletions(-)
>>>>
>>>> [...]
>>>>
>>>> @@ -745,7 +762,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>>>>    		 * processed in hardware. */
>>>>    		if (info.is_tunnel == 1) {
>>>>    			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
>>>> -				testpmd_ol_flags);
>>>> +				testpmd_ol_flags, ol_flags & PKT_TX_TCP_SEG);
>>>>    		}
>>>>
>>>>    		/* step 4: fill the mbuf meta data (flags and header lengths) */
>>>> @@ -806,6 +823,10 @@
>>> It was a while since I looked a t it closely, but shouldn't you also update step 4 below:
>>>
>>> if (info.is_tunnel == 1) {
>>>                           if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) {
>>>                                   m->outer_l2_len = info.outer_l2_len;
>>>                                   m->outer_l3_len = info.outer_l3_len;
>>>                                   m->l2_len = info.l2_len;
>>>                                   m->l3_len = info.l3_len;
>>>                                   m->l4_len = info.l4_len;
>>>                           }
>>>                           else {
>>>                                   /* if there is a outer UDP cksum
>>>                                      processed in sw and the inner in hw,
>>>                                      the outer checksum will be wrong as
>>>                                      the payload will be modified by the
>>>                                      hardware */
>>>                                   m->l2_len = info.outer_l2_len +
>>>                                           info.outer_l3_len + info.l2_len;
>>>                                   m->l3_len = info.l3_len;
>>>                                   m->l4_len = info.l4_len;
>>>                           }
>>>
>>>
>>> ?
>>>
>>> In particular shouldn't it be something like:
>>> if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) != 0 ||
>>>         ((testmpd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) != 0 &&
>>> info.tso_segsz != 0)) { ....
>>> ?
>> Sorry for late response, because I also take some time to refresh memory. And, you are right, I missed this corner case. After applying
>> your way above, it works!
>>
>> The case below settings in testpmd:
>> $ set fwd csum
>> $ csum parse_tunnel on 0
>> $ tso set 800 0
>> <keep outer-ip checksum offload is sw>
> Great :)
>
>> And unfortunately, our previous verification is based on "outer-ip checksum offload is hw".
>>
>>> Another thought, might be it is worth to introduce new flag:
>>> TESTPMD_TX_OFFLOAD_TSO_TUNNEL, and new command in cmdline.c, that would set/clear that flag.
>>> Instead of trying to make assumptions does user wants tso for tunneled
>>> packets based on 2 different things:
>>> - enable/disable tso
>>> - enable/disable tunneled packets parsing ?
>> Currently, if we do parse_tunnel is based on the command "csum parse_tunnel on/off <port>".
>> If we add a command like "tso_tunnel set <length> <port>", it's a little duplicated with "tso set <length> <port>", and there is too
>> much info to just set a flag like TESTPMD_TX_OFFLOAD_TSO_TUNNEL; If we add a command like "csum tunnel_tso on <port>", it also
>> depends on "csum parse_tunnel on <port>" so that tunnel packets are parsed.
> But I thought in some cases user might want to enable tunnel parsing, but do tso for non-tunneled packets only.
> I.E.
>   - enable tunnel parsing
> - for non-tunneled packets do tso
> - for tunneled packets don't do tso
> My understanding that with current set commands/flags this is not possible, correct?
> Konstantin

Yes, correct, above case is not supported now. A twin case would be:
- for non-tunneled packets, don't do tso
- for tunneled packets, do tso

Considering above two cases, so how about adding a command like;
$ tunnel_tso set 800 0
which needs "csum parse_tunnel on 0" has been set before it.

And original "tso set 800 0" will only control tso of non-tunneled packets.
?


Thanks,
Jianfeng

Re: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling packet

[Ananyev, Konstantin]

Hi Jianfeng,

> 
> Hi Konstantin,
> 
> 
> On 9/21/2016 11:47 PM, Ananyev, Konstantin wrote:
> > Hi Jianfeng,
> >
> >> Hi Konstantin,
> >>
> >>
> >> On 9/19/2016 8:09 PM, Ananyev, Konstantin wrote:
> >>> Hi Jainfeng,
> >>>
> >>>> -----Original Message-----
> >>>> From: Tan, Jianfeng
> >>>> Sent: Monday, August 1, 2016 4:57 AM
> >>>> To: dev@dpdk.org
> >>>> Cc: thomas.monjalon@6wind.com; De Lara Guarch, Pablo
> >>>> <pablo.de.lara.guarch@intel.com>; Ananyev, Konstantin
> >>>> <konstantin.ananyev@intel.com>; Wu, Jingjing
> >>>> <jingjing.wu@intel.com>; Zhang, Helin <helin.zhang@intel.com>; Tan,
> >>>> Jianfeng <jianfeng.tan@intel.com>; Tao, Zhe <zhe.tao@intel.com>
> >>>> Subject: [PATCH v4 3/3] app/testpmd: fix Tx offload on tunneling
> >>>> packet
> >>>>
> >>>> Tx offload on tunneling packet now requires applications to
> >>>> correctly set tunneling type. Without setting it, i40e driver does
> >>>> not parse tunneling parameters. Besides that, add a check to see if
> >>>> NIC supports TSO on tunneling packet when executing "csum
> >> parse_tunnel on _port"
> >>>> after "tso set _size _port" or the other way around.
> >>>>
> >>>> Fixes: b51c47536a9e ("app/testpmd: support TSO in checksum forward
> >>>> engine")
> >>>>
> >>>> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> >>>> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> >>>> ---
> >>>>    app/test-pmd/cmdline.c  | 42 ++++++++++++++++++++++++++++++++++++------
> >>>>    app/test-pmd/csumonly.c | 37 +++++++++++++++++++++++++++++--------
> >>>>    2 files changed, 65 insertions(+), 14 deletions(-)
> >>>>
> >>>> [...]
> >>>>
> >>>> @@ -745,7 +762,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
> >>>>    		 * processed in hardware. */
> >>>>    		if (info.is_tunnel == 1) {
> >>>>    			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
> >>>> -				testpmd_ol_flags);
> >>>> +				testpmd_ol_flags, ol_flags & PKT_TX_TCP_SEG);
> >>>>    		}
> >>>>
> >>>>    		/* step 4: fill the mbuf meta data (flags and header lengths)
> >>>> */ @@ -806,6 +823,10 @@
> >>> It was a while since I looked a t it closely, but shouldn't you also update step 4 below:
> >>>
> >>> if (info.is_tunnel == 1) {
> >>>                           if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) {
> >>>                                   m->outer_l2_len = info.outer_l2_len;
> >>>                                   m->outer_l3_len = info.outer_l3_len;
> >>>                                   m->l2_len = info.l2_len;
> >>>                                   m->l3_len = info.l3_len;
> >>>                                   m->l4_len = info.l4_len;
> >>>                           }
> >>>                           else {
> >>>                                   /* if there is a outer UDP cksum
> >>>                                      processed in sw and the inner in hw,
> >>>                                      the outer checksum will be wrong as
> >>>                                      the payload will be modified by the
> >>>                                      hardware */
> >>>                                   m->l2_len = info.outer_l2_len +
> >>>                                           info.outer_l3_len + info.l2_len;
> >>>                                   m->l3_len = info.l3_len;
> >>>                                   m->l4_len = info.l4_len;
> >>>                           }
> >>>
> >>>
> >>> ?
> >>>
> >>> In particular shouldn't it be something like:
> >>> if ((testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) != 0 ||
> >>>         ((testmpd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) != 0
> >>> && info.tso_segsz != 0)) { ....
> >>> ?
> >> Sorry for late response, because I also take some time to refresh
> >> memory. And, you are right, I missed this corner case. After applying your way above, it works!
> >>
> >> The case below settings in testpmd:
> >> $ set fwd csum
> >> $ csum parse_tunnel on 0
> >> $ tso set 800 0
> >> <keep outer-ip checksum offload is sw>
> > Great :)
> >
> >> And unfortunately, our previous verification is based on "outer-ip checksum offload is hw".
> >>
> >>> Another thought, might be it is worth to introduce new flag:
> >>> TESTPMD_TX_OFFLOAD_TSO_TUNNEL, and new command in cmdline.c, that would set/clear that flag.
> >>> Instead of trying to make assumptions does user wants tso for
> >>> tunneled packets based on 2 different things:
> >>> - enable/disable tso
> >>> - enable/disable tunneled packets parsing ?
> >> Currently, if we do parse_tunnel is based on the command "csum parse_tunnel on/off <port>".
> >> If we add a command like "tso_tunnel set <length> <port>", it's a
> >> little duplicated with "tso set <length> <port>", and there is too
> >> much info to just set a flag like TESTPMD_TX_OFFLOAD_TSO_TUNNEL; If we add a command like "csum tunnel_tso on <port>", it
> also depends on "csum parse_tunnel on <port>" so that tunnel packets are parsed.
> > But I thought in some cases user might want to enable tunnel parsing, but do tso for non-tunneled packets only.
> > I.E.
> >   - enable tunnel parsing
> > - for non-tunneled packets do tso
> > - for tunneled packets don't do tso
> > My understanding that with current set commands/flags this is not possible, correct?
> > Konstantin
> 
> Yes, correct, above case is not supported now. A twin case would be:
> - for non-tunneled packets, don't do tso
> - for tunneled packets, do tso

Yep, you right.

> 
> Considering above two cases, so how about adding a command like; $ tunnel_tso set 800 0 which needs "csum parse_tunnel on 0" has
> been set before it.
> 
> And original "tso set 800 0" will only control tso of non-tunneled packets.

Looks good for me.
Konstantin

[PATCH v5 3/3] app/testpmd: support tunneled TSO in csum fwd engine

[Jianfeng Tan]

Add a new command "tunnel_tso set <tso_segsz> <port>" to enable
segmentation offload and set MSS to tso_segsz. Another command,
"tunnel_tso show <port>" is added to show tunneled packet MSS.
Result 0 means tunnel_tso is disabled.

The original commands, "tso set <tso_segsz> <port>" and "tso show
<port>" are only reponsible for non-tunneled packets. And the new
commands are for tunneled packets.

Below conditions are needed to make it work:
  a. tunnel TSO is supported by the NIC;
  b. "csum parse_tunnel" must be set so that tunneled pkts are
     recognized;
  c. for tunneled pkts with outer L3 is IPv4, "csum set outer-ip"
     must be set to hw, because after tso, total_len of outer IP
     header is changed, and the checksum of outer IP header calculated
     by sw should be wrong; that is not necessary for IPv6 tunneled
     pkts because there's no checksum field to be filled anymore.

Suggested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
Signed-off-by: Zhe Tao <zhe.tao@intel.com>
Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
---
v5:
 -- Instead of reuse original tso command, add a new command for
    tunneled tso;
 -- Fix a implicit conversion from long -> int bug, as the parameter
    of process_outer_cksums() in previous version.
 app/test-pmd/cmdline.c  | 132 +++++++++++++++++++++++++++++++++++++++++++++---
 app/test-pmd/csumonly.c |  69 ++++++++++++++++++-------
 app/test-pmd/testpmd.h  |   3 +-
 3 files changed, 179 insertions(+), 25 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 17d238f..a1da8b8 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -3472,7 +3472,7 @@ cmdline_parse_inst_t cmd_csum_tunnel = {
 	},
 };
 
-/* *** ENABLE HARDWARE SEGMENTATION IN TX PACKETS *** */
+/* *** ENABLE HARDWARE SEGMENTATION IN TX NON-TUNNELED PACKETS *** */
 struct cmd_tso_set_result {
 	cmdline_fixed_string_t tso;
 	cmdline_fixed_string_t mode;
@@ -3495,9 +3495,9 @@ cmd_tso_set_parsed(void *parsed_result,
 		ports[res->port_id].tso_segsz = res->tso_segsz;
 
 	if (ports[res->port_id].tso_segsz == 0)
-		printf("TSO is disabled\n");
+		printf("TSO for non-tunneled packets is disabled\n");
 	else
-		printf("TSO segment size is %d\n",
+		printf("TSO segment size for non-tunneled packets is %d\n",
 			ports[res->port_id].tso_segsz);
 
 	/* display warnings if configuration is not supported by the NIC */
@@ -3525,8 +3525,8 @@ cmdline_parse_token_num_t cmd_tso_set_portid =
 cmdline_parse_inst_t cmd_tso_set = {
 	.f = cmd_tso_set_parsed,
 	.data = NULL,
-	.help_str = "Set TSO segment size for csum engine (0 to disable): "
-	"tso set <tso_segsz> <port>",
+	.help_str = "Set TSO segment size of non-tunneled packets "
+	"for csum engine (0 to disable): tso set <tso_segsz> <port>",
 	.tokens = {
 		(void *)&cmd_tso_set_tso,
 		(void *)&cmd_tso_set_mode,
@@ -3544,8 +3544,8 @@ cmdline_parse_token_string_t cmd_tso_show_mode =
 cmdline_parse_inst_t cmd_tso_show = {
 	.f = cmd_tso_set_parsed,
 	.data = NULL,
-	.help_str = "Show TSO segment size for csum engine: "
-	"tso show <port>",
+	.help_str = "Show TSO segment size of non-tunneled packets "
+	"for csum engine: tso show <port>",
 	.tokens = {
 		(void *)&cmd_tso_set_tso,
 		(void *)&cmd_tso_show_mode,
@@ -3554,6 +3554,122 @@ cmdline_parse_inst_t cmd_tso_show = {
 	},
 };
 
+/* *** ENABLE HARDWARE SEGMENTATION IN TX TUNNELED PACKETS *** */
+struct cmd_tunnel_tso_set_result {
+	cmdline_fixed_string_t tso;
+	cmdline_fixed_string_t mode;
+	uint16_t tso_segsz;
+	uint8_t port_id;
+};
+
+static void
+check_tunnel_tso_nic_support(uint8_t port_id)
+{
+	struct rte_eth_dev_info dev_info;
+
+	rte_eth_dev_info_get(port_id, &dev_info);
+	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_VXLAN_TNL_TSO))
+		printf("Warning: TSO enabled but VXLAN TUNNEL TSO not "
+		       "supported by port %d\n", port_id);
+	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_GRE_TNL_TSO))
+		printf("Warning: TSO enabled but GRE TUNNEL TSO not "
+			"supported by port %d\n", port_id);
+	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_IPIP_TNL_TSO))
+		printf("Warning: TSO enabled but IPIP TUNNEL TSO not "
+		       "supported by port %d\n", port_id);
+	if (!(dev_info.tx_offload_capa & DEV_TX_OFFLOAD_GENEVE_TNL_TSO))
+		printf("Warning: TSO enabled but GENEVE TUNNEL TSO not "
+		       "supported by port %d\n", port_id);
+}
+
+static void
+cmd_tunnel_tso_set_parsed(void *parsed_result,
+			  __attribute__((unused)) struct cmdline *cl,
+			  __attribute__((unused)) void *data)
+{
+	struct cmd_tunnel_tso_set_result *res = parsed_result;
+
+	if (port_id_is_invalid(res->port_id, ENABLED_WARN))
+		return;
+
+	if (!strcmp(res->mode, "set"))
+		ports[res->port_id].tunnel_tso_segsz = res->tso_segsz;
+
+	if (ports[res->port_id].tunnel_tso_segsz == 0)
+		printf("TSO for tunneled packets is disabled\n");
+	else {
+		printf("TSO segment size for tunneled packets is %d\n",
+			ports[res->port_id].tunnel_tso_segsz);
+
+		/* Below conditions are needed to make it work:
+		 * (1) tunnel TSO is supported by the NIC;
+		 * (2) "csum parse_tunnel" must be set so that tunneled pkts
+		 * are recognized;
+		 * (3) for tunneled pkts with outer L3 of IPv4,
+		 * "csum set outer-ip" must be set to hw, because after tso,
+		 * total_len of outer IP header is changed, and the checksum
+		 * of outer IP header calculated by sw should be wrong; that
+		 * is not necessary for IPv6 tunneled pkts because there's no
+		 * checksum in IP header anymore.
+		 */
+		check_tunnel_tso_nic_support(res->port_id);
+
+		if (!(ports[res->port_id].tx_ol_flags &
+		      TESTPMD_TX_OFFLOAD_PARSE_TUNNEL))
+			printf("Warning: csum parse_tunnel must be set "
+				"so that tunneled packets are recognized\n");
+		if (!(ports[res->port_id].tx_ol_flags &
+		      TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM))
+			printf("Warning: csum set outer-ip must be set to hw "
+				"if outer L3 is IPv4; not necessary for IPv6\n");
+	}
+}
+
+cmdline_parse_token_string_t cmd_tunnel_tso_set_tso =
+	TOKEN_STRING_INITIALIZER(struct cmd_tunnel_tso_set_result,
+				tso, "tunnel_tso");
+cmdline_parse_token_string_t cmd_tunnel_tso_set_mode =
+	TOKEN_STRING_INITIALIZER(struct cmd_tunnel_tso_set_result,
+				mode, "set");
+cmdline_parse_token_num_t cmd_tunnel_tso_set_tso_segsz =
+	TOKEN_NUM_INITIALIZER(struct cmd_tunnel_tso_set_result,
+				tso_segsz, UINT16);
+cmdline_parse_token_num_t cmd_tunnel_tso_set_portid =
+	TOKEN_NUM_INITIALIZER(struct cmd_tunnel_tso_set_result,
+				port_id, UINT8);
+
+cmdline_parse_inst_t cmd_tunnel_tso_set = {
+	.f = cmd_tunnel_tso_set_parsed,
+	.data = NULL,
+	.help_str = "Set TSO segment size of tunneled packets for csum engine "
+	"(0 to disable): tunnel_tso set <tso_segsz> <port>",
+	.tokens = {
+		(void *)&cmd_tunnel_tso_set_tso,
+		(void *)&cmd_tunnel_tso_set_mode,
+		(void *)&cmd_tunnel_tso_set_tso_segsz,
+		(void *)&cmd_tunnel_tso_set_portid,
+		NULL,
+	},
+};
+
+cmdline_parse_token_string_t cmd_tunnel_tso_show_mode =
+	TOKEN_STRING_INITIALIZER(struct cmd_tunnel_tso_set_result,
+				mode, "show");
+
+
+cmdline_parse_inst_t cmd_tunnel_tso_show = {
+	.f = cmd_tunnel_tso_set_parsed,
+	.data = NULL,
+	.help_str = "Show TSO segment size of tunneled packets "
+	"for csum engine: tunnel_tso show <port>",
+	.tokens = {
+		(void *)&cmd_tunnel_tso_set_tso,
+		(void *)&cmd_tunnel_tso_show_mode,
+		(void *)&cmd_tunnel_tso_set_portid,
+		NULL,
+	},
+};
+
 /* *** ENABLE/DISABLE FLUSH ON RX STREAMS *** */
 struct cmd_set_flush_rx {
 	cmdline_fixed_string_t set;
@@ -10646,6 +10762,8 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_csum_tunnel,
 	(cmdline_parse_inst_t *)&cmd_tso_set,
 	(cmdline_parse_inst_t *)&cmd_tso_show,
+	(cmdline_parse_inst_t *)&cmd_tunnel_tso_set,
+	(cmdline_parse_inst_t *)&cmd_tunnel_tso_show,
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set,
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_rx,
 	(cmdline_parse_inst_t *)&cmd_link_flow_control_set_tx,
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 21cb78f..4fe038d 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -101,6 +101,7 @@ struct testpmd_offload_info {
 	uint16_t outer_l3_len;
 	uint8_t outer_l4_proto;
 	uint16_t tso_segsz;
+	uint16_t tunnel_tso_segsz;
 };
 
 /* simplified GRE header */
@@ -349,7 +350,9 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 		ipv4_hdr->hdr_checksum = 0;
 
 		ol_flags |= PKT_TX_IPV4;
-		if (info->tso_segsz != 0 && info->l4_proto == IPPROTO_TCP) {
+		if (info->l4_proto == IPPROTO_TCP &&
+		    ((info->is_tunnel && info->tunnel_tso_segsz != 0) ||
+		     (!info->is_tunnel && info->tso_segsz != 0))) {
 			ol_flags |= PKT_TX_IP_CKSUM;
 		} else {
 			if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_IP_CKSUM)
@@ -381,7 +384,8 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 	} else if (info->l4_proto == IPPROTO_TCP) {
 		tcp_hdr = (struct tcp_hdr *)((char *)l3_hdr + info->l3_len);
 		tcp_hdr->cksum = 0;
-		if (info->tso_segsz != 0) {
+		if ((info->is_tunnel && info->tunnel_tso_segsz != 0) ||
+		    (!info->is_tunnel && info->tso_segsz != 0)) {
 			ol_flags |= PKT_TX_TCP_SEG;
 			tcp_hdr->cksum = get_psd_sum(l3_hdr, info->ethertype,
 				ol_flags);
@@ -411,12 +415,10 @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info *info,
 	return ol_flags;
 }
 
-/* Calculate the checksum of outer header (only vxlan is supported,
- * meaning IP + UDP). The caller already checked that it's a vxlan
- * packet */
+/* Calculate the checksum of outer header */
 static uint64_t
 process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
-	uint16_t testpmd_ol_flags)
+	uint16_t testpmd_ol_flags, int tso_enabled)
 {
 	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
 	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr;
@@ -437,10 +439,20 @@ process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	if (info->outer_l4_proto != IPPROTO_UDP)
 		return ol_flags;
 
-	/* outer UDP checksum is always done in software as we have no
-	 * hardware supporting it today, and no API for it. */
-
 	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info->outer_l3_len);
+
+	/* outer UDP checksum is done in software as we have no hardware
+	 * supporting it today, and no API for it. In the other side, for
+	 * UDP tunneling, like VXLAN or Geneve, outer UDP checksum can be
+	 * set to zero.
+	 *
+	 * If a packet will be TSOed into small packets by NIC, we cannot
+	 * set/calculate a non-zero checksum, because it will be a wrong
+	 * value after the packet be split into several small packets.
+	 */
+	if (tso_enabled)
+		udp_hdr->dgram_cksum = 0;
+
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
 		udp_hdr->dgram_cksum = 0;
@@ -674,6 +686,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 	testpmd_ol_flags = txp->tx_ol_flags;
 	memset(&info, 0, sizeof(info));
 	info.tso_segsz = txp->tso_segsz;
+	info.tunnel_tso_segsz = txp->tunnel_tso_segsz;
 
 	for (i = 0; i < nb_rx; i++) {
 		if (likely(i < nb_rx - 1))
@@ -703,18 +716,27 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) {
 			if (info.l4_proto == IPPROTO_UDP) {
 				struct udp_hdr *udp_hdr;
+
 				udp_hdr = (struct udp_hdr *)((char *)l3_hdr +
 					info.l3_len);
 				parse_vxlan(udp_hdr, &info, m->packet_type);
+				if (info.is_tunnel)
+					ol_flags |= PKT_TX_TUNNEL_VXLAN;
 			} else if (info.l4_proto == IPPROTO_GRE) {
 				struct simple_gre_hdr *gre_hdr;
+
 				gre_hdr = (struct simple_gre_hdr *)
 					((char *)l3_hdr + info.l3_len);
 				parse_gre(gre_hdr, &info);
+				if (info.is_tunnel)
+					ol_flags |= PKT_TX_TUNNEL_GRE;
 			} else if (info.l4_proto == IPPROTO_IPIP) {
 				void *encap_ip_hdr;
+
 				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
 				parse_encap_ip(encap_ip_hdr, &info);
+				if (info.is_tunnel)
+					ol_flags |= PKT_TX_TUNNEL_IPIP;
 			}
 		}
 
@@ -744,18 +766,21 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		 * processed in hardware. */
 		if (info.is_tunnel == 1) {
 			ol_flags |= process_outer_cksums(outer_l3_hdr, &info,
-				testpmd_ol_flags);
+					testpmd_ol_flags,
+					!!(ol_flags & PKT_TX_TCP_SEG));
 		}
 
 		/* step 4: fill the mbuf meta data (flags and header lengths) */
 
 		if (info.is_tunnel == 1) {
-			if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) {
+			if (info.tunnel_tso_segsz ||
+			    testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) {
 				m->outer_l2_len = info.outer_l2_len;
 				m->outer_l3_len = info.outer_l3_len;
 				m->l2_len = info.l2_len;
 				m->l3_len = info.l3_len;
 				m->l4_len = info.l4_len;
+				m->tso_segsz = info.tunnel_tso_segsz;
 			}
 			else {
 				/* if there is a outer UDP cksum
@@ -775,8 +800,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 			m->l2_len = info.l2_len;
 			m->l3_len = info.l3_len;
 			m->l4_len = info.l4_len;
+			m->tso_segsz = info.tso_segsz;
 		}
-		m->tso_segsz = info.tso_segsz;
 		m->ol_flags = ol_flags;
 
 		/* Do split & copy for the packet. */
@@ -805,6 +830,10 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 				{ PKT_TX_OUTER_IPV4, PKT_TX_OUTER_IPV4 },
 				{ PKT_TX_OUTER_IPV6, PKT_TX_OUTER_IPV6 },
 				{ PKT_TX_TCP_SEG, PKT_TX_TCP_SEG },
+				{ PKT_TX_TUNNEL_VXLAN, PKT_TX_TUNNEL_MASK },
+				{ PKT_TX_TUNNEL_GRE, PKT_TX_TUNNEL_MASK },
+				{ PKT_TX_TUNNEL_IPIP, PKT_TX_TUNNEL_MASK },
+				{ PKT_TX_TUNNEL_GENEVE, PKT_TX_TUNNEL_MASK },
 			};
 			unsigned j;
 			const char *name;
@@ -831,11 +860,17 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 				printf("tx: m->l2_len=%d m->l3_len=%d "
 					"m->l4_len=%d\n",
 					m->l2_len, m->l3_len, m->l4_len);
-			if ((info.is_tunnel == 1) &&
-				(testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM))
-				printf("tx: m->outer_l2_len=%d m->outer_l3_len=%d\n",
-					m->outer_l2_len, m->outer_l3_len);
-			if (info.tso_segsz != 0)
+			if (info.is_tunnel == 1) {
+				if (testpmd_ol_flags &
+				    TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM)
+					printf("tx: m->outer_l2_len=%d "
+						"m->outer_l3_len=%d\n",
+						m->outer_l2_len,
+						m->outer_l3_len);
+				if (info.tunnel_tso_segsz != 0)
+					printf("tx: m->tso_segsz=%d\n",
+						m->tso_segsz);
+			} else if (info.tso_segsz != 0)
 				printf("tx: m->tso_segsz=%d\n", m->tso_segsz);
 			printf("tx: flags=");
 			for (j = 0; j < sizeof(tx_flags)/sizeof(*tx_flags); j++) {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 2b281cc..881d283 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -152,7 +152,8 @@ struct rte_port {
 	struct fwd_stream       *tx_stream; /**< Port TX stream, if unique */
 	unsigned int            socket_id;  /**< For NUMA support */
 	uint16_t                tx_ol_flags;/**< TX Offload Flags (TESTPMD_TX_OFFLOAD...). */
-	uint16_t                tso_segsz;  /**< MSS for segmentation offload. */
+	uint16_t                tso_segsz;  /**< Segmentation offload MSS for non-tunneled packets. */
+	uint16_t                tunnel_tso_segsz; /**< Segmentation offload MSS for tunneled pkts. */
 	uint16_t                tx_vlan_id;/**< The tag ID */
 	uint16_t                tx_vlan_id_outer;/**< The outer tag ID */
 	void                    *fwd_ctx;   /**< Forwarding mode context */
-- 
2.7.4

Re: [PATCH v5 3/3] app/testpmd: support tunneled TSO in csum fwd engine

[Ananyev, Konstantin]

> -----Original Message-----
> From: Tan, Jianfeng
> Sent: Monday, September 26, 2016 2:49 PM
> To: dev@dpdk.org
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Tan, Jianfeng
> <jianfeng.tan@intel.com>; Zhe Tao <zhe.tao@intel.com>
> Subject: [PATCH v5 3/3] app/testpmd: support tunneled TSO in csum fwd engine
> 
> Add a new command "tunnel_tso set <tso_segsz> <port>" to enable
> segmentation offload and set MSS to tso_segsz. Another command,
> "tunnel_tso show <port>" is added to show tunneled packet MSS.
> Result 0 means tunnel_tso is disabled.
> 
> The original commands, "tso set <tso_segsz> <port>" and "tso show
> <port>" are only reponsible for non-tunneled packets. And the new
> commands are for tunneled packets.
> 
> Below conditions are needed to make it work:
>   a. tunnel TSO is supported by the NIC;
>   b. "csum parse_tunnel" must be set so that tunneled pkts are
>      recognized;
>   c. for tunneled pkts with outer L3 is IPv4, "csum set outer-ip"
>      must be set to hw, because after tso, total_len of outer IP
>      header is changed, and the checksum of outer IP header calculated
>      by sw should be wrong; that is not necessary for IPv6 tunneled
>      pkts because there's no checksum field to be filled anymore.
> 
> Suggested-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> ---
> v5:
>  -- Instead of reuse original tso command, add a new command for
>     tunneled tso;
>  -- Fix a implicit conversion from long -> int bug, as the parameter
>     of process_outer_cksums() in previous version.
>  app/test-pmd/cmdline.c  | 132 +++++++++++++++++++++++++++++++++++++++++++++---
>  app/test-pmd/csumonly.c |  69 ++++++++++++++++++-------
>  app/test-pmd/testpmd.h  |   3 +-
>  3 files changed, 179 insertions(+), 25 deletions(-)
> 

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>

Re: [PATCH v4 0/3] Add TSO on tunneling packet

[Ananyev, Konstantin]

> > > -----Original Message-----
> > > From: Tan, Jianfeng
> > > Sent: Monday, August 1, 2016 11:57 AM
> > > To: dev@dpdk.org
> > > Cc: thomas.monjalon@6wind.com; De Lara Guarch, Pablo; Ananyev,
> > > Konstantin; Wu, Jingjing; Zhang, Helin; Tan, Jianfeng; Tao, Zhe
> > > Subject: [PATCH v4 0/3] Add TSO on tunneling packet
> > >
> > > Patch 1: mbuf: add Tx side tunneling type Patch 2: net/i40e: add TSO
> > > support on tunneling packet Patch 3: app/testpmd: fix Tx offload on
> > > tunneling packet
> > >
> > > v4:
> > >   - According to tunnel type flag to parse tunneling parameters.
> > >   - Add new capabilities to indicate support of TSO on tunneling packets.
> > >   - Add check to see if TSO on tunneling packets are supported for the
> > >     specified NIC.
> > >   - Add support for geneve (as i40e does not differentiate UDP tunneling.
> > >   - Split into three patches.
> > >
> > > v3:
> > >   - added external IP offload flag when TSO is enabled for
> > > tunnelling packets
> > > v2:
> > >   - edited the comments
> > >
> > > Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> > > Signed-off-by: Jianfeng Tan <jianfeng.tan@intel.com>
> > >
> > > Jianfeng Tan (3):
> > >   mbuf: add Tx side tunneling type
> > >   net/i40e: add TSO support on tunneling packet
> > >   app/testpmd: fix Tx offload on tunneling packet
> > >
> > >  app/test-pmd/cmdline.c         | 42 +++++++++++++++++---
> > >  app/test-pmd/csumonly.c        | 37 +++++++++++++----
> > >  drivers/net/i40e/i40e_ethdev.c |  6 ++-
> > >  drivers/net/i40e/i40e_rxtx.c   | 90 +++++++++++++++++++++++++++++-----
> > > --------
> > >  lib/librte_ether/rte_ethdev.h  |  4 ++
> > >  lib/librte_mbuf/rte_mbuf.c     |  4 ++
> > >  lib/librte_mbuf/rte_mbuf.h     | 17 +++++++-
> > >  7 files changed, 157 insertions(+), 43 deletions(-)
> > >
> > > --

Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
I think you need to rebase your first one: mbuf: add Tx side tunneling type
against the mainline.
Also 3-rd one is v5 actually.

> > > 2.7.4

Re: [PATCH v4 0/3] Add TSO on tunneling packet

[Tan, Jianfeng]

Hi Konstantin,


On 9/28/2016 1:29 AM, Ananyev, Konstantin wrote:
>
[...]
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> I think you need to rebase your first one: mbuf: add Tx side tunneling type
> against the mainline.

The first one can be applied by 3-way merge, git am -k -3, so I did not 
rebase the first two.

Thomas, shall I do that?

Thanks,
Jianfeng

> Also 3-rd one is v5 actually.
>
>>>> 2.7.4

Re: [PATCH v4 0/3] Add TSO on tunneling packet

[Thomas Monjalon]

2016-09-28 01:52, Tan, Jianfeng:
> Hi Konstantin,
> 
> 
> On 9/28/2016 1:29 AM, Ananyev, Konstantin wrote:
> >
> [...]
> > Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> > I think you need to rebase your first one: mbuf: add Tx side tunneling type
> > against the mainline.
> 
> The first one can be applied by 3-way merge, git am -k -3, so I did not 
> rebase the first two.
> 
> Thomas, shall I do that?

Not mandatory.

Re: [PATCH v4 0/3] Add TSO on tunneling packet

[Thomas Monjalon]

2016-09-27 17:29, Ananyev, Konstantin:
> > > From: Tan, Jianfeng
> > > > Patch 1: mbuf: add Tx side tunneling type Patch 2: net/i40e: add TSO
> > > > support on tunneling packet Patch 3: app/testpmd: fix Tx offload on
> > > > tunneling packet
> 
> Acked-by: Konstantin Ananyev <konstantin.ananyev@intel.com>
> I think you need to rebase your first one: mbuf: add Tx side tunneling type
> against the mainline.
> Also 3-rd one is v5 actually.

Applied (with 3/3 v5), thanks

Re: [PATCH v2] i40: fix the VXLAN TSO issue

[Wu, Jingjing]

NACK.

This fix has been done by a new one which is already merged:  http://dpdk.org/dev/patchwork/patch/15059/


> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Zhe Tao
> Sent: Thursday, July 7, 2016 12:27 PM
> To: dev@dpdk.org
> Cc: zhe.tao@intel.com; Wu, Jingjing <jingjing.wu@intel.com>
> Subject: [dpdk-dev] [PATCH v2] i40: fix the VXLAN TSO issue
> 
> Problem:
> When using the TSO + VXLAN feature in i40e, the outer UDP length fields in
> the multiple UDP segments which are TSOed by the i40e will have the wrong
> value.
> 
> Fix this problem by adding the tunnel type field in the i40e descriptor which
> was missed before.
> 
> Fixes: 77b8301733c3 ("i40e: VXLAN Tx checksum offload")
> 
> Signed-off-by: Zhe Tao <zhe.tao@intel.com>
> ---
> V2: Edited some comments for mbuf structure and i40e driver.
> 
>  app/test-pmd/csumonly.c      | 26 +++++++++++++++++++-------
>  drivers/net/i40e/i40e_rxtx.c | 12 +++++++++---
>  lib/librte_mbuf/rte_mbuf.h   | 16 +++++++++++++++-
>  3 files changed, 43 insertions(+), 11 deletions(-)
> 
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> ac4bd8f..d423c20 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -204,7 +204,8 @@ parse_ethernet(struct ether_hdr *eth_hdr, struct
> testpmd_offload_info *info)  static void  parse_vxlan(struct udp_hdr
> *udp_hdr,
>  	    struct testpmd_offload_info *info,
> -	    uint32_t pkt_type)
> +	    uint32_t pkt_type,
> +	    uint64_t *ol_flags)
>  {
>  	struct ether_hdr *eth_hdr;
> 
> @@ -215,6 +216,7 @@ parse_vxlan(struct udp_hdr *udp_hdr,
>  		RTE_ETH_IS_TUNNEL_PKT(pkt_type) == 0)
>  		return;
> 
> +	*ol_flags |= PKT_TX_TUNNEL_VXLAN;
>  	info->is_tunnel = 1;
>  	info->outer_ethertype = info->ethertype;
>  	info->outer_l2_len = info->l2_len;
> @@ -231,7 +233,9 @@ parse_vxlan(struct udp_hdr *udp_hdr,
> 
>  /* Parse a gre header */
>  static void
> -parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info
> *info)
> +parse_gre(struct simple_gre_hdr *gre_hdr,
> +	  struct testpmd_offload_info *info,
> +	  uint64_t *ol_flags)
>  {
>  	struct ether_hdr *eth_hdr;
>  	struct ipv4_hdr *ipv4_hdr;
> @@ -242,6 +246,8 @@ parse_gre(struct simple_gre_hdr *gre_hdr, struct
> testpmd_offload_info *info)
>  	if ((gre_hdr->flags & _htons(~GRE_SUPPORTED_FIELDS)) != 0)
>  		return;
> 
> +	*ol_flags |= PKT_TX_TUNNEL_GRE;
> +
>  	gre_len += sizeof(struct simple_gre_hdr);
> 
>  	if (gre_hdr->flags & _htons(GRE_KEY_PRESENT)) @@ -417,7 +423,7
> @@ process_inner_cksums(void *l3_hdr, const struct testpmd_offload_info
> *info,
>   * packet */
>  static uint64_t
>  process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info
> *info,
> -	uint16_t testpmd_ol_flags)
> +	uint16_t testpmd_ol_flags, uint64_t orig_ol_flags)
>  {
>  	struct ipv4_hdr *ipv4_hdr = outer_l3_hdr;
>  	struct ipv6_hdr *ipv6_hdr = outer_l3_hdr; @@ -442,6 +448,9 @@
> process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info
> *info,
>  	 * hardware supporting it today, and no API for it. */
> 
>  	udp_hdr = (struct udp_hdr *)((char *)outer_l3_hdr + info-
> >outer_l3_len);
> +	if ((orig_ol_flags & PKT_TX_TCP_SEG) &&
> +	    ((orig_ol_flags & PKT_TX_TUNNEL_MASK) ==
> PKT_TX_TUNNEL_VXLAN))
> +		udp_hdr->dgram_cksum = 0;
>  	/* do not recalculate udp cksum if it was 0 */
>  	if (udp_hdr->dgram_cksum != 0) {
>  		udp_hdr->dgram_cksum = 0;
> @@ -705,15 +714,18 @@ pkt_burst_checksum_forward(struct fwd_stream
> *fs)
>  			if (info.l4_proto == IPPROTO_UDP) {
>  				struct udp_hdr *udp_hdr;
>  				udp_hdr = (struct udp_hdr *)((char *)l3_hdr
> +
> -					info.l3_len);
> -				parse_vxlan(udp_hdr, &info, m-
> >packet_type);
> +					   info.l3_len);
> +				parse_vxlan(udp_hdr, &info, m-
> >packet_type,
> +					    &ol_flags);
>  			} else if (info.l4_proto == IPPROTO_GRE) {
>  				struct simple_gre_hdr *gre_hdr;
>  				gre_hdr = (struct simple_gre_hdr *)
>  					((char *)l3_hdr + info.l3_len);
> -				parse_gre(gre_hdr, &info);
> +				parse_gre(gre_hdr, &info, &ol_flags);
>  			} else if (info.l4_proto == IPPROTO_IPIP) {
>  				void *encap_ip_hdr;
> +
> +				ol_flags |= PKT_TX_TUNNEL_IPIP;
>  				encap_ip_hdr = (char *)l3_hdr + info.l3_len;
>  				parse_encap_ip(encap_ip_hdr, &info);
>  			}
> @@ -745,7 +757,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>  		 * processed in hardware. */
>  		if (info.is_tunnel == 1) {
>  			ol_flags |= process_outer_cksums(outer_l3_hdr,
> &info,
> -				testpmd_ol_flags);
> +				testpmd_ol_flags, ol_flags);
>  		}
> 
>  		/* step 4: fill the mbuf meta data (flags and header lengths)
> */ diff --git a/drivers/net/i40e/i40e_rxtx.c b/drivers/net/i40e/i40e_rxtx.c
> index 049a813..4c987f2 100644
> --- a/drivers/net/i40e/i40e_rxtx.c
> +++ b/drivers/net/i40e/i40e_rxtx.c
> @@ -801,6 +801,12 @@ i40e_txd_enable_checksum(uint64_t ol_flags,
>  			union i40e_tx_offload tx_offload,
>  			uint32_t *cd_tunneling)
>  {
> +	/* Tx pkts tunnel type*/
> +	if ((ol_flags & PKT_TX_TUNNEL_MASK) == PKT_TX_TUNNEL_VXLAN)
> +		*cd_tunneling |= I40E_TXD_CTX_UDP_TUNNELING;
> +	else if ((ol_flags & PKT_TX_TUNNEL_MASK) ==
> PKT_TX_TUNNEL_GRE)
> +		*cd_tunneling |= I40E_TXD_CTX_GRE_TUNNELING;
> +
>  	/* UDP tunneling packet TX checksum offload */
>  	if (ol_flags & PKT_TX_OUTER_IP_CKSUM) {
> 
> @@ -1510,7 +1516,8 @@ i40e_calc_context_desc(uint64_t flags)
> 
>  /* set i40e TSO context descriptor */
>  static inline uint64_t
> -i40e_set_tso_ctx(struct rte_mbuf *mbuf, union i40e_tx_offload tx_offload)
> +i40e_set_tso_ctx(struct rte_mbuf *mbuf,
> +		 union i40e_tx_offload tx_offload)
>  {
>  	uint64_t ctx_desc = 0;
>  	uint32_t cd_cmd, hdr_len, cd_tso_len;
> @@ -1521,7 +1528,7 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union
> i40e_tx_offload tx_offload)
>  	}
> 
>  	/**
> -	 * in case of tunneling packet, the outer_l2_len and
> +	 * in case of non tunneling packet, the outer_l2_len and
>  	 * outer_l3_len must be 0.
>  	 */
>  	hdr_len = tx_offload.outer_l2_len +
> @@ -1537,7 +1544,6 @@ i40e_set_tso_ctx(struct rte_mbuf *mbuf, union
> i40e_tx_offload tx_offload)
>  		 I40E_TXD_CTX_QW1_TSO_LEN_SHIFT) |
>  		((uint64_t)mbuf->tso_segsz <<
>  		 I40E_TXD_CTX_QW1_MSS_SHIFT);
> -
>  	return ctx_desc;
>  }
> 
> diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h index
> 15e3a10..8eb0d33 100644
> --- a/lib/librte_mbuf/rte_mbuf.h
> +++ b/lib/librte_mbuf/rte_mbuf.h
> @@ -133,6 +133,17 @@ extern "C" {
>  /* add new TX flags here */
> 
>  /**
> + * Bits 45:48 used for the tunnel type.
> + * When doing Tx offload like TSO or checksum, the HW needs to
> +configure the
> + * tunnel type into the HW descriptors.
> + */
> +#define PKT_TX_TUNNEL_VXLAN   (1ULL << 45)
> +#define PKT_TX_TUNNEL_GRE   (2ULL << 45)
> +#define PKT_TX_TUNNEL_IPIP    (3ULL << 45)
> +/* add new TX TUNNEL type here */
> +#define PKT_TX_TUNNEL_MASK    (0xFULL << 45)
> +
> +/**
>   * Second VLAN insertion (QinQ) flag.
>   */
>  #define PKT_TX_QINQ_PKT    (1ULL << 49)   /**< TX packet with double
> VLAN inserted. */
> @@ -867,7 +878,10 @@ struct rte_mbuf {
>  	union {
>  		uint64_t tx_offload;       /**< combined for easy fetch */
>  		struct {
> -			uint64_t l2_len:7; /**< L2 (MAC) Header Length. */
> +			/* L2 (MAC) Header Length if it is not a tunneling pkt.
> +			 * for tunnel it is outer L4 len+tunnel len+inner L2 len
> +			 */
> +			uint64_t l2_len:7;
>  			uint64_t l3_len:9; /**< L3 (IP) Header Length. */
>  			uint64_t l4_len:8; /**< L4 (TCP/UDP) Header Length.
> */
>  			uint64_t tso_segsz:16; /**< TCP TSO segment size */
> --
> 2.1.4

Re: [PATCH v2] i40: fix the VXLAN TSO issue

[Yuanhan Liu]

On Mon, Oct 10, 2016 at 03:58:57AM +0000, Wu, Jingjing wrote:
> NACK.
> 
> This fix has been done by a new one which is already merged:  http://dpdk.org/dev/patchwork/patch/15059/

Just want to let you know, that you don't have to NACK an old patchset. In
the process of the patchwork, ideally, the patch author should mark the old
one as "Superseded" once he send out a new version.

Meanwhile, the committers might/could also do the mark while reviewing the
patchwork. I have done that a lot, since I didn't see any one submitted
virtio/vhost patches have done that before :/

	--yliu