[dpdk-dev] [PATCH v2 0/7] Support flow director programming on fortville

[dpdk-dev] [PATCH v2 0/7] Support flow director programming on fortville

[Jingjing Wu]

The patch set supports flow director programming on fortville.
It includes:
 - reserve i40e resources for flow director, such as queue and vsi.
 - support the new ethdev API rx_classification_filter_ctl for all
   the configuration or queries for receive classification filters.
 - support programming 6 flow types for the flow director filters,
   which is called PCTYPE in fortville: ipv4, tcpv4, udpv4, ipv6,
   tcpv6, udpv6.
 - support flushing flow director table (all filters).
 - support to get flow diretor information.
 - support match statistics and FD ID report.
 - fix the the Marco conflict between rte_ip.h and netinet/in.h.
 
v2 changes:
 - create real fdir vsi and assign queue 0 pair to it.
 - check filter status report on the rx queue 0
 
further plan:
 - add flexible payload comprasion as flow director's input
 - support sctpv4 and sctpv6 PCTYPEs

jingjing.wu (7):
  flow director resource reserve and initialize on i40e
  define new ethdev API rx_classification_filter_ctl
  function implement in i40e for flow director filter programming
  function implement in i40e for flow director flush and info get
  fix the Marco conflict
  support FD ID report and match counter for i40e flow director
  add commands and config functions for i40e flow director support

 app/test-pmd/cmdline.c              | 665 ++++++++++++++++++++++++++++++++++++
 app/test-pmd/config.c               |  54 ++-
 app/test-pmd/testpmd.c              |  22 ++
 app/test-pmd/testpmd.h              |  57 ++++
 lib/librte_ether/Makefile           |   3 +-
 lib/librte_ether/rte_eth_features.h |  65 ++++
 lib/librte_ether/rte_ethdev.c       |  19 +-
 lib/librte_ether/rte_ethdev.h       | 108 +++---
 lib/librte_net/rte_ip.h             |   5 +-
 lib/librte_pmd_i40e/Makefile        |   5 +
 lib/librte_pmd_i40e/i40e_ethdev.c   | 137 +++++++-
 lib/librte_pmd_i40e/i40e_ethdev.h   |  32 +-
 lib/librte_pmd_i40e/i40e_fdir.c     | 500 +++++++++++++++++++++++++++
 lib/librte_pmd_i40e/i40e_rxtx.c     | 176 +++++++++-
 lib/librte_pmd_i40e/rte_i40e.h      | 125 +++++++
 15 files changed, 1906 insertions(+), 67 deletions(-)
 create mode 100644 lib/librte_ether/rte_eth_features.h
 create mode 100644 lib/librte_pmd_i40e/i40e_fdir.c
 create mode 100644 lib/librte_pmd_i40e/rte_i40e.h

-- 
1.8.1.4

[dpdk-dev] [PATCH v2 1/7] i40e: flow director resource reserve and initialize on i40e

[Jingjing Wu]

flow director resource reserve and initialize on i40e, it includes
 - queue initialization and switch on and vsi creation during setup
 - queue vsi for flow director release during close
 
Signed-off-by: jingjing.wu <jingjing.wu@intel.com>
Reviewed-by: Helin Zhang <helin.zhang@intel.com>
Reviewed-by: Jing Chen <jing.d.chen@intel.com>
Reviewed-by: Jijiang Liu <jijiang.liu@intel.com>

---
 lib/librte_pmd_i40e/Makefile      |   1 +
 lib/librte_pmd_i40e/i40e_ethdev.c |  81 ++++++++++++---
 lib/librte_pmd_i40e/i40e_ethdev.h |  22 +++-
 lib/librte_pmd_i40e/i40e_fdir.c   | 208 ++++++++++++++++++++++++++++++++++++++
 lib/librte_pmd_i40e/i40e_rxtx.c   | 127 +++++++++++++++++++++++
 5 files changed, 426 insertions(+), 13 deletions(-)
 create mode 100644 lib/librte_pmd_i40e/i40e_fdir.c

diff --git a/lib/librte_pmd_i40e/Makefile b/lib/librte_pmd_i40e/Makefile
index 4b31675..6537654 100644
--- a/lib/librte_pmd_i40e/Makefile
+++ b/lib/librte_pmd_i40e/Makefile
@@ -87,6 +87,7 @@ SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_ethdev.c
 SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_rxtx.c
 SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_ethdev_vf.c
 SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_pf.c
+SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_fdir.c
 # this lib depends upon:
 DEPDIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += lib/librte_eal lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += lib/librte_mempool lib/librte_mbuf
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 4e65ca4..a08b43c 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -523,7 +523,8 @@ eth_i40e_dev_init(__rte_unused struct eth_driver *eth_drv,
 	return 0;
 
 err_setup_pf_switch:
-	rte_free(pf->main_vsi);
+	i40e_fdir_teardown(pf);
+	i40e_vsi_release(pf->main_vsi);
 err_get_mac_addr:
 err_configure_lan_hmc:
 	(void)i40e_shutdown_lan_hmc(hw);
@@ -780,6 +781,12 @@ i40e_dev_start(struct rte_eth_dev *dev)
 	i40e_vsi_queues_bind_intr(vsi);
 	i40e_vsi_enable_queues_intr(vsi);
 
+	/* enable FDIR MSIX interrupt */
+	if (pf->flags & I40E_FLAG_FDIR) {
+		i40e_vsi_queues_bind_intr(pf->fdir.fdir_vsi);
+		i40e_vsi_enable_queues_intr(pf->fdir.fdir_vsi);
+	}
+
 	/* Enable all queues which have been configured */
 	ret = i40e_vsi_switch_queues(vsi, TRUE);
 	if (ret != I40E_SUCCESS) {
@@ -845,6 +852,7 @@ i40e_dev_close(struct rte_eth_dev *dev)
 	i40e_shutdown_lan_hmc(hw);
 
 	/* release all the existing VSIs and VEBs */
+	i40e_fdir_teardown(pf);
 	i40e_vsi_release(pf->main_vsi);
 
 	/* shutdown the adminq */
@@ -2584,16 +2592,30 @@ i40e_vsi_setup(struct i40e_pf *pf,
 	case I40E_VSI_SRIOV :
 		vsi->nb_qps = pf->vf_nb_qps;
 		break;
+	case I40E_VSI_FDIR:
+		vsi->nb_qps = pf->fdir_nb_qps;
+		break;
 	default:
 		goto fail_mem;
 	}
-	ret = i40e_res_pool_alloc(&pf->qp_pool, vsi->nb_qps);
-	if (ret < 0) {
-		PMD_DRV_LOG(ERR, "VSI %d allocate queue failed %d",
-				vsi->seid, ret);
-		goto fail_mem;
-	}
-	vsi->base_queue = ret;
+	/*
+	 * The filter status descriptor is reported in rx queue 0,
+	 * while the tx queue for fdir filter programming has no
+	 * such constraints, can be non-zero queues.
+	 * To simplify it, choose FDIR vsi use queue 0 pair.
+	 * To make sure it will use queue 0 pair, queue allocation
+	 * need be done before this function is called
+	 */
+	if (type != I40E_VSI_FDIR) {
+		ret = i40e_res_pool_alloc(&pf->qp_pool, vsi->nb_qps);
+			if (ret < 0) {
+				PMD_DRV_LOG(ERR, "VSI %d allocate queue failed %d",
+						vsi->seid, ret);
+				goto fail_mem;
+			}
+			vsi->base_queue = ret;
+	} else
+		vsi->base_queue = I40E_FDIR_QUEUE_ID;
 
 	/* VF has MSIX interrupt in VF range, don't allocate here */
 	if (type != I40E_VSI_SRIOV) {
@@ -2725,8 +2747,24 @@ i40e_vsi_setup(struct i40e_pf *pf,
 		 * Since VSI is not created yet, only configure parameter,
 		 * will add vsi below.
 		 */
-	}
-	else {
+	} else if (type == I40E_VSI_FDIR) {
+		vsi->uplink_seid = uplink_vsi->uplink_seid;
+		ctxt.pf_num = hw->pf_id;
+		ctxt.vf_num = 0;
+		ctxt.uplink_seid = vsi->uplink_seid;
+		ctxt.connection_type = 0x1;     /* regular data port */
+		ctxt.flags = I40E_AQ_VSI_TYPE_PF;
+		ret = i40e_vsi_config_tc_queue_mapping(vsi, &ctxt.info,
+						I40E_DEFAULT_TCMAP);
+		if (ret != I40E_SUCCESS) {
+			PMD_DRV_LOG(ERR, "Failed to configure "
+					"TC queue mapping\n");
+			goto fail_msix_alloc;
+		}
+		ctxt.info.up_enable_bits = I40E_DEFAULT_TCMAP;
+		ctxt.info.valid_sections |=
+			rte_cpu_to_le_16(I40E_AQ_VSI_PROP_SCHED_VALID);
+	} else {
 		PMD_DRV_LOG(ERR, "VSI: Not support other type VSI yet\n");
 		goto fail_msix_alloc;
 	}
@@ -2912,8 +2950,16 @@ i40e_pf_setup(struct i40e_pf *pf)
 		PMD_DRV_LOG(ERR, "Could not get switch config, err %d", ret);
 		return ret;
 	}
-
-	/* VSI setup */
+	if (pf->flags & I40E_FLAG_FDIR) {
+		/* make queue allocated first, let FDIR use queue pair 0*/
+		ret = i40e_res_pool_alloc(&pf->qp_pool, I40E_DEFAULT_QP_NUM_FDIR);
+		if (ret != I40E_FDIR_QUEUE_ID) {
+			PMD_DRV_LOG(ERR, "queue allocation fails for FDIR :"
+				    " ret =%d", ret);
+			pf->flags &= ~I40E_FLAG_FDIR;
+		}
+	}
+	/*  main VSI setup */
 	vsi = i40e_vsi_setup(pf, I40E_VSI_MAIN, NULL, 0);
 	if (!vsi) {
 		PMD_DRV_LOG(ERR, "Setup of main vsi failed");
@@ -2923,9 +2969,20 @@ i40e_pf_setup(struct i40e_pf *pf)
 	dev_data->nb_rx_queues = vsi->nb_qps;
 	dev_data->nb_tx_queues = vsi->nb_qps;
 
+	/* setup FDIR after main vsi created.*/
+	if (pf->flags & I40E_FLAG_FDIR) {
+		ret = i40e_fdir_setup(pf);
+		if (ret != I40E_SUCCESS) {
+			PMD_DRV_LOG(ERR, "Failed to setup flow director\n");
+			pf->flags &= ~I40E_FLAG_FDIR;
+		}
+	}
+
 	/* Configure filter control */
 	memset(&settings, 0, sizeof(settings));
 	settings.hash_lut_size = I40E_HASH_LUT_SIZE_128;
+	if (pf->flags & I40E_FLAG_FDIR)
+		settings.enable_fdir = TRUE;
 	/* Enable ethtype and macvlan filters */
 	settings.enable_ethtype = TRUE;
 	settings.enable_macvlan = TRUE;
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.h b/lib/librte_pmd_i40e/i40e_ethdev.h
index 64deef2..c2c7fa9 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.h
+++ b/lib/librte_pmd_i40e/i40e_ethdev.h
@@ -46,11 +46,12 @@
 /* number of VSIs and queue default setting */
 #define I40E_MAX_QP_NUM_PER_VF    16
 #define I40E_DEFAULT_QP_NUM_VMDQ  64
-#define I40E_DEFAULT_QP_NUM_FDIR  64
+#define I40E_DEFAULT_QP_NUM_FDIR  1
 #define I40E_UINT32_BIT_SIZE      (CHAR_BIT * sizeof(uint32_t))
 #define I40E_VFTA_SIZE            (4096 / I40E_UINT32_BIT_SIZE)
 /* Default TC traffic in case DCB is not enabled */
 #define I40E_DEFAULT_TCMAP        0x1
+#define I40E_FDIR_QUEUE_ID        0
 
 /* i40e flags */
 #define I40E_FLAG_RSS                   (1ULL << 0)
@@ -189,6 +190,16 @@ struct i40e_pf_vf {
 };
 
 /*
+ *  A structure used to define fields of a FDIR related info.
+ */
+struct i40e_fdir_info {
+	struct i40e_vsi *fdir_vsi;     /* pointer to fdir VSI structure */
+	uint16_t match_counter_index;  /* Statistic counter index used for fdir*/
+	struct i40e_tx_queue *txq;
+	struct i40e_rx_queue *rxq;
+};
+
+/*
  * Structure to store private data specific for PF instance.
  */
 struct i40e_pf {
@@ -216,6 +227,7 @@ 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 */
+	struct i40e_fdir_info fdir; /* flow director info */
 };
 
 enum pending_msg {
@@ -312,6 +324,14 @@ void i40e_vsi_queues_unbind_intr(struct i40e_vsi *vsi);
 int i40e_vsi_vlan_pvid_set(struct i40e_vsi *vsi,
 				struct i40e_vsi_vlan_pvid_info *info);
 int i40e_vsi_config_vlan_stripping(struct i40e_vsi *vsi, bool on);
+enum i40e_status_code
+i40e_fdir_setup_tx_resources(struct i40e_pf *pf,
+				    unsigned int socket_id);
+enum i40e_status_code
+i40e_fdir_setup_rx_resources(struct i40e_pf *pf,
+				    unsigned int socket_id);
+int i40e_fdir_setup(struct i40e_pf *pf);
+void i40e_fdir_teardown(struct i40e_pf *pf);
 
 /* I40E_DEV_PRIVATE_TO */
 #define I40E_DEV_PRIVATE_TO_PF(adapter) \
diff --git a/lib/librte_pmd_i40e/i40e_fdir.c b/lib/librte_pmd_i40e/i40e_fdir.c
new file mode 100644
index 0000000..f6297a8
--- /dev/null
+++ b/lib/librte_pmd_i40e/i40e_fdir.c
@@ -0,0 +1,208 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <sys/queue.h>
+#include <stdio.h>
+#include <errno.h>
+#include <stdint.h>
+#include <string.h>
+#include <unistd.h>
+#include <stdarg.h>
+
+#include <rte_ether.h>
+#include <rte_ethdev.h>
+#include <rte_log.h>
+#include <rte_mbuf.h>
+
+#include "i40e_logs.h"
+#include "i40e/i40e_type.h"
+#include "i40e_ethdev.h"
+#include "i40e_rxtx.h"
+
+#define I40E_COUNTER_PF           2
+/* Statistic counter index for one pf */
+#define I40E_COUNTER_INDEX_FDIR(pf_id)   (0 + (pf_id) * I40E_COUNTER_PF)
+
+static int
+i40e_fdir_rx_queue_init(struct i40e_rx_queue *rxq)
+{
+	struct i40e_hw *hw = I40E_VSI_TO_HW(rxq->vsi);
+	struct i40e_hmc_obj_rxq rx_ctx;
+	int err = I40E_SUCCESS;
+
+	memset(&rx_ctx, 0, sizeof(struct i40e_hmc_obj_rxq));
+	/* Init the RX queue in hardware */
+	rx_ctx.dbuff = I40E_RXBUF_SZ_1024 >> I40E_RXQ_CTX_DBUFF_SHIFT;
+	rx_ctx.hbuff = 0;
+	rx_ctx.base = rxq->rx_ring_phys_addr / I40E_QUEUE_BASE_ADDR_UNIT;
+	rx_ctx.qlen = rxq->nb_rx_desc;
+#ifndef RTE_LIBRTE_I40E_16BYTE_RX_DESC
+	rx_ctx.dsize = 1;
+#endif
+	rx_ctx.dtype = i40e_header_split_none;
+	rx_ctx.hsplit_0 = I40E_HEADER_SPLIT_NONE;
+	rx_ctx.rxmax = ETHER_MAX_LEN;
+	rx_ctx.tphrdesc_ena = 1;
+	rx_ctx.tphwdesc_ena = 1;
+	rx_ctx.tphdata_ena = 1;
+	rx_ctx.tphhead_ena = 1;
+	rx_ctx.lrxqthresh = 2;
+	rx_ctx.crcstrip = 0;
+	rx_ctx.l2tsel = 1;
+	rx_ctx.showiv = 1;
+	rx_ctx.prefena = 1;
+
+	err = i40e_clear_lan_rx_queue_context(hw, rxq->reg_idx);
+	if (err != I40E_SUCCESS) {
+		PMD_DRV_LOG(ERR, "Failed to clear FDIR RX queue context\n");
+		return err;
+	}
+	err = i40e_set_lan_rx_queue_context(hw, rxq->reg_idx, &rx_ctx);
+	if (err != I40E_SUCCESS) {
+		PMD_DRV_LOG(ERR, "Failed to set FDIR RX queue context\n");
+		return err;
+	}
+	rxq->qrx_tail = hw->hw_addr +
+		I40E_QRX_TAIL(rxq->vsi->base_queue);
+
+	rte_wmb();
+	/* Init the RX tail regieter. */
+	I40E_PCI_REG_WRITE(rxq->qrx_tail, 0);
+	I40E_PCI_REG_WRITE(rxq->qrx_tail, rxq->nb_rx_desc - 1);
+
+	return err;
+}
+
+/*
+ * i40e_fdir_setup - reserve and initialize the Flow Director resources
+ * @pf: board private structure
+ **/
+int
+i40e_fdir_setup(struct i40e_pf *pf)
+{
+	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
+	struct i40e_vsi *vsi;
+	int err = I40E_SUCCESS;
+
+	PMD_DRV_LOG(INFO, "FDIR HW Capabilities: num_filters_guaranteed = %u,"
+			" num_filters_best_effort = %u.\n",
+			hw->func_caps.fd_filters_guaranteed,
+			hw->func_caps.fd_filters_best_effort);
+
+	vsi = pf->fdir.fdir_vsi;
+	if (vsi) {
+		PMD_DRV_LOG(ERR, "FDIR vsi pointer needs"
+				 "to be null before creation\n");
+		return I40E_ERR_BAD_PTR;
+	}
+	/* make new FDIR VSI */
+	vsi = i40e_vsi_setup(pf, I40E_VSI_FDIR, pf->main_vsi, 0);
+	if (!vsi) {
+		PMD_DRV_LOG(ERR, "Couldn't create FDIR VSI\n");
+		return I40E_ERR_NO_AVAILABLE_VSI;
+	}
+	pf->fdir.fdir_vsi = vsi;
+
+	/*Fdir tx queue setup*/
+	err = i40e_fdir_setup_tx_resources(pf, 0);
+	if (err) {
+		PMD_DRV_LOG(ERR, "Failed to setup FDIR TX resources\n");
+		goto fail_setup_tx;
+	}
+
+	/*Fdir rx queue setup*/
+	err = i40e_fdir_setup_rx_resources(pf, 0);
+	if (err) {
+		PMD_DRV_LOG(ERR, "Failed to setup FDIR RX resources\n");
+		goto fail_setup_rx;
+	}
+
+	err = i40e_tx_queue_init(pf->fdir.txq);
+	if (err) {
+		PMD_DRV_LOG(ERR, "Failed to do FDIR TX initialization\n");
+		goto fail_mem;
+	}
+
+	/* need switch on before dev start*/
+	err = i40e_switch_tx_queue(hw, vsi->base_queue, TRUE);
+	if (err) {
+		PMD_DRV_LOG(ERR, "Failed to do fdir TX switch on\n");
+		goto fail_mem;
+	}
+
+	/* Init the rx queue in hardware */
+	err = i40e_fdir_rx_queue_init(pf->fdir.rxq);
+	if (err) {
+		PMD_DRV_LOG(ERR, "Failed to do FDIR RX initialization\n");
+		goto fail_mem;
+	}
+
+	/* switch on rx queue */
+	err = i40e_switch_rx_queue(hw, vsi->base_queue, TRUE);
+	if (err) {
+		PMD_DRV_LOG(ERR, "Failed to do FDIR RX switch on\n");
+		goto fail_mem;
+	}
+
+	pf->fdir.match_counter_index = I40E_COUNTER_INDEX_FDIR(hw->pf_id);
+	PMD_DRV_LOG(INFO, "FDIR setup successfully, with programming queue %u.\n",
+		    vsi->base_queue);
+	return I40E_SUCCESS;
+
+fail_mem:
+	i40e_dev_rx_queue_release(pf->fdir.rxq);
+fail_setup_rx:
+	i40e_dev_tx_queue_release(pf->fdir.txq);
+fail_setup_tx:
+	i40e_vsi_release(vsi);
+	return err;
+}
+
+/*
+ * i40e_fdir_teardown - release the Flow Director resources
+ * @pf: board private structure
+ */
+void
+i40e_fdir_teardown(struct i40e_pf *pf)
+{
+	struct i40e_vsi *vsi;
+
+	vsi = pf->fdir.fdir_vsi;
+	i40e_dev_rx_queue_release(pf->fdir.rxq);
+	pf->fdir.rxq = NULL;
+	i40e_dev_tx_queue_release(pf->fdir.txq);
+	pf->fdir.txq = NULL;
+	i40e_vsi_release(vsi);
+	pf->fdir.fdir_vsi = NULL;
+	return;
+}
diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index f153844..8bfbc8c 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -2089,6 +2089,8 @@ i40e_tx_queue_init(struct i40e_tx_queue *txq)
 	tx_ctx.base = txq->tx_ring_phys_addr / I40E_QUEUE_BASE_ADDR_UNIT;
 	tx_ctx.qlen = txq->nb_tx_desc;
 	tx_ctx.rdylist = rte_le_to_cpu_16(vsi->info.qs_handle[0]);
+	if (vsi->type == I40E_VSI_FDIR)
+		tx_ctx.fd_ena = TRUE;
 
 	err = i40e_clear_lan_tx_queue_context(hw, pf_q);
 	if (err != I40E_SUCCESS) {
@@ -2306,3 +2308,128 @@ i40e_dev_clear_queues(struct rte_eth_dev *dev)
 		i40e_reset_rx_queue(dev->data->rx_queues[i]);
 	}
 }
+
+enum i40e_status_code
+i40e_fdir_setup_tx_resources(struct i40e_pf *pf,
+				    unsigned int socket_id)
+{
+	struct i40e_tx_queue *txq;
+	const struct rte_memzone *tz = NULL;
+	uint32_t ring_size;
+	struct rte_eth_dev *dev = pf->adapter->eth_dev;
+
+#define I40E_FDIR_NUM_TX_DESC  I40E_MIN_RING_DESC
+	if (!pf) {
+		PMD_DRV_LOG(ERR, "PF is not available");
+		return I40E_ERR_BAD_PTR;
+	}
+
+	/* Allocate the TX queue data structure. */
+	txq = rte_zmalloc_socket("i40e fdir tx queue",
+				  sizeof(struct i40e_tx_queue),
+				  CACHE_LINE_SIZE,
+				  socket_id);
+	if (!txq) {
+		PMD_DRV_LOG(ERR, "Failed to allocate memory for "
+					"tx queue structure\n");
+		return I40E_ERR_NO_MEMORY;
+	}
+
+	/* Allocate TX hardware ring descriptors. */
+	ring_size = sizeof(struct i40e_tx_desc) * I40E_FDIR_NUM_TX_DESC;
+	ring_size = RTE_ALIGN(ring_size, I40E_DMA_MEM_ALIGN);
+
+	tz = i40e_ring_dma_zone_reserve(dev,
+					"fdir_tx_ring",
+					I40E_FDIR_QUEUE_ID,
+					ring_size,
+					socket_id);
+	if (!tz) {
+		i40e_dev_tx_queue_release(txq);
+		PMD_DRV_LOG(ERR, "Failed to reserve DMA memory for TX\n");
+		return I40E_ERR_NO_MEMORY;
+	}
+
+	txq->nb_tx_desc = I40E_FDIR_NUM_TX_DESC;
+	txq->queue_id = I40E_FDIR_QUEUE_ID;
+	txq->reg_idx = pf->fdir.fdir_vsi->base_queue;
+	txq->vsi = pf->fdir.fdir_vsi;
+
+#ifdef RTE_LIBRTE_XEN_DOM0
+	txq->tx_ring_phys_addr = rte_mem_phy2mch(tz->memseg_id, tz->phys_addr);
+#else
+	txq->tx_ring_phys_addr = (uint64_t)tz->phys_addr;
+#endif
+	txq->tx_ring = (struct i40e_tx_desc *)tz->addr;
+	/*
+	 * don't need to allocate software ring and reset for the fdir
+	 * program queue just set the queue has been configured.
+	 */
+	txq->q_set = TRUE;
+	pf->fdir.txq = txq;
+
+	return I40E_SUCCESS;
+}
+
+enum i40e_status_code
+i40e_fdir_setup_rx_resources(struct i40e_pf *pf,
+				    unsigned int socket_id)
+{
+	struct i40e_rx_queue *rxq;
+	const struct rte_memzone *rz = NULL;
+	uint32_t ring_size;
+	struct rte_eth_dev *dev = pf->adapter->eth_dev;
+
+#define I40E_FDIR_NUM_RX_DESC  I40E_MIN_RING_DESC
+	if (!pf) {
+		PMD_DRV_LOG(ERR, "PF is not available");
+		return I40E_ERR_BAD_PTR;
+	}
+
+	/* Allocate the TX queue data structure. */
+	rxq = rte_zmalloc_socket("i40e fdir rx queue",
+				  sizeof(struct i40e_rx_queue),
+				  CACHE_LINE_SIZE,
+				  socket_id);
+	if (!rxq) {
+		PMD_DRV_LOG(ERR, "Failed to allocate memory for "
+					"rx queue structure\n");
+		return I40E_ERR_NO_MEMORY;
+	}
+
+	/* Allocate TX hardware ring descriptors. */
+	ring_size = sizeof(union i40e_rx_desc) * I40E_FDIR_NUM_RX_DESC;
+	ring_size = RTE_ALIGN(ring_size, I40E_DMA_MEM_ALIGN);
+
+	rz = i40e_ring_dma_zone_reserve(dev,
+					"fdir_rx_ring",
+					I40E_FDIR_QUEUE_ID,
+					ring_size,
+					socket_id);
+	if (!rz) {
+		i40e_dev_rx_queue_release(rxq);
+		PMD_DRV_LOG(ERR, "Failed to reserve DMA memory for RX\n");
+		return I40E_ERR_NO_MEMORY;
+	}
+
+	rxq->nb_rx_desc = I40E_FDIR_NUM_RX_DESC;
+	rxq->queue_id = I40E_FDIR_QUEUE_ID;
+	rxq->reg_idx = pf->fdir.fdir_vsi->base_queue;
+	rxq->vsi = pf->fdir.fdir_vsi;
+
+#ifdef RTE_LIBRTE_XEN_DOM0
+	rxq->rx_ring_phys_addr = rte_mem_phy2mch(rz->memseg_id, rz->phys_addr);
+#else
+	rxq->rx_ring_phys_addr = (uint64_t)rz->phys_addr;
+#endif
+	rxq->rx_ring = (union i40e_rx_desc *)rz->addr;
+
+	/*
+	 * Don't need to allocate software ring and reset for the fdir
+	 * rx queue, just set the queue has been configured.
+	 */
+	rxq->q_set = TRUE;
+	pf->fdir.rxq = rxq;
+
+	return I40E_SUCCESS;
+}
-- 
1.8.1.4

Re: [dpdk-dev] [PATCH v2 1/7] i40e: flow director resource reserve and initialize on i40e

[Thomas Monjalon]

Hi Jingjing,

I don't understand the title. What is a "flow director resource"?

2014-08-27 10:13, Jingjing Wu:
> flow director resource reserve and initialize on i40e, it includes
>  - queue initialization and switch on and vsi creation during setup
>  - queue vsi for flow director release during close

If you have 2 separated items, it probably means it should be 2 patches.

-- 
Thomas

Re: [dpdk-dev] [PATCH v2 1/7] i40e: flow director resource reserve and initialize on i40e

[Wu, Jingjing]

Hi, Thomas

The "flow director resource" means we need to reserve a specific queue and VSI on HW to support. The queue and vsi is used for programming flow director filter, not common tx/rx queues.

2 separated items, one is about setup, another is for release. I will try to separate them.


> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, August 27, 2014 10:18 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 1/7] i40e: flow director resource reserve and initialize on
> i40e
> 
> Hi Jingjing,
> 
> I don't understand the title. What is a "flow director resource"?
> 
> 2014-08-27 10:13, Jingjing Wu:
> > flow director resource reserve and initialize on i40e, it includes
> >  - queue initialization and switch on and vsi creation during setup
> >  - queue vsi for flow director release during close
> 
> If you have 2 separated items, it probably means it should be 2 patches.
> 
> --
> Thomas

[dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl

[Jingjing Wu]

support a new ethdev API rx_classification_filter_ctl for all
the configuration or queries for receive classification filters.
this patch supports commands the API used below:
  RTE_CMD_FDIR_RULE_ADD
  RTE_CMD_FDIR_RULE_DEL
  RTE_CMD_FDIR_FLUSH
  RTE_CMD_FDIR_INFO_GET
 
Signed-off-by: jingjing.wu <jingjing.wu@intel.com>
Reviewed-by: Helin Zhang <helin.zhang@intel.com>
Reviewed-by: Jing Chen <jing.d.chen@intel.com>
Reviewed-by: Jijiang Liu <jijiang.liu@intel.com>
 
---
 lib/librte_ether/Makefile           |   3 +-
 lib/librte_ether/rte_eth_features.h |  65 ++++++++++++++++++++++
 lib/librte_ether/rte_ethdev.c       |  19 ++++++-
 lib/librte_ether/rte_ethdev.h       | 108 +++++++++++++++++++++++-------------
 4 files changed, 155 insertions(+), 40 deletions(-)
 create mode 100644 lib/librte_ether/rte_eth_features.h

diff --git a/lib/librte_ether/Makefile b/lib/librte_ether/Makefile
index b310f8b..03dec8a 100644
--- a/lib/librte_ether/Makefile
+++ b/lib/librte_ether/Makefile
@@ -46,8 +46,9 @@ SRCS-y += rte_ethdev.c
 #
 SYMLINK-y-include += rte_ether.h
 SYMLINK-y-include += rte_ethdev.h
+SYMLINK-y-include += rte_eth_features.h
 
 # this lib depends upon:
-DEPDIRS-y += lib/librte_eal lib/librte_mempool lib/librte_ring lib/librte_mbuf
+DEPDIRS-y += lib/librte_eal lib/librte_mempool lib/librte_ring lib/librte_mbuf lib/librte_net
 
 include $(RTE_SDK)/mk/rte.lib.mk
diff --git a/lib/librte_ether/rte_eth_features.h b/lib/librte_ether/rte_eth_features.h
new file mode 100644
index 0000000..ea77b69
--- /dev/null
+++ b/lib/librte_ether/rte_eth_features.h
@@ -0,0 +1,65 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_ETH_FEATURES_H_
+#define _RTE_ETH_FEATURES_H_
+
+/**
+ * @file
+ *
+ * Ethernet device specific features
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/* Commands defined for NIC specific features */
+enum rte_eth_command {
+	RTE_CMD_UNKNOWN = 0,
+	RTE_CMD_FDIR_RULE_ADD,
+	/**< Command to add a new FDIR filter rule. */
+	RTE_CMD_FDIR_RULE_DEL,
+	/**< Command to delete a FDIR filter rule. */
+	RTE_CMD_FDIR_FLUSH,
+	/**< Command to clear all FDIR filter rules. */
+	RTE_CMD_FDIR_INFO_GET,
+	/**< Command to get FDIR information. */
+	RTE_CMD_MAX = 255,
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_ETH_FEATURES_H_ */
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index fd1010a..10a08de 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -41,7 +41,6 @@
 #include <errno.h>
 #include <stdint.h>
 #include <inttypes.h>
-#include <netinet/in.h>
 
 #include <rte_byteorder.h>
 #include <rte_log.h>
@@ -66,6 +65,7 @@
 #include <rte_errno.h>
 #include <rte_spinlock.h>
 #include <rte_string_fns.h>
+#include <rte_ip.h>
 
 #include "rte_ether.h"
 #include "rte_ethdev.h"
@@ -3002,3 +3002,20 @@ rte_eth_dev_get_flex_filter(uint8_t port_id, uint16_t index,
 	return (*dev->dev_ops->get_flex_filter)(dev, index, filter,
 						rx_queue);
 }
+
+int
+rte_eth_dev_rx_classification_filter_ctl(uint8_t port_id,
+					 enum rte_eth_command cmd,
+					 void *args)
+{
+	struct rte_eth_dev *dev;
+
+	if (port_id >= nb_ports) {
+		PMD_DEBUG_TRACE("Invalid port_id=%d\n", port_id);
+		return -ENODEV;
+	}
+	dev = &rte_eth_devices[port_id];
+	FUNC_PTR_OR_ERR_RET(*dev->dev_ops->rx_classification_filter_ctl,
+								-ENOTSUP);
+	return (*dev->dev_ops->rx_classification_filter_ctl)(dev, cmd, args);
+}
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 50df654..a2d9596 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -177,6 +177,7 @@ extern "C" {
 #include <rte_pci.h>
 #include <rte_mbuf.h>
 #include "rte_ether.h"
+#include "rte_eth_features.h"
 
 /**
  * A structure used to retrieve statistics for an Ethernet port.
@@ -345,47 +346,47 @@ struct rte_eth_rss_conf {
 #define ETH_RSS_IPV4_UDP_SHIFT                6
 #define ETH_RSS_IPV6_UDP_SHIFT                7
 #define ETH_RSS_IPV6_UDP_EX_SHIFT             8
-/* for 40G only */
-#define ETH_RSS_NONF_IPV4_UDP_SHIFT           31
-#define ETH_RSS_NONF_IPV4_TCP_SHIFT           33
-#define ETH_RSS_NONF_IPV4_SCTP_SHIFT          34
-#define ETH_RSS_NONF_IPV4_OTHER_SHIFT         35
-#define ETH_RSS_FRAG_IPV4_SHIFT               36
-#define ETH_RSS_NONF_IPV6_UDP_SHIFT           41
-#define ETH_RSS_NONF_IPV6_TCP_SHIFT           43
-#define ETH_RSS_NONF_IPV6_SCTP_SHIFT          44
-#define ETH_RSS_NONF_IPV6_OTHER_SHIFT         45
-#define ETH_RSS_FRAG_IPV6_SHIFT               46
-#define ETH_RSS_FCOE_OX_SHIFT                 48
-#define ETH_RSS_FCOE_RX_SHIFT                 49
-#define ETH_RSS_FCOE_OTHER_SHIFT              50
-#define ETH_RSS_L2_PAYLOAD_SHIFT              63
+/* Packet Classification Type for 40G only */
+#define ETH_PCTYPE_NONF_IPV4_UDP              31
+#define ETH_PCTYPE_NONF_IPV4_TCP              33
+#define ETH_PCTYPE_NONF_IPV4_SCTP             34
+#define ETH_PCTYPE_NONF_IPV4_OTHER            35
+#define ETH_PCTYPE_FRAG_IPV4                  36
+#define ETH_PCTYPE_NONF_IPV6_UDP              41
+#define ETH_PCTYPE_NONF_IPV6_TCP              43
+#define ETH_PCTYPE_NONF_IPV6_SCTP             44
+#define ETH_PCTYPE_NONF_IPV6_OTHER            45
+#define ETH_PCTYPE_FRAG_IPV6                  46
+#define ETH_PCTYPE_FCOE_OX                    48 /* not used */
+#define ETH_PCTYPE_FCOE_RX                    49 /* not used */
+#define ETH_PCTYPE_FCOE_OTHER                 50 /* not used */
+#define ETH_PCTYPE_L2_PAYLOAD                 63
 
 /* for 1G & 10G */
-#define ETH_RSS_IPV4                    ((uint16_t)1 << ETH_RSS_IPV4_SHIFT)
-#define ETH_RSS_IPV4_TCP                ((uint16_t)1 << ETH_RSS_IPV4_TCP_SHIFT)
-#define ETH_RSS_IPV6                    ((uint16_t)1 << ETH_RSS_IPV6_SHIFT)
-#define ETH_RSS_IPV6_EX                 ((uint16_t)1 << ETH_RSS_IPV6_EX_SHIFT)
-#define ETH_RSS_IPV6_TCP                ((uint16_t)1 << ETH_RSS_IPV6_TCP_SHIFT)
-#define ETH_RSS_IPV6_TCP_EX             ((uint16_t)1 << ETH_RSS_IPV6_TCP_EX_SHIFT)
-#define ETH_RSS_IPV4_UDP                ((uint16_t)1 << ETH_RSS_IPV4_UDP_SHIFT)
-#define ETH_RSS_IPV6_UDP                ((uint16_t)1 << ETH_RSS_IPV6_UDP_SHIFT)
-#define ETH_RSS_IPV6_UDP_EX             ((uint16_t)1 << ETH_RSS_IPV6_UDP_EX_SHIFT)
+#define ETH_RSS_IPV4                    (1 << ETH_RSS_IPV4_SHIFT)
+#define ETH_RSS_IPV4_TCP                (1 << ETH_RSS_IPV4_TCP_SHIFT)
+#define ETH_RSS_IPV6                    (1 << ETH_RSS_IPV6_SHIFT)
+#define ETH_RSS_IPV6_EX                 (1 << ETH_RSS_IPV6_EX_SHIFT)
+#define ETH_RSS_IPV6_TCP                (1 << ETH_RSS_IPV6_TCP_SHIFT)
+#define ETH_RSS_IPV6_TCP_EX             (1 << ETH_RSS_IPV6_TCP_EX_SHIFT)
+#define ETH_RSS_IPV4_UDP                (1 << ETH_RSS_IPV4_UDP_SHIFT)
+#define ETH_RSS_IPV6_UDP                (1 << ETH_RSS_IPV6_UDP_SHIFT)
+#define ETH_RSS_IPV6_UDP_EX             (1 << ETH_RSS_IPV6_UDP_EX_SHIFT)
 /* for 40G only */
-#define ETH_RSS_NONF_IPV4_UDP           ((uint64_t)1 << ETH_RSS_NONF_IPV4_UDP_SHIFT)
-#define ETH_RSS_NONF_IPV4_TCP           ((uint64_t)1 << ETH_RSS_NONF_IPV4_TCP_SHIFT)
-#define ETH_RSS_NONF_IPV4_SCTP          ((uint64_t)1 << ETH_RSS_NONF_IPV4_SCTP_SHIFT)
-#define ETH_RSS_NONF_IPV4_OTHER         ((uint64_t)1 << ETH_RSS_NONF_IPV4_OTHER_SHIFT)
-#define ETH_RSS_FRAG_IPV4               ((uint64_t)1 << ETH_RSS_FRAG_IPV4_SHIFT)
-#define ETH_RSS_NONF_IPV6_UDP           ((uint64_t)1 << ETH_RSS_NONF_IPV6_UDP_SHIFT)
-#define ETH_RSS_NONF_IPV6_TCP           ((uint64_t)1 << ETH_RSS_NONF_IPV6_TCP_SHIFT)
-#define ETH_RSS_NONF_IPV6_SCTP          ((uint64_t)1 << ETH_RSS_NONF_IPV6_SCTP_SHIFT)
-#define ETH_RSS_NONF_IPV6_OTHER         ((uint64_t)1 << ETH_RSS_NONF_IPV6_OTHER_SHIFT)
-#define ETH_RSS_FRAG_IPV6               ((uint64_t)1 << ETH_RSS_FRAG_IPV6_SHIFT)
-#define ETH_RSS_FCOE_OX                 ((uint64_t)1 << ETH_RSS_FCOE_OX_SHIFT) /* not used */
-#define ETH_RSS_FCOE_RX                 ((uint64_t)1 << ETH_RSS_FCOE_RX_SHIFT) /* not used */
-#define ETH_RSS_FCOE_OTHER              ((uint64_t)1 << ETH_RSS_FCOE_OTHER_SHIFT) /* not used */
-#define ETH_RSS_L2_PAYLOAD              ((uint64_t)1 << ETH_RSS_L2_PAYLOAD_SHIFT)
+#define ETH_RSS_NONF_IPV4_UDP           (1ULL << ETH_PCTYPE_NONF_IPV4_UDP)
+#define ETH_RSS_NONF_IPV4_TCP           (1ULL << ETH_PCTYPE_NONF_IPV4_TCP)
+#define ETH_RSS_NONF_IPV4_SCTP          (1ULL << ETH_PCTYPE_NONF_IPV4_SCTP)
+#define ETH_RSS_NONF_IPV4_OTHER         (1ULL << ETH_PCTYPE_NONF_IPV4_OTHER)
+#define ETH_RSS_FRAG_IPV4               (1ULL << ETH_PCTYPE_FRAG_IPV4)
+#define ETH_RSS_NONF_IPV6_UDP           (1ULL << ETH_PCTYPE_NONF_IPV6_UDP)
+#define ETH_RSS_NONF_IPV6_TCP           (1ULL << ETH_PCTYPE_NONF_IPV6_TCP)
+#define ETH_RSS_NONF_IPV6_SCTP          (1ULL << ETH_PCTYPE_NONF_IPV6_SCTP)
+#define ETH_RSS_NONF_IPV6_OTHER         (1ULL << ETH_PCTYPE_NONF_IPV6_OTHER)
+#define ETH_RSS_FRAG_IPV6               (1ULL << ETH_PCTYPE_FRAG_IPV6)
+#define ETH_RSS_FCOE_OX                 (1ULL << ETH_PCTYPE_FCOE_OX)
+#define ETH_RSS_FCOE_RX                 (1ULL << ETH_PCTYPE_FCOE_RX)
+#define ETH_RSS_FCOE_OTHER              (1ULL << ETH_PCTYPE_FCOE_OTHER)
+#define ETH_RSS_L2_PAYLOAD              (1ULL << ETH_PCTYPE_L2_PAYLOAD)
 
 #define ETH_RSS_IP ( \
 		ETH_RSS_IPV4 | \
@@ -1361,6 +1362,11 @@ typedef int (*eth_get_flex_filter_t)(struct rte_eth_dev *dev,
 			uint16_t *rx_queue);
 /**< @internal Get a flex filter rule on an Ethernet device */
 
+typedef int (*eth_rx_classification_filter_ctl_t)(struct rte_eth_dev *dev,
+						  enum rte_eth_command cmd,
+						  void *arg);
+/**< @internal receive classification filter control operations */
+
 /**
  * @internal A structure containing the functions exported by an Ethernet driver.
  */
@@ -1467,6 +1473,8 @@ struct eth_dev_ops {
 	eth_add_flex_filter_t          add_flex_filter;      /**< add flex filter. */
 	eth_remove_flex_filter_t       remove_flex_filter;   /**< remove flex filter. */
 	eth_get_flex_filter_t          get_flex_filter;      /**< get flex filter. */
+	/*common ccontrol function for receive classification filters*/
+	eth_rx_classification_filter_ctl_t rx_classification_filter_ctl;
 };
 
 /**
@@ -3557,6 +3565,30 @@ int rte_eth_dev_remove_flex_filter(uint8_t port_id, uint16_t index);
 int rte_eth_dev_get_flex_filter(uint8_t port_id, uint16_t index,
 			struct rte_flex_filter *filter, uint16_t *rx_queue);
 
+
+/**
+ * Configure the receive classification filter, including hash function
+ * selection. The commands are NIC specific in its exported public header
+ * file. Different types of NIC may have different commands.
+ * All the supported commands are defined in 'rte_eth_features.h'.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param cmd
+ *   The command.
+ * @param args
+ *   A pointer to arguments defined specifically for the command.
+ *
+ * @return
+ *   - (0) if successful.
+ *   - (-ENOTSUP) if hardware doesn't support.
+ *   - (-ENODEV) if <port_id> is invalid.
+ *   - others depends on the specific command implementation.
+ */
+int rte_eth_dev_rx_classification_filter_ctl(uint8_t port_id,
+					     enum rte_eth_command cmd,
+					     void *args);
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.1.4

Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl

[Thomas Monjalon]

Hi Jingjing,

2014-08-27 10:13, Jingjing Wu:
> support a new ethdev API rx_classification_filter_ctl for all
> the configuration or queries for receive classification filters.
> this patch supports commands the API used below:
>   RTE_CMD_FDIR_RULE_ADD
>   RTE_CMD_FDIR_RULE_DEL
>   RTE_CMD_FDIR_FLUSH
>   RTE_CMD_FDIR_INFO_GET

Could you explain why existing API (flow director + filters) is not enough?
I'd really like to see a common API for all filtering stuff.

> -/* for 40G only */
> -#define ETH_RSS_NONF_IPV4_UDP_SHIFT           31
> -#define ETH_RSS_NONF_IPV4_TCP_SHIFT           33
> -#define ETH_RSS_NONF_IPV4_SCTP_SHIFT          34
> -#define ETH_RSS_NONF_IPV4_OTHER_SHIFT         35
> -#define ETH_RSS_FRAG_IPV4_SHIFT               36
> -#define ETH_RSS_NONF_IPV6_UDP_SHIFT           41
> -#define ETH_RSS_NONF_IPV6_TCP_SHIFT           43
> -#define ETH_RSS_NONF_IPV6_SCTP_SHIFT          44
> -#define ETH_RSS_NONF_IPV6_OTHER_SHIFT         45
> -#define ETH_RSS_FRAG_IPV6_SHIFT               46
> -#define ETH_RSS_FCOE_OX_SHIFT                 48
> -#define ETH_RSS_FCOE_RX_SHIFT                 49
> -#define ETH_RSS_FCOE_OTHER_SHIFT              50
> -#define ETH_RSS_L2_PAYLOAD_SHIFT              63
> +/* Packet Classification Type for 40G only */
> +#define ETH_PCTYPE_NONF_IPV4_UDP              31
> +#define ETH_PCTYPE_NONF_IPV4_TCP              33
> +#define ETH_PCTYPE_NONF_IPV4_SCTP             34
> +#define ETH_PCTYPE_NONF_IPV4_OTHER            35
> +#define ETH_PCTYPE_FRAG_IPV4                  36
> +#define ETH_PCTYPE_NONF_IPV6_UDP              41
> +#define ETH_PCTYPE_NONF_IPV6_TCP              43
> +#define ETH_PCTYPE_NONF_IPV6_SCTP             44
> +#define ETH_PCTYPE_NONF_IPV6_OTHER            45
> +#define ETH_PCTYPE_FRAG_IPV6                  46
> +#define ETH_PCTYPE_FCOE_OX                    48 /* not used */
> +#define ETH_PCTYPE_FCOE_RX                    49 /* not used */
> +#define ETH_PCTYPE_FCOE_OTHER                 50 /* not used */
> +#define ETH_PCTYPE_L2_PAYLOAD                 63

Why is it specific to i40e? Could we have something generic?
Please take care at having only generic things in librte_ether.

By the way, these renamings should be in a separated patch.

-- 
Thomas

Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl

[Wu, Jingjing]

HI, Thomas

Just as zhang, helin's said in his pacth "[dpdk-dev] [PATCH 2/5] ethdev: add new ops of 'check_command_supported' and 'rx_classification_filter_ctl'":

'rx_classification_filter_ctl' is for receive classification filter configuring. e.g. hash function configuration, flow director configuration. It is a common API where a lot of commands can be implemented for different sub features.
We want to implement several common API for NIC specific features, to avoid creating quite a lot of ops in 'struct eth_dev_ops'. The idea came from ioctl.

Because about flow director feature, there is large gap between i40e and ixgbe. The existed flow director APIs looks specific to IXGBE, so I choose this new API to implement i40e's flow director feature.

Here, I briefly describe how the new 'rx_classification_filter_ctl' works:

The API is like below:
typedef int (*eth_rx_classification_filter_ctl_t)(struct rte_eth_dev *dev,
						  enum rte_eth_command cmd,
						  void *arg);
Define a head file called rte_i40e.h in lib/librte_pmd_i40e, which contains the definition about structures specific to i40e, linked to the arg parameters above.
Define a head file called rte_eth_features.h in lib/librte_ether, which contains the commands' definition linked to cmd parameters above.
And if user want to use i40e specific features, then the head file rte_i40e.h need to be included user's application, for example, test-pmd. And user can encode these structures and call XXX_ctl API to configure their features.

Do you think it make sense?

And about the rename things, I will move it to a separate patch.


> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, August 27, 2014 10:23 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API
> rx_classification_filter_ctl
> 
> Hi Jingjing,
> 
> 2014-08-27 10:13, Jingjing Wu:
> > support a new ethdev API rx_classification_filter_ctl for all
> > the configuration or queries for receive classification filters.
> > this patch supports commands the API used below:
> >   RTE_CMD_FDIR_RULE_ADD
> >   RTE_CMD_FDIR_RULE_DEL
> >   RTE_CMD_FDIR_FLUSH
> >   RTE_CMD_FDIR_INFO_GET
> 
> Could you explain why existing API (flow director + filters) is not enough?
> I'd really like to see a common API for all filtering stuff.
> 
> > -/* for 40G only */
> > -#define ETH_RSS_NONF_IPV4_UDP_SHIFT           31
> > -#define ETH_RSS_NONF_IPV4_TCP_SHIFT           33
> > -#define ETH_RSS_NONF_IPV4_SCTP_SHIFT          34
> > -#define ETH_RSS_NONF_IPV4_OTHER_SHIFT         35
> > -#define ETH_RSS_FRAG_IPV4_SHIFT               36
> > -#define ETH_RSS_NONF_IPV6_UDP_SHIFT           41
> > -#define ETH_RSS_NONF_IPV6_TCP_SHIFT           43
> > -#define ETH_RSS_NONF_IPV6_SCTP_SHIFT          44
> > -#define ETH_RSS_NONF_IPV6_OTHER_SHIFT         45
> > -#define ETH_RSS_FRAG_IPV6_SHIFT               46
> > -#define ETH_RSS_FCOE_OX_SHIFT                 48
> > -#define ETH_RSS_FCOE_RX_SHIFT                 49
> > -#define ETH_RSS_FCOE_OTHER_SHIFT              50
> > -#define ETH_RSS_L2_PAYLOAD_SHIFT              63
> > +/* Packet Classification Type for 40G only */
> > +#define ETH_PCTYPE_NONF_IPV4_UDP              31
> > +#define ETH_PCTYPE_NONF_IPV4_TCP              33
> > +#define ETH_PCTYPE_NONF_IPV4_SCTP             34
> > +#define ETH_PCTYPE_NONF_IPV4_OTHER            35
> > +#define ETH_PCTYPE_FRAG_IPV4                  36
> > +#define ETH_PCTYPE_NONF_IPV6_UDP              41
> > +#define ETH_PCTYPE_NONF_IPV6_TCP              43
> > +#define ETH_PCTYPE_NONF_IPV6_SCTP             44
> > +#define ETH_PCTYPE_NONF_IPV6_OTHER            45
> > +#define ETH_PCTYPE_FRAG_IPV6                  46
> > +#define ETH_PCTYPE_FCOE_OX                    48 /* not used */
> > +#define ETH_PCTYPE_FCOE_RX                    49 /* not used */
> > +#define ETH_PCTYPE_FCOE_OTHER                 50 /* not used */
> > +#define ETH_PCTYPE_L2_PAYLOAD                 63
> 
> Why is it specific to i40e? Could we have something generic?
> Please take care at having only generic things in librte_ether.
> 
> By the way, these renamings should be in a separated patch.
> 
> --
> Thomas

Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl

[Thomas Monjalon]

2014-08-28 03:30, Wu, Jingjing:
> We want to implement several common API for NIC specific features,
> to avoid creating quite a lot of ops in 'struct eth_dev_ops'.
> The idea came from ioctl.

The approach can be interesting.

> Because about flow director feature, there is large gap between i40e
> and ixgbe. The existed flow director APIs looks specific to IXGBE,
> so I choose this new API to implement i40e's flow director feature.

The API is not OK for you so you create another one.
I'm OK to change APIs but you should remove the old one, or at least,
implement your new API in existing drivers to allow deprecation of the
old API.
I think it would help if you start by doing ixgbe work and then apply it
to i40e.

> The API is like below:
> typedef int (*eth_rx_classification_filter_ctl_t)(struct rte_eth_dev *dev,
> 						  enum rte_eth_command cmd,
> 						  void *arg);
> Define a head file called rte_i40e.h in lib/librte_pmd_i40e, which contains
> the definition about structures specific to i40e, linked to the arg
> parameters above.
> Define a head file called rte_eth_features.h in lib/librte_ether, which
> contains the commands' definition linked to cmd parameters above.

Why creating a rte_eth_features.h? Don't you think rte_ethdev.h is a good place?

> And if user want to use i40e specific features, then the head file
> rte_i40e.h need to be included user's application, for example, test-pmd.
> And user can encode these structures and call XXX_ctl API to configure
> their features.

OK, but the question is to know what is a specific feature?
I don't think flow director is a specific feature. We shouldn't have
to care if port is i40e or ixgbe to setup flow director.
Is it possible to have a common API and maybe an inheritance of the
common structure with PMD specific fields?

Example:

struct fdir_entry {
	fdir_input input;
	fdir_action action;
	enum rte_driver driver;
};
fdir_entry_generic fdir_entry = {.driver = RTE_DRIVER_GENERIC};
filter_ctl(port, FDIR_RULE_ADD, fdir_entry);

struct i40e_fdir_entry {
	struct fdir_entry generic;
	i40e_fdir_input i40e_input;
};

So i40e_input will be handled by the PMD if driver == RTE_DRIVER_I40E.

It's just an idea, comments are welcome.

-- 
Thomas

Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl

[Ananyev, Konstantin]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Thursday, August 28, 2014 11:56 AM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl
> 
> 2014-08-28 03:30, Wu, Jingjing:
> > We want to implement several common API for NIC specific features,
> > to avoid creating quite a lot of ops in 'struct eth_dev_ops'.
> > The idea came from ioctl.
> 
> The approach can be interesting.
> 
> > Because about flow director feature, there is large gap between i40e
> > and ixgbe. The existed flow director APIs looks specific to IXGBE,
> > so I choose this new API to implement i40e's flow director feature.
> 
> The API is not OK for you so you create another one.
> I'm OK to change APIs but you should remove the old one, or at least,
> implement your new API in existing drivers to allow deprecation of the
> old API.
> I think it would help if you start by doing ixgbe work and then apply it
> to i40e.
> 
> > The API is like below:
> > typedef int (*eth_rx_classification_filter_ctl_t)(struct rte_eth_dev *dev,
> > 						  enum rte_eth_command cmd,
> > 						  void *arg);
> > Define a head file called rte_i40e.h in lib/librte_pmd_i40e, which contains
> > the definition about structures specific to i40e, linked to the arg
> > parameters above.
> > Define a head file called rte_eth_features.h in lib/librte_ether, which
> > contains the commands' definition linked to cmd parameters above.
> 
> Why creating a rte_eth_features.h? Don't you think rte_ethdev.h is a good place?

As I remember the long term idea was
(Jingjing please correct me, if I am wrong here):

Keep rte_ethdev.h for generic API.
Put features specific for different NICs into rte_eth_features.h 
To make things clearer and avoid polluting of rte_ethdev.h.

Provide API for the upper layer to query list of specific features(commands) supported by the underlying device.
Something like: 
rte_eth_dev_get_rx_filter_supported(port, ...);

And ioctl-type API to configure HW specific features: 
rte_eth_dev_rx_classification_filter_ctl(port, cmd, cmd_spedific_arg);

So, instead of application knows in advance "which specific NIC it is using",
application would query which features/commannds the NIC provides and then configure them appropriately.

> 
> > And if user want to use i40e specific features, then the head file
> > rte_i40e.h need to be included user's application, for example, test-pmd.
> > And user can encode these structures and call XXX_ctl API to configure
> > their features.
> 
> OK, but the question is to know what is a specific feature?
> I don't think flow director is a specific feature. We shouldn't have
> to care if port is i40e or ixgbe to setup flow director.
> Is it possible to have a common API and maybe an inheritance of the
> common structure with PMD specific fields?
> 
> Example:
> 
> struct fdir_entry {
> 	fdir_input input;
> 	fdir_action action;
> 	enum rte_driver driver;
> };
> fdir_entry_generic fdir_entry = {.driver = RTE_DRIVER_GENERIC};
> filter_ctl(port, FDIR_RULE_ADD, fdir_entry);
> 
> struct i40e_fdir_entry {
> 	struct fdir_entry generic;
> 	i40e_fdir_input i40e_input;
> };
> 
> So i40e_input will be handled by the PMD if driver == RTE_DRIVER_I40E.
> 
> It's just an idea, comments are welcome.
> 
> --
> Thomas

Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl

[Wu, Jingjing]

Hi, Konstantin

Thanks a lot.

> -----Original Message-----
> From: Ananyev, Konstantin
> Sent: Thursday, August 28, 2014 7:49 PM
> To: Thomas Monjalon; Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API
> rx_classification_filter_ctl
> 
> 
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> > Sent: Thursday, August 28, 2014 11:56 AM
> > To: Wu, Jingjing
> > Cc: dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API
> rx_classification_filter_ctl
> >
> > 2014-08-28 03:30, Wu, Jingjing:
> > > We want to implement several common API for NIC specific features,
> > > to avoid creating quite a lot of ops in 'struct eth_dev_ops'.
> > > The idea came from ioctl.
> >
> > The approach can be interesting.
> >
> > > Because about flow director feature, there is large gap between i40e
> > > and ixgbe. The existed flow director APIs looks specific to IXGBE,
> > > so I choose this new API to implement i40e's flow director feature.
> >
> > The API is not OK for you so you create another one.
> > I'm OK to change APIs but you should remove the old one, or at least,
> > implement your new API in existing drivers to allow deprecation of the
> > old API.
> > I think it would help if you start by doing ixgbe work and then apply it
> > to i40e.
> >
> > > The API is like below:
> > > typedef int (*eth_rx_classification_filter_ctl_t)(struct rte_eth_dev *dev,
> > > 						  enum rte_eth_command cmd,
> > > 						  void *arg);
> > > Define a head file called rte_i40e.h in lib/librte_pmd_i40e, which contains
> > > the definition about structures specific to i40e, linked to the arg
> > > parameters above.
> > > Define a head file called rte_eth_features.h in lib/librte_ether, which
> > > contains the commands' definition linked to cmd parameters above.
> >
> > Why creating a rte_eth_features.h? Don't you think rte_ethdev.h is a good place?
> 
> As I remember the long term idea was
> (Jingjing please correct me, if I am wrong here):
> 
> Keep rte_ethdev.h for generic API.
> Put features specific for different NICs into rte_eth_features.h
> To make things clearer and avoid polluting of rte_ethdev.h.
> 
> Provide API for the upper layer to query list of specific features(commands) supported by the
> underlying device.
> Something like:
> rte_eth_dev_get_rx_filter_supported(port, ...);

Yes, in helin's patch "[dpdk-dev] [PATCH v2 2/6] ethdev: add new ops of
'is_command_supported' and 'rx_classification_filter_ctl'", he already
defined rte_eth_dev_is_command_supported. It can be used to check
whether such command can be supported by the queried port.

Actually, some features are based on this architecture, including helin's
"Support configuring hash functions" and other non-ready patch.
We supposed after any patch of ours is applied, others need to rework
then. We are using the same approach.

> And ioctl-type API to configure HW specific features:
> rte_eth_dev_rx_classification_filter_ctl(port, cmd, cmd_spedific_arg);
> 
> So, instead of application knows in advance "which specific NIC it is using",
> application would query which features/commannds the NIC provides and then configure
> them appropriately.
> 
> >
> > > And if user want to use i40e specific features, then the head file
> > > rte_i40e.h need to be included user's application, for example, test-pmd.
> > > And user can encode these structures and call XXX_ctl API to configure
> > > their features.
> >
> > OK, but the question is to know what is a specific feature?
> > I don't think flow director is a specific feature. We shouldn't have
> > to care if port is i40e or ixgbe to setup flow director.
> > Is it possible to have a common API and maybe an inheritance of the
> > common structure with PMD specific fields?
> >
> > Example:
> >
> > struct fdir_entry {
> > 	fdir_input input;
> > 	fdir_action action;
> > 	enum rte_driver driver;
> > };
> > fdir_entry_generic fdir_entry = {.driver = RTE_DRIVER_GENERIC};
> > filter_ctl(port, FDIR_RULE_ADD, fdir_entry);
> >
> > struct i40e_fdir_entry {
> > 	struct fdir_entry generic;
> > 	i40e_fdir_input i40e_input;
> > };
> >
> > So i40e_input will be handled by the PMD if driver == RTE_DRIVER_I40E.
> >
> > It's just an idea, comments are welcome.
> >
> > --
> > Thomas

Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl

[Wu, Jingjing]

Hi, Thomas

Please see my comments below. 

Thanks a lot.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, August 28, 2014 6:56 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API
> rx_classification_filter_ctl
> 
> 2014-08-28 03:30, Wu, Jingjing:
> > We want to implement several common API for NIC specific features,
> > to avoid creating quite a lot of ops in 'struct eth_dev_ops'.
> > The idea came from ioctl.
> 
> The approach can be interesting.

Yes, we have discussed this approach inside. And some other Fortville
Features are also based on it, such as RSS hash function support,
mac vlan filters.
Maybe it a good chance to discuss in open forum now.

> 
> > Because about flow director feature, there is large gap between i40e
> > and ixgbe. The existed flow director APIs looks specific to IXGBE,
> > so I choose this new API to implement i40e's flow director feature.
> 
> The API is not OK for you so you create another one.
> I'm OK to change APIs but you should remove the old one, or at least,
> implement your new API in existing drivers to allow deprecation of the
> old API.
> I think it would help if you start by doing ixgbe work and then apply it
> to i40e.
> 

Yes, it will be perfect if we can use this new API to achieve flow director 
setting all types of NICs. But the concern is downward compatibility. 
Users who is planning update DPDK version need to change their code
to adapt such changes.
That's why we choose a new API instead of modifying current APIs. And 
Of course, the ideal plan is adding such XXX_ctl function in Ixgbe and
Igb to moving smoothly without removing current APIs.
I'm not sure whether I understanding correctly about the importance of
compatibility. 

> > The API is like below:
> > typedef int (*eth_rx_classification_filter_ctl_t)(struct rte_eth_dev *dev,
> > 						  enum rte_eth_command cmd,
> > 						  void *arg);
> > Define a head file called rte_i40e.h in lib/librte_pmd_i40e, which contains
> > the definition about structures specific to i40e, linked to the arg
> > parameters above.
> > Define a head file called rte_eth_features.h in lib/librte_ether, which
> > contains the commands' definition linked to cmd parameters above.
> 
> Why creating a rte_eth_features.h? Don't you think rte_ethdev.h is a good place?
> 
> > And if user want to use i40e specific features, then the head file
> > rte_i40e.h need to be included user's application, for example, test-pmd.
> > And user can encode these structures and call XXX_ctl API to configure
> > their features.
> 
> OK, but the question is to know what is a specific feature?
> I don't think flow director is a specific feature. We shouldn't have
> to care if port is i40e or ixgbe to setup flow director.
> Is it possible to have a common API and maybe an inheritance of the
> common structure with PMD specific fields?
> 

Yes, flow director is not a specific feature. Even ixgbe and i40 use the same 
name. But the context and key have much difference. That's why I called it
specific.

> Example:
> 
> struct fdir_entry {
> 	fdir_input input;
> 	fdir_action action;
> 	enum rte_driver driver;
> };
> fdir_entry_generic fdir_entry = {.driver = RTE_DRIVER_GENERIC};
> filter_ctl(port, FDIR_RULE_ADD, fdir_entry);
> 
> struct i40e_fdir_entry {
> 	struct fdir_entry generic;
> 	i40e_fdir_input i40e_input;
> };
> 
> So i40e_input will be handled by the PMD if driver == RTE_DRIVER_I40E.
> 
> It's just an idea, comments are welcome.

Yes, it's a good idea about an inheritance of the common structure. I think it
may support new NIC integration in future. We can do it with the new API 
architecture. But the concern is still how to be compatible with old version.

> --
> Thomas

Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl

[Thomas Monjalon]

2014-08-28 13:39, Wu, Jingjing:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > I'm OK to change APIs but you should remove the old one, or at least,
> > implement your new API in existing drivers to allow deprecation of the
> > old API.
> > I think it would help if you start by doing ixgbe work and then apply it
> > to i40e.
> > 
> 
> Yes, it will be perfect if we can use this new API to achieve flow director 
> setting all types of NICs. But the concern is downward compatibility.

In this case, cleanup is more important than compatibility.

> Users who is planning update DPDK version need to change their code
> to adapt such changes.

Yes, but we can keep deprecated function during 1 release.

> That's why we choose a new API instead of modifying current APIs. And 
> Of course, the ideal plan is adding such XXX_ctl function in Ixgbe and
> Igb to moving smoothly without removing current APIs.

Yes

> > I don't think flow director is a specific feature. We shouldn't have
> > to care if port is i40e or ixgbe to setup flow director.
> > Is it possible to have a common API and maybe an inheritance of the
> > common structure with PMD specific fields?
> 
> Yes, flow director is not a specific feature. Even ixgbe and i40 use the same 
> name. But the context and key have much difference. That's why I called it
> specific.
> 
> Yes, it's a good idea about an inheritance of the common structure. I think it
> may support new NIC integration in future. We can do it with the new API 
> architecture. But the concern is still how to be compatible with old version.

There is no compatibility blocker here.
If we can keep deprecated functions a while, we'll do. Otherwise, just go with
the new API.
I prefer we concentrate on good design rather than on compatibility.

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API rx_classification_filter_ctl

[Wu, Jingjing]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, August 28, 2014 10:21 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 2/7] ethdev: define new ethdev API
> rx_classification_filter_ctl
> 
> 2014-08-28 13:39, Wu, Jingjing:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > I'm OK to change APIs but you should remove the old one, or at least,
> > > implement your new API in existing drivers to allow deprecation of the
> > > old API.
> > > I think it would help if you start by doing ixgbe work and then apply it
> > > to i40e.
> > >
> >
> > Yes, it will be perfect if we can use this new API to achieve flow director
> > setting all types of NICs. But the concern is downward compatibility.
> 
> In this case, cleanup is more important than compatibility.
> 
> > Users who is planning update DPDK version need to change their code
> > to adapt such changes.
> 
> Yes, but we can keep deprecated function during 1 release.
> 
> > That's why we choose a new API instead of modifying current APIs. And
> > Of course, the ideal plan is adding such XXX_ctl function in Ixgbe and
> > Igb to moving smoothly without removing current APIs.
> 
> Yes
> 
> > > I don't think flow director is a specific feature. We shouldn't have
> > > to care if port is i40e or ixgbe to setup flow director.
> > > Is it possible to have a common API and maybe an inheritance of the
> > > common structure with PMD specific fields?
> >
> > Yes, flow director is not a specific feature. Even ixgbe and i40 use the same
> > name. But the context and key have much difference. That's why I called it
> > specific.
> >
> > Yes, it's a good idea about an inheritance of the common structure. I think it
> > may support new NIC integration in future. We can do it with the new API
> > architecture. But the concern is still how to be compatible with old version.
> 
> There is no compatibility blocker here.
> If we can keep deprecated functions a while, we'll do. Otherwise, just go with
> the new API.
> I prefer we concentrate on good design rather than on compatibility.
> 

OK, now I have a rough understanding about your opinion, I guess there will be lots of rework need to be done. I will try. Thanks for your explanation.

> Thanks
> --
> Thomas

Thanks
Jingjing

[dpdk-dev] [PATCH v2 3/7] i40e: function implement in i40e for flow director filter programming

[Jingjing Wu]

support the API ops defined in ethdev, the behavior according to each command:
  RTE_CMD_FDIR_RULE_ADD: add a new FDIR filter rule.
  RTE_CMD_FDIR_RULE_DEL: delete a FDIR filter rule.
 
Signed-off-by: jingjing.wu <jingjing.wu@intel.com>
Reviewed-by: Helin Zhang <helin.zhang@intel.com>
Reviewed-by: Jing Chen <jing.d.chen@intel.com>
Reviewed-by: Jijiang Liu <jijiang.liu@intel.com>
 
---
 lib/librte_pmd_i40e/Makefile      |   4 +
 lib/librte_pmd_i40e/i40e_ethdev.c |  43 ++++++++
 lib/librte_pmd_i40e/i40e_ethdev.h |   7 ++
 lib/librte_pmd_i40e/i40e_fdir.c   | 225 ++++++++++++++++++++++++++++++++++++++
 lib/librte_pmd_i40e/rte_i40e.h    | 111 +++++++++++++++++++
 5 files changed, 390 insertions(+)
 create mode 100644 lib/librte_pmd_i40e/rte_i40e.h

diff --git a/lib/librte_pmd_i40e/Makefile b/lib/librte_pmd_i40e/Makefile
index 6537654..3da20c5 100644
--- a/lib/librte_pmd_i40e/Makefile
+++ b/lib/librte_pmd_i40e/Makefile
@@ -88,6 +88,10 @@ SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_rxtx.c
 SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_ethdev_vf.c
 SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_pf.c
 SRCS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += i40e_fdir.c
+
+# install this header file
+SYMLINK-$(CONFIG_RTE_LIBRTE_I40E_PMD)-include := rte_i40e.h
+
 # this lib depends upon:
 DEPDIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += lib/librte_eal lib/librte_ether
 DEPDIRS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += lib/librte_mempool lib/librte_mbuf
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index a08b43c..7dcf964 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_features.h>
 
 #include "i40e_logs.h"
 #include "i40e/i40e_register_x710_int.h"
@@ -205,6 +206,9 @@ 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_rx_classification_filter_ctl(struct rte_eth_dev *dev,
+					     enum rte_eth_command cmd,
+					     void *args);
 
 /* Default hash key buffer for RSS */
 static uint32_t rss_key_default[I40E_PFQF_HKEY_MAX_INDEX + 1];
@@ -256,6 +260,7 @@ 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,
+	.rx_classification_filter_ctl = i40e_rx_classification_filter_ctl,
 };
 
 static struct eth_driver rte_i40e_pmd = {
@@ -4185,3 +4190,41 @@ i40e_pf_config_mq_rx(struct i40e_pf *pf)
 
 	return 0;
 }
+
+static int
+i40e_rx_classification_filter_ctl(struct rte_eth_dev *dev,
+				  enum rte_eth_command cmd,
+				  void *args)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct rte_i40e_fdir_entry *fdir_entry = NULL;
+	int ret = I40E_SUCCESS;
+
+	switch (cmd) {
+	case RTE_CMD_FDIR_RULE_ADD:
+		if (args == NULL)
+			return I40E_ERR_PARAM;
+		fdir_entry = (struct rte_i40e_fdir_entry *)args;
+		ret = i40e_fdir_filter_programming(pf,
+			fdir_entry->soft_id,
+			&fdir_entry->input,
+			&fdir_entry->action,
+			TRUE);
+		break;
+	case RTE_CMD_FDIR_RULE_DEL:
+		if (args == NULL)
+			return I40E_ERR_PARAM;
+		fdir_entry = (struct rte_i40e_fdir_entry *)args;
+		ret = i40e_fdir_filter_programming(pf,
+			fdir_entry->soft_id,
+			&fdir_entry->input,
+			&fdir_entry->action,
+			FALSE);
+		break;
+	default:
+		PMD_DRV_LOG(ERR, "unknown command type %u\n", cmd);
+		ret = I40E_ERR_PARAM;
+		break;
+	}
+	return ret;
+}
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.h b/lib/librte_pmd_i40e/i40e_ethdev.h
index c2c7fa9..5edb99e 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.h
+++ b/lib/librte_pmd_i40e/i40e_ethdev.h
@@ -34,6 +34,8 @@
 #ifndef _I40E_ETHDEV_H_
 #define _I40E_ETHDEV_H_
 
+#include "rte_i40e.h"
+
 #define I40E_AQ_LEN               32
 #define I40E_AQ_BUF_SZ            4096
 /* Number of queues per TC should be one of 1, 2, 4, 8, 16, 32, 64 */
@@ -332,6 +334,11 @@ i40e_fdir_setup_rx_resources(struct i40e_pf *pf,
 				    unsigned int socket_id);
 int i40e_fdir_setup(struct i40e_pf *pf);
 void i40e_fdir_teardown(struct i40e_pf *pf);
+int i40e_fdir_filter_programming(struct i40e_pf *pf,
+			uint16_t soft_id,
+			struct rte_i40e_fdir_input *fdir_filter,
+			struct rte_i40e_fdir_action *fdir_action,
+			bool add);
 
 /* I40E_DEV_PRIVATE_TO */
 #define I40E_DEV_PRIVATE_TO_PF(adapter) \
diff --git a/lib/librte_pmd_i40e/i40e_fdir.c b/lib/librte_pmd_i40e/i40e_fdir.c
index f6297a8..df9a889 100644
--- a/lib/librte_pmd_i40e/i40e_fdir.c
+++ b/lib/librte_pmd_i40e/i40e_fdir.c
@@ -48,11 +48,21 @@
 #include "i40e/i40e_type.h"
 #include "i40e_ethdev.h"
 #include "i40e_rxtx.h"
+/* Wait count and inteval for fdir filter programming */
+#define I40E_FDIR_WAIT_COUNT       10
+#define I40E_FDIR_WAIT_INTERVAL_US 1000
 
 #define I40E_COUNTER_PF           2
 /* Statistic counter index for one pf */
 #define I40E_COUNTER_INDEX_FDIR(pf_id)   (0 + (pf_id) * I40E_COUNTER_PF)
 
+#ifndef RTE_MBUF_DATA_DMA_ADDR
+#define RTE_MBUF_DATA_DMA_ADDR(mb) \
+	((uint64_t)((mb)->buf_physaddr + \
+	(uint64_t)((char *)((mb)->pkt.data) - \
+	(char *)(mb)->buf_addr)))
+#endif
+
 static int
 i40e_fdir_rx_queue_init(struct i40e_rx_queue *rxq)
 {
@@ -206,3 +216,218 @@ i40e_fdir_teardown(struct i40e_pf *pf)
 	pf->fdir.fdir_vsi = NULL;
 	return;
 }
+
+/* Construct the tx flags */
+static inline uint64_t
+i40e_build_ctob(uint32_t td_cmd,
+		uint32_t td_offset,
+		unsigned int size,
+		uint32_t td_tag)
+{
+	return rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DATA |
+			((uint64_t)td_cmd  << I40E_TXD_QW1_CMD_SHIFT) |
+			((uint64_t)td_offset << I40E_TXD_QW1_OFFSET_SHIFT) |
+			((uint64_t)size  << I40E_TXD_QW1_TX_BUF_SZ_SHIFT) |
+			((uint64_t)td_tag  << I40E_TXD_QW1_L2TAG1_SHIFT));
+}
+
+/*
+ * check the programming status descriptor in rx queue.
+ * done after Programming Flow Director is programmed on
+ * tx queue
+ */
+static inline int
+i40e_check_fdir_programming_status(struct i40e_rx_queue *rxq)
+{
+	volatile union i40e_rx_desc *rxdp;
+	uint64_t qword1;
+	uint32_t rx_status;
+	uint32_t len, id;
+	uint32_t error;
+	int ret = 0;
+
+	rxdp = &rxq->rx_ring[rxq->rx_tail];
+	qword1 = rte_le_to_cpu_64(rxdp->wb.qword1.status_error_len);
+	rx_status = (qword1 & I40E_RXD_QW1_STATUS_MASK)
+			>> I40E_RXD_QW1_STATUS_SHIFT;
+
+	if (rx_status & (1 << I40E_RX_DESC_STATUS_DD_SHIFT)) {
+		len = qword1 >> I40E_RX_PROG_STATUS_DESC_LENGTH_SHIFT;
+		id = (qword1 & I40E_RX_PROG_STATUS_DESC_QW1_PROGID_MASK) >>
+			    I40E_RX_PROG_STATUS_DESC_QW1_PROGID_SHIFT;
+
+		if (len  == I40E_RX_PROG_STATUS_DESC_LENGTH &&
+		    id == I40E_RX_PROG_STATUS_DESC_FD_FILTER_STATUS) {
+			error = (qword1 &
+				I40E_RX_PROG_STATUS_DESC_QW1_ERROR_MASK) >>
+				I40E_RX_PROG_STATUS_DESC_QW1_ERROR_SHIFT;
+			if (error == (0x1 <<
+				I40E_RX_PROG_STATUS_DESC_FD_TBL_FULL_SHIFT)) {
+				PMD_DRV_LOG(ERR, "Failed to add FDIR filter"
+					    " (FD_ID %u): programming status"
+					    " reported\n",
+					    rxdp->wb.qword0.hi_dword.fd_id);
+				ret = -1;
+			} else if (error == (0x1 <<
+				I40E_RX_PROG_STATUS_DESC_NO_FD_ENTRY_SHIFT)) {
+				PMD_DRV_LOG(ERR, "Failed to delete FDIR filter"
+					    " (FD_ID %u): programming status"
+					    " reported\n",
+					    rxdp->wb.qword0.hi_dword.fd_id);
+				ret = -1;
+			} else
+				PMD_DRV_LOG(ERR, "invalid programming status"
+					    " reported, error = %u\n", error);
+		} else
+			PMD_DRV_LOG(ERR, "unknown programming status"
+				    " reported,len = %d, id = %u\n", len, id);
+		rxdp->wb.qword1.status_error_len = 0;
+		rxq->rx_tail++;
+		if (unlikely(rxq->rx_tail == rxq->nb_rx_desc))
+			rxq->rx_tail = 0;
+	}
+	return ret;
+}
+
+/*
+ * Program a flow diretor filter rule.
+ * Is done by Flow Director Programming
+ * Descriptor  followed by packet structure that contains the filter fields
+ * need to match.
+ */
+int
+i40e_fdir_filter_programming(struct i40e_pf *pf,
+			uint16_t soft_id,
+			struct rte_i40e_fdir_input *fdir_input,
+			struct rte_i40e_fdir_action *fdir_action,
+			bool add)
+{
+	struct i40e_tx_queue *txq = pf->fdir.txq;
+	struct i40e_rx_queue *rxq = pf->fdir.rxq;
+	volatile struct i40e_tx_desc *txdp;
+	struct rte_mbuf *mbuf = fdir_input->data;
+	volatile struct i40e_filter_program_desc *fdirdp;
+	uint64_t dma_addr;
+	uint32_t td_cmd;
+	uint16_t i;
+	uint8_t dest;
+
+	if (!(pf->flags & I40E_FLAG_FDIR)) {
+		PMD_DRV_LOG(ERR, "unsupported operation,"
+				 "FDIR is not enabled.\n");
+		return I40E_NOT_SUPPORTED;
+	}
+
+	PMD_DRV_LOG(INFO, "filling filter prgramming descriptor\n");
+	fdirdp = (volatile struct i40e_filter_program_desc *)
+			(&(txq->tx_ring[txq->tx_tail]));
+
+	fdirdp->qindex_flex_ptype_vsi =
+			rte_cpu_to_le_32((fdir_action->rx_queue <<
+					  I40E_TXD_FLTR_QW0_QINDEX_SHIFT) &
+					  I40E_TXD_FLTR_QW0_QINDEX_MASK);
+
+	fdirdp->qindex_flex_ptype_vsi |=
+			rte_cpu_to_le_32((fdir_input->flex_off <<
+					  I40E_TXD_FLTR_QW0_FLEXOFF_SHIFT) &
+					  I40E_TXD_FLTR_QW0_FLEXOFF_MASK);
+
+	fdirdp->qindex_flex_ptype_vsi |=
+			rte_cpu_to_le_32((fdir_input->pctype <<
+					  I40E_TXD_FLTR_QW0_PCTYPE_SHIFT) &
+					  I40E_TXD_FLTR_QW0_PCTYPE_MASK);
+
+	/* Use LAN VSI Id if not programmed by user */
+	if (fdir_input->dest_vsi == 0)
+		fdirdp->qindex_flex_ptype_vsi |=
+			rte_cpu_to_le_32((pf->main_vsi->vsi_id <<
+					  I40E_TXD_FLTR_QW0_DEST_VSI_SHIFT) &
+					  I40E_TXD_FLTR_QW0_DEST_VSI_MASK);
+	else
+		fdirdp->qindex_flex_ptype_vsi |=
+			rte_cpu_to_le_32((fdir_input->dest_vsi <<
+					  I40E_TXD_FLTR_QW0_DEST_VSI_SHIFT) &
+					  I40E_TXD_FLTR_QW0_DEST_VSI_MASK);
+
+	fdirdp->dtype_cmd_cntindex =
+			rte_cpu_to_le_32(I40E_TX_DESC_DTYPE_FILTER_PROG);
+
+	if (add)
+		fdirdp->dtype_cmd_cntindex |= rte_cpu_to_le_32(
+				I40E_FILTER_PROGRAM_DESC_PCMD_ADD_UPDATE <<
+				I40E_TXD_FLTR_QW1_PCMD_SHIFT);
+	else
+		fdirdp->dtype_cmd_cntindex |= rte_cpu_to_le_32(
+				I40E_FILTER_PROGRAM_DESC_PCMD_REMOVE <<
+				I40E_TXD_FLTR_QW1_PCMD_SHIFT);
+
+	if (fdir_action->drop == RTE_I40E_DEST_DROP_PACKET)
+		dest = I40E_FILTER_PROGRAM_DESC_DEST_DROP_PACKET;
+	else
+		dest = I40E_FILTER_PROGRAM_DESC_DEST_DIRECT_PACKET_QINDEX;
+	fdirdp->dtype_cmd_cntindex |= rte_cpu_to_le_32((dest <<
+				I40E_TXD_FLTR_QW1_DEST_SHIFT) &
+				I40E_TXD_FLTR_QW1_DEST_MASK);
+
+	fdirdp->dtype_cmd_cntindex |=
+		rte_cpu_to_le_32((fdir_action->report_status<<
+				I40E_TXD_FLTR_QW1_FD_STATUS_SHIFT) &
+				I40E_TXD_FLTR_QW1_FD_STATUS_MASK);
+
+	fdirdp->dtype_cmd_cntindex |=
+			rte_cpu_to_le_32(I40E_TXD_FLTR_QW1_CNT_ENA_MASK);
+	if (fdir_action->cnt_index != 0)
+		fdirdp->dtype_cmd_cntindex |=
+				rte_cpu_to_le_32((fdir_action->cnt_index <<
+				I40E_TXD_FLTR_QW1_CNTINDEX_SHIFT) &
+				I40E_TXD_FLTR_QW1_CNTINDEX_MASK);
+	else
+		fdirdp->dtype_cmd_cntindex |=
+				rte_cpu_to_le_32((pf->fdir.match_counter_index <<
+				I40E_TXD_FLTR_QW1_CNTINDEX_SHIFT) &
+				I40E_TXD_FLTR_QW1_CNTINDEX_MASK);
+
+	fdirdp->fd_id = rte_cpu_to_le_32(soft_id);
+	txq->tx_tail++;
+	if (txq->tx_tail >= txq->nb_tx_desc)
+		txq->tx_tail = 0;
+
+	PMD_DRV_LOG(INFO, "filling transmit descriptor\n");
+	txdp = &(txq->tx_ring[txq->tx_tail]);
+	dma_addr = RTE_MBUF_DATA_DMA_ADDR(mbuf);
+	txdp->buffer_addr = rte_cpu_to_le_64(dma_addr);
+	td_cmd = I40E_TX_DESC_CMD_EOP |
+		 I40E_TX_DESC_CMD_RS  |
+		 I40E_TX_DESC_CMD_DUMMY;
+
+	txdp->cmd_type_offset_bsz =
+		i40e_build_ctob(td_cmd, 0,
+				mbuf->pkt.data_len, 0);
+
+	txq->tx_tail++;
+	if (txq->tx_tail >= txq->nb_tx_desc)
+		txq->tx_tail = 0;
+	/* Update the tx tail register */
+	rte_wmb();
+	I40E_PCI_REG_WRITE(txq->qtx_tail, txq->tx_tail);
+
+	for (i = 0; i < I40E_FDIR_WAIT_COUNT; i++) {
+		rte_delay_us(I40E_FDIR_WAIT_INTERVAL_US);
+		if (txdp->cmd_type_offset_bsz &
+				rte_cpu_to_le_64(I40E_TX_DESC_DTYPE_DESC_DONE))
+			break;
+	}
+	if (i >= I40E_FDIR_WAIT_COUNT) {
+		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
+			    " timeout to get DD on tx queue\n");
+		return I40E_ERR_TIMEOUT;
+	}
+	/* totally delay 10 ms to check programming status*/
+	rte_delay_us((I40E_FDIR_WAIT_COUNT - i) * I40E_FDIR_WAIT_INTERVAL_US);
+	if (i40e_check_fdir_programming_status(rxq) < 0) {
+		PMD_DRV_LOG(ERR, "Failed to program FDIR filter:"
+			    " programming status reported\n");
+		return I40E_ERR_CONFIG;
+	}
+	return I40E_SUCCESS;
+}
diff --git a/lib/librte_pmd_i40e/rte_i40e.h b/lib/librte_pmd_i40e/rte_i40e.h
new file mode 100644
index 0000000..3d9cc3d
--- /dev/null
+++ b/lib/librte_pmd_i40e/rte_i40e.h
@@ -0,0 +1,111 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of Intel Corporation nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_I40E_H_
+#define _RTE_I40E_H_
+
+/**
+ * @file
+ *
+ * RTE I40E
+ *
+ * The I40E defines the commands and structures specifically for i40e hardware
+ * features. As different types of NIC hardware may have different features,
+ * they might not be common for all types of NIC hardwares. The commands and
+ * structures can be used in applications directly together with generalized
+ * APIs declared in rte_ethdev.h. The commands couldn't be supported by
+ * non-i40e PMD.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#define I40E_FDIR_PKT_LEN                   512
+#define I40E_FDIR_IP_DEFAULT_LEN            0x003C
+#define I40E_FDIR_IP_DEFAULT_TTL            0x40
+#define I40E_FDIR_IP_DEFAULT_VERSION_IHL    0x45
+#define I40E_FDIR_TCP_DEFAULT_DATAOFF       0x50
+#define I40E_FDIR_IPv6_DEFAULT_VTC_FLOW     0x60300000
+#define I40E_FDIR_IPv6_DEFAULT_PAYLOAD_LEN  0x0014
+#define I40E_FDIR_IPv6_DEFAULT_HOP_LIMITS   0xFF
+#define I40E_FDIR_UDP_DEFAULT_LEN           0x0028
+
+enum rte_i40e_fdir_status {
+	RTE_I40E_FDIR_NO_REPORT_STATUS = 0, /**< no report FDIR. */
+	RTE_I40E_FDIR_REPORT_FD_ID,         /**< only report FD ID. */
+	RTE_I40E_FDIR_REPORT_FD_ID_FLEX_4,  /**< report FD ID and 4 flex bytes. */
+	RTE_I40E_FDIR_REPORT_FLEX_8,        /**< report 8 flex bytes. */
+};
+
+#define RTE_I40E_DEST_DROP_PACKET          0x01
+#define RTE_I40E_DEST_DIRECT_PACKET_QINDEX 0x02
+
+/**
+ * A structure used to define the input for an flow director filter entry
+ */
+struct rte_i40e_fdir_input {
+	uint8_t  pctype;
+	uint8_t  flex_off;
+	uint16_t dest_vsi;      /**< destination VSI ID*/
+	struct rte_mbuf *data;  /**< mbuf store raw packet used to program */
+};
+
+/**
+ * A structure used to define an action when match FDIR packet filter.
+ */
+struct rte_i40e_fdir_action {
+	uint16_t rx_queue;        /**< queue assigned to if fdir match. */
+	uint16_t cnt_index;       /**< statistic count index */
+	uint8_t  drop;            /**< accept or reject */
+	enum rte_i40e_fdir_status report_status;  /**< status report. */
+};
+
+/**
+ * For commands:
+ * 'RTE_CMD_FDIR_RULE_ADD'
+ * 'RTE_CMD_FDIR_RULE_DEL'
+ *
+ * A structure used to define the flow director filter entry
+ */
+struct rte_i40e_fdir_entry {
+	uint16_t soft_id;                   /**< id */
+	struct rte_i40e_fdir_input input;   /**< input set */
+	struct rte_i40e_fdir_action action; /**< action taken when match */
+};
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_I40E_H_ */
-- 
1.8.1.4

