[PATCH 0/2] lib: add TCP IPv4 GRO support

[PATCH 0/2] lib: add TCP IPv4 GRO support

[Jiayu Hu]

Generic Receive Offload (GRO) is a widely used SW offloading technique,
which reassemble small packets into large ones, to reduce processing
overheads for upper layer applications in receiving side, like networking
stack. Therefore, we propose to add GRO support in DPDK.

DPDK GRO is implemented as a standalone library, which provides GRO
functions for various of protocols. In the design of DPDK GRO, different
protocols have own reassembly functions. Applications should explicitly
invoke specific reassembly functions according to packet types.

This patchset provides TCP IPv4 GRO reassembly functions, and
demonstrates the usage of these functions in app/testpmd.

We perform two iperf tests (with DPDK GRO and without DPDK GRO) to see
the performance gains from DPDK GRO. Specifically, the experiment
environment is:
	a. Two 10Gbps physical ports (p0 and p1) on one host are linked
	together, and are in two network namespaces (ns1 and ns2);
	b. iperf client runs on p0, which is in charge of sending TCP IPv4
	packets; testpmd runs on p1. Besides, testpmd connects with one
	virtual machine (VM) via vhost-user and virtio-kernel. The VM runs
	iperf server, whose IP is 1.1.2.4;
	c. p0 turns on TSO; VM turns off kernel GRO; testpmd runs in iofwd
	mode;
	d. iperf client and server use the following commands:
	- iperf client: ip netns exec ns1 iperf -c 1.1.2.4 -i2 -t 60 -f g -m
	- iperf server: iperf -s -f g
Two test cases are:
	a. w/o DPDK GRO: disable TCP IPv4 GRO on testpmd
	b. w DPDK GRO: enable TCP IPv4 GRO on testpmd
Test results:
	a. w/o DPDK GRO: 5.5 Gbits/sec
	b. w DPDK GRO: 8.46 Gbits/sec
As we can see, the throughput improvement from DPDK GRO is around 50%.

Jiayu Hu (2):
  lib: add Generic Receive Offload support for TCP IPv4 packets
  app/testpmd: provide TCP IPv4 GRO function in iofwd mode

 app/test-pmd/cmdline.c       |  48 +++++++
 app/test-pmd/config.c        |  59 +++++++++
 app/test-pmd/iofwd.c         |   7 +
 app/test-pmd/testpmd.c       |  10 ++
 app/test-pmd/testpmd.h       |   6 +
 config/common_base           |   5 +
 lib/Makefile                 |   1 +
 lib/librte_gro/Makefile      |  50 +++++++
 lib/librte_gro/rte_gro_tcp.c | 301 +++++++++++++++++++++++++++++++++++++++++++
 lib/librte_gro/rte_gro_tcp.h | 114 ++++++++++++++++
 mk/rte.app.mk                |   1 +
 11 files changed, 602 insertions(+)
 create mode 100644 lib/librte_gro/Makefile
 create mode 100644 lib/librte_gro/rte_gro_tcp.c
 create mode 100644 lib/librte_gro/rte_gro_tcp.h

-- 
2.7.4

[PATCH 1/2] lib: add Generic Receive Offload support for TCP IPv4 packets

[Jiayu Hu]

Introduce two new functions to support TCP IPv4 GRO:
- rte_gro_tcp4_tbl_create: create a lookup table for TCP IPv4 GRO.
- rte_gro_tcp4_reassemble_burst: reassemble a bulk of TCP IPv4 packets
at a time.

rte_gro_tcp4_reassemble_burst works in burst-mode, which processes a bulk
of packets at a time. That is, applications are in charge of classifying
and accumulating TCP IPv4 packets before calling it. If applications
provide non-TCP IPv4 packets, rte_gro_tcp4_reassemble_burst won't process
them.

Before using rte_gro_tcp4_reassemble_burst, applications need to create
TCP IPv4 lookup tables via rte_gro_tcp4_tbl_create, which are used by
rte_gro_tcp4_reassemble_burst. The TCP IPv4 lookup table is a cuckoo
hashing table, whose keys are rules of merging TCP IPv4 packets, and
whose values point to item-lists. Each item-list contains items whose
keys are the same.

To process an incoming packet, there are following four steps:
a. Check if the packet should be processed. TCP IPv4 GRO doesn't
process the following types packets:
	-   non TCP-IPv4 packets
	-   packets without data
	-   packets with wrong checksums
	-   fragmented packets
b. Lookup the hash table to find a item-list, which stores packets that
may be able to merge with the incoming packet.
c. If lookup successfully, check all items in the item-list. If find one
that is the neighbor of the incoming packet, chaining them together and
update the packet header fields and mbuf metadata; if don't find,
allocate a new item for the incoming packet and insert it into the
item-list.
d. If fail to find a item-list, allocate a new item-list for the incoming
packet and insert it into the hash table.

After processing all packets, update checksums for the merged ones, and
clear the content of the lookup table.

Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
---
 config/common_base           |   5 +
 lib/Makefile                 |   1 +
 lib/librte_gro/Makefile      |  50 +++++++
 lib/librte_gro/rte_gro_tcp.c | 301 +++++++++++++++++++++++++++++++++++++++++++
 lib/librte_gro/rte_gro_tcp.h | 114 ++++++++++++++++
 mk/rte.app.mk                |   1 +
 6 files changed, 472 insertions(+)
 create mode 100644 lib/librte_gro/Makefile
 create mode 100644 lib/librte_gro/rte_gro_tcp.c
 create mode 100644 lib/librte_gro/rte_gro_tcp.h

diff --git a/config/common_base b/config/common_base
index 37aa1e1..29475ad 100644
--- a/config/common_base
+++ b/config/common_base
@@ -609,6 +609,11 @@ CONFIG_RTE_LIBRTE_VHOST_DEBUG=n
 CONFIG_RTE_LIBRTE_PMD_VHOST=n
 
 #
+# Compile GRO library
+#
+CONFIG_RTE_LIBRTE_GRO=y
+
+#
 #Compile Xen domain0 support
 #
 CONFIG_RTE_LIBRTE_XEN_DOM0=n
diff --git a/lib/Makefile b/lib/Makefile
index 4178325..0665f58 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -59,6 +59,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_TABLE) += librte_table
 DIRS-$(CONFIG_RTE_LIBRTE_PIPELINE) += librte_pipeline
 DIRS-$(CONFIG_RTE_LIBRTE_REORDER) += librte_reorder
 DIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += librte_pdump
+DIRS-$(CONFIG_RTE_LIBRTE_GRO) += librte_gro
 
 ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
 DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
