[dpdk-dev] [PATCH v4 0/7] e1000: add rx interrupt support

[dpdk-dev] [PATCH v4 0/7] e1000: add rx interrupt support

[Yong Liu]

This patch set will enable interrup for physical and emulated e1000 device.
Rx queue interrupt will work with uio driver or vfio driver with msi mode.
l3fwd-power will disable interrupt immediately when wake-up for that e1000 not
support interrupt auto clear.
LSC and rxq interrupt will be seperated for e1000 can only support one
interrupt cause in the same time.

The patch set is developed based on one previous patch set
"[PATCH v1 00/11] interrupt mode for i40e"
http://www.dpdk.org/ml/archives/dev/2015-September/023903.html

v4 change:
	release note merged with first patch

v3 change:
    add in release note

v2 changes:
    describe interrupt mode work with uio and vfio+msi
    replace attribuite __unused__ with __rte_unused

Marvin Liu (7):
  e1000: add rx interrupt support
  e1000: separate lsc and rxq interrupt disable function
  e1000: add ethdev rxq enable and disable function
  e1000: add rxq interrupt handler
  e1000: check lsc and rxq not enable in the same time
  e1000: lsc interrupt setup function only enable itself
  l3fwd-power: disable interrupt when wake up from sleep

 doc/guides/rel_notes/release_2_2.rst |   1 +
 drivers/net/e1000/em_ethdev.c        | 181 +++++++++++++++++++++++++++++++----
 examples/l3fwd-power/main.c          |   1 +
 3 files changed, 164 insertions(+), 19 deletions(-)

-- 
1.9.3

[dpdk-dev] [PATCH v4 1/7] e1000: enable device rx queue interrupt

[Yong Liu]

Enable rx queue interrupt support on e1000 physical and emulated device.

Signed-off-by: Marvin Liu <yong.liu@intel.com>

diff --git a/doc/guides/rel_notes/release_2_2.rst b/doc/guides/rel_notes/release_2_2.rst
index be6f827..7655148 100644
--- a/doc/guides/rel_notes/release_2_2.rst
+++ b/doc/guides/rel_notes/release_2_2.rst
@@ -23,6 +23,7 @@ New Features
 
 * **Added vhost-user multiple queue support.**
 
+* **Support interrupt mode on e1000.**
 
 Resolved Issues
 ---------------
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 912f5dd..3be8269 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -81,6 +81,7 @@ static int eth_em_flow_ctrl_get(struct rte_eth_dev *dev,
 static int eth_em_flow_ctrl_set(struct rte_eth_dev *dev,
 				struct rte_eth_fc_conf *fc_conf);
 static int eth_em_interrupt_setup(struct rte_eth_dev *dev);
+static int eth_em_rxq_interrupt_setup(struct rte_eth_dev *dev);
 static int eth_em_interrupt_get_status(struct rte_eth_dev *dev);
 static int eth_em_interrupt_action(struct rte_eth_dev *dev);
 static void eth_em_interrupt_handler(struct rte_intr_handle *handle,
@@ -107,6 +108,7 @@ static void em_vlan_hw_strip_disable(struct rte_eth_dev *dev);
 static void eth_em_vlan_filter_set(struct rte_eth_dev *dev,
 					uint16_t vlan_id, int on);
 */
+static void em_rxq_intr_enable(struct e1000_hw *hw);
 static int eth_em_led_on(struct rte_eth_dev *dev);
 static int eth_em_led_off(struct rte_eth_dev *dev);
 
@@ -608,6 +610,9 @@ eth_em_start(struct rte_eth_dev *dev)
 			return ret;
 		}
 	}
+	/* check if rxq interrupt is enabled */
+	if (dev->data->dev_conf.intr_conf.rxq != 0)
+		eth_em_rxq_interrupt_setup(dev);
 
 	adapter->stopped = 0;
 
