[dpdk-dev] [PATCH v6 0/9] Support VxLAN on Fortville

[dpdk-dev] [PATCH v6 0/9] Support VxLAN on Fortville

[Jijiang Liu]

The patch set supports VxLAN on Fortville based on latest rte_mbuf structure.

It includes:
 - Support VxLAN packet identification by configuring UDP tunneling port.
 - Support VxLAN packet filters. It uses MAC and VLAN to point
   to a queue. The filter types supported are listed below:
   1. Inner MAC and Inner VLAN ID
   2. Inner MAC address, inner VLAN ID and tenant ID.
   3. Inner MAC and tenant ID
   4. Inner MAC address
   5. Outer MAC address, tenant ID and inner MAC
 - Support VxLAN TX checksum offload, which include outer L3(IP), inner L3(IP) and inner L4(UDP,TCP and SCTP)
 
Change notes:
 
 v6)  * Split the rte_mbuf structure changes as a seperate patch.
      * Remove the initialization configuration of VXLAN UDP port.
      * Change the filter_type field in rte_eth_tunnel_filter_conf to "uint16_t" type. 
      * Add more descriptions about some API comments and commit logs.


Jijiang Liu (9):
  rte_mbuf structure changes
  add VxLAN packet identification API in librte_ether
  support VxLAN packet identification in librte_pmd_i40e
  test VxLAN packet identification in testpmd.
  add data structures of tunneling filter in rte_eth_ctrl.h
  implement the API of VxLAN packet filter in librte_pmd_i40e
  test VxLAN packet filter
  support VxLAN Tx checksum offload
  test VxLAN Tx checksum offload

 app/test-pmd/cmdline.c            |  230 ++++++++++++++++++++++++-
 app/test-pmd/config.c             |    6 +-
 app/test-pmd/csumonly.c           |  195 +++++++++++++++++++--
 app/test-pmd/parameters.c         |   13 ++
 app/test-pmd/rxonly.c             |   49 ++++++
 app/test-pmd/testpmd.c            |    8 +
 app/test-pmd/testpmd.h            |    4 +
 lib/librte_ether/rte_eth_ctrl.h   |   64 +++++++
 lib/librte_ether/rte_ethdev.c     |   63 +++++++
 lib/librte_ether/rte_ethdev.h     |   63 +++++++
 lib/librte_ether/rte_ether.h      |    8 +
 lib/librte_mbuf/rte_mbuf.h        |   26 +++-
 lib/librte_pmd_i40e/i40e_ethdev.c |  341 ++++++++++++++++++++++++++++++++++++-
 lib/librte_pmd_i40e/i40e_ethdev.h |    8 +-
 lib/librte_pmd_i40e/i40e_rxtx.c   |   55 ++++++-
 15 files changed, 1101 insertions(+), 32 deletions(-)

-- 
1.7.7.6

[dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Jijiang Liu]

Remove the "reserved2" field and add the "packet_type" and the "inner_l2_l3_len" fields in the rte_mbuf structure.

The packet type field is used to indicate ordinary L2 packet format and also tunneling packet format such as IP in IP,
IP in GRE, MAC in GRE and MAC in UDP.

The inner L2 length and the inner L3 length are used for TX offloading of tunneling packet.
 
Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
---
 lib/librte_mbuf/rte_mbuf.h |   25 ++++++++++++++++++++++++-
 1 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index ddadc21..98951a6 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -163,7 +163,14 @@ struct rte_mbuf {
 
 	/* remaining bytes are set on RX when pulling packet from descriptor */
 	MARKER rx_descriptor_fields1;
-	uint16_t reserved2;       /**< Unused field. Required for padding */
+
+	/**
+	 * Packet type, which is used to indicate ordinary L2 packet format and
+	 * also tunneled packet format such as IP in IP, IP in GRE, MAC in GRE
+	 * and MAC in UDP.
+	 */
+	uint16_t packet_type;
+
 	uint16_t data_len;        /**< Amount of data in segment buffer. */
 	uint32_t pkt_len;         /**< Total pkt len: sum of all segments. */
 	uint16_t vlan_tci;        /**< VLAN Tag Control Identifier (CPU order) */
@@ -196,6 +203,18 @@ struct rte_mbuf {
 			uint16_t l2_len:7;      /**< L2 (MAC) Header Length. */
 		};
 	};
