diff mbox

[dpdk-dev,v2,3/6] net/vmxnet3: Generate link-state change notifications

Message ID 1497528973-16330-4-git-send-email-ciwillia@brocade.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers

Checks

Context Check Description
ci/checkpatch warning coding style issues
ci/Intel-compilation fail Compilation issues

Commit Message

Chas Williams June 15, 2017, 12:16 p.m. UTC 
  From: Robert Shearman <rshearma@brocade.com>

Generate link-state change notifications by listening to interrupts
generated by the device. Make use of the existing
vmxnet3_process_events function that was compiled out, but change it
to call vmxnet3_dev_link_update on a VMXNET3_ECR_LINK event and to not
be so noisy in its log messages.

Enable interrupts on starting the device, using a new helper function,
vmxnet3_enable_intr, based on vmxnet3_disable_intr and validated
against the FreeBSD driver.

Keep track of the number of interrupts registered for to avoid
hardcoding these in vmxnet3_enable/disable_intr and to provision for
any future rxq intr support.

Factor out the guts of vmxnet3_dev_link_update minus the started check
to allow the new function to be called from vmxnet3_dev_start in the
lsc-enabled case to ensure that the link state is correctly set from
the actual state at that point.

Signed-off-by: Robert Shearman <rshearma@brocade.com>
---
 drivers/net/vmxnet3/vmxnet3_ethdev.c | 116 +++++++++++++++++++++++++++--------
 drivers/net/vmxnet3/vmxnet3_ethdev.h |   2 +
 2 files changed, 91 insertions(+), 27 deletions(-)

Comments

Ferruh Yigit June 27, 2017, 1:52 p.m. UTC | #1 
On 6/15/2017 1:16 PM, Charles (Chas) Williams wrote:
> From: Robert Shearman <rshearma@brocade.com>
> 
> Generate link-state change notifications by listening to interrupts
> generated by the device. Make use of the existing
> vmxnet3_process_events function that was compiled out, but change it
> to call vmxnet3_dev_link_update on a VMXNET3_ECR_LINK event and to not
> be so noisy in its log messages.
> 
> Enable interrupts on starting the device, using a new helper function,
> vmxnet3_enable_intr, based on vmxnet3_disable_intr and validated
> against the FreeBSD driver.
> 
> Keep track of the number of interrupts registered for to avoid
> hardcoding these in vmxnet3_enable/disable_intr and to provision for
> any future rxq intr support.
> 
> Factor out the guts of vmxnet3_dev_link_update minus the started check
> to allow the new function to be called from vmxnet3_dev_start in the
> lsc-enabled case to ensure that the link state is correctly set from
> the actual state at that point.
> 
> Signed-off-by: Robert Shearman <rshearma@brocade.com>

It is close to the integration deadline, and all patches in the set
acked except this one, I am for getting the set and if any issue found
related LSC, fix it after integration deadline.

If there is an objection to get the patchset, please comment soon.

Thanks,
ferruh
diff mbox

Patch

diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index f5a9832..73277fb 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -83,6 +83,8 @@  static void vmxnet3_dev_promiscuous_enable(struct rte_eth_dev *dev);
 static void vmxnet3_dev_promiscuous_disable(struct rte_eth_dev *dev);
 static void vmxnet3_dev_allmulticast_enable(struct rte_eth_dev *dev);
 static void vmxnet3_dev_allmulticast_disable(struct rte_eth_dev *dev);
+static int __vmxnet3_dev_link_update(struct rte_eth_dev *dev,
+				     int wait_to_complete);
 static int vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 				   int wait_to_complete);
 static void vmxnet3_hw_stats_save(struct vmxnet3_hw *hw);
@@ -102,10 +104,8 @@  static int vmxnet3_dev_vlan_filter_set(struct rte_eth_dev *dev,
 static void vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask);
 static void vmxnet3_mac_addr_set(struct rte_eth_dev *dev,
 				 struct ether_addr *mac_addr);
+static void vmxnet3_interrupt_handler(void *param);
 
-#if PROCESS_SYS_EVENTS == 1
-static void vmxnet3_process_events(struct vmxnet3_hw *);
-#endif
 /*
  * The set of PCI devices this driver supports
  */
@@ -250,10 +250,22 @@  vmxnet3_disable_intr(struct vmxnet3_hw *hw)
 	PMD_INIT_FUNC_TRACE();
 
 	hw->shared->devRead.intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
