[dpdk-dev,RFC,11/16] testpmd: rename vxlan in outer_ip in csum commands

Message ID 1421883395-27235-12-git-send-email-olivier.matz@6wind.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Olivier Matz Jan. 21, 2015, 11:36 p.m. UTC
The tx_checksum command concerns outer IP checksum, not VxLAN checksum.
Actually there is no checkum in VxLAN header, there is one checksum in
outer IP header, and one checksum in outer UDP header. This option only
controls the outer IP checksum.

Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
---
 app/test-pmd/cmdline.c  | 16 ++++++++--------
 app/test-pmd/csumonly.c | 25 ++++++++++++++-----------
 app/test-pmd/testpmd.h  |  4 ++--
 3 files changed, 24 insertions(+), 21 deletions(-)
  

Comments

Jijiang Liu Jan. 23, 2015, 11:21 a.m. UTC | #1
Hi,

> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz@6wind.com]
> Sent: Thursday, January 22, 2015 7:37 AM
> To: dev@dpdk.org
> Cc: olivier.matz@6wind.com; Ananyev, Konstantin; Liu, Jijiang
> Subject: [RFC 11/16] testpmd: rename vxlan in outer_ip in csum commands
> 
> The tx_checksum command concerns outer IP checksum, not VxLAN checksum.
> Actually there is no checkum in VxLAN header, there is one checksum in outer IP
> header, and one checksum in outer UDP header. This option only controls the
> outer IP checksum.
> 
> Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> ---
>  app/test-pmd/cmdline.c  | 16 ++++++++--------  app/test-pmd/csumonly.c | 25
> ++++++++++++++-----------  app/test-pmd/testpmd.h  |  4 ++--
>  3 files changed, 24 insertions(+), 21 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 1d294bc..451c728 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -316,12 +316,12 @@ static void cmd_help_long_parsed(void
> *parsed_result,
>  			"    Disable hardware insertion of a VLAN header in"
>  			" packets sent on a port.\n\n"
> 
> -			"csum set (ip|udp|tcp|sctp|vxlan) (hw|sw) (port_id)\n"
> +			"csum set (ip|udp|tcp|sctp|outer-ip) (hw|sw)
> (port_id)\n"
>  			"    Select hardware or software calculation of the"
>  			" checksum with when transmitting a packet using the"
>  			" csum forward engine.\n"
>  			"    ip|udp|tcp|sctp always concern the inner layer.\n"
> -			"    vxlan concerns the outer IP and UDP layer (in"
> +			"    outer-ip concerns the outer IP layer (in"
>  			" case the packet is recognized as a vxlan packet by"
>  			" the forward engine)\n"
>  			"    Please check the NIC datasheet for HW limits.\n\n"
> @@ -2887,8 +2887,8 @@ csum_show(int port_id)
>  		(ol_flags & TESTPMD_TX_OFFLOAD_TCP_CKSUM) ? "hw" : "sw");
>  	printf("SCTP checksum offload is %s\n",
>  		(ol_flags & TESTPMD_TX_OFFLOAD_SCTP_CKSUM) ? "hw" :
> "sw");
> -	printf("VxLAN checksum offload is %s\n",
> -		(ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) ? "hw" :
> "sw");
> +	printf("Outer-Ip checksum offload is %s\n",
> +		(ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) ? "hw" :
> "sw");
> 
>  	/* display warnings if configuration is not supported by the NIC */
>  	rte_eth_dev_info_get(port_id, &dev_info); @@ -2942,8 +2942,8 @@
> cmd_csum_parsed(void *parsed_result,
>  			mask = TESTPMD_TX_OFFLOAD_TCP_CKSUM;
>  		} else if (!strcmp(res->proto, "sctp")) {
>  			mask = TESTPMD_TX_OFFLOAD_SCTP_CKSUM;
> -		} else if (!strcmp(res->proto, "vxlan")) {
> -			mask = TESTPMD_TX_OFFLOAD_VXLAN_CKSUM;
> +		} else if (!strcmp(res->proto, "outer-ip")) {
> +			mask = TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM;
>  		}
> 
>  		if (hw)
> @@ -2962,7 +2962,7 @@ cmdline_parse_token_string_t cmd_csum_mode =
>  				mode, "set");
>  cmdline_parse_token_string_t cmd_csum_proto =
>  	TOKEN_STRING_INITIALIZER(struct cmd_csum_result,
> -				proto, "ip#tcp#udp#sctp#vxlan");
> +				proto, "ip#tcp#udp#sctp#outer-ip");
>  cmdline_parse_token_string_t cmd_csum_hwsw =
>  	TOKEN_STRING_INITIALIZER(struct cmd_csum_result,
>  				hwsw, "hw#sw");
> @@ -2974,7 +2974,7 @@ cmdline_parse_inst_t cmd_csum_set = {
>  	.f = cmd_csum_parsed,
>  	.data = NULL,
>  	.help_str = "enable/disable hardware calculation of L3/L4 checksum
> when "
> -		"using csum forward engine: csum set ip|tcp|udp|sctp|vxlan
> hw|sw <port>",
> +		"using csum forward engine: csum set ip|tcp|udp|sctp|outer-ip
> hw|sw
> +<port>",
>  	.tokens = {
>  		(void *)&cmd_csum_csum,
>  		(void *)&cmd_csum_mode,
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> 858eb47..3921643 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -259,13 +259,16 @@ process_outer_cksums(void *outer_l3_hdr, uint16_t
> outer_ethertype,
>  		ipv4_hdr->hdr_checksum = 0;
>  		ol_flags |= PKT_TX_OUTER_IPV4;
> 
> -		if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM)
> +		if (testpmd_ol_flags &
> TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM)
>  			ol_flags |= PKT_TX_OUTER_IP_CKSUM;
>  		else
>  			ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
> -	} else if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM)
> +	} else if (testpmd_ol_flags &
> TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM)
>  		ol_flags |= PKT_TX_OUTER_IPV6;
> 
> +	/* 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 + outer_l3_len);
>  	/* do not recalculate udp cksum if it was 0 */
>  	if (udp_hdr->dgram_cksum != 0) {
> @@ -300,8 +303,8 @@ process_outer_cksums(void *outer_l3_hdr, uint16_t
> outer_ethertype,
>   * The testpmd command line for this forward engine sets the flags
>   * TESTPMD_TX_OFFLOAD_* in ports[tx_port].tx_ol_flags. They control
>   * wether a checksum must be calculated in software or in hardware. The
> - * IP, UDP, TCP and SCTP flags always concern the inner layer.  The
> - * VxLAN flag concerns the outer IP (if packet is recognized as a vxlan packet).
> + * IP, UDP, TCP and SCTP flags always concern the inner layer. The
> + * OUTER_IP is only useful for tunnel packets.
>   */
>  static void
>  pkt_burst_checksum_forward(struct fwd_stream *fs) @@ -432,18 +435,18 @@
> pkt_burst_checksum_forward(struct fwd_stream *fs)
>  		/* step 4: fill the mbuf meta data (flags and header lengths) */
> 
>  		if (tunnel == 1) {
> -			if (testpmd_ol_flags &
> TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) {
> +			if (testpmd_ol_flags &
> TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) {
>  				m->outer_l2_len = outer_l2_len;
>  				m->outer_l3_len = outer_l3_len;
>  				m->l2_len = l4_tun_len + l2_len;
>  				m->l3_len = l3_len;

There should be m->l4_len = l4_len here.

>  			}
>  			else {
> -				/* if we don't do vxlan cksum in hw,
> -				   outer checksum will be wrong because
> -				   we changed the ip, but it shows that
> -				   we can process the inner header cksum
> -				   in the nic */
> +				/* 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 = outer_l2_len + outer_l3_len +
>  					sizeof(struct udp_hdr) +
>  					sizeof(struct vxlan_hdr) + l2_len; @@ -
> 502,7 +505,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>  					"m->l4_len=%d\n",
>  					m->l2_len, m->l3_len, m->l4_len);
>  			if ((tunnel == 1) &&
> -				(testpmd_ol_flags &
> TESTPMD_TX_OFFLOAD_VXLAN_CKSUM))
> +				(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 (tso_segsz != 0)
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h index
> 36277de..ecb7d38 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -125,8 +125,8 @@ struct fwd_stream {
>  #define TESTPMD_TX_OFFLOAD_TCP_CKSUM         0x0004
>  /** Offload SCTP checksum in csum forward engine */
>  #define TESTPMD_TX_OFFLOAD_SCTP_CKSUM        0x0008
> -/** Offload VxLAN checksum in csum forward engine */
> -#define TESTPMD_TX_OFFLOAD_VXLAN_CKSUM       0x0010
> +/** Offload outer IP checksum in csum forward engine for recognized tunnels
> */
> +#define TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM    0x0010
>  /** Parse tunnel in csum forward engine. If set, dissect tunnel headers
>   * of rx packets. If not set, treat inner headers as payload. */
>  #define TESTPMD_TX_OFFLOAD_PARSE_TUNNEL      0x0020
> --
> 2.1.3
  
Olivier Matz Jan. 23, 2015, 5:49 p.m. UTC | #2
Hi Jijiang,

On 01/23/2015 12:21 PM, Liu, Jijiang wrote:
>>  static void
>>  pkt_burst_checksum_forward(struct fwd_stream *fs) @@ -432,18 +435,18 @@
>> pkt_burst_checksum_forward(struct fwd_stream *fs)
>>  		/* step 4: fill the mbuf meta data (flags and header lengths) */
>>
>>  		if (tunnel == 1) {
>> -			if (testpmd_ol_flags &
>> TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) {
>> +			if (testpmd_ol_flags &
>> TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) {
>>  				m->outer_l2_len = outer_l2_len;
>>  				m->outer_l3_len = outer_l3_len;
>>  				m->l2_len = l4_tun_len + l2_len;
>>  				m->l3_len = l3_len;
> 
> There should be m->l4_len = l4_len here.

Right, thank you for reporting it.

I'll fix that (but not in this patch which is just a renaming
of another part of the code).


Regards,
Olivier
  

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 1d294bc..451c728 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -316,12 +316,12 @@  static void cmd_help_long_parsed(void *parsed_result,
 			"    Disable hardware insertion of a VLAN header in"
 			" packets sent on a port.\n\n"
 
-			"csum set (ip|udp|tcp|sctp|vxlan) (hw|sw) (port_id)\n"
+			"csum set (ip|udp|tcp|sctp|outer-ip) (hw|sw) (port_id)\n"
 			"    Select hardware or software calculation of the"
 			" checksum with when transmitting a packet using the"
 			" csum forward engine.\n"
 			"    ip|udp|tcp|sctp always concern the inner layer.\n"
-			"    vxlan concerns the outer IP and UDP layer (in"
+			"    outer-ip concerns the outer IP layer (in"
 			" case the packet is recognized as a vxlan packet by"
 			" the forward engine)\n"
 			"    Please check the NIC datasheet for HW limits.\n\n"
@@ -2887,8 +2887,8 @@  csum_show(int port_id)
 		(ol_flags & TESTPMD_TX_OFFLOAD_TCP_CKSUM) ? "hw" : "sw");
 	printf("SCTP checksum offload is %s\n",
 		(ol_flags & TESTPMD_TX_OFFLOAD_SCTP_CKSUM) ? "hw" : "sw");
-	printf("VxLAN checksum offload is %s\n",
-		(ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) ? "hw" : "sw");
+	printf("Outer-Ip checksum offload is %s\n",
+		(ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) ? "hw" : "sw");
 
 	/* display warnings if configuration is not supported by the NIC */
 	rte_eth_dev_info_get(port_id, &dev_info);
@@ -2942,8 +2942,8 @@  cmd_csum_parsed(void *parsed_result,
 			mask = TESTPMD_TX_OFFLOAD_TCP_CKSUM;
 		} else if (!strcmp(res->proto, "sctp")) {
 			mask = TESTPMD_TX_OFFLOAD_SCTP_CKSUM;
-		} else if (!strcmp(res->proto, "vxlan")) {
-			mask = TESTPMD_TX_OFFLOAD_VXLAN_CKSUM;
+		} else if (!strcmp(res->proto, "outer-ip")) {
+			mask = TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM;
 		}
 
 		if (hw)
@@ -2962,7 +2962,7 @@  cmdline_parse_token_string_t cmd_csum_mode =
 				mode, "set");
 cmdline_parse_token_string_t cmd_csum_proto =
 	TOKEN_STRING_INITIALIZER(struct cmd_csum_result,
-				proto, "ip#tcp#udp#sctp#vxlan");
+				proto, "ip#tcp#udp#sctp#outer-ip");
 cmdline_parse_token_string_t cmd_csum_hwsw =
 	TOKEN_STRING_INITIALIZER(struct cmd_csum_result,
 				hwsw, "hw#sw");
@@ -2974,7 +2974,7 @@  cmdline_parse_inst_t cmd_csum_set = {
 	.f = cmd_csum_parsed,
 	.data = NULL,
 	.help_str = "enable/disable hardware calculation of L3/L4 checksum when "
-		"using csum forward engine: csum set ip|tcp|udp|sctp|vxlan hw|sw <port>",
+		"using csum forward engine: csum set ip|tcp|udp|sctp|outer-ip hw|sw <port>",
 	.tokens = {
 		(void *)&cmd_csum_csum,
 		(void *)&cmd_csum_mode,
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index 858eb47..3921643 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -259,13 +259,16 @@  process_outer_cksums(void *outer_l3_hdr, uint16_t outer_ethertype,
 		ipv4_hdr->hdr_checksum = 0;
 		ol_flags |= PKT_TX_OUTER_IPV4;
 
-		if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM)
+		if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM)
 			ol_flags |= PKT_TX_OUTER_IP_CKSUM;
 		else
 			ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
-	} else if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM)
+	} else if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM)
 		ol_flags |= PKT_TX_OUTER_IPV6;
 
+	/* 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 + outer_l3_len);
 	/* do not recalculate udp cksum if it was 0 */
 	if (udp_hdr->dgram_cksum != 0) {
@@ -300,8 +303,8 @@  process_outer_cksums(void *outer_l3_hdr, uint16_t outer_ethertype,
  * The testpmd command line for this forward engine sets the flags
  * TESTPMD_TX_OFFLOAD_* in ports[tx_port].tx_ol_flags. They control
  * wether a checksum must be calculated in software or in hardware. The
- * IP, UDP, TCP and SCTP flags always concern the inner layer.  The
- * VxLAN flag concerns the outer IP (if packet is recognized as a vxlan packet).
+ * IP, UDP, TCP and SCTP flags always concern the inner layer. The
+ * OUTER_IP is only useful for tunnel packets.
  */
 static void
 pkt_burst_checksum_forward(struct fwd_stream *fs)
@@ -432,18 +435,18 @@  pkt_burst_checksum_forward(struct fwd_stream *fs)
 		/* step 4: fill the mbuf meta data (flags and header lengths) */
 
 		if (tunnel == 1) {
-			if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM) {
+			if (testpmd_ol_flags & TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM) {
 				m->outer_l2_len = outer_l2_len;
 				m->outer_l3_len = outer_l3_len;
 				m->l2_len = l4_tun_len + l2_len;
 				m->l3_len = l3_len;
 			}
 			else {
-				/* if we don't do vxlan cksum in hw,
-				   outer checksum will be wrong because
-				   we changed the ip, but it shows that
-				   we can process the inner header cksum
-				   in the nic */
+				/* 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 = outer_l2_len + outer_l3_len +
 					sizeof(struct udp_hdr) +
 					sizeof(struct vxlan_hdr) + l2_len;
@@ -502,7 +505,7 @@  pkt_burst_checksum_forward(struct fwd_stream *fs)
 					"m->l4_len=%d\n",
 					m->l2_len, m->l3_len, m->l4_len);
 			if ((tunnel == 1) &&
-				(testpmd_ol_flags & TESTPMD_TX_OFFLOAD_VXLAN_CKSUM))
+				(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 (tso_segsz != 0)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 36277de..ecb7d38 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -125,8 +125,8 @@  struct fwd_stream {
 #define TESTPMD_TX_OFFLOAD_TCP_CKSUM         0x0004
 /** Offload SCTP checksum in csum forward engine */
 #define TESTPMD_TX_OFFLOAD_SCTP_CKSUM        0x0008
-/** Offload VxLAN checksum in csum forward engine */
-#define TESTPMD_TX_OFFLOAD_VXLAN_CKSUM       0x0010
+/** Offload outer IP checksum in csum forward engine for recognized tunnels */
+#define TESTPMD_TX_OFFLOAD_OUTER_IP_CKSUM    0x0010
 /** Parse tunnel in csum forward engine. If set, dissect tunnel headers
  * of rx packets. If not set, treat inner headers as payload. */
 #define TESTPMD_TX_OFFLOAD_PARSE_TUNNEL      0x0020