[PATCH v3 0/5] ixgbe: fix bugs or just improve.

[PATCH v3 0/5] ixgbe: fix bugs or just improve.

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

The patches in the patchset have no dependency. But all of them
is about ixgbe or ixgbevf. The patch 1 and 2 add the itr configuration
for ixgbe and ixgbevf, the user and developer can configure it for
their platform. Other patches refine the ixgbe or ixgbevf.

v2 --> v3:
remove the patch: http://dpdk.org/dev/patchwork/patch/33698

Tonghao Zhang (5):
  net/ixgbevf: set the inter-interrupt interval for EITR.
  net/ixgbe: set the ITR via configuration.
  net/ixgbe: write disable to ITR counter.
  net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.
  net/ixgbe: remove the unnecessary call rte_intr_enable.

 config/common_base               |  2 +
 drivers/net/ixgbe/ixgbe_ethdev.c | 79 +++++++++++++++++++++++-----------------
 drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++
 drivers/net/ixgbe/ixgbe_rxtx.c   |  3 +-
 4 files changed, 61 insertions(+), 35 deletions(-)

-- 
1.8.3.1

[PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

Set EITR interval as default. This patch can improve the
performance when we enable the rx-intrrupt to process the
packets because we hope rx-intrrupt reduce CPU. For example,
the 200us value of EITR makes the performance better with
the low CPU.

Users can configure the value of ITR via DPDK configuration.

The default value of ITR is 500us, compatible with RSC of ixgbe PF,
and next patch will use the default value.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
v1 --> v2:
use the configure file, for different user.
suggested by Beilei Xing, http://dpdk.org/dev/patchwork/patch/32989
---
 config/common_base               |  2 ++
 drivers/net/ixgbe/ixgbe_ethdev.c |  7 +++++++
 drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++++++++
 3 files changed, 21 insertions(+)

diff --git a/config/common_base b/config/common_base
index e74febe..2e9fded 100644
--- a/config/common_base
+++ b/config/common_base
@@ -196,6 +196,8 @@ CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n
 CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n
 CONFIG_RTE_IXGBE_INC_VECTOR=y
 CONFIG_RTE_LIBRTE_IXGBE_BYPASS=n
+# interval up to 1024 us
+CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL=-1
 
 #
 # Compile burst-oriented I40E PMD driver
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index e67389f..495e72c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -5780,6 +5780,13 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 		if (vector_idx < base + intr_handle->nb_efd - 1)
 			vector_idx++;
 	}
+
+	/* As RX queue setting above show, all queues use the vector 0.
+	 * Set only the ITR value of IXGBE_MISC_VEC_ID.
+	 */
+	IXGBE_WRITE_REG(hw, IXGBE_VTEITR(IXGBE_MISC_VEC_ID),
+			ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL)
+			| IXGBE_EITR_CNT_WDIS);
 }
 
 /**
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
index 1db29bd..c779001 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/ixgbe/ixgbe_ethdev.h
@@ -58,6 +58,18 @@
 		IXGBE_EITR_ITR_INT_MASK)
 
 
+#define IXGBE_QUEUE_ITR_INTERVAL_MAX	1024 /* 1024us */
+#define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
+
+static inline uint16_t
+ixgbe_calc_itr_interval(int16_t interval)
+{
+	if (interval < 0 || interval > IXGBE_QUEUE_ITR_INTERVAL_MAX)
+		interval = IXGBE_QUEUE_ITR_INTERVAL_DEFAULT;
+
+	return IXGBE_EITR_INTERVAL_US(interval);
+}
+
 /* Loopback operation modes */
 /* 82599 specific loopback operation types */
 #define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is disabled. */
-- 
1.8.3.1

[PATCH v3 2/5] net/ixgbe: set the ITR via configuration.

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

With this patch, the ITR value of ixgbe PF, can be
configured as wanted.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 5 +----
 drivers/net/ixgbe/ixgbe_rxtx.c   | 3 ++-
 2 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 495e72c..14296ea 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -60,9 +60,6 @@
  */
 #define IXGBE_FC_LO    0x40
 
-/* Default minimum inter-interrupt interval for EITR configuration */
-#define IXGBE_MIN_INTER_INTERRUPT_INTERVAL_DEFAULT    0x79E
-
 /* Timer value included in XOFF frames. */
 #define IXGBE_FC_PAUSE 0x680
 
@@ -5856,7 +5853,7 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 		break;
 	}
 	IXGBE_WRITE_REG(hw, IXGBE_EITR(IXGBE_MISC_VEC_ID),
-			IXGBE_MIN_INTER_INTERRUPT_INTERVAL_DEFAULT & 0xFFF);
+			ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL));
 
 	/* set up to autoclear timer, and the vectors */
 	mask = IXGBE_EIMS_ENABLE_MASK;
diff --git a/drivers/net/ixgbe/ixgbe_rxtx.c b/drivers/net/ixgbe/ixgbe_rxtx.c
index 9bc8462..cb51f46 100644
--- a/drivers/net/ixgbe/ixgbe_rxtx.c
+++ b/drivers/net/ixgbe/ixgbe_rxtx.c
@@ -4695,7 +4695,8 @@ void __attribute__((cold))
 		 * at most 500us latency for a single RSC aggregation.
 		 */
 		eitr &= ~IXGBE_EITR_ITR_INT_MASK;
-		eitr |= IXGBE_EITR_INTERVAL_US(500) | IXGBE_EITR_CNT_WDIS;
+		eitr |= ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL);
+		eitr |= IXGBE_EITR_CNT_WDIS;
 
 		IXGBE_WRITE_REG(hw, IXGBE_SRRCTL(rxq->reg_idx), srrctl);
 		IXGBE_WRITE_REG(hw, IXGBE_RSCCTL(rxq->reg_idx), rscctl);
-- 
1.8.3.1

[PATCH v3 3/5] net/ixgbe: write disable to ITR counter.

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

ixgbe doesn't write the ITR counter, disable it now.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 14296ea..0ea8fe8 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -5853,7 +5853,8 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 		break;
 	}
 	IXGBE_WRITE_REG(hw, IXGBE_EITR(IXGBE_MISC_VEC_ID),
-			ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL));
+			ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL)
+			| IXGBE_EITR_CNT_WDIS);
 
 	/* set up to autoclear timer, and the vectors */
 	mask = IXGBE_EIMS_ENABLE_MASK;
-- 
1.8.3.1

[PATCH v3 4/5] net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