+
+	/* fields for TX offloading of tunnels */
+	union {
+		uint16_t inner_l2_l3_len;
+		/**< combined inner l2/l3 lengths as single var */
+		struct {
+			uint16_t inner_l3_len:9;
+			/**< inner L3 (IP) Header Length. */
+			uint16_t inner_l2_len:7;
+			/**< inner L2 (MAC) Header Length. */
+		};
+	};
 } __rte_cache_aligned;
 
 /**
@@ -546,11 +565,13 @@ static inline void rte_pktmbuf_reset(struct rte_mbuf *m)
 	m->next = NULL;
 	m->pkt_len = 0;
 	m->l2_l3_len = 0;
+	m->inner_l2_l3_len = 0;
 	m->vlan_tci = 0;
 	m->nb_segs = 1;
 	m->port = 0xff;
 
 	m->ol_flags = 0;
+	m->packet_type = 0;
 	m->data_off = (RTE_PKTMBUF_HEADROOM <= m->buf_len) ?
 			RTE_PKTMBUF_HEADROOM : m->buf_len;
 
@@ -614,12 +635,14 @@ static inline void rte_pktmbuf_attach(struct rte_mbuf *mi, struct rte_mbuf *md)
 	mi->port = md->port;
 	mi->vlan_tci = md->vlan_tci;
 	mi->l2_l3_len = md->l2_l3_len;
+	mi->inner_l2_l3_len = md->inner_l2_l3_len;
 	mi->hash = md->hash;
 
 	mi->next = NULL;
 	mi->pkt_len = mi->data_len;
 	mi->nb_segs = 1;
 	mi->ol_flags = md->ol_flags;
+	mi->packet_type = md->packet_type;
 
 	__rte_mbuf_sanity_check(mi, 1);
 	__rte_mbuf_sanity_check(md, 0);
-- 
1.7.7.6

[dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet identification API in librte_ether

[Jijiang Liu]

There are "some" destination UDP port numbers that have unque meaning.
In terms of VxLAN, "IANA has assigned the value 4789 for the VXLAN UDP port, and this value
SHOULD be used by default as the destination UDP port. Some early implementations of VXLAN
have used other values for the destination port. To enable interoperability with these 
implementations, the destination port SHOULD be configurable."

Add two APIs in librte_ether for supporting UDP tunneling port configuration on i40e.
Currently, only VxLAN is implemented in this patch set.
 
Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
---
 lib/librte_ether/rte_ethdev.c |   63 ++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.h |   75 +++++++++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ether.h  |    8 ++++
 3 files changed, 146 insertions(+), 0 deletions(-)

diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 50f10d9..9e111b6 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -2038,6 +2038,69 @@ rte_eth_dev_rss_hash_conf_get(uint8_t port_id,
 }
 
 int
+rte_eth_dev_udp_tunnel_add(uint8_t port_id,
+			   struct rte_eth_udp_tunnel *udp_tunnel,
+			   uint8_t count)
+{
+	uint8_t i;
+	struct rte_eth_dev *dev;
+	struct rte_eth_udp_tunnel *tunnel;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return -ENODEV;
+	}
+
+	if (udp_tunnel == NULL) {
+		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
+		return -EINVAL;
+	}
+	tunnel = udp_tunnel;
+
+	for (i = 0; i < count; i++, tunnel++) {
+		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
+			PMD_DEBUG_TRACE("Invalid tunnel type\n");
+			return -EINVAL;
+		}
+	}
+
+	dev = &rte_eth_devices[port_id];
+	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->udp_tunnel_add, -ENOTSUP);
+	return (*dev->dev_ops->udp_tunnel_add)(dev, udp_tunnel, count);
+}
+
+int
+rte_eth_dev_udp_tunnel_delete(uint8_t port_id,
+			      struct rte_eth_udp_tunnel *udp_tunnel,
+			      uint8_t count)
+{
+	uint8_t i;
+	struct rte_eth_dev *dev;
+	struct rte_eth_udp_tunnel *tunnel;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return -ENODEV;
+	}
+	dev = &rte_eth_devices[port_id];
+
+	if (udp_tunnel == NULL) {
+		PMD_DEBUG_TRACE("Invalid udp_tunnel parametr\n");
+		return -EINVAL;
+	}
+	tunnel = udp_tunnel;
+	for (i = 0; i < count; i++, tunnel++) {
+		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
+			PMD_DEBUG_TRACE("Invalid tunnel type\n");
+			return -EINVAL;
+		}
+	}
+
+	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->udp_tunnel_del, -ENOTSUP);
+	return (*dev->dev_ops->udp_tunnel_del)(dev, udp_tunnel, count);
+}
+
+int
 rte_eth_led_on(uint8_t port_id)
 {
 	struct rte_eth_dev *dev;
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index b69a6af..9ad11ec 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -710,6 +710,26 @@ struct rte_fdir_conf {
 };
 
 /**
+ * UDP tunneling configuration.
+ */
+struct rte_eth_udp_tunnel {
+	uint16_t udp_port;
+	uint8_t prot_type;
+};
+
+/**
+ * Tunneled type.
+ */
+enum rte_eth_tunnel_type {
+	RTE_TUNNEL_TYPE_NONE = 0,
+	RTE_TUNNEL_TYPE_VXLAN,
+	RTE_TUNNEL_TYPE_GENEVE,
+	RTE_TUNNEL_TYPE_TEREDO,
+	RTE_TUNNEL_TYPE_NVGRE,
+	RTE_TUNNEL_TYPE_MAX,
+};
+
+/**
  *  Possible l4type of FDIR filters.
  */
 enum rte_l4type {
@@ -831,6 +851,7 @@ struct rte_intr_conf {
  * configuration settings may be needed.
  */
 struct rte_eth_conf {
+	enum rte_eth_tunnel_type tunnel_type;
 	uint16_t link_speed;
 	/**< ETH_LINK_SPEED_10[0|00|000], or 0 for autonegotation */
 	uint16_t link_duplex;
@@ -1266,6 +1287,17 @@ typedef int (*eth_mirror_rule_reset_t)(struct rte_eth_dev *dev,
 				  uint8_t rule_id);
 /**< @internal Remove a traffic mirroring rule on an Ethernet device */
 
+typedef int (*eth_udp_tunnel_add_t)(struct rte_eth_dev *dev,
+				    struct rte_eth_udp_tunnel *tunnel_udp,
+				    uint8_t count);
+/**< @internal Add tunneling UDP info */
+
+typedef int (*eth_udp_tunnel_del_t)(struct rte_eth_dev *dev,
+				    struct rte_eth_udp_tunnel *tunnel_udp,
+				    uint8_t count);
+/**< @internal Delete tunneling UDP info */
+
+
 #ifdef RTE_NIC_BYPASS
 
 enum {
@@ -1446,6 +1478,8 @@ struct eth_dev_ops {
 	eth_set_vf_rx_t            set_vf_rx;  /**< enable/disable a VF receive */
 	eth_set_vf_tx_t            set_vf_tx;  /**< enable/disable a VF transmit */
 	eth_set_vf_vlan_filter_t   set_vf_vlan_filter;  /**< Set VF VLAN filter */
+	eth_udp_tunnel_add_t       udp_tunnel_add;
+	eth_udp_tunnel_del_t       udp_tunnel_del;
 	eth_set_queue_rate_limit_t set_queue_rate_limit;   /**< Set queue rate limit */
 	eth_set_vf_rate_limit_t    set_vf_rate_limit;   /**< Set VF rate limit */
 
@@ -3342,6 +3376,47 @@ int
 rte_eth_dev_rss_hash_conf_get(uint8_t port_id,
 			      struct rte_eth_rss_conf *rss_conf);
 
+ /**
+ * Add UDP tunneling port of an Ethernet device for filtering a specific
+ * tunneling packet by UDP port number.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param tunnel_udp
+ *   UDP tunneling configuration.
+ * @param count
+ *   configuration count.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if port identifier is invalid.
+ *   - (-ENOTSUP) if hardware doesn't support tunnel type.
+ */
+int
+rte_eth_dev_udp_tunnel_add(uint8_t port_id,
+			   struct rte_eth_udp_tunnel *tunnel_udp,
+			   uint8_t count);
+
+ /**
+ * Detete UDP tunneling port configuration of Ethernet device
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param tunnel_udp
+ *   UDP tunneling configuration.
+ * @param count
+ *   configuration count.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENODEV) if port identifier is invalid.
+ *   - (-ENOTSUP) if hardware doesn't support tunnel type.
+ */
+int
+rte_eth_dev_udp_tunnel_delete(uint8_t port_id,
+			      struct rte_eth_udp_tunnel *tunnel_udp,
+			      uint8_t count);
+
 /**
  * add syn filter
  *
diff --git a/lib/librte_ether/rte_ether.h b/lib/librte_ether/rte_ether.h
index 2e08f23..ddbdbb3 100644
--- a/lib/librte_ether/rte_ether.h
+++ b/lib/librte_ether/rte_ether.h
@@ -286,6 +286,12 @@ struct vlan_hdr {
 	uint16_t eth_proto;/**< Ethernet type of encapsulated frame. */
 } __attribute__((__packed__));
 
+/* VXLAN protocol header */
+struct vxlan_hdr {
+	uint32_t vx_flags; /**< VxLAN flag. */
+	uint32_t vx_vni;   /**< VxLAN ID. */
+} __attribute__((__packed__));
+
 /* Ethernet frame types */
 #define ETHER_TYPE_IPv4 0x0800 /**< IPv4 Protocol. */
 #define ETHER_TYPE_IPv6 0x86DD /**< IPv6 Protocol. */
@@ -294,6 +300,8 @@ struct vlan_hdr {
 #define ETHER_TYPE_VLAN 0x8100 /**< IEEE 802.1Q VLAN tagging. */
 #define ETHER_TYPE_1588 0x88F7 /**< IEEE 802.1AS 1588 Precise Time Protocol. */
 
+#define ETHER_VXLAN_HLEN (sizeof(struct udp_hdr) + sizeof(struct vxlan_hdr))
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.7.7.6

[dpdk-dev] [PATCH v6 3/9] i40e:support VxLAN packet identification in librte_pmd_i40e

[Jijiang Liu]

Implement configuration of VxLAN destination UDP port number in librte_pmd_i40e.
 
Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
---
 lib/librte_pmd_i40e/i40e_ethdev.c |  164 +++++++++++++++++++++++++++++++++++++
 lib/librte_pmd_i40e/i40e_ethdev.h |    8 ++-
 lib/librte_pmd_i40e/i40e_rxtx.c   |    9 ++
 3 files changed, 180 insertions(+), 1 deletions(-)

diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 3b75f0f..8a84e30 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -186,6 +186,12 @@ static int i40e_dev_rss_hash_update(struct rte_eth_dev *dev,
 				    struct rte_eth_rss_conf *rss_conf);
 static int i40e_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 				      struct rte_eth_rss_conf *rss_conf);
+static int i40e_dev_udp_tunnel_add(struct rte_eth_dev *dev,
+				struct rte_eth_udp_tunnel *udp_tunnel,
+				uint8_t count);
+static int i40e_dev_udp_tunnel_del(struct rte_eth_dev *dev,
+				struct rte_eth_udp_tunnel *udp_tunnel,
+				uint8_t count);
 static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
 				enum rte_filter_type filter_type,
 				enum rte_filter_op filter_op,
@@ -241,6 +247,8 @@ static struct eth_dev_ops i40e_eth_dev_ops = {
 	.reta_query                   = i40e_dev_rss_reta_query,
 	.rss_hash_update              = i40e_dev_rss_hash_update,
 	.rss_hash_conf_get            = i40e_dev_rss_hash_conf_get,
+	.udp_tunnel_add               = i40e_dev_udp_tunnel_add,
+	.udp_tunnel_del               = i40e_dev_udp_tunnel_del,
 	.filter_ctrl                  = i40e_dev_filter_ctrl,
 };
 
@@ -3178,6 +3186,10 @@ i40e_vsi_rx_init(struct i40e_vsi *vsi)
 	uint16_t i;
 
 	i40e_pf_config_mq_rx(pf);
+
+	if (data->dev_conf.tunnel_type == RTE_TUNNEL_TYPE_VXLAN)
+		pf->flags |= I40E_FLAG_VXLAN;
+
 	for (i = 0; i < data->nb_rx_queues; i++) {
 		ret = i40e_rx_queue_init(data->rx_queues[i]);
 		if (ret != I40E_SUCCESS) {
@@ -4092,6 +4104,158 @@ i40e_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+i40e_get_vxlan_port_idx(struct i40e_pf *pf, uint16_t port)
+{
+	uint8_t i;
+
+	for (i = 0; i < I40E_MAX_PF_UDP_OFFLOAD_PORTS; i++) {
+		if (pf->vxlan_ports[i] == port)
+			return i;
+	}
+
+	return -1;
+}
+
+static int
+i40e_add_vxlan_port(struct i40e_pf *pf, uint16_t port)
+{
+	int  idx, ret;
+	uint8_t filter_idx;
+	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
+
+	if (!(pf->flags & I40E_FLAG_VXLAN)) {
+		PMD_DRV_LOG(ERR, "VxLAN tunneling mode is not configured\n");
+		return -EINVAL;
+	}
+
+	idx = i40e_get_vxlan_port_idx(pf, port);
+
+	/* Check if port already exists */
+	if (idx >= 0) {
+		PMD_DRV_LOG(ERR, "Port %d already offloaded\n", port);
+		return -EINVAL;
+	}
+
+	/* Now check if there is space to add the new port */
+	idx = i40e_get_vxlan_port_idx(pf, 0);
+	if (idx < 0) {
+		PMD_DRV_LOG(ERR, "Maximum number of UDP ports reached,"
+			"not adding port %d\n", port);
+		return -ENOSPC;
+	}
+
+	ret =  i40e_aq_add_udp_tunnel(hw, port, I40E_AQC_TUNNEL_TYPE_VXLAN,
+					&filter_idx, NULL);
+	if (ret < 0) {
+		PMD_DRV_LOG(ERR, "Failed to add VxLAN UDP port %d\n", port);
+		return -1;
+	}
+
+	PMD_DRV_LOG(INFO, "Added %s port %d with AQ command with index %d\n",
+			 port,  filter_index);
+
+	/* New port: add it and mark its index in the bitmap */
+	pf->vxlan_ports[idx] = port;
+	pf->vxlan_bitmap |= (1 << idx);
+
+	return 0;
+}
+
+static int
+i40e_del_vxlan_port(struct i40e_pf *pf, uint16_t port)
+{
+	int idx;
+	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
+
+	if (!(pf->flags & I40E_FLAG_VXLAN)) {
+		PMD_DRV_LOG(ERR, "VxLAN tunneling mode is not configured\n");
+		return -EINVAL;
+	}
+
+	idx = i40e_get_vxlan_port_idx(pf, port);
+
+	if (idx < 0) {
+		PMD_DRV_LOG(ERR, "Port %d doesn't exist\n", port);
+		return -EINVAL;
+	}
+
+	if (i40e_aq_del_udp_tunnel(hw, idx, NULL) < 0) {
+		PMD_DRV_LOG(ERR, "Failed to delete VxLAN UDP port %d\n", port);
+		return -1;
+	}
+
+	PMD_DRV_LOG(INFO, "Deleted port %d with AQ command with index %d\n",
+			port, idx);
+
+	pf->vxlan_ports[idx] = 0;
+	pf->vxlan_bitmap &= ~(1 << idx);
+
+	return 0;
+}
+
+/* Add UDP tunneling port */
+static int
+i40e_dev_udp_tunnel_add(struct rte_eth_dev *dev,
+			struct rte_eth_udp_tunnel *udp_tunnel,
+			uint8_t count)
+{
+	uint16_t i;
+	int ret = 0;
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+	for (i = 0; i < count; i++, udp_tunnel++) {
+		switch (udp_tunnel->prot_type) {
+		case RTE_TUNNEL_TYPE_VXLAN:
+			ret = i40e_add_vxlan_port(pf, udp_tunnel->udp_port);
+			break;
+
+		case RTE_TUNNEL_TYPE_GENEVE:
+		case RTE_TUNNEL_TYPE_TEREDO:
+			PMD_DRV_LOG(ERR, "Tunnel type is not supported now.\n");
+			ret = -1;
+			break;
+
+		default:
+			PMD_DRV_LOG(ERR, "Invalid tunnel type\n");
+			ret = -1;
+			break;
+		}
+	}
+
+	return ret;
+}
+
+/* Remove UDP tunneling port */
+static int
+i40e_dev_udp_tunnel_del(struct rte_eth_dev *dev,
+			struct rte_eth_udp_tunnel *udp_tunnel,
+			uint8_t count)
+{
+	uint16_t i;
+	int ret = 0;
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+
+	for (i = 0; i < count; i++, udp_tunnel++) {
+		switch (udp_tunnel->prot_type) {
+		case RTE_TUNNEL_TYPE_VXLAN:
+			ret = i40e_del_vxlan_port(pf, udp_tunnel->udp_port);
+			break;
+		case RTE_TUNNEL_TYPE_GENEVE:
+		case RTE_TUNNEL_TYPE_TEREDO:
+			PMD_DRV_LOG(ERR, "Tunnel type is not supported now.\n");
+			ret = -1;
+			break;
+		default:
+			PMD_DRV_LOG(ERR, "Invalid tunnel type\n");
+			ret = -1;
+			break;
+		}
+	}
+
+	return ret;
+}
+
 /* Configure RSS */
 static int
 i40e_pf_config_rss(struct i40e_pf *pf)
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.h b/lib/librte_pmd_i40e/i40e_ethdev.h
index 1d42cd2..826a1fb 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.h
+++ b/lib/librte_pmd_i40e/i40e_ethdev.h
@@ -60,13 +60,15 @@
 #define I40E_FLAG_HEADER_SPLIT_DISABLED (1ULL << 4)
 #define I40E_FLAG_HEADER_SPLIT_ENABLED  (1ULL << 5)
 #define I40E_FLAG_FDIR                  (1ULL << 6)
+#define I40E_FLAG_VXLAN                 (1ULL << 7)
 #define I40E_FLAG_ALL (I40E_FLAG_RSS | \
 		       I40E_FLAG_DCB | \
 		       I40E_FLAG_VMDQ | \
 		       I40E_FLAG_SRIOV | \
 		       I40E_FLAG_HEADER_SPLIT_DISABLED | \
 		       I40E_FLAG_HEADER_SPLIT_ENABLED | \
-		       I40E_FLAG_FDIR)
+		       I40E_FLAG_FDIR | \
+		       I40E_FLAG_VXLAN)
 
 #define I40E_RSS_OFFLOAD_ALL ( \
 	ETH_RSS_NONF_IPV4_UDP | \
@@ -246,6 +248,10 @@ struct i40e_pf {
 	uint16_t vmdq_nb_qps; /* The number of queue pairs of VMDq */
 	uint16_t vf_nb_qps; /* The number of queue pairs of VF */
 	uint16_t fdir_nb_qps; /* The number of queue pairs of Flow Director */
+
+	/* store VxLAN UDP ports */
+	uint16_t vxlan_ports[I40E_MAX_PF_UDP_OFFLOAD_PORTS];
+	uint16_t vxlan_bitmap; /* Vxlan bit mask */
 };
 
 enum pending_msg {
diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index 2b53677..7c3809f 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -638,6 +638,10 @@ i40e_rx_scan_hw_ring(struct i40e_rx_queue *rxq)
 			pkt_flags |= i40e_rxd_error_to_pkt_flags(qword1);
 			pkt_flags |= i40e_rxd_ptype_to_pkt_flags(qword1);
 			mb->ol_flags = pkt_flags;
+
+			mb->packet_type = (uint16_t)((qword1 &
+					I40E_RXD_QW1_PTYPE_MASK) >>
+					I40E_RXD_QW1_PTYPE_SHIFT);
 			if (pkt_flags & PKT_RX_RSS_HASH)
 				mb->hash.rss = rte_le_to_cpu_32(\
 					rxdp->wb.qword0.hi_dword.rss);
@@ -873,6 +877,8 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		pkt_flags = i40e_rxd_status_to_pkt_flags(qword1);
 		pkt_flags |= i40e_rxd_error_to_pkt_flags(qword1);
 		pkt_flags |= i40e_rxd_ptype_to_pkt_flags(qword1);
+		rxm->packet_type = (uint16_t)((qword1 & I40E_RXD_QW1_PTYPE_MASK) >>
+				I40E_RXD_QW1_PTYPE_SHIFT);
 		rxm->ol_flags = pkt_flags;
 		if (pkt_flags & PKT_RX_RSS_HASH)
 			rxm->hash.rss =
@@ -1027,6 +1033,9 @@ i40e_recv_scattered_pkts(void *rx_queue,
 		pkt_flags = i40e_rxd_status_to_pkt_flags(qword1);
 		pkt_flags |= i40e_rxd_error_to_pkt_flags(qword1);
 		pkt_flags |= i40e_rxd_ptype_to_pkt_flags(qword1);
+		first_seg->packet_type = (uint16_t)((qword1 &
+					I40E_RXD_QW1_PTYPE_MASK) >>
+					I40E_RXD_QW1_PTYPE_SHIFT);
 		first_seg->ol_flags = pkt_flags;
 		if (pkt_flags & PKT_RX_RSS_HASH)
 			rxm->hash.rss =
-- 
1.7.7.6

[dpdk-dev] [PATCH v6 4/9] app/test-pmd:test VxLAN packet identification

[Jijiang Liu]

Add two commands to test VxLAN packet identification, which include
 - use commands to add/delete VxLAN UDP port.
 - use rxonly mode to receive VxLAN packet.
 
Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
---
 app/test-pmd/cmdline.c    |   65 +++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/parameters.c |   13 +++++++++
 app/test-pmd/rxonly.c     |   49 ++++++++++++++++++++++++++++++++++
 app/test-pmd/testpmd.c    |    8 +++++
 app/test-pmd/testpmd.h    |    4 +++
 5 files changed, 139 insertions(+), 0 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 0b972f9..7160e38 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -285,6 +285,12 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Set the outer VLAN TPID for Packet Filtering on"
 			" a port\n\n"
 
+			"rx_vxlan_port add (udp_port) (port_id)\n"
+			"    Add an UDP port for VxLAN packet filter on a port\n\n"
+
+			"rx_vxlan_port rm (udp_port) (port_id)\n"
+			"    Remove an UDP port for VxLAN packet filter on a port\n\n"
+
 			"tx_vlan set vlan_id (port_id)\n"
 			"    Set hardware insertion of VLAN ID in packets sent"
 			" on a port.\n\n"
@@ -6225,6 +6231,64 @@ cmdline_parse_inst_t cmd_vf_rate_limit = {
 	},
 };
 
+/* *** CONFIGURE TUNNEL UDP PORT *** */
+struct cmd_tunnel_udp_config {
+	cmdline_fixed_string_t cmd;
+	cmdline_fixed_string_t what;
+	uint16_t udp_port;
+	uint8_t port_id;
+};
+
+static void
+cmd_tunnel_udp_config_parsed(void *parsed_result,
+			  __attribute__((unused)) struct cmdline *cl,
+			  __attribute__((unused)) void *data)
+{
+	struct cmd_tunnel_udp_config *res = parsed_result;
+	struct rte_eth_udp_tunnel tunnel_udp;
+	int ret;
+
+	tunnel_udp.udp_port = res->udp_port;
+
+	if (!strcmp(res->cmd, "rx_vxlan_port"))
+		tunnel_udp.prot_type = RTE_TUNNEL_TYPE_VXLAN;
+
+	if (!strcmp(res->what, "add"))
+		ret = rte_eth_dev_udp_tunnel_add(res->port_id, &tunnel_udp, 1);
+	else
+		ret = rte_eth_dev_udp_tunnel_delete(res->port_id, &tunnel_udp, 1);
+
+	if (ret < 0)
+		printf("udp tunneling add error: (%s)\n", strerror(-ret));
+}
+
+cmdline_parse_token_string_t cmd_tunnel_udp_config_cmd =
+	TOKEN_STRING_INITIALIZER(struct cmd_tunnel_udp_config,
+				cmd, "rx_vxlan_port");
+cmdline_parse_token_string_t cmd_tunnel_udp_config_what =
+	TOKEN_STRING_INITIALIZER(struct cmd_tunnel_udp_config,
+				what, "add#rm");
+cmdline_parse_token_num_t cmd_tunnel_udp_config_udp_port =
+	TOKEN_NUM_INITIALIZER(struct cmd_tunnel_udp_config,
+				udp_port, UINT16);
+cmdline_parse_token_num_t cmd_tunnel_udp_config_port_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_tunnel_udp_config,
+				port_id, UINT8);
+
+cmdline_parse_inst_t cmd_tunnel_udp_config = {
+	.f = cmd_tunnel_udp_config_parsed,
+	.data = (void *)0,
+	.help_str = "add/rm an tunneling UDP port filter: "
+			"rx_vxlan_port add udp_port port_id",
+	.tokens = {
+		(void *)&cmd_tunnel_udp_config_cmd,
+		(void *)&cmd_tunnel_udp_config_what,
+		(void *)&cmd_tunnel_udp_config_udp_port,
+		(void *)&cmd_tunnel_udp_config_port_id,
+		NULL,
+	},
+};
+
 /* *** CONFIGURE VM MIRROR VLAN/POOL RULE *** */
 struct cmd_set_mirror_mask_result {
 	cmdline_fixed_string_t set;
@@ -7518,6 +7582,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_vf_rxvlan_filter,
 	(cmdline_parse_inst_t *)&cmd_queue_rate_limit,
 	(cmdline_parse_inst_t *)&cmd_vf_rate_limit,
+	(cmdline_parse_inst_t *)&cmd_tunnel_udp_config,
 	(cmdline_parse_inst_t *)&cmd_set_mirror_mask,
 	(cmdline_parse_inst_t *)&cmd_set_mirror_link,
 	(cmdline_parse_inst_t *)&cmd_reset_mirror_rule,
diff --git a/app/test-pmd/parameters.c b/app/test-pmd/parameters.c
index 9573a43..fda8c1d 100644
--- a/app/test-pmd/parameters.c
+++ b/app/test-pmd/parameters.c
@@ -202,6 +202,10 @@ usage(char* progname)
 	printf("  --txpkts=X[,Y]*: set TX segment sizes.\n");
 	printf("  --disable-link-check: disable check on link status when "
 	       "starting/stopping ports.\n");
+	printf("  --tunnel-type=N: set tunneling packet type "
+	       "(0 <= N <= 4).(0:non-tunneling packet;1:VxLAN; "
+	       "2:GENEVE;3: TEREDO;4: NVGRE)\n");
+
 }
 
 #ifdef RTE_LIBRTE_CMDLINE
@@ -600,6 +604,7 @@ launch_args_parse(int argc, char** argv)
 		{ "no-flush-rx",	0, 0, 0 },
 		{ "txpkts",			1, 0, 0 },
 		{ "disable-link-check",		0, 0, 0 },
+		{ "tunnel-type",                1, 0, 0 },
 		{ 0, 0, 0, 0 },
 	};
 
@@ -1032,6 +1037,14 @@ launch_args_parse(int argc, char** argv)
 				else
 					rte_exit(EXIT_FAILURE, "rxfreet must be >= 0\n");
 			}
+			if (!strcmp(lgopts[opt_idx].name, "tunnel-type")) {
+				n = atoi(optarg);
+				if ((n >= 0) && (n < RTE_TUNNEL_TYPE_MAX))
+					rx_tunnel_type = (uint16_t)n;
+				else
+					rte_exit(EXIT_FAILURE, "tunnel-type must be 0-%d\n",
+						RTE_TUNNEL_TYPE_MAX);
+			}
 			if (!strcmp(lgopts[opt_idx].name, "tx-queue-stats-mapping")) {
 				if (parse_queue_stats_mapping_config(optarg, TX)) {
 					rte_exit(EXIT_FAILURE,
diff --git a/app/test-pmd/rxonly.c b/app/test-pmd/rxonly.c
index 98c788b..db34af1 100644
--- a/app/test-pmd/rxonly.c
+++ b/app/test-pmd/rxonly.c
@@ -66,6 +66,8 @@
 #include <rte_ether.h>
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
+#include <rte_ip.h>
+#include <rte_udp.h>
 
 #include "testpmd.h"
 
@@ -112,6 +114,9 @@ pkt_burst_receive(struct fwd_stream *fs)
 	uint64_t ol_flags;
 	uint16_t nb_rx;
 	uint16_t i;
+	uint8_t ptype;
+	uint8_t is_encapsulation;
+
 #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
 	uint64_t start_tsc;
 	uint64_t end_tsc;
@@ -152,6 +157,11 @@ pkt_burst_receive(struct fwd_stream *fs)
 		eth_hdr = rte_pktmbuf_mtod(mb, struct ether_hdr *);
 		eth_type = RTE_BE_TO_CPU_16(eth_hdr->ether_type);
 		ol_flags = mb->ol_flags;
+		ptype = mb->packet_type;
+
+		is_encapsulation = IS_ETH_IPV4_TUNNEL(ptype) |
+					IS_ETH_IPV6_TUNNEL(ptype);
+
 		print_ether_addr("  src=", &eth_hdr->s_addr);
 		print_ether_addr(" - dst=", &eth_hdr->d_addr);
 		printf(" - type=0x%04x - length=%u - nb_segs=%d",
@@ -166,6 +176,45 @@ pkt_burst_receive(struct fwd_stream *fs)
 			       mb->hash.fdir.hash, mb->hash.fdir.id);
 		if (ol_flags & PKT_RX_VLAN_PKT)
 			printf(" - VLAN tci=0x%x", mb->vlan_tci);
+		if (is_encapsulation) {
+			struct ipv4_hdr *ipv4_hdr;
+			struct ipv6_hdr *ipv6_hdr;
+			struct udp_hdr *udp_hdr;
+			uint8_t l2_len;
+			uint8_t l3_len;
+			uint8_t l4_len;
+			uint8_t l4_proto;
+			struct  vxlan_hdr *vxlan_hdr;
+
+			l2_len  = sizeof(struct ether_hdr);
+
+			 /* Do not support ipv4 option field */
+			if (IS_ETH_IPV4_TUNNEL(ptype)) {
+				l3_len = sizeof(struct ipv4_hdr);
+				ipv4_hdr = (struct ipv4_hdr *) (rte_pktmbuf_mtod(mb,
+						unsigned char *) + l2_len);
+				l4_proto = ipv4_hdr->next_proto_id;
+			} else {
+				l3_len = sizeof(struct ipv6_hdr);
+				ipv6_hdr = (struct ipv6_hdr *) (rte_pktmbuf_mtod(mb,
+						unsigned char *) + l2_len);
+				l4_proto = ipv6_hdr->proto;
+			}
+			if (l4_proto == IPPROTO_UDP) {
+				udp_hdr = (struct udp_hdr *) (rte_pktmbuf_mtod(mb,
+						unsigned char *) + l2_len + l3_len);
+				l4_len = sizeof(struct udp_hdr);
+				vxlan_hdr = (struct vxlan_hdr *) (rte_pktmbuf_mtod(mb,
+						unsigned char *) + l2_len + l3_len
+						 + l4_len);
+
+				printf(" - VxLAN packet: packet type =%d, "
+					"Destination UDP port =%d, VNI = %d",
+					ptype, RTE_BE_TO_CPU_16(udp_hdr->dst_port),
+					rte_be_to_cpu_32(vxlan_hdr->vx_vni) >> 8);
+			}
+		}
+		printf(" - Receive queue=0x%x", (unsigned) fs->rx_queue);
 		printf("\n");
 		if (ol_flags != 0) {
 			int rxf;
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index f76406f..580f52e 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -245,6 +245,12 @@ uint16_t tx_free_thresh = 0; /* Use default values. */
 uint16_t tx_rs_thresh = 0; /* Use default values. */
 
 /*
+ * Configurable value of tunnel type.
+ */
+
+uint8_t rx_tunnel_type = 0; /* Use default values. */
+
+/*
  * Configurable value of TX queue flags.
  */
 uint32_t txq_flags = 0; /* No flags set. */
@@ -1691,6 +1697,8 @@ init_port_config(void)
 		port = &ports[pid];
 		port->dev_conf.rxmode = rx_mode;
 		port->dev_conf.fdir_conf = fdir_conf;
+		if (rx_tunnel_type == 1)
+			port->dev_conf.tunnel_type = RTE_TUNNEL_TYPE_VXLAN;
 		if (nb_rxq > 1) {
 			port->dev_conf.rx_adv_conf.rss_conf.rss_key = NULL;
 			port->dev_conf.rx_adv_conf.rss_conf.rss_hf = rss_hf;
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index 9cbfeac..a797536 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -80,6 +80,9 @@ typedef uint16_t streamid_t;
 
 #define MAX_QUEUE_ID ((1 << (sizeof(queueid_t) * 8)) - 1)
 
+#define IS_ETH_IPV4_TUNNEL(ptype) ((ptype > 58) && (ptype < 87))
+#define IS_ETH_IPV6_TUNNEL(ptype) ((ptype > 124) && (ptype < 153))
+
 enum {
 	PORT_TOPOLOGY_PAIRED,
 	PORT_TOPOLOGY_CHAINED,
@@ -342,6 +345,7 @@ extern uint8_t rx_drop_en;
 extern uint16_t tx_free_thresh;
 extern uint16_t tx_rs_thresh;
 extern uint32_t txq_flags;
+extern uint8_t rx_tunnel_type;
 
 extern uint8_t dcb_config;
 extern uint8_t dcb_test;
-- 
1.7.7.6

[dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of VxLAN filter

[Jijiang Liu]

Add definations of the data structures of tunneling packet filter in the rte_eth_ctrl.h file.

Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
---
 lib/librte_ether/rte_eth_ctrl.h |   64 +++++++++++++++++++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.h   |   12 -------
 2 files changed, 64 insertions(+), 12 deletions(-)

diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h
index df21ac6..a04333c 100644
--- a/lib/librte_ether/rte_eth_ctrl.h
+++ b/lib/librte_ether/rte_eth_ctrl.h
@@ -51,6 +51,7 @@ extern "C" {
  */
 enum rte_filter_type {
 	RTE_ETH_FILTER_NONE = 0,
+	RTE_ETH_FILTER_TUNNEL,
 	RTE_ETH_FILTER_MAX
 };
 
@@ -71,6 +72,69 @@ enum rte_filter_op {
 	RTE_ETH_FILTER_OP_MAX
 };
 
+/**
+ * filter type of tunneling packet
+ */
+#define ETH_TUNNEL_FILTER_OMAC  0x01 /**< filter by outer MAC addr */
+#define ETH_TUNNEL_FILTER_OIP   0x02 /**< filter by outer IP Addr */
+#define ETH_TUNNEL_FILTER_TENID 0x04 /**< filter by tenant ID */
+#define ETH_TUNNEL_FILTER_IMAC  0x08 /**< filter by inner MAC addr */
+#define ETH_TUNNEL_FILTER_IVLAN 0x10 /**< filter by inner VLAN ID */
+#define ETH_TUNNEL_FILTER_IIP   0x20 /**< filter by inner IP addr */
+
+#define RTE_TUNNEL_FILTER_TO_QUEUE 1 /**< point to an queue by filter type */
+
+#define RTE_TUNNEL_FILTER_IMAC_IVLAN (ETH_TUNNEL_FILTER_IMAC | \
+					ETH_TUNNEL_FILTER_IVLAN)
+#define RTE_TUNNEL_FILTER_IMAC_IVLAN_TENID (ETH_TUNNEL_FILTER_IMAC | \
+					ETH_TUNNEL_FILTER_IVLAN | \
+					ETH_TUNNEL_FILTER_TENID)
+#define RTE_TUNNEL_FILTER_IMAC_TENID (ETH_TUNNEL_FILTER_IMAC | \
+					ETH_TUNNEL_FILTER_TENID)
+#define RTE_TUNNEL_FILTER_OMAC_TENID_IMAC (ETH_TUNNEL_FILTER_OMAC | \
+					ETH_TUNNEL_FILTER_TENID | \
+					ETH_TUNNEL_FILTER_IMAC)
+
+/**
+ *  Select IPv4 or IPv6 for tunnel filters.
+ */
+enum rte_tunnel_iptype {
+	RTE_TUNNEL_IPTYPE_IPV4 = 0, /**< IPv4. */
+	RTE_TUNNEL_IPTYPE_IPV6,     /**< IPv6. */
+};
+
+/**
+ * Tunneled type.
+ */
+enum rte_eth_tunnel_type {
+	RTE_TUNNEL_TYPE_NONE = 0,
+	RTE_TUNNEL_TYPE_VXLAN,
+	RTE_TUNNEL_TYPE_GENEVE,
+	RTE_TUNNEL_TYPE_TEREDO,
+	RTE_TUNNEL_TYPE_NVGRE,
+	RTE_TUNNEL_TYPE_MAX,
+};
+
+/**
+ * Tunneling Packet filter configuration.
+ */
+struct rte_eth_tunnel_filter_conf {
+	struct ether_addr *outer_mac;  /**< Outer MAC address filter. */
+	struct ether_addr *inner_mac;  /**< Inner MAC address filter. */
+	uint16_t inner_vlan;           /**< Inner VLAN filter. */
+	enum rte_tunnel_iptype ip_type; /**< IP address type. */
+	union {
+		uint32_t ipv4_addr;    /**< IPv4 source address to match. */
+		uint32_t ipv6_addr[4]; /**< IPv6 source address to match. */
+	} ip_addr; /**< IPv4/IPv6 source address to match (union of above). */
+
+	uint16_t filter_type;   /**< Filter type. */
+	uint8_t to_queue;       /**< Use MAC and VLAN to point to a queue. */
+	enum rte_eth_tunnel_type tunnel_type; /**< Tunnel Type. */
+	uint32_t tenant_id;     /** < Tenant number. */
+	uint16_t queue_id;      /** < queue number. */
+};
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9ad11ec..c8fb89a 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -718,18 +718,6 @@ struct rte_eth_udp_tunnel {
 };
 
 /**
- * Tunneled type.
- */
-enum rte_eth_tunnel_type {
-	RTE_TUNNEL_TYPE_NONE = 0,
-	RTE_TUNNEL_TYPE_VXLAN,
-	RTE_TUNNEL_TYPE_GENEVE,
-	RTE_TUNNEL_TYPE_TEREDO,
-	RTE_TUNNEL_TYPE_NVGRE,
-	RTE_TUNNEL_TYPE_MAX,
-};
-
-/**
  *  Possible l4type of FDIR filters.
  */
 enum rte_l4type {
-- 
1.7.7.6

[dpdk-dev] [PATCH v6 6/9] i40e:implement API of VxLAN packet filter in librte_pmd_i40e

[Jijiang Liu]

The implementation of VxLAN tunnel filter in librte_pmd_i40e, which include
 - add the i40e_tunnel_filter_handle() function.
 - add the i40e_dev_tunnel_filter_set() function.
 
Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
---
 lib/librte_pmd_i40e/i40e_ethdev.c |  177 ++++++++++++++++++++++++++++++++++++-
 1 files changed, 175 insertions(+), 2 deletions(-)

diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 8a84e30..726a972 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -48,6 +48,7 @@
 #include <rte_malloc.h>
 #include <rte_memcpy.h>
 #include <rte_dev.h>
+#include <rte_eth_ctrl.h>
 
 #include "i40e_logs.h"
 #include "i40e/i40e_register_x710_int.h"
@@ -192,6 +193,9 @@ static int i40e_dev_udp_tunnel_add(struct rte_eth_dev *dev,
 static int i40e_dev_udp_tunnel_del(struct rte_eth_dev *dev,
 				struct rte_eth_udp_tunnel *udp_tunnel,
 				uint8_t count);
+static int i40e_dev_tunnel_filter_set(struct i40e_pf *pf,
+				struct rte_eth_tunnel_filter_conf *tunnel_filter,
+				uint8_t add);
 static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
 				enum rte_filter_type filter_type,
 				enum rte_filter_op filter_op,
@@ -4105,6 +4109,108 @@ i40e_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
 }
 
 static int
+i40e_dev_get_filter_type(uint16_t filter_type, uint16_t *flag)
+{
+	switch (filter_type) {
+	case RTE_TUNNEL_FILTER_IMAC_IVLAN:
+		*flag = I40E_AQC_ADD_CLOUD_FILTER_IMAC_IVLAN;
+		break;
+	case RTE_TUNNEL_FILTER_IMAC_IVLAN_TENID:
+		*flag = I40E_AQC_ADD_CLOUD_FILTER_IMAC_IVLAN_TEN_ID;
+		break;
+	case RTE_TUNNEL_FILTER_IMAC_TENID:
+		*flag = I40E_AQC_ADD_CLOUD_FILTER_IMAC_TEN_ID;
+		break;
+	case RTE_TUNNEL_FILTER_OMAC_TENID_IMAC:
+		*flag = I40E_AQC_ADD_CLOUD_FILTER_OMAC_TEN_ID_IMAC;
+		break;
+	case ETH_TUNNEL_FILTER_IMAC:
+		*flag = I40E_AQC_ADD_CLOUD_FILTER_IMAC;
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "invalid tunnel filter type\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+i40e_dev_tunnel_filter_set(struct i40e_pf *pf,
+			struct rte_eth_tunnel_filter_conf *tunnel_filter,
+			uint8_t add)
+{
+	uint16_t ip_type;
+	uint8_t tun_type = 0;
+	int val, ret = 0;
+	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
+	struct i40e_vsi *vsi = pf->main_vsi;
+	struct i40e_aqc_add_remove_cloud_filters_element_data  *cld_filter;
+	struct i40e_aqc_add_remove_cloud_filters_element_data  *pfilter;
+
+	cld_filter = rte_zmalloc("tunnel_filter",
+		sizeof(struct i40e_aqc_add_remove_cloud_filters_element_data),
+		0);
+
+	if (NULL == cld_filter) {
+		PMD_DRV_LOG(ERR, "Failed to alloc memory.\n");
+		return -EINVAL;
+	}
+	pfilter = cld_filter;
+
+	(void)rte_memcpy(&pfilter->outer_mac, tunnel_filter->outer_mac,
+			sizeof(struct ether_addr));
+	(void)rte_memcpy(&pfilter->inner_mac, tunnel_filter->inner_mac,
+			sizeof(struct ether_addr));
+
+	pfilter->inner_vlan = tunnel_filter->inner_vlan;
+	if (tunnel_filter->ip_type == RTE_TUNNEL_IPTYPE_IPV4) {
+		ip_type = I40E_AQC_ADD_CLOUD_FLAGS_IPV4;
+		(void)rte_memcpy(&pfilter->ipaddr.v4.data,
+				&tunnel_filter->ip_addr,
+				sizeof(pfilter->ipaddr.v4.data));
+	} else {
+		ip_type = I40E_AQC_ADD_CLOUD_FLAGS_IPV6;
+		(void)rte_memcpy(&pfilter->ipaddr.v6.data,
+				&tunnel_filter->ip_addr,
+				sizeof(pfilter->ipaddr.v6.data));
+	}
+
+	/* check tunneled type */
+	switch (tunnel_filter->tunnel_type) {
+	case RTE_TUNNEL_TYPE_VXLAN:
+		tun_type = I40E_AQC_ADD_CLOUD_TNL_TYPE_XVLAN;
+		break;
+	default:
+		/* Other tunnel types is not supported. */
+		PMD_DRV_LOG(ERR, "tunnel type is not supported.\n");
+		rte_free(cld_filter);
+		return -EINVAL;
+	}
+
+	val = i40e_dev_get_filter_type(tunnel_filter->filter_type,
+						&pfilter->flags);
+	if (val < 0) {
+		rte_free(cld_filter);
+		return -EINVAL;
+	}
+
+	pfilter->flags |= I40E_AQC_ADD_CLOUD_FLAGS_TO_QUEUE | ip_type |
+		(tun_type << I40E_AQC_ADD_CLOUD_TNL_TYPE_SHIFT);
+	pfilter->tenant_id = tunnel_filter->tenant_id;
+	pfilter->queue_number = tunnel_filter->queue_id;
+
+	if (add)
+		ret = i40e_aq_add_cloud_filters(hw, vsi->seid, cld_filter, 1);
+	else
+		ret = i40e_aq_remove_cloud_filters(hw, vsi->seid,
+						cld_filter, 1);
+
+	rte_free(cld_filter);
+	return ret;
+}
+
+static int
 i40e_get_vxlan_port_idx(struct i40e_pf *pf, uint16_t port)
 {
 	uint8_t i;
@@ -4293,6 +4399,72 @@ i40e_pf_config_rss(struct i40e_pf *pf)
 }
 
 static int
+i40e_tunnel_filter_param_check(struct i40e_pf *pf,
+			struct rte_eth_tunnel_filter_conf *filter)
+{
+	if (pf == NULL || filter == NULL) {
+		PMD_DRV_LOG(ERR, "Invalid parameter\n");
+		return -EINVAL;
+	}
+
+	if (filter->queue_id >= pf->dev_data->nb_rx_queues) {
+		PMD_DRV_LOG(ERR, "Invalid queue ID\n");
+		return -EINVAL;
+	}
+
+	if (filter->inner_vlan > ETHER_MAX_VLAN_ID) {
+		PMD_DRV_LOG(ERR, "Invalid inner VLAN ID\n");
+		return -EINVAL;
+	}
+
+	if ((filter->filter_type & ETH_TUNNEL_FILTER_OMAC) &&
+		(is_zero_ether_addr(filter->outer_mac))) {
+		PMD_DRV_LOG(ERR, "Cannot add NULL outer MAC address\n");
+		return -EINVAL;
+	}
+
+	if ((filter->filter_type & ETH_TUNNEL_FILTER_IMAC) &&
+		(is_zero_ether_addr(filter->inner_mac))) {
+		PMD_DRV_LOG(ERR, "Cannot add NULL inner MAC address\n");
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int
+i40e_tunnel_filter_handle(struct rte_eth_dev *dev, enum rte_filter_op filter_op,
+			void *arg)
+{
+	struct rte_eth_tunnel_filter_conf *filter;
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	int ret = I40E_SUCCESS;
+
+	filter = (struct rte_eth_tunnel_filter_conf *)(arg);
+
+	if (i40e_tunnel_filter_param_check(pf, filter) < 0)
+		return I40E_ERR_PARAM;
+
+	switch (filter_op) {
+	case RTE_ETH_FILTER_NOP:
+		if (!(pf->flags & I40E_FLAG_VXLAN))
+			ret = I40E_NOT_SUPPORTED;
+	case RTE_ETH_FILTER_ADD:
+		ret = i40e_dev_tunnel_filter_set(pf, filter, 1);
+		break;
+	case RTE_ETH_FILTER_DELETE:
+		ret = i40e_dev_tunnel_filter_set(pf, filter, 0);
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "unknown operation %u\n", filter_op);
+		ret = I40E_ERR_PARAM;
+		break;
+	}
+
+	return ret;
+}
+
+static int
 i40e_pf_config_mq_rx(struct i40e_pf *pf)
 {
 	if (!pf->dev_data->sriov.active) {
@@ -4316,13 +4488,14 @@ i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
 		     void *arg)
 {
 	int ret = 0;
-	(void)filter_op;
-	(void)arg;
 
 	if (dev == NULL)
 		return -EINVAL;
 
 	switch (filter_type) {
+	case RTE_ETH_FILTER_TUNNEL:
+		ret = i40e_tunnel_filter_handle(dev, filter_op, arg);
+		break;
 	default:
 		PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
 							filter_type);
-- 
1.7.7.6

[dpdk-dev] [PATCH v6 7/9] app/testpmd:test VxLAN packet filter

[Jijiang Liu]

Add the tunnel_filter command in testpmd to test the API of VxLAN packet filter.
 
Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>

---
 app/test-pmd/cmdline.c |  152 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 152 insertions(+), 0 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 7160e38..bc9a30c 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -285,6 +285,14 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Set the outer VLAN TPID for Packet Filtering on"
 			" a port\n\n"
 
+			"tunnel_filter add (port_id) (outer_mac) (inner_mac) (ip_addr) "
+			"(inner_vlan) (tunnel_type) (filter_type) (tenant_id) (queue_id)\n"
+			"   add a tunnel filter of a port.\n\n"
+
+			"tunnel_filter rm (port_id) (outer_mac) (inner_mac) (ip_addr) "
+			"(inner_vlan) (tunnel_type) (filter_type) (tenant_id) (queue_id)\n"
+			"   remove a tunnel filter of a port.\n\n"
+
 			"rx_vxlan_port add (udp_port) (port_id)\n"
 			"    Add an UDP port for VxLAN packet filter on a port\n\n"
 
@@ -6231,6 +6239,149 @@ cmdline_parse_inst_t cmd_vf_rate_limit = {
 	},
 };
 
+/* *** ADD TUNNEL FILTER OF A PORT *** */
+struct cmd_tunnel_filter_result {
+	cmdline_fixed_string_t cmd;
+	cmdline_fixed_string_t what;
+	uint8_t port_id;
+	struct ether_addr outer_mac;
+	struct ether_addr inner_mac;
+	cmdline_ipaddr_t ip_value;
+	uint16_t inner_vlan;
+	cmdline_fixed_string_t tunnel_type;
+	cmdline_fixed_string_t filter_type;
+	uint32_t tenant_id;
+	uint16_t queue_num;
+};
+
+static void
+cmd_tunnel_filter_parsed(void *parsed_result,
+			  __attribute__((unused)) struct cmdline *cl,
+			  __attribute__((unused)) void *data)
+{
+	struct cmd_tunnel_filter_result *res = parsed_result;
+	struct rte_eth_tunnel_filter_conf tunnel_filter_conf;
+	int ret = 0;
+
+	tunnel_filter_conf.outer_mac = &res->outer_mac;
+	tunnel_filter_conf.inner_mac = &res->inner_mac;
+	tunnel_filter_conf.inner_vlan = res->inner_vlan;
+
+	if (res->ip_value.family == AF_INET) {
+		tunnel_filter_conf.ip_addr.ipv4_addr =
+			res->ip_value.addr.ipv4.s_addr;
+		tunnel_filter_conf.ip_type = RTE_TUNNEL_IPTYPE_IPV4;
+	} else {
+		memcpy(&(tunnel_filter_conf.ip_addr.ipv6_addr),
+			&(res->ip_value.addr.ipv6),
+			sizeof(struct in6_addr));
+		tunnel_filter_conf.ip_type = RTE_TUNNEL_IPTYPE_IPV6;
+	}
+
+	if (!strcmp(res->filter_type, "imac-ivlan"))
+		tunnel_filter_conf.filter_type = RTE_TUNNEL_FILTER_IMAC_IVLAN;
+	else if (!strcmp(res->filter_type, "imac-ivlan-tenid"))
+		tunnel_filter_conf.filter_type =
+			RTE_TUNNEL_FILTER_IMAC_IVLAN_TENID;
+	else if (!strcmp(res->filter_type, "imac-tenid"))
+		tunnel_filter_conf.filter_type = RTE_TUNNEL_FILTER_IMAC_TENID;
+	else if (!strcmp(res->filter_type, "imac"))
+		tunnel_filter_conf.filter_type = ETH_TUNNEL_FILTER_IMAC;
+	else if (!strcmp(res->filter_type, "omac-imac-tenid"))
+		tunnel_filter_conf.filter_type =
+			RTE_TUNNEL_FILTER_OMAC_TENID_IMAC;
+	else {
+		printf("The filter type is not supported");
+		return;
+	}
+
+	tunnel_filter_conf.to_queue = RTE_TUNNEL_FILTER_TO_QUEUE;
+
+	if (!strcmp(res->tunnel_type, "vxlan"))
+		tunnel_filter_conf.tunnel_type = RTE_TUNNEL_TYPE_VXLAN;
+	else {
+		printf("Only VxLAN is supported now.\n");
+		return;
+	}
+
+	tunnel_filter_conf.tenant_id = res->tenant_id;
+	tunnel_filter_conf.queue_id = res->queue_num;
+	if (!strcmp(res->what, "add"))
+		ret = rte_eth_dev_filter_ctrl(res->port_id,
+					RTE_ETH_FILTER_TUNNEL,
+					RTE_ETH_FILTER_ADD,
+					&tunnel_filter_conf);
+	else
+		ret = rte_eth_dev_filter_ctrl(res->port_id,
+					RTE_ETH_FILTER_TUNNEL,
+					RTE_ETH_FILTER_DELETE,
+					&tunnel_filter_conf);
+	if (ret < 0)
+		printf("cmd_tunnel_filter_parsed error: (%s)\n",
+				strerror(-ret));
+
+}
+cmdline_parse_token_string_t cmd_tunnel_filter_cmd =
+	TOKEN_STRING_INITIALIZER(struct cmd_tunnel_filter_result,
+	cmd, "tunnel_filter");
+cmdline_parse_token_string_t cmd_tunnel_filter_what =
+	TOKEN_STRING_INITIALIZER(struct cmd_tunnel_filter_result,
+	what, "add#rm");
+cmdline_parse_token_num_t cmd_tunnel_filter_port_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_tunnel_filter_result,
+	port_id, UINT8);
+cmdline_parse_token_etheraddr_t cmd_tunnel_filter_outer_mac =
+	TOKEN_ETHERADDR_INITIALIZER(struct cmd_tunnel_filter_result,
+	outer_mac);
+cmdline_parse_token_etheraddr_t cmd_tunnel_filter_inner_mac =
+	TOKEN_ETHERADDR_INITIALIZER(struct cmd_tunnel_filter_result,
+	inner_mac);
+cmdline_parse_token_num_t cmd_tunnel_filter_innner_vlan =
+	TOKEN_NUM_INITIALIZER(struct cmd_tunnel_filter_result,
+	inner_vlan, UINT16);
+cmdline_parse_token_ipaddr_t cmd_tunnel_filter_ip_value =
+	TOKEN_IPADDR_INITIALIZER(struct cmd_tunnel_filter_result,
+	ip_value);
+cmdline_parse_token_string_t cmd_tunnel_filter_tunnel_type =
+	TOKEN_STRING_INITIALIZER(struct cmd_tunnel_filter_result,
+	tunnel_type, "vxlan");
+
+cmdline_parse_token_string_t cmd_tunnel_filter_filter_type =
+	TOKEN_STRING_INITIALIZER(struct cmd_tunnel_filter_result,
+	filter_type, "imac-ivlan#imac-ivlan-tenid#imac-tenid#"
+		"imac#omac-imac-tenid");
+cmdline_parse_token_num_t cmd_tunnel_filter_tenant_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_tunnel_filter_result,
+	tenant_id, UINT32);
+cmdline_parse_token_num_t cmd_tunnel_filter_queue_num =
+	TOKEN_NUM_INITIALIZER(struct cmd_tunnel_filter_result,
+	queue_num, UINT16);
+
+cmdline_parse_inst_t cmd_tunnel_filter = {
+	.f = cmd_tunnel_filter_parsed,
+	.data = (void *)0,
+	.help_str = "add/rm tunnel filter of a port: "
+			"tunnel_filter add port_id outer_mac inner_mac ip "
+			"inner_vlan tunnel_type(vxlan) filter_type "
+			"(imac-ivlan|imac-ivlan-tenid|imac-tenid|"
+			"imac|omac-imac-tenid) "
+			"tenant_id queue_num",
+	.tokens = {
+		(void *)&cmd_tunnel_filter_cmd,
+		(void *)&cmd_tunnel_filter_what,
+		(void *)&cmd_tunnel_filter_port_id,
+		(void *)&cmd_tunnel_filter_outer_mac,
+		(void *)&cmd_tunnel_filter_inner_mac,
+		(void *)&cmd_tunnel_filter_ip_value,
+		(void *)&cmd_tunnel_filter_innner_vlan,
+		(void *)&cmd_tunnel_filter_tunnel_type,
+		(void *)&cmd_tunnel_filter_filter_type,
+		(void *)&cmd_tunnel_filter_tenant_id,
+		(void *)&cmd_tunnel_filter_queue_num,
+		NULL,
+	},
+};
+
 /* *** CONFIGURE TUNNEL UDP PORT *** */
 struct cmd_tunnel_udp_config {
 	cmdline_fixed_string_t cmd;
@@ -7582,6 +7733,7 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_vf_rxvlan_filter,
 	(cmdline_parse_inst_t *)&cmd_queue_rate_limit,
 	(cmdline_parse_inst_t *)&cmd_vf_rate_limit,
+	(cmdline_parse_inst_t *)&cmd_tunnel_filter,
 	(cmdline_parse_inst_t *)&cmd_tunnel_udp_config,
 	(cmdline_parse_inst_t *)&cmd_set_mirror_mask,
 	(cmdline_parse_inst_t *)&cmd_set_mirror_link,
-- 
1.7.7.6

[dpdk-dev] [PATCH v6 8/9] i40e:support VxLAN Tx checksum offload

[Jijiang Liu]

Support VxLAN Tx checksum offload, which include
  - outer L3(IP) checksum offload
  - inner L3(IP) checksum offload
  - inner L4(UDP, TCP and SCTP) checksum offload
 
Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
---
 lib/librte_mbuf/rte_mbuf.h      |    1 +
 lib/librte_pmd_i40e/i40e_rxtx.c |   46 +++++++++++++++++++++++++++++++++-----
 2 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h
index 98951a6..4144c0d 100644
--- a/lib/librte_mbuf/rte_mbuf.h
+++ b/lib/librte_mbuf/rte_mbuf.h
@@ -94,6 +94,7 @@ extern "C" {
 
 #define PKT_TX_VLAN_PKT      (1ULL << 55) /**< TX packet is a 802.1q VLAN packet. */
 #define PKT_TX_IP_CKSUM      (1ULL << 54) /**< IP cksum of TX pkt. computed by NIC. */
+#define PKT_TX_VXLAN_CKSUM   (1ULL << 50) /**< TX checksum of VxLAN computed by NIC */
 #define PKT_TX_IPV4_CSUM     PKT_TX_IP_CKSUM /**< Alias of PKT_TX_IP_CKSUM. */
 #define PKT_TX_IPV4          PKT_RX_IPV4_HDR /**< IPv4 with no IP checksum offload. */
 #define PKT_TX_IPV6          PKT_RX_IPV6_HDR /**< IPv6 packet */
diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index 7c3809f..592787f 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -411,11 +411,14 @@ i40e_rxd_ptype_to_pkt_flags(uint64_t qword)
 }
 
 static inline void
-i40e_txd_enable_checksum(uint32_t ol_flags,
+i40e_txd_enable_checksum(uint64_t ol_flags,
 			uint32_t *td_cmd,
 			uint32_t *td_offset,
 			uint8_t l2_len,
-			uint8_t l3_len)
+			uint16_t l3_len,
+			uint8_t inner_l2_len,
+			uint16_t inner_l3_len,
+			uint32_t *cd_tunneling)
 {
 	if (!l2_len) {
 		PMD_DRV_LOG(DEBUG, "L2 length set to 0");
@@ -428,6 +431,27 @@ i40e_txd_enable_checksum(uint32_t ol_flags,
 		return;
 	}
 
+	/* VxLAN packet TX checksum offload */
+	if (unlikely(ol_flags & PKT_TX_VXLAN_CKSUM)) {
+		uint8_t l4tun_len;
+
+		l4tun_len = ETHER_VXLAN_HLEN + inner_l2_len;
+
+		if (ol_flags & PKT_TX_IPV4_CSUM)
+			*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV4;
+		else if (ol_flags & PKT_TX_IPV6)
+			*cd_tunneling |= I40E_TX_CTX_EXT_IP_IPV6;
+
+		/* Now set the ctx descriptor fields */
+		*cd_tunneling |= (l3_len >> 2) <<
+				I40E_TXD_CTX_QW0_EXT_IPLEN_SHIFT |
+				I40E_TXD_CTX_UDP_TUNNELING |
+				(l4tun_len >> 1) <<
+				I40E_TXD_CTX_QW0_NATLEN_SHIFT;
+
+		l3_len = inner_l3_len;
+	}
+
 	/* Enable L3 checksum offloads */
 	if (ol_flags & PKT_TX_IPV4_CSUM) {
 		*td_cmd |= I40E_TX_DESC_CMD_IIPT_IPV4_CSUM;
@@ -1077,7 +1101,10 @@ i40e_recv_scattered_pkts(void *rx_queue,
 static inline uint16_t
 i40e_calc_context_desc(uint64_t flags)
 {
-	uint16_t mask = 0;
+	uint64_t mask = 0ULL;
+
+	if (flags | PKT_TX_VXLAN_CKSUM)
+		mask |= PKT_TX_VXLAN_CKSUM;
 
 #ifdef RTE_LIBRTE_IEEE1588
 	mask |= PKT_TX_IEEE1588_TMST;
@@ -1098,6 +1125,7 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	volatile struct i40e_tx_desc *txr;
 	struct rte_mbuf *tx_pkt;
 	struct rte_mbuf *m_seg;
+	uint32_t cd_tunneling_params;
 	uint16_t tx_id;
 	uint16_t nb_tx;
 	uint32_t td_cmd;
@@ -1106,7 +1134,9 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 	uint32_t td_tag;
 	uint64_t ol_flags;
 	uint8_t l2_len;
-	uint8_t l3_len;
+	uint16_t l3_len;
+	uint8_t inner_l2_len;
+	uint16_t inner_l3_len;
 	uint16_t nb_used;
 	uint16_t nb_ctx;
 	uint16_t tx_last;
@@ -1134,7 +1164,9 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 
 		ol_flags = tx_pkt->ol_flags;
 		l2_len = tx_pkt->l2_len;
+		inner_l2_len = tx_pkt->inner_l2_len;
 		l3_len = tx_pkt->l3_len;
+		inner_l3_len = tx_pkt->inner_l3_len;
 
 		/* Calculate the number of context descriptors needed. */
 		nb_ctx = i40e_calc_context_desc(ol_flags);
@@ -1182,15 +1214,17 @@ i40e_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts, uint16_t nb_pkts)
 		td_cmd |= I40E_TX_DESC_CMD_ICRC;
 
 		/* Enable checksum offloading */
+		cd_tunneling_params = 0;
 		i40e_txd_enable_checksum(ol_flags, &td_cmd, &td_offset,
-							l2_len, l3_len);
+						l2_len, l3_len, inner_l2_len,
+						inner_l3_len,
+						&cd_tunneling_params);
 
 		if (unlikely(nb_ctx)) {
 			/* Setup TX context descriptor if required */
 			volatile struct i40e_tx_context_desc *ctx_txd =
 				(volatile struct i40e_tx_context_desc *)\
 							&txr[tx_id];
-			uint32_t cd_tunneling_params = 0;
 			uint16_t cd_l2tag2 = 0;
 			uint64_t cd_type_cmd_tso_mss =
 				I40E_TX_DESC_DTYPE_CONTEXT;
-- 
1.7.7.6

[dpdk-dev] [PATCH v6 9/9] app/testpmd:test VxLAN Tx checksum offload

[Jijiang Liu]

Add test cases in testpmd to test VxLAN Tx Checksum offload, which include
 - IPv4 and IPv6 packet 
 - outer L3, inner L3 and L4 checksum offload for Tx side.
 
Signed-off-by: Jijiang Liu <jijiang.liu@intel.com>
Acked-by: Helin Zhang <helin.zhang@intel.com>
Acked-by: Jingjing Wu <jingjing.wu@intel.com>
Acked-by: Jing Chen <jing.d.chen@intel.com>
---
 app/test-pmd/cmdline.c  |   13 ++-
 app/test-pmd/config.c   |    6 +-
 app/test-pmd/csumonly.c |  195 +++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 192 insertions(+), 22 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index bc9a30c..d738258 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -310,13 +310,17 @@ static void cmd_help_long_parsed(void *parsed_result,
 			"    Disable hardware insertion of a VLAN header in"
 			" packets sent on a port.\n\n"
 
-			"tx_checksum set mask (port_id)\n"
+			"tx_checksum set (mask) (port_id)\n"
 			"    Enable hardware insertion of checksum offload with"
-			" the 4-bit mask, 0~0xf, in packets sent on a port.\n"
+			" the 8-bit mask, 0~0xff, in packets sent on a port.\n"
 			"        bit 0 - insert ip   checksum offload if set\n"
 			"        bit 1 - insert udp  checksum offload if set\n"
 			"        bit 2 - insert tcp  checksum offload if set\n"
 			"        bit 3 - insert sctp checksum offload if set\n"
+			"        bit 4 - insert inner ip  checksum offload if set\n"
+			"        bit 5 - insert inner udp checksum offload if set\n"
+			"        bit 6 - insert inner tcp checksum offload if set\n"
+			"        bit 7 - insert inner sctp checksum offload if set\n"
 			"    Please check the NIC datasheet for HW limits.\n\n"
 
 			"set fwd (%s)\n"
@@ -2763,8 +2767,9 @@ cmdline_parse_inst_t cmd_tx_cksum_set = {
 	.f = cmd_tx_cksum_set_parsed,
 	.data = NULL,
 	.help_str = "enable hardware insertion of L3/L4checksum with a given "
-	"mask in packets sent on a port, the bit mapping is given as, Bit 0 for ip"
-	"Bit 1 for UDP, Bit 2 for TCP, Bit 3 for SCTP",
+	"mask in packets sent on a port, the bit mapping is given as, Bit 0 for ip, "
+	"Bit 1 for UDP, Bit 2 for TCP, Bit 3 for SCTP, Bit 4 for inner ip, "
+	"Bit 5 for inner UDP, Bit 6 for inner TCP, Bit 7 for inner SCTP",
 	.tokens = {
 		(void *)&cmd_tx_cksum_set_tx_cksum,
 		(void *)&cmd_tx_cksum_set_set,
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 2a1b93f..9bc08f4 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1753,9 +1753,9 @@ tx_cksum_set(portid_t port_id, uint64_t ol_flags)
 	uint64_t tx_ol_flags;
 	if (port_id_is_invalid(port_id))
 		return;
-	/* Clear last 4 bits and then set L3/4 checksum mask again */
-	tx_ol_flags = ports[port_id].tx_ol_flags & (~0x0Full);
-	ports[port_id].tx_ol_flags = ((ol_flags & 0xf) | tx_ol_flags);
+	/* Clear last 8 bits and then set L3/4 checksum mask again */
+	tx_ol_flags = ports[port_id].tx_ol_flags & (~0x0FFull);
+	ports[port_id].tx_ol_flags = ((ol_flags & 0xff) | tx_ol_flags);
 }
 
 void
diff --git a/app/test-pmd/csumonly.c b/app/test-pmd/csumonly.c
index fcc4876..e2ac129 100644
--- a/app/test-pmd/csumonly.c
+++ b/app/test-pmd/csumonly.c
@@ -196,7 +196,6 @@ get_ipv6_udptcp_checksum(struct ipv6_hdr *ipv6_hdr, uint16_t *l4_hdr)
 	return (uint16_t)cksum;
 }
 
-
 /*
  * Forwarding of packets. Change the checksum field with HW or SW methods
  * The HW/SW method selection depends on the ol_flags on every packet
@@ -209,10 +208,16 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 	struct rte_mbuf  *mb;
 	struct ether_hdr *eth_hdr;
 	struct ipv4_hdr  *ipv4_hdr;
+	struct ether_hdr *inner_eth_hdr;
+	struct ipv4_hdr  *inner_ipv4_hdr = NULL;
 	struct ipv6_hdr  *ipv6_hdr;
+	struct ipv6_hdr  *inner_ipv6_hdr = NULL;
 	struct udp_hdr   *udp_hdr;
+	struct udp_hdr   *inner_udp_hdr;
 	struct tcp_hdr   *tcp_hdr;
+	struct tcp_hdr   *inner_tcp_hdr;
 	struct sctp_hdr  *sctp_hdr;
+	struct sctp_hdr  *inner_sctp_hdr;
 
 	uint16_t nb_rx;
 	uint16_t nb_tx;
@@ -221,12 +226,18 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 	uint64_t pkt_ol_flags;
 	uint64_t tx_ol_flags;
 	uint16_t l4_proto;
+	uint16_t inner_l4_proto = 0;
 	uint16_t eth_type;
 	uint8_t  l2_len;
 	uint8_t  l3_len;
+	uint8_t  inner_l2_len = 0;
+	uint8_t  inner_l3_len = 0;
 
 	uint32_t rx_bad_ip_csum;
 	uint32_t rx_bad_l4_csum;
+	uint8_t  ipv4_tunnel;
+	uint8_t  ipv6_tunnel;
+	uint16_t ptype;
 
 #ifdef RTE_TEST_PMD_RECORD_CORE_CYCLES
 	uint64_t start_tsc;
@@ -262,7 +273,9 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		l2_len  = sizeof(struct ether_hdr);
 		pkt_ol_flags = mb->ol_flags;
 		ol_flags = (pkt_ol_flags & (~PKT_TX_L4_MASK));
-
+		ptype = mb->packet_type;
+		ipv4_tunnel = IS_ETH_IPV4_TUNNEL(ptype);
+		ipv6_tunnel = IS_ETH_IPV6_TUNNEL(ptype);
 		eth_hdr = rte_pktmbuf_mtod(mb, struct ether_hdr *);
 		eth_type = rte_be_to_cpu_16(eth_hdr->ether_type);
 		if (eth_type == ETHER_TYPE_VLAN) {
@@ -295,7 +308,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		 *      + ipv4 or ipv6
 		 *      + udp or tcp or sctp or others
 		 */
-		if (pkt_ol_flags & PKT_RX_IPV4_HDR) {
+		if (pkt_ol_flags & (PKT_RX_IPV4_HDR | PKT_RX_IPV4_HDR_EXT)) {
 
 			/* Do not support ipv4 option field */
 			l3_len = sizeof(struct ipv4_hdr) ;
@@ -325,17 +338,92 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 				if (tx_ol_flags & 0x2) {
 					/* HW Offload */
 					ol_flags |= PKT_TX_UDP_CKSUM;
-					/* Pseudo header sum need be set properly */
-					udp_hdr->dgram_cksum = get_ipv4_psd_sum(ipv4_hdr);
+					if (ipv4_tunnel)
+						udp_hdr->dgram_cksum = 0;
+					else
+						/* Pseudo header sum need be set properly */
+						udp_hdr->dgram_cksum =
+							get_ipv4_psd_sum(ipv4_hdr);
 				}
 				else {
 					/* SW Implementation, clear checksum field first */
 					udp_hdr->dgram_cksum = 0;
 					udp_hdr->dgram_cksum = get_ipv4_udptcp_checksum(ipv4_hdr,
-							(uint16_t*)udp_hdr);
+									(uint16_t *)udp_hdr);
 				}
-			}
-			else if (l4_proto == IPPROTO_TCP){
+
+				if (ipv4_tunnel) {
+
+					uint16_t len;
+
+					/* Check if inner L3/L4 checkum flag is set */
+					if (tx_ol_flags & 0xF0)
+						ol_flags |= PKT_TX_VXLAN_CKSUM;
+
+					inner_l2_len  = sizeof(struct ether_hdr);
+					inner_eth_hdr = (struct ether_hdr *) (rte_pktmbuf_mtod(mb,
+								unsigned char *) + l2_len + l3_len
+								 + ETHER_VXLAN_HLEN);
+
+					eth_type = rte_be_to_cpu_16(inner_eth_hdr->ether_type);
+					if (eth_type == ETHER_TYPE_VLAN) {
+						inner_l2_len += sizeof(struct vlan_hdr);
+						eth_type = rte_be_to_cpu_16(*(uint16_t *)
+							((uintptr_t)&eth_hdr->ether_type +
+								sizeof(struct vlan_hdr)));
+					}
+
+					len = l2_len + l3_len + ETHER_VXLAN_HLEN + inner_l2_len;
+					if (eth_type == ETHER_TYPE_IPv4) {
+						inner_l3_len = sizeof(struct ipv4_hdr);
+						inner_ipv4_hdr = (struct ipv4_hdr *) (rte_pktmbuf_mtod(mb,
+								unsigned char *) + len);
+						inner_l4_proto = inner_ipv4_hdr->next_proto_id;
+
+						if (tx_ol_flags & 0x10) {
+
+							/* Do not delete, this is required by HW*/
+							inner_ipv4_hdr->hdr_checksum = 0;
+							ol_flags |= PKT_TX_IPV4_CSUM;
+						}
+
+					} else if (eth_type == ETHER_TYPE_IPv6) {
+						inner_l3_len = sizeof(struct ipv6_hdr);
+						inner_ipv6_hdr = (struct ipv6_hdr *) (rte_pktmbuf_mtod(mb,
+								unsigned char *) + len);
+						inner_l4_proto = inner_ipv6_hdr->proto;
+					}
+					if ((inner_l4_proto == IPPROTO_UDP) && (tx_ol_flags & 0x20)) {
+
+						/* HW Offload */
+						ol_flags |= PKT_TX_UDP_CKSUM;
+						inner_udp_hdr = (struct udp_hdr *) (rte_pktmbuf_mtod(mb,
+								unsigned char *) + len + inner_l3_len);
+						if (eth_type == ETHER_TYPE_IPv4)
+							inner_udp_hdr->dgram_cksum = get_ipv4_psd_sum(inner_ipv4_hdr);
+						else if (eth_type == ETHER_TYPE_IPv6)
+							inner_udp_hdr->dgram_cksum = get_ipv6_psd_sum(inner_ipv6_hdr);
+
+					} else if ((inner_l4_proto == IPPROTO_TCP) && (tx_ol_flags & 0x40)) {
+						/* HW Offload */
+						ol_flags |= PKT_TX_TCP_CKSUM;
+						inner_tcp_hdr = (struct tcp_hdr *) (rte_pktmbuf_mtod(mb,
+								unsigned char *) + len + inner_l3_len);
+						if (eth_type == ETHER_TYPE_IPv4)
+							inner_tcp_hdr->cksum = get_ipv4_psd_sum(inner_ipv4_hdr);
+						else if (eth_type == ETHER_TYPE_IPv6)
+							inner_tcp_hdr->cksum = get_ipv6_psd_sum(inner_ipv6_hdr);
+					} else if ((inner_l4_proto == IPPROTO_SCTP) && (tx_ol_flags & 0x80)) {
+						/* HW Offload */
+						ol_flags |= PKT_TX_SCTP_CKSUM;
+						inner_sctp_hdr = (struct sctp_hdr *) (rte_pktmbuf_mtod(mb,
+								unsigned char *) + len + inner_l3_len);
+						inner_sctp_hdr->cksum = 0;
+					}
+
+				}
+
+			} else if (l4_proto == IPPROTO_TCP) {
 				tcp_hdr = (struct tcp_hdr*) (rte_pktmbuf_mtod(mb,
 						unsigned char *) + l2_len + l3_len);
 				if (tx_ol_flags & 0x4) {
@@ -347,8 +435,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 					tcp_hdr->cksum = get_ipv4_udptcp_checksum(ipv4_hdr,
 							(uint16_t*)tcp_hdr);
 				}
-			}
-			else if (l4_proto == IPPROTO_SCTP) {
+			} else if (l4_proto == IPPROTO_SCTP) {
 				sctp_hdr = (struct sctp_hdr*) (rte_pktmbuf_mtod(mb,
 						unsigned char *) + l2_len + l3_len);
 
@@ -367,9 +454,7 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 				}
 			}
 			/* End of L4 Handling*/
-		}
-		else if (pkt_ol_flags & PKT_RX_IPV6_HDR) {
-
+		} else if (pkt_ol_flags & (PKT_RX_IPV6_HDR | PKT_RX_IPV6_HDR_EXT)) {
 			ipv6_hdr = (struct ipv6_hdr *) (rte_pktmbuf_mtod(mb,
 					unsigned char *) + l2_len);
 			l3_len = sizeof(struct ipv6_hdr) ;
@@ -382,15 +467,93 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 				if (tx_ol_flags & 0x2) {
 					/* HW Offload */
 					ol_flags |= PKT_TX_UDP_CKSUM;
-					udp_hdr->dgram_cksum = get_ipv6_psd_sum(ipv6_hdr);
+					if (ipv6_tunnel)
+						udp_hdr->dgram_cksum = 0;
+					else
+						udp_hdr->dgram_cksum =
+							get_ipv6_psd_sum(ipv6_hdr);
 				}
 				else {
 					/* SW Implementation */
 					/* checksum field need be clear first */
 					udp_hdr->dgram_cksum = 0;
 					udp_hdr->dgram_cksum = get_ipv6_udptcp_checksum(ipv6_hdr,
-							(uint16_t*)udp_hdr);
+								(uint16_t *)udp_hdr);
 				}
+
+				if (ipv6_tunnel) {
+
+					uint16_t len;
+
+					/* Check if inner L3/L4 checksum flag is set */
+					if (tx_ol_flags & 0xF0)
+						ol_flags |= PKT_TX_VXLAN_CKSUM;
+
+					inner_l2_len  = sizeof(struct ether_hdr);
+					inner_eth_hdr = (struct ether_hdr *) (rte_pktmbuf_mtod(mb,
+						unsigned char *) + l2_len + l3_len + ETHER_VXLAN_HLEN);
+					eth_type = rte_be_to_cpu_16(inner_eth_hdr->ether_type);
+
+					if (eth_type == ETHER_TYPE_VLAN) {
+						inner_l2_len += sizeof(struct vlan_hdr);
+						eth_type = rte_be_to_cpu_16(*(uint16_t *)
+							((uintptr_t)&eth_hdr->ether_type +
+							sizeof(struct vlan_hdr)));
+					}
+
+					len = l2_len + l3_len + ETHER_VXLAN_HLEN + inner_l2_len;
+
+					if (eth_type == ETHER_TYPE_IPv4) {
+						inner_l3_len = sizeof(struct ipv4_hdr);
+						inner_ipv4_hdr = (struct ipv4_hdr *) (rte_pktmbuf_mtod(mb,
+								unsigned char *) + len);
+						inner_l4_proto = inner_ipv4_hdr->next_proto_id;
+
+						/* HW offload */
+						if (tx_ol_flags & 0x10) {
+
+							/* Do not delete, this is required by HW*/
+							inner_ipv4_hdr->hdr_checksum = 0;
+							ol_flags |= PKT_TX_IPV4_CSUM;
+						}
+					} else if (eth_type == ETHER_TYPE_IPv6) {
+						inner_l3_len = sizeof(struct ipv6_hdr);
+						inner_ipv6_hdr = (struct ipv6_hdr *) (rte_pktmbuf_mtod(mb,
+							unsigned char *) + len);
+						inner_l4_proto = inner_ipv6_hdr->proto;
+					}
+
+					if ((inner_l4_proto == IPPROTO_UDP) && (tx_ol_flags & 0x20)) {
+						inner_udp_hdr = (struct udp_hdr *) (rte_pktmbuf_mtod(mb,
+							unsigned char *) + len + inner_l3_len);
+						/* HW offload */
+						ol_flags |= PKT_TX_UDP_CKSUM;
+						inner_udp_hdr->dgram_cksum = 0;
+						if (eth_type == ETHER_TYPE_IPv4)
+							inner_udp_hdr->dgram_cksum = get_ipv4_psd_sum(inner_ipv4_hdr);
+						else if (eth_type == ETHER_TYPE_IPv6)
+							inner_udp_hdr->dgram_cksum = get_ipv6_psd_sum(inner_ipv6_hdr);
+					} else if ((inner_l4_proto == IPPROTO_TCP) && (tx_ol_flags & 0x40)) {
+						/* HW offload */
+						ol_flags |= PKT_TX_TCP_CKSUM;
+						inner_tcp_hdr = (struct tcp_hdr *) (rte_pktmbuf_mtod(mb,
+								unsigned char *) + len + inner_l3_len);
+
+						if (eth_type == ETHER_TYPE_IPv4)
+							inner_tcp_hdr->cksum = get_ipv4_psd_sum(inner_ipv4_hdr);
+						else if (eth_type == ETHER_TYPE_IPv6)
+							inner_tcp_hdr->cksum = get_ipv6_psd_sum(inner_ipv6_hdr);
+
+					} else if ((inner_l4_proto == IPPROTO_SCTP) && (tx_ol_flags & 0x80)) {
+						/* HW offload */
+						ol_flags |= PKT_TX_SCTP_CKSUM;
+						inner_sctp_hdr = (struct sctp_hdr *) (rte_pktmbuf_mtod(mb,
+								unsigned char *) + len + inner_l3_len);
+						inner_sctp_hdr->cksum = 0;
+					}
+
+				}
+
 			}
 			else if (l4_proto == IPPROTO_TCP) {
 				tcp_hdr = (struct tcp_hdr*) (rte_pktmbuf_mtod(mb,
@@ -434,6 +597,8 @@ pkt_burst_checksum_forward(struct fwd_stream *fs)
 		/* Combine the packet header write. VLAN is not consider here */
 		mb->l2_len = l2_len;
 		mb->l3_len = l3_len;
+		mb->inner_l2_len = inner_l2_len;
+		mb->inner_l3_len = inner_l3_len;
 		mb->ol_flags = ol_flags;
 	}
 	nb_tx = rte_eth_tx_burst(fs->tx_port, fs->tx_queue, pkts_burst, nb_rx);
-- 
1.7.7.6

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Thomas Monjalon]

Hi Jijiang,

2014-10-21 16:46, Jijiang Liu:
> Remove the "reserved2" field and add the "packet_type"

"Remove and add" can be said "Replace".

> and the "inner_l2_l3_len" fields in the rte_mbuf structure.

Please explain that you are using 2 bytes of the second cache line
for TX offloading of tunnels.

>  	/* remaining bytes are set on RX when pulling packet from descriptor */
>  	MARKER rx_descriptor_fields1;
> -	uint16_t reserved2;       /**< Unused field. Required for padding */
> +
> +	/**
> +	 * Packet type, which is used to indicate ordinary L2 packet format and
> +	 * also tunneled packet format such as IP in IP, IP in GRE, MAC in GRE
> +	 * and MAC in UDP.
> +	 */
> +	uint16_t packet_type;

Why not name it "l2_type"?

-- 
Thomas

Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet identification API in librte_ether

[Thomas Monjalon]

2014-10-21 16:46, Jijiang Liu:
> There are "some" destination UDP port numbers that have unque meaning.
> In terms of VxLAN, "IANA has assigned the value 4789 for the VXLAN UDP port, and this value
> SHOULD be used by default as the destination UDP port. Some early implementations of VXLAN
> have used other values for the destination port. To enable interoperability with these 
> implementations, the destination port SHOULD be configurable."
> 
> Add two APIs in librte_ether for supporting UDP tunneling port configuration on i40e.
> Currently, only VxLAN is implemented in this patch set.

Actually, there are 2 different things in this patch
- new tunnelling API
- VXLAN macros
Please split in 2 patches.

>  int
> +rte_eth_dev_udp_tunnel_add(uint8_t port_id,
> +			   struct rte_eth_udp_tunnel *udp_tunnel,
> +			   uint8_t count)
> +{
> +	uint8_t i;
> +	struct rte_eth_dev *dev;
> +	struct rte_eth_udp_tunnel *tunnel;
> +
> +	if (port_id >= nb_ports) {
> +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> +		return -ENODEV;
> +	}
> +
> +	if (udp_tunnel == NULL) {
> +		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
> +		return -EINVAL;
> +	}
> +	tunnel = udp_tunnel;
> +
> +	for (i = 0; i < count; i++, tunnel++) {
> +		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
> +			PMD_DEBUG_TRACE("Invalid tunnel type\n");
> +			return -EINVAL;
> +		}
> +	}

I'm not sure it's a good idea to provide a count parameter to iterate in a loop.
It's probably something that the application should do by itself.

> +
> +	dev = &rte_eth_devices[port_id];
> +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->udp_tunnel_add, -ENOTSUP);
> +	return (*dev->dev_ops->udp_tunnel_add)(dev, udp_tunnel, count);
> +}

[...]

> +/**
> + * Tunneled type.
> + */
> +enum rte_eth_tunnel_type {
> +	RTE_TUNNEL_TYPE_NONE = 0,
> +	RTE_TUNNEL_TYPE_VXLAN,
> +	RTE_TUNNEL_TYPE_GENEVE,
> +	RTE_TUNNEL_TYPE_TEREDO,
> +	RTE_TUNNEL_TYPE_NVGRE,
> +	RTE_TUNNEL_TYPE_MAX,
> +};

This is moved later from rte_ethdev.h to rte_eth_ctrl.h.
Please choose where is the right location in this patch.
By the way, I think ethdev is the right location because it's not directly
related to ctrl API, right?

>  struct rte_eth_conf {
> +	enum rte_eth_tunnel_type tunnel_type;
>  	uint16_t link_speed;
>  	/**< ETH_LINK_SPEED_10[0|00|000], or 0 for autonegotation */
>  	uint16_t link_duplex;

Please don't add this field as the first. It's more logical to start
port configuration with link speed, duplex, etc.
Then please add a comment to explain how it should be used.
But I doubt we should configure a tunnel type for a whole port.
There's something weird here.

> +/* VXLAN protocol header */

This comment should be doxygen'ed.

> +struct vxlan_hdr {
> +	uint32_t vx_flags; /**< VxLAN flag. */
> +	uint32_t vx_vni;   /**< VxLAN ID. */
> +} __attribute__((__packed__));
> +
[...]
>  #define ETHER_TYPE_VLAN 0x8100 /**< IEEE 802.1Q VLAN tagging. */
>  #define ETHER_TYPE_1588 0x88F7 /**< IEEE 802.1AS 1588 Precise Time Protocol. */
>  
> +#define ETHER_VXLAN_HLEN (sizeof(struct udp_hdr) + sizeof(struct vxlan_hdr))

Please add a doxygen comment.

-- 
Thomas

Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet identification API in librte_ether

[Liu, Jijiang]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, October 21, 2014 6:51 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet
> identification API in librte_ether
> 
> 2014-10-21 16:46, Jijiang Liu:
> > There are "some" destination UDP port numbers that have unque meaning.
> > In terms of VxLAN, "IANA has assigned the value 4789 for the VXLAN UDP
> > port, and this value SHOULD be used by default as the destination UDP
> > port. Some early implementations of VXLAN have used other values for
> > the destination port. To enable interoperability with these
> implementations, the destination port SHOULD be configurable."
> >
> > Add two APIs in librte_ether for supporting UDP tunneling port
> configuration on i40e.
> > Currently, only VxLAN is implemented in this patch set.
> 
> Actually, there are 2 different things in this patch
> - new tunnelling API
> - VXLAN macros
> Please split in 2 patches.
Ok
> >  int
> > +rte_eth_dev_udp_tunnel_add(uint8_t port_id,
> > +			   struct rte_eth_udp_tunnel *udp_tunnel,
> > +			   uint8_t count)
> > +{
> > +	uint8_t i;
> > +	struct rte_eth_dev *dev;
> > +	struct rte_eth_udp_tunnel *tunnel;
> > +
> > +	if (port_id >= nb_ports) {
> > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > +		return -ENODEV;
> > +	}
> > +
> > +	if (udp_tunnel == NULL) {
> > +		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
> > +		return -EINVAL;
> > +	}
> > +	tunnel = udp_tunnel;
> > +
> > +	for (i = 0; i < count; i++, tunnel++) {
> > +		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
> > +			PMD_DEBUG_TRACE("Invalid tunnel type\n");
> > +			return -EINVAL;
> > +		}
> > +	}
> 
> I'm not sure it's a good idea to provide a count parameter to iterate in a
> loop.
> It's probably something that the application should do by itself.

It is necessary to check if this prot_type(tunnel type) is valid here in case applications don't do that. 

> > +
> > +	dev = &rte_eth_devices[port_id];
> > +	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->udp_tunnel_add, -
> ENOTSUP);
> > +	return (*dev->dev_ops->udp_tunnel_add)(dev, udp_tunnel, count); }
> 
> [...]
> 
> > +/**
> > + * Tunneled type.
> > + */
> > +enum rte_eth_tunnel_type {
> > +	RTE_TUNNEL_TYPE_NONE = 0,
> > +	RTE_TUNNEL_TYPE_VXLAN,
> > +	RTE_TUNNEL_TYPE_GENEVE,
> > +	RTE_TUNNEL_TYPE_TEREDO,
> > +	RTE_TUNNEL_TYPE_NVGRE,
> > +	RTE_TUNNEL_TYPE_MAX,
> > +};
> 
> This is moved later from rte_ethdev.h to rte_eth_ctrl.h.
> Please choose where is the right location in this patch.
> By the way, I think ethdev is the right location because it's not directly
> related to ctrl API, right?

It is used in both rte_ethdev.h file and rte_eth_ctrl.h file. 　
In rte_eth_ctrl.h file, it is used in rte_eth_tunnel_filter_conf structure.
+struct rte_eth_tunnel_filter_conf {
+	...
+	enum rte_eth_tunnel_type tunnel_type; /**< Tunnel Type. */
+	...
+};

I will put the rte_eth_tunnel_type definition in the rte_eth_ctrl.h file at the beginning of definition.


> >  struct rte_eth_conf {
> > +	enum rte_eth_tunnel_type tunnel_type;
> >  	uint16_t link_speed;
> >  	/**< ETH_LINK_SPEED_10[0|00|000], or 0 for autonegotation */
> >  	uint16_t link_duplex;
> 
> Please don't add this field as the first. It's more logical to start port
> configuration with link speed, duplex, etc.

ok
> Then please add a comment to explain how it should be used.
I will add more comment for it.
> But I doubt we should configure a tunnel type for a whole port.
Yes, your understanding is correct. It is for a whole port/PF, that's why we should add 
tunnel_type in  rte_eth_conf structure.

> There's something weird here.


> > +/* VXLAN protocol header */
> 
> This comment should be doxygen'ed.
> 
> > +struct vxlan_hdr {
> > +	uint32_t vx_flags; /**< VxLAN flag. */
> > +	uint32_t vx_vni;   /**< VxLAN ID. */
> > +} __attribute__((__packed__));
> > +
> [...]
> >  #define ETHER_TYPE_VLAN 0x8100 /**< IEEE 802.1Q VLAN tagging. */
> > #define ETHER_TYPE_1588 0x88F7 /**< IEEE 802.1AS 1588 Precise Time
> > Protocol. */
> >
> > +#define ETHER_VXLAN_HLEN (sizeof(struct udp_hdr) + sizeof(struct
> > +vxlan_hdr))
> 
> Please add a doxygen comment.
ok
> --
> Thomas

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Liu, Jijiang]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, October 21, 2014 6:26 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure
> changes
> 
> Hi Jijiang,
> 
> 2014-10-21 16:46, Jijiang Liu:
> > Remove the "reserved2" field and add the "packet_type"
> 
> "Remove and add" can be said "Replace".
> 
> > and the "inner_l2_l3_len" fields in the rte_mbuf structure.
> 
> Please explain that you are using 2 bytes of the second cache line for TX
> offloading of tunnels.
> 
> >  	/* remaining bytes are set on RX when pulling packet from descriptor
> */
> >  	MARKER rx_descriptor_fields1;
> > -	uint16_t reserved2;       /**< Unused field. Required for padding */
> > +
> > +	/**
> > +	 * Packet type, which is used to indicate ordinary L2 packet format
> and
> > +	 * also tunneled packet format such as IP in IP, IP in GRE, MAC in GRE
> > +	 * and MAC in UDP.
> > +	 */
> > +	uint16_t packet_type;
> 
> Why not name it "l2_type"?

In datasheet, this term is called packet type(s).
Personally , I think packet type is  more clear what meaning of this field is . ^_^

> --
> Thomas

Re: [dpdk-dev] [PATCH v6 0/9] Support VxLAN on Fortville

[Liu, Yong]

Tested-by: Yong Liu <yong.liu@intel.com>

- Tested Commit: 455d09e54b92a4626e178b020fe9c23e43ede3f7
- OS: Fedora20 3.15.8-200.fc20.x86_64
- GCC: gcc version 4.8.3 20140624
- CPU: Intel(R) Xeon(R) CPU E5-2680 v2 @ 2.80GHz
- NIC: Intel Corporation Ethernet Controller XL710 for 40GbE QSFP+ [8086:1583]
- Default x86_64-native-linuxapp-gcc configuration
- Total 6 cases, 6 passed, 0 failed

- Case: vxlan_ipv4_detect
  Description: Check testpmd can receive and detect vxlan packet 
  Command / instruction:
	Start testpmd with vxlan enabled and rss disabled
		testpmd -c ffff -n 4 -- -i --tunnel-type=1 --disable-rss --rxq=4 --txq=4 --nb-cores=8 --nb-ports=2
	Enable VxLAN on both ports and UDP dport setting to 4789
		testpmd>rx_vxlan_port add 4789 0
		testpmd>rx_vxlan_port add 4789 1
	Set forward type to rxonly and enable detail log output
		testpmd>set fwd rxonly
		testpmd>set verbose 1
		testpmd>start
	Send packets with udp/tcp/sctp inner L4 data
  Expected test result:
	testpmd can receive the vxlan packet with different inner L4 data and detect whether the packet is vxlan packet.

- Case: vxlan_ipv6_detect
  Description: Check testpmd can receive and detect ipv6 vxlan packet
  Command / instruction:
	Start testpmd with vxlan enabled and rss disabled
		testpmd -c ffff -n 4 -- -i --tunnel-type=1 --disable-rss --rxq=4 --txq=4 --nb-cores=8 --nb-ports=2
	Enable VxLAN on both ports and UDP dport setting to 4789
		testpmd>rx_vxlan_port add 4789 0
		testpmd>rx_vxlan_port add 4789 1
	Set forward type to rxonly and enable detail log output
		testpmd>set fwd rxonly
		testpmd>set verbose 1
		testpmd>start
	Send vxlan packets with outer IPv6 header and inner IPv6 header.
  Expected test result:
	testpmd can receive the vxlan packet with different inner L4 data and detect whether the packet is IPv6 vxlan packet.

- Case: vxlan_ipv4_checksum_offload
  Description: Check testpmd can offload vxlan checksum and forward the packet
  Command / instruction:
	Start testpmd with vxlan enabled and rss disabled.
		testpmd -c ffff -n 4 -- -i --tunnel-type=1 --disable-rss --rxq=4 --txq=4 --nb-cores=8 --nb-ports=2
	Enable VxLAN on both ports and UDP dport setting to 4789
		testpmd>rx_vxlan_port add 4789 0
		testpmd>rx_vxlan_port add 4789 1
	Set csum packet forwarding mode and enable verbose log.
		testpmd>set fwd csum
		testpmd>set verbose 1
		testpmd>start
	Enable outer IP,UDP,TCP,SCTP and inner IP,UDP checksum offload when inner L4 protocal is UDP.
		testpmd>tx_checksum set 0 0xf3
	Enable outer IP,UDP,TCP,SCTP and inner IP,TCP,SCTP checksum offload when inner L4 protocal is TCP or SCTP.
		testpmd>tx_checksum set 0 0xfd
	Send ipv4 vxlan packet with invalid outer/inner l3 or l4 checksum.	
  Expected test result:
	testpmd can forwarded vxlan packet and the checksum is corrected. The chksum error counter also increased.

- Case: vxlan_ipv6_checksum_offload
  Description: Check testpmd can offload ipv6 vxlan checksum and forward the packet 
  Command / instruction:
	Start testpmd with vxlan enabled and rss disabled.
		testpmd -c ffff -n 4 -- -i --tunnel-type=1 --disable-rss --rxq=4 --txq=4 --nb-cores=8 --nb-ports=2
	Enable VxLAN on both ports and UDP dport setting to 4789
		testpmd>rx_vxlan_port add 4789 0
		testpmd>rx_vxlan_port add 4789 1
	Set csum packet forwarding mode and enable verbose log.
		testpmd>set fwd csum
		testpmd>set verbose 1
		testpmd>start
	Enable outer IP,UDP,TCP,SCTP and inner IP,UDP checksum offload when inner L4 protocal is UDP.
		testpmd>tx_checksum set 0 0xf3
	Enable outer IP,UDP,TCP,SCTP and inner IP,TCP,SCTP checksum offload when inner L4 protocal is TCP or SCTP.
		testpmd>tx_checksum set 0 0xfd
	Send ipv6 vxlan packet with invalid outer/inner l3 or l4 checksum.
  Expected test result:
	testpmd can forwarded vxlan packet and the checksum is corrected. The chksum error counter also increased.


- Case: tunnel_filter
  Description: Check FVL vxlan tunnel filter function work with testpmd.
  Command / instruction:
	Start testpmd with vxlan enabled and rss disabled.
		testpmd -c ffff -n 4 -- -i --tunnel-type=1 --disable-rss --rxq=4 --txq=4 --nb-cores=8 --nb-ports=2
	Enable VxLAN on both ports and UDP dport setting to 4789
		testpmd>rx_vxlan_port add 4789 0
		testpmd>rx_vxlan_port add 4
	Set rxonly forwarding mode and enable verbose log.
		testpmd>set fwd rxonly
		testpmd>set verbose 1
		testpmd>start
	Add cloud filter with specified inner dst mac and inner vlan.
		testpmd>tunnel_filter add 0 11:22:33:44:55:66 00:00:20:00:00:01 192.168.2.2 1 vxlan imac-ivlan 1 3
	Add cloud filter with specified inner dst mac,inner vlan and tunnel id. 
		testpmd>tunnel_filter add 0 11:22:33:44:55:66 00:00:20:00:00:01 192.168.2.2 1 vxlan imac-ivlan-tenid 1 3
	Add cloud filter with specified inner dst mac and tunnel id.
		testpmd>tunnel_filter add 0 11:22:33:44:55:66 00:00:20:00:00:01 192.168.2.2 1 vxlan imac-tenid 1 3
	Add cloud filter with specified inner dst mac.
		testpmd>tunnel_filter add 0 11:22:33:44:55:66 00:00:20:00:00:01 192.168.2.2 1 vxlan imac 1 3
	Add cloud filter with specified outer dst mac, inner dst mac and tunnel id.
		testpmd>tunnel_filter add 0 11:22:33:44:55:66 00:00:20:00:00:01 192.168.2.2 1 vxlan omac-imac-tenid 1 3
	Send vxlan packet matched the specified fields.
  Expected test result:
	testpmd can received the  vxlan packet on the assigned queue 3.

- Case: tunnel_filter_invalid
  Description: Check FVL vxlan tunnel  filter invalid command cannot accepted by testpmd 
  Command / instruction:
	Start testpmd with vxlan enabled and rss disabled.
		testpmd -c ffff -n 4 -- -i --tunnel-type=1 --disble-rss --rxq=4 --txq=4 --nb-cores=8 --nb-ports=2
	Enable VxLAN on both ports and UDP dport setting to 4789
		testpmd>rx_vxlan_port add 4789 0
		testpmd>rx_vxlan_port add 4
	Add Clould filter with invalid Mac address.
	Add Clould filter with invalid ip address.
	Add Clould filter with invalid vlan.
	Add Clould filter with invalid vni.
	Add Clould filter with invalid queue id.
  Expected test result: testpmd will report the parameter is invalid.

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jijiang Liu
> Sent: Tuesday, October 21, 2014 4:46 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v6 0/9] Support VxLAN on Fortville
> 
> The patch set supports VxLAN on Fortville based on latest rte_mbuf
> structure.
> 
> It includes:
>  - Support VxLAN packet identification by configuring UDP tunneling port.
>  - Support VxLAN packet filters. It uses MAC and VLAN to point
>    to a queue. The filter types supported are listed below:
>    1. Inner MAC and Inner VLAN ID
>    2. Inner MAC address, inner VLAN ID and tenant ID.
>    3. Inner MAC and tenant ID
>    4. Inner MAC address
>    5. Outer MAC address, tenant ID and inner MAC
>  - Support VxLAN TX checksum offload, which include outer L3(IP), inner L3(IP)
> and inner L4(UDP,TCP and SCTP)
> 
> Change notes:
> 
>  v6)  * Split the rte_mbuf structure changes as a seperate patch.
>       * Remove the initialization configuration of VXLAN UDP port.
>       * Change the filter_type field in rte_eth_tunnel_filter_conf to "uint16_t"
> type.
>       * Add more descriptions about some API comments and commit logs.
> 
> 
> Jijiang Liu (9):
>   rte_mbuf structure changes
>   add VxLAN packet identification API in librte_ether
>   support VxLAN packet identification in librte_pmd_i40e
>   test VxLAN packet identification in testpmd.
>   add data structures of tunneling filter in rte_eth_ctrl.h
>   implement the API of VxLAN packet filter in librte_pmd_i40e
>   test VxLAN packet filter
>   support VxLAN Tx checksum offload
>   test VxLAN Tx checksum offload
> 
>  app/test-pmd/cmdline.c            |  230 ++++++++++++++++++++++++-
>  app/test-pmd/config.c             |    6 +-
>  app/test-pmd/csumonly.c           |  195 +++++++++++++++++++--
>  app/test-pmd/parameters.c         |   13 ++
>  app/test-pmd/rxonly.c             |   49 ++++++
>  app/test-pmd/testpmd.c            |    8 +
>  app/test-pmd/testpmd.h            |    4 +
>  lib/librte_ether/rte_eth_ctrl.h   |   64 +++++++
>  lib/librte_ether/rte_ethdev.c     |   63 +++++++
>  lib/librte_ether/rte_ethdev.h     |   63 +++++++
>  lib/librte_ether/rte_ether.h      |    8 +
>  lib/librte_mbuf/rte_mbuf.h        |   26 +++-
>  lib/librte_pmd_i40e/i40e_ethdev.c |  341
> ++++++++++++++++++++++++++++++++++++-
>  lib/librte_pmd_i40e/i40e_ethdev.h |    8 +-
>  lib/librte_pmd_i40e/i40e_rxtx.c   |   55 ++++++-
>  15 files changed, 1101 insertions(+), 32 deletions(-)
> 
> --
> 1.7.7.6

Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of VxLAN filter

[Thomas Monjalon]

2014-10-21 16:46, Jijiang Liu:
> +#define RTE_TUNNEL_FILTER_TO_QUEUE 1 /**< point to an queue by filter type */

Sorry, I don't understand what is this value for?

> +#define RTE_TUNNEL_FILTER_IMAC_IVLAN (ETH_TUNNEL_FILTER_IMAC | \
> +					ETH_TUNNEL_FILTER_IVLAN)
> +#define RTE_TUNNEL_FILTER_IMAC_IVLAN_TENID (ETH_TUNNEL_FILTER_IMAC | \
> +					ETH_TUNNEL_FILTER_IVLAN | \
> +					ETH_TUNNEL_FILTER_TENID)
> +#define RTE_TUNNEL_FILTER_IMAC_TENID (ETH_TUNNEL_FILTER_IMAC | \
> +					ETH_TUNNEL_FILTER_TENID)
> +#define RTE_TUNNEL_FILTER_OMAC_TENID_IMAC (ETH_TUNNEL_FILTER_OMAC | \
> +					ETH_TUNNEL_FILTER_TENID | \
> +					ETH_TUNNEL_FILTER_IMAC)

I thought you agree that these definitions are useless?

-- 
Thomas

Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet identification API in librte_ether

[Thomas Monjalon]

2014-10-21 13:48, Liu, Jijiang:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-10-21 16:46, Jijiang Liu:
> > >  int
> > > +rte_eth_dev_udp_tunnel_add(uint8_t port_id,
> > > +			   struct rte_eth_udp_tunnel *udp_tunnel,
> > > +			   uint8_t count)
> > > +{
> > > +	uint8_t i;
> > > +	struct rte_eth_dev *dev;
> > > +	struct rte_eth_udp_tunnel *tunnel;
> > > +
> > > +	if (port_id >= nb_ports) {
> > > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > > +		return -ENODEV;
> > > +	}
> > > +
> > > +	if (udp_tunnel == NULL) {
> > > +		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
> > > +		return -EINVAL;
> > > +	}
> > > +	tunnel = udp_tunnel;
> > > +
> > > +	for (i = 0; i < count; i++, tunnel++) {
> > > +		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
> > > +			PMD_DEBUG_TRACE("Invalid tunnel type\n");
> > > +			return -EINVAL;
> > > +		}
> > > +	}
> > 
> > I'm not sure it's a good idea to provide a count parameter to iterate in a
> > loop.
> > It's probably something that the application should do by itself.
> 
> It is necessary to check if this prot_type(tunnel type) is valid here in case
> applications don't do that. 

Yes, you have to check prot_type but looping for several tunnels is not needed
at this level.

> > But I doubt we should configure a tunnel type for a whole port.
> 
> Yes, your understanding is correct. It is for a whole port/PF, that's why we
> should add tunnel_type in rte_eth_conf structure.

Please explain me why a tunnel type should be associated to a port.
This design looks really broken.

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet identification API in librte_ether

[Liu, Jijiang]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 22, 2014 5:19 AM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet
> identification API in librte_ether
> 
> 2014-10-21 13:48, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-21 16:46, Jijiang Liu:
> > > >  int
> > > > +rte_eth_dev_udp_tunnel_add(uint8_t port_id,
> > > > +			   struct rte_eth_udp_tunnel *udp_tunnel,
> > > > +			   uint8_t count)
> > > > +{
> > > > +	uint8_t i;
> > > > +	struct rte_eth_dev *dev;
> > > > +	struct rte_eth_udp_tunnel *tunnel;
> > > > +
> > > > +	if (port_id >= nb_ports) {
> > > > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > > > +		return -ENODEV;
> > > > +	}
> > > > +
> > > > +	if (udp_tunnel == NULL) {
> > > > +		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
> > > > +		return -EINVAL;
> > > > +	}
> > > > +	tunnel = udp_tunnel;
> > > > +
> > > > +	for (i = 0; i < count; i++, tunnel++) {
> > > > +		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
> > > > +			PMD_DEBUG_TRACE("Invalid tunnel type\n");
> > > > +			return -EINVAL;
> > > > +		}
> > > > +	}
> > >
> > > I'm not sure it's a good idea to provide a count parameter to
> > > iterate in a loop.
> > > It's probably something that the application should do by itself.
> >
> > It is necessary to check if this prot_type(tunnel type) is valid here
> > in case applications don't do that.
> 
> Yes, you have to check prot_type but looping for several tunnels is not
> needed at this level.
> 
> > > But I doubt we should configure a tunnel type for a whole port.
> >
> > Yes, your understanding is correct. It is for a whole port/PF, that's
> > why we should add tunnel_type in rte_eth_conf structure.
> 
> Please explain me why a tunnel type should be associated to a port.
> This design looks really broken.

I don't think this design looks really broken.

Currently, A PF  associated to a port, right? What tunnel type should be supported in a PF, which is required we configure it.
Tunneling packet is encapsulation packet, in terms of VxLAN, packet format is outer L2 header+ outer L3 header +outer L4 header + tunneling header+ inner L2 header + inner L3 header + inner L4 header +PAY4.
For a VM/VF, the  real useful packet data is "inner L2 header + inner L3 header + inner L4 header +PAY4".  

In NIC, A port/PF receive this kind of tunneling packet(outer L2+...PAY4),  software should be responsible for decapsulating the packet and deliver real data(innerL2 + PAY4) to VM/VF。

DPDK just provide API/mechanism to guarantee a PF/port to receive the tunneling packet data, the encapsulation/ decapsulation work should be done by user application.

Normally, the tunneling packet processing like below:
Tunneling packet ------>PF processing/receive ---------> application SW do decapsulation -------> VF/VM processing

> Thanks
> --
> Thomas

Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of VxLAN filter

[Liu, Jijiang]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, October 21, 2014 11:13 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of
> VxLAN filter
> 
> 2014-10-21 16:46, Jijiang Liu:
> > +#define RTE_TUNNEL_FILTER_TO_QUEUE 1 /**< point to an queue by filter
> type */
> 
> Sorry, I don't understand what is this value for?
> 
> > +#define RTE_TUNNEL_FILTER_IMAC_IVLAN (ETH_TUNNEL_FILTER_IMAC | \
> > +					ETH_TUNNEL_FILTER_IVLAN)
> > +#define RTE_TUNNEL_FILTER_IMAC_IVLAN_TENID
> (ETH_TUNNEL_FILTER_IMAC | \
> > +					ETH_TUNNEL_FILTER_IVLAN | \
> > +					ETH_TUNNEL_FILTER_TENID)
> > +#define RTE_TUNNEL_FILTER_IMAC_TENID (ETH_TUNNEL_FILTER_IMAC | \
> > +					ETH_TUNNEL_FILTER_TENID)
> > +#define RTE_TUNNEL_FILTER_OMAC_TENID_IMAC
> (ETH_TUNNEL_FILTER_OMAC | \
> > +					ETH_TUNNEL_FILTER_TENID | \
> > +					ETH_TUNNEL_FILTER_IMAC)
> 
> I thought you agree that these definitions are useless?
> 

