diff mbox series

[v4] app/testpmd: enable GTP header parse and Tx checksum offload

Message ID 20191021122913.35281-1-ting.xu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Ferruh Yigit
Headers
Series [v4] app/testpmd: enable GTP header parse and Tx checksum offload |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues
ci/Performance-Testing fail build patch failure
ci/travis-robot warning Travis build: failed

Commit Message

Xu, Ting Oct. 21, 2019, 12:29 p.m. UTC 
  This patch enables testpmd to forward GTP packet in csum fwd mode.
GTP header structure (without optional fields and extension header)
and parser function are added. GTPU and GTPC packets are both
supported, with respective UDP destination port and GTP message
type.

Signed-off-by: Ting Xu <ting.xu@intel.com>

---
v4: Move GTP header defination to rte_ether.h
v3: correct coding style issue.
v2: modify commit log
depend on patch: lib/mbuf: add GTP tunnel type flag.
---
 app/test-pmd/csumonly.c    | 96 ++++++++++++++++++++++++++++++++++----
 lib/librte_net/rte_ether.h | 23 +++++++++
 2 files changed, 109 insertions(+), 10 deletions(-)

Comments

Ferruh Yigit Oct. 21, 2019, 9:28 a.m. UTC | #1 
On 10/21/2019 1:29 PM, Ting Xu wrote:
> This patch enables testpmd to forward GTP packet in csum fwd mode.
> GTP header structure (without optional fields and extension header)
> and parser function are added. GTPU and GTPC packets are both
> supported, with respective UDP destination port and GTP message
> type.
> 
> Signed-off-by: Ting Xu <ting.xu@intel.com>
> 
> ---
> v4: Move GTP header defination to rte_ether.h
> v3: correct coding style issue.
> v2: modify commit log
> depend on patch: lib/mbuf: add GTP tunnel type flag.
> ---
>  app/test-pmd/csumonly.c    | 96 ++++++++++++++++++++++++++++++++++----
>  lib/librte_net/rte_ether.h | 23 +++++++++
>  2 files changed, 109 insertions(+), 10 deletions(-)

<...>