@@ -1277,6 +1282,42 @@ eth_em_interrupt_setup(struct rte_eth_dev *dev)
 }
 
 /*
+ * It clears the interrupt causes and enables the interrupt.
+ * It will be called once only during nic initialized.
+ *
+ * @param dev
+ *  Pointer to struct rte_eth_dev.
+ *
+ * @return
+ *  - On success, zero.
+ *  - On failure, a negative value.
+ */
+static int
+eth_em_rxq_interrupt_setup(struct rte_eth_dev *dev)
+{
+	struct e1000_hw *hw =
+	E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	E1000_READ_REG(hw, E1000_ICR);
+	em_rxq_intr_enable(hw);
+	return 0;
+}
+
+/*
+ * It enable receive packet interrupt.
+ * @param hw
+ * Pointer to struct e1000_hw
+ *
+ * @return
+ */
+static void
+em_rxq_intr_enable(struct e1000_hw *hw)
+{
+	E1000_WRITE_REG(hw, E1000_IMS, E1000_IMS_RXT0);
+	E1000_WRITE_FLUSH(hw);
+}
+
+/*
  * It reads ICR and gets interrupt causes, check it and set a bit flag
  * to update link status.
  *
-- 
1.9.3

[dpdk-dev] [PATCH v4 2/7] e1000: separate lsc and rxq interrupt disable function

[Yong Liu]

Separate lsc and rxq interrupt for they have different interrupt handlers.

Signed-off-by: Marvin Liu <yong.liu@intel.com>

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 3be8269..39f330a 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -108,11 +108,12 @@ static void em_vlan_hw_strip_disable(struct rte_eth_dev *dev);
 static void eth_em_vlan_filter_set(struct rte_eth_dev *dev,
 					uint16_t vlan_id, int on);
 */
+static void em_lsc_intr_disable(struct e1000_hw *hw);
 static void em_rxq_intr_enable(struct e1000_hw *hw);
+static void em_rxq_intr_disable(struct e1000_hw *hw);
 static int eth_em_led_on(struct rte_eth_dev *dev);
 static int eth_em_led_off(struct rte_eth_dev *dev);
 
-static void em_intr_disable(struct e1000_hw *hw);
 static int em_get_rx_buffer_size(struct e1000_hw *hw);
 static void eth_em_rar_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr,
 		uint32_t index, uint32_t pool);
@@ -640,7 +641,9 @@ eth_em_stop(struct rte_eth_dev *dev)
 	struct rte_eth_link link;
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	em_intr_disable(hw);
+	em_rxq_intr_disable(hw);
+	em_lsc_intr_disable(hw);
+
 	e1000_reset_hw(hw);
 	if (hw->mac.type >= e1000_82544)
 		E1000_WRITE_REG(hw, E1000_WUC, 0);
@@ -1254,13 +1257,7 @@ eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	}
 }
 
-static void
-em_intr_disable(struct e1000_hw *hw)
-{
-	E1000_WRITE_REG(hw, E1000_IMC, ~0);
-}
-
-/**
+/*
  * It enables the interrupt mask and then enable the interrupt.
  *
  * @param dev
@@ -1304,6 +1301,35 @@ eth_em_rxq_interrupt_setup(struct rte_eth_dev *dev)
 }
 
 /*
+ * It disabled lsc interrupt.
+ * @param hw
+ * Pointer to struct e1000_hw
+ *
+ * @return
+ */
+static void
+em_lsc_intr_disable(struct e1000_hw *hw)
+{
+	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_LSC);
+	E1000_WRITE_FLUSH(hw);
+}
+
+/*
+ * It disabled receive packet interrupt.
+ * @param hw
+ * Pointer to struct e1000_hw
+ *
+ * @return
+ */
+static void
+em_rxq_intr_disable(struct e1000_hw *hw)
+{
+	E1000_READ_REG(hw, E1000_ICR);
+	E1000_WRITE_REG(hw, E1000_IMC, E1000_IMS_RXT0);
+	E1000_WRITE_FLUSH(hw);
+}
+
+/*
  * It enable receive packet interrupt.
  * @param hw
  * Pointer to struct e1000_hw
-- 
1.9.3

[dpdk-dev] [PATCH v4 3/7] e1000: add ethdev rxq enable and disable function

[Yong Liu]

Implement rxq interrupt related functions in eth_dev_ops structure.

Signed-off-by: Marvin Liu <yong.liu@intel.com>

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 39f330a..6dc2534 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -108,9 +108,13 @@ static void em_vlan_hw_strip_disable(struct rte_eth_dev *dev);
 static void eth_em_vlan_filter_set(struct rte_eth_dev *dev,
 					uint16_t vlan_id, int on);
 */