Sorry, this MAY be  some misunderstanding, I don't think these definition are useless. I just thought change "uint16_t filter_type" is better than define "enum filter_type".

Let me explain here again.
The filter condition are: 
1.  inner MAC + inner VLAN
2. inner MAC + IVLAN + tenant ID
..
5. outer MAC + tenant ID + inner MAC

For each filter condition, we need to check if the mandatory parameters are valid by checking corresponding bit MASK.

An pseudo code example:

       Switch (filter_type)
       Case 1:  //inner MAC + inner VLAN
	If (filter_type & ETH_TUNNEL_FILTER_IMAC )
                            if   (IMAC==NULL)
                                      return -1;

       case 5: // outer MAC + tenant ID + inner MAC
            If (filter_type & ETH_TUNNEL_FILTER_IMAC )
                            if   (IMAC==NULL)
                                      return -1;
          
             If (filter_type & ETH_TUNNEL_FILTER_OMAC )
                            if   (IMAC==NULL)
                                      return -1;
   ......
 









> Thomas

Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet identification API in librte_ether

[Liu, Jijiang]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 22, 2014 5:19 AM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet
> identification API in librte_ether
> 
> 2014-10-21 13:48, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-21 16:46, Jijiang Liu:
> > > >  int
> > > > +rte_eth_dev_udp_tunnel_add(uint8_t port_id,
> > > > +			   struct rte_eth_udp_tunnel *udp_tunnel,
> > > > +			   uint8_t count)
> > > > +{
> > > > +	uint8_t i;
> > > > +	struct rte_eth_dev *dev;
> > > > +	struct rte_eth_udp_tunnel *tunnel;
> > > > +
> > > > +	if (port_id >= nb_ports) {
> > > > +		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
> > > > +		return -ENODEV;
> > > > +	}
> > > > +
> > > > +	if (udp_tunnel == NULL) {
> > > > +		PMD_DEBUG_TRACE("Invalid udp_tunnel parameter\n");
> > > > +		return -EINVAL;
> > > > +	}
> > > > +	tunnel = udp_tunnel;
> > > > +
> > > > +	for (i = 0; i < count; i++, tunnel++) {
> > > > +		if (tunnel->prot_type >= RTE_TUNNEL_TYPE_MAX) {
> > > > +			PMD_DEBUG_TRACE("Invalid tunnel type\n");
> > > > +			return -EINVAL;
> > > > +		}
> > > > +	}
> > >
> > > I'm not sure it's a good idea to provide a count parameter to
> > > iterate in a loop.
> > > It's probably something that the application should do by itself.
> >
> > It is necessary to check if this prot_type(tunnel type) is valid here
> > in case applications don't do that.
> 
> Yes, you have to check prot_type but looping for several tunnels is not
> needed at this level.

Ok, remove loop check from here.
 
> Thanks
> --
> Thomas

Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of VxLAN filter

[Liu, Jijiang]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, October 21, 2014 11:13 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of
> VxLAN filter
> 
> 2014-10-21 16:46, Jijiang Liu:
> > +#define RTE_TUNNEL_FILTER_TO_QUEUE 1 /**< point to an queue by filter
> type */
> 
> Sorry, I don't understand what is this value for?

This MACRO is used to indicate if user application hope to filter incoming packet(s) to a specific queue(multi-queue configuration is required) using filter type(such as inner MAC + tenant ID).
If the flag is not set, and all incoming packet will always go to queue 0.


> --
> Thomas

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Thomas Monjalon]