Re: [dpdk-dev] [PATCH v2 3/7] i40e: function implement in i40e for flow director filter programming

[Thomas Monjalon]

2014-08-27 10:13, Jingjing Wu:
> support the API ops defined in ethdev, the behavior according to each command:
>   RTE_CMD_FDIR_RULE_ADD: add a new FDIR filter rule.
>   RTE_CMD_FDIR_RULE_DEL: delete a FDIR filter rule.

Here title is really too complicated. Be concise.
I'd change it to:
	i40e: flow director filter

-- 
Thomas

Re: [dpdk-dev] [PATCH v2 3/7] i40e: function implement in i40e for flow director filter programming

[Wu, Jingjing]

Hi, Thomas

I will rework on it.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, August 27, 2014 10:25 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 3/7] i40e: function implement in i40e for flow director
> filter programming
> 
> 2014-08-27 10:13, Jingjing Wu:
> > support the API ops defined in ethdev, the behavior according to each command:
> >   RTE_CMD_FDIR_RULE_ADD: add a new FDIR filter rule.
> >   RTE_CMD_FDIR_RULE_DEL: delete a FDIR filter rule.
> 
> Here title is really too complicated. Be concise.
> I'd change it to:
> 	i40e: flow director filter
> 
> --
> Thomas

[dpdk-dev] [PATCH v2 4/7] i40e: function implement in i40e for flow director flush and info get