> --- a/lib/librte_net/rte_ether.h
> +++ b/lib/librte_net/rte_ether.h
> @@ -342,6 +342,29 @@ struct rte_vxlan_gpe_hdr {
>  			      sizeof(struct rte_vxlan_gpe_hdr))
>  /**< VXLAN-GPE tunnel header length. */
>  
> +/**
> + * Simplified GTP protocol header.
> + * Contains 8-bit flag, 8-bit message type,
> + * 16-bit message length, 32-bit TEID.
> + * No optional fields and next extension header.
> + */
> +struct rte_gtp_hdr {
> +	uint8_t gtp_hdr_info;
> +	uint8_t msg_type;
> +	uint16_t msg_len;
> +	uint32_t teid;
> +} __attribute__((__packed__));
> +
> +/* GTP header length */
> +#define RTE_ETHER_GTP_HLEN \
> +	(sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
> +/* GTP next protocal type */
> +#define RTE_GTP_TYPE_IPV4 0x40
> +#define RTE_GTP_TYPE_IPV6 0x60
> +/* GTP destination port number */
> +#define RTE_GTPC_UDP_PORT 2123
> +#define RTE_GTPU_UDP_PORT 2152
> +
>  /**
>   * Extract VLAN tag information into mbuf
>   *
> 

lgtm, @Oliver, any comment/objection on GTP part in rte_ether.h?
Ferruh Yigit Oct. 21, 2019, 10:52 a.m. UTC | #2 
On 10/21/2019 10:28 AM, Ferruh Yigit wrote:
> On 10/21/2019 1:29 PM, Ting Xu wrote:
>> This patch enables testpmd to forward GTP packet in csum fwd mode.
>> GTP header structure (without optional fields and extension header)
>> and parser function are added. GTPU and GTPC packets are both
>> supported, with respective UDP destination port and GTP message
>> type.
>>
>> Signed-off-by: Ting Xu <ting.xu@intel.com>
>>
>> ---
>> v4: Move GTP header defination to rte_ether.h
>> v3: correct coding style issue.
>> v2: modify commit log
>> depend on patch: lib/mbuf: add GTP tunnel type flag.
>> ---
>>  app/test-pmd/csumonly.c    | 96 ++++++++++++++++++++++++++++++++++----
>>  lib/librte_net/rte_ether.h | 23 +++++++++
>>  2 files changed, 109 insertions(+), 10 deletions(-)
> 
> <...>
> 
>> --- a/lib/librte_net/rte_ether.h
>> +++ b/lib/librte_net/rte_ether.h
>> @@ -342,6 +342,29 @@ struct rte_vxlan_gpe_hdr {
>>  			      sizeof(struct rte_vxlan_gpe_hdr))
>>  /**< VXLAN-GPE tunnel header length. */
>>  
>> +/**
>> + * Simplified GTP protocol header.
>> + * Contains 8-bit flag, 8-bit message type,
>> + * 16-bit message length, 32-bit TEID.
>> + * No optional fields and next extension header.
>> + */
>> +struct rte_gtp_hdr {
>> +	uint8_t gtp_hdr_info;
>> +	uint8_t msg_type;
>> +	uint16_t msg_len;
>> +	uint32_t teid;
>> +} __attribute__((__packed__));
>> +
>> +/* GTP header length */
>> +#define RTE_ETHER_GTP_HLEN \
>> +	(sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
>> +/* GTP next protocal type */
>> +#define RTE_GTP_TYPE_IPV4 0x40
>> +#define RTE_GTP_TYPE_IPV6 0x60
>> +/* GTP destination port number */
>> +#define RTE_GTPC_UDP_PORT 2123
>> +#define RTE_GTPU_UDP_PORT 2152
>> +
>>  /**
>>   * Extract VLAN tag information into mbuf
>>   *
>>
> 
> lgtm, @Oliver, any comment/objection on GTP part in rte_ether.h?
> 

What about moving the defines to its own header, like rte_gtp? Is these defines
part of the ether?
Olivier Matz Oct. 21, 2019, 4:50 p.m. UTC | #3 
On Mon, Oct 21, 2019 at 11:52:46AM +0100, Ferruh Yigit wrote:
> On 10/21/2019 10:28 AM, Ferruh Yigit wrote:
> > On 10/21/2019 1:29 PM, Ting Xu wrote:
> >> This patch enables testpmd to forward GTP packet in csum fwd mode.
> >> GTP header structure (without optional fields and extension header)
> >> and parser function are added. GTPU and GTPC packets are both
> >> supported, with respective UDP destination port and GTP message
> >> type.
> >>
> >> Signed-off-by: Ting Xu <ting.xu@intel.com>
> >>
> >> ---
> >> v4: Move GTP header defination to rte_ether.h
> >> v3: correct coding style issue.
> >> v2: modify commit log
> >> depend on patch: lib/mbuf: add GTP tunnel type flag.
> >> ---
> >>  app/test-pmd/csumonly.c    | 96 ++++++++++++++++++++++++++++++++++----
> >>  lib/librte_net/rte_ether.h | 23 +++++++++
> >>  2 files changed, 109 insertions(+), 10 deletions(-)
> > 
> > <...>
> > 
> >> --- a/lib/librte_net/rte_ether.h
> >> +++ b/lib/librte_net/rte_ether.h
> >> @@ -342,6 +342,29 @@ struct rte_vxlan_gpe_hdr {
> >>  			      sizeof(struct rte_vxlan_gpe_hdr))
> >>  /**< VXLAN-GPE tunnel header length. */
> >>  
> >> +/**
> >> + * Simplified GTP protocol header.
> >> + * Contains 8-bit flag, 8-bit message type,
> >> + * 16-bit message length, 32-bit TEID.
> >> + * No optional fields and next extension header.
> >> + */
> >> +struct rte_gtp_hdr {
> >> +	uint8_t gtp_hdr_info;
> >> +	uint8_t msg_type;
> >> +	uint16_t msg_len;
> >> +	uint32_t teid;
> >> +} __attribute__((__packed__));
> >> +
> >> +/* GTP header length */
> >> +#define RTE_ETHER_GTP_HLEN \
> >> +	(sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
> >> +/* GTP next protocal type */
> >> +#define RTE_GTP_TYPE_IPV4 0x40
> >> +#define RTE_GTP_TYPE_IPV6 0x60
> >> +/* GTP destination port number */
> >> +#define RTE_GTPC_UDP_PORT 2123
> >> +#define RTE_GTPU_UDP_PORT 2152
> >> +
> >>  /**
> >>   * Extract VLAN tag information into mbuf
> >>   *
> >>
> > 
> > lgtm, @Oliver, any comment/objection on GTP part in rte_ether.h?
> > 
> 
> What about moving the defines to its own header, like rte_gtp? Is these defines
> part of the ether?

+1

I also think it deserves its own file.
Xu, Ting Oct. 22, 2019, 5:34 a.m. UTC | #4 
Hi, Ferruh, Olivier,

Thanks for your advice. I think I misunderstood the comments in the last commit.
This time I move the GTP header definition in the new file rte_gtp.h. Hope it would be better.

Thanks.

-----Original Message-----
From: Olivier Matz [mailto:olivier.matz@6wind.com] 
Sent: Tuesday, October 22, 2019 12:51 AM
To: Yigit, Ferruh <ferruh.yigit@intel.com>
Cc: Xu, Ting <ting.xu@intel.com>; dev@dpdk.org; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Iremonger, Bernard <bernard.iremonger@intel.com>
Subject: Re: [dpdk-dev] [PATCH v4] app/testpmd: enable GTP header parse and Tx checksum offload

On Mon, Oct 21, 2019 at 11:52:46AM +0100, Ferruh Yigit wrote:
> On 10/21/2019 10:28 AM, Ferruh Yigit wrote:
> > On 10/21/2019 1:29 PM, Ting Xu wrote:
> >> This patch enables testpmd to forward GTP packet in csum fwd mode.
> >> GTP header structure (without optional fields and extension header) 
> >> and parser function are added. GTPU and GTPC packets are both 
> >> supported, with respective UDP destination port and GTP message 
> >> type.
> >>
> >> Signed-off-by: Ting Xu <ting.xu@intel.com>
> >>
> >> ---
> >> v4: Move GTP header defination to rte_ether.h
> >> v3: correct coding style issue.
> >> v2: modify commit log
> >> depend on patch: lib/mbuf: add GTP tunnel type flag.
> >> ---
> >>  app/test-pmd/csumonly.c    | 96 ++++++++++++++++++++++++++++++++++----
> >>  lib/librte_net/rte_ether.h | 23 +++++++++
> >>  2 files changed, 109 insertions(+), 10 deletions(-)
> > 
> > <...>
> > 
> >> --- a/lib/librte_net/rte_ether.h
> >> +++ b/lib/librte_net/rte_ether.h
> >> @@ -342,6 +342,29 @@ struct rte_vxlan_gpe_hdr {
> >>  			      sizeof(struct rte_vxlan_gpe_hdr))  /**< VXLAN-GPE tunnel 
> >> header length. */
> >>  
> >> +/**
> >> + * Simplified GTP protocol header.
> >> + * Contains 8-bit flag, 8-bit message type,
> >> + * 16-bit message length, 32-bit TEID.
> >> + * No optional fields and next extension header.
> >> + */
> >> +struct rte_gtp_hdr {
> >> +	uint8_t gtp_hdr_info;
> >> +	uint8_t msg_type;
> >> +	uint16_t msg_len;
> >> +	uint32_t teid;
> >> +} __attribute__((__packed__));
> >> +
> >> +/* GTP header length */
> >> +#define RTE_ETHER_GTP_HLEN \
> >> +	(sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
> >> +/* GTP next protocal type */
> >> +#define RTE_GTP_TYPE_IPV4 0x40
> >> +#define RTE_GTP_TYPE_IPV6 0x60
> >> +/* GTP destination port number */
> >> +#define RTE_GTPC_UDP_PORT 2123
> >> +#define RTE_GTPU_UDP_PORT 2152
> >> +
> >>  /**
> >>   * Extract VLAN tag information into mbuf
> >>   *
> >>
> > 
> > lgtm, @Oliver, any comment/objection on GTP part in rte_ether.h?
> > 
> 
> What about moving the defines to its own header, like rte_gtp? Is 
> these defines part of the ether?

+1

I also think it deserves its own file.
diff mbox series

Patch

diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index e1cb7fb70..52adfffe1 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -179,6 +179,67 @@  parse_ethernet(struct rte_ether_hdr *eth_hdr, struct testpmd_offload_info *info)
 	}
 }
 
+/*
+ * Parse a GTP protocol header.
+ * No optional fields and next extension header type.
+ */
+static void
+parse_gtp(struct rte_udp_hdr *udp_hdr,
+	  struct testpmd_offload_info *info)
+{
+	struct rte_ipv4_hdr *ipv4_hdr;
+	struct rte_ipv6_hdr *ipv6_hdr;
+	struct rte_gtp_hdr *gtp_hdr;
+	uint8_t gtp_len = sizeof(*gtp_hdr);
+	uint8_t ip_ver;
+
+	/* Check udp destination port. */
+	if (udp_hdr->dst_port != _htons(RTE_GTPC_UDP_PORT) &&
+	    udp_hdr->src_port != _htons(RTE_GTPC_UDP_PORT) &&
+	    udp_hdr->dst_port != _htons(RTE_GTPU_UDP_PORT))
+		return;
+
+	info->is_tunnel = 1;
+	info->outer_ethertype = info->ethertype;
+	info->outer_l2_len = info->l2_len;
+	info->outer_l3_len = info->l3_len;
+	info->outer_l4_proto = info->l4_proto;
+	info->l2_len = 0;
+
+	gtp_hdr = (struct rte_gtp_hdr *)((char *)udp_hdr +
+		  sizeof(struct rte_udp_hdr));
+
+	/*
+	 * Check message type. If message type is 0xff, it is
+	 * a GTP data packet. If not, it is a GTP control packet
+	 */
+	if (gtp_hdr->msg_type == 0xff) {
+		ip_ver = *(uint8_t *)((char *)udp_hdr +
+			 sizeof(struct rte_udp_hdr) +
+			 sizeof(struct rte_gtp_hdr));
+		ip_ver = (ip_ver) & 0xf0;
+
+		if (ip_ver == RTE_GTP_TYPE_IPV4) {
+			ipv4_hdr = (struct rte_ipv4_hdr *)((char *)gtp_hdr +
+				   gtp_len);
+			info->ethertype = _htons(RTE_ETHER_TYPE_IPV4);
+			parse_ipv4(ipv4_hdr, info);
+		} else if (ip_ver == RTE_GTP_TYPE_IPV6) {
+			ipv6_hdr = (struct rte_ipv6_hdr *)((char *)gtp_hdr +
+				   gtp_len);
+			info->ethertype = _htons(RTE_ETHER_TYPE_IPV6);
+			parse_ipv6(ipv6_hdr, info);
+		}
+	} else {
+		info->ethertype = 0;
+		info->l4_len = 0;
+		info->l3_len = 0;
+		info->l4_proto = 0;
+	}
+
+	info->l2_len += RTE_ETHER_GTP_HLEN;
+}
+
 /* Parse a vxlan header */
 static void
 parse_vxlan(struct rte_udp_hdr *udp_hdr,
@@ -478,15 +539,22 @@  process_outer_cksums(void *outer_l3_hdr, struct testpmd_offload_info *info,
 	if (info->outer_l4_proto != IPPROTO_UDP)
 		return ol_flags;
 
+	udp_hdr = (struct rte_udp_hdr *)
+		((char *)outer_l3_hdr + info->outer_l3_len);
+
 	/* Skip SW outer UDP checksum generation if HW supports it */
 	if (tx_offloads & DEV_TX_OFFLOAD_OUTER_UDP_CKSUM) {
+		if (info->outer_ethertype == _htons(RTE_ETHER_TYPE_IPV4))
+			udp_hdr->dgram_cksum
+				= rte_ipv4_phdr_cksum(ipv4_hdr, ol_flags);
+		else
+			udp_hdr->dgram_cksum
+				= rte_ipv6_phdr_cksum(ipv6_hdr, ol_flags);
+
 		ol_flags |= PKT_TX_OUTER_UDP_CKSUM;
 		return ol_flags;
 	}
 
-	udp_hdr = (struct rte_udp_hdr *)
-		((char *)outer_l3_hdr + info->outer_l3_len);
-
 	/* outer UDP checksum is done in software. In the other side, for
 	 * UDP tunneling, like VXLAN or Geneve, outer UDP checksum can be
 	 * set to zero.
@@ -679,6 +747,7 @@  pkt_copy_split(const struct rte_mbuf *pkt)
  *           UDP|TCP|SCTP
  *   Ether / (vlan) / outer IP|IP6 / outer UDP / VXLAN-GPE / IP|IP6 /
  *           UDP|TCP|SCTP
+ *   Ether / (vlan) / outer IP / outer UDP / GTP / IP|IP6 / UDP|TCP|SCTP
  *   Ether / (vlan) / outer IP|IP6 / GRE / Ether / IP|IP6 / UDP|TCP|SCTP
  *   Ether / (vlan) / outer IP|IP6 / GRE / IP|IP6 / UDP|TCP|SCTP
  *   Ether / (vlan) / outer IP|IP6 / IP|IP6 / UDP|TCP|SCTP
@@ -787,16 +856,22 @@  pkt_burst_checksum_forward(struct fwd_stream *fs)
 
 				udp_hdr = (struct rte_udp_hdr *)
 					((char *)l3_hdr + info.l3_len);
+				parse_gtp(udp_hdr, &info);
+				if (info.is_tunnel) {
+					tx_ol_flags |= PKT_TX_TUNNEL_GTP;
+					goto tunnel_update;
+				}
 				parse_vxlan_gpe(udp_hdr, &info);
 				if (info.is_tunnel) {
-					tx_ol_flags |= PKT_TX_TUNNEL_VXLAN_GPE;
-				} else {
-					parse_vxlan(udp_hdr, &info,
-						    m->packet_type);
-					if (info.is_tunnel)
-						tx_ol_flags |=
-							PKT_TX_TUNNEL_VXLAN;
+					tx_ol_flags |=
+						PKT_TX_TUNNEL_VXLAN_GPE;
+					goto tunnel_update;
 				}
+				parse_vxlan(udp_hdr, &info,
+					    m->packet_type);
+				if (info.is_tunnel)
+					tx_ol_flags |=
+						PKT_TX_TUNNEL_VXLAN;
 			} else if (info.l4_proto == IPPROTO_GRE) {
 				struct simple_gre_hdr *gre_hdr;
 
@@ -815,6 +890,7 @@  pkt_burst_checksum_forward(struct fwd_stream *fs)
 			}
 		}
 
+tunnel_update:
 		/* update l3_hdr and outer_l3_hdr if a tunnel was parsed */
 		if (info.is_tunnel) {
 			outer_l3_hdr = l3_hdr;
diff --git a/lib/librte_net/rte_ether.h b/lib/librte_net/rte_ether.h
index aa6eff037..33ad347d8 100644
--- a/lib/librte_net/rte_ether.h
+++ b/lib/librte_net/rte_ether.h
@@ -342,6 +342,29 @@  struct rte_vxlan_gpe_hdr {
 			      sizeof(struct rte_vxlan_gpe_hdr))
 /**< VXLAN-GPE tunnel header length. */
 
+/**
+ * Simplified GTP protocol header.
+ * Contains 8-bit flag, 8-bit message type,
+ * 16-bit message length, 32-bit TEID.
+ * No optional fields and next extension header.
+ */
+struct rte_gtp_hdr {
+	uint8_t gtp_hdr_info;
+	uint8_t msg_type;
+	uint16_t msg_len;
+	uint32_t teid;
+} __attribute__((__packed__));
+
+/* GTP header length */
+#define RTE_ETHER_GTP_HLEN \
+	(sizeof(struct rte_udp_hdr) + sizeof(struct rte_gtp_hdr))
+/* GTP next protocal type */
+#define RTE_GTP_TYPE_IPV4 0x40
+#define RTE_GTP_TYPE_IPV6 0x60
+/* GTP destination port number */
+#define RTE_GTPC_UDP_PORT 2123
+#define RTE_GTPU_UDP_PORT 2152
+
 /**
  * Extract VLAN tag information into mbuf
  *