2014-10-21 14:14, Liu, Jijiang:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-10-21 16:46, Jijiang Liu:
> > > -	uint16_t reserved2;       /**< Unused field. Required for padding */
> > > +
> > > +	/**
> > > +	 * Packet type, which is used to indicate ordinary L2 packet format and
> > > +	 * also tunneled packet format such as IP in IP, IP in GRE, MAC in GRE
> > > +	 * and MAC in UDP.
> > > +	 */
> > > +	uint16_t packet_type;
> > 
> > Why not name it "l2_type"?
> 
> In datasheet, this term is called packet type(s).

That's exactly the point I want you really understand!
This is a field in generic mbuf structure, so your datasheet has no value here.

> Personally , I think packet type is  more clear what meaning of this field is . ^_^

You cannot add an API field without knowing what will be its generic meaning.
Please think about it and describe its scope.

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Liu, Jijiang]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 22, 2014 4:46 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure
> changes
> 
> 2014-10-21 14:14, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-21 16:46, Jijiang Liu:
> > > > -	uint16_t reserved2;       /**< Unused field. Required for padding */
> > > > +
> > > > +	/**
> > > > +	 * Packet type, which is used to indicate ordinary L2 packet format
> and
> > > > +	 * also tunneled packet format such as IP in IP, IP in GRE, MAC in GRE
> > > > +	 * and MAC in UDP.
> > > > +	 */
> > > > +	uint16_t packet_type;
> > >
> > > Why not name it "l2_type"?
> >
> > In datasheet, this term is called packet type(s).
> 
> That's exactly the point I want you really understand!
> This is a field in generic mbuf structure, so your datasheet has no value here.
> 
> > Personally , I think packet type is  more clear what meaning of this field is .
> ^_^
> 
> You cannot add an API field without knowing what will be its generic
> meaning.
> Please think about it and describe its scope.
 its generic meaning is that each UNIT  number stand for a kind of packet format。