-	for (i = 0; i < VMXNET3_MAX_INTRS; i++)
+	for (i = 0; i < hw->num_intrs; i++)
 		VMXNET3_WRITE_BAR0_REG(hw, VMXNET3_REG_IMR + i * 8, 1);
 }
 
+static void
+vmxnet3_enable_intr(struct vmxnet3_hw *hw)
+{
+	int i;
+
+	PMD_INIT_FUNC_TRACE();
+
+	hw->shared->devRead.intrConf.intrCtrl &= ~VMXNET3_IC_DISABLE_ALL;
+	for (i = 0; i < hw->num_intrs; i++)
+		VMXNET3_WRITE_BAR0_REG(hw, VMXNET3_REG_IMR + i * 8, 0);
+}
+
 /*
  * Gets tx data ring descriptor size.
  */
@@ -425,7 +437,7 @@  static int eth_vmxnet3_pci_remove(struct rte_pci_device *pci_dev)
 
 static struct rte_pci_driver rte_vmxnet3_pmd = {
 	.id_table = pci_id_vmxnet3_map,
-	.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
+	.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 	.probe = eth_vmxnet3_pci_probe,
 	.remove = eth_vmxnet3_pci_remove,
 };
@@ -648,11 +660,11 @@  vmxnet3_setup_driver_shared(struct rte_eth_dev *dev)
 
 	/*
 	 * Set number of interrupts to 1
-	 * PMD disables all the interrupts but this is MUST to activate device
-	 * It needs at least one interrupt for link events to handle
-	 * So we'll disable it later after device activation if needed
+	 * PMD by default disables all the interrupts but this is MUST
+	 * to activate device. It needs at least one interrupt for
+	 * link events to handle
 	 */
-	devRead->intrConf.numIntrs = 1;
+	hw->num_intrs = devRead->intrConf.numIntrs = 1;
 	devRead->intrConf.intrCtrl |= VMXNET3_IC_DISABLE_ALL;
 
 	for (i = 0; i < hw->num_tx_queues; i++) {
@@ -747,6 +759,20 @@  vmxnet3_dev_start(struct rte_eth_dev *dev)
 	if (ret != VMXNET3_SUCCESS)
 		return ret;
 
+	/* check if lsc interrupt feature is enabled */
+	if (dev->data->dev_conf.intr_conf.lsc) {
+		struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
+
+		/* Setup interrupt callback  */
+		rte_intr_callback_register(&pci_dev->intr_handle,
+					   vmxnet3_interrupt_handler, dev);
+
+		if (rte_intr_enable(&pci_dev->intr_handle) < 0) {
+			PMD_INIT_LOG(ERR, "interrupt enable failed");
+			return -EIO;
+		}
+	}
+
 	/* Exchange shared data with device */
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DSAL,
 			       VMXNET3_GET_ADDR_LO(hw->sharedPA));
@@ -794,14 +820,19 @@  vmxnet3_dev_start(struct rte_eth_dev *dev)
 	/* Setting proper Rx Mode and issue Rx Mode Update command */
 	vmxnet3_dev_set_rxmode(hw, VMXNET3_RXM_UCAST | VMXNET3_RXM_BCAST, 1);
 
-	/*
-	 * Don't need to handle events for now
-	 */
-#if PROCESS_SYS_EVENTS == 1
-	events = VMXNET3_READ_BAR1_REG(hw, VMXNET3_REG_ECR);
-	PMD_INIT_LOG(DEBUG, "Reading events: 0x%X", events);
-	vmxnet3_process_events(hw);
-#endif
+	if (dev->data->dev_conf.intr_conf.lsc) {
+		vmxnet3_enable_intr(hw);
+
+		/*
+		 * Update link state from device since this won't be
+		 * done upon starting with lsc in use. This is done
+		 * only after enabling interrupts to avoid any race
+		 * where the link state could change without an
+		 * interrupt being fired.
+		 */
+		__vmxnet3_dev_link_update(dev, 0);
+	}
+
 	return VMXNET3_SUCCESS;
 }
 
@@ -824,6 +855,15 @@  vmxnet3_dev_stop(struct rte_eth_dev *dev)
 	/* disable interrupts */
 	vmxnet3_disable_intr(hw);
 
+	if (dev->data->dev_conf.intr_conf.lsc) {
+		struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
+
+		rte_intr_disable(&pci_dev->intr_handle);
+
+		rte_intr_callback_unregister(&pci_dev->intr_handle,
+					     vmxnet3_interrupt_handler, dev);
+	}
+
 	/* quiesce the device first */
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_CMD, VMXNET3_CMD_QUIESCE_DEV);
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_DSAL, 0);
@@ -1110,17 +1150,13 @@  vmxnet3_mac_addr_set(struct rte_eth_dev *dev, struct ether_addr *mac_addr)
 
 /* return 0 means link status changed, -1 means not changed */
 static int