[Jingjing Wu]

support the API ops defined in ethdev, the behavior according to each command:
  RTE_CMD_FDIR_FLUSH   : clear all FDIR filter rules.
  RTE_CMD_FDIR_INFO_GET: get FDIR information.
 
Signed-off-by: jingjing.wu <jingjing.wu@intel.com>
Reviewed-by: Helin Zhang <helin.zhang@intel.com>
Reviewed-by: Jing Chen <jing.d.chen@intel.com>
Reviewed-by: Jijiang Liu <jijiang.liu@intel.com>
 
---
 lib/librte_pmd_i40e/i40e_ethdev.c |  8 +++++
 lib/librte_pmd_i40e/i40e_ethdev.h |  3 ++
 lib/librte_pmd_i40e/i40e_fdir.c   | 67 +++++++++++++++++++++++++++++++++++++++
 lib/librte_pmd_i40e/rte_i40e.h    | 14 ++++++++
 4 files changed, 92 insertions(+)

diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 7dcf964..10797ba 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -4221,6 +4221,14 @@ i40e_rx_classification_filter_ctl(struct rte_eth_dev *dev,
 			&fdir_entry->action,
 			FALSE);
 		break;
+	case RTE_CMD_FDIR_INFO_GET:
+		if (args == NULL)
+			return I40E_ERR_PARAM;
+		i40e_fdir_info_get(dev, (struct rte_i40e_fdir_info *)args);
+		break;
+	case RTE_CMD_FDIR_FLUSH:
+		ret = i40e_fdir_flush(pf);
+		break;
 	default:
 		PMD_DRV_LOG(ERR, "unknown command type %u\n", cmd);
 		ret = I40E_ERR_PARAM;