I will add more description for this field.


> Thanks
> --
> Thomas

Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of VxLAN filter

[Thomas Monjalon]

2014-10-22 06:45, Liu, Jijiang:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-10-21 16:46, Jijiang Liu:
> > > +#define RTE_TUNNEL_FILTER_TO_QUEUE 1 /**< point to an queue by filter
> > type */
> > 
> > Sorry, I don't understand what is this value for?
> 
> This MACRO is used to indicate if user application hope to filter
> incoming packet(s) to a specific queue(multi-queue configuration
> is required) using filter type(such as inner MAC + tenant ID).
> If the flag is not set, and all incoming packet will always go
> to queue 0.

1) It's a boolean, so a new constant is not required.
2) You set the variable "to_queue" with this value but the variable is not used
3) Is there a sense to add a filter with this variable to 0?

I think "to_queue" variable and this constant are useless.

PS: it seems I'm the only person worried by the filtering API.
So maybe we shouldn't integrate it at all?
-- 
Thomas

Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of VxLAN filter

[Thomas Monjalon]

2014-10-22 02:25, Liu, Jijiang:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-10-21 16:46, Jijiang Liu:
> > > +#define RTE_TUNNEL_FILTER_IMAC_IVLAN (ETH_TUNNEL_FILTER_IMAC | \
> > > +					ETH_TUNNEL_FILTER_IVLAN)
> > > +#define RTE_TUNNEL_FILTER_IMAC_IVLAN_TENID (ETH_TUNNEL_FILTER_IMAC | \
> > > +					ETH_TUNNEL_FILTER_IVLAN | \
> > > +					ETH_TUNNEL_FILTER_TENID)
> > > +#define RTE_TUNNEL_FILTER_IMAC_TENID (ETH_TUNNEL_FILTER_IMAC | \
> > > +					ETH_TUNNEL_FILTER_TENID)
> > > +#define RTE_TUNNEL_FILTER_OMAC_TENID_IMAC (ETH_TUNNEL_FILTER_OMAC | \
> > > +					ETH_TUNNEL_FILTER_TENID | \
> > > +					ETH_TUNNEL_FILTER_IMAC)
> > 
> > I thought you agree that these definitions are useless?
> 
> Sorry, this MAY be  some misunderstanding, I don't think these definition
> are useless. I just thought change "uint16_t filter_type" is better than
> define "enum filter_type".
> 
> Let me explain here again.
> The filter condition are: 
> 1.  inner MAC + inner VLAN
> 2. inner MAC + IVLAN + tenant ID
> ..
> 5. outer MAC + tenant ID + inner MAC
> 
> For each filter condition, we need to check if the mandatory parameters are
> valid by checking corresponding bit MASK.