If dpdk APPs call the rte_eth_dev_rx_intr_enable or
rte_eth_dev_rx_intr_disable frequently, and ixgbe vf will read
the IXGBE_VTEIMS register everytime. The patch saves the IXGBE_VTEIMS
to mask to avoid read frequently.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Acked-by: Beilei Xing <beilei.xing@intel.com>
Acked-by: Wei Dai <wei.dai@intel.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 55 +++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 20 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 0ea8fe8..7928144 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -238,8 +238,8 @@ static int ixgbevf_dev_link_update(struct rte_eth_dev *dev,
 static void ixgbevf_dev_stop(struct rte_eth_dev *dev);
 static void ixgbevf_dev_close(struct rte_eth_dev *dev);
 static int  ixgbevf_dev_reset(struct rte_eth_dev *dev);
-static void ixgbevf_intr_disable(struct ixgbe_hw *hw);
-static void ixgbevf_intr_enable(struct ixgbe_hw *hw);
+static void ixgbevf_intr_disable(struct rte_eth_dev *dev);
+static void ixgbevf_intr_enable(struct rte_eth_dev *dev);
 static int ixgbevf_dev_stats_get(struct rte_eth_dev *dev,
 		struct rte_eth_stats *stats);
 static void ixgbevf_dev_stats_reset(struct rte_eth_dev *dev);
@@ -1633,7 +1633,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 	ixgbevf_dev_stats_reset(eth_dev);
 
 	/* Disable the interrupts for VF */
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(eth_dev);
 
 	hw->mac.num_rar_entries = 128; /* The MAX of the underlying PF */
 	diag = hw->mac.ops.reset_hw(hw);
@@ -1702,7 +1702,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 	rte_intr_callback_register(intr_handle,
 				   ixgbevf_dev_interrupt_handler, eth_dev);
 	rte_intr_enable(intr_handle);
-	ixgbevf_intr_enable(hw);
+	ixgbevf_intr_enable(eth_dev);
 
 	PMD_INIT_LOG(DEBUG, "port %d vendorID=0x%x deviceID=0x%x mac.type=%s",
 		     eth_dev->data->port_id, pci_dev->id.vendor_id,
@@ -1735,7 +1735,7 @@ static int ixgbe_l2_tn_filter_init(struct rte_eth_dev *eth_dev)
 	eth_dev->tx_pkt_burst = NULL;
 
 	/* Disable the interrupts for VF */
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(eth_dev);
 
 	rte_free(eth_dev->data->mac_addrs);
 	eth_dev->data->mac_addrs = NULL;
@@ -4921,19 +4921,32 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
  * Virtual Function operations
  */
 static void
-ixgbevf_intr_disable(struct ixgbe_hw *hw)
+ixgbevf_intr_disable(struct rte_eth_dev *dev)
 {
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
 	PMD_INIT_FUNC_TRACE();
 
 	/* Clear interrupt mask to stop from interrupts being generated */
 	IXGBE_WRITE_REG(hw, IXGBE_VTEIMC, IXGBE_VF_IRQ_CLEAR_MASK);
 
 	IXGBE_WRITE_FLUSH(hw);
+
+	/* Clear mask value. */
+	intr->mask = 0;
 }
 
 static void
-ixgbevf_intr_enable(struct ixgbe_hw *hw)
+ixgbevf_intr_enable(struct rte_eth_dev *dev)
 {
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
+	struct ixgbe_hw *hw =
+		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
 	PMD_INIT_FUNC_TRACE();
 
 	/* VF enable interrupt autoclean */
@@ -4942,6 +4955,9 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, IXGBE_VF_IRQ_ENABLE_MASK);
 
 	IXGBE_WRITE_FLUSH(hw);
+
+	/* Save IXGBE_VTEIMS value to mask. */
+	intr->mask = IXGBE_VF_IRQ_ENABLE_MASK;
 }
 
 static int
@@ -5067,7 +5083,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 	rte_intr_enable(intr_handle);
 
 	/* Re-enable interrupt for VF */
-	ixgbevf_intr_enable(hw);
+	ixgbevf_intr_enable(dev);
 
 	return 0;
 }
@@ -5081,7 +5097,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 
 	PMD_INIT_FUNC_TRACE();
 
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(dev);
 
 	hw->adapter_stopped = 1;
 	ixgbe_stop_adapter(hw);
@@ -5579,17 +5595,17 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 {
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
-	uint32_t mask;
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	uint32_t vec = IXGBE_MISC_VEC_ID;
 
-	mask = IXGBE_READ_REG(hw, IXGBE_VTEIMS);
 	if (rte_intr_allow_others(intr_handle))
 		vec = IXGBE_RX_VEC_START;
-	mask |= (1 << vec);
+	intr->mask |= (1 << vec);
 	RTE_SET_USED(queue_id);
-	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, mask);
+	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
 
 	rte_intr_enable(intr_handle);
 
@@ -5599,19 +5615,19 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 static int
 ixgbevf_dev_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id)
 {
-	uint32_t mask;
+	struct ixgbe_interrupt *intr =
+		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
 	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
 	uint32_t vec = IXGBE_MISC_VEC_ID;
 
-	mask = IXGBE_READ_REG(hw, IXGBE_VTEIMS);
 	if (rte_intr_allow_others(intr_handle))
 		vec = IXGBE_RX_VEC_START;
-	mask &= ~(1 << vec);
+	intr->mask &= ~(1 << vec);
 	RTE_SET_USED(queue_id);
-	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, mask);
+	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
 
 	return 0;
 }
@@ -8172,7 +8188,7 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
 	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
-	ixgbevf_intr_disable(hw);
+	ixgbevf_intr_disable(dev);
 
 	/* read-on-clear nic registers here */
 	eicr = IXGBE_READ_REG(hw, IXGBE_VTEICR);
@@ -8189,7 +8205,6 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
 static int
 ixgbevf_dev_interrupt_action(struct rte_eth_dev *dev)
 {
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 
@@ -8198,7 +8213,7 @@ static void ixgbevf_mbx_process(struct rte_eth_dev *dev)
 		intr->flags &= ~IXGBE_FLAG_MAILBOX;
 	}
 
-	ixgbevf_intr_enable(hw);
+	ixgbevf_intr_enable(dev);
 
 	return 0;
 }
-- 
1.8.3.1

[PATCH v3 5/5] net/ixgbe: remove the unnecessary call rte_intr_enable.

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

When binding the ixgbe pf or vf to vfio and call the
rte_eth_dev_rx_intr_enable frequently, the interrupt setting
(msi_set_mask_bit) will take more CPU as show below. rte_intr_enable
calls the ioctl to map the fd to interrupts frequently.

perf top:
5.45%  [kernel]   [k] msi_set_mask_bit

It is unnecessary to call the rte_intr_enable in
ixgbe_dev_rx_queue_intr_enable. Because the fds has been mapped to
interrupt and not unmapped in ixgbe_dev_rx_queue_intr_disable.