diff --git a/lib/librte_pmd_i40e/i40e_ethdev.h b/lib/librte_pmd_i40e/i40e_ethdev.h
index 5edb99e..7755f5a 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.h
+++ b/lib/librte_pmd_i40e/i40e_ethdev.h
@@ -334,11 +334,14 @@ i40e_fdir_setup_rx_resources(struct i40e_pf *pf,
 				    unsigned int socket_id);
 int i40e_fdir_setup(struct i40e_pf *pf);
 void i40e_fdir_teardown(struct i40e_pf *pf);
+int i40e_fdir_flush(struct i40e_pf *pf);
 int i40e_fdir_filter_programming(struct i40e_pf *pf,
 			uint16_t soft_id,
 			struct rte_i40e_fdir_input *fdir_filter,
 			struct rte_i40e_fdir_action *fdir_action,
 			bool add);
+void i40e_fdir_info_get(struct rte_eth_dev *dev,
+			   struct rte_i40e_fdir_info *fdir);
 
 /* I40E_DEV_PRIVATE_TO */
 #define I40E_DEV_PRIVATE_TO_PF(adapter) \
diff --git a/lib/librte_pmd_i40e/i40e_fdir.c b/lib/librte_pmd_i40e/i40e_fdir.c
index df9a889..39e9e88 100644
--- a/lib/librte_pmd_i40e/i40e_fdir.c
+++ b/lib/librte_pmd_i40e/i40e_fdir.c
@@ -51,6 +51,9 @@
 /* Wait count and inteval for fdir filter programming */
 #define I40E_FDIR_WAIT_COUNT       10
 #define I40E_FDIR_WAIT_INTERVAL_US 1000
+/* Wait count and inteval for fdir filter flush */
+#define I40E_FDIR_FLUSH_RETRY       50
+#define I40E_FDIR_FLUSH_INTERVAL_MS 5
 
 #define I40E_COUNTER_PF           2
 /* Statistic counter index for one pf */
@@ -217,6 +220,45 @@ i40e_fdir_teardown(struct i40e_pf *pf)
 	return;
 }
 
+/*
+ * i40e_fdir_flush - clear all filters of Flow Director
+ * @pf: board private structure
+ */
+int
+i40e_fdir_flush(struct i40e_pf *pf)
+{
+	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
+	uint32_t reg;
+	uint16_t guarant_cnt, best_cnt;
+	int i;
+
+	I40E_WRITE_REG(hw, I40E_PFQF_CTL_1, I40E_PFQF_CTL_1_CLEARFDTABLE_MASK);
+	I40E_WRITE_FLUSH(hw);
+
+	for (i = 0; i < I40E_FDIR_FLUSH_RETRY; i++) {
+		rte_delay_ms(I40E_FDIR_FLUSH_INTERVAL_MS);
+		reg = I40E_READ_REG(hw, I40E_PFQF_CTL_1);
+		if (!(reg & I40E_PFQF_CTL_1_CLEARFDTABLE_MASK))
+			break;
+	}
+	if (i >= I40E_FDIR_FLUSH_RETRY) {
+		PMD_DRV_LOG(ERR, "FD table did not flush, may need more time\n");
+		return I40E_ERR_TIMEOUT;
+	}
+	guarant_cnt = (uint16_t)((I40E_READ_REG(hw, I40E_PFQF_FDSTAT) &
+				I40E_PFQF_FDSTAT_GUARANT_CNT_MASK) >>
+				I40E_PFQF_FDSTAT_GUARANT_CNT_SHIFT);
+	best_cnt = (uint16_t)((I40E_READ_REG(hw, I40E_PFQF_FDSTAT) &
+				I40E_PFQF_FDSTAT_BEST_CNT_MASK) >>
+				I40E_PFQF_FDSTAT_BEST_CNT_SHIFT);
+	if (guarant_cnt != 0 || best_cnt != 0) {
+		PMD_DRV_LOG(ERR, "Failed to flush FD table\n");
+		return I40E_ERR_CONFIG;
+	} else
+		PMD_DRV_LOG(INFO, "FD table Flush success\n");
+	return I40E_SUCCESS;
+}
+
 /* Construct the tx flags */
 static inline uint64_t
 i40e_build_ctob(uint32_t td_cmd,
@@ -431,3 +473,28 @@ i40e_fdir_filter_programming(struct i40e_pf *pf,
 	}
 	return I40E_SUCCESS;
 }
+
+/*
+ * i40e_fdir_info_get - get information of Flow Director
+ * @dev: ethernet device to add filter to
+ * @fdir: a pointer to a structure of type *rte_eth_dev_fdir* to be filled with
+ *    the flow director information.
+ **/
+void
+i40e_fdir_info_get(struct rte_eth_dev *dev, struct rte_i40e_fdir_info *fdir)
+{
+	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	uint32_t pfqf_ctl;
+
+	pfqf_ctl = I40E_READ_REG(hw, I40E_PFQF_CTL_0);
+	fdir->mode = pfqf_ctl & I40E_PFQF_CTL_0_FD_ENA_MASK ? 1 : 0;
+	fdir->guarant_spc = (uint16_t)hw->func_caps.fd_filters_guaranteed;
+	fdir->guarant_cnt = (uint16_t)((I40E_READ_REG(hw, I40E_PFQF_FDSTAT) &
+				I40E_PFQF_FDSTAT_GUARANT_CNT_MASK) >>
+				I40E_PFQF_FDSTAT_GUARANT_CNT_SHIFT);
+	fdir->best_spc = (uint16_t)hw->func_caps.fd_filters_best_effort;
+	fdir->best_cnt = (uint16_t)((I40E_READ_REG(hw, I40E_PFQF_FDSTAT) &
+				I40E_PFQF_FDSTAT_BEST_CNT_MASK) >>
+				I40E_PFQF_FDSTAT_BEST_CNT_SHIFT);
+	return;
+}
diff --git a/lib/librte_pmd_i40e/rte_i40e.h b/lib/librte_pmd_i40e/rte_i40e.h
index 3d9cc3d..1176a37 100644
--- a/lib/librte_pmd_i40e/rte_i40e.h
+++ b/lib/librte_pmd_i40e/rte_i40e.h
@@ -104,6 +104,20 @@ struct rte_i40e_fdir_entry {
 	struct rte_i40e_fdir_action action; /**< action taken when match */
 };
 
+/**
+ * For commands:
+ * 'RTE_CMD_FDIR_INFO_GET'
+ *
+ * A structure used for user to get the information of fdir feature.
+ */
+struct rte_i40e_fdir_info {
+	uint8_t  mode;         /**< 0 is disable, 1 is enable. */
+	uint16_t guarant_spc;  /**< guaranteed spaces.*/
+	uint16_t guarant_cnt;  /**< Number of filters in guaranteed spaces. */
+	uint16_t best_spc;     /**< best effort spaces.*/
+	uint16_t best_cnt;     /**< Number of filters in best effort spaces. */
+};
+
 #ifdef __cplusplus
 }
 #endif
-- 
1.8.1.4

[dpdk-dev] [PATCH v2 5/7] fix the Marco conflict

[Jingjing Wu]

fix the Marco conflict between rte_ip.h and netinet/in.h
 
Signed-off-by: jingjing.wu <jingjing.wu@intel.com>
Reviewed-by: Helin Zhang <helin.zhang@intel.com>
Reviewed-by: Jing Chen <jing.d.chen@intel.com>
Reviewed-by: Jijiang Liu <jijiang.liu@intel.com>
 
---
 lib/librte_net/rte_ip.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/librte_net/rte_ip.h b/lib/librte_net/rte_ip.h
index e3f65c1..67b6c2d 100644
--- a/lib/librte_net/rte_ip.h
+++ b/lib/librte_net/rte_ip.h
@@ -116,6 +116,8 @@ struct ipv4_hdr {
 
 #define	IPV4_HDR_OFFSET_UNITS	8
 
+#ifndef _NETINET_IN_H
+#ifndef _NETINET_IN_H_
 /* IPv4 protocols */
 #define IPPROTO_IP         0  /**< dummy for IP */
 #define IPPROTO_HOPOPTS    0  /**< IP6 hop-by-hop options */
@@ -226,7 +228,8 @@ struct ipv4_hdr {
 #define IPPROTO_DIVERT   254  /**< divert pseudo-protocol */
 #define IPPROTO_RAW      255  /**< raw IP packet */
 #define IPPROTO_MAX      256  /**< maximum protocol number */
-
+#endif /* _NETINET_IN_H_ */
+#endif /* _NETINET_IN_H */
 /*
  * IPv4 address types
  */
-- 
1.8.1.4

Re: [dpdk-dev] [PATCH v2 5/7] fix the Marco conflict

[Thomas Monjalon]

2014-08-27 10:13, Jingjing Wu:
> fix the Marco conflict between rte_ip.h and netinet/in.h

Who is Marco?

> +#ifndef _NETINET_IN_H
> +#ifndef _NETINET_IN_H_
>  /* IPv4 protocols */
>  #define IPPROTO_IP         0  /**< dummy for IP */
>  #define IPPROTO_HOPOPTS    0  /**< IP6 hop-by-hop options */
> @@ -226,7 +228,8 @@ struct ipv4_hdr {
>  #define IPPROTO_DIVERT   254  /**< divert pseudo-protocol */
>  #define IPPROTO_RAW      255  /**< raw IP packet */
>  #define IPPROTO_MAX      256  /**< maximum protocol number */
> -
> +#endif /* _NETINET_IN_H_ */
> +#endif /* _NETINET_IN_H */

Please explain your "fix" (which seems to be a workaround).

-- 
Thomas

Re: [dpdk-dev] [PATCH v2 5/7] fix the Marco conflict

[Wu, Jingjing]

Hi, Thomas

Because these macros such as IPPROTO_TCP, IPPROTO_UDP are already defined in <netinet/in.h>. If user's application include <netinet/in.h> and rte_ip.h at the same time, there will be conflict error, for example cmdline.c in testpmd.
I remember there was someone also raised this issue few month ago.
So just use the way "#ifndef #endif" to avoid the conflict. And it is exactly workaround as you said.

Oh, it should be macro, but not marco, my spelling mistake.
Sorry for that. I will rename this.


> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, August 27, 2014 10:28 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 5/7] fix the Marco conflict
> 
> 2014-08-27 10:13, Jingjing Wu:
> > fix the Marco conflict between rte_ip.h and netinet/in.h
> 
> Who is Marco?
> 
> > +#ifndef _NETINET_IN_H
> > +#ifndef _NETINET_IN_H_
> >  /* IPv4 protocols */
> >  #define IPPROTO_IP         0  /**< dummy for IP */
> >  #define IPPROTO_HOPOPTS    0  /**< IP6 hop-by-hop options */
> > @@ -226,7 +228,8 @@ struct ipv4_hdr {
> >  #define IPPROTO_DIVERT   254  /**< divert pseudo-protocol */
> >  #define IPPROTO_RAW      255  /**< raw IP packet */
> >  #define IPPROTO_MAX      256  /**< maximum protocol number */
> > -
> > +#endif /* _NETINET_IN_H_ */
> > +#endif /* _NETINET_IN_H */
> 
> Please explain your "fix" (which seems to be a workaround).
> 
> --
> Thomas

Re: [dpdk-dev] [PATCH v2 5/7] fix the Marco conflict

[Thomas Monjalon]

2014-08-28 03:39, Wu, Jingjing:
> Because these macros such as IPPROTO_TCP, IPPROTO_UDP are already
> defined in <netinet/in.h>. If user's application include <netinet/in.h>
> and rte_ip.h at the same time, there will be conflict error, for
> example cmdline.c in testpmd.

Yes

> I remember there was someone also raised this issue few month ago.

Yes, and the question was: "should we totally remove these definitions"?
I think yes.

> So just use the way "#ifndef #endif" to avoid the conflict.

But you didn't explain difference between _NETINET_IN_H and _NETINET_IN_H_.

> And it is exactly workaround as you said.

Yes, it's a workaround.
If rte_ip.h is included before netinet/in.h, I think there is still a problem.
 
-- 
Thomas

Re: [dpdk-dev] [PATCH v2 5/7] fix the Marco conflict

[Wu, Jingjing]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, August 28, 2014 4:56 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 5/7] fix the Marco conflict
> 
> 2014-08-28 03:39, Wu, Jingjing:
> > Because these macros such as IPPROTO_TCP, IPPROTO_UDP are already
> > defined in <netinet/in.h>. If user's application include <netinet/in.h>
> > and rte_ip.h at the same time, there will be conflict error, for
> > example cmdline.c in testpmd.
> 
> Yes
> 
> > I remember there was someone also raised this issue few month ago.
> 
> Yes, and the question was: "should we totally remove these definitions"?
> I think yes.
> 
Yes, it will be clear. But it also provides a way to user who doesn't use netinet/in.h.

> > So just use the way "#ifndef #endif" to avoid the conflict.
> 
> But you didn't explain difference between _NETINET_IN_H and _NETINET_IN_H_.

It is due to the different versions of in.h, some use _NETINET_IN_H_ to define the
head file, while some use _NETINET_IN_H
> 
> > And it is exactly workaround as you said.
> 
> Yes, it's a workaround.
> If rte_ip.h is included before netinet/in.h, I think there is still a problem.

Yes, it's just workaround.
As my understanding, in DPDK's source code
System head files includes first should be as following. So I think it's OK then.
> 
> --
> Thomas

Can I send a separate patch for this? Because it has no strict relationship with flow director.

Thanks 
Jingjing

Re: [dpdk-dev] [PATCH v2 5/7] fix the Marco conflict

[Thomas Monjalon]

2014-08-28 14:37, Wu, Jingjing:
> Can I send a separate patch for this?
> Because it has no strict relationship with flow director.

Yes, please.
But I think you should look at the removal option (no redefinition
of at all).

Thanks
-- 
Thomas

[dpdk-dev] [PATCH v2 6/7] i40e: support FD ID report and match counter for i40e flow director

[Jingjing Wu]

support to get the fdir_match counter
support to set the FDIR flag and FD_ID reported in mbuf
 
Signed-off-by: jingjing.wu <jingjing.wu@intel.com>
Reviewed-by: Helin Zhang <helin.zhang@intel.com>
Reviewed-by: Jing Chen <jing.d.chen@intel.com>
Reviewed-by: Jijiang Liu <jijiang.liu@intel.com>
 
---
 lib/librte_pmd_i40e/i40e_ethdev.c |  5 ++++
 lib/librte_pmd_i40e/i40e_rxtx.c   | 49 ++++++++++++++++++++++++++++++++++++++-
 2 files changed, 53 insertions(+), 1 deletion(-)

diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e_ethdev.c
index 10797ba..89b7eb7 100644
--- a/lib/librte_pmd_i40e/i40e_ethdev.c
+++ b/lib/librte_pmd_i40e/i40e_ethdev.c
@@ -1273,6 +1273,9 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 			    I40E_GLPRT_PTC9522L(hw->port),
 			    pf->offset_loaded, &os->tx_size_big,
 			    &ns->tx_size_big);
+	i40e_stat_update_32(hw, I40E_GLQF_PCNT(pf->fdir.match_counter_index),
+			   pf->offset_loaded,
+			   &os->fd_sb_match, &ns->fd_sb_match);
 	/* GLPRT_MSPDC not supported */
 	/* GLPRT_XEC not supported */
 
@@ -1286,6 +1289,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	stats->obytes   = ns->eth.tx_bytes;
 	stats->oerrors  = ns->eth.tx_errors;
 	stats->imcasts  = ns->eth.rx_multicast;
+	stats->fdirmatch = ns->fd_sb_match;
 
 	if (pf->main_vsi)
 		i40e_update_vsi_stats(pf->main_vsi);
@@ -1351,6 +1355,7 @@ i40e_dev_stats_get(struct rte_eth_dev *dev, struct rte_eth_stats *stats)
 	printf("mac_short_packet_dropped: %lu\n",
 			ns->mac_short_packet_dropped);
 	printf("checksum_error:           %lu\n", ns->checksum_error);
+	printf("fdir_match:               %lu\n", ns->fd_sb_match);
 	printf("***************** PF stats end ********************\n");
 #endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
 }