Checking bit mask doesn't imply to define all combinations of bit masks.
There's probably something obvious that one of us is missing.

> An pseudo code example:
> 
>        Switch (filter_type)
>        Case 1:  //inner MAC + inner VLAN
> 	If (filter_type & ETH_TUNNEL_FILTER_IMAC )
>                             if   (IMAC==NULL)
>                                       return -1;
> 
>        case 5: // outer MAC + tenant ID + inner MAC
>             If (filter_type & ETH_TUNNEL_FILTER_IMAC )
>                             if   (IMAC==NULL)
>                                       return -1;
>           
>              If (filter_type & ETH_TUNNEL_FILTER_OMAC )
>                             if   (IMAC==NULL)
>                                       return -1;
>    ......

Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet identification API in librte_ether

[Thomas Monjalon]

2014-10-22 01:46, Liu, Jijiang:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-10-21 13:48, Liu, Jijiang:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > But I doubt we should configure a tunnel type for a whole port.
> > >
> > > Yes, your understanding is correct. It is for a whole port/PF, that's
> > > why we should add tunnel_type in rte_eth_conf structure.
> > 
> > Please explain me why a tunnel type should be associated to a port.
> > This design looks really broken.
> 
> I don't think this design looks really broken.
> 
> Currently, A PF  associated to a port, right? What tunnel type should
> be supported in a PF, which is required we configure it.
> Tunneling packet is encapsulation packet, in terms of VxLAN, packet format
> is outer L2 header+ outer L3 header +outer L4 header + tunneling header+
> inner L2 header + inner L3 header + inner L4 header +PAY4.
> For a VM/VF, the  real useful packet data is "inner L2 header +
> inner L3 header + inner L4 header +PAY4".  
> 
> In NIC, A port/PF receive this kind of tunneling packet(outer L2+...PAY4),
> software should be responsible for decapsulating the packet and deliver
> real data(innerL2 + PAY4) to VM/VF。
> 
> DPDK just provide API/mechanism to guarantee a PF/port to receive the
> tunneling packet data, the encapsulation/ decapsulation work should be
> done by user application.

You mean that all packets received on the PF which doesn't match the configured
tunnel type, won't be received by the software?

Other question, why a port is associated with only one tunnel type?

> Normally, the tunneling packet processing like below:
> Tunneling packet ------>PF processing/receive ---------> application SW do decapsulation -------> VF/VM processing

I really try to understand what you have in mind. Thanks for explaining
-- 
Thomas

Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of VxLAN filter

[Liu, Jijiang]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 22, 2014 5:31 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of
> VxLAN filter
> 
> 2014-10-22 02:25, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-21 16:46, Jijiang Liu:
> > > > +#define RTE_TUNNEL_FILTER_IMAC_IVLAN (ETH_TUNNEL_FILTER_IMAC
> | \
> > > > +					ETH_TUNNEL_FILTER_IVLAN)
> > > > +#define RTE_TUNNEL_FILTER_IMAC_IVLAN_TENID
> (ETH_TUNNEL_FILTER_IMAC | \
> > > > +					ETH_TUNNEL_FILTER_IVLAN | \
> > > > +					ETH_TUNNEL_FILTER_TENID)
> > > > +#define RTE_TUNNEL_FILTER_IMAC_TENID (ETH_TUNNEL_FILTER_IMAC
> | \
> > > > +					ETH_TUNNEL_FILTER_TENID)
> > > > +#define RTE_TUNNEL_FILTER_OMAC_TENID_IMAC
> (ETH_TUNNEL_FILTER_OMAC | \
> > > > +					ETH_TUNNEL_FILTER_TENID | \
> > > > +					ETH_TUNNEL_FILTER_IMAC)
> > >
> > > I thought you agree that these definitions are useless?
> >
> > Sorry, this MAY be  some misunderstanding, I don't think these
> > definition are useless. I just thought change "uint16_t filter_type"
> > is better than define "enum filter_type".
> >
> > Let me explain here again.
> > The filter condition are:
> > 1.  inner MAC + inner VLAN
> > 2. inner MAC + IVLAN + tenant ID
> > ..
> > 5. outer MAC + tenant ID + inner MAC
> >
> > For each filter condition, we need to check if the mandatory
> > parameters are valid by checking corresponding bit MASK.
> 
> Checking bit mask doesn't imply to define all combinations of bit masks.
> There's probably something obvious that one of us is missing.

Anybody else have comments on this? 