-vmxnet3_dev_link_update(struct rte_eth_dev *dev,
-			__rte_unused int wait_to_complete)
+__vmxnet3_dev_link_update(struct rte_eth_dev *dev,
+			  __rte_unused int wait_to_complete)
 {
 	struct vmxnet3_hw *hw = dev->data->dev_private;
 	struct rte_eth_link old = { 0 }, link;
 	uint32_t ret;
 
-	/* Link status doesn't change for stopped dev */
-	if (dev->data->dev_started == 0)
-		return -1;
-
 	memset(&link, 0, sizeof(link));
 	vmxnet3_dev_atomic_read_link_status(dev, &old);
 
@@ -1139,6 +1175,16 @@  vmxnet3_dev_link_update(struct rte_eth_dev *dev,
 	return (old.link_status == link.link_status) ? -1 : 0;
 }
 
+static int
+vmxnet3_dev_link_update(struct rte_eth_dev *dev, int wait_to_complete)
+{
+	/* Link status doesn't change for stopped dev */
+	if (dev->data->dev_started == 0)
+		return -1;
+
+	return __vmxnet3_dev_link_update(dev, wait_to_complete);
+}
+
 /* Updating rxmode through Vmxnet3_DriverShared structure in adapter */
 static void
 vmxnet3_dev_set_rxmode(struct vmxnet3_hw *hw, uint32_t feature, int set)
@@ -1255,10 +1301,10 @@  vmxnet3_dev_vlan_offload_set(struct rte_eth_dev *dev, int mask)
 	}
 }
 
-#if PROCESS_SYS_EVENTS == 1
 static void
-vmxnet3_process_events(struct vmxnet3_hw *hw)
+vmxnet3_process_events(struct rte_eth_dev *dev)
 {
+	struct vmxnet3_hw *hw = dev->data->dev_private;
 	uint32_t events = hw->shared->ecr;
 
 	if (!events) {
@@ -1273,10 +1319,15 @@  vmxnet3_process_events(struct vmxnet3_hw *hw)
 	VMXNET3_WRITE_BAR1_REG(hw, VMXNET3_REG_ECR, events);
 
 	/* Check if link state has changed */
-	if (events & VMXNET3_ECR_LINK)
+	if (events & VMXNET3_ECR_LINK) {
 		PMD_INIT_LOG(ERR,
 			     "Process events in %s(): VMXNET3_ECR_LINK event",
 			     __func__);
+		if (vmxnet3_dev_link_update(dev, 0) == 0)
+			_rte_eth_dev_callback_process(dev,
+						      RTE_ETH_EVENT_INTR_LSC,
+						      NULL);
+	}
 
 	/* Check if there is an error on xmit/recv queues */
 	if (events & (VMXNET3_ECR_TQERR | VMXNET3_ECR_RQERR)) {
@@ -1301,7 +1352,18 @@  vmxnet3_process_events(struct vmxnet3_hw *hw)
 	if (events & VMXNET3_ECR_DEBUG)
 		PMD_INIT_LOG(ERR, "Debug event generated by device.");
 }
-#endif
+
+static void
+vmxnet3_interrupt_handler(void *param)
+{
+	struct rte_eth_dev *dev = param;
+	struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
+
+	vmxnet3_process_events(dev);
+
+	if (rte_intr_enable(&pci_dev->intr_handle) < 0)
+		PMD_DRV_LOG(ERR, "interrupt enable failed");
+}
 
 RTE_PMD_REGISTER_PCI(net_vmxnet3, rte_vmxnet3_pmd);
 RTE_PMD_REGISTER_PCI_TABLE(net_vmxnet3, pci_id_vmxnet3_map);
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.h b/drivers/net/vmxnet3/vmxnet3_ethdev.h
index e93e4a7..b48058a 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.h
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.h
@@ -106,6 +106,8 @@  struct vmxnet3_hw {
 	uint16_t txdata_desc_size; /* tx data ring buffer size */
 	uint16_t rxdata_desc_size; /* rx data ring buffer size */
 
+	uint8_t num_intrs;
+
 	Vmxnet3_TxQueueDesc   *tqd_start;	/* start address of all tx queue desc */
 	Vmxnet3_RxQueueDesc   *rqd_start;	/* start address of all rx queue desc */