diff --git a/lib/librte_pmd_i40e/i40e_rxtx.c b/lib/librte_pmd_i40e/i40e_rxtx.c
index 8bfbc8c..954385a 100644
--- a/lib/librte_pmd_i40e/i40e_rxtx.c
+++ b/lib/librte_pmd_i40e/i40e_rxtx.c
@@ -107,6 +107,10 @@ i40e_rxd_status_to_pkt_flags(uint64_t qword)
 					I40E_RX_DESC_FLTSTAT_RSS_HASH) ==
 			I40E_RX_DESC_FLTSTAT_RSS_HASH) ? PKT_RX_RSS_HASH : 0);
 
+	/* Check if FDIR Match */
+	flags |= (uint16_t)(qword & (1 << I40E_RX_DESC_STATUS_FLM_SHIFT) ?
+							PKT_RX_FDIR : 0);
+
 	return flags;
 }
 
@@ -623,7 +627,22 @@ i40e_rx_scan_hw_ring(struct i40e_rx_queue *rxq)
 			mb->ol_flags = pkt_flags;
 			if (pkt_flags & PKT_RX_RSS_HASH)
 				mb->pkt.hash.rss = rte_le_to_cpu_32(\
-					rxdp->wb.qword0.hi_dword.rss);
+					rxdp[j].wb.qword0.hi_dword.rss);
+			if (pkt_flags & PKT_RX_FDIR) {
+#ifdef RTE_LIBRTE_I40E_16BYTE_RX_DESC
+				if (((qword1 >> I40E_RX_DESC_STATUS_FLTSTAT_SHIFT) &
+						I40E_RX_DESC_FLTSTAT_RSS_HASH) ==
+						I40E_RX_DESC_FLTSTAT_RSV_FD_ID)
+					mb->pkt.hash.fdir.id = (uint16_t)
+						rte_le_to_cpu_32(rxdp[j].wb.qword0.hi_dword.fd);
+#else
+				if (((rxdp[j].wb.qword2.ext_status >>
+					I40E_RX_DESC_EXT_STATUS_FLEXBH_SHIFT) &
+					0x03) == 0x01)
+					mb->pkt.hash.fdir.id = (uint16_t)
+						rte_le_to_cpu_32(rxdp[j].wb.qword3.hi_dword.fd_id);
+#endif
+			}
 		}
 
 		for (j = 0; j < I40E_LOOK_AHEAD; j++)
@@ -861,6 +880,20 @@ i40e_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts)
 		if (pkt_flags & PKT_RX_RSS_HASH)
 			rxm->pkt.hash.rss =
 				rte_le_to_cpu_32(rxd.wb.qword0.hi_dword.rss);
+		if (pkt_flags & PKT_RX_FDIR) {
+#ifdef RTE_LIBRTE_I40E_16BYTE_RX_DESC
+			if (((qword1 >> I40E_RX_DESC_STATUS_FLTSTAT_SHIFT) &
+					I40E_RX_DESC_FLTSTAT_RSS_HASH) ==
+					I40E_RX_DESC_FLTSTAT_RSV_FD_ID)
+				rxm->pkt.hash.fdir.id = (uint16_t)
+					rte_le_to_cpu_32(rxd.wb.qword0.hi_dword.fd);
+#else
+			if (((rxd.wb.qword2.ext_status >> I40E_RX_DESC_EXT_STATUS_FLEXBH_SHIFT) &
+				0x03) == 0x01)
+				rxm->pkt.hash.fdir.id = (uint16_t)
+					rte_le_to_cpu_32(rxd.wb.qword3.hi_dword.fd_id);
+#endif
+		}
 
 		rx_pkts[nb_rx++] = rxm;
 	}
@@ -1014,6 +1047,20 @@ i40e_recv_scattered_pkts(void *rx_queue,
 		if (pkt_flags & PKT_RX_RSS_HASH)
 			rxm->pkt.hash.rss =
 				rte_le_to_cpu_32(rxd.wb.qword0.hi_dword.rss);
+		if (pkt_flags & PKT_RX_FDIR) {
+#ifdef RTE_LIBRTE_I40E_16BYTE_RX_DESC
+			if (((qword1 >> I40E_RX_DESC_STATUS_FLTSTAT_SHIFT) &
+					I40E_RX_DESC_FLTSTAT_RSS_HASH) ==
+					I40E_RX_DESC_FLTSTAT_RSV_FD_ID)
+				rxm->pkt.hash.fdir.id = (uint16_t)
+					rte_le_to_cpu_32(rxd.wb.qword0.hi_dword.fd);
+#else
+			if (((rxd.wb.qword2.ext_status >> I40E_RX_DESC_EXT_STATUS_FLEXBH_SHIFT) &
+				0x03) == 0x01)
+				rxm->pkt.hash.fdir.id = (uint16_t)
+					rte_le_to_cpu_32(rxd.wb.qword3.hi_dword.fd_id);
+#endif
+		}
 
 		/* Prefetch data of first segment, if configured to do so. */
 		rte_prefetch0(first_seg->pkt.data);
-- 
1.8.1.4

[dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support

[Jingjing Wu]

add structure definition to construct programming packet.
add commands to programming 6 flow types for the flow director filters,
which is called PCTYPE in fortville: ipv4, tcpv4, udpv4, ipv6, tcpv6, udpv6
add commands to support flushing flow director table and get info
 
Signed-off-by: jingjing.wu <jingjing.wu@intel.com>
Reviewed-by: Helin Zhang <helin.zhang@intel.com>
Reviewed-by: Jing Chen <jing.d.chen@intel.com>
Reviewed-by: Jijiang Liu <jijiang.liu@intel.com>
 
---
 app/test-pmd/cmdline.c | 665 +++++++++++++++++++++++++++++++++++++++++++++++++
 app/test-pmd/config.c  |  54 +++-
 app/test-pmd/testpmd.c |  22 ++
 app/test-pmd/testpmd.h |  57 +++++
 4 files changed, 786 insertions(+), 12 deletions(-)

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index b04a4e8..d4729e7 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -74,6 +74,14 @@
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
 #include <rte_devargs.h>
+#include <rte_ip.h>
+#include <rte_udp.h>
+#include <rte_tcp.h>
+#include <rte_sctp.h>
+#include <rte_eth_features.h>
+#ifdef RTE_LIBRTE_I40E_PMD
+#include <rte_i40e.h>
+#endif /* RTE_LIBRTE_I40E_PMD */
 
 #include <cmdline_rdline.h>
 #include <cmdline_parse.h>
@@ -660,6 +668,25 @@ static void cmd_help_long_parsed(void *parsed_result,
 
 			"get_flex_filter (port_id) index (idx)\n"
 			"    get info of a flex filter.\n\n"
+
+#ifdef RTE_LIBRTE_I40E_PMD
+			"i40e_flow_director_filter (port_id) (add|del)"
+			" flow (ip4|ip6) src (src_ip_address) dst (dst_ip_address)"
+			" flexwords (flexwords_value) (drop|fwd)"
+			" queue (queue_id) fd_id (fd_id_value)\n"
+			"    Add/Del a IP type flow director filter for i40e NIC.\n\n"
+
+			"i40e_flow_director_filter (port_id) (add|del)"
+			" flow (udp4|tcp4|udp6|tcp6)"
+			" src (src_ip_address) (src_port)"
+			" dst (dst_ip_address) (dst_port)"
+			" flexwords (flexwords_value) (drop|fwd)"
+			" queue (queue_id) fd_id (fd_id_value)\n"
+			"    Add/Del a UDP/TCP type flow director filter for i40e NIC.\n\n"
+
+			"i40e_flush_flow_diretor (port_id)\n"
+			"    Flush all flow director entries of a device on i40e NIC.\n\n"
+#endif /* RTE_LIBRTE_I40E_PMD */
 		);
 	}
 }
@@ -7403,6 +7430,639 @@ cmdline_parse_inst_t cmd_get_flex_filter = {
 	},
 };
 