> > An pseudo code example:
> >
> >        Switch (filter_type)
> >        Case 1:  //inner MAC + inner VLAN
> > 	If (filter_type & ETH_TUNNEL_FILTER_IMAC )
> >                             if   (IMAC==NULL)
> >                                       return -1;
> >
> >        case 5: // outer MAC + tenant ID + inner MAC
> >             If (filter_type & ETH_TUNNEL_FILTER_IMAC )
> >                             if   (IMAC==NULL)
> >                                       return -1;
> >
> >              If (filter_type & ETH_TUNNEL_FILTER_OMAC )
> >                             if   (IMAC==NULL)
> >                                       return -1;
> >    ......

Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet identification API in librte_ether

[Liu, Jijiang]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 22, 2014 5:52 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet
> identification API in librte_ether
> 
> 2014-10-22 01:46, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-21 13:48, Liu, Jijiang:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > But I doubt we should configure a tunnel type for a whole port.
> > > >
> > > > Yes, your understanding is correct. It is for a whole port/PF,
> > > > that's why we should add tunnel_type in rte_eth_conf structure.
> > >
> > > Please explain me why a tunnel type should be associated to a port.
> > > This design looks really broken.
> >
> > I don't think this design looks really broken.
> >
> > Currently, A PF  associated to a port, right? What tunnel type should
> > be supported in a PF, which is required we configure it.
> > Tunneling packet is encapsulation packet, in terms of VxLAN, packet
> > format is outer L2 header+ outer L3 header +outer L4 header +
> > tunneling header+ inner L2 header + inner L3 header + inner L4 header
> +PAY4.
> > For a VM/VF, the  real useful packet data is "inner L2 header + inner
> > L3 header + inner L4 header +PAY4".
> >
> > In NIC, A port/PF receive this kind of tunneling packet(outer
> > L2+...PAY4), software should be responsible for decapsulating the
> > packet and deliver real data(innerL2 + PAY4) to VM/VF。
> >
> > DPDK just provide API/mechanism to guarantee a PF/port to receive the
> > tunneling packet data, the encapsulation/ decapsulation work should be
> > done by user application.
> 
> You mean that all packets received on the PF which doesn't match the
> configured tunnel type, won't be received by the software?

No, it will be received by the software.
In terms of VxLAN packet, if tunnel type is not configured VXLAN type, and software can't use API to configure the UDP destination number. 
Even if the incoming packet format is VXLAN packet format, hardware and software don't think it is VXLAN packet because we didn't configure UDP Destination port. 

Now I want to remove this limitation,  even if the  tunnel type is not configured at PF initialization phase, user also can configure the VxLAN UDP destination number.
It is more flexible and reasonable.

> Other question, why a port is associated with only one tunnel type?

Good question. Now I think we had better remove this limitation because it is NIC related.

Two points are summarized here.
1. The tunnel types is for a whole port/PF, I have explained it a lots.
2. I will remove tunnel type configuration from rte_eth_conf structure.

Any comments?

> > Normally, the tunneling packet processing like below:
> > Tunneling packet ------>PF processing/receive ---------> application
> > SW do decapsulation -------> VF/VM processing
> 
> I really try to understand what you have in mind. Thanks for explaining
> --
> Thomas

Re: [dpdk-dev] [PATCH v6 2/9] librte_ether:add VxLAN packet identification API in librte_ether

[Thomas Monjalon]

2014-10-22 12:47, Liu, Jijiang:
> > > Currently, A PF  associated to a port, right? What tunnel type should
> > > be supported in a PF, which is required we configure it.
> > > Tunneling packet is encapsulation packet, in terms of VxLAN, packet
> > > format is outer L2 header+ outer L3 header +outer L4 header +
> > > tunneling header+ inner L2 header + inner L3 header + inner L4 header +PAY4.
> > > For a VM/VF, the  real useful packet data is "inner L2 header + inner
> > > L3 header + inner L4 header +PAY4".
> > >
> > > In NIC, A port/PF receive this kind of tunneling packet(outer
> > > L2+...PAY4), software should be responsible for decapsulating the
> > > packet and deliver real data(innerL2 + PAY4) to VM/VF。
> > >
> > > DPDK just provide API/mechanism to guarantee a PF/port to receive the
> > > tunneling packet data, the encapsulation/ decapsulation work should be
> > > done by user application.
> > 
> > You mean that all packets received on the PF which doesn't match the
> > configured tunnel type, won't be received by the software?
> 
> No, it will be received by the software.
> In terms of VxLAN packet, if tunnel type is not configured VXLAN type,
> and software can't use API to configure the UDP destination number. 
> Even if the incoming packet format is VXLAN packet format, hardware and
> software don't think it is VXLAN packet because we didn't configure UDP
> Destination port. 
> 
> Now I want to remove this limitation,  even if the  tunnel type is not
> configured at PF initialization phase, user also can configure the VxLAN
> UDP destination number.
> It is more flexible and reasonable.
> 
> > Other question, why a port is associated with only one tunnel type?
> 
> Good question. Now I think we had better remove this limitation because it is NIC related.
> 
> Two points are summarized here.
> 1. The tunnel types is for a whole port/PF, I have explained it a lots.
> 2. I will remove tunnel type configuration from rte_eth_conf structure.
> 
> Any comments?

Honestly, I haven't understood your explanation :)
I just understood that you want to remove tunnel type from rte_eth_conf
and I think it's a really good thing.

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of VxLAN filter

[Liu, Jijiang]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 22, 2014 5:25 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of
> VxLAN filter
> 
> 2014-10-22 06:45, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-21 16:46, Jijiang Liu:
> > > > +#define RTE_TUNNEL_FILTER_TO_QUEUE 1 /**< point to an queue by
> > > > +filter
> > > type */
> > >
> > > Sorry, I don't understand what is this value for?
> >
> > This MACRO is used to indicate if user application hope to filter
> > incoming packet(s) to a specific queue(multi-queue configuration is
> > required) using filter type(such as inner MAC + tenant ID).
> > If the flag is not set, and all incoming packet will always go to
> > queue 0.
> 
> 1) It's a boolean, so a new constant is not required.
OK. define bool to_queue. 
> 2) You set the variable "to_queue" with this value but the variable is not
> used
> 3) Is there a sense to add a filter with this variable to 0?
> 
> I think "to_queue" variable and this constant are useless.
No, It is useful. I will change to_queue to be configurable. 
> PS: it seems I'm the only person worried by the filtering API.
> So maybe we shouldn't integrate it at all?

I know you are joking. 
 We should certainly integrate the feature into dpdk.org
> --
> Thomas

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Zhang, Helin]

Hi

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Wednesday, October 22, 2014 4:46 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure
> changes
> 
> 2014-10-21 14:14, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-21 16:46, Jijiang Liu:
> > > > -	uint16_t reserved2;       /**< Unused field. Required for padding
> */
> > > > +
> > > > +	/**
> > > > +	 * Packet type, which is used to indicate ordinary L2 packet format
> and
> > > > +	 * also tunneled packet format such as IP in IP, IP in GRE, MAC in GRE
> > > > +	 * and MAC in UDP.
> > > > +	 */
> > > > +	uint16_t packet_type;
> > >
> > > Why not name it "l2_type"?
'packet_type' is for storing the hardware identified packet type upon different layers
of protocols (l2, l3, l4, ...).
It is quite useful for user application or middle layer software stacks, it can know
what the packet type is without checking the packet too much by software.
Actually ixgbe already has packet types (less than 10), which is transcoded into 'ol_flags'.
For i40e, the packet type can represent about 256 types of packet, 'ol_flags' does not
have enough bits for it anymore. So put the i40e packet types into mbuf would be better.
Also this field can be used for NON-Intel NICs, I think there must be the similar concepts
of other NICs. And 16 bits 'packet_type' has severl reserved bits for future and NON-Intel NICs.

> >
> > In datasheet, this term is called packet type(s).
> 
> That's exactly the point I want you really understand!
> This is a field in generic mbuf structure, so your datasheet has no value here.
> 
> > Personally , I think packet type is  more clear what meaning of this field is .
> ^_^
> 
> You cannot add an API field without knowing what will be its generic meaning.
> Please think about it and describe its scope.
> 
> Thanks
> --
> Thomas

Regards,
Helin

Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data structures of VxLAN filter

[Chilikin, Andrey]

For me these defines make perfect sense - tunnelling filters require combinations of different tunnel components, but not all combinations are valid. So defining valid combinations separately helps. 

Regards,
Andrey

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, October 22, 2014 5:31 PM
> To: Liu, Jijiang
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v6 5/9] librte_ether:add data 
> structures of VxLAN filter
> 
> 2014-10-22 02:25, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-21 16:46, Jijiang Liu:
> > > > +#define RTE_TUNNEL_FILTER_IMAC_IVLAN (ETH_TUNNEL_FILTER_IMAC
> | \
> > > > +					ETH_TUNNEL_FILTER_IVLAN)
> > > > +#define RTE_TUNNEL_FILTER_IMAC_IVLAN_TENID
> (ETH_TUNNEL_FILTER_IMAC | \
> > > > +					ETH_TUNNEL_FILTER_IVLAN | \
> > > > +					ETH_TUNNEL_FILTER_TENID)
> > > > +#define RTE_TUNNEL_FILTER_IMAC_TENID (ETH_TUNNEL_FILTER_IMAC
> | \
> > > > +					ETH_TUNNEL_FILTER_TENID)
> > > > +#define RTE_TUNNEL_FILTER_OMAC_TENID_IMAC
> (ETH_TUNNEL_FILTER_OMAC | \
> > > > +					ETH_TUNNEL_FILTER_TENID | \
> > > > +					ETH_TUNNEL_FILTER_IMAC)
> > >
> > > I thought you agree that these definitions are useless?
> >
> > Sorry, this MAY be  some misunderstanding, I don't think these 
> > definition are useless. I just thought change "uint16_t filter_type"
> > is better than define "enum filter_type".
> >
> > Let me explain here again.
> > The filter condition are:
> > 1.  inner MAC + inner VLAN
> > 2. inner MAC + IVLAN + tenant ID
> > ..
> > 5. outer MAC + tenant ID + inner MAC
> >
> > For each filter condition, we need to check if the mandatory 
> > parameters are valid by checking corresponding bit MASK.
> 
> Checking bit mask doesn't imply to define all combinations of bit masks.
> There's probably something obvious that one of us is missing.

Anybody else have comments on this? 

> > An pseudo code example:
> >
> >        Switch (filter_type)
> >        Case 1:  //inner MAC + inner VLAN
> > 	If (filter_type & ETH_TUNNEL_FILTER_IMAC )
> >                             if   (IMAC==NULL)
> >                                       return -1;
> >
> >        case 5: // outer MAC + tenant ID + inner MAC
> >             If (filter_type & ETH_TUNNEL_FILTER_IMAC )
> >                             if   (IMAC==NULL)
> >                                       return -1;
> >
> >              If (filter_type & ETH_TUNNEL_FILTER_OMAC )
> >                             if   (IMAC==NULL)
> >                                       return -1;
> >    ......

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Thomas Monjalon]

Hi guys,

We still have some problems with the mbuf changes introduced for VXLAN.
I want to raise the packet type issue here.

2014-10-23 02:23, Zhang, Helin:
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > 2014-10-21 14:14, Liu, Jijiang:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > 2014-10-21 16:46, Jijiang Liu:
> > > > > -	uint16_t reserved2;       /**< Unused field. Required for padding */
> > > > > +
> > > > > +	/**
> > > > > +	 * Packet type, which is used to indicate ordinary L2 packet format and
> > > > > +	 * also tunneled packet format such as IP in IP, IP in GRE, MAC in GRE
> > > > > +	 * and MAC in UDP.
> > > > > +	 */
> > > > > +	uint16_t packet_type;
> > > >
> > > > Why not name it "l2_type"?
> 
> 'packet_type' is for storing the hardware identified packet type upon different layers
> of protocols (l2, l3, l4, ...).
> It is quite useful for user application or middle layer software stacks, it can know
> what the packet type is without checking the packet too much by software.
> Actually ixgbe already has packet types (less than 10), which is transcoded into 'ol_flags'.
> For i40e, the packet type can represent about 256 types of packet, 'ol_flags' does not
> have enough bits for it anymore. So put the i40e packet types into mbuf would be better.
> Also this field can be used for NON-Intel NICs, I think there must be the similar concepts
> of other NICs. And 16 bits 'packet_type' has severl reserved bits for future and NON-Intel NICs.

Thanks Helin, that's the best description of packet_type I've seen so far.
It's not so clear in the commit log:
	http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf

> > > In datasheet, this term is called packet type(s).
> > 
> > That's exactly the point I want you really understand!
> > This is a field in generic mbuf structure, so your datasheet has no value here.
> > 
> > > Personally , I think packet type is  more clear what meaning of this field is .
> > 
> > You cannot add an API field without knowing what will be its generic meaning.
> > Please think about it and describe its scope.

I integrated this patch with the VXLAN patchset in the hope that you'll
improve the situation afterwards.
This is the answer you recently gave to Olivier:
	http://dpdk.org/ml/archives/dev/2014-November/007599.html
"
	Regarding adding a packet_type in mbuf, we ever had a lot of discussions as follows:
	http://dpdk.org/ml/archives/dev/2014-October/007027.html
	http://dpdk.org/ml/archives/dev/2014-September/005240.html
	http://dpdk.org/ml/archives/dev/2014-September/005241.html
	http://dpdk.org/ml/archives/dev/2014-September/005274.html
"

To sum up the situation:
- We don't know what are the possible values of packet_type
- It's only filled by i40e, while other drivers use ol_flags
- There is no special value "unknown" which should be set by drivers
  not supporting this feature.
- Its only usage is to print a decimal value in app/test-pmd/rxonly.c

It's now clear that nobody cares about this part of the API.
So I'm going to remove packet_type from mbuf.
I don't want to keep something that we don't know how to use, that is
not consistent across drivers, and that overlap another API part (ol_flags).

-- 
Thomas

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Zhang, Helin]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, November 12, 2014 9:26 PM
> To: Liu, Jijiang
> Cc: Zhang, Helin; dev@dpdk.org; Richardson, Bruce
> Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure
> changes
> 
> Hi guys,
> 
> We still have some problems with the mbuf changes introduced for VXLAN.
> I want to raise the packet type issue here.
> 
> 2014-10-23 02:23, Zhang, Helin:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > > 2014-10-21 14:14, Liu, Jijiang:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > 2014-10-21 16:46, Jijiang Liu:
> > > > > > -	uint16_t reserved2;       /**< Unused field. Required for padding */
> > > > > > +
> > > > > > +	/**
> > > > > > +	 * Packet type, which is used to indicate ordinary L2 packet
> format and
> > > > > > +	 * also tunneled packet format such as IP in IP, IP in GRE, MAC in
> GRE
> > > > > > +	 * and MAC in UDP.
> > > > > > +	 */
> > > > > > +	uint16_t packet_type;
> > > > >
> > > > > Why not name it "l2_type"?
> >
> > 'packet_type' is for storing the hardware identified packet type upon
> > different layers of protocols (l2, l3, l4, ...).
> > It is quite useful for user application or middle layer software
> > stacks, it can know what the packet type is without checking the packet too
> much by software.
> > Actually ixgbe already has packet types (less than 10), which is transcoded into
> 'ol_flags'.
> > For i40e, the packet type can represent about 256 types of packet,
> > 'ol_flags' does not have enough bits for it anymore. So put the i40e packet
> types into mbuf would be better.
> > Also this field can be used for NON-Intel NICs, I think there must be
> > the similar concepts of other NICs. And 16 bits 'packet_type' has severl
> reserved bits for future and NON-Intel NICs.
> 
> Thanks Helin, that's the best description of packet_type I've seen so far.
> It's not so clear in the commit log:
> 	http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf
> 
> > > > In datasheet, this term is called packet type(s).
> > >
> > > That's exactly the point I want you really understand!
> > > This is a field in generic mbuf structure, so your datasheet has no value here.
> > >
> > > > Personally , I think packet type is  more clear what meaning of this field is .
> > >
> > > You cannot add an API field without knowing what will be its generic meaning.
> > > Please think about it and describe its scope.
> 
> I integrated this patch with the VXLAN patchset in the hope that you'll improve
> the situation afterwards.
> This is the answer you recently gave to Olivier:
> 	http://dpdk.org/ml/archives/dev/2014-November/007599.html
> "
> 	Regarding adding a packet_type in mbuf, we ever had a lot of discussions as
> follows:
> 	http://dpdk.org/ml/archives/dev/2014-October/007027.html
> 	http://dpdk.org/ml/archives/dev/2014-September/005240.html
> 	http://dpdk.org/ml/archives/dev/2014-September/005241.html
> 	http://dpdk.org/ml/archives/dev/2014-September/005274.html
> "
> 
> To sum up the situation:
> - We don't know what are the possible values of packet_type
> - It's only filled by i40e, while other drivers use ol_flags
> - There is no special value "unknown" which should be set by drivers
>   not supporting this feature.
> - Its only usage is to print a decimal value in app/test-pmd/rxonly.c
Though I haven't investigate this too much, my opinion is that we should
use packet_type in the future, and rework igb/ixgbe PMD to remove all
packet types in ol_flags and use packet_type instead.
Then example app can use the packet type directly. And all igb, ixgbe and
i40e packet_type are consistent. Sure we might need to define all packet
types in rte_ethdev.h or similar header files.

> 
> It's now clear that nobody cares about this part of the API.
> So I'm going to remove packet_type from mbuf.
> I don't want to keep something that we don't know how to use, that is not
> consistent across drivers, and that overlap another API part (ol_flags).
> 
> --
> Thomas

Regards,
Helin

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Thomas Monjalon]

2014-11-12 14:31, Zhang, Helin:
> > 2014-10-23 02:23, Zhang, Helin:
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > > > 2014-10-21 14:14, Liu, Jijiang:
> > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > 2014-10-21 16:46, Jijiang Liu:
> > > > > > > +	uint16_t packet_type;
> > > > > >
> > > > > > Why not name it "l2_type"?
> > >
> > > 'packet_type' is for storing the hardware identified packet type upon
> > > different layers of protocols (l2, l3, l4, ...).
> > > It is quite useful for user application or middle layer software
> > > stacks, it can know what the packet type is without checking the packet too
> > much by software.
> > > Actually ixgbe already has packet types (less than 10), which is transcoded into
> > 'ol_flags'.
> > > For i40e, the packet type can represent about 256 types of packet,
> > > 'ol_flags' does not have enough bits for it anymore. So put the i40e packet
> > types into mbuf would be better.
> > > Also this field can be used for NON-Intel NICs, I think there must be
> > > the similar concepts of other NICs. And 16 bits 'packet_type' has severl
> > reserved bits for future and NON-Intel NICs.
> > 
> > Thanks Helin, that's the best description of packet_type I've seen so far.
> > It's not so clear in the commit log:
> > 	http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf
> > 
> > > > > In datasheet, this term is called packet type(s).
> > > >
> > > > That's exactly the point I want you really understand!
> > > > This is a field in generic mbuf structure, so your datasheet has no value here.
> > > >
> > > > > Personally , I think packet type is  more clear what meaning of this field is .
> > > >
> > > > You cannot add an API field without knowing what will be its generic meaning.
> > > > Please think about it and describe its scope.
> > 
> > I integrated this patch with the VXLAN patchset in the hope that you'll improve
> > the situation afterwards.
> > This is the answer you recently gave to Olivier:
> > 	http://dpdk.org/ml/archives/dev/2014-November/007599.html
> > "
> > 	Regarding adding a packet_type in mbuf, we ever had a lot of discussions as
> > follows:
> > 	http://dpdk.org/ml/archives/dev/2014-October/007027.html
> > 	http://dpdk.org/ml/archives/dev/2014-September/005240.html
> > 	http://dpdk.org/ml/archives/dev/2014-September/005241.html
> > 	http://dpdk.org/ml/archives/dev/2014-September/005274.html
> > "
> > 
> > To sum up the situation:
> > - We don't know what are the possible values of packet_type
> > - It's only filled by i40e, while other drivers use ol_flags
> > - There is no special value "unknown" which should be set by drivers
> >   not supporting this feature.
> > - Its only usage is to print a decimal value in app/test-pmd/rxonly.c
> 
> Though I haven't investigate this too much, my opinion is that we should
> use packet_type in the future, and rework igb/ixgbe PMD to remove all
> packet types in ol_flags and use packet_type instead.
> Then example app can use the packet type directly. And all igb, ixgbe and
> i40e packet_type are consistent. Sure we might need to define all packet
> types in rte_ethdev.h or similar header files.

Exact!

> > It's now clear that nobody cares about this part of the API.
> > So I'm going to remove packet_type from mbuf.
> > I don't want to keep something that we don't know how to use, that is not
> > consistent across drivers, and that overlap another API part (ol_flags).