With the patch, msi_set_mask_bit is not listed in perl anymore.

Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index 7928144..ef01e0c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4253,7 +4253,7 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
  */
 static int
 ixgbe_dev_interrupt_action(struct rte_eth_dev *dev,
-			   struct rte_intr_handle *intr_handle)
+			   __rte_unused struct rte_intr_handle *intr_handle)
 {
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
@@ -4304,7 +4304,6 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 
 	PMD_DRV_LOG(DEBUG, "enable intr immediately");
 	ixgbe_enable_intr(dev);
-	rte_intr_enable(intr_handle);
 
 	return 0;
 }
@@ -4327,8 +4326,6 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 ixgbe_dev_interrupt_delayed_handler(void *param)
 {
 	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
-	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
-	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
 	struct ixgbe_interrupt *intr =
 		IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);
 	struct ixgbe_hw *hw =
@@ -4366,7 +4363,6 @@ static int ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,
 
 	PMD_DRV_LOG(DEBUG, "enable intr in delayed handler S[%08x]", eicr);
 	ixgbe_enable_intr(dev);
-	rte_intr_enable(intr_handle);
 }
 
 /**
@@ -5607,8 +5603,6 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 	RTE_SET_USED(queue_id);
 	IXGBE_WRITE_REG(hw, IXGBE_VTEIMS, intr->mask);
 
-	rte_intr_enable(intr_handle);
-
 	return 0;
 }
 
@@ -5635,8 +5629,6 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 static int
 ixgbe_dev_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id)
 {
-	struct rte_pci_device *pci_dev = RTE_ETH_DEV_TO_PCI(dev);
-	struct rte_intr_handle *intr_handle = &pci_dev->intr_handle;
 	uint32_t mask;
 	struct ixgbe_hw *hw =
 		IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -5656,7 +5648,6 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
 		mask &= (1 << (queue_id - 32));
 		IXGBE_WRITE_REG(hw, IXGBE_EIMS_EX(1), mask);
 	}
-	rte_intr_enable(intr_handle);
 
 	return 0;
 }
-- 
1.8.3.1

Re: [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.

[Ferruh Yigit]

On 3/22/2018 1:01 PM, xiangxia.m.yue@gmail.com wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> 
> Set EITR interval as default. This patch can improve the
> performance when we enable the rx-intrrupt to process the
> packets because we hope rx-intrrupt reduce CPU. For example,
> the 200us value of EITR makes the performance better with
> the low CPU.
> 
> Users can configure the value of ITR via DPDK configuration.
> 
> The default value of ITR is 500us, compatible with RSC of ixgbe PF,
> and next patch will use the default value.
> 
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> ---
> v1 --> v2:
> use the configure file, for different user.
> suggested by Beilei Xing, http://dpdk.org/dev/patchwork/patch/32989
> ---
>  config/common_base               |  2 ++
>  drivers/net/ixgbe/ixgbe_ethdev.c |  7 +++++++
>  drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++++++++
>  3 files changed, 21 insertions(+)
> 
> diff --git a/config/common_base b/config/common_base
> index e74febe..2e9fded 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -196,6 +196,8 @@ CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n
>  CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n
>  CONFIG_RTE_IXGBE_INC_VECTOR=y
>  CONFIG_RTE_LIBRTE_IXGBE_BYPASS=n
> +# interval up to 1024 us
> +CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL=-1

I can understand this is to let user to ability to fine tune this value, the
down side is when number of these kind of tune parameters increased, it become
confusing and config options because less useful or perhaps useless.

And overall we are trying to reduce config options we have.

I can see there is already some default values in the header file, what do you
think removing the config option and go with default values defined in header?

>  
>  #
>  # Compile burst-oriented I40E PMD driver
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index e67389f..495e72c 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -5780,6 +5780,13 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
>  		if (vector_idx < base + intr_handle->nb_efd - 1)
>  			vector_idx++;
>  	}
> +
> +	/* As RX queue setting above show, all queues use the vector 0.
> +	 * Set only the ITR value of IXGBE_MISC_VEC_ID.
> +	 */
> +	IXGBE_WRITE_REG(hw, IXGBE_VTEITR(IXGBE_MISC_VEC_ID),
> +			ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL)
> +			| IXGBE_EITR_CNT_WDIS);
>  }
>  
>  /**
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
> index 1db29bd..c779001 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> @@ -58,6 +58,18 @@
>  		IXGBE_EITR_ITR_INT_MASK)
>  
>  
> +#define IXGBE_QUEUE_ITR_INTERVAL_MAX	1024 /* 1024us */
> +#define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT	500 /* 500us */
> +
> +static inline uint16_t
> +ixgbe_calc_itr_interval(int16_t interval)
> +{
> +	if (interval < 0 || interval > IXGBE_QUEUE_ITR_INTERVAL_MAX)
> +		interval = IXGBE_QUEUE_ITR_INTERVAL_DEFAULT;
> +
> +	return IXGBE_EITR_INTERVAL_US(interval);
> +}
> +
>  /* Loopback operation modes */
>  /* 82599 specific loopback operation types */
>  #define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is disabled. */
> 