+/* *** Classification Filters Control *** */
+#ifdef RTE_LIBRTE_I40E_PMD
+/* *** deal with i40e flow director filter *** */
+struct cmd_i40e_flow_director_result {
+	cmdline_fixed_string_t flow_director_filter;
+	uint8_t port_id;
+	cmdline_fixed_string_t ops;
+	cmdline_fixed_string_t flow;
+	cmdline_fixed_string_t flow_type;
+	cmdline_fixed_string_t src;
+	cmdline_ipaddr_t ip_src;
+	uint16_t port_src;
+	cmdline_fixed_string_t dst;
+	cmdline_ipaddr_t ip_dst;
+	uint16_t port_dst;
+	cmdline_fixed_string_t flexwords;
+	cmdline_fixed_string_t flexwords_value;
+	cmdline_fixed_string_t drop;
+	cmdline_fixed_string_t queue;
+	uint16_t  queue_id;
+	cmdline_fixed_string_t fd_id;
+	uint32_t  fd_id_value;
+};
+
+static inline int
+parse_flexwords(const char *q_arg, uint16_t *flexwords)
+{
+#define MAX_NUM_WORD 8
+	char s[256];
+	const char *p, *p0 = q_arg;
+	char *end;
+	unsigned long int_fld[MAX_NUM_WORD];
+	char *str_fld[MAX_NUM_WORD];
+	int i;
+	unsigned size;
+	int num_words = -1;
+
+	p = strchr(p0, '(');
+	if (p == NULL)
+		return -1;
+	++p;
+	p0 = strchr(p, ')');
+	if (p0 == NULL)
+		return -1;
+
+	size = p0 - p;
+	if (size >= sizeof(s))
+		return -1;
+
+	snprintf(s, sizeof(s), "%.*s", size, p);
+	num_words = rte_strsplit(s, sizeof(s), str_fld, MAX_NUM_WORD, ',');
+	if (num_words < 0 || num_words > MAX_NUM_WORD)
+		return -1;
+	for (i = 0; i < num_words; i++) {
+		errno = 0;
+		int_fld[i] = strtoul(str_fld[i], &end, 0);
+		if (errno != 0 || end == str_fld[i] || int_fld[i] > UINT16_MAX)
+			return -1;
+		flexwords[i] = rte_cpu_to_be_16((uint16_t)int_fld[i]);
+	}
+	return num_words;
+}
+
+static inline struct rte_mbuf *
+tx_mbuf_alloc(struct rte_mempool *mp)
+{
+	struct rte_mbuf *m;
+
+	m = __rte_mbuf_raw_alloc(mp);
+	__rte_mbuf_sanity_check_raw(m, RTE_MBUF_PKT, 0);
+	return m;
+}
+
+static inline void
+rte_i40e_fdir_construct_ip4_input(struct ipv4_other_flow *flow,
+			unsigned char *raw_pkt)
+{
+	struct ether_hdr *ether;
+	struct ipv4_hdr *ip;
+	unsigned char *payload;
+
+	ether = (struct ether_hdr *)raw_pkt;
+	ip = (struct ipv4_hdr *)(raw_pkt + sizeof(struct ether_hdr));
+	payload = raw_pkt + sizeof(struct ether_hdr) + sizeof(struct ipv4_hdr);
+
+	ether->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
+	ip->version_ihl = I40E_FDIR_IP_DEFAULT_VERSION_IHL;
+	/* set len to 46 bytes by default */
+	ip->total_length = rte_cpu_to_be_16(I40E_FDIR_IP_DEFAULT_LEN);
+	ip->time_to_live = I40E_FDIR_IP_DEFAULT_TTL;
+
+	/*
+	 * The source and destination fields in the transmitted packet need
+	 * to be presented in a reversed order with respect to the expected
+	 * received packets.
+	 */
+	ip->src_addr = flow->dst_ip;
+	ip->dst_addr = flow->src_ip;
+	(void)rte_memcpy(payload,
+			 flow->flexwords,
+			 BYTES_PER_WORD * flow->num_flexwords);
+}
+
+static inline void
+rte_i40e_fdir_construct_udp4_input(struct ipv4_udp_flow *flow,
+			unsigned char *raw_pkt)
+{
+	struct ether_hdr *ether;
+	struct ipv4_hdr *ip;
+	struct udp_hdr *udp;
+	unsigned char *payload;
+
+	ether = (struct ether_hdr *)raw_pkt;
+	ip = (struct ipv4_hdr *)(raw_pkt + sizeof(struct ether_hdr));
+	udp = (struct udp_hdr *)(raw_pkt + sizeof(struct ether_hdr) +
+			sizeof(struct ipv4_hdr));
+	payload = raw_pkt + sizeof(struct ether_hdr) +
+			sizeof(struct ipv4_hdr) + sizeof(struct udp_hdr);
+
+	ether->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
+	ip->version_ihl = I40E_FDIR_IP_DEFAULT_VERSION_IHL;
+	/* set len to by default */
+	ip->total_length = rte_cpu_to_be_16(I40E_FDIR_IP_DEFAULT_LEN);
+	ip->time_to_live = I40E_FDIR_IP_DEFAULT_TTL;
+
+	/*
+	 * The source and destination fields in the transmitted packet need
+	 * to be presented in a reversed order with respect to the expected
+	 * received packets.
+	 */
+	ip->src_addr = flow->dst_ip;
+	ip->dst_addr = flow->src_ip;
+	ip->next_proto_id = IPPROTO_UDP;
+	udp->src_port = flow->dst_port;
+	udp->dst_port = flow->src_port;
+	udp->dgram_len = rte_cpu_to_be_16(I40E_FDIR_UDP_DEFAULT_LEN);
+	(void)rte_memcpy(payload,
+			 flow->flexwords,
+			 BYTES_PER_WORD * flow->num_flexwords);
+}
+
+static inline void
+rte_i40e_fdir_construct_tcp4_input(struct ipv4_tcp_flow *flow,
+			unsigned char *raw_pkt)
+{
+	struct ether_hdr *ether;
+	struct ipv4_hdr *ip;
+	struct tcp_hdr *tcp;
+	unsigned char *payload;
+
+	ether = (struct ether_hdr *)raw_pkt;
+	ip = (struct ipv4_hdr *)(raw_pkt + sizeof(struct ether_hdr));
+	tcp = (struct tcp_hdr *)(raw_pkt + sizeof(struct ether_hdr) +
+			sizeof(struct ipv4_hdr));
+	payload = raw_pkt + sizeof(struct ether_hdr) +
+			sizeof(struct ipv4_hdr) + sizeof(struct tcp_hdr);
+
+	ether->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv4);
+	ip->version_ihl = I40E_FDIR_IP_DEFAULT_VERSION_IHL;
+	/* set len by default */
+	ip->total_length = rte_cpu_to_be_16(I40E_FDIR_IP_DEFAULT_LEN);
+	ip->time_to_live = I40E_FDIR_IP_DEFAULT_TTL;
+
+	/*
+	 * The source and destination fields in the transmitted packet need
+	 * to be presented in a reversed order with respect to the expected
+	 * received packets.
+	 */
+	ip->src_addr = flow->dst_ip;
+	ip->dst_addr = flow->src_ip;
+	ip->next_proto_id = IPPROTO_TCP;
+	tcp->src_port = flow->dst_port;
+	tcp->dst_port = flow->src_port;
+	tcp->data_off = I40E_FDIR_TCP_DEFAULT_DATAOFF;
+	(void)rte_memcpy(payload,
+			 flow->flexwords,
+			 BYTES_PER_WORD * flow->num_flexwords);
+}
+
+static inline void
+rte_i40e_fdir_construct_ip6_input(struct ipv6_other_flow *flow,
+			unsigned char *raw_pkt)
+{
+	struct ether_hdr *ether;
+	struct ipv6_hdr *ip;
+	unsigned char *payload;
+
+	ether = (struct ether_hdr *)raw_pkt;
+	ip = (struct ipv6_hdr *)(raw_pkt + sizeof(struct ether_hdr));
+	payload = raw_pkt + sizeof(struct ether_hdr) + sizeof(struct ipv6_hdr);
+
+	ether->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv6);
+	ip->vtc_flow = rte_cpu_to_be_32(I40E_FDIR_IPv6_DEFAULT_VTC_FLOW);
+	ip->payload_len = rte_cpu_to_be_16(I40E_FDIR_IPv6_DEFAULT_PAYLOAD_LEN);
+	ip->hop_limits = I40E_FDIR_IPv6_DEFAULT_HOP_LIMITS;
+
+	/*
+	 * The source and destination fields in the transmitted packet need
+	 * to be presented in a reversed order with respect to the expected
+	 * received packets.
+	 */
+	rte_memcpy(&(ip->src_addr), &(flow->dst_ip), IPV6_ADDR_LEN);
+	rte_memcpy(&(ip->dst_addr), &(flow->src_ip), IPV6_ADDR_LEN);
+	(void)rte_memcpy(payload,
+			 flow->flexwords,
+			 BYTES_PER_WORD * flow->num_flexwords);
+}
+
+static inline void
+rte_i40e_fdir_construct_udp6_input(struct ipv6_udp_flow *flow,
+			unsigned char *raw_pkt)
+{
+	struct ether_hdr *ether;
+	struct ipv6_hdr *ip;
+	struct udp_hdr *udp;
+	unsigned char *payload;
+
+	ether = (struct ether_hdr *)raw_pkt;
+	ip = (struct ipv6_hdr *)(raw_pkt + sizeof(struct ether_hdr));
+	udp = (struct udp_hdr *)(raw_pkt + sizeof(struct ether_hdr) +
+			sizeof(struct ipv6_hdr));
+	payload = raw_pkt + sizeof(struct ether_hdr) +
+			sizeof(struct ipv6_hdr) + sizeof(struct udp_hdr);
+
+	ether->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv6);
+	ip->vtc_flow = rte_cpu_to_be_32(I40E_FDIR_IPv6_DEFAULT_VTC_FLOW);
+	ip->payload_len = rte_cpu_to_be_16(I40E_FDIR_IPv6_DEFAULT_PAYLOAD_LEN);
+	ip->hop_limits = I40E_FDIR_IPv6_DEFAULT_HOP_LIMITS;
+	/*
+	 * The source and destination fields in the transmitted packet need
+	 * to be presented in a reversed order with respect to the expected
+	 * received packets.
+	 */
+	rte_memcpy(&(ip->src_addr), &(flow->dst_ip), IPV6_ADDR_LEN);
+	rte_memcpy(&(ip->dst_addr), &(flow->src_ip), IPV6_ADDR_LEN);
+	ip->proto = IPPROTO_UDP;
+	udp->src_port = flow->dst_port;
+	udp->dst_port = flow->src_port;
+	udp->dgram_len = rte_cpu_to_be_16(I40E_FDIR_UDP_DEFAULT_LEN);
+	(void)rte_memcpy(payload,
+			 flow->flexwords,
+			 BYTES_PER_WORD * flow->num_flexwords);
+}
+
+static inline void
+rte_i40e_fdir_construct_tcp6_input(struct ipv6_tcp_flow *flow,
+			unsigned char *raw_pkt)
+{
+	struct ether_hdr *ether;
+	struct ipv6_hdr *ip;
+	struct tcp_hdr *tcp;
+	unsigned char *payload;
+
+	ether = (struct ether_hdr *)raw_pkt;
+	ip = (struct ipv6_hdr *)(raw_pkt + sizeof(struct ether_hdr));
+	tcp = (struct tcp_hdr *)(raw_pkt + sizeof(struct ether_hdr) +
+			sizeof(struct ipv6_hdr));
+	payload = raw_pkt + sizeof(struct ether_hdr) +
+			sizeof(struct ipv6_hdr) + sizeof(struct tcp_hdr);
+
+	ether->ether_type = rte_cpu_to_be_16(ETHER_TYPE_IPv6);
+	ip->vtc_flow = rte_cpu_to_be_32(I40E_FDIR_IPv6_DEFAULT_VTC_FLOW);
+	ip->payload_len = rte_cpu_to_be_16(I40E_FDIR_IPv6_DEFAULT_PAYLOAD_LEN);
+	ip->hop_limits = I40E_FDIR_IPv6_DEFAULT_HOP_LIMITS;
+	/*
+	 * The source and destination fields in the transmitted packet need
+	 * to be presented in a reversed order with respect to the expected
+	 * received packets.
+	 */
+	rte_memcpy(&(ip->src_addr), &(flow->dst_ip), IPV6_ADDR_LEN);
+	rte_memcpy(&(ip->dst_addr), &(flow->src_ip), IPV6_ADDR_LEN);
+	ip->proto = IPPROTO_TCP;
+	tcp->data_off = I40E_FDIR_TCP_DEFAULT_DATAOFF;
+	tcp->src_port = flow->dst_port;
+	tcp->dst_port = flow->src_port;
+	(void)rte_memcpy(payload,
+			 flow->flexwords,
+			 BYTES_PER_WORD * flow->num_flexwords);
+}
+
+static void
+cmd_i40e_flow_director_parsed(void *parsed_result,
+			  __attribute__((unused)) struct cmdline *cl,
+			  __attribute__((unused)) void *data)
+{
+	struct cmd_i40e_flow_director_result *res = parsed_result;
+	struct rte_port *port;
+	struct rte_i40e_fdir_entry entry;
+	struct rte_mbuf *m_pkt = NULL;
+	uint16_t flexwords[8];
+	enum rte_eth_command cmd;
+	int num_flexwords;
+	int ret = 0;
+
+	memset(flexwords, 0, sizeof(flexwords));
+	memset(&entry, 0, sizeof(struct rte_i40e_fdir_entry));
+	num_flexwords = parse_flexwords(res->flexwords_value, flexwords);
+	if (num_flexwords < 0) {
+		printf("error: Cannot pase flexwords input.\n");
+		return;
+	}
+	port = &ports[res->port_id];
+	m_pkt = tx_mbuf_alloc(port->i40e_fdir_mp);
+	if (m_pkt == NULL) {
+		printf("error: Cannot malloc mbuf for fdir.\n");
+		return;
+	}
+
+	if (!strcmp(res->flow_type, "ip4")) {
+		struct ipv4_other_flow ip4_flow;
+		memset(&ip4_flow, 0, sizeof(struct ipv4_other_flow));
+		/* no need to convert, already big endian. */
+		if (res->ip_dst.family == AF_INET)
+			ip4_flow.dst_ip = res->ip_dst.addr.ipv4.s_addr;
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		if (res->ip_src.family == AF_INET)
+			ip4_flow.src_ip = res->ip_src.addr.ipv4.s_addr;
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		ip4_flow.num_flexwords = num_flexwords;
+		rte_memcpy(ip4_flow.flexwords,
+				 flexwords,
+				 BYTES_PER_WORD * num_flexwords);
+		rte_i40e_fdir_construct_ip4_input(&ip4_flow, m_pkt->pkt.data);
+		entry.input.pctype = ETH_PCTYPE_NONF_IPV4_OTHER;
+	} else if (!strcmp(res->flow_type, "udp4")) {
+		struct ipv4_udp_flow udp4_flow;
+		memset(&udp4_flow, 0, sizeof(struct ipv4_udp_flow));
+		/* no need to convert, already big endian. */
+		if (res->ip_dst.family == AF_INET)
+			udp4_flow.dst_ip = res->ip_dst.addr.ipv4.s_addr;
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		if (res->ip_src.family == AF_INET)
+			udp4_flow.src_ip = res->ip_src.addr.ipv4.s_addr;
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		/* need convert to big endian. */
+		udp4_flow.dst_port = rte_cpu_to_be_16(res->port_dst);
+		udp4_flow.src_port = rte_cpu_to_be_16(res->port_src);
+		udp4_flow.num_flexwords = num_flexwords;
+		rte_memcpy(udp4_flow.flexwords,
+				 flexwords,
+				 BYTES_PER_WORD * num_flexwords);
+		rte_i40e_fdir_construct_udp4_input(&udp4_flow, m_pkt->pkt.data);
+		entry.input.pctype = ETH_PCTYPE_NONF_IPV4_UDP;
+	} else if (!strcmp(res->flow_type, "tcp4")) {
+		struct ipv4_tcp_flow tcp4_flow;
+		memset(&tcp4_flow, 0, sizeof(struct ipv4_tcp_flow));
+		if (res->ip_dst.family == AF_INET)
+			tcp4_flow.dst_ip = res->ip_dst.addr.ipv4.s_addr;
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		if (res->ip_src.family == AF_INET)
+			tcp4_flow.src_ip = res->ip_src.addr.ipv4.s_addr;
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		/* need convert to big endian. */
+		tcp4_flow.dst_port = rte_cpu_to_be_16(res->port_dst);
+		tcp4_flow.src_port = rte_cpu_to_be_16(res->port_src);
+		tcp4_flow.num_flexwords = num_flexwords;
+		rte_memcpy(tcp4_flow.flexwords,
+				 flexwords,
+				 BYTES_PER_WORD * num_flexwords);
+		rte_i40e_fdir_construct_tcp4_input(&tcp4_flow, m_pkt->pkt.data);
+		entry.input.pctype = ETH_PCTYPE_NONF_IPV4_TCP;
+	} else if (!strcmp(res->flow_type, "ip6")) {
+		struct ipv6_other_flow ip6_flow;
+		memset(&ip6_flow, 0, sizeof(struct ipv6_other_flow));
+		if (res->ip_src.family == AF_INET6)
+			(void)rte_memcpy(&(ip6_flow.src_ip),
+					 &(res->ip_src.addr.ipv6),
+					 sizeof(struct in6_addr));
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		if (res->ip_dst.family == AF_INET6)
+			(void)rte_memcpy(&(ip6_flow.dst_ip),
+					 &(res->ip_dst.addr.ipv6),
+					 sizeof(struct in6_addr));
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		ip6_flow.num_flexwords = num_flexwords;
+		rte_memcpy(ip6_flow.flexwords,
+				 flexwords,
+				 BYTES_PER_WORD * num_flexwords);
+		rte_i40e_fdir_construct_ip6_input(&ip6_flow, m_pkt->pkt.data);
+		entry.input.pctype = ETH_PCTYPE_NONF_IPV6_OTHER;
+	} else if (!strcmp(res->flow_type, "udp6")) {
+		struct ipv6_udp_flow udp6_flow;
+		memset(&udp6_flow, 0, sizeof(struct ipv6_udp_flow));
+		if (res->ip_src.family == AF_INET6)
+			(void)rte_memcpy(&(udp6_flow.src_ip),
+					 &(res->ip_src.addr.ipv6),
+					 sizeof(struct in6_addr));
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		if (res->ip_dst.family == AF_INET6)
+			(void)rte_memcpy(&(udp6_flow.dst_ip),
+					 &(res->ip_dst.addr.ipv6),
+					 sizeof(struct in6_addr));
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		udp6_flow.dst_port = rte_cpu_to_be_16(res->port_dst);
+		udp6_flow.src_port = rte_cpu_to_be_16(res->port_src);
+		udp6_flow.num_flexwords = num_flexwords;
+		rte_memcpy(udp6_flow.flexwords,
+				 flexwords,
+				 BYTES_PER_WORD * num_flexwords);
+		rte_i40e_fdir_construct_udp6_input(&udp6_flow, m_pkt->pkt.data);
+		entry.input.pctype = ETH_PCTYPE_NONF_IPV6_UDP;
+	} else if (!strcmp(res->flow_type, "tcp6")) {
+		struct ipv6_tcp_flow tcp6_flow;
+		memset(&tcp6_flow, 0, sizeof(struct ipv6_tcp_flow));
+		if (res->ip_src.family == AF_INET6)
+			(void)rte_memcpy(&(tcp6_flow.src_ip),
+					 &(res->ip_src.addr.ipv6),
+					 sizeof(struct in6_addr));
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		if (res->ip_dst.family == AF_INET6)
+			(void)rte_memcpy(&(tcp6_flow.dst_ip),
+					 &(res->ip_dst.addr.ipv6),
+					 sizeof(struct in6_addr));
+		else {
+			printf("error paramters.\n");
+			goto pktbuf_free;
+		}
+		tcp6_flow.dst_port = rte_cpu_to_be_16(res->port_dst);
+		tcp6_flow.src_port = rte_cpu_to_be_16(res->port_src);
+		tcp6_flow.num_flexwords = num_flexwords;
+		rte_memcpy(tcp6_flow.flexwords,
+				 flexwords,
+				 BYTES_PER_WORD * num_flexwords);
+		rte_i40e_fdir_construct_tcp6_input(&tcp6_flow, m_pkt->pkt.data);
+		entry.input.pctype = ETH_PCTYPE_NONF_IPV6_TCP;
+	}
+	m_pkt->pkt.data_len = I40E_FDIR_PKT_LEN;
+	m_pkt->pkt.next = NULL;
+
+	entry.input.data = m_pkt;
+	entry.input.dest_vsi = 0; /* if set to 0, will use main vsi by default*/
+	entry.input.flex_off = 0;  /*use 0 by default*/
+	if (!strcmp(res->drop, "drop"))
+		entry.action.drop = RTE_I40E_DEST_DROP_PACKET;
+	else
+		entry.action.drop = RTE_I40E_DEST_DIRECT_PACKET_QINDEX;
+	/* set to report FD ID by default temporary*/
+	entry.action.report_status = RTE_I40E_FDIR_REPORT_FD_ID;
+	entry.action.rx_queue = res->queue_id;
+	/* use 0 by default, will set it to fdir counter per dev */
+	entry.action.cnt_index = 0;
+	entry.soft_id = res->fd_id_value;
+	if (!strcmp(res->ops, "add"))
+		cmd = RTE_CMD_FDIR_RULE_ADD;
+	else
+		cmd = RTE_CMD_FDIR_RULE_DEL;
+	ret = rte_eth_dev_rx_classification_filter_ctl(res->port_id, cmd, &entry);
+	if (ret < 0)
+		printf("i40e flow director programming error,"
+			" return code = %d \n", ret);
+pktbuf_free:
+	rte_pktmbuf_free(entry.input.data);
+}
+
+cmdline_parse_token_string_t cmd_i40e_flow_director_filter =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 flow_director_filter, "i40e_flow_director_filter");
+cmdline_parse_token_num_t cmd_i40e_flow_director_port_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_i40e_flow_director_result,
+			      port_id, UINT8);
+cmdline_parse_token_string_t cmd_i40e_flow_director_ops =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 ops, "add#del");
+cmdline_parse_token_string_t cmd_i40e_flow_director_flow =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 flow, "flow");
+cmdline_parse_token_string_t cmd_i40e_flow_director_flow_type =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 flow_type, "ip4#tcp4#udp4#ip6#tcp6#udp6");
+cmdline_parse_token_string_t cmd_i40e_flow_director_src =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 src, "src");
+cmdline_parse_token_ipaddr_t cmd_i40e_flow_director_ip_src =
+	TOKEN_IPADDR_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 ip_src);
+cmdline_parse_token_num_t cmd_i40e_flow_director_port_src =
+	TOKEN_NUM_INITIALIZER(struct cmd_i40e_flow_director_result,
+			      port_src, UINT16);
+cmdline_parse_token_string_t cmd_i40e_flow_director_dst =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 dst, "dst");
+cmdline_parse_token_ipaddr_t cmd_i40e_flow_director_ip_dst =
+	TOKEN_IPADDR_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 ip_dst);
+cmdline_parse_token_num_t cmd_i40e_flow_director_port_dst =
+	TOKEN_NUM_INITIALIZER(struct cmd_i40e_flow_director_result,
+			      port_dst, UINT16);
+cmdline_parse_token_string_t cmd_i40e_flow_director_flexwords =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 flexwords, "flexwords");
+cmdline_parse_token_string_t cmd_i40e_flow_director_flexwords_value =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flow_director_result,
+			      flexwords_value, NULL);
+cmdline_parse_token_string_t cmd_i40e_flow_director_drop =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 drop, "drop#fwd");
+cmdline_parse_token_string_t cmd_i40e_flow_director_queue =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 queue, "queue");
+cmdline_parse_token_num_t cmd_i40e_flow_director_queue_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_i40e_flow_director_result,
+			      queue_id, UINT16);
+cmdline_parse_token_string_t cmd_i40e_flow_director_fd_id =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flow_director_result,
+				 fd_id, "fd_id");
+cmdline_parse_token_num_t cmd_i40e_flow_director_fd_id_value =
+	TOKEN_NUM_INITIALIZER(struct cmd_i40e_flow_director_result,
+			      fd_id_value, UINT32);
+
+cmdline_parse_inst_t cmd_i40e_add_del_ip_flow_director = {
+	.f = cmd_i40e_flow_director_parsed,
+	.data = NULL,
+	.help_str = "add or delete a ip flow director entry on i40e NIC",
+	.tokens = {
+		(void *)&cmd_i40e_flow_director_filter,
+		(void *)&cmd_i40e_flow_director_port_id,
+		(void *)&cmd_i40e_flow_director_ops,
+		(void *)&cmd_i40e_flow_director_flow,
+		(void *)&cmd_i40e_flow_director_flow_type,
+		(void *)&cmd_i40e_flow_director_src,
+		(void *)&cmd_i40e_flow_director_ip_src,
+		(void *)&cmd_i40e_flow_director_dst,
+		(void *)&cmd_i40e_flow_director_ip_dst,
+		(void *)&cmd_i40e_flow_director_flexwords,
+		(void *)&cmd_i40e_flow_director_flexwords_value,
+		(void *)&cmd_i40e_flow_director_drop,
+		(void *)&cmd_i40e_flow_director_queue,
+		(void *)&cmd_i40e_flow_director_queue_id,
+		(void *)&cmd_i40e_flow_director_fd_id,
+		(void *)&cmd_i40e_flow_director_fd_id_value,
+		NULL,
+	},
+};
+
+cmdline_parse_inst_t cmd_i40e_add_del_udp_flow_director = {
+	.f = cmd_i40e_flow_director_parsed,
+	.data = NULL,
+	.help_str = "add or delete a udp/tcp flow director entry on i40e NIC",
+	.tokens = {
+		(void *)&cmd_i40e_flow_director_filter,
+		(void *)&cmd_i40e_flow_director_port_id,
+		(void *)&cmd_i40e_flow_director_ops,
+		(void *)&cmd_i40e_flow_director_flow,
+		(void *)&cmd_i40e_flow_director_flow_type,
+		(void *)&cmd_i40e_flow_director_src,
+		(void *)&cmd_i40e_flow_director_ip_src,
+		(void *)&cmd_i40e_flow_director_port_src,
+		(void *)&cmd_i40e_flow_director_dst,
+		(void *)&cmd_i40e_flow_director_ip_dst,
+		(void *)&cmd_i40e_flow_director_port_dst,
+		(void *)&cmd_i40e_flow_director_flexwords,
+		(void *)&cmd_i40e_flow_director_flexwords_value,
+		(void *)&cmd_i40e_flow_director_drop,
+		(void *)&cmd_i40e_flow_director_queue,
+		(void *)&cmd_i40e_flow_director_queue_id,
+		(void *)&cmd_i40e_flow_director_fd_id,
+		(void *)&cmd_i40e_flow_director_fd_id_value,
+		NULL,
+	},
+};
+
+struct cmd_i40e_flush_flow_director_result {
+	cmdline_fixed_string_t flush_flow_director;
+	uint8_t port_id;
+};
+
+cmdline_parse_token_string_t cmd_i40e_flush_flow_director_flush =
+	TOKEN_STRING_INITIALIZER(struct cmd_i40e_flush_flow_director_result,
+				 flush_flow_director, "i40e_flush_flow_director");
+cmdline_parse_token_num_t cmd_i40e_flush_flow_director_port_id =
+	TOKEN_NUM_INITIALIZER(struct cmd_i40e_flush_flow_director_result,
+			      port_id, UINT8);
+
+static void
+cmd_i40e_flush_flow_director_parsed(void *parsed_result,
+			  __attribute__((unused)) struct cmdline *cl,
+			  __attribute__((unused)) void *data)
+{
+	struct cmd_i40e_flow_director_result *res = parsed_result;
+	int ret = 0;
+
+	ret = rte_eth_dev_rx_classification_filter_ctl(res->port_id,
+			RTE_CMD_FDIR_FLUSH, NULL);
+	if (ret < 0)
+		printf("i40e flow director table flushing error,"
+			" return code = %d \n", ret);
+}
+
+cmdline_parse_inst_t cmd_i40e_flush_flow_director = {
+	.f = cmd_i40e_flush_flow_director_parsed,
+	.data = NULL,
+	.help_str = "flush all flow director entries of a device on i40e NIC",
+	.tokens = {
+		(void *)&cmd_i40e_flush_flow_director_flush,
+		(void *)&cmd_i40e_flush_flow_director_port_id,
+		NULL,
+	},
+};
+#endif /* RTE_LIBRTE_I40E_PMD */
+
 /* ******************************************************************************** */
 
 /* list of instructions */
@@ -7529,6 +8189,11 @@ cmdline_parse_ctx_t main_ctx[] = {
 	(cmdline_parse_inst_t *)&cmd_add_flex_filter,
 	(cmdline_parse_inst_t *)&cmd_remove_flex_filter,
 	(cmdline_parse_inst_t *)&cmd_get_flex_filter,
+#ifdef RTE_LIBRTE_I40E_PMD
+	(cmdline_parse_inst_t *)&cmd_i40e_add_del_ip_flow_director,
+	(cmdline_parse_inst_t *)&cmd_i40e_add_del_udp_flow_director,
+	(cmdline_parse_inst_t *)&cmd_i40e_flush_flow_director,
+#endif /* RTE_LIBRTE_I40E_PMD */
 	NULL,
 };
 
diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 606e34a..86a52c3 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -93,6 +93,10 @@
 #include <rte_ether.h>
 #include <rte_ethdev.h>
 #include <rte_string_fns.h>
+#include <rte_eth_features.h>
+#ifdef RTE_LIBRTE_I40E_PMD
+#include <rte_i40e.h>
+#endif /* RTE_LIBRTE_I40E_PMD */
 
 #include "testpmd.h"
 
