[dpdk-dev] [PATCH 1/3] net/enic: cleanup interrupt setup when stopping port

[dpdk-dev] [PATCH 1/3] net/enic: cleanup interrupt setup when stopping port

[Nelson Escobar]

enic_disable() wasn't calling rte_intr_disable() or
rte_intr_callback_unregister().  If stopping/starting a port, the
latter omission would result in the same interrupt callback being
registered multiple times, which would then cause it to be called
multiple times on every interrupt.

Fixes: fefed3d1e62c ("enic: new driver")

Signed-off-by: Nelson Escobar <neescoba@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 drivers/net/enic/enic_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index b4ca371..aec24d2 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -798,6 +798,10 @@ int enic_disable(struct enic *enic)
 
 	vnic_intr_mask(&enic->intr);
 	(void)vnic_intr_masked(&enic->intr); /* flush write */
+	rte_intr_disable(&enic->pdev->intr_handle);
+	rte_intr_callback_unregister(&enic->pdev->intr_handle,
+				     enic_intr_handler,
+				     (void *)enic->rte_dev);
 
 	vnic_dev_disable(enic->vdev);
 
-- 
2.7.0

[dpdk-dev] [PATCH 2/3] net/enic: move code checking link status

[Nelson Escobar]

Move link check code so that it can be used by interrupt handler.

Signed-off-by: Nelson Escobar <neescoba@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 drivers/net/enic/enic.h        |  1 +
 drivers/net/enic/enic_ethdev.c | 10 +---------
 drivers/net/enic/enic_main.c   | 15 +++++++++++++++
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/net/enic/enic.h b/drivers/net/enic/enic.h
index 4c16ef1..5c9345f 100644
--- a/drivers/net/enic/enic.h
+++ b/drivers/net/enic/enic.h
@@ -273,4 +273,5 @@ uint16_t enic_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts,
 uint16_t enic_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 			       uint16_t nb_pkts);
 int enic_set_mtu(struct enic *enic, uint16_t new_mtu);
+int enic_link_update(struct enic *enic);
 #endif /* _ENIC_H_ */
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 6cbecb1..e10c824 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -403,17 +403,9 @@ static int enicpmd_dev_link_update(struct rte_eth_dev *eth_dev,
 	__rte_unused int wait_to_complete)
 {
 	struct enic *enic = pmd_priv(eth_dev);
-	int ret;
-	int link_status = 0;
 
 	ENICPMD_FUNC_TRACE();
-	link_status = enic_get_link_status(enic);
-	ret = (link_status == enic->link_status);
-	enic->link_status = link_status;
-	eth_dev->data->dev_link.link_status = link_status;
-	eth_dev->data->dev_link.link_duplex = ETH_LINK_FULL_DUPLEX;
-	eth_dev->data->dev_link.link_speed = vnic_dev_port_speed(enic->vdev);
-	return ret;
+	return enic_link_update(enic);
 }
 
 static void enicpmd_dev_stats_get(struct rte_eth_dev *eth_dev,
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index aec24d2..aaf47e6 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -411,6 +411,21 @@ enic_free_consistent(void *priv,
 	rte_free(mze);
 }
 
+int enic_link_update(struct enic *enic)
+{
+	struct rte_eth_dev *eth_dev = enic->rte_dev;
+	int ret;
+	int link_status = 0;
+
+	link_status = enic_get_link_status(enic);
+	ret = (link_status == enic->link_status);
+	enic->link_status = link_status;
+	eth_dev->data->dev_link.link_status = link_status;
+	eth_dev->data->dev_link.link_duplex = ETH_LINK_FULL_DUPLEX;
+	eth_dev->data->dev_link.link_speed = vnic_dev_port_speed(enic->vdev);
+	return ret;
+}
+
 static void
 enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
 	void *arg)
-- 
2.7.0

[dpdk-dev] [PATCH 3/3] net/enic: enable link check interrupts

[Nelson Escobar]

Signed-off-by: Nelson Escobar <neescoba@cisco.com>
Reviewed-by: John Daley <johndale@cisco.com>
---
 doc/guides/nics/enic.rst       |  3 ++-
 drivers/net/enic/enic_ethdev.c |  2 +-
 drivers/net/enic/enic_main.c   | 19 ++++++++++++++++++-
 3 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/doc/guides/nics/enic.rst b/doc/guides/nics/enic.rst