Helin, I feel you perfectly understood the problem.
As the responsible of i40e, you can make a choice for 1.8 release:
- remove (incomplete) packet_type
- or complete it quickly

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Liu, Jijiang]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, November 12, 2014 9:26 PM
> To: Liu, Jijiang
> Cc: Zhang, Helin; dev@dpdk.org; Richardson, Bruce
> Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure
> changes
> 
> Hi guys,
> 
> We still have some problems with the mbuf changes introduced for VXLAN.
> I want to raise the packet type issue here.
> 
> 2014-10-23 02:23, Zhang, Helin:
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > > 2014-10-21 14:14, Liu, Jijiang:
> > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > 2014-10-21 16:46, Jijiang Liu:
> > > > > > -	uint16_t reserved2;       /**< Unused field. Required for padding
> */
> > > > > > +
> > > > > > +	/**
> > > > > > +	 * Packet type, which is used to indicate ordinary L2 packet
> format and
> > > > > > +	 * also tunneled packet format such as IP in IP, IP in GRE, MAC in
> GRE
> > > > > > +	 * and MAC in UDP.
> > > > > > +	 */
> > > > > > +	uint16_t packet_type;
> > > > >
> > > > > Why not name it "l2_type"?
> >
> > 'packet_type' is for storing the hardware identified packet type upon
> > different layers of protocols (l2, l3, l4, ...).
> > It is quite useful for user application or middle layer software
> > stacks, it can know what the packet type is without checking the packet too
> much by software.
> > Actually ixgbe already has packet types (less than 10), which is transcoded into
> 'ol_flags'.
> > For i40e, the packet type can represent about 256 types of packet,
> > 'ol_flags' does not have enough bits for it anymore. So put the i40e packet types
> into mbuf would be better.
> > Also this field can be used for NON-Intel NICs, I think there must be
> > the similar concepts of other NICs. And 16 bits 'packet_type' has severl
> reserved bits for future and NON-Intel NICs.
> 
> Thanks Helin, that's the best description of packet_type I've seen so far.
> It's not so clear in the commit log:
> 	http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf
> 
> > > > In datasheet, this term is called packet type(s).
> > >
> > > That's exactly the point I want you really understand!
> > > This is a field in generic mbuf structure, so your datasheet has no value here.
> > >
> > > > Personally , I think packet type is  more clear what meaning of this field is .
> > >
> > > You cannot add an API field without knowing what will be its generic meaning.
> > > Please think about it and describe its scope.
> 
> I integrated this patch with the VXLAN patchset in the hope that you'll improve
> the situation afterwards.
> This is the answer you recently gave to Olivier:
> 	http://dpdk.org/ml/archives/dev/2014-November/007599.html
> "
> 	Regarding adding a packet_type in mbuf, we ever had a lot of discussions
> as follows:
> 	http://dpdk.org/ml/archives/dev/2014-October/007027.html
> 	http://dpdk.org/ml/archives/dev/2014-September/005240.html
> 	http://dpdk.org/ml/archives/dev/2014-September/005241.html
> 	http://dpdk.org/ml/archives/dev/2014-September/005274.html
> "
> 
> To sum up the situation:
> - We don't know what are the possible values of packet_type
> - It's only filled by i40e, while other drivers use ol_flags
> - There is no special value "unknown" which should be set by drivers
>   not supporting this feature.
> - Its only usage is to print a decimal value in app/test-pmd/rxonly.c
> 
> It's now clear that nobody cares about this part of the API.
> So I'm going to remove packet_type from mbuf.
> I don't want to keep something that we don't know how to use, that is not
> consistent across drivers, and that overlap another API part (ol_flags).

The packet type in 40e is very important for user, using packet type can help to speed up packet analysis/identification in their application, especially tunneling packet format.
Now I'm working on implementing packet type definition in rte_ethdev.h file and  translation table in i40e, which is almost done. 
The packet type  definition in in rte_ethdev.h file like below. 
/*
 * Ethernet packet type
 */
enum rte_eth_ptype {
        /* undefined packet type, means HW can't recognise it */
        RTE_PTYPE_UNDEF = 0,
...

        /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv4 */
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4FRAG_PAY3,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_PAY3,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_UDP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_TCP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_SCTP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_ICMP_PAY4,
 
        /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv6 */
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6FRAG_PAY3
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_PAY3,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_UDP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_TCP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_SCTP_PAY4,
        RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_ICMP_PAY4,
 
        /* IPv4 --> GRE/Teredo/VXLAN --> MAC/VLAN */
        RTE_PTYPE_IPv4_GRENAT_MACVLAN_PAY3,
... 
}

Yes, we don't use packet type in many places now, which doesn't mean we don't use it  in the future( when supporting another tunneling packet).

It is ok for me if you want to remove the packet_type filed in mbuf,  but we will send a separate patch set for introducing packet type in the future, which includes 1g/10/40g PMD changes.

> --
> Thomas

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Thomas Monjalon]

2014-11-13 03:17, Liu, Jijiang:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-10-23 02:23, Zhang, Helin:
> > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > > > 2014-10-21 14:14, Liu, Jijiang:
> > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > 2014-10-21 16:46, Jijiang Liu:
> > > > > > > +	uint16_t packet_type;
> > > > > >
> > > > > > Why not name it "l2_type"?
> > >
> > > 'packet_type' is for storing the hardware identified packet type upon
> > > different layers of protocols (l2, l3, l4, ...).
> > > It is quite useful for user application or middle layer software
> > > stacks, it can know what the packet type is without checking the packet too
> > much by software.
> > > Actually ixgbe already has packet types (less than 10), which is transcoded into
> > 'ol_flags'.
> > > For i40e, the packet type can represent about 256 types of packet,
> > > 'ol_flags' does not have enough bits for it anymore. So put the i40e packet types
> > into mbuf would be better.
> > > Also this field can be used for NON-Intel NICs, I think there must be
> > > the similar concepts of other NICs. And 16 bits 'packet_type' has severl
> > reserved bits for future and NON-Intel NICs.
> > 
> > Thanks Helin, that's the best description of packet_type I've seen so far.
> > It's not so clear in the commit log:
> > 	http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf
> > 
> > > > > In datasheet, this term is called packet type(s).
> > > >
> > > > That's exactly the point I want you really understand!
> > > > This is a field in generic mbuf structure, so your datasheet has no value here.
> > > >
> > > > > Personally , I think packet type is  more clear what meaning of this field is .
> > > >
> > > > You cannot add an API field without knowing what will be its generic meaning.
> > > > Please think about it and describe its scope.
> > 
> > I integrated this patch with the VXLAN patchset in the hope that you'll improve
> > the situation afterwards.
> > This is the answer you recently gave to Olivier:
> > 	http://dpdk.org/ml/archives/dev/2014-November/007599.html
> > "
> > 	Regarding adding a packet_type in mbuf, we ever had a lot of discussions
> > as follows:
> > 	http://dpdk.org/ml/archives/dev/2014-October/007027.html
> > 	http://dpdk.org/ml/archives/dev/2014-September/005240.html
> > 	http://dpdk.org/ml/archives/dev/2014-September/005241.html
> > 	http://dpdk.org/ml/archives/dev/2014-September/005274.html
> > "
> > 
> > To sum up the situation:
> > - We don't know what are the possible values of packet_type
> > - It's only filled by i40e, while other drivers use ol_flags
> > - There is no special value "unknown" which should be set by drivers
> >   not supporting this feature.
> > - Its only usage is to print a decimal value in app/test-pmd/rxonly.c
> > 
> > It's now clear that nobody cares about this part of the API.
> > So I'm going to remove packet_type from mbuf.
> > I don't want to keep something that we don't know how to use, that is not
> > consistent across drivers, and that overlap another API part (ol_flags).
> 
> The packet type in 40e is very important for user, using packet type can
> help to speed up packet analysis/identification in their application,
> especially tunneling packet format.
> Now I'm working on implementing packet type definition in rte_ethdev.h
> file and  translation table in i40e, which is almost done. 
> The packet type  definition in in rte_ethdev.h file like below. 
> /*
>  * Ethernet packet type
>  */
> enum rte_eth_ptype {
>         /* undefined packet type, means HW can't recognise it */
>         RTE_PTYPE_UNDEF = 0,
> ...
> 
>         /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv4 */
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4FRAG_PAY3,
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_PAY3,
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_UDP_PAY4,
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_TCP_PAY4,
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_SCTP_PAY4,
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_ICMP_PAY4,
>  
>         /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv6 */
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6FRAG_PAY3
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_PAY3,
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_UDP_PAY4,
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_TCP_PAY4,
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_SCTP_PAY4,
>         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_ICMP_PAY4,
>  
>         /* IPv4 --> GRE/Teredo/VXLAN --> MAC/VLAN */
>         RTE_PTYPE_IPv4_GRENAT_MACVLAN_PAY3,
> ... 
> }

OK, it seems well abstracted.
I think the last part of these names (PAY3/PAY4) is useless.

When this patch for API and i40e will be ready?
I'd prefer fixing the API instead of removing it.

> Yes, we don't use packet type in many places now, which doesn't mean
> we don't use it  in the future (when supporting another tunneling packet).
> 
> It is ok for me if you want to remove the packet_type filed in mbuf,
> but we will send a separate patch set for introducing packet type in
> the future, which includes 1g/10/40g PMD changes.

When the patches for igb/ixgbe will be ready?

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Liu, Jijiang]

Hi, 

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, November 13, 2014 4:53 PM
> To: Liu, Jijiang
> Cc: Zhang, Helin; dev@dpdk.org; Richardson, Bruce
> Subject: Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure
> changes
> 
> 2014-11-13 03:17, Liu, Jijiang:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-10-23 02:23, Zhang, Helin:
> > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas
> > > > Monjalon
> > > > > 2014-10-21 14:14, Liu, Jijiang:
> > > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > > 2014-10-21 16:46, Jijiang Liu:
> > > > > > > > +	uint16_t packet_type;
> > > > > > >
> > > > > > > Why not name it "l2_type"?
> > > >
> > > > 'packet_type' is for storing the hardware identified packet type
> > > > upon different layers of protocols (l2, l3, l4, ...).
> > > > It is quite useful for user application or middle layer software
> > > > stacks, it can know what the packet type is without checking the
> > > > packet too
> > > much by software.
> > > > Actually ixgbe already has packet types (less than 10), which is
> > > > transcoded into
> > > 'ol_flags'.
> > > > For i40e, the packet type can represent about 256 types of packet,
> > > > 'ol_flags' does not have enough bits for it anymore. So put the
> > > > i40e packet types
> > > into mbuf would be better.
> > > > Also this field can be used for NON-Intel NICs, I think there must
> > > > be the similar concepts of other NICs. And 16 bits 'packet_type'
> > > > has severl
> > > reserved bits for future and NON-Intel NICs.
> > >
> > > Thanks Helin, that's the best description of packet_type I've seen so far.
> > > It's not so clear in the commit log:
> > > 	http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf
> > >
> > > > > > In datasheet, this term is called packet type(s).
> > > > >
> > > > > That's exactly the point I want you really understand!
> > > > > This is a field in generic mbuf structure, so your datasheet has no value
> here.
> > > > >
> > > > > > Personally , I think packet type is  more clear what meaning of this field
> is .
> > > > >
> > > > > You cannot add an API field without knowing what will be its generic
> meaning.
> > > > > Please think about it and describe its scope.
> > >
> > > I integrated this patch with the VXLAN patchset in the hope that
> > > you'll improve the situation afterwards.
> > > This is the answer you recently gave to Olivier:
> > > 	http://dpdk.org/ml/archives/dev/2014-November/007599.html
> > > "
> > > 	Regarding adding a packet_type in mbuf, we ever had a lot of
> > > discussions as follows:
> > > 	http://dpdk.org/ml/archives/dev/2014-October/007027.html
> > > 	http://dpdk.org/ml/archives/dev/2014-September/005240.html
> > > 	http://dpdk.org/ml/archives/dev/2014-September/005241.html
> > > 	http://dpdk.org/ml/archives/dev/2014-September/005274.html
> > > "
> > >
> > > To sum up the situation:
> > > - We don't know what are the possible values of packet_type
> > > - It's only filled by i40e, while other drivers use ol_flags
> > > - There is no special value "unknown" which should be set by drivers
> > >   not supporting this feature.
> > > - Its only usage is to print a decimal value in
> > > app/test-pmd/rxonly.c
> > >
> > > It's now clear that nobody cares about this part of the API.
> > > So I'm going to remove packet_type from mbuf.
> > > I don't want to keep something that we don't know how to use, that
> > > is not consistent across drivers, and that overlap another API part (ol_flags).
> >
> > The packet type in 40e is very important for user, using packet type
> > can help to speed up packet analysis/identification in their
> > application, especially tunneling packet format.
> > Now I'm working on implementing packet type definition in rte_ethdev.h
> > file and  translation table in i40e, which is almost done.
> > The packet type  definition in in rte_ethdev.h file like below.
> > /*
> >  * Ethernet packet type
> >  */
> > enum rte_eth_ptype {
> >         /* undefined packet type, means HW can't recognise it */
> >         RTE_PTYPE_UNDEF = 0,
> > ...
> >
> >         /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv4 */
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4FRAG_PAY3,
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_PAY3,
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_UDP_PAY4,
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_TCP_PAY4,
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_SCTP_PAY4,
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_ICMP_PAY4,
> >
> >         /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv6 */
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6FRAG_PAY3
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_PAY3,
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_UDP_PAY4,
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_TCP_PAY4,
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_SCTP_PAY4,
> >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_ICMP_PAY4,
> >
> >         /* IPv4 --> GRE/Teredo/VXLAN --> MAC/VLAN */
> >         RTE_PTYPE_IPv4_GRENAT_MACVLAN_PAY3,
> > ...
> > }
> 
> OK, it seems well abstracted.
> I think the last part of these names (PAY3/PAY4) is useless.
> 
> When this patch for API and i40e will be ready?
> I'd prefer fixing the API instead of removing it.

If needed, next week, I can send a patch for this.

> > Yes, we don't use packet type in many places now, which doesn't mean
> > we don't use it  in the future (when supporting another tunneling packet).
> >
> > It is ok for me if you want to remove the packet_type filed in mbuf,
> > but we will send a separate patch set for introducing packet type in
> > the future, which includes 1g/10/40g PMD changes.
> 
> When the patches for igb/ixgbe will be ready?
We need some time to investigate this for igb/ixgbe, probably some example codes and test application codes need to changed. 
You can assume that it cannot be done in DPDK1.8.

So here are my three suggestions:

1. keep packet_type in mbuf and wait for all the igb/ixgb/i40e changes done in DPDK2.0. Now, I don't send a separate  patch set for it. 
2. keep  packet_type in mbuf, I just send i40e patch set for this in DPDK1.8. In DPDK2.0, we will send a patch set  for igb/ixgbe.
3. It can be removed now,  and we will send a separate patch set for introducing packet type in the future.

> Thanks
> --
> Thomas

Re: [dpdk-dev] [PATCH v6 1/9] librte_mbuf:the rte_mbuf structure changes

[Thomas Monjalon]

2014-11-13 11:24, Liu, Jijiang:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-11-13 03:17, Liu, Jijiang:
> > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > 2014-10-23 02:23, Zhang, Helin:
> > > > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas
> > > > > Monjalon
> > > > > > 2014-10-21 14:14, Liu, Jijiang:
> > > > > > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > > > > > > 2014-10-21 16:46, Jijiang Liu:
> > > > > > > > > +	uint16_t packet_type;
> > > > > > > >
> > > > > > > > Why not name it "l2_type"?
> > > > >
> > > > > 'packet_type' is for storing the hardware identified packet type
> > > > > upon different layers of protocols (l2, l3, l4, ...).
> > > > > It is quite useful for user application or middle layer software
> > > > > stacks, it can know what the packet type is without checking the
> > > > > packet too
> > > > much by software.
> > > > > Actually ixgbe already has packet types (less than 10), which is
> > > > > transcoded into
> > > > 'ol_flags'.
> > > > > For i40e, the packet type can represent about 256 types of packet,
> > > > > 'ol_flags' does not have enough bits for it anymore. So put the
> > > > > i40e packet types
> > > > into mbuf would be better.
> > > > > Also this field can be used for NON-Intel NICs, I think there must
> > > > > be the similar concepts of other NICs. And 16 bits 'packet_type'
> > > > > has severl
> > > > reserved bits for future and NON-Intel NICs.
> > > >
> > > > Thanks Helin, that's the best description of packet_type I've seen so far.
> > > > It's not so clear in the commit log:
> > > > 	http://dpdk.org/browse/dpdk/commit/?id=73b7d59cf4f6faf
> > > >
> > > > > > > In datasheet, this term is called packet type(s).
> > > > > >
> > > > > > That's exactly the point I want you really understand!
> > > > > > This is a field in generic mbuf structure, so your datasheet has no value here.
> > > > > >
> > > > > > > Personally , I think packet type is  more clear what meaning of this field is .
> > > > > >
> > > > > > You cannot add an API field without knowing what will be its generic meaning.
> > > > > > Please think about it and describe its scope.
> > > >
> > > > I integrated this patch with the VXLAN patchset in the hope that
> > > > you'll improve the situation afterwards.
> > > > This is the answer you recently gave to Olivier:
> > > > 	http://dpdk.org/ml/archives/dev/2014-November/007599.html
> > > > "
> > > > 	Regarding adding a packet_type in mbuf, we ever had a lot of
> > > > discussions as follows:
> > > > 	http://dpdk.org/ml/archives/dev/2014-October/007027.html
> > > > 	http://dpdk.org/ml/archives/dev/2014-September/005240.html
> > > > 	http://dpdk.org/ml/archives/dev/2014-September/005241.html
> > > > 	http://dpdk.org/ml/archives/dev/2014-September/005274.html
> > > > "
> > > >
> > > > To sum up the situation:
> > > > - We don't know what are the possible values of packet_type
> > > > - It's only filled by i40e, while other drivers use ol_flags
> > > > - There is no special value "unknown" which should be set by drivers
> > > >   not supporting this feature.
> > > > - Its only usage is to print a decimal value in
> > > > app/test-pmd/rxonly.c
> > > >
> > > > It's now clear that nobody cares about this part of the API.
> > > > So I'm going to remove packet_type from mbuf.
> > > > I don't want to keep something that we don't know how to use, that
> > > > is not consistent across drivers, and that overlap another API part (ol_flags).
> > >
> > > The packet type in 40e is very important for user, using packet type
> > > can help to speed up packet analysis/identification in their
> > > application, especially tunneling packet format.
> > > Now I'm working on implementing packet type definition in rte_ethdev.h
> > > file and  translation table in i40e, which is almost done.
> > > The packet type  definition in in rte_ethdev.h file like below.
> > > /*
> > >  * Ethernet packet type
> > >  */
> > > enum rte_eth_ptype {
> > >         /* undefined packet type, means HW can't recognise it */
> > >         RTE_PTYPE_UNDEF = 0,
> > > ...
> > >
> > >         /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv4 */
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4FRAG_PAY3,
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_PAY3,
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_UDP_PAY4,
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_TCP_PAY4,
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_SCTP_PAY4,
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv4_ICMP_PAY4,
> > >
> > >         /* IPv4 --> GRE/Teredo/VXLAN --> MAC --> IPv6 */
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6FRAG_PAY3
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_PAY3,
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_UDP_PAY4,
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_TCP_PAY4,
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_SCTP_PAY4,
> > >         RTE_PTYPE_IPv4_GRENAT_MAC_IPv6_ICMP_PAY4,
> > >
> > >         /* IPv4 --> GRE/Teredo/VXLAN --> MAC/VLAN */
> > >         RTE_PTYPE_IPv4_GRENAT_MACVLAN_PAY3,
> > > ...
> > > }
> > 
> > OK, it seems well abstracted.
> > I think the last part of these names (PAY3/PAY4) is useless.
> > 
> > When this patch for API and i40e will be ready?
> > I'd prefer fixing the API instead of removing it.
> 
> If needed, next week, I can send a patch for this.
> 
> > > Yes, we don't use packet type in many places now, which doesn't mean
> > > we don't use it  in the future (when supporting another tunneling packet).
> > >
> > > It is ok for me if you want to remove the packet_type filed in mbuf,
> > > but we will send a separate patch set for introducing packet type in
> > > the future, which includes 1g/10/40g PMD changes.
> > 
> > When the patches for igb/ixgbe will be ready?
> 
> We need some time to investigate this for igb/ixgbe, probably some
> example codes and test application codes need to changed. 
> You can assume that it cannot be done in DPDK1.8.
> 
> So here are my three suggestions:
> 
> 1. keep packet_type in mbuf and wait for all the igb/ixgb/i40e changes
> done in DPDK2.0. Now, I don't send a separate  patch set for it. 
> 2. keep  packet_type in mbuf, I just send i40e patch set for this in
> DPDK1.8. In DPDK2.0, we will send a patch set  for igb/ixgbe.
> 3. It can be removed now,  and we will send a separate patch set for
> introducing packet type in the future.

Option 2 please :)
My main concerns are:
- clearly document it
- have hardware abstraction

Thanks
-- 
Thomas