diff --git a/lib/librte_gro/Makefile b/lib/librte_gro/Makefile
new file mode 100644
index 0000000..71bdb04
--- /dev/null
+++ b/lib/librte_gro/Makefile
@@ -0,0 +1,50 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of Intel Corporation nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+include $(RTE_SDK)/mk/rte.vars.mk
+
+# library name
+LIB = librte_gro.a
+
+CFLAGS += -O3
+CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR)
+
+EXPORT_MAP := rte_gro_version.map
+
+LIBABIVER := 1
+
+#source files
+SRCS-$(CONFIG_RTE_LIBRTE_GRO) += rte_gro_tcp.c
+
+# install this header file
+SYMLINK-$(CONFIG_RTE_LIBRTE_GRO)-include += rte_gro_tcp.h
+
+include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_gro/rte_gro_tcp.c b/lib/librte_gro/rte_gro_tcp.c
new file mode 100644
index 0000000..9fd3efe
--- /dev/null
+++ b/lib/librte_gro/rte_gro_tcp.c
@@ -0,0 +1,301 @@
+#include "rte_gro_tcp.h"
+
+struct rte_hash *
+rte_gro_tcp4_tbl_create(char *name,
+		uint32_t nb_entries, uint16_t socket_id)
+{
+	struct rte_hash_parameters ht_param = {
+		.entries = nb_entries,
+		.name = name,
+		.key_len = sizeof(struct gro_tcp4_pre_rules),
+		.hash_func = rte_jhash,
+		.hash_func_init_val = 0,
+		.socket_id = socket_id,
+	};
+	struct rte_hash *tbl;
+
+	tbl = rte_hash_create(&ht_param);
+	if (tbl == NULL)
+		printf("GRO TCP4: allocate hash table fail\n");
+	return tbl;
+}
+
+/* update TCP IPv4 checksum */
+static void
+gro_tcp4_cksum_update(struct rte_mbuf *pkt)
+{
+	uint32_t len, offset, cksum;
+	struct ether_hdr *eth_hdr;
+	struct ipv4_hdr *ipv4_hdr;
+	struct tcp_hdr *tcp_hdr;
+	uint16_t ipv4_ihl, cksum_pld;
+
+	if (pkt == NULL)
+		return;
+
+	len = pkt->pkt_len;
+	eth_hdr = rte_pktmbuf_mtod(pkt, struct ether_hdr *);
+	ipv4_hdr = (struct ipv4_hdr *)(eth_hdr + 1);
+	ipv4_ihl = IPv4_HDR_LEN(ipv4_hdr);
+	tcp_hdr = (struct tcp_hdr *)((char *)ipv4_hdr + ipv4_ihl);
+
+	offset = sizeof(struct ether_hdr) + ipv4_ihl;
+	len -= offset;
+
+	/* TCP cksum without IP pseudo header */
+	ipv4_hdr->hdr_checksum = 0;
+	tcp_hdr->cksum = 0;
+	if (rte_raw_cksum_mbuf(pkt, offset, len, &cksum_pld) < 0) {
+		printf("invalid param for raw_cksum_mbuf\n");
+		return;
+	}
+	/* IP pseudo header cksum */
+	cksum = cksum_pld;
+	cksum += rte_ipv4_phdr_cksum(ipv4_hdr, 0);
+
+	/* combine TCP checksum and IP pseudo header checksum */
+	cksum = ((cksum & 0xffff0000) >> 16) + (cksum & 0xffff);
+	cksum = (~cksum) & 0xffff;
+	cksum = (cksum == 0) ? 0xffff : cksum;
+	tcp_hdr->cksum = cksum;
+
+	/* update IP header cksum */
+	ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
+}
+
+/**
+ * This function traverses the item-list to find one item that can be
+ * merged with the incoming packet. If merge successfully, the merged
+ * packets are chained together; if not, insert the incoming packet into
+ * the item-list.
+ */
+static uint64_t
+gro_tcp4_reassemble(struct gro_tcp_item_list *list,
+		struct rte_mbuf *pkt,
+		uint32_t pkt_sent_seq,
+		uint32_t pkt_idx)
+{
+	struct gro_tcp_item *items;
+	struct ipv4_hdr *ipv4_hdr1;
+	struct tcp_hdr *tcp_hdr1;
+	uint16_t ipv4_ihl1, tcp_hl1, tcp_dl1;
+
+	items = list->items;
+	ipv4_hdr1 = (struct ipv4_hdr *)(rte_pktmbuf_mtod(pkt, struct
+				ether_hdr *) + 1);
+	ipv4_ihl1 = IPv4_HDR_LEN(ipv4_hdr1);
+	tcp_hdr1 = (struct tcp_hdr *)((char *)ipv4_hdr1 + ipv4_ihl1);
+	tcp_hl1 = TCP_HDR_LEN(tcp_hdr1);
+	tcp_dl1 = rte_be_to_cpu_16(ipv4_hdr1->total_length) - ipv4_ihl1
+		- tcp_hl1;
+
+	for (uint16_t i = 0; i < list->nb_item; i++) {
+		/* check if the two packets are neighbor */
+		if ((pkt_sent_seq ^ items[i].next_sent_seq) == 0) {
+			struct ipv4_hdr *ipv4_hdr2;
+			struct tcp_hdr *tcp_hdr2;
+			uint16_t ipv4_ihl2, tcp_hl2;
+			struct rte_mbuf *tail;
+
+			ipv4_hdr2 = (struct ipv4_hdr *) (rte_pktmbuf_mtod(
+						items[i].segment, struct ether_hdr *)
+					+ 1);
+
+			/* check if the option fields equal */
+			if (tcp_hl1 > sizeof(struct tcp_hdr)) {
+				ipv4_ihl2 = IPv4_HDR_LEN(ipv4_hdr2);
+				tcp_hdr2 = (struct tcp_hdr *)
+					((char *)ipv4_hdr2 + ipv4_ihl2);
+				tcp_hl2 = TCP_HDR_LEN(tcp_hdr2);
+				if ((tcp_hl1 != tcp_hl2) ||
+						(memcmp(tcp_hdr1 + 1, tcp_hdr2 + 1,
+								tcp_hl2 - sizeof(struct tcp_hdr))
+						 != 0))
+					continue;
+			}
+			/* check if the packet length will be beyond 64K */
+			if (items[i].segment->pkt_len + tcp_dl1 > UINT16_MAX)
+				goto merge_fail;
+
+			/* remove the header of the incoming packet */
+			rte_pktmbuf_adj(pkt, sizeof(struct ether_hdr) +
+					ipv4_ihl1 + tcp_hl1);
+			/* chain the two packet together */
+			tail = rte_pktmbuf_lastseg(items[i].segment);
+			tail->next = pkt;
+
+			/* update IP header for the merged packet */
+			ipv4_hdr2->total_length = rte_cpu_to_be_16(
+					rte_be_to_cpu_16(ipv4_hdr2->total_length)
+					+ tcp_dl1);
+
+			/* update the next expected sequence number */
+			items[i].next_sent_seq += tcp_dl1;
+
+			/* update mbuf metadata for the merged packet */
+			items[i].segment->nb_segs++;
+			items[i].segment->pkt_len += pkt->pkt_len;
+
+			return items[i].segment_idx + 1;
+		}
+	}
+
+merge_fail:
+	/* fail to merge. Insert the incoming packet into the item-list */
+	items[list->nb_item].next_sent_seq = pkt_sent_seq + tcp_dl1;
+	items[list->nb_item].segment = pkt;
+	items[list->nb_item].segment_idx = pkt_idx;
+	list->nb_item++;
+
+	return 0;
+}
+
+uint32_t
+rte_gro_tcp4_reassemble_burst(struct rte_hash *hash_tbl,
+		struct rte_mbuf **pkts,
+		const uint32_t nb_pkts)
+{
+	struct ether_hdr *eth_hdr;
+	struct ipv4_hdr *ipv4_hdr;
+	struct tcp_hdr *tcp_hdr;
+	uint16_t ipv4_ihl, tcp_hl, tcp_dl, tcp_cksum, ip_cksum;
+	uint32_t sent_seq;
+	struct gro_tcp4_pre_rules key;
+	struct gro_tcp_item_list *list;
+
+	/* preallocated items. Each packet has nb_pkts items */
+	struct gro_tcp_item items_pool[nb_pkts * nb_pkts];
+
+	struct gro_tcp_info gro_infos[nb_pkts];
+	uint64_t ol_flags, idx;
+	int ret, is_performed_gro = 0;
+	uint32_t nb_after_gro = nb_pkts;
+
+	if (hash_tbl == NULL || pkts == NULL || nb_pkts == 0) {
+		printf("GRO TCP4: invalid parameters\n");
+		goto end;
+	}
+	memset(&key, 0, sizeof(struct gro_tcp4_pre_rules));
+
+	for (uint32_t i = 0; i < nb_pkts; i++) {
+		gro_infos[i].nb_merged_pkts = 1;
+
+		eth_hdr = rte_pktmbuf_mtod(pkts[i], struct ether_hdr *);
+		ipv4_hdr = (struct ipv4_hdr *)(eth_hdr + 1);
+		ipv4_ihl = IPv4_HDR_LEN(ipv4_hdr);
+
+		/* 1. check if the packet should be processed */
+		if (ipv4_ihl < sizeof(struct ipv4_hdr))
+			continue;
+		if (ipv4_hdr->next_proto_id != IPPROTO_TCP)
+			continue;
+		if ((ipv4_hdr->fragment_offset &
+					rte_cpu_to_be_16(IPV4_HDR_DF_MASK))
+				== 0)
+			continue;
+
+		tcp_hdr = (struct tcp_hdr *)((char *)ipv4_hdr + ipv4_ihl);
+		tcp_hl = TCP_HDR_LEN(tcp_hdr);
+		tcp_dl = rte_be_to_cpu_16(ipv4_hdr->total_length) - ipv4_ihl
+			- tcp_hl;
+		if (tcp_dl == 0)
+			continue;
+
+		ol_flags = pkts[i]->ol_flags;
+		/**
+		 * 2. if HW rx checksum offload isn't enabled, recalculate the
+		 * checksum in SW. Then, check if the checksum is correct
+		 */
+		if ((ol_flags & PKT_RX_IP_CKSUM_MASK) !=
+				PKT_RX_IP_CKSUM_UNKNOWN) {
+			if (ol_flags == PKT_RX_IP_CKSUM_BAD)
+				continue;
+		} else {
+			ip_cksum = ipv4_hdr->hdr_checksum;
+			ipv4_hdr->hdr_checksum = 0;
+			ipv4_hdr->hdr_checksum = rte_ipv4_cksum(ipv4_hdr);
+			if (ipv4_hdr->hdr_checksum ^ ip_cksum)
+				continue;
+		}
+
+		if ((ol_flags & PKT_RX_L4_CKSUM_MASK) !=
+				PKT_RX_L4_CKSUM_UNKNOWN) {
+			if (ol_flags == PKT_RX_L4_CKSUM_BAD)
+				continue;
+		} else {
+			tcp_cksum = tcp_hdr->cksum;
+			tcp_hdr->cksum = 0;
+			tcp_hdr->cksum = rte_ipv4_udptcp_cksum
+				(ipv4_hdr, tcp_hdr);
+			if (tcp_hdr->cksum ^ tcp_cksum)
+				continue;
+		}
+
+		/* 3. search for the corresponding item-list for the packet */
+		key.eth_saddr = eth_hdr->s_addr;
+		key.eth_daddr = eth_hdr->d_addr;
+		key.ip_src_addr = rte_be_to_cpu_32(ipv4_hdr->src_addr);
+		key.ip_dst_addr = rte_be_to_cpu_32(ipv4_hdr->dst_addr);
+		key.src_port = rte_be_to_cpu_16(tcp_hdr->src_port);
+		key.dst_port = rte_be_to_cpu_16(tcp_hdr->dst_port);
+		key.recv_ack = rte_be_to_cpu_32(tcp_hdr->recv_ack);
+		key.tcp_flags = tcp_hdr->tcp_flags;
+
+		sent_seq = rte_be_to_cpu_32(tcp_hdr->sent_seq);
+		ret = rte_hash_lookup_data(hash_tbl, &key, (void **)&list);
+
+		/* try to reassemble the packet */
+		if (ret >= 0) {
+			idx = gro_tcp4_reassemble(list, pkts[i], sent_seq, i);
+			/* merge successfully, update gro_info */
+			if (idx > 0) {
+				gro_infos[i].nb_merged_pkts = 0;
+				gro_infos[--idx].nb_merged_pkts++;
+				nb_after_gro--;
+			}
+		} else {
+			/**
+			 * fail to find a item-list. Allocate a new item-list
+			 * for the incoming packet and insert it into the hash
+			 * table.
+			 */
+			list = &(gro_infos[i].item_list);
+			list->items = &(items_pool[nb_pkts * i]);
+			list->nb_item = 1;
+			list->items[0].next_sent_seq = sent_seq + tcp_dl;
+			list->items[0].segment = pkts[i];
+			list->items[0].segment_idx = i;
+
+			if (unlikely(rte_hash_add_key_data(hash_tbl, &key, list)
+						!= 0))
+				printf("GRO TCP hash insert fail.\n");
+
+			is_performed_gro = 1;
+		}
+	}
+
+	/**
+	 * if there are packets been merged, update their checksum,
+	 * and remove useless packet addresses from packet array
+	 */
+	if (nb_after_gro < nb_pkts) {
+		struct rte_mbuf *tmp[nb_pkts];
+
+		memset(tmp, 0, sizeof(struct rte_mbuf *) * nb_pkts);
+		/* update checksum */
+		for (uint32_t i = 0, j = 0; i < nb_pkts; i++) {
+			if (gro_infos[i].nb_merged_pkts > 1)
+				gro_tcp4_cksum_update(pkts[i]);
+			if (gro_infos[i].nb_merged_pkts != 0)
+				tmp[j++] = pkts[i];
+		}
+		/* update the packet array */
+		rte_memcpy(pkts, tmp, nb_pkts * sizeof(struct rte_mbuf *));
+	}
+
+	/* if GRO is performed, reset the hash table */
+	if (is_performed_gro)
+		rte_hash_reset(hash_tbl);
+end:
+	return nb_after_gro;
+}
diff --git a/lib/librte_gro/rte_gro_tcp.h b/lib/librte_gro/rte_gro_tcp.h
new file mode 100644
index 0000000..aa99a06
--- /dev/null
+++ b/lib/librte_gro/rte_gro_tcp.h
@@ -0,0 +1,114 @@
+#ifndef _RTE_GRO_TCP_H_
+#define _RTE_GRO_TCP_H_
+
+#include <rte_ethdev.h>
+#include <rte_ip.h>
+#include <rte_tcp.h>
+#include <rte_hash.h>
+#include <rte_jhash.h>
+#include <rte_malloc.h>
+
+#if RTE_BYTE_ORDER == RTE_LITTLE_ENDIAN
+#define TCP_HDR_LEN(tcph) \
+	((tcph->data_off >> 4) * 4)
+#define IPv4_HDR_LEN(iph) \
+	((iph->version_ihl & 0x0f) * 4)
+#else
+#define TCP_DATAOFF_MASK 0x0f
+#define TCP_HDR_LEN(tcph) \
+	((tcph->data_off & TCP_DATAOFF_MASK) * 4)
+#define IPv4_HDR_LEN(iph) \
+	((iph->version_ihl >> 4) * 4)
+#endif
+
+#define IPV4_HDR_DF_SHIFT 14
+#define IPV4_HDR_DF_MASK (1 << IPV4_HDR_DF_SHIFT)
+
+#define RTE_GRO_TCP_HASH_ENTRIES_MIN RTE_HASH_BUCKET_ENTRIES
+#define RTE_GRO_TCP_HASH_ENTRIES_MAX RTE_HASH_ENTRIES_MAX
+
+/**
+ * key structure of TCP ipv4 hash table. It describes the prerequsite
+ * rules of merging packets.
+ */
+struct gro_tcp4_pre_rules {
+	struct ether_addr eth_saddr;
+	struct ether_addr eth_daddr;
+	uint32_t ip_src_addr;
+	uint32_t ip_dst_addr;
+
+	uint32_t recv_ack;	/**< acknowledgment sequence number. */
+	uint16_t src_port;
+	uint16_t dst_port;
+	uint8_t tcp_flags;	/**< TCP flags. */
+
+	uint8_t padding[3];
+};
+
+/**
+ * Item structure
+ */
+struct gro_tcp_item {
+	struct rte_mbuf *segment;	/**< packet address. */
+	uint32_t next_sent_seq;	/**< sequence number of the next packet. */
+	uint32_t segment_idx;	/**< packet index. */
+} __rte_cache_aligned;
+
+/**
+ * Item-list structure, which is the value in the TCP ipv4 hash table.
+ */
+struct gro_tcp_item_list {
+	struct gro_tcp_item *items;	/**< items array */
+	uint32_t nb_item;	/**< item number */
+};
+
+/**
+ * Local data structure. Every packet has an object of this structure,
+ * which is used for reassembling.
+ */
+struct gro_tcp_info {
+	struct gro_tcp_item_list item_list;	/**< preallocated item-list */
+	uint32_t nb_merged_pkts;	/**< the number of merged packets */
+};
+
+/**
+ * Create a new TCP ipv4 GRO lookup table.
+ *
+ * @param name
+ *	Lookup table name
+ * @param nb_entries
+ *  Lookup table elements number, whose value should be larger than or
+ *  equal to RTE_GRO_TCP_HASH_ENTRIES_MIN, and less than or equal to
+ *  RTE_GRO_TCP_HASH_ENTRIES_MAX, and should be power of two.
+ * @param socket_id
+ *  socket id
+ * @return
+ *  lookup table address
+ */
+struct rte_hash *
+rte_gro_tcp4_tbl_create(char *name, uint32_t nb_entries,
+		uint16_t socket_id);
+/**
+ * This function reassembles a bulk of TCP IPv4 packets. For non-TCP IPv4
+ * packets, the function won't process them.
+ *
+ * @param hash_tbl
+ *	Lookup table used to reassemble packets. It stores key-value pairs.
+ *	The key describes the prerequsite rules to merge two TCP IPv4 packets;
+ *	the value is a pointer pointing to a item-list, which contains
+ *	packets that have the same prerequisite TCP IPv4 rules. Note that
+ *	applications need to guarantee the hash_tbl is clean when first call
+ *	this function.
+ * @param pkts
+ *	Packets to reassemble.
+ * @param nb_pkts
+ *	The number of packets to reassemble.
+ * @return
+ *	The packet number after GRO. If reassemble successfully, the value is
+ *	less than nb_pkts; if not, the value is equal to nb_pkts.
+ */
+uint32_t
+rte_gro_tcp4_reassemble_burst(struct rte_hash *hash_tbl,
+		struct rte_mbuf **pkts,
+		const uint32_t nb_pkts);
+#endif
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 0e0b600..521d20e 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -99,6 +99,7 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_RING)           += -lrte_ring
 _LDLIBS-$(CONFIG_RTE_LIBRTE_EAL)            += -lrte_eal
 _LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE)        += -lrte_cmdline
 _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER)        += -lrte_reorder
