[dpdk-dev] [PATCH 13/20] testpmd: support gre tunnels in csum fwd engine

[Liu, Jijiang]

Hi Olivier,


> -----Original Message-----
> From: Olivier Matz [mailto:olivier.matz at 6wind.com]
> Sent: Friday, January 30, 2015 9:16 PM
> To: dev at dpdk.org
> Cc: Ananyev, Konstantin; Liu, Jijiang; Zhang, Helin; olivier.matz at 6wind.com
> Subject: [PATCH 13/20] testpmd: support gre tunnels in csum fwd engine
> 
> Add support for Ethernet over GRE and IP over GRE tunnels.
> 
> Signed-off-by: Olivier Matz <olivier.matz at 6wind.com>
> ---
>  app/test-pmd/cmdline.c  |  6 ++--
>  app/test-pmd/csumonly.c | 87
> +++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 84 insertions(+), 9 deletions(-)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c index
> 98f7a1c..aa5c178 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -321,9 +321,9 @@ static void cmd_help_long_parsed(void
> *parsed_result,
>  			" checksum with when transmitting a packet using
> the"
>  			" csum forward engine.\n"
>  			"    ip|udp|tcp|sctp always concern the inner
> layer.\n"
> -			"    outer-ip concerns the outer IP layer (in"
> -			" case the packet is recognized as a vxlan packet by"
> -			" the forward engine)\n"
> +			"    outer-ip concerns the outer IP layer in"
> +			" case the packet is recognized as a tunnel packet by"
> +			" the forward engine (vxlan and gre are
> supported)\n"
>  			"    Please check the NIC datasheet for HW
> limits.\n\n"
> 
>  			"csum parse-tunnel (on|off) (tx_port_id)\n"
> diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c index
> 52af0e7..02c01f6 100644
> --- a/app/test-pmd/csumonly.c
> +++ b/app/test-pmd/csumonly.c
> @@ -100,6 +100,12 @@ struct testpmd_offload_info {
>  	uint16_t tso_segsz;
>  };
> 
> +/* simplified GRE header (flags must be 0) */ struct simple_gre_hdr {
> +	uint16_t flags;
> +	uint16_t proto;
> +};
I suggest you to remove the comment ' flags must be 0'，the reason is that we just use the header structure to check what the protocol is.
It is not necessary to require the flag must be 0.

>  static uint16_t
>  get_psd_sum(void *l3_hdr, uint16_t ethertype, uint64_t ol_flags)  { @@ -
> 218,6 +224,60 @@ parse_vxlan(struct udp_hdr *udp_hdr, struct
> testpmd_offload_info *info,
>  	info->l2_len += ETHER_VXLAN_HLEN; /* add udp + vxlan */  }
> 
> +/* Parse a gre header */
> +static void
> +parse_gre(struct simple_gre_hdr *gre_hdr, struct testpmd_offload_info
> +*info) {
> +	struct ether_hdr *eth_hdr;
> +	struct ipv4_hdr *ipv4_hdr;
> +	struct ipv6_hdr *ipv6_hdr;
> +
> +	/* if flags != 0; it's not supported */
> +	if (gre_hdr->flags != 0)
> +		return;
I suggest you remove the check here, you can add some comments in front of this function to explain that actual NVGRE and IP over GRE is not supported now.

For example, when I want to support NVGRE TX checksum offload, I will do the following change.

Or you still keep it here, but anyway, I will change it later.


> +
> +	if (gre_hdr->proto == _htons(ETHER_TYPE_IPv4)) {
> +		info->is_tunnel = 1;
> +		info->outer_ethertype = info->ethertype;
> +		info->outer_l2_len = info->l2_len;
> +		info->outer_l3_len = info->l3_len;
> +
> +		ipv4_hdr = (struct ipv4_hdr *)((char *)gre_hdr +
> +			sizeof(struct simple_gre_hdr));
> +
> +		parse_ipv4(ipv4_hdr, info);
> +		info->ethertype = _htons(ETHER_TYPE_IPv4);
> +		info->l2_len = 0;
> +
> +	} else if (gre_hdr->proto == _htons(ETHER_TYPE_IPv6)) {
> +		info->is_tunnel = 1;
> +		info->outer_ethertype = info->ethertype;
> +		info->outer_l2_len = info->l2_len;
> +		info->outer_l3_len = info->l3_len;
> +
> +		ipv6_hdr = (struct ipv6_hdr *)((char *)gre_hdr +
> +			sizeof(struct simple_gre_hdr));
> +
> +		info->ethertype = _htons(ETHER_TYPE_IPv6);
> +		parse_ipv6(ipv6_hdr, info);
> +		info->l2_len = 0;
> +
> +	} else if (gre_hdr->proto == _htons(0x6558)) { /* ETH_P_TEB in linux
> */
> +		info->is_tunnel = 1;
> +		info->outer_ethertype = info->ethertype;
> +		info->outer_l2_len = info->l2_len;
> +		info->outer_l3_len = info->l3_len;
> +
> +		eth_hdr = (struct ether_hdr *)((char *)gre_hdr +
> +			sizeof(struct simple_gre_hdr));

For NVGRE:
I will do some change here.
eth_hdr = (struct ether_hdr *)((char *)gre_hdr +
		sizeof(struct nvgre_hdr)); // replace  simple_gre_hdr with nvgre_hdr.


> +		parse_ethernet(eth_hdr, info);
> +	} else
> +		return;
> +
> +	info->l2_len += sizeof(struct simple_gre_hdr); }
> +
>  /* modify the IPv4 or IPv4 source address of a packet */  static void
> change_ip_addresses(void *l3_hdr, uint16_t ethertype) @@ -368,6 +428,8
> @@ uint16_t testpmd_ol_flags)
>   *   Ether / (vlan) / IP|IP6 / UDP|TCP|SCTP .
>   *   Ether / (vlan) / outer IP|IP6 / outer UDP / VxLAN / Ether / 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
>   *
>   * The testpmd command line for this forward engine sets the flags
>   * TESTPMD_TX_OFFLOAD_* in ports[tx_port].tx_ol_flags. They control @@
> -437,12 +499,25 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
>  		parse_ethernet(eth_hdr, &info);
>  		l3_hdr = (char *)eth_hdr + info.l2_len;
> 
> -		/* check if it's a supported tunnel (only vxlan for now) */
> -		if ((testpmd_ol_flags &
> TESTPMD_TX_OFFLOAD_PARSE_TUNNEL) &&
> -			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->ol_flags);
> +		/* check if it's a supported tunnel */
> +		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->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);
> +			}
> +		}
> +			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);
>  		}
> 
>  		/* update l3_hdr and outer_l3_hdr if a tunnel was parsed */
> --
> 2.1.4