+
+static int eth_em_rx_queue_intr_enable(struct rte_eth_dev *dev, uint16_t queue_id);
+static int eth_em_rx_queue_intr_disable(struct rte_eth_dev *dev, uint16_t queue_id);
 static void em_lsc_intr_disable(struct e1000_hw *hw);
 static void em_rxq_intr_enable(struct e1000_hw *hw);
 static void em_rxq_intr_disable(struct e1000_hw *hw);
+
 static int eth_em_led_on(struct rte_eth_dev *dev);
 static int eth_em_led_off(struct rte_eth_dev *dev);
 
@@ -162,6 +166,8 @@ static const struct eth_dev_ops eth_em_ops = {
 	.rx_descriptor_done   = eth_em_rx_descriptor_done,
 	.tx_queue_setup       = eth_em_tx_queue_setup,
 	.tx_queue_release     = eth_em_tx_queue_release,
+	.rx_queue_intr_enable = eth_em_rx_queue_intr_enable,
+	.rx_queue_intr_disable = eth_em_rx_queue_intr_disable,
 	.dev_led_on           = eth_em_led_on,
 	.dev_led_off          = eth_em_led_off,
 	.flow_ctrl_get        = eth_em_flow_ctrl_get,
@@ -890,6 +896,27 @@ eth_em_stats_reset(struct rte_eth_dev *dev)
 	memset(hw_stats, 0, sizeof(*hw_stats));
 }
 
+static int
+eth_em_rx_queue_intr_enable(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id)
+{
+	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	em_rxq_intr_enable(hw);
+	rte_intr_enable(&(dev->pci_dev->intr_handle));
+
+	return 0;
+}
+
+static int
+eth_em_rx_queue_intr_disable(struct rte_eth_dev *dev, __rte_unused uint16_t queue_id)
+{
+	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+
+	em_rxq_intr_disable(hw);
+
+	return 0;
+}
+
 static uint32_t
 em_get_max_pktlen(const struct e1000_hw *hw)
 {
-- 
1.9.3

[dpdk-dev] [PATCH v4 4/7] e1000: add rxq interrupt handler

[Yong Liu]

When datapath rxq interupt is enabled, enable related device rxq.
Remove the interrupt handler after device stopped.

Signed-off-by: Marvin Liu <yong.liu@intel.com>

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index 6dc2534..fc3cc1e 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -501,7 +501,9 @@ eth_em_start(struct rte_eth_dev *dev)
 		E1000_DEV_PRIVATE(dev->data->dev_private);
 	struct e1000_hw *hw =
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_intr_handle *intr_handle = &dev->pci_dev->intr_handle;
 	int ret, mask;
+	uint32_t intr_vector = 0;
 
 	PMD_INIT_FUNC_TRACE();
 
@@ -537,6 +539,26 @@ eth_em_start(struct rte_eth_dev *dev)
 	/* Configure for OS presence */
 	em_init_manageability(hw);
 
+	if (dev->data->dev_conf.intr_conf.rxq != 0) {
+		intr_vector = dev->data->nb_rx_queues;
+		if (rte_intr_efd_enable(intr_handle, intr_vector))
+			return -1;
+	}
+
+	if (rte_intr_dp_is_en(intr_handle)) {
+		intr_handle->intr_vec =
+			rte_zmalloc("intr_vec",
+					dev->data->nb_rx_queues * sizeof(int), 0);
+		if (intr_handle->intr_vec == NULL) {
+			PMD_INIT_LOG(ERR, "Failed to allocate %d rx_queues"
+						" intr_vec\n", dev->data->nb_rx_queues);
+			return -ENOMEM;
+		}
+
+		/* enable rx interrupt */
+		em_rxq_intr_enable(hw);
+	}
+
 	eth_em_tx_init(dev);
 
 	ret = eth_em_rx_init(dev);
@@ -621,6 +643,8 @@ eth_em_start(struct rte_eth_dev *dev)
 	if (dev->data->dev_conf.intr_conf.rxq != 0)
 		eth_em_rxq_interrupt_setup(dev);
 
+	rte_intr_enable(intr_handle);
+
 	adapter->stopped = 0;
 
 	PMD_INIT_LOG(DEBUG, "<<");
@@ -646,6 +670,7 @@ eth_em_stop(struct rte_eth_dev *dev)
 {
 	struct rte_eth_link link;
 	struct e1000_hw *hw = E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	struct rte_intr_handle *intr_handle = &dev->pci_dev->intr_handle;
 
 	em_rxq_intr_disable(hw);
 	em_lsc_intr_disable(hw);
@@ -662,6 +687,13 @@ eth_em_stop(struct rte_eth_dev *dev)
 	/* clear the recorded link status */
 	memset(&link, 0, sizeof(link));
 	rte_em_dev_atomic_write_link_status(dev, &link);
+
+	/* Clean datapath event and queue/vec mapping */
+	rte_intr_efd_disable(intr_handle);
+	if (intr_handle->intr_vec != NULL) {
+		rte_free(intr_handle->intr_vec);
+		intr_handle->intr_vec = NULL;
+	}
 }
 
 static void
-- 
1.9.3

[dpdk-dev] [PATCH v4 5/7] e1000: check lsc and rxq not enable in the same time

[Yong Liu]

e1000 only support one type of interrupt cause, so remove lsc interrupt
handler if rxq enabled.

Signed-off-by: Marvin Liu <yong.liu@intel.com>

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index fc3cc1e..b1e0c3c 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -630,14 +630,22 @@ eth_em_start(struct rte_eth_dev *dev)
 	}
 	e1000_setup_link(hw);
 