+_LDLIBS-$(CONFIG_RTE_LIBRTE_GRO)        	+= -lrte_gro
 
 ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n)
 # plugins (link only if static libraries)
-- 
2.7.4

[PATCH 2/2] app/testpmd: provide TCP IPv4 GRO function in iofwd mode

[Jiayu Hu]

This patch demonstrates the usage of the TCP IPv4 GRO library in testpmd.
Currently, only the iofwd mode supports this feature. By default, TCP
IPv4 GRO is turned off. The command, "gro tcp4 on", turns on this
feature; the command, "gro tcp4 off", turns off it.

Once the feature is turned on, all received packets are performed TCP
IPv4 GRO procedure before been forwarded.

Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
---
 app/test-pmd/cmdline.c | 48 ++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/config.c  | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/iofwd.c   |  7 ++++++
 app/test-pmd/testpmd.c | 10 +++++++++
 app/test-pmd/testpmd.h |  6 +++++
 5 files changed, 130 insertions(+)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 47f935d..618b9da 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -76,6 +76,7 @@
 #include <rte_devargs.h>
 #include <rte_eth_ctrl.h>
 #include <rte_flow.h>
+#include <rte_gro_tcp.h>
 
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
@@ -396,6 +397,9 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"tso show (portid)"
 			"    Display the status of TCP Segmentation Offload.\n\n"
 
+			"gro tcp4 (on|off)"
+			"    Enable or disable TCP IPv4 Receive Offload.\n\n"
+
 			"set fwd (%s)\n"
 			"    Set packet forwarding mode.\n\n"
 
@@ -3784,6 +3788,49 @@ cmdline_parse_inst_t cmd_tunnel_tso_show = {
 	},
 };
 