Re: [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.

[Tonghao Zhang]

On Tue, Apr 17, 2018 at 7:00 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
> On 3/22/2018 1:01 PM, xiangxia.m.yue@gmail.com wrote:
>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>
>> Set EITR interval as default. This patch can improve the
>> performance when we enable the rx-intrrupt to process the
>> packets because we hope rx-intrrupt reduce CPU. For example,
>> the 200us value of EITR makes the performance better with
>> the low CPU.
>>
>> Users can configure the value of ITR via DPDK configuration.
>>
>> The default value of ITR is 500us, compatible with RSC of ixgbe PF,
>> and next patch will use the default value.
>>
>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> ---
>> v1 --> v2:
>> use the configure file, for different user.
>> suggested by Beilei Xing, http://dpdk.org/dev/patchwork/patch/32989
>> ---
>>  config/common_base               |  2 ++
>>  drivers/net/ixgbe/ixgbe_ethdev.c |  7 +++++++
>>  drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++++++++
>>  3 files changed, 21 insertions(+)
>>
>> diff --git a/config/common_base b/config/common_base
>> index e74febe..2e9fded 100644
>> --- a/config/common_base
>> +++ b/config/common_base
>> @@ -196,6 +196,8 @@ CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n
>>  CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n
>>  CONFIG_RTE_IXGBE_INC_VECTOR=y
>>  CONFIG_RTE_LIBRTE_IXGBE_BYPASS=n
>> +# interval up to 1024 us
>> +CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL=-1
>
> I can understand this is to let user to ability to fine tune this value, the
> down side is when number of these kind of tune parameters increased, it become
> confusing and config options because less useful or perhaps useless.
>
> And overall we are trying to reduce config options we have.
>
> I can see there is already some default values in the header file, what do you
> think removing the config option and go with default values defined in header?
v2 used the default value in header, but for configuring it like as
i40e option (CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL).
so v3 makes some changes. These patches may have been applied. For
reducing config option, should we do it in individual patches
to fix the ixgbe/i40e option ?

>>
>>  #
>>  # Compile burst-oriented I40E PMD driver
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>> index e67389f..495e72c 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> @@ -5780,6 +5780,13 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
>>               if (vector_idx < base + intr_handle->nb_efd - 1)
>>                       vector_idx++;
>>       }
>> +
>> +     /* As RX queue setting above show, all queues use the vector 0.
>> +      * Set only the ITR value of IXGBE_MISC_VEC_ID.
>> +      */
>> +     IXGBE_WRITE_REG(hw, IXGBE_VTEITR(IXGBE_MISC_VEC_ID),
>> +                     ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL)
>> +                     | IXGBE_EITR_CNT_WDIS);
>>  }
>>
>>  /**
>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
>> index 1db29bd..c779001 100644
>> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
>> @@ -58,6 +58,18 @@
>>               IXGBE_EITR_ITR_INT_MASK)
>>
>>
>> +#define IXGBE_QUEUE_ITR_INTERVAL_MAX 1024 /* 1024us */
>> +#define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT     500 /* 500us */
>> +
>> +static inline uint16_t
>> +ixgbe_calc_itr_interval(int16_t interval)
>> +{
>> +     if (interval < 0 || interval > IXGBE_QUEUE_ITR_INTERVAL_MAX)
>> +             interval = IXGBE_QUEUE_ITR_INTERVAL_DEFAULT;
>> +
>> +     return IXGBE_EITR_INTERVAL_US(interval);
>> +}
>> +
>>  /* Loopback operation modes */
>>  /* 82599 specific loopback operation types */
>>  #define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is disabled. */
>>
>

Re: [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.

[Ferruh Yigit]

On 4/18/2018 2:01 AM, Tonghao Zhang wrote:
> On Tue, Apr 17, 2018 at 7:00 PM, Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>> On 3/22/2018 1:01 PM, xiangxia.m.yue@gmail.com wrote:
>>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>>
>>> Set EITR interval as default. This patch can improve the
>>> performance when we enable the rx-intrrupt to process the
>>> packets because we hope rx-intrrupt reduce CPU. For example,
>>> the 200us value of EITR makes the performance better with
>>> the low CPU.
>>>
>>> Users can configure the value of ITR via DPDK configuration.
>>>
>>> The default value of ITR is 500us, compatible with RSC of ixgbe PF,
>>> and next patch will use the default value.
>>>
>>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>>> ---
>>> v1 --> v2:
>>> use the configure file, for different user.
>>> suggested by Beilei Xing, http://dpdk.org/dev/patchwork/patch/32989
>>> ---
>>>  config/common_base               |  2 ++
>>>  drivers/net/ixgbe/ixgbe_ethdev.c |  7 +++++++
>>>  drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++++++++
>>>  3 files changed, 21 insertions(+)
>>>
>>> diff --git a/config/common_base b/config/common_base
>>> index e74febe..2e9fded 100644
>>> --- a/config/common_base
>>> +++ b/config/common_base
>>> @@ -196,6 +196,8 @@ CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n
>>>  CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n
>>>  CONFIG_RTE_IXGBE_INC_VECTOR=y
>>>  CONFIG_RTE_LIBRTE_IXGBE_BYPASS=n
>>> +# interval up to 1024 us
>>> +CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL=-1
>>
>> I can understand this is to let user to ability to fine tune this value, the
>> down side is when number of these kind of tune parameters increased, it become
>> confusing and config options because less useful or perhaps useless.
>>
>> And overall we are trying to reduce config options we have.
>>
>> I can see there is already some default values in the header file, what do you
>> think removing the config option and go with default values defined in header?
> v2 used the default value in header, but for configuring it like as
> i40e option (CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL).
> so v3 makes some changes. These patches may have been applied. For
> reducing config option, should we do it in individual patches
> to fix the ixgbe/i40e option ?

Patches are still in Helin's tree, so this up to him how manage.
Since patches are not in main repo yet I would suggest making new version if
fits to Helin.

> 
>>>
>>>  #
>>>  # Compile burst-oriented I40E PMD driver
>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>>> index e67389f..495e72c 100644
>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>>> @@ -5780,6 +5780,13 @@ static void ixgbevf_set_vfta_all(struct rte_eth_dev *dev, bool on)
>>>               if (vector_idx < base + intr_handle->nb_efd - 1)
>>>                       vector_idx++;
>>>       }
>>> +
>>> +     /* As RX queue setting above show, all queues use the vector 0.
>>> +      * Set only the ITR value of IXGBE_MISC_VEC_ID.
>>> +      */
>>> +     IXGBE_WRITE_REG(hw, IXGBE_VTEITR(IXGBE_MISC_VEC_ID),
>>> +                     ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL)
>>> +                     | IXGBE_EITR_CNT_WDIS);
>>>  }
>>>
>>>  /**
>>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h b/drivers/net/ixgbe/ixgbe_ethdev.h
>>> index 1db29bd..c779001 100644
>>> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
>>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
>>> @@ -58,6 +58,18 @@
>>>               IXGBE_EITR_ITR_INT_MASK)
>>>
>>>
>>> +#define IXGBE_QUEUE_ITR_INTERVAL_MAX 1024 /* 1024us */
>>> +#define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT     500 /* 500us */
>>> +
>>> +static inline uint16_t
>>> +ixgbe_calc_itr_interval(int16_t interval)
>>> +{
>>> +     if (interval < 0 || interval > IXGBE_QUEUE_ITR_INTERVAL_MAX)
>>> +             interval = IXGBE_QUEUE_ITR_INTERVAL_DEFAULT;
>>> +
>>> +     return IXGBE_EITR_INTERVAL_US(interval);
>>> +}
>>> +
>>>  /* Loopback operation modes */
>>>  /* 82599 specific loopback operation types */
>>>  #define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback is disabled. */
>>>
>>