index 42e781e..8170286 100644
--- a/doc/guides/nics/enic.rst
+++ b/doc/guides/nics/enic.rst
@@ -76,7 +76,8 @@ Configuration information
 
     Only one interrupt per vNIC interface should be configured in the UCS
     manager regardless of the number receive/transmit queues. The ENIC PMD
-    uses this interrupt to   get information about errors in the fast path.
+    uses this interrupt to get information about link status and errors
+    in the fast path.
 
 Limitations
 -----------
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index e10c824..44105d6 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -607,7 +607,7 @@ static struct eth_driver rte_enic_pmd = {
 	.pci_drv = {
 		.name = "rte_enic_pmd",
 		.id_table = pci_id_enic_map,
-		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 	},
 	.eth_dev_init = eth_enicpmd_dev_init,
 	.dev_private_size = sizeof(struct enic),
diff --git a/drivers/net/enic/enic_main.c b/drivers/net/enic/enic_main.c
index aaf47e6..55c7217 100644
--- a/drivers/net/enic/enic_main.c
+++ b/drivers/net/enic/enic_main.c
@@ -430,10 +430,13 @@ static void
 enic_intr_handler(__rte_unused struct rte_intr_handle *handle,
 	void *arg)
 {
-	struct enic *enic = pmd_priv((struct rte_eth_dev *)arg);
+	struct rte_eth_dev *dev = (struct rte_eth_dev *)arg;
+	struct enic *enic = pmd_priv(dev);
 
 	vnic_intr_return_all_credits(&enic->intr);
 
+	enic_link_update(enic);
+	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
 	enic_log_q_error(enic);
 }
 
@@ -447,6 +450,13 @@ int enic_enable(struct enic *enic)
 	eth_dev->data->dev_link.link_duplex = ETH_LINK_FULL_DUPLEX;
 	vnic_dev_notify_set(enic->vdev, -1); /* No Intr for notify */
 
+	/* vnic notification of link status has already been turned on in
+	 * enic_dev_init() which is called during probe time.  Here we are
+	 * just turning on interrupt vector 0 if needed.
+	 */
+	if (eth_dev->data->dev_conf.intr_conf.lsc)
+		vnic_dev_notify_set(enic->vdev, 0);
+
 	if (enic_clsf_init(enic))
 		dev_warning(enic, "Init of hash table for clsf failed."\
 			"Flow director feature will not work\n");
@@ -838,6 +848,13 @@ int enic_disable(struct enic *enic)
 		}
 	}
 
+	/* If we were using interrupts, set the interrupt vector to -1
+	 * to disable interrupts.  We are not disabling link notifcations,
+	 * though, as we want the polling of link status to continue working.
+	 */
+	if (enic->rte_dev->data->dev_conf.intr_conf.lsc)
+		vnic_dev_notify_set(enic->vdev, -1);
+
 	vnic_dev_set_reset_flag(enic->vdev, 1);
 	vnic_dev_notify_unset(enic->vdev);
 
-- 
2.7.0

Re: [dpdk-dev] [PATCH 3/3] net/enic: enable link check interrupts

[Bruce Richardson]

On Mon, Sep 19, 2016 at 11:50:11AM -0700, Nelson Escobar wrote:
> Signed-off-by: Nelson Escobar <neescoba@cisco.com>
> Reviewed-by: John Daley <johndale@cisco.com>
> ---
>  doc/guides/nics/enic.rst       |  3 ++-
>  drivers/net/enic/enic_ethdev.c |  2 +-
>  drivers/net/enic/enic_main.c   | 19 ++++++++++++++++++-
>  3 files changed, 21 insertions(+), 3 deletions(-)
> 
I believe an update is also needed for the overview table in the NIC guides to
call out that enic supports link status events. If there are no objections,
I'll add that entry on apply.

/Bruce

Re: [dpdk-dev] [PATCH 1/3] net/enic: cleanup interrupt setup when stopping port

[Bruce Richardson]

On Mon, Sep 19, 2016 at 11:50:09AM -0700, Nelson Escobar wrote:
> enic_disable() wasn't calling rte_intr_disable() or
> rte_intr_callback_unregister().  If stopping/starting a port, the
> latter omission would result in the same interrupt callback being
> registered multiple times, which would then cause it to be called
> multiple times on every interrupt.
> 
> Fixes: fefed3d1e62c ("enic: new driver")
> 
> Signed-off-by: Nelson Escobar <neescoba@cisco.com>
> Reviewed-by: John Daley <johndale@cisco.com>
> ---

Patchset applied to dpdk-next-net/rel_16_11

/Bruce