@@ -1781,26 +1785,52 @@ fdir_remove_signature_filter(portid_t port_id,
 void
 fdir_get_infos(portid_t port_id)
 {
-	struct rte_eth_fdir fdir_infos;
+	struct rte_eth_dev_info dev_info;
 
 	static const char *fdir_stats_border = "########################";
 
 	if (port_id_is_invalid(port_id))
 		return;
 
-	rte_eth_dev_fdir_get_infos(port_id, &fdir_infos);
-
 	printf("\n  %s FDIR infos for port %-2d     %s\n",
 	       fdir_stats_border, port_id, fdir_stats_border);
-
-	printf("  collision: %-10"PRIu64"  free:     %"PRIu64"\n"
-	       "  maxhash:   %-10"PRIu64"  maxlen:   %"PRIu64"\n"
-	       "  add:       %-10"PRIu64"  remove:   %"PRIu64"\n"
-	       "  f_add:     %-10"PRIu64"  f_remove: %"PRIu64"\n",
-	       (uint64_t)(fdir_infos.collision), (uint64_t)(fdir_infos.free),
-	       (uint64_t)(fdir_infos.maxhash), (uint64_t)(fdir_infos.maxlen),
-	       fdir_infos.add, fdir_infos.remove,
-	       fdir_infos.f_add, fdir_infos.f_remove);
+	memset(&dev_info, 0, sizeof(dev_info));
+	rte_eth_dev_info_get(port_id, &dev_info);
+#ifdef RTE_LIBRTE_I40E_PMD
+	if (strstr(dev_info.driver_name, "i40e") != NULL) {
+		struct rte_i40e_fdir_info i40e_fdir_info;
+		memset(&i40e_fdir_info, 0, sizeof(struct rte_i40e_fdir_info));
+		rte_eth_dev_rx_classification_filter_ctl(port_id,
+			RTE_CMD_FDIR_INFO_GET, (void *)&i40e_fdir_info);
+		if (i40e_fdir_info.mode) {
+			printf("  FDIR is enabled\n");
+			printf("  guarant_space: %-10"PRIu16
+			       "  best_space:     %"PRIu16"\n",
+			       i40e_fdir_info.guarant_spc,
+			       i40e_fdir_info.best_spc);
+			printf("  guarant_count: %-10"PRIu16
+			       "  best_count:     %"PRIu16"\n",
+			       i40e_fdir_info.guarant_cnt,
+			       i40e_fdir_info.best_cnt);
+		} else
+			printf("  FDIR is disabled\n");
+	} else {
+#endif /* RTE_LIBRTE_I40E_PMD */
+		struct rte_eth_fdir fdir_infos;
+
+		rte_eth_dev_fdir_get_infos(port_id, &fdir_infos);
+
+		printf("  collision: %-10"PRIu64"  free:     %"PRIu64"\n"
+		       "  maxhash:   %-10"PRIu64"  maxlen:   %"PRIu64"\n"
+		       "  add:       %-10"PRIu64"  remove:   %"PRIu64"\n"
+		       "  f_add:     %-10"PRIu64"  f_remove: %"PRIu64"\n",
+		       (uint64_t)(fdir_infos.collision), (uint64_t)(fdir_infos.free),
+		       (uint64_t)(fdir_infos.maxhash), (uint64_t)(fdir_infos.maxlen),
+		       fdir_infos.add, fdir_infos.remove,
+		       fdir_infos.f_add, fdir_infos.f_remove);
+#ifdef RTE_LIBRTE_I40E_PMD
+	}
+#endif /* RTE_LIBRTE_I40E_PMD */
 	printf("  %s############################%s\n",
 	       fdir_stats_border, fdir_stats_border);
 }
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index a112559..f36866a 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -1688,6 +1688,28 @@ init_port_config(void)
 		port = &ports[pid];
 		port->dev_conf.rxmode = rx_mode;
 		port->dev_conf.fdir_conf = fdir_conf;
+#ifdef RTE_LIBRTE_I40E_PMD
+#define I40E_FDIR_NB_MBUF          4
+#define I40E_FDIR_MBUF_SIZE        (512 + sizeof(struct rte_mbuf))
+#define I40E_FDIR_PKT_LEN          512
+#define MEMPOOL_CACHE_SIZE         256
+#define I40E_FDIR_MBUF_NAME        "FDIR_MBUF_POOL"
+
+		port->i40e_fdir_mp = rte_mempool_lookup(I40E_FDIR_MBUF_NAME);
+		if (port->i40e_fdir_mp == NULL) {
+			port->i40e_fdir_mp = rte_mempool_create(I40E_FDIR_MBUF_NAME,
+				I40E_FDIR_NB_MBUF, I40E_FDIR_MBUF_SIZE,
+				MEMPOOL_CACHE_SIZE,
+				sizeof(struct rte_pktmbuf_pool_private),
+				rte_pktmbuf_pool_init, NULL,
+				rte_pktmbuf_init, NULL,
+				0, 0);
+
+			if (port->i40e_fdir_mp == NULL)
+				printf("error: Cannot init mbuf pool for"
+					"i40e flow director.\n");
+		}
+#endif /* RTE_LIBRTE_I40E_PMD */
 		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 b8322a2..ddcc9e9 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -73,6 +73,9 @@ int main(int argc, char **argv);
 #define NUMA_NO_CONFIG 0xFF
 #define UMA_NO_CONFIG  0xFF
 
+#define BYTES_PER_WORD  2
+#define IPV6_ADDR_LEN 16
+
 typedef uint8_t  lcoreid_t;
 typedef uint8_t  portid_t;
 typedef uint16_t queueid_t;
@@ -155,6 +158,9 @@ struct rte_port {
 	uint8_t			dcb_flag;   /**< enable dcb */
 	struct rte_eth_rxconf   rx_conf;    /**< rx configuration */
 	struct rte_eth_txconf   tx_conf;    /**< tx configuration */
+#ifdef RTE_LIBRTE_I40E_PMD
+	struct rte_mempool      *i40e_fdir_mp;  /**< mempool used for fdir programing */
+#endif /* RTE_LIBRTE_I40E_PMD */
 };
 
 /**
@@ -352,6 +358,57 @@ extern uint32_t param_total_num_mbufs;
 
 extern struct rte_fdir_conf fdir_conf;
 
+#ifdef RTE_LIBRTE_I40E_PMD
+struct ipv4_udp_flow {
+	uint32_t src_ip;         /**< IPv4 source address to match. */
+	uint32_t dst_ip;         /**< IPv4 destination address to match. */
+	uint16_t src_port;       /**< UDP Source port to match. */
+	uint16_t dst_port;       /**< UDP Destination port to match. */
+	uint8_t num_flexwords;   /**< number of the flexwords. */
+	uint16_t flexwords[8];   /**< flexwords in payload. */
+};
+
+struct ipv4_tcp_flow {
+	uint32_t src_ip;         /**< IPv4 source address to match. */
+	uint32_t dst_ip;         /**< IPv4 destination address to match. */
+	uint16_t src_port;       /**< TCP Source port to match. */
+	uint16_t dst_port;       /**< TCP Destination port to match. */
+	uint8_t num_flexwords;   /**< number of the flexwords. */
+	uint16_t flexwords[8];   /**< flexwords in payload. */
+};
+
+struct ipv4_other_flow {
+	uint32_t src_ip;         /**< IPv4 source address to match. */
+	uint32_t dst_ip;         /**< IPv4 destination address to match. */
+	uint8_t num_flexwords;   /**< number of the flexwords. */
+	uint16_t flexwords[8];   /**< flexwords in payload. */
+};
+
+struct ipv6_udp_flow {
+	uint32_t src_ip[4];      /**< IPv6 source address to match. */
+	uint32_t dst_ip[4];      /**< IPv6 destination address to match. */
+	uint16_t src_port;       /**< UDP Source port to match. */
+	uint16_t dst_port;       /**< UDP Destination port to match. */
+	uint8_t num_flexwords;   /**< number of the flexwords. */
+	uint16_t flexwords[8];   /**< flexwords in payload. */
+};
+
+struct ipv6_tcp_flow {
+	uint32_t src_ip[4];      /**< IPv6 source address to match. */
+	uint32_t dst_ip[4];      /**< IPv6 destination address to match. */
+	uint16_t src_port;       /**< TCP Source port to match. */
+	uint16_t dst_port;       /**< TCP Destination port to match. */
+	uint8_t num_flexwords;   /**< number of the flexwords. */
+	uint16_t flexwords[8];   /**< flexwords in payload. */
+};
+
+struct ipv6_other_flow {
+	uint32_t src_ip[4];      /**< IPv6 source address to match. */
+	uint32_t dst_ip[4];      /**< IPv6 destination address to match. */
+	uint8_t num_flexwords;   /**< number of the flexwords. */
+	uint16_t flexwords[8];   /**< flexwords in payload. */
+};
+#endif /* RTE_LIBRTE_I40E_PMD */
 /*
  * Configuration of packet segments used by the "txonly" processing engine.
  */
-- 
1.8.1.4

Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support

[Thomas Monjalon]

Hi Jingjing,

2014-08-27 10:13, Jingjing Wu:
> add structure definition to construct programming packet.

What is a "programming packet"?

> +#ifdef RTE_LIBRTE_I40E_PMD
> +			"i40e_flow_director_filter (port_id) (add|del)"
> +			" flow (ip4|ip6) src (src_ip_address) dst (dst_ip_address)"
> +			" flexwords (flexwords_value) (drop|fwd)"
> +			" queue (queue_id) fd_id (fd_id_value)\n"
> +			"    Add/Del a IP type flow director filter for i40e NIC.\n\n"
> +
> +			"i40e_flow_director_filter (port_id) (add|del)"
> +			" flow (udp4|tcp4|udp6|tcp6)"
> +			" src (src_ip_address) (src_port)"
> +			" dst (dst_ip_address) (dst_port)"
> +			" flexwords (flexwords_value) (drop|fwd)"
> +			" queue (queue_id) fd_id (fd_id_value)\n"
> +			"    Add/Del a UDP/TCP type flow director filter for i40e NIC.\n\n"
> +
> +			"i40e_flush_flow_diretor (port_id)\n"
> +			"    Flush all flow director entries of a device on i40e NIC.\n\n"
> +#endif /* RTE_LIBRTE_I40E_PMD */

I'd really like to stop seeing this kind of thing.
We cannot add some ifdef for each PMD in generic code.

I stopped reading after that.

Sorry, I don't want to be rude but my feeling is that adding such feature
with global picture in mind is not easy. I know you want to offer all i40e
capabilities but you should think at future evolutions and how other drivers
will be integrated with yours.

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support

[Venkatesan, Venky]

On 8/27/2014 7:35 AM, Thomas Monjalon wrote:
> Hi Jingjing,
>
> 2014-08-27 10:13, Jingjing Wu:
>> add structure definition to construct programming packet.
> What is a "programming packet"?
>
>> +#ifdef RTE_LIBRTE_I40E_PMD
>> +			"i40e_flow_director_filter (port_id) (add|del)"
>> +			" flow (ip4|ip6) src (src_ip_address) dst (dst_ip_address)"
>> +			" flexwords (flexwords_value) (drop|fwd)"
>> +			" queue (queue_id) fd_id (fd_id_value)\n"
>> +			"    Add/Del a IP type flow director filter for i40e NIC.\n\n"
>> +
>> +			"i40e_flow_director_filter (port_id) (add|del)"
>> +			" flow (udp4|tcp4|udp6|tcp6)"
>> +			" src (src_ip_address) (src_port)"
>> +			" dst (dst_ip_address) (dst_port)"
>> +			" flexwords (flexwords_value) (drop|fwd)"
>> +			" queue (queue_id) fd_id (fd_id_value)\n"
>> +			"    Add/Del a UDP/TCP type flow director filter for i40e NIC.\n\n"
>> +
>> +			"i40e_flush_flow_diretor (port_id)\n"
>> +			"    Flush all flow director entries of a device on i40e NIC.\n\n"
>> +#endif /* RTE_LIBRTE_I40E_PMD */
> I'd really like to stop seeing this kind of thing.
> We cannot add some ifdef for each PMD in generic code.
>
> I stopped reading after that.
>
> Sorry, I don't want to be rude but my feeling is that adding such feature
> with global picture in mind is not easy. I know you want to offer all i40e
> capabilities but you should think at future evolutions and how other drivers
> will be integrated with yours.
>
> Thanks
Second that. Any PMD-specifics need to be contained within the PMD 
itself, and not in generic code. Please rework this.
-Venky

Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support

[Wu, Jingjing]

Hi, Thomas

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Wednesday, August 27, 2014 10:36 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for
> i40e flow director support
> 
> Hi Jingjing,
> 
> 2014-08-27 10:13, Jingjing Wu:
> > add structure definition to construct programming packet.
> 
> What is a "programming packet"?
For Fortville, we need to set a flow director filter by sending a packet which contains the input set values through the queue belonging to flow director.
> 
> > +#ifdef RTE_LIBRTE_I40E_PMD
> > +			"i40e_flow_director_filter (port_id) (add|del)"
> > +			" flow (ip4|ip6) src (src_ip_address) dst (dst_ip_address)"
> > +			" flexwords (flexwords_value) (drop|fwd)"
> > +			" queue (queue_id) fd_id (fd_id_value)\n"
> > +			"    Add/Del a IP type flow director filter for i40e NIC.\n\n"
> > +
> > +			"i40e_flow_director_filter (port_id) (add|del)"
> > +			" flow (udp4|tcp4|udp6|tcp6)"
> > +			" src (src_ip_address) (src_port)"
> > +			" dst (dst_ip_address) (dst_port)"
> > +			" flexwords (flexwords_value) (drop|fwd)"
> > +			" queue (queue_id) fd_id (fd_id_value)\n"
> > +			"    Add/Del a UDP/TCP type flow director filter for i40e NIC.\n\n"
> > +
> > +			"i40e_flush_flow_diretor (port_id)\n"
> > +			"    Flush all flow director entries of a device on i40e NIC.\n\n"
> > +#endif /* RTE_LIBRTE_I40E_PMD */
> 
> I'd really like to stop seeing this kind of thing.
> We cannot add some ifdef for each PMD in generic code.
> 
> I stopped reading after that.
> 
> Sorry, I don't want to be rude but my feeling is that adding such feature
> with global picture in mind is not easy. I know you want to offer all i40e
> capabilities but you should think at future evolutions and how other drivers
> will be integrated with yours.
> 

Sorry to make you feel uncomfortable for such code. Just as you say, I want to offer more i40e capabilities. I will rework code in testpmd. 

> Thanks
> --
> Thomas

Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support

[Thomas Monjalon]

2014-08-28 03:51, Wu, Jingjing:
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > 2014-08-27 10:13, Jingjing Wu:
> > > add structure definition to construct programming packet.
> > 
> > What is a "programming packet"?
> 
> For Fortville, we need to set a flow director filter by sending a
> packet which contains the input set values through the queue
> belonging to flow director.

OK. To be more clear, some detailed explanations are required in the
commit log. Please try to be very descriptive.
You can think comments like this:
- if comments are absolutely needed to understand the code, you should
put comments in the code (or write the code differently)
- if some feature context can help for the review, you should explain
context and design in the commit log

> > > +#ifdef RTE_LIBRTE_I40E_PMD
> > > +			"i40e_flow_director_filter (port_id) (add|del)"
> > > +			" flow (ip4|ip6) src (src_ip_address) dst (dst_ip_address)"
> > > +			" flexwords (flexwords_value) (drop|fwd)"
> > > +			" queue (queue_id) fd_id (fd_id_value)\n"
> > > +			"    Add/Del a IP type flow director filter for i40e NIC.\n\n"
> > > +
> > > +			"i40e_flow_director_filter (port_id) (add|del)"
> > > +			" flow (udp4|tcp4|udp6|tcp6)"
> > > +			" src (src_ip_address) (src_port)"
> > > +			" dst (dst_ip_address) (dst_port)"
> > > +			" flexwords (flexwords_value) (drop|fwd)"
> > > +			" queue (queue_id) fd_id (fd_id_value)\n"
> > > +			"    Add/Del a UDP/TCP type flow director filter for i40e NIC.\n\n"
> > > +
> > > +			"i40e_flush_flow_diretor (port_id)\n"
> > > +			"    Flush all flow director entries of a device on i40e NIC.\n\n"
> > > +#endif /* RTE_LIBRTE_I40E_PMD */
> > 
> > I'd really like to stop seeing this kind of thing.
> > We cannot add some ifdef for each PMD in generic code.
> > 
> > I stopped reading after that.
> > 
> > Sorry, I don't want to be rude but my feeling is that adding such feature
> > with global picture in mind is not easy. I know you want to offer all i40e
> > capabilities but you should think at future evolutions and how other drivers
> > will be integrated with yours.
> > 
> 
> Sorry to make you feel uncomfortable for such code. Just as you say,
> I want to offer more i40e capabilities. I will rework code in testpmd. 

Thanks
-- 
Thomas

Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support

[Wu, Jingjing]

Hi, Thomas

Thanks for your tips.

I have another question:
If we use the way 'rx_classification_filter_ctl' works, the specific structures defined in rte_i40e.h will be visible in user's application, such as testpmd.
I know I shouldn't make commands linked with i40e like what I did before. But will the i40e specific structures become visible be acceptable?



> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, August 28, 2014 4:51 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for
> i40e flow director support
> 
> 2014-08-28 03:51, Wu, Jingjing:
> > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> > > 2014-08-27 10:13, Jingjing Wu:
> > > > add structure definition to construct programming packet.
> > >
> > > What is a "programming packet"?
> >
> > For Fortville, we need to set a flow director filter by sending a
> > packet which contains the input set values through the queue
> > belonging to flow director.
> 
> OK. To be more clear, some detailed explanations are required in the
> commit log. Please try to be very descriptive.
> You can think comments like this:
> - if comments are absolutely needed to understand the code, you should
> put comments in the code (or write the code differently)
> - if some feature context can help for the review, you should explain
> context and design in the commit log
> 
> > > > +#ifdef RTE_LIBRTE_I40E_PMD
> > > > +			"i40e_flow_director_filter (port_id) (add|del)"
> > > > +			" flow (ip4|ip6) src (src_ip_address) dst (dst_ip_address)"
> > > > +			" flexwords (flexwords_value) (drop|fwd)"
> > > > +			" queue (queue_id) fd_id (fd_id_value)\n"
> > > > +			"    Add/Del a IP type flow director filter for i40e NIC.\n\n"
> > > > +
> > > > +			"i40e_flow_director_filter (port_id) (add|del)"
> > > > +			" flow (udp4|tcp4|udp6|tcp6)"
> > > > +			" src (src_ip_address) (src_port)"
> > > > +			" dst (dst_ip_address) (dst_port)"
> > > > +			" flexwords (flexwords_value) (drop|fwd)"
> > > > +			" queue (queue_id) fd_id (fd_id_value)\n"
> > > > +			"    Add/Del a UDP/TCP type flow director filter for i40e NIC.\n\n"
> > > > +
> > > > +			"i40e_flush_flow_diretor (port_id)\n"
> > > > +			"    Flush all flow director entries of a device on i40e NIC.\n\n"
> > > > +#endif /* RTE_LIBRTE_I40E_PMD */
> > >
> > > I'd really like to stop seeing this kind of thing.
> > > We cannot add some ifdef for each PMD in generic code.
> > >
> > > I stopped reading after that.
> > >
> > > Sorry, I don't want to be rude but my feeling is that adding such feature
> > > with global picture in mind is not easy. I know you want to offer all i40e
> > > capabilities but you should think at future evolutions and how other drivers
> > > will be integrated with yours.
> > >
> >
> > Sorry to make you feel uncomfortable for such code. Just as you say,
> > I want to offer more i40e capabilities. I will rework code in testpmd.
> 
> Thanks
> --
> Thomas

Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support

[Thomas Monjalon]

2014-08-28 09:01, Wu, Jingjing:
> I have another question:
> If we use the way 'rx_classification_filter_ctl' works, the specific
> structures defined in rte_i40e.h will be visible in user's application,
> such as testpmd.
> I know I shouldn't make commands linked with i40e like what I did before.
> But will the i40e specific structures become visible be acceptable?

I think testpmd should be limited to generic API.
So it wouldn't be acceptable to be dependent of i40e files.
But having some specific i40e tests in examples or app/test is OK.

-- 
Thomas

Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support

[Ananyev, Konstantin]

> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Thursday, August 28, 2014 12:01 PM
> To: Wu, Jingjing
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support
> 
> 2014-08-28 09:01, Wu, Jingjing:
> > I have another question:
> > If we use the way 'rx_classification_filter_ctl' works, the specific
> > structures defined in rte_i40e.h will be visible in user's application,
> > such as testpmd.
> > I know I shouldn't make commands linked with i40e like what I did before.
> > But will the i40e specific structures become visible be acceptable?
> 
> I think testpmd should be limited to generic API.
> So it wouldn't be acceptable to be dependent of i40e files.
> But having some specific i40e tests in examples or app/test is OK.
> 

Probably I didn't get you right:
Are you suggesting to have a new clone of testpmd for any new device we are going to support?
That seems like too much hassle to me.
Plus what to do if someone would like to test configuration with two different devices involved: ixgbe and i40e for example? 
I suggest we keep one testpmd for all devices we support.
Of course we'll probably have to make some rework to avoid if (strncmp(drv_name, "xxx") spread all over it.
We need to find some better way to discover/setup HW specific features.
Thanks
Konstantin

Re: [dpdk-dev] [PATCH v2 7/7]app/testpmd: add commands and config functions for i40e flow director support

[Thomas Monjalon]

2014-08-28 11:30, Ananyev, Konstantin:
> From: Thomas Monjalon
> > 2014-08-28 09:01, Wu, Jingjing:
> > > I have another question:
> > > If we use the way 'rx_classification_filter_ctl' works, the specific
> > > structures defined in rte_i40e.h will be visible in user's application,
> > > such as testpmd.
> > > I know I shouldn't make commands linked with i40e like what I did before.
> > > But will the i40e specific structures become visible be acceptable?
> > 
> > I think testpmd should be limited to generic API.
> > So it wouldn't be acceptable to be dependent of i40e files.
> > But having some specific i40e tests in examples or app/test is OK.
> > 
> 
> Probably I didn't get you right:

Indeed ;)

> Are you suggesting to have a new clone of testpmd for any new device
> we are going to support?

No. I say there shouldn't be any PMD dependency on testpmd.
It means we should use only generic API.

> That seems like too much hassle to me.
> Plus what to do if someone would like to test configuration with two
> different devices involved: ixgbe and i40e for example?

ixgbe and i40e features should use the same generic API for flow director.

> I suggest we keep one testpmd for all devices we support.
> Of course we'll probably have to make some rework to avoid
> if (strncmp(drv_name, "xxx") spread all over it.
> We need to find some better way to discover/setup HW specific features.

Agreed
-- 
Thomas

Re: [dpdk-dev] [PATCH v2 0/7] Support flow director programming on fortville

[Cao, Min]

Tested-by: Min Cao <min.cao@intel.com>

I have tested this patch with Fortville. Flow director is tested with 2*40G, 1*40G and 4*10G NIC.


-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Jingjing Wu
Sent: Wednesday, August 27, 2014 10:14 AM
To: dev@dpdk.org
Subject: [dpdk-dev] [PATCH v2 0/7] Support flow director programming on fortville

The patch set supports flow director programming on fortville.
It includes:
 - reserve i40e resources for flow director, such as queue and vsi.
 - support the new ethdev API rx_classification_filter_ctl for all
   the configuration or queries for receive classification filters.
 - support programming 6 flow types for the flow director filters,
   which is called PCTYPE in fortville: ipv4, tcpv4, udpv4, ipv6,
   tcpv6, udpv6.
 - support flushing flow director table (all filters).
 - support to get flow diretor information.
 - support match statistics and FD ID report.
 - fix the the Marco conflict between rte_ip.h and netinet/in.h.
 
v2 changes:
 - create real fdir vsi and assign queue 0 pair to it.
 - check filter status report on the rx queue 0
 
further plan:
 - add flexible payload comprasion as flow director's input
 - support sctpv4 and sctpv6 PCTYPEs

jingjing.wu (7):
  flow director resource reserve and initialize on i40e
  define new ethdev API rx_classification_filter_ctl
  function implement in i40e for flow director filter programming
  function implement in i40e for flow director flush and info get
  fix the Marco conflict
  support FD ID report and match counter for i40e flow director
  add commands and config functions for i40e flow director support

 app/test-pmd/cmdline.c              | 665 ++++++++++++++++++++++++++++++++++++
 app/test-pmd/config.c               |  54 ++-
 app/test-pmd/testpmd.c              |  22 ++
 app/test-pmd/testpmd.h              |  57 ++++
 lib/librte_ether/Makefile           |   3 +-
 lib/librte_ether/rte_eth_features.h |  65 ++++
 lib/librte_ether/rte_ethdev.c       |  19 +-
 lib/librte_ether/rte_ethdev.h       | 108 +++---
 lib/librte_net/rte_ip.h             |   5 +-
 lib/librte_pmd_i40e/Makefile        |   5 +
 lib/librte_pmd_i40e/i40e_ethdev.c   | 137 +++++++-
 lib/librte_pmd_i40e/i40e_ethdev.h   |  32 +-
 lib/librte_pmd_i40e/i40e_fdir.c     | 500 +++++++++++++++++++++++++++
 lib/librte_pmd_i40e/i40e_rxtx.c     | 176 +++++++++-
 lib/librte_pmd_i40e/rte_i40e.h      | 125 +++++++
 15 files changed, 1906 insertions(+), 67 deletions(-)
 create mode 100644 lib/librte_ether/rte_eth_features.h
 create mode 100644 lib/librte_pmd_i40e/i40e_fdir.c
 create mode 100644 lib/librte_pmd_i40e/rte_i40e.h

-- 
1.8.1.4