Re: [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.

[Zhang, Qi Z]

Hi Tonghao:
	Would you submit v4 with Ferruh' s fix and also do a rebase on next-net?
Thanks
Qi

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
> Sent: Thursday, April 19, 2018 12:10 AM
> To: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Dai,
> Wei <wei.dai@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt
> interval for EITR.
> 
> On 4/18/2018 2:01 AM, Tonghao Zhang wrote:
> > On Tue, Apr 17, 2018 at 7:00 PM, Ferruh Yigit <ferruh.yigit@intel.com>
> wrote:
> >> On 3/22/2018 1:01 PM, xiangxia.m.yue@gmail.com wrote:
> >>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >>>
> >>> Set EITR interval as default. This patch can improve the performance
> >>> when we enable the rx-intrrupt to process the packets because we
> >>> hope rx-intrrupt reduce CPU. For example, the 200us value of EITR
> >>> makes the performance better with the low CPU.
> >>>
> >>> Users can configure the value of ITR via DPDK configuration.
> >>>
> >>> The default value of ITR is 500us, compatible with RSC of ixgbe PF,
> >>> and next patch will use the default value.
> >>>
> >>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >>> ---
> >>> v1 --> v2:
> >>> use the configure file, for different user.
> >>> suggested by Beilei Xing, http://dpdk.org/dev/patchwork/patch/32989
> >>> ---
> >>>  config/common_base               |  2 ++
> >>>  drivers/net/ixgbe/ixgbe_ethdev.c |  7 +++++++
> >>> drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++++++++
> >>>  3 files changed, 21 insertions(+)
> >>>
> >>> diff --git a/config/common_base b/config/common_base index
> >>> e74febe..2e9fded 100644
> >>> --- a/config/common_base
> >>> +++ b/config/common_base
> >>> @@ -196,6 +196,8 @@ CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n
> >>>  CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n
> >>>  CONFIG_RTE_IXGBE_INC_VECTOR=y
> >>>  CONFIG_RTE_LIBRTE_IXGBE_BYPASS=n
> >>> +# interval up to 1024 us
> >>> +CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL=-1
> >>
> >> I can understand this is to let user to ability to fine tune this
> >> value, the down side is when number of these kind of tune parameters
> >> increased, it become confusing and config options because less useful or
> perhaps useless.
> >>
> >> And overall we are trying to reduce config options we have.
> >>
> >> I can see there is already some default values in the header file,
> >> what do you think removing the config option and go with default values
> defined in header?
> > v2 used the default value in header, but for configuring it like as
> > i40e option (CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL).
> > so v3 makes some changes. These patches may have been applied. For
> > reducing config option, should we do it in individual patches to fix
> > the ixgbe/i40e option ?
> 
> Patches are still in Helin's tree, so this up to him how manage.
> Since patches are not in main repo yet I would suggest making new version if
> fits to Helin.
> 
> >
> >>>
> >>>  #
> >>>  # Compile burst-oriented I40E PMD driver diff --git
> >>> a/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> b/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> index e67389f..495e72c 100644
> >>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> >>> @@ -5780,6 +5780,13 @@ static void ixgbevf_set_vfta_all(struct
> rte_eth_dev *dev, bool on)
> >>>               if (vector_idx < base + intr_handle->nb_efd - 1)
> >>>                       vector_idx++;
> >>>       }
> >>> +
> >>> +     /* As RX queue setting above show, all queues use the vector 0.
> >>> +      * Set only the ITR value of IXGBE_MISC_VEC_ID.
> >>> +      */
> >>> +     IXGBE_WRITE_REG(hw, IXGBE_VTEITR(IXGBE_MISC_VEC_ID),
> >>> +
> ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL)
> >>> +                     | IXGBE_EITR_CNT_WDIS);
> >>>  }
> >>>
> >>>  /**
> >>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
> >>> b/drivers/net/ixgbe/ixgbe_ethdev.h
> >>> index 1db29bd..c779001 100644
> >>> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
> >>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
> >>> @@ -58,6 +58,18 @@
> >>>               IXGBE_EITR_ITR_INT_MASK)
> >>>
> >>>
> >>> +#define IXGBE_QUEUE_ITR_INTERVAL_MAX 1024 /* 1024us */
> >>> +#define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT     500 /* 500us */
> >>> +
> >>> +static inline uint16_t
> >>> +ixgbe_calc_itr_interval(int16_t interval) {
> >>> +     if (interval < 0 || interval > IXGBE_QUEUE_ITR_INTERVAL_MAX)
> >>> +             interval = IXGBE_QUEUE_ITR_INTERVAL_DEFAULT;
> >>> +
> >>> +     return IXGBE_EITR_INTERVAL_US(interval); }
> >>> +
> >>>  /* Loopback operation modes */
> >>>  /* 82599 specific loopback operation types */
> >>>  #define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback
> is disabled. */
> >>>
> >>