-	/* check if lsc interrupt feature is enabled */
-	if (dev->data->dev_conf.intr_conf.lsc != 0) {
-		ret = eth_em_interrupt_setup(dev);
-		if (ret) {
-			PMD_INIT_LOG(ERR, "Unable to setup interrupts");
-			em_dev_clear_queues(dev);
-			return ret;
-		}
+	if (rte_intr_allow_others(intr_handle)) {
+		/* check if lsc interrupt is enabled */
+		if (dev->data->dev_conf.intr_conf.lsc != 0)
+			ret = eth_em_interrupt_setup(dev);
+			if (ret) {
+				PMD_INIT_LOG(ERR, "Unable to setup interrupts");
+				em_dev_clear_queues(dev);
+				return ret;
+			}
+	} else {
+		rte_intr_callback_unregister(intr_handle,
+						eth_em_interrupt_handler,
+						(void *)dev);
+		if (dev->data->dev_conf.intr_conf.lsc != 0)
+			PMD_INIT_LOG(INFO, "lsc won't enable because of"
+				     " no intr multiplex\n");
 	}
 	/* check if rxq interrupt is enabled */
 	if (dev->data->dev_conf.intr_conf.rxq != 0)
@@ -688,6 +696,12 @@ eth_em_stop(struct rte_eth_dev *dev)
 	memset(&link, 0, sizeof(link));
 	rte_em_dev_atomic_write_link_status(dev, &link);
 
+	if (!rte_intr_allow_others(intr_handle))
+		/* resume to the default handler */
+		rte_intr_callback_register(intr_handle,
+					   eth_em_interrupt_handler,
+					   (void *)dev);
+
 	/* Clean datapath event and queue/vec mapping */
 	rte_intr_efd_disable(intr_handle);
 	if (intr_handle->intr_vec != NULL) {
-- 
1.9.3

[dpdk-dev] [PATCH v4 6/7] e1000: lsc interrupt setup function only enable itself

[Yong Liu]

Only mask lsc interrupt bit when setup device interrupt.

Signed-off-by: Marvin Liu <yong.liu@intel.com>

diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index b1e0c3c..d2d017c 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -1343,11 +1343,14 @@ eth_em_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 static int
 eth_em_interrupt_setup(struct rte_eth_dev *dev)
 {
+	uint32_t regval;
 	struct e1000_hw *hw =
 		E1000_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 
-	E1000_WRITE_REG(hw, E1000_IMS, E1000_ICR_LSC);
-	rte_intr_enable(&(dev->pci_dev->intr_handle));
+	/* clear interrupt */
+	E1000_READ_REG(hw, E1000_ICR);
+	regval = E1000_READ_REG(hw, E1000_IMS);
+	E1000_WRITE_REG(hw, E1000_IMS, regval | E1000_ICR_LSC);
 	return (0);
 }
 
-- 
1.9.3

[dpdk-dev] [PATCH v4 7/7] l3fwd-power: disable interrupt when wake up from sleep

[Yong Liu]

Signed-off-by: Marvin Liu <yong.liu@intel.com>

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index 8bb88ce..9175989 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -798,6 +798,7 @@ sleep_until_rx_interrupt(int num)
 		port_id = ((uintptr_t)data) >> CHAR_BIT;
 		queue_id = ((uintptr_t)data) &
 			RTE_LEN2MASK(CHAR_BIT, uint8_t);
+		rte_eth_dev_rx_intr_disable(port_id, queue_id);
 		RTE_LOG(INFO, L3FWD_POWER,
 			"lcore %u is waked up from rx interrupt on"
 			" port %d queue %d\n",
-- 
1.9.3

Re: [dpdk-dev] [PATCH v4 0/7] e1000: add rx interrupt support

[Thomas Monjalon]

2015-10-29 23:56, Yong Liu:
> This patch set will enable interrup for physical and emulated e1000 device.
> Rx queue interrupt will work with uio driver or vfio driver with msi mode.
> l3fwd-power will disable interrupt immediately when wake-up for that e1000 not
> support interrupt auto clear.
> LSC and rxq interrupt will be seperated for e1000 can only support one
> interrupt cause in the same time.

Don't you think it should be explained in a doc?
	doc/guides/nics/e1000em.rst
I'm especially confused by the need of changing an example for this PMD.
Does it mean the API behaviour must be changed?

[...]
> Marvin Liu (7):
>   e1000: add rx interrupt support
>   e1000: separate lsc and rxq interrupt disable function
>   e1000: add ethdev rxq enable and disable function
>   e1000: add rxq interrupt handler
>   e1000: check lsc and rxq not enable in the same time
>   e1000: lsc interrupt setup function only enable itself
>   l3fwd-power: disable interrupt when wake up from sleep

Announcing a support in first patch and making it work later
is a strange logic to review.

You forgot to keep the Acked-by: Cunming Liang <cunming.liang@intel.com>

Applied in this order:
	e1000: restrict link interrupt setup scope
	e1000: separate link and Rx interrupt disabling
	e1000: support Rx interrupt setup
	e1000: add Rx interrupt handler
	l3fwd-power: disable Rx interrupt when waking up
Thanks

Re: [dpdk-dev] [PATCH v4 0/7] e1000: add rx interrupt support

[Liu, Yong]

Thanks Thomas.

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, October 30, 2015 2:19 AM
> To: Liu, Yong
> Cc: dev@dpdk.org; Mcnamara, John
> Subject: Re: [dpdk-dev] [PATCH v4 0/7] e1000: add rx interrupt support
> 
> 2015-10-29 23:56, Yong Liu:
> > This patch set will enable interrup for physical and emulated e1000
> device.
> > Rx queue interrupt will work with uio driver or vfio driver with msi
> mode.
> > l3fwd-power will disable interrupt immediately when wake-up for that
> e1000 not
> > support interrupt auto clear.
> > LSC and rxq interrupt will be seperated for e1000 can only support one
> > interrupt cause in the same time.
> 
> Don't you think it should be explained in a doc?
> 	doc/guides/nics/e1000em.rst

Yes, will update those information in the doc.

> I'm especially confused by the need of changing an example for this PMD.
> Does it mean the API behaviour must be changed?
> 

After return back from sleep, Rx interrupt should always disabled until return to sleep.
Some NICs like Niantic and Fortville support interrupt auto disable, so it worked normally.
But e1000 not support that, so add rte_eth_dev_rx_intr_disable here to disable related interrupt.

> [...]
> > Marvin Liu (7):
> >   e1000: add rx interrupt support
> >   e1000: separate lsc and rxq interrupt disable function
> >   e1000: add ethdev rxq enable and disable function
> >   e1000: add rxq interrupt handler
> >   e1000: check lsc and rxq not enable in the same time
> >   e1000: lsc interrupt setup function only enable itself
> >   l3fwd-power: disable interrupt when wake up from sleep
> 
> Announcing a support in first patch and making it work later
> is a strange logic to review.
> 
> You forgot to keep the Acked-by: Cunming Liang <cunming.liang@intel.com>
> 
> Applied in this order:
> 	e1000: restrict link interrupt setup scope
> 	e1000: separate link and Rx interrupt disabling
> 	e1000: support Rx interrupt setup
> 	e1000: add Rx interrupt handler
> 	l3fwd-power: disable Rx interrupt when waking up
> Thanks