+/* *** SET TCP IPv4 Receive Offload FOR RX PKTS *** */
+struct cmd_gro_result {
+	cmdline_fixed_string_t cmd_keyword;
+	cmdline_fixed_string_t protocol;
+	cmdline_fixed_string_t mode;
+};
+
+static void
+cmd_set_gro_parsed(void *parsed_result,
+		__attribute__((unused)) struct cmdline *cl,
+		__attribute__((unused)) void *data)
+{
+	struct cmd_gro_result *res;
+
+	res = parsed_result;
+	if (strcmp(res->protocol, "tcp4") == 0)
+		setup_gro_tcp4(res->mode);
+	else
+		printf("unsupported GRO protocol\n");
+}
+
+cmdline_parse_token_string_t cmd_gro_keyword =
+	TOKEN_STRING_INITIALIZER(struct cmd_gro_result,
+			cmd_keyword, "gro");
+cmdline_parse_token_string_t cmd_gro_protocol =
+	TOKEN_STRING_INITIALIZER(struct cmd_gro_result,
+			protocol, NULL);
+cmdline_parse_token_string_t cmd_gro_mode =
+	TOKEN_STRING_INITIALIZER(struct cmd_gro_result,
+			mode, NULL);
+
+cmdline_parse_inst_t cmd_set_gro = {
+	.f = cmd_set_gro_parsed,
+	.data = NULL,
+	.help_str = "gro tcp4 on|off",
+	.tokens = {
+		(void *)&cmd_gro_keyword,
+		(void *)&cmd_gro_protocol,
+		(void *)&cmd_gro_mode,
+		NULL,
+	},
+};
+
 /* *** ENABLE/DISABLE FLUSH ON RX STREAMS *** */
 struct cmd_set_flush_rx {
 	cmdline_fixed_string_t set;
@@ -12464,6 +12511,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(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_set_gro,
 	(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/config.c b/app/test-pmd/config.c
index 80491fc..b4144a3 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -97,6 +97,7 @@
 #ifdef RTE_LIBRTE_IXGBE_PMD
 #include <rte_pmd_ixgbe.h>
 #endif
+#include <rte_gro_tcp.h>
 
 #include "testpmd.h"
 
@@ -2415,6 +2416,64 @@ set_tx_pkt_segments(unsigned *seg_lengths, unsigned nb_segs)
 	tx_pkt_nb_segs = (uint8_t) nb_segs;
 }
 
+void
+setup_gro_tcp4(const char *mode)
+{
+	lcoreid_t lc_id;
+	streamid_t sm_id;
+	uint64_t nb_entries = 64;	/* lookup table entry number */
+
+	if (strcmp(mode, "on") == 0) {
+		if (test_done == 0) {
+			printf("Before enable TCP IPv4 GRO,"
+					" please stop forwarding first\n");
+			return;
+		}
+		if (enable_gro_tcp4 == 1) {
+			printf("GRO TCP IPv4 has been turned on\n");
+			return;
+		}
+		for (lc_id = 0; lc_id < cur_fwd_config.nb_fwd_lcores; lc_id++) {
+			char name[20];
+
+			snprintf(name, sizeof(name), "GRO_TCP4_%u", lc_id);
+			if (gro_tcp4_tbls[lc_id])
+				rte_hash_free(gro_tcp4_tbls[lc_id]);
+
+			gro_tcp4_tbls[lc_id] = rte_gro_tcp4_tbl_create(
+					name,
+					nb_entries,
+					rte_lcore_to_socket_id
+					(fwd_lcores_cpuids[lc_id]));
+			if (gro_tcp4_tbls[lc_id] == NULL) {
+				enable_gro_tcp4 = 0;
+				return;
+			}
+			for (sm_id = fwd_lcores[lc_id]->stream_idx; sm_id <
+					fwd_lcores[lc_id]->stream_idx +
+					fwd_lcores[lc_id]->stream_nb; sm_id++) {
+				fwd_streams[sm_id]->tbl_idx = lc_id;
+			}
+		}
+		enable_gro_tcp4 = 1;
+	} else if (strcmp(mode, "off") == 0) {
+		if (test_done == 0) {
+			printf("Before disable TCP IPv4 GRO,"
+					" please stop forwarding first\n");
+			return;
+		}
+		if (enable_gro_tcp4 == 0) {
+			printf("GRO TCP IPv4 has been turned off\n");
+			return;
+		}
+		for (lc_id = 0; lc_id < cur_fwd_config.nb_fwd_lcores; lc_id++) {
+			rte_hash_free(gro_tcp4_tbls[lc_id]);
+			gro_tcp4_tbls[lc_id] = NULL;
+		}
+		enable_gro_tcp4 = 0;
+	}
+}
+
 char*
 list_pkt_forwarding_modes(void)
 {
diff --git a/app/test-pmd/iofwd.c b/app/test-pmd/iofwd.c
index 15cb4a2..ec05d6f 100644
--- a/app/test-pmd/iofwd.c
+++ b/app/test-pmd/iofwd.c
@@ -65,6 +65,7 @@
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
 #include <rte_flow.h>
+#include <rte_gro_tcp.h>
 
 #include "testpmd.h"
 
@@ -99,6 +100,12 @@ pkt_burst_io_forward(struct fwd_stream *fs)
 			pkts_burst, nb_pkt_per_burst);
 	if (unlikely(nb_rx == 0))
 		return;
+	if (enable_gro_tcp4) {
+		nb_rx = rte_gro_tcp4_reassemble_burst(
+				gro_tcp4_tbls[fs->tbl_idx],
+				pkts_burst,
+				nb_rx);
+	}
 	fs->rx_packets += nb_rx;
 
 #ifdef RTE_TEST_PMD_RECORD_BURST_STATS
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index e04e215..caf8a61 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -273,6 +273,16 @@ uint32_t bypass_timeout = RTE_BYPASS_TMT_OFF;
 #endif
 
 /*
+ * TCP IPv4 lookup tables. Each lcore has a lookup table.
+ */
+struct rte_hash *gro_tcp4_tbls[RTE_MAX_LCORE];
+
+/*
+ * TCP IPv4 GRO enable/disable flag.
+ */
+uint8_t enable_gro_tcp4 = 0;	/* turn off by default */
+
+/*
  * Ethernet device configuration.
  */
 struct rte_eth_rxmode rx_mode = {
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 8cf2860..bfd1e52 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -109,6 +109,8 @@ struct fwd_stream {
 	queueid_t  tx_queue;  /**< TX queue to send forwarded packets */
 	streamid_t peer_addr; /**< index of peer ethernet address of packets */
 
+	uint16_t tbl_idx;	/**< TCP IPv4 GRO lookup tale index */
+
 	unsigned int retry_enabled;
 
 	/* "read-write" results */
@@ -420,6 +422,9 @@ extern struct ether_addr peer_eth_addrs[RTE_MAX_ETHPORTS];
 extern uint32_t burst_tx_delay_time; /**< Burst tx delay time(us) for mac-retry. */
 extern uint32_t burst_tx_retry_num;  /**< Burst tx retry number for mac-retry. */
 
+extern struct rte_hash *gro_tcp4_tbls[RTE_MAX_LCORE];
+extern uint8_t enable_gro_tcp4;
+
 static inline unsigned int
 lcore_num(void)
 {
@@ -616,6 +621,7 @@ void get_2tuple_filter(uint8_t port_id, uint16_t index);
 void get_5tuple_filter(uint8_t port_id, uint16_t index);
 int rx_queue_id_is_invalid(queueid_t rxq_id);
 int tx_queue_id_is_invalid(queueid_t txq_id);
+void setup_gro_tcp4(const char *mode);
 
 /* Functions to manage the set of filtered Multicast MAC addresses */
 void mcast_addr_add(uint8_t port_id, struct ether_addr *mc_addr);
-- 
2.7.4

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Jiayu Hu]

On Thu, Mar 23, 2017 at 09:12:56PM +0800, Ananyev, Konstantin wrote:
> Hi everyone,
> 
> > >
> > >
> > > > -----Original Message-----
> > > > From: Hu, Jiayu
> > > > Sent: Thursday, March 23, 2017 6:25 AM
> > > > To: Wiles, Keith <keith.wiles@intel.com>
> > > > Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Richardson, Bruce
> > > > <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> > > > Ananyev, Konstantin <konstantin.ananyev@intel.com>
> > > > Subject: Re: [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support
> > > >
> > > > On Thu, Mar 23, 2017 at 01:29:06PM +0800, Wiles, Keith wrote:
> > > > >
> > > > > > On Mar 22, 2017, at 9:15 PM, Hu, Jiayu <jiayu.hu@intel.com> wrote:
> > > > > >
> > > > > > On Wed, Mar 22, 2017 at 10:19:41PM +0800, Wiles, Keith wrote:
> > > > > >> Off list.
> > > > > >>
> > > > > >> This support for GRO, seems it needs to be a feature for all ethernet
> > > > devices and some way the developer can enable this feature like the other
> > > > offloads in DPDK. The GRO support should be set by the developer and then
> > > > the apis are called within ethdev or the PMD to process the packets. The
> > > > code is generic and creating a library is not the best overall solution
> > > > IMO.
> > > > > >
> > > > > > Indeed, in this patchset, GRO is just proposed as a application-used
> > > > library.
> > > > > > But actually, these GRO APIs can also be used by ethdev and PMD. For
> > > > > > example, register rte_gro_tcp4_reassemble_burst as a rx callback.
> > > > > > Therefore, maybe we can support GRO in two ways. The one is a
> > > > > > application-used library, the other is a device feature. Applications
> > > > decide which one to use.
> > > > > >
> > > > > > How do you think of it?
> > > > >
> > > > > I would prefer to use it only in a offload design, meaning the GRO is
> > > > just another ethernet offload the user can turn on. Using something like a
> > > > RX callback to handle the GRO for the developer. This way he just turns it
> > > > on in via a ethdev offload support feature and then setup the RX callback
> > > > via ethdev. The developer only needs to enable the feature and never calls
> > > > GRO APIs.
> > > >
> > > > The advantage of providing an application-used GRO library can enable more
> > > > flexibility to applications. Applications can flexibly use GRO functions
> > > > according to own realistic scenario. Therefore, I think it makes sense to
> > > > provide an application-used GRO library.
> > > > >
> > > > > Adding a new GRO library may not get much support and having a whole
> > > > library for GRO seems a bit odd.
> > > >
> > > > In my opinion, we just need to provide one GRO library. But it can be used
> > > > by ethernet devices and applications at the same time. Ethernet devices
> > > > use it to provide an offload feature. If applications want more
> > > > flexibility, they can just turn off this device feature, and use GRO APIs
> > > > directly.
> > > >
> > > > +Konstantin
> > > >
> > >
> > > [Apologies for the basic questions, I haven't studied the patchset in detail]
> > > Rather than adding a whole new library for it, can it just fit into librte_net or an existing lib? Are we planning a sample to show off
> > tighter integration with ethdev or changes to the ethdev library to transparently use the library when needed?
> > 
> > Currently, we have an individual library, librte_ip_frag, which provides IP fragment
> > and ressembly abilities. Similiarly, DPDK GRO will provide reassembly ability for
> > various of protocols, not only TCP. So I think it's good to make a new library for
> > this feature.
> > 
> > About GRO, we had a discussion two monthes ago. You can see it in
> > http://dpdk.org/ml/archives/dev/2017-January/056276.html
> > In that discussion, we agree to support GRO in two steps. The first is to implement
> > GRO as a standalone library, and see how much performance we can get. The second
> > step is to discuss how to integrate GRO into DPDK. Therefore, if we agree to support
> > GRO as a device feature, we need to discuss how to enable/disable this device feature.
> > Once we reach an agreement, there will be a sample to demonstrate the integration.
> 
> I think that having a separate library for GRO is a step in a right direction.
> >From my perspective - it provides a clean and flexible way to use that feature.
> If later someone would like to put GRO into ethdev layer (or particular PMD),
> he can use existing librte_gro for that.

Agree. I think introducing more flexibility is an important thing for applications.

> I didn't  have a closer look yet, but I think that caught my attention:
> API fir the lib seems too IPv4/TCP oriented -
> though I understand that the most common case and should be implemented first.
> I wonder can we have it a bit more generic and extendable, so user can specify what combination of protocols
> he is interested in (let say: ipv4/tcp,  ipv6/tcp, etc.).
> Even if right now we'll have it implemented only for ipv4/tcp.
> Then internally we can have some check is that supported or not and if yes setup things accordingly.

Indeed, current apis are too specific. It's not very friendly to applications.
Maybe we can use macro to define the combination of protocols, like GRO_TCP_IPV4
and GRO_UDP_IPV6; and provide a generic setup function and reassembly function.
Both of them perform different operations according to the macro value inputted
by the application.

> BTW, that's for 17.08, right?

Yes, it's for 17.08.

Jiayu
> 
> Konstantin
>   
>  

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Wiles, Keith]

> On Mar 23, 2017, at 9:23 PM, Hu, Jiayu <jiayu.hu@intel.com> wrote:
> 
> On Thu, Mar 23, 2017 at 09:12:56PM +0800, Ananyev, Konstantin wrote:
>> Hi everyone,
>> 
>>>> 
>>>> 
>>>>> -----Original Message-----
>>>>> From: Hu, Jiayu
>>>>> Sent: Thursday, March 23, 2017 6:25 AM
>>>>> To: Wiles, Keith <keith.wiles@intel.com>
>>>>> Cc: Yuanhan Liu <yuanhan.liu@linux.intel.com>; Richardson, Bruce
>>>>> <bruce.richardson@intel.com>; Yigit, Ferruh <ferruh.yigit@intel.com>;
>>>>> Ananyev, Konstantin <konstantin.ananyev@intel.com>
>>>>> Subject: Re: [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support
>>>>> 
>>>>> On Thu, Mar 23, 2017 at 01:29:06PM +0800, Wiles, Keith wrote:
>>>>>> 
>>>>>>> On Mar 22, 2017, at 9:15 PM, Hu, Jiayu <jiayu.hu@intel.com> wrote:
>>>>>>> 
>>>>>>> On Wed, Mar 22, 2017 at 10:19:41PM +0800, Wiles, Keith wrote:
>>>>>>>> Off list.
>>>>>>>> 
>>>>>>>> This support for GRO, seems it needs to be a feature for all ethernet
>>>>> devices and some way the developer can enable this feature like the other
>>>>> offloads in DPDK. The GRO support should be set by the developer and then
>>>>> the apis are called within ethdev or the PMD to process the packets. The
>>>>> code is generic and creating a library is not the best overall solution
>>>>> IMO.
>>>>>>> 
>>>>>>> Indeed, in this patchset, GRO is just proposed as a application-used
>>>>> library.
>>>>>>> But actually, these GRO APIs can also be used by ethdev and PMD. For
>>>>>>> example, register rte_gro_tcp4_reassemble_burst as a rx callback.
>>>>>>> Therefore, maybe we can support GRO in two ways. The one is a
>>>>>>> application-used library, the other is a device feature. Applications
>>>>> decide which one to use.
>>>>>>> 
>>>>>>> How do you think of it?
>>>>>> 
>>>>>> I would prefer to use it only in a offload design, meaning the GRO is
>>>>> just another ethernet offload the user can turn on. Using something like a
>>>>> RX callback to handle the GRO for the developer. This way he just turns it
>>>>> on in via a ethdev offload support feature and then setup the RX callback
>>>>> via ethdev. The developer only needs to enable the feature and never calls
>>>>> GRO APIs.
>>>>> 
>>>>> The advantage of providing an application-used GRO library can enable more
>>>>> flexibility to applications. Applications can flexibly use GRO functions
>>>>> according to own realistic scenario. Therefore, I think it makes sense to
>>>>> provide an application-used GRO library.
>>>>>> 
>>>>>> Adding a new GRO library may not get much support and having a whole
>>>>> library for GRO seems a bit odd.
>>>>> 
>>>>> In my opinion, we just need to provide one GRO library. But it can be used
>>>>> by ethernet devices and applications at the same time. Ethernet devices
>>>>> use it to provide an offload feature. If applications want more
>>>>> flexibility, they can just turn off this device feature, and use GRO APIs
>>>>> directly.
>>>>> 
>>>>> +Konstantin
>>>>> 
>>>> 
>>>> [Apologies for the basic questions, I haven't studied the patchset in detail]
>>>> Rather than adding a whole new library for it, can it just fit into librte_net or an existing lib? Are we planning a sample to show off
>>> tighter integration with ethdev or changes to the ethdev library to transparently use the library when needed?
>>> 
>>> Currently, we have an individual library, librte_ip_frag, which provides IP fragment
>>> and ressembly abilities. Similiarly, DPDK GRO will provide reassembly ability for
>>> various of protocols, not only TCP. So I think it's good to make a new library for
>>> this feature.
>>> 
>>> About GRO, we had a discussion two monthes ago. You can see it in
>>> http://dpdk.org/ml/archives/dev/2017-January/056276.html
>>> In that discussion, we agree to support GRO in two steps. The first is to implement
>>> GRO as a standalone library, and see how much performance we can get. The second
>>> step is to discuss how to integrate GRO into DPDK. Therefore, if we agree to support
>>> GRO as a device feature, we need to discuss how to enable/disable this device feature.
>>> Once we reach an agreement, there will be a sample to demonstrate the integration.
>> 
>> I think that having a separate library for GRO is a step in a right direction.
>>> From my perspective - it provides a clean and flexible way to use that feature.
>> If later someone would like to put GRO into ethdev layer (or particular PMD),
>> he can use existing librte_gro for that.
> 
> Agree. I think introducing more flexibility is an important thing for applications.

Creating a new library just for GRO is not a reasonable solution, but adding that support to an existing library like librte_net would be cleaner and not create yet another library.

Creating more flexibility is not the best goal as we really want to make GRO easy and simple for the developer to use for any device without having to change his applications to take advantage of the feature. Some times providing more flexibility just means making it more complexed and more APIs the developer needs to understand. Providing GRO as a offload feature is the better direction as it makes it simple for an application to use.

If we provide GRO as a standard offload similar to the other offloads we currently have makes it easy for the developer. The best goal for a feature is the best performance for the application without having the application make even more APIs calls along with simple and easy to use.

> 
>> I didn't  have a closer look yet, but I think that caught my attention:
>> API fir the lib seems too IPv4/TCP oriented -
>> though I understand that the most common case and should be implemented first.
>> I wonder can we have it a bit more generic and extendable, so user can specify what combination of protocols
>> he is interested in (let say: ipv4/tcp,  ipv6/tcp, etc.).
>> Even if right now we'll have it implemented only for ipv4/tcp.
>> Then internally we can have some check is that supported or not and if yes setup things accordingly.
> 
> Indeed, current apis are too specific. It's not very friendly to applications.
> Maybe we can use macro to define the combination of protocols, like GRO_TCP_IPV4
> and GRO_UDP_IPV6; and provide a generic setup function and reassembly function.
> Both of them perform different operations according to the macro value inputted
> by the application.
> 
>> BTW, that's for 17.08, right?
> 
> Yes, it's for 17.08.
> 
> Jiayu
>> 
>> Konstantin
>> 
>> 

Regards,
Keith

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Yuanhan Liu]

On Fri, Mar 24, 2017 at 06:18:48AM +0000, Wiles, Keith wrote:
> >> I think that having a separate library for GRO is a step in a right direction.
> >>> From my perspective - it provides a clean and flexible way to use that feature.
> >> If later someone would like to put GRO into ethdev layer (or particular PMD),
> >> he can use existing librte_gro for that.
> > 
> > Agree. I think introducing more flexibility is an important thing for applications.
> 
> Creating a new library just for GRO is not a reasonable solution, but adding that support to an existing library like librte_net would be cleaner and not create yet another library.

Librte_net seems like a good suggestion to me, especially when we are
considering to add GSO in future. The only concern to me is "net" may
be too generic. It maybe kind of hard to decide which should be in
librte_net, and which should be added as a standalone lib. For example,
shouldn't 'lpm' and 'ip_frag' also belong to librte_net?

> Creating more flexibility is not the best goal as we really want to make GRO easy and simple for the developer to use for any device without having to change his applications to take advantage of the feature. Some times providing more flexibility just means making it more complexed and more APIs the developer needs to understand. Providing GRO as a offload feature is the better direction as it makes it simple for an application to use.
> 
> If we provide GRO as a standard offload similar to the other offloads we currently have makes it easy for the developer. The best goal for a feature is the best performance for the application without having the application make even more APIs calls along with simple and easy to use.

In general, I'd agree with you, if no one is object to add a short piece
of code at the end of rte_eth_rx_burst:

     +       if (eth_gro_is_enabled(dev))
     +               nb_rx = rte_net_gro(...);
     +
             return nb_rx;
      }

Objections?

But one way or another, we need put the gro code at somewhere and we
need introduce a generic API for that. It could be librte_net as you
suggested. So the good thing is that we all at least come an agreement
that it should be implemented in lib, right? The only controversy is
should we export it to application and let them to invoke it, or hide
it inside rte_eth_rx_burst.

Though it may take some time for all of us to come an agreement on that,
but the good thing is that it would be a very trivial change once it's
done. Agree?

Thus I'd suggest Jiayu to focus on the the GRO code developement, such
as making it generic enough and adding other protocols support. And I
would like to ask you guys to help review them. Makes sense to all?

Thanks.

	--yliu


> >> I didn't  have a closer look yet, but I think that caught my attention:
> >> API fir the lib seems too IPv4/TCP oriented -
> >> though I understand that the most common case and should be implemented first.
> >> I wonder can we have it a bit more generic and extendable, so user can specify what combination of protocols
> >> he is interested in (let say: ipv4/tcp,  ipv6/tcp, etc.).
> >> Even if right now we'll have it implemented only for ipv4/tcp.
> >> Then internally we can have some check is that supported or not and if yes setup things accordingly.
> > 
> > Indeed, current apis are too specific. It's not very friendly to applications.
> > Maybe we can use macro to define the combination of protocols, like GRO_TCP_IPV4
> > and GRO_UDP_IPV6; and provide a generic setup function and reassembly function.
> > Both of them perform different operations according to the macro value inputted
> > by the application.
> > 
> >> BTW, that's for 17.08, right?
> > 
> > Yes, it's for 17.08.
> > 
> > Jiayu
> >> 
> >> Konstantin
> >> 
> >> 
> 
> Regards,
> Keith

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Jiayu Hu]

On Fri, Mar 24, 2017 at 03:22:30PM +0800, Yuanhan Liu wrote:
> On Fri, Mar 24, 2017 at 06:18:48AM +0000, Wiles, Keith wrote:
> > >> I think that having a separate library for GRO is a step in a right direction.
> > >>> From my perspective - it provides a clean and flexible way to use that feature.
> > >> If later someone would like to put GRO into ethdev layer (or particular PMD),
> > >> he can use existing librte_gro for that.
> > > 
> > > Agree. I think introducing more flexibility is an important thing for applications.
> > 
> > Creating a new library just for GRO is not a reasonable solution, but adding that support to an existing library like librte_net would be cleaner and not create yet another library.
> 
> Librte_net seems like a good suggestion to me, especially when we are
> considering to add GSO in future. The only concern to me is "net" may
> be too generic. It maybe kind of hard to decide which should be in
> librte_net, and which should be added as a standalone lib. For example,
> shouldn't 'lpm' and 'ip_frag' also belong to librte_net?
> 
> > Creating more flexibility is not the best goal as we really want to make GRO easy and simple for the developer to use for any device without having to change his applications to take advantage of the feature. Some times providing more flexibility just means making it more complexed and more APIs the developer needs to understand. Providing GRO as a offload feature is the better direction as it makes it simple for an application to use.
> > 
> > If we provide GRO as a standard offload similar to the other offloads we currently have makes it easy for the developer. The best goal for a feature is the best performance for the application without having the application make even more APIs calls along with simple and easy to use.
> 
> In general, I'd agree with you, if no one is object to add a short piece
> of code at the end of rte_eth_rx_burst:
> 
>      +       if (eth_gro_is_enabled(dev))
>      +               nb_rx = rte_net_gro(...);
>      +
>              return nb_rx;
>       }
> 
> Objections?
> 
> But one way or another, we need put the gro code at somewhere and we
> need introduce a generic API for that. It could be librte_net as you
> suggested. So the good thing is that we all at least come an agreement
> that it should be implemented in lib, right? The only controversy is
> should we export it to application and let them to invoke it, or hide
> it inside rte_eth_rx_burst.
> 
> Though it may take some time for all of us to come an agreement on that,
> but the good thing is that it would be a very trivial change once it's
> done. Agree?

Agree.

> 
> Thus I'd suggest Jiayu to focus on the the GRO code developement, such
> as making it generic enough and adding other protocols support. And I
> would like to ask you guys to help review them. Makes sense to all?
> 

Agree again. No matter where to put GRO code, the apis should be generic
and extensible. And more protocols should be supported.

Thanks,
Jiayu

> Thanks.
> 
> 	--yliu
> 
> 
> > >> I didn't  have a closer look yet, but I think that caught my attention:
> > >> API fir the lib seems too IPv4/TCP oriented -
> > >> though I understand that the most common case and should be implemented first.
> > >> I wonder can we have it a bit more generic and extendable, so user can specify what combination of protocols
> > >> he is interested in (let say: ipv4/tcp,  ipv6/tcp, etc.).
> > >> Even if right now we'll have it implemented only for ipv4/tcp.
> > >> Then internally we can have some check is that supported or not and if yes setup things accordingly.
> > > 
> > > Indeed, current apis are too specific. It's not very friendly to applications.
> > > Maybe we can use macro to define the combination of protocols, like GRO_TCP_IPV4
> > > and GRO_UDP_IPV6; and provide a generic setup function and reassembly function.
> > > Both of them perform different operations according to the macro value inputted
> > > by the application.
> > > 
> > >> BTW, that's for 17.08, right?
> > > 
> > > Yes, it's for 17.08.
> > > 
> > > Jiayu
> > >> 
> > >> Konstantin
> > >> 
> > >> 
> > 
> > Regards,
> > Keith

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Ananyev, Konstantin]

> -----Original Message-----
> From: Hu, Jiayu
> Sent: Friday, March 24, 2017 8:07 AM
> To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
> Cc: Wiles, Keith <keith.wiles@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce
> <bruce.richardson@intel.com>; Stephen Hemminger <stephen@networkplumber.org>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> dev@dpdk.org; Liang, Cunming <cunming.liang@intel.com>; Thomas Monjalon <thomas.monjalon@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support
> 
> On Fri, Mar 24, 2017 at 03:22:30PM +0800, Yuanhan Liu wrote:
> > On Fri, Mar 24, 2017 at 06:18:48AM +0000, Wiles, Keith wrote:
> > > >> I think that having a separate library for GRO is a step in a right direction.
> > > >>> From my perspective - it provides a clean and flexible way to use that feature.
> > > >> If later someone would like to put GRO into ethdev layer (or particular PMD),
> > > >> he can use existing librte_gro for that.
> > > >
> > > > Agree. I think introducing more flexibility is an important thing for applications.
> > >
> > > Creating a new library just for GRO is not a reasonable solution, but adding that support to an existing library like librte_net would
> be cleaner and not create yet another library.
> >
> > Librte_net seems like a good suggestion to me, especially when we are
> > considering to add GSO in future. The only concern to me is "net" may
> > be too generic. It maybe kind of hard to decide which should be in
> > librte_net, and which should be added as a standalone lib. For example,
> > shouldn't 'lpm' and 'ip_frag' also belong to librte_net?

About librte_gro vs librte_net:

Right now librte_net is quite lightweight one - it mostly contains a net protocols definitions
plus some extra helper functions: to parse the l2/l3 headers to determine ptype, to calculate cksum, etc.
GRO code is quite different - it has to allocate and manage hash table(s), etc.
Again my understanding it would keep growing (with new proto support).
Again as mentioned above if GRO should go into librte_net, then librte_ipfrag and future GSO should also be here.
Which would create quite a monstrous library.  
So I think it is better to keep librte_net small and tidy and put GRO functionality into the new library.

> >
> > > Creating more flexibility is not the best goal as we really want to make GRO easy and simple for the developer to use for any
> device without having to change his applications to take advantage of the feature. Some times providing more flexibility just means
> making it more complexed and more APIs the developer needs to understand. Providing GRO as a offload feature is the better
> direction as it makes it simple for an application to use.
> > >
> > > If we provide GRO as a standard offload similar to the other offloads we currently have makes it easy for the developer. The best
> goal for a feature is the best performance for the application without having the application make even more APIs calls along with
> simple and easy to use.
> >
> > In general, I'd agree with you, if no one is object to add a short piece
> > of code at the end of rte_eth_rx_burst:
> >
> >      +       if (eth_gro_is_enabled(dev))
> >      +               nb_rx = rte_net_gro(...);
> >      +
> >              return nb_rx;
> >       }
> >
> > Objections?

I'd better not to open that door.
If we'll allow that for GRO - we'll have to allow that for every other stuff:
- ip reassembly
- l3/l4 cksum calculation if underlying HW doesn't support it
- SW ptype recognition
- etc.

Adding all these things into rx_burst() would most likely slow things down
(remember it is a data-path) and pretty soon would bring rx_burst() into
messy monster that would be hard to maintain and debug.    

My preference would be to keep rte_ethdev data-path as small and tidy as possible.
If in future we'd really like to introduce all these SW things into dev layer -
my preference would be to create some sort of new abstraction on top of current ethdev:
rte_eth_smartdev or so.
So it would be:
rte_eth_smartdev_rx_burst(....)
{
   nb_rx =  rte_eth_rx_burst(...);
   /* apply GRO, reassembly, etc. */
  ...
} 

Something similar with what 6Wind trying to introduce with their failsafe dev concept.

> >
> > But one way or another, we need put the gro code at somewhere and we
> > need introduce a generic API for that. It could be librte_net as you
> > suggested. So the good thing is that we all at least come an agreement
> > that it should be implemented in lib, right? The only controversy is
> > should we export it to application and let them to invoke it, or hide
> > it inside rte_eth_rx_burst.
> >
> > Though it may take some time for all of us to come an agreement on that,
> > but the good thing is that it would be a very trivial change once it's
> > done. Agree?
> 
> Agree.
> 
> >
> > Thus I'd suggest Jiayu to focus on the the GRO code developement, such
> > as making it generic enough and adding other protocols support. And I
> > would like to ask you guys to help review them. Makes sense to all?
> >
> 
> Agree again. No matter where to put GRO code, the apis should be generic
> and extensible. And more protocols should be supported.

Yep, that's what my take from the beginning:
Let's develop a librte_gro first and make it successful, then we can think should
we (and how) put into ethdev layer.

Konstantin

> 
> Thanks,
> Jiayu
> 
> > Thanks.
> >
> > 	--yliu
> >
> >
> > > >> I didn't  have a closer look yet, but I think that caught my attention:
> > > >> API fir the lib seems too IPv4/TCP oriented -
> > > >> though I understand that the most common case and should be implemented first.
> > > >> I wonder can we have it a bit more generic and extendable, so user can specify what combination of protocols
> > > >> he is interested in (let say: ipv4/tcp,  ipv6/tcp, etc.).
> > > >> Even if right now we'll have it implemented only for ipv4/tcp.
> > > >> Then internally we can have some check is that supported or not and if yes setup things accordingly.
> > > >
> > > > Indeed, current apis are too specific. It's not very friendly to applications.
> > > > Maybe we can use macro to define the combination of protocols, like GRO_TCP_IPV4
> > > > and GRO_UDP_IPV6; and provide a generic setup function and reassembly function.
> > > > Both of them perform different operations according to the macro value inputted
> > > > by the application.
> > > >
> > > >> BTW, that's for 17.08, right?
> > > >
> > > > Yes, it's for 17.08.
> > > >
> > > > Jiayu
> > > >>
> > > >> Konstantin
> > > >>
> > > >>
> > >
> > > Regards,
> > > Keith

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Wiles, Keith]