Re: [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt interval for EITR.

[Tonghao Zhang]

On Thu, Apr 26, 2018 at 9:14 PM, Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> Hi Tonghao:
>         Would you submit v4 with Ferruh' s fix and also do a rebase on next-net?
Yes

> Thanks
> Qi
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ferruh Yigit
>> Sent: Thursday, April 19, 2018 12:10 AM
>> To: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> Cc: Lu, Wenzhuo <wenzhuo.lu@intel.com>; Ananyev, Konstantin
>> <konstantin.ananyev@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Dai,
>> Wei <wei.dai@intel.com>; dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v3 1/5] net/ixgbevf: set the inter-interrupt
>> interval for EITR.
>>
>> On 4/18/2018 2:01 AM, Tonghao Zhang wrote:
>> > On Tue, Apr 17, 2018 at 7:00 PM, Ferruh Yigit <ferruh.yigit@intel.com>
>> wrote:
>> >> On 3/22/2018 1:01 PM, xiangxia.m.yue@gmail.com wrote:
>> >>> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> >>>
>> >>> Set EITR interval as default. This patch can improve the performance
>> >>> when we enable the rx-intrrupt to process the packets because we
>> >>> hope rx-intrrupt reduce CPU. For example, the 200us value of EITR
>> >>> makes the performance better with the low CPU.
>> >>>
>> >>> Users can configure the value of ITR via DPDK configuration.
>> >>>
>> >>> The default value of ITR is 500us, compatible with RSC of ixgbe PF,
>> >>> and next patch will use the default value.
>> >>>
>> >>> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> >>> ---
>> >>> v1 --> v2:
>> >>> use the configure file, for different user.
>> >>> suggested by Beilei Xing, http://dpdk.org/dev/patchwork/patch/32989
>> >>> ---
>> >>>  config/common_base               |  2 ++
>> >>>  drivers/net/ixgbe/ixgbe_ethdev.c |  7 +++++++
>> >>> drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++++++++
>> >>>  3 files changed, 21 insertions(+)
>> >>>
>> >>> diff --git a/config/common_base b/config/common_base index
>> >>> e74febe..2e9fded 100644
>> >>> --- a/config/common_base
>> >>> +++ b/config/common_base
>> >>> @@ -196,6 +196,8 @@ CONFIG_RTE_LIBRTE_IXGBE_DEBUG_DRIVER=n
>> >>>  CONFIG_RTE_LIBRTE_IXGBE_PF_DISABLE_STRIP_CRC=n
>> >>>  CONFIG_RTE_IXGBE_INC_VECTOR=y
>> >>>  CONFIG_RTE_LIBRTE_IXGBE_BYPASS=n
>> >>> +# interval up to 1024 us
>> >>> +CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL=-1
>> >>
>> >> I can understand this is to let user to ability to fine tune this
>> >> value, the down side is when number of these kind of tune parameters
>> >> increased, it become confusing and config options because less useful or
>> perhaps useless.
>> >>
>> >> And overall we are trying to reduce config options we have.
>> >>
>> >> I can see there is already some default values in the header file,
>> >> what do you think removing the config option and go with default values
>> defined in header?
>> > v2 used the default value in header, but for configuring it like as
>> > i40e option (CONFIG_RTE_LIBRTE_IXGBE_ITR_INTERVAL).
>> > so v3 makes some changes. These patches may have been applied. For
>> > reducing config option, should we do it in individual patches to fix
>> > the ixgbe/i40e option ?
>>
>> Patches are still in Helin's tree, so this up to him how manage.
>> Since patches are not in main repo yet I would suggest making new version if
>> fits to Helin.
>>
>> >
>> >>>
>> >>>  #
>> >>>  # Compile burst-oriented I40E PMD driver diff --git
>> >>> a/drivers/net/ixgbe/ixgbe_ethdev.c
>> >>> b/drivers/net/ixgbe/ixgbe_ethdev.c
>> >>> index e67389f..495e72c 100644
>> >>> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
>> >>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>> >>> @@ -5780,6 +5780,13 @@ static void ixgbevf_set_vfta_all(struct
>> rte_eth_dev *dev, bool on)
>> >>>               if (vector_idx < base + intr_handle->nb_efd - 1)
>> >>>                       vector_idx++;
>> >>>       }
>> >>> +
>> >>> +     /* As RX queue setting above show, all queues use the vector 0.
>> >>> +      * Set only the ITR value of IXGBE_MISC_VEC_ID.
>> >>> +      */
>> >>> +     IXGBE_WRITE_REG(hw, IXGBE_VTEITR(IXGBE_MISC_VEC_ID),
>> >>> +
>> ixgbe_calc_itr_interval(RTE_LIBRTE_IXGBE_ITR_INTERVAL)
>> >>> +                     | IXGBE_EITR_CNT_WDIS);
>> >>>  }
>> >>>
>> >>>  /**
>> >>> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.h
>> >>> b/drivers/net/ixgbe/ixgbe_ethdev.h
>> >>> index 1db29bd..c779001 100644
>> >>> --- a/drivers/net/ixgbe/ixgbe_ethdev.h
>> >>> +++ b/drivers/net/ixgbe/ixgbe_ethdev.h
>> >>> @@ -58,6 +58,18 @@
>> >>>               IXGBE_EITR_ITR_INT_MASK)
>> >>>
>> >>>
>> >>> +#define IXGBE_QUEUE_ITR_INTERVAL_MAX 1024 /* 1024us */
>> >>> +#define IXGBE_QUEUE_ITR_INTERVAL_DEFAULT     500 /* 500us */
>> >>> +
>> >>> +static inline uint16_t
>> >>> +ixgbe_calc_itr_interval(int16_t interval) {
>> >>> +     if (interval < 0 || interval > IXGBE_QUEUE_ITR_INTERVAL_MAX)
>> >>> +             interval = IXGBE_QUEUE_ITR_INTERVAL_DEFAULT;
>> >>> +
>> >>> +     return IXGBE_EITR_INTERVAL_US(interval); }
>> >>> +
>> >>>  /* Loopback operation modes */
>> >>>  /* 82599 specific loopback operation types */
>> >>>  #define IXGBE_LPBK_82599_NONE   0x0 /* Default value. Loopback
>> is disabled. */
>> >>>
>> >>
>

Re: [PATCH v3 0/5] ixgbe: fix bugs or just improve.

[Zhang, Helin]

> -----Original Message-----
> From: Zhang, Qi Z
> Sent: Thursday, April 12, 2018 1:22 PM
> To: Tonghao Zhang; Xing, Beilei; Dai, Wei; Zhang, Helin; Lu, Wenzhuo
> Cc: dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve.
> 
> On Mon, Feb 5, 2018 at 8:51 AM,  <xiangxia.m.yue@gmail.com> wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > The patches in the patchset have no dependency. But all of them is
> > about ixgbe or ixgbevf. The patch 1 and 2 add the itr configuration
> > for ixgbe and ixgbevf, the user and developer can configure it for
> > their platform. Other patches refine the ixgbe or ixgbevf.
> >
> > v2 --> v3:
> > remove the patch: http://dpdk.org/dev/patchwork/patch/33698
> >
> > Tonghao Zhang (5):
> >   net/ixgbevf: set the inter-interrupt interval for EITR.
> >   net/ixgbe: set the ITR via configuration.
> >   net/ixgbe: write disable to ITR counter.
> >   net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.
> >   net/ixgbe: remove the unnecessary call rte_intr_enable.
> >
> >  config/common_base               |  2 +
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 79
> > +++++++++++++++++++++++-----------------
> >  drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++
> >  drivers/net/ixgbe/ixgbe_rxtx.c   |  3 +-
> >  4 files changed, 61 insertions(+), 35 deletions(-)
> >
> > --
> > 1.8.3.1
> >
> 
> Acked-by: Qi Zhang <qi.z.zhang@intel.com>
Patch series (1 to 4) applied to dpdk-next-net-intel, patch 5 rejected. Thanks!

/Helin
> 
> Note: patch 5 will be removed as agreed by author.

Re: [PATCH v3 0/5] ixgbe: fix bugs or just improve.

[Zhang, Qi Z]

On Mon, Feb 5, 2018 at 8:51 AM,  <xiangxia.m.yue@gmail.com> wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> The patches in the patchset have no dependency. But all of them is
> about ixgbe or ixgbevf. The patch 1 and 2 add the itr configuration
> for ixgbe and ixgbevf, the user and developer can configure it for
> their platform. Other patches refine the ixgbe or ixgbevf.
>
> v2 --> v3:
> remove the patch: http://dpdk.org/dev/patchwork/patch/33698
>
> Tonghao Zhang (5):
>   net/ixgbevf: set the inter-interrupt interval for EITR.
>   net/ixgbe: set the ITR via configuration.
>   net/ixgbe: write disable to ITR counter.
>   net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.
>   net/ixgbe: remove the unnecessary call rte_intr_enable.
>
>  config/common_base               |  2 +
>  drivers/net/ixgbe/ixgbe_ethdev.c | 79
> +++++++++++++++++++++++-----------------
>  drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++
>  drivers/net/ixgbe/ixgbe_rxtx.c   |  3 +-
>  4 files changed, 61 insertions(+), 35 deletions(-)
>
> --
> 1.8.3.1
>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>

Note: patch 5 will be removed as agreed by author. 

Re: [PATCH v3 0/5] ixgbe: fix bugs or just improve.

[Tonghao Zhang]

On Wed, Apr 11, 2018 at 8:21 PM, Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> Hi tonghao:
>
>> -----Original Message-----
>> From: Tonghao Zhang [mailto:xiangxia.m.yue@gmail.com]
>> Sent: Monday, April 9, 2018 8:53 AM
>> To: Zhang, Qi Z <qi.z.zhang@intel.com>
>> Cc: Xing, Beilei <beilei.xing@intel.com>; Dai, Wei <wei.dai@intel.com>; Zhang,
>> Helin <helin.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
>> dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve.
>>
>> On Wed, Apr 4, 2018 at 10:48 AM, Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
>> >
>> >
>> >> -----Original Message-----
>> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tonghao Zhang
>> >> Sent: Thursday, March 8, 2018 6:57 PM
>> >> To: Xing, Beilei <beilei.xing@intel.com>; Dai, Wei
>> >> <wei.dai@intel.com>; Zhang, Helin <helin.zhang@intel.com>; Lu,
>> >> Wenzhuo <wenzhuo.lu@intel.com>
>> >> Cc: dev@dpdk.org; Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> >> Subject: Re: [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve.
>> >>
>> >> ping ....
>> >>
>> >> On Mon, Feb 5, 2018 at 8:51 AM,  <xiangxia.m.yue@gmail.com> wrote:
>> >> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> >> >
>> >> > The patches in the patchset have no dependency. But all of them is
>> >> > about ixgbe or ixgbevf. The patch 1 and 2 add the itr configuration
>> >> > for ixgbe and ixgbevf, the user and developer can configure it for
>> >> > their platform. Other patches refine the ixgbe or ixgbevf.
>> >> >
>> >> > v2 --> v3:
>> >> > remove the patch: http://dpdk.org/dev/patchwork/patch/33698
>> >> >
>> >> > Tonghao Zhang (5):
>> >> >   net/ixgbevf: set the inter-interrupt interval for EITR.
>> >> >   net/ixgbe: set the ITR via configuration.
>> >> >   net/ixgbe: write disable to ITR counter.
>> >> >   net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.
>> >> >   net/ixgbe: remove the unnecessary call rte_intr_enable.
>> >> >
>> >> >  config/common_base               |  2 +
>> >> >  drivers/net/ixgbe/ixgbe_ethdev.c | 79
>> >> > +++++++++++++++++++++++-----------------
>> >> >  drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++
>> >> >  drivers/net/ixgbe/ixgbe_rxtx.c   |  3 +-
>> >> >  4 files changed, 61 insertions(+), 35 deletions(-)
>> >> >
>> >> > --
>> >> > 1.8.3.1
>> >> >
>> >
>> > Review-by: Qi Zhang <qi.z.zhang@intel.com>
>> >
>> > Minor typo error in patch1 need to be fixed.
>> Should I sent v4 ?
>
> One more capture on patch 5,
> https://dpdk.org/dev/patchwork/patch/36433/
> Remove rte_intr_enable in interrupt handler may not work with legacy interrupt mode which ixgbe supposed to support.
Remove it, and thanks for your review.

> It's better to so some test on that mode if you want patch 5 be applied,
> or we can just remove patch 5 and I'd like to ack patch 1-4, what do you think?
>
> Regards
> Qi
>
>

Re: [PATCH v3 0/5] ixgbe: fix bugs or just improve.

[Zhang, Qi Z]

Hi tonghao:

> -----Original Message-----
> From: Tonghao Zhang [mailto:xiangxia.m.yue@gmail.com]
> Sent: Monday, April 9, 2018 8:53 AM
> To: Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: Xing, Beilei <beilei.xing@intel.com>; Dai, Wei <wei.dai@intel.com>; Zhang,
> Helin <helin.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve.
> 
> On Wed, Apr 4, 2018 at 10:48 AM, Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tonghao Zhang
> >> Sent: Thursday, March 8, 2018 6:57 PM
> >> To: Xing, Beilei <beilei.xing@intel.com>; Dai, Wei
> >> <wei.dai@intel.com>; Zhang, Helin <helin.zhang@intel.com>; Lu,
> >> Wenzhuo <wenzhuo.lu@intel.com>
> >> Cc: dev@dpdk.org; Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >> Subject: Re: [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve.
> >>
> >> ping ....
> >>
> >> On Mon, Feb 5, 2018 at 8:51 AM,  <xiangxia.m.yue@gmail.com> wrote:
> >> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >> >
> >> > The patches in the patchset have no dependency. But all of them is
> >> > about ixgbe or ixgbevf. The patch 1 and 2 add the itr configuration
> >> > for ixgbe and ixgbevf, the user and developer can configure it for
> >> > their platform. Other patches refine the ixgbe or ixgbevf.
> >> >
> >> > v2 --> v3:
> >> > remove the patch: http://dpdk.org/dev/patchwork/patch/33698
> >> >
> >> > Tonghao Zhang (5):
> >> >   net/ixgbevf: set the inter-interrupt interval for EITR.
> >> >   net/ixgbe: set the ITR via configuration.
> >> >   net/ixgbe: write disable to ITR counter.
> >> >   net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.
> >> >   net/ixgbe: remove the unnecessary call rte_intr_enable.
> >> >
> >> >  config/common_base               |  2 +
> >> >  drivers/net/ixgbe/ixgbe_ethdev.c | 79
> >> > +++++++++++++++++++++++-----------------
> >> >  drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++
> >> >  drivers/net/ixgbe/ixgbe_rxtx.c   |  3 +-
> >> >  4 files changed, 61 insertions(+), 35 deletions(-)
> >> >
> >> > --
> >> > 1.8.3.1
> >> >
> >
> > Review-by: Qi Zhang <qi.z.zhang@intel.com>
> >
> > Minor typo error in patch1 need to be fixed.
> Should I sent v4 ?

One more capture on patch 5,
https://dpdk.org/dev/patchwork/patch/36433/
Remove rte_intr_enable in interrupt handler may not work with legacy interrupt mode which ixgbe supposed to support.

It's better to so some test on that mode if you want patch 5 be applied, 
or we can just remove patch 5 and I'd like to ack patch 1-4, what do you think?

Regards
Qi

Re: [PATCH v3 0/5] ixgbe: fix bugs or just improve.

[Tonghao Zhang]

On Wed, Apr 4, 2018 at 10:48 AM, Zhang, Qi Z <qi.z.zhang@intel.com> wrote:
>
>
>> -----Original Message-----
>> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tonghao Zhang
>> Sent: Thursday, March 8, 2018 6:57 PM
>> To: Xing, Beilei <beilei.xing@intel.com>; Dai, Wei <wei.dai@intel.com>; Zhang,
>> Helin <helin.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
>> Cc: dev@dpdk.org; Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> Subject: Re: [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve.
>>
>> ping ....
>>
>> On Mon, Feb 5, 2018 at 8:51 AM,  <xiangxia.m.yue@gmail.com> wrote:
>> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>> >
>> > The patches in the patchset have no dependency. But all of them is
>> > about ixgbe or ixgbevf. The patch 1 and 2 add the itr configuration
>> > for ixgbe and ixgbevf, the user and developer can configure it for
>> > their platform. Other patches refine the ixgbe or ixgbevf.
>> >
>> > v2 --> v3:
>> > remove the patch: http://dpdk.org/dev/patchwork/patch/33698
>> >
>> > Tonghao Zhang (5):
>> >   net/ixgbevf: set the inter-interrupt interval for EITR.
>> >   net/ixgbe: set the ITR via configuration.
>> >   net/ixgbe: write disable to ITR counter.
>> >   net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.
>> >   net/ixgbe: remove the unnecessary call rte_intr_enable.
>> >
>> >  config/common_base               |  2 +
>> >  drivers/net/ixgbe/ixgbe_ethdev.c | 79
>> > +++++++++++++++++++++++-----------------
>> >  drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++
>> >  drivers/net/ixgbe/ixgbe_rxtx.c   |  3 +-
>> >  4 files changed, 61 insertions(+), 35 deletions(-)
>> >
>> > --
>> > 1.8.3.1
>> >
>
> Review-by: Qi Zhang <qi.z.zhang@intel.com>
>
> Minor typo error in patch1 need to be fixed.
Should I sent v4 ?

Re: [PATCH v3 0/5] ixgbe: fix bugs or just improve.

[Zhang, Qi Z]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Tonghao Zhang
> Sent: Thursday, March 8, 2018 6:57 PM
> To: Xing, Beilei <beilei.xing@intel.com>; Dai, Wei <wei.dai@intel.com>; Zhang,
> Helin <helin.zhang@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>
> Cc: dev@dpdk.org; Tonghao Zhang <xiangxia.m.yue@gmail.com>
> Subject: Re: [dpdk-dev] [PATCH v3 0/5] ixgbe: fix bugs or just improve.
> 
> ping ....
> 
> On Mon, Feb 5, 2018 at 8:51 AM,  <xiangxia.m.yue@gmail.com> wrote:
> > From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
> >
> > The patches in the patchset have no dependency. But all of them is
> > about ixgbe or ixgbevf. The patch 1 and 2 add the itr configuration
> > for ixgbe and ixgbevf, the user and developer can configure it for
> > their platform. Other patches refine the ixgbe or ixgbevf.
> >
> > v2 --> v3:
> > remove the patch: http://dpdk.org/dev/patchwork/patch/33698
> >
> > Tonghao Zhang (5):
> >   net/ixgbevf: set the inter-interrupt interval for EITR.
> >   net/ixgbe: set the ITR via configuration.
> >   net/ixgbe: write disable to ITR counter.
> >   net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.
> >   net/ixgbe: remove the unnecessary call rte_intr_enable.
> >
> >  config/common_base               |  2 +
> >  drivers/net/ixgbe/ixgbe_ethdev.c | 79
> > +++++++++++++++++++++++-----------------
> >  drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++
> >  drivers/net/ixgbe/ixgbe_rxtx.c   |  3 +-
> >  4 files changed, 61 insertions(+), 35 deletions(-)
> >
> > --
> > 1.8.3.1
> >

Review-by: Qi Zhang <qi.z.zhang@intel.com>

Minor typo error in patch1 need to be fixed.

Re: [PATCH v3 0/5] ixgbe: fix bugs or just improve.

[Tonghao Zhang]

ping ....

On Mon, Feb 5, 2018 at 8:51 AM,  <xiangxia.m.yue@gmail.com> wrote:
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> The patches in the patchset have no dependency. But all of them
> is about ixgbe or ixgbevf. The patch 1 and 2 add the itr configuration
> for ixgbe and ixgbevf, the user and developer can configure it for
> their platform. Other patches refine the ixgbe or ixgbevf.
>
> v2 --> v3:
> remove the patch: http://dpdk.org/dev/patchwork/patch/33698
>
> Tonghao Zhang (5):
>   net/ixgbevf: set the inter-interrupt interval for EITR.
>   net/ixgbe: set the ITR via configuration.
>   net/ixgbe: write disable to ITR counter.
>   net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.
>   net/ixgbe: remove the unnecessary call rte_intr_enable.
>
>  config/common_base               |  2 +
>  drivers/net/ixgbe/ixgbe_ethdev.c | 79 +++++++++++++++++++++++-----------------
>  drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++
>  drivers/net/ixgbe/ixgbe_rxtx.c   |  3 +-
>  4 files changed, 61 insertions(+), 35 deletions(-)
>
> --
> 1.8.3.1
>

[PATCH v3 0/5] ixgbe: fix bugs or just improve.

[xiangxia.m.yue]

From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

The patches in the patchset have no dependency. But all of them
is about ixgbe or ixgbevf. The patch 1 and 2 add the itr configuration
for ixgbe and ixgbevf, the user and developer can configure it for
their platform. Other patches refine the ixgbe or ixgbevf.

v2 --> v3:
remove the patch: http://dpdk.org/dev/patchwork/patch/33698

Tonghao Zhang (5):
  net/ixgbevf: set the inter-interrupt interval for EITR.
  net/ixgbe: set the ITR via configuration.
  net/ixgbe: write disable to ITR counter.
  net/ixgbevf: save IXGBE_VTEIMS to intr->mask for performance.
  net/ixgbe: remove the unnecessary call rte_intr_enable.

 config/common_base               |  2 +
 drivers/net/ixgbe/ixgbe_ethdev.c | 79 +++++++++++++++++++++++-----------------
 drivers/net/ixgbe/ixgbe_ethdev.h | 12 ++++++
 drivers/net/ixgbe/ixgbe_rxtx.c   |  3 +-
 4 files changed, 61 insertions(+), 35 deletions(-)

-- 
1.8.3.1