> On Mar 24, 2017, at 6:43 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Hu, Jiayu
>> Sent: Friday, March 24, 2017 8:07 AM
>> To: Yuanhan Liu <yuanhan.liu@linux.intel.com>
>> Cc: Wiles, Keith <keith.wiles@intel.com>; Ananyev, Konstantin <konstantin.ananyev@intel.com>; Richardson, Bruce
>> <bruce.richardson@intel.com>; Stephen Hemminger <stephen@networkplumber.org>; Yigit, Ferruh <ferruh.yigit@intel.com>;
>> dev@dpdk.org; Liang, Cunming <cunming.liang@intel.com>; Thomas Monjalon <thomas.monjalon@6wind.com>
>> Subject: Re: [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support
>> 
>> On Fri, Mar 24, 2017 at 03:22:30PM +0800, Yuanhan Liu wrote:
>>> On Fri, Mar 24, 2017 at 06:18:48AM +0000, Wiles, Keith wrote:
>>>>>> I think that having a separate library for GRO is a step in a right direction.
>>>>>>> From my perspective - it provides a clean and flexible way to use that feature.
>>>>>> If later someone would like to put GRO into ethdev layer (or particular PMD),
>>>>>> he can use existing librte_gro for that.
>>>>> 
>>>>> Agree. I think introducing more flexibility is an important thing for applications.
>>>> 
>>>> Creating a new library just for GRO is not a reasonable solution, but adding that support to an existing library like librte_net would
>> be cleaner and not create yet another library.
>>> 
>>> Librte_net seems like a good suggestion to me, especially when we are
>>> considering to add GSO in future. The only concern to me is "net" may
>>> be too generic. It maybe kind of hard to decide which should be in
>>> librte_net, and which should be added as a standalone lib. For example,
>>> shouldn't 'lpm' and 'ip_frag' also belong to librte_net?
> 
> About librte_gro vs librte_net:
> 
> Right now librte_net is quite lightweight one - it mostly contains a net protocols definitions
> plus some extra helper functions: to parse the l2/l3 headers to determine ptype, to calculate cksum, etc.
> GRO code is quite different - it has to allocate and manage hash table(s), etc.
> Again my understanding it would keep growing (with new proto support).
> Again as mentioned above if GRO should go into librte_net, then librte_ipfrag and future GSO should also be here.
> Which would create quite a monstrous library.  
> So I think it is better to keep librte_net small and tidy and put GRO functionality into the new library.

The size of a library is not a concern we have some pretty big ones already.

-rw-rw-r-- 2 rkwiles rkwiles 1.2M Mar 21 15:22 librte_acl.a
-rw-rw-r-- 1 rkwiles rkwiles  39K Mar 21 15:22 librte_cfgfile.a
-rw-rw-r-- 1 rkwiles rkwiles 292K Mar 21 15:22 librte_cmdline.a
-rw-rw-r-- 1 rkwiles rkwiles 211K Mar 21 15:23 librte_cryptodev.a
-rw-rw-r-- 1 rkwiles rkwiles  75K Mar 21 15:22 librte_distributor.a
-rw-rw-r-- 1 rkwiles rkwiles 1.4M Mar 21 15:22 librte_eal.a
-rw-rw-r-- 1 rkwiles rkwiles 323K Mar 21 15:23 librte_efd.a
-rw-rw-r-- 1 rkwiles rkwiles 374K Mar 22 09:31 librte_ethdev.a
-rw-rw-r-- 1 rkwiles rkwiles 675K Mar 21 15:22 librte_hash.a
-rw-rw-r-- 1 rkwiles rkwiles 366K Mar 21 15:23 librte_ip_frag.a
-rw-rw-r-- 1 rkwiles rkwiles  29K Mar 21 15:22 librte_jobstats.a
-rw-rw-r-- 1 rkwiles rkwiles 167K Mar 21 15:23 librte_kni.a
-rw-rw-r-- 1 rkwiles rkwiles  19K Mar 21 15:22 librte_kvargs.a
-rw-rw-r-- 1 rkwiles rkwiles 309K Mar 21 15:22 librte_lpm.a
-rw-rw-r-- 1 rkwiles rkwiles  93K Mar 21 15:22 librte_mbuf.a
-rw-rw-r-- 1 rkwiles rkwiles 270K Mar 21 15:22 librte_mempool.a
-rw-rw-r-- 1 rkwiles rkwiles  17K Mar 21 15:22 librte_meter.a
-rw-rw-r-- 1 rkwiles rkwiles  71K Mar 21 15:22 librte_net.a
-rw-rw-r-- 1 rkwiles rkwiles 211K Mar 21 15:23 librte_pdump.a
-rw-rw-r-- 1 rkwiles rkwiles 140K Mar 21 15:23 librte_pipeline.a
-rw-rw-r-- 1 rkwiles rkwiles 1.4M Mar 21 15:23 librte_port.a
-rw-rw-r-- 1 rkwiles rkwiles 122K Mar 21 15:22 librte_power.a
-rw-rw-r-- 1 rkwiles rkwiles 154K Mar 21 15:22 librte_reorder.a
-rw-rw-r-- 1 rkwiles rkwiles  63K Mar 21 15:22 librte_ring.a
-rw-rw-r-- 1 rkwiles rkwiles 377K Mar 21 15:23 librte_sched.a
-rw-rw-r-- 1 rkwiles rkwiles 1.4M Mar 21 15:23 librte_table.a
-rw-rw-r-- 1 rkwiles rkwiles  53K Mar 21 15:22 librte_timer.a
-rw-rw-r-- 1 rkwiles rkwiles 609K Mar 21 15:23 librte_vhost.a

Removed the PMD archives and ls is not a great way to get the true size compared to using ‘size’.

If you look at the size values, the ‘size *.a’ is interesting too.

The size of librte_net is 71K + ip_frag 366K is pretty small compared to a few others. I would assume GRO is pretty small too, so adding GRO into librte_net is very reasonable. We could leave ip_frag out as currently it is a standalone lib, but continue to add GSO to librte_net. I would not assume the size would that large and it seems like the best place to put the code.

If you still want to create a gso lib then I guess you can, just seems unreasonable to me.

> 
>>> 
>>>> Creating more flexibility is not the best goal as we really want to make GRO easy and simple for the developer to use for any
>> device without having to change his applications to take advantage of the feature. Some times providing more flexibility just means
>> making it more complexed and more APIs the developer needs to understand. Providing GRO as a offload feature is the better
>> direction as it makes it simple for an application to use.
>>>> 
>>>> If we provide GRO as a standard offload similar to the other offloads we currently have makes it easy for the developer. The best
>> goal for a feature is the best performance for the application without having the application make even more APIs calls along with
>> simple and easy to use.
>>> 
>>> In general, I'd agree with you, if no one is object to add a short piece
>>> of code at the end of rte_eth_rx_burst:
>>> 
>>>     +       if (eth_gro_is_enabled(dev))
>>>     +               nb_rx = rte_net_gro(...);
>>>     +
>>>             return nb_rx;
>>>      }
>>> 
>>> Objections?

Why do we need to modify every driver, why not use the RX callback feature then no drivers were harmed in this patch :-)

> 
> I'd better not to open that door.
> If we'll allow that for GRO - we'll have to allow that for every other stuff:
> - ip reassembly
> - l3/l4 cksum calculation if underlying HW doesn't support it
> - SW ptype recognition
> - etc.
> 
> Adding all these things into rx_burst() would most likely slow things down
> (remember it is a data-path) and pretty soon would bring rx_burst() into
> messy monster that would be hard to maintain and debug.    
> 
> My preference would be to keep rte_ethdev data-path as small and tidy as possible.
> If in future we'd really like to introduce all these SW things into dev layer -
> my preference would be to create some sort of new abstraction on top of current ethdev:
> rte_eth_smartdev or so.
> So it would be:
> rte_eth_smartdev_rx_burst(....)
> {
>   nb_rx =  rte_eth_rx_burst(...);
>   /* apply GRO, reassembly, etc. */
>  ...
> } 

Adding a new API is still not required, as the RX callback code is already in place for these types of post processing of mbufs.

> 
> Something similar with what 6Wind trying to introduce with their failsafe dev concept.

The 6Wind is a different story here and not a post processing of RX mbufs.

> 
>>> 
>>> But one way or another, we need put the gro code at somewhere and we
>>> need introduce a generic API for that. It could be librte_net as you
>>> suggested. So the good thing is that we all at least come an agreement
>>> that it should be implemented in lib, right? The only controversy is
>>> should we export it to application and let them to invoke it, or hide
>>> it inside rte_eth_rx_burst.
>>> 
>>> Though it may take some time for all of us to come an agreement on that,
>>> but the good thing is that it would be a very trivial change once it's
>>> done. Agree?
>> 
>> Agree.
>> 
>>> 
>>> Thus I'd suggest Jiayu to focus on the the GRO code developement, such
>>> as making it generic enough and adding other protocols support. And I
>>> would like to ask you guys to help review them. Makes sense to all?
>>> 
>> 
>> Agree again. No matter where to put GRO code, the apis should be generic
>> and extensible. And more protocols should be supported.
> 
> Yep, that's what my take from the beginning:
> Let's develop a librte_gro first and make it successful, then we can think should
> we (and how) put into ethdev layer.

Let not create a gro library and put the code into librte_net as size is not a concern yet and it is the best place to put the code. As for ip_frag someone can move it into librte_net if someone writes the patch.

> 
> Konstantin
> 
>> 
>> Thanks,
>> Jiayu
>> 
>>> Thanks.
>>> 
>>> 	--yliu
>>> 
>>> 
>>>>>> I didn't  have a closer look yet, but I think that caught my attention:
>>>>>> API fir the lib seems too IPv4/TCP oriented -
>>>>>> though I understand that the most common case and should be implemented first.
>>>>>> I wonder can we have it a bit more generic and extendable, so user can specify what combination of protocols
>>>>>> he is interested in (let say: ipv4/tcp,  ipv6/tcp, etc.).
>>>>>> Even if right now we'll have it implemented only for ipv4/tcp.
>>>>>> Then internally we can have some check is that supported or not and if yes setup things accordingly.
>>>>> 
>>>>> Indeed, current apis are too specific. It's not very friendly to applications.
>>>>> Maybe we can use macro to define the combination of protocols, like GRO_TCP_IPV4
>>>>> and GRO_UDP_IPV6; and provide a generic setup function and reassembly function.
>>>>> Both of them perform different operations according to the macro value inputted
>>>>> by the application.
>>>>> 
>>>>>> BTW, that's for 17.08, right?
>>>>> 
>>>>> Yes, it's for 17.08.
>>>>> 
>>>>> Jiayu
>>>>>> 
>>>>>> Konstantin
>>>>>> 
>>>>>> 
>>>> 
>>>> Regards,
>>>> Keith

Regards,
Keith

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Olivier Matz]

On Fri, 24 Mar 2017 14:37:04 +0000, "Wiles, Keith" <keith.wiles@intel.com> wrote:
> > On Mar 24, 2017, at 6:43 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
> > 
> > 
> >   

[...]

> > Yep, that's what my take from the beginning:
> > Let's develop a librte_gro first and make it successful, then we can think should
> > we (and how) put into ethdev layer.  
> 
> Let not create a gro library and put the code into librte_net as size is not a concern yet and it is the best place to put the code. As for ip_frag someone can move it into librte_net if someone writes the patch.

The size of a library _is_ an argument. Not the binary size in bytes, but
its API, because that's what the developper sees. Today, librte_net contains
protocol headers definitions and some network helpers, and the API surface
is already quite big (look at the number of lines of .h files).

I really like having a library name which matches its content.
The anwser to "what can I find in librte_gro?" is quite obvious.


Regards
Olivier

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Wiles, Keith]

> On Mar 24, 2017, at 9:59 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> 
> On Fri, 24 Mar 2017 14:37:04 +0000, "Wiles, Keith" <keith.wiles@intel.com> wrote:
>>> On Mar 24, 2017, at 6:43 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
>>> 
>>> 
>>> 
> 
> [...]
> 
>>> Yep, that's what my take from the beginning:
>>> Let's develop a librte_gro first and make it successful, then we can think should
>>> we (and how) put into ethdev layer.  
>> 
>> Let not create a gro library and put the code into librte_net as size is not a concern yet and it is the best place to put the code. As for ip_frag someone can move it into librte_net if someone writes the patch.
> 
> The size of a library _is_ an argument. Not the binary size in bytes, but
> its API, because that's what the developper sees. Today, librte_net contains
> protocol headers definitions and some network helpers, and the API surface
> is already quite big (look at the number of lines of .h files).
> 
> I really like having a library name which matches its content.
> The anwser to "what can I find in librte_gro?" is quite obvious.

If we are going to talk about API surface area lets talk about ethdev then :-)

Ok, lets create a new librte_gro, but I am not convinced it is reasonable. Maybe a better generic name is needed if we are going to add GSO to the library too. So a new name for the lib is better then librte_gro, unless you are going to create another library for GSO.

I still think the design needs to be integrated in as a real offload as I stated before and that is not something I am willing let drop.

> 
> 
> Regards
> Olivier

Regards,
Keith

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Wiles, Keith]

> On Mar 24, 2017, at 10:07 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
> 
>> 
>> On Mar 24, 2017, at 9:59 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
>> 
>> On Fri, 24 Mar 2017 14:37:04 +0000, "Wiles, Keith" <keith.wiles@intel.com> wrote:
>>>> On Mar 24, 2017, at 6:43 AM, Ananyev, Konstantin <konstantin.ananyev@intel.com> wrote:
>>>> 
>>>> 
>>>> 
>> 
>> [...]
>> 
>>>> Yep, that's what my take from the beginning:
>>>> Let's develop a librte_gro first and make it successful, then we can think should
>>>> we (and how) put into ethdev layer.  
>>> 
>>> Let not create a gro library and put the code into librte_net as size is not a concern yet and it is the best place to put the code. As for ip_frag someone can move it into librte_net if someone writes the patch.
>> 
>> The size of a library _is_ an argument. Not the binary size in bytes, but
>> its API, because that's what the developper sees. Today, librte_net contains
>> protocol headers definitions and some network helpers, and the API surface
>> is already quite big (look at the number of lines of .h files).
>> 
>> I really like having a library name which matches its content.
>> The anwser to "what can I find in librte_gro?" is quite obvious.
> 
> If we are going to talk about API surface area lets talk about ethdev then :-)
> 
> Ok, lets create a new librte_gro, but I am not convinced it is reasonable. Maybe a better generic name is needed if we are going to add GSO to the library too. So a new name for the lib is better then librte_gro, unless you are going to create another library for GSO.
> 
> I still think the design needs to be integrated in as a real offload as I stated before and that is not something I am willing let drop.

I guess we agree to create the library librte_gro and the current code needs to be updated to be include as a real offload support to DPDK as I see no real conclusion to this topic.

> 
>> 
>> 
>> Regards
>> Olivier
> 
> Regards,
> Keith

Regards,
Keith

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Hu, Jiayu]

> -----Original Message-----
> From: Wiles, Keith
> Sent: Tuesday, March 28, 2017 9:40 PM
> To: Olivier Matz <olivier.matz@6wind.com>
> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Hu, Jiayu
> <jiayu.hu@intel.com>; Yuanhan Liu <yuanhan.liu@linux.intel.com>;
> Richardson, Bruce <bruce.richardson@intel.com>; Stephen Hemminger
> <stephen@networkplumber.org>; Yigit, Ferruh <ferruh.yigit@intel.com>;
> dev@dpdk.org; Liang, Cunming <cunming.liang@intel.com>; Thomas
> Monjalon <thomas.monjalon@6wind.com>
> Subject: Re: [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support
> 
> 
> > On Mar 24, 2017, at 10:07 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
> >
> >>
> >> On Mar 24, 2017, at 9:59 AM, Olivier Matz <olivier.matz@6wind.com>
> wrote:
> >>
> >> On Fri, 24 Mar 2017 14:37:04 +0000, "Wiles, Keith"
> <keith.wiles@intel.com> wrote:
> >>>> On Mar 24, 2017, at 6:43 AM, Ananyev, Konstantin
> <konstantin.ananyev@intel.com> wrote:
> >>>>
> >>>>
> >>>>
> >>
> >> [...]
> >>
> >>>> Yep, that's what my take from the beginning:
> >>>> Let's develop a librte_gro first and make it successful, then we can think
> should
> >>>> we (and how) put into ethdev layer.
> >>>
> >>> Let not create a gro library and put the code into librte_net as size is not
> a concern yet and it is the best place to put the code. As for ip_frag someone
> can move it into librte_net if someone writes the patch.
> >>
> >> The size of a library _is_ an argument. Not the binary size in bytes, but
> >> its API, because that's what the developper sees. Today, librte_net
> contains
> >> protocol headers definitions and some network helpers, and the API
> surface
> >> is already quite big (look at the number of lines of .h files).
> >>
> >> I really like having a library name which matches its content.
> >> The anwser to "what can I find in librte_gro?" is quite obvious.
> >
> > If we are going to talk about API surface area lets talk about ethdev then :-)
> >
> > Ok, lets create a new librte_gro, but I am not convinced it is reasonable.
> Maybe a better generic name is needed if we are going to add GSO to the
> library too. So a new name for the lib is better then librte_gro, unless you are
> going to create another library for GSO.
> >
> > I still think the design needs to be integrated in as a real offload as I stated
> before and that is not something I am willing let drop.
> 
> I guess we agree to create the library librte_gro and the current code needs
> to be updated to be include as a real offload support to DPDK as I see no real
> conclusion to this topic.

OK, I have known your opinions, and I agree with you. I will provide a real offloading
example to demonstrate the usage of librte_gro in the next patch.

Thanks,
Jiayu
> 
> >
> >>
> >>
> >> Regards
> >> Olivier
> >
> > Regards,
> > Keith
> 
> Regards,
> Keith

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Wiles, Keith]

> On Mar 28, 2017, at 8:57 AM, Hu, Jiayu <jiayu.hu@intel.com> wrote:
> 
> 
> 
>> -----Original Message-----
>> From: Wiles, Keith
>> Sent: Tuesday, March 28, 2017 9:40 PM
>> To: Olivier Matz <olivier.matz@6wind.com>
>> Cc: Ananyev, Konstantin <konstantin.ananyev@intel.com>; Hu, Jiayu
>> <jiayu.hu@intel.com>; Yuanhan Liu <yuanhan.liu@linux.intel.com>;
>> Richardson, Bruce <bruce.richardson@intel.com>; Stephen Hemminger
>> <stephen@networkplumber.org>; Yigit, Ferruh <ferruh.yigit@intel.com>;
>> dev@dpdk.org; Liang, Cunming <cunming.liang@intel.com>; Thomas
>> Monjalon <thomas.monjalon@6wind.com>
>> Subject: Re: [dpdk-dev] [PATCH 0/2] lib: add TCP IPv4 GRO support
>> 
>> 
>>> On Mar 24, 2017, at 10:07 AM, Wiles, Keith <keith.wiles@intel.com> wrote:
>>> 
>>>> 
>>>> On Mar 24, 2017, at 9:59 AM, Olivier Matz <olivier.matz@6wind.com>
>> wrote:
>>>> 
>>>> On Fri, 24 Mar 2017 14:37:04 +0000, "Wiles, Keith"
>> <keith.wiles@intel.com> wrote:
>>>>>> On Mar 24, 2017, at 6:43 AM, Ananyev, Konstantin
>> <konstantin.ananyev@intel.com> wrote:
>>>>>> 
>>>>>> 
>>>>>> 
>>>> 
>>>> [...]
>>>> 
>>>>>> Yep, that's what my take from the beginning:
>>>>>> Let's develop a librte_gro first and make it successful, then we can think
>> should
>>>>>> we (and how) put into ethdev layer.
>>>>> 
>>>>> Let not create a gro library and put the code into librte_net as size is not
>> a concern yet and it is the best place to put the code. As for ip_frag someone
>> can move it into librte_net if someone writes the patch.
>>>> 
>>>> The size of a library _is_ an argument. Not the binary size in bytes, but
>>>> its API, because that's what the developper sees. Today, librte_net
>> contains
>>>> protocol headers definitions and some network helpers, and the API
>> surface
>>>> is already quite big (look at the number of lines of .h files).
>>>> 
>>>> I really like having a library name which matches its content.
>>>> The anwser to "what can I find in librte_gro?" is quite obvious.
>>> 
>>> If we are going to talk about API surface area lets talk about ethdev then :-)
>>> 
>>> Ok, lets create a new librte_gro, but I am not convinced it is reasonable.
>> Maybe a better generic name is needed if we are going to add GSO to the
>> library too. So a new name for the lib is better then librte_gro, unless you are
>> going to create another library for GSO.
>>> 
>>> I still think the design needs to be integrated in as a real offload as I stated
>> before and that is not something I am willing let drop.
>> 
>> I guess we agree to create the library librte_gro and the current code needs
>> to be updated to be include as a real offload support to DPDK as I see no real
>> conclusion to this topic.
> 
> OK, I have known your opinions, and I agree with you. I will provide a real offloading
> example to demonstrate the usage of librte_gro in the next patch.

Thanks

> 
> Thanks,
> Jiayu
>> 
>>> 
>>>> 
>>>> 
>>>> Regards
>>>> Olivier
>>> 
>>> Regards,
>>> Keith
>> 
>> Regards,
>> Keith

Regards,
Keith

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Morten Brørup]

I have two points to this discussion:

GRO/LRO must be disabled by default! Truly transparent network appliances, such as classic Layer 3 routers and Layer 2 switches (and our SmartShare StraightShaper appliance), may not want to merge multiple smaller packets into one larger packet, regardless of any processing performance benefit. Also refer to Wikipedia (https://en.wikipedia.org/wiki/Large_receive_offload), especially reference 9 (https://bugzilla.redhat.com/show_bug.cgi?id=772317). And if any of you developers don't intuitively agree, just ask any old network engineer about all the mess he had to deal with "back in the days" when packet sizes were important... flaky path MTU discovery, MSS clamping for PPPoE and VPN tunnels, packet reassembly causing performance degradation and packet loss on underpowered VPN routers, etc.

We should consider librte_net a library for miscellaneous utilities, i.e. mainly stateless functions. Functions for packet merging (of multiple IP packets or actual IP fragments), which clearly requires a lot of memory and statefulness, does not belong here. This is worth noting, not only for the GRO/GSO library, but for similar future discussions. (The size of the compiled library is irrelevant - only its purpose matters.)


Med venlig hilsen / kind regards

Morten Brørup
CTO


SmartShare Systems A/S
Tonsbakken 16-18
DK-2740 Skovlunde
Denmark

Office      +45 70 20 00 93
Direct      +45 89 93 50 22
Mobile     +45 25 40 82 12

mb@smartsharesystems.com
www.smartsharesystems.com

Re: [PATCH 0/2] lib: add TCP IPv4 GRO support

[Wiles, Keith]

> On Mar 29, 2017, at 5:47 AM, Morten Brørup <mb@smartsharesystems.com> wrote:
> 
> I have two points to this discussion:
> 
> GRO/LRO must be disabled by default! Truly transparent network appliances, such as classic Layer 3 routers and Layer 2 switches (and our SmartShare StraightShaper appliance), may not want to merge multiple smaller packets into one larger packet, regardless of any processing performance benefit. Also refer to Wikipedia (https://en.wikipedia.org/wiki/Large_receive_offload), especially reference 9 (https://bugzilla.redhat.com/show_bug.cgi?id=772317). And if any of you developers don't intuitively agree, just ask any old network engineer about all the mess he had to deal with "back in the days" when packet sizes were important... flaky path MTU discovery, MSS clamping for PPPoE and VPN tunnels, packet reassembly causing performance degradation and packet loss on underpowered VPN routers, etc.

I agree we should not have any offload enabled by default and I assumed that was not the case here unless I missed something. My point was to make GRO more transparent when enabled instead of the application having to use a different API for GRO when we can put this feature inline with current rx_burst APIs. Using the RX callbacks or some other method as long as the performance of the current rx_burst APIs is not effected when the GRO is not enabled.

> 
> We should consider librte_net a library for miscellaneous utilities, i.e. mainly stateless functions. Functions for packet merging (of multiple IP packets or actual IP fragments), which clearly requires a lot of memory and statefulness, does not belong here. This is worth noting, not only for the GRO/GSO library, but for similar future discussions. (The size of the compiled library is irrelevant - only its purpose matters.)

I already decided to except the new library, but the name is not great as other features could be added to the new library like LRO. If we name it GRO then it can only be GRO.

> 
> 
> Med venlig hilsen / kind regards
> 
> Morten Brørup
> CTO
> 
> 
> SmartShare Systems A/S
> Tonsbakken 16-18
> DK-2740 Skovlunde
> Denmark
> 
> Office      +45 70 20 00 93
> Direct      +45 89 93 50 22
> Mobile     +45 25 40 82 12
> 
> mb@smartsharesystems.com
> www.smartsharesystems.com

Regards,
Keith

[PATCH v2 0/3] support GRO in DPDK

[Jiayu Hu]

Generic Receive Offload (GRO) is a widely used SW-based offloading
technique to reduce per-packet processing overhead. It gains performance
by reassembling small packets into large ones. Therefore, we propose to
add GRO support in DPDK.

DPDK GRO is designed as a device ability, which is turned off by default.
The unit to enable/disable GRO is port. And once a port is enabled GRO,
all of its queues will reassemble packets as many as possible.

For applications, the procedure of merging packets is entirely invisible.
To use GRO, they just need to decide which ports need/needn't GRO and
invoke GRO enabling/disabling functions for these ports. For a port, if
it's enabled GRO, one generic reassembly function is registered as a RX
callback for all of its queues. That is, the reassembly procedure is
performed inside rte_eth_rx_burst.

This patchset is to support GRO in DPDK. The first patch is to provide a
GRO API framework, which enables applications to use GRO ability and
enable developers to add GRO supports for specific protocols. The second
patch supports TCP/IPv4 GRO. The last patch demonstrates how to use GRO
ability in app/testpmd.

We perform two iperf tests (with DPDK GRO and without DPDK GRO) to see
the performance gains from DPDK GRO. Specifically, the experiment
environment is:
a. Two 10Gbps physical ports (p0 and p1) on one host are linked together;
b. p0 is in networking namespace ns1, whose IP is 1.1.2.3. iperf client
runs on p0, which sends TCP/IPv4 packets;
c. testpmd runs on p1. Besides, testpmd has a vdev which connects to a
VM via vhost-user and virtio-kernel. The VM runs iperf server, whose IP
is 1.1.2.4;
d. p0 turns on TSO; VM turns off kernel GRO; testpmd runs in iofwd mode.
iperf client and server use the following commands:
	- client: ip netns exec ns1 iperf -c 1.1.2.4 -i2 -t 60 -f g -m
	- server: iperf -s -f g
Two test cases are:
a. w/o DPDK GRO: run testpmd without GRO
b. w DPDK GRO: testpmd enables GRO for p1
Result:
With GRO, the throughput improvement is around 50%.

Change log
==========
v2:
- provide generic reassembly function;
- implement GRO as a device ability:
add APIs for devices to support GRO;
add APIs for applications to enable/disable GRO;
- update testpmd example. 

Jiayu Hu (3):
  lib: add Generic Receive Offload API framework
  lib/gro: add TCP/IPv4 GRO support
  app/testpmd: enable GRO feature

 app/test-pmd/cmdline.c          |  45 ++++++
 app/test-pmd/config.c           |  26 ++++
 app/test-pmd/iofwd.c            |   1 +
 app/test-pmd/testpmd.c          |   5 +
 app/test-pmd/testpmd.h          |   3 +
 config/common_base              |   5 +
 lib/Makefile                    |   1 +
 lib/librte_gro/Makefile         |  51 +++++++
 lib/librte_gro/rte_gro.c        | 293 ++++++++++++++++++++++++++++++++++++++++
 lib/librte_gro/rte_gro.h        |  29 ++++
 lib/librte_gro/rte_gro_common.h |  77 +++++++++++
 lib/librte_gro/rte_gro_tcp.c    | 270 ++++++++++++++++++++++++++++++++++++
 lib/librte_gro/rte_gro_tcp.h    |  95 +++++++++++++
 mk/rte.app.mk                   |   1 +
 14 files changed, 902 insertions(+)
 create mode 100644 lib/librte_gro/Makefile
 create mode 100644 lib/librte_gro/rte_gro.c
 create mode 100644 lib/librte_gro/rte_gro.h
 create mode 100644 lib/librte_gro/rte_gro_common.h
 create mode 100644 lib/librte_gro/rte_gro_tcp.c
 create mode 100644 lib/librte_gro/rte_gro_tcp.h

-- 
2.7.4

[PATCH v2 1/3] lib: add Generic Receive Offload API framework

[Jiayu Hu]

In DPDK, GRO is a device ability. The unit of enabling/disabling GRO is
port. To support GRO, this patch implements a GRO API framework, which
includes two parts. One is external functions provided to applications to
use GRO ability; the other is a generic reassembly function provided to
devices.

For applications, DPDK GRO provides three external functions to
enable/disable GRO:
- rte_gro_init: initialize GRO environment;
- rte_gro_enable: enable GRO for all queues of a given port;
- rte_gro_disable: disable GRO for all queues of a given port.
Before using GRO, applications should explicitly call rte_gro_init to
initizalize GRO environment. After that, applications can call
rte_gro_enable to enable GRO and call rte_gro_disable to disable GRO for
specific ports.

DPDK GRO has a generic reassembly function, rte_gro_reassemble_burst,
which processes all inputted packets in a burst-mode. If a port is
enabled GRO, rte_gro_reassemble_burst is registered as a RX callback for
all queues of this port; if the port wants to disable GRO, all the 
callbacks of its queues will be removed. Therefore, GRO procedure is
performed in ethdev layer.

In DPDK GRO, we name GRO types according to packet types, like TCP/IPV4
GRO. Each GRO type has a reassembly function, which is in charge of
processing packets of own type. Each reassembly function uses a hashing
table to merge packets. The structures of hashing table differ from GRO
types. That is, each GRO type defines own hashing table structure.
rte_gro_reassemble_burst calls these specific reassembly functions
according to packet types, and packets with unsupported protocols types
are not processed.

Signed-off-by: Jiayu Hu <jiayu.hu@intel.com>
---
 config/common_base              |   5 +
 lib/Makefile                    |   1 +
 lib/librte_gro/Makefile         |  50 ++++++++++
 lib/librte_gro/rte_gro.c        | 216 ++++++++++++++++++++++++++++++++++++++++
 lib/librte_gro/rte_gro.h        |  29 ++++++
 lib/librte_gro/rte_gro_common.h |  75 ++++++++++++++
 mk/rte.app.mk                   |   1 +
 7 files changed, 377 insertions(+)
 create mode 100644 lib/librte_gro/Makefile
 create mode 100644 lib/librte_gro/rte_gro.c
 create mode 100644 lib/librte_gro/rte_gro.h
 create mode 100644 lib/librte_gro/rte_gro_common.h

diff --git a/config/common_base b/config/common_base
index 41191c8..720dbc4 100644
--- a/config/common_base
+++ b/config/common_base
@@ -612,6 +612,11 @@ CONFIG_RTE_LIBRTE_VHOST_DEBUG=n
 CONFIG_RTE_LIBRTE_PMD_VHOST=n
 
 #
+# Compile GRO library
+#
+CONFIG_RTE_LIBRTE_GRO=y
+
+#
 #Compile Xen domain0 support
 #
 CONFIG_RTE_LIBRTE_XEN_DOM0=n
diff --git a/lib/Makefile b/lib/Makefile
index 531b162..74637c7 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -98,6 +98,7 @@ DIRS-$(CONFIG_RTE_LIBRTE_REORDER) += librte_reorder
 DEPDIRS-librte_reorder := librte_eal librte_mempool librte_mbuf
 DIRS-$(CONFIG_RTE_LIBRTE_PDUMP) += librte_pdump
 DEPDIRS-librte_pdump := librte_eal librte_mempool librte_mbuf librte_ether
+DIRS-$(CONFIG_RTE_LIBRTE_GRO) += librte_gro
 
 ifeq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),y)
 DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
diff --git a/lib/librte_gro/Makefile b/lib/librte_gro/Makefile
new file mode 100644
index 0000000..fb3a36c
--- /dev/null
+++ b/lib/librte_gro/Makefile
@@ -0,0 +1,50 @@
+#   BSD LICENSE
+#
+#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+#   All rights reserved.
+#
+#   Redistribution and use in source and binary forms, with or without
+#   modification, are permitted provided that the following conditions
+#   are met:
+#
+#     * Redistributions of source code must retain the above copyright
+#       notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above copyright
+#       notice, this list of conditions and the following disclaimer in
+#       the documentation and/or other materials provided with the
+#       distribution.
+#     * Neither the name of Intel Corporation nor the names of its
+#       contributors may be used to endorse or promote products derived
+#       from this software without specific prior written permission.
+#
+#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+#   "AS IS" AND ANY EX