DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt
@ 2016-10-13  6:07 Qiming Yang
  2016-10-13  6:07 ` [dpdk-dev] [PATCH 2/2] net/i40e: fix VF bonded device link down Qiming Yang
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Qiming Yang @ 2016-10-13  6:07 UTC (permalink / raw)
  To: dev; +Cc: Qiming Yang

Previously, link status interrupt in i40e is achieved by checking
LINK_STAT_CHANGE_MASK in PFINT_ICR0 register which is provided only
for diagnostic use. Instead, drivers need to get the link status
change notification by using LSE (Link Status Event).

This patch enables LSE and calls LSC callback when the event is
received. This patch also removes the processing on
LINK_STAT_CHANGE_MASK.

Fixes: 5c9222058df7 ("i40e: move to drivers/net/")

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 97 +++++++++---------------------------------
 1 file changed, 19 insertions(+), 78 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index d0aeb70..9c1f542 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -108,7 +108,6 @@
 		I40E_PFINT_ICR0_ENA_GRST_MASK | \
 		I40E_PFINT_ICR0_ENA_PCI_EXCEPTION_MASK | \
 		I40E_PFINT_ICR0_ENA_STORM_DETECT_MASK | \
-		I40E_PFINT_ICR0_ENA_LINK_STAT_CHANGE_MASK | \
 		I40E_PFINT_ICR0_ENA_HMC_ERR_MASK | \
 		I40E_PFINT_ICR0_ENA_PE_CRITERR_MASK | \
 		I40E_PFINT_ICR0_ENA_VFLR_MASK | \
@@ -1768,6 +1767,16 @@ i40e_dev_start(struct rte_eth_dev *dev)
 		if (dev->data->dev_conf.intr_conf.lsc != 0)
 			PMD_INIT_LOG(INFO, "lsc won't enable because of"
 				     " no intr multiplex\n");
+	} else if (dev->data->dev_conf.intr_conf.lsc != 0) {
+		ret = i40e_aq_set_phy_int_mask(hw,
+					       ~(I40E_AQ_EVENT_LINK_UPDOWN |
+					       I40E_AQ_EVENT_MODULE_QUAL_FAIL |
+					       I40E_AQ_EVENT_MEDIA_NA), NULL);
+		if (ret != I40E_SUCCESS)
+			PMD_DRV_LOG(WARNING, "Fail to set phy mask");
+
+		/* Call get_link_info aq commond to enable LSE */
+		i40e_dev_link_update(dev, 0);
 	}
 
 	/* enable uio intr after callback register */
@@ -1984,6 +1993,7 @@ i40e_dev_link_update(struct rte_eth_dev *dev,
 	struct rte_eth_link link, old;
 	int status;
 	unsigned rep_cnt = MAX_REPEAT_TIME;
+	bool enable_lse = dev->data->dev_conf.intr_conf.lsc ? true : false;
 
 	memset(&link, 0, sizeof(link));
 	memset(&old, 0, sizeof(old));
@@ -1992,7 +2002,8 @@ i40e_dev_link_update(struct rte_eth_dev *dev,
 
 	do {
 		/* Get link status information from hardware */
-		status = i40e_aq_get_link_info(hw, false, &link_status, NULL);
+		status = i40e_aq_get_link_info(hw, enable_lse,
+						&link_status, NULL);
 		if (status != I40E_SUCCESS) {
 			link.link_speed = ETH_SPEED_NUM_100M;
 			link.link_duplex = ETH_LINK_FULL_DUPLEX;
@@ -5442,6 +5453,12 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
 					info.msg_buf,
 					info.msg_len);
 			break;
+		case i40e_aqc_opc_get_link_status:
+			ret = i40e_dev_link_update(dev, 0);
+			if (!ret)
+				_rte_eth_dev_callback_process(dev,
+					RTE_ETH_EVENT_INTR_LSC);
+			break;
 		default:
 			PMD_DRV_LOG(ERR, "Request %u is not supported yet",
 				    opcode);
@@ -5451,57 +5468,6 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
 	rte_free(info.msg_buf);
 }
 
-/*
- * Interrupt handler is registered as the alarm callback for handling LSC
- * interrupt in a definite of time, in order to wait the NIC into a stable
- * state. Currently it waits 1 sec in i40e for the link up interrupt, and
- * no need for link down interrupt.
- */
-static void
-i40e_dev_interrupt_delayed_handler(void *param)
-{
-	struct rte_eth_dev *dev = (struct rte_eth_dev *)param;
-	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	uint32_t icr0;
-
-	/* read interrupt causes again */
-	icr0 = I40E_READ_REG(hw, I40E_PFINT_ICR0);
-
-#ifdef RTE_LIBRTE_I40E_DEBUG_DRIVER
-	if (icr0 & I40E_PFINT_ICR0_ECC_ERR_MASK)
-		PMD_DRV_LOG(ERR, "ICR0: unrecoverable ECC error\n");
-	if (icr0 & I40E_PFINT_ICR0_MAL_DETECT_MASK)
-		PMD_DRV_LOG(ERR, "ICR0: malicious programming detected\n");
-	if (icr0 & I40E_PFINT_ICR0_GRST_MASK)
-		PMD_DRV_LOG(INFO, "ICR0: global reset requested\n");
-	if (icr0 & I40E_PFINT_ICR0_PCI_EXCEPTION_MASK)
-		PMD_DRV_LOG(INFO, "ICR0: PCI exception\n activated\n");
-	if (icr0 & I40E_PFINT_ICR0_STORM_DETECT_MASK)
-		PMD_DRV_LOG(INFO, "ICR0: a change in the storm control "
-								"state\n");
-	if (icr0 & I40E_PFINT_ICR0_HMC_ERR_MASK)
-		PMD_DRV_LOG(ERR, "ICR0: HMC error\n");
-	if (icr0 & I40E_PFINT_ICR0_PE_CRITERR_MASK)
-		PMD_DRV_LOG(ERR, "ICR0: protocol engine critical error\n");
-#endif /* RTE_LIBRTE_I40E_DEBUG_DRIVER */
-
-	if (icr0 & I40E_PFINT_ICR0_VFLR_MASK) {
-		PMD_DRV_LOG(INFO, "INT:VF reset detected\n");
-		i40e_dev_handle_vfr_event(dev);
-	}
-	if (icr0 & I40E_PFINT_ICR0_ADMINQ_MASK) {
-		PMD_DRV_LOG(INFO, "INT:ADMINQ event\n");
-		i40e_dev_handle_aq_msg(dev);
-	}
-
-	/* handle the link up interrupt in an alarm callback */
-	i40e_dev_link_update(dev, 0);
-	_rte_eth_dev_callback_process(dev, RTE_ETH_EVENT_INTR_LSC);
-
-	i40e_pf_enable_irq0(hw);
-	rte_intr_enable(&(dev->pci_dev->intr_handle));
-}
-
 /**
  * Interrupt handler triggered by NIC  for handling
  * specific interrupt.
@@ -5558,31 +5524,6 @@ i40e_dev_interrupt_handler(__rte_unused struct rte_intr_handle *handle,
 		PMD_DRV_LOG(INFO, "ICR0: adminq event");
 		i40e_dev_handle_aq_msg(dev);
 	}
-
-	/* Link Status Change interrupt */
-	if (icr0 & I40E_PFINT_ICR0_LINK_STAT_CHANGE_MASK) {
-#define I40E_US_PER_SECOND 1000000
-		struct rte_eth_link link;
-
-		PMD_DRV_LOG(INFO, "ICR0: link status changed\n");
-		memset(&link, 0, sizeof(link));
-		rte_i40e_dev_atomic_read_link_status(dev, &link);
-		i40e_dev_link_update(dev, 0);
-
-		/*
-		 * For link up interrupt, it needs to wait 1 second to let the
-		 * hardware be a stable state. Otherwise several consecutive
-		 * interrupts can be observed.
-		 * For link down interrupt, no need to wait.
-		 */
-		if (!link.link_status && rte_eal_alarm_set(I40E_US_PER_SECOND,
-			i40e_dev_interrupt_delayed_handler, (void *)dev) >= 0)
-			return;
-		else
-			_rte_eth_dev_callback_process(dev,
-				RTE_ETH_EVENT_INTR_LSC);
-	}
-
 done:
 	/* Enable interrupt */
 	i40e_pf_enable_irq0(hw);
-- 
2.5.5

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [dpdk-dev] [PATCH 2/2] net/i40e: fix VF bonded device link down
  2016-10-13  6:07 [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt Qiming Yang
@ 2016-10-13  6:07 ` Qiming Yang
  2016-10-24  9:50   ` Wu, Jingjing
  2016-10-19 10:56 ` [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt Ferruh Yigit
  2016-10-24  9:32 ` Wu, Jingjing
  2 siblings, 1 reply; 6+ messages in thread
From: Qiming Yang @ 2016-10-13  6:07 UTC (permalink / raw)
  To: dev; +Cc: Qiming Yang

Originally, using DPDK as host driver, when VF bonded device
uses I40E_VIRTCHNL_OP_GET_LINK_STAT to query PF the link status,
VF will wait for the interrupt from PF to get this link status.
PF uses interrupt which already exists to communication with VF.
These two kinds of interrupt will be confusing and fail to converge.

This patch uses PF to notify link status instead of VF query.

Fixes: 5c9222058df7 ("i40e: move to drivers/net/")

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c    | 22 ++++++++++-
 drivers/net/i40e/i40e_ethdev.h    |  4 +-
 drivers/net/i40e/i40e_ethdev_vf.c | 81 +++++++++++++--------------------------
 drivers/net/i40e/i40e_pf.c        | 33 ++++++++--------
 drivers/net/i40e/i40e_pf.h        |  3 +-
 5 files changed, 67 insertions(+), 76 deletions(-)

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 9c1f542..13060db 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -5418,6 +5418,24 @@ i40e_dev_handle_vfr_event(struct rte_eth_dev *dev)
 }
 
 static void
+i40e_notify_all_vfs_link_status(struct rte_eth_dev *dev)
+{
+	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
+	struct i40e_virtchnl_pf_event event;
+	int i;
+
+	event.event = I40E_VIRTCHNL_EVENT_LINK_CHANGE;
+	event.event_data.link_event.link_status =
+		dev->data->dev_link.link_status;
+	event.event_data.link_event.link_speed =
+		dev->data->dev_link.link_speed;
+
+	for (i = 0; i < pf->vf_num; i++)
+		i40e_pf_host_send_msg_to_vf(&pf->vfs[i], I40E_VIRTCHNL_OP_EVENT,
+				I40E_SUCCESS, (uint8_t *)&event, sizeof(event));
+}
+
+static void
 i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
 {
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
@@ -5455,9 +5473,11 @@ i40e_dev_handle_aq_msg(struct rte_eth_dev *dev)
 			break;
 		case i40e_aqc_opc_get_link_status:
 			ret = i40e_dev_link_update(dev, 0);
-			if (!ret)
+			if (!ret) {
+				i40e_notify_all_vfs_link_status(dev);
 				_rte_eth_dev_callback_process(dev,
 					RTE_ETH_EVENT_INTR_LSC);
+			}
 			break;
 		default:
 			PMD_DRV_LOG(ERR, "Request %u is not supported yet",
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 92c8fad..61dfa93 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -599,7 +599,9 @@ int i40e_hash_filter_inset_select(struct i40e_hw *hw,
 			     struct rte_eth_input_set_conf *conf);
 int i40e_fdir_filter_inset_select(struct i40e_pf *pf,
 			     struct rte_eth_input_set_conf *conf);
-
+int i40e_pf_host_send_msg_to_vf(struct i40e_pf_vf *vf, uint32_t opcode,
+				uint32_t retval, uint8_t *msg,
+				uint16_t msglen);
 void i40e_rxq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
 	struct rte_eth_rxq_info *qinfo);
 void i40e_txq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index a616ae0..ba63a7f 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -126,8 +126,6 @@ static void i40evf_dev_promiscuous_enable(struct rte_eth_dev *dev);
 static void i40evf_dev_promiscuous_disable(struct rte_eth_dev *dev);
 static void i40evf_dev_allmulticast_enable(struct rte_eth_dev *dev);
 static void i40evf_dev_allmulticast_disable(struct rte_eth_dev *dev);
-static int i40evf_get_link_status(struct rte_eth_dev *dev,
-				  struct rte_eth_link *link);
 static int i40evf_init_vlan(struct rte_eth_dev *dev);
 static int i40evf_dev_rx_queue_start(struct rte_eth_dev *dev,
 				     uint16_t rx_queue_id);
@@ -1084,31 +1082,6 @@ i40evf_del_vlan(struct rte_eth_dev *dev, uint16_t vlanid)
 	return err;
 }
 
-static int
-i40evf_get_link_status(struct rte_eth_dev *dev, struct rte_eth_link *link)
-{
-	struct i40e_vf *vf = I40EVF_DEV_PRIVATE_TO_VF(dev->data->dev_private);
-	int err;
-	struct vf_cmd_info args;
-	struct rte_eth_link *new_link;
-
-	args.ops = (enum i40e_virtchnl_ops)I40E_VIRTCHNL_OP_GET_LINK_STAT;
-	args.in_args = NULL;
-	args.in_args_size = 0;
-	args.out_buffer = vf->aq_resp;
-	args.out_size = I40E_AQ_BUF_SZ;
-	err = i40evf_execute_vf_cmd(dev, &args);
-	if (err) {
-		PMD_DRV_LOG(ERR, "fail to execute command OP_GET_LINK_STAT");
-		return err;
-	}
-
-	new_link = (struct rte_eth_link *)args.out_buffer;
-	(void)rte_memcpy(link, new_link, sizeof(*link));
-
-	return 0;
-}
-
 static const struct rte_pci_id pci_id_i40evf_map[] = {
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_VF) },
 	{ RTE_PCI_DEVICE(I40E_INTEL_VENDOR_ID, I40E_DEV_ID_VF_HV) },
@@ -2166,35 +2139,33 @@ i40evf_dev_link_update(struct rte_eth_dev *dev,
 	 * DPDK pf host provide interfacet to acquire link status
 	 * while Linux driver does not
 	 */
-	if (vf->version_major == I40E_DPDK_VERSION_MAJOR)
-		i40evf_get_link_status(dev, &new_link);
-	else {
-		/* Linux driver PF host */
-		switch (vf->link_speed) {
-		case I40E_LINK_SPEED_100MB:
-			new_link.link_speed = ETH_SPEED_NUM_100M;
-			break;
-		case I40E_LINK_SPEED_1GB:
-			new_link.link_speed = ETH_SPEED_NUM_1G;
-			break;
-		case I40E_LINK_SPEED_10GB:
-			new_link.link_speed = ETH_SPEED_NUM_10G;
-			break;
-		case I40E_LINK_SPEED_20GB:
-			new_link.link_speed = ETH_SPEED_NUM_20G;
-			break;
-		case I40E_LINK_SPEED_40GB:
-			new_link.link_speed = ETH_SPEED_NUM_40G;
-			break;
-		default:
-			new_link.link_speed = ETH_SPEED_NUM_100M;
-			break;
-		}
-		/* full duplex only */
-		new_link.link_duplex = ETH_LINK_FULL_DUPLEX;
-		new_link.link_status = vf->link_up ? ETH_LINK_UP :
-						     ETH_LINK_DOWN;
+
+	/* Linux driver PF host */
+	switch (vf->link_speed) {
+	case I40E_LINK_SPEED_100MB:
+		new_link.link_speed = ETH_SPEED_NUM_100M;
+		break;
+	case I40E_LINK_SPEED_1GB:
+		new_link.link_speed = ETH_SPEED_NUM_1G;
+		break;
+	case I40E_LINK_SPEED_10GB:
+		new_link.link_speed = ETH_SPEED_NUM_10G;
+		break;
+	case I40E_LINK_SPEED_20GB:
+		new_link.link_speed = ETH_SPEED_NUM_20G;
+		break;
+	case I40E_LINK_SPEED_40GB:
+		new_link.link_speed = ETH_SPEED_NUM_40G;
+		break;
+	default:
+		new_link.link_speed = ETH_SPEED_NUM_100M;
+		break;
 	}
+	/* full duplex only */
+	new_link.link_duplex = ETH_LINK_FULL_DUPLEX;
+	new_link.link_status = vf->link_up ? ETH_LINK_UP :
+					     ETH_LINK_DOWN;
+
 	i40evf_dev_atomic_write_link_status(dev, &new_link);
 
 	return 0;
diff --git a/drivers/net/i40e/i40e_pf.c b/drivers/net/i40e/i40e_pf.c
index d5b2d45..350f6a0 100644
--- a/drivers/net/i40e/i40e_pf.c
+++ b/drivers/net/i40e/i40e_pf.c
@@ -250,7 +250,7 @@ i40e_pf_host_vf_reset(struct i40e_pf_vf *vf, bool do_hw_reset)
 	return ret;
 }
 
-static int
+int
 i40e_pf_host_send_msg_to_vf(struct i40e_pf_vf *vf,
 			    uint32_t opcode,
 			    uint32_t retval,
@@ -847,18 +847,6 @@ i40e_pf_host_process_cmd_get_stats(struct i40e_pf_vf *vf)
 	return I40E_SUCCESS;
 }
 
-static void
-i40e_pf_host_process_cmd_get_link_status(struct i40e_pf_vf *vf)
-{
-	struct rte_eth_dev *dev = I40E_VSI_TO_ETH_DEV(vf->pf->main_vsi);
-
-	/* Update link status first to acquire latest link change */
-	i40e_dev_link_update(dev, 1);
-	i40e_pf_host_send_msg_to_vf(vf, I40E_VIRTCHNL_OP_GET_LINK_STAT,
-		I40E_SUCCESS, (uint8_t *)&dev->data->dev_link,
-				sizeof(struct rte_eth_link));
-}
-
 static int
 i40e_pf_host_process_cmd_cfg_vlan_offload(
 					struct i40e_pf_vf *vf,
@@ -909,6 +897,20 @@ send_msg:
 	return ret;
 }
 
+static void
+i40e_notify_vf_link_status(struct rte_eth_dev *dev, struct i40e_pf_vf *vf)
+{
+	struct i40e_virtchnl_pf_event event;
+
+	event.event = I40E_VIRTCHNL_EVENT_LINK_CHANGE;
+	event.event_data.link_event.link_status =
+			dev->data->dev_link.link_status;
+	event.event_data.link_event.link_speed =
+			dev->data->dev_link.link_speed;
+	i40e_pf_host_send_msg_to_vf(vf, I40E_VIRTCHNL_OP_EVENT,
+			I40E_SUCCESS, (uint8_t *)&event, sizeof(event));
+}
+
 void
 i40e_pf_host_handle_vf_msg(struct rte_eth_dev *dev,
 			   uint16_t abs_vf_id, uint32_t opcode,
@@ -964,6 +966,7 @@ i40e_pf_host_handle_vf_msg(struct rte_eth_dev *dev,
 	case I40E_VIRTCHNL_OP_ENABLE_QUEUES:
 		PMD_DRV_LOG(INFO, "OP_ENABLE_QUEUES received");
 		i40e_pf_host_process_cmd_enable_queues(vf, msg, msglen);
+		i40e_notify_vf_link_status(dev, vf);
 		break;
 	case I40E_VIRTCHNL_OP_DISABLE_QUEUES:
 		PMD_DRV_LOG(INFO, "OP_DISABLE_QUEUE received");
@@ -993,10 +996,6 @@ i40e_pf_host_handle_vf_msg(struct rte_eth_dev *dev,
 		PMD_DRV_LOG(INFO, "OP_GET_STATS received");
 		i40e_pf_host_process_cmd_get_stats(vf);
 		break;
-	case I40E_VIRTCHNL_OP_GET_LINK_STAT:
-		PMD_DRV_LOG(INFO, "OP_GET_LINK_STAT received");
-		i40e_pf_host_process_cmd_get_link_status(vf);
-		break;
 	case I40E_VIRTCHNL_OP_CFG_VLAN_OFFLOAD:
 		PMD_DRV_LOG(INFO, "OP_CFG_VLAN_OFFLOAD received");
 		i40e_pf_host_process_cmd_cfg_vlan_offload(vf, msg, msglen);
diff --git a/drivers/net/i40e/i40e_pf.h b/drivers/net/i40e/i40e_pf.h
index 9c01829..cddc45c 100644
--- a/drivers/net/i40e/i40e_pf.h
+++ b/drivers/net/i40e/i40e_pf.h
@@ -59,9 +59,8 @@ enum i40e_virtchnl_ops_dpdk {
 	 * Keep some gap between Linux PF commands and
 	 * DPDK PF extended commands.
 	 */
-	I40E_VIRTCHNL_OP_GET_LINK_STAT = I40E_VIRTCHNL_OP_VERSION +
+	I40E_VIRTCHNL_OP_CFG_VLAN_OFFLOAD = I40E_VIRTCHNL_OP_VERSION +
 						I40E_DPDK_OFFSET,
-	I40E_VIRTCHNL_OP_CFG_VLAN_OFFLOAD,
 	I40E_VIRTCHNL_OP_CFG_VLAN_PVID,
 	I40E_VIRTCHNL_OP_CONFIG_VSI_QUEUES_EXT,
 };
-- 
2.5.5

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt
  2016-10-13  6:07 [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt Qiming Yang
  2016-10-13  6:07 ` [dpdk-dev] [PATCH 2/2] net/i40e: fix VF bonded device link down Qiming Yang
@ 2016-10-19 10:56 ` Ferruh Yigit
  2016-10-24  2:21   ` Yang, Qiming
  2016-10-24  9:32 ` Wu, Jingjing
  2 siblings, 1 reply; 6+ messages in thread
From: Ferruh Yigit @ 2016-10-19 10:56 UTC (permalink / raw)
  To: Qiming Yang, dev; +Cc: Jingjing Wu

Hi Qiming,

On 10/13/2016 7:07 AM, Qiming Yang wrote:
> Previously, link status interrupt in i40e is achieved by checking
> LINK_STAT_CHANGE_MASK in PFINT_ICR0 register which is provided only
> for diagnostic use. Instead, drivers need to get the link status
> change notification by using LSE (Link Status Event).
> 
> This patch enables LSE and calls LSC callback when the event is
> received. This patch also removes the processing on
> LINK_STAT_CHANGE_MASK.
> 
> Fixes: 5c9222058df7 ("i40e: move to drivers/net/")
> 
> Signed-off-by: Qiming Yang <qiming.yang@intel.com>

CC: Jingjing Wu <jingjing.wu@intel.com>

Can you please add maintainer(s) to CC when sending a patch, mail
traffic in dpdk.org is keep increasing and it is hard to follow. Adding
maintainer to the CC helps a lot to the maintainer.

I guess everybody knows, but just as a reminder, maintainer list kept in
file: http://dpdk.org/browse/dpdk/tree/MAINTAINERS


Thanks,
ferruh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt
  2016-10-19 10:56 ` [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt Ferruh Yigit
@ 2016-10-24  2:21   ` Yang, Qiming
  0 siblings, 0 replies; 6+ messages in thread
From: Yang, Qiming @ 2016-10-24  2:21 UTC (permalink / raw)
  To: Yigit, Ferruh, dev; +Cc: Wu, Jingjing, Zhang, Helin

Ferruh,
Thank you for your reminder, I will remember to CC to the maintainer in future. 

Jingjing and Helin,
Can you help to review these two patches?

Thanks,
Qiming

-----Original Message-----
From: Yigit, Ferruh 
Sent: Wednesday, October 19, 2016 6:57 PM
To: Yang, Qiming <qiming.yang@intel.com>; dev@dpdk.org
Cc: Wu, Jingjing <jingjing.wu@intel.com>
Subject: Re: [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt

Hi Qiming,

On 10/13/2016 7:07 AM, Qiming Yang wrote:
> Previously, link status interrupt in i40e is achieved by checking 
> LINK_STAT_CHANGE_MASK in PFINT_ICR0 register which is provided only 
> for diagnostic use. Instead, drivers need to get the link status 
> change notification by using LSE (Link Status Event).
> 
> This patch enables LSE and calls LSC callback when the event is 
> received. This patch also removes the processing on 
> LINK_STAT_CHANGE_MASK.
> 
> Fixes: 5c9222058df7 ("i40e: move to drivers/net/")
> 
> Signed-off-by: Qiming Yang <qiming.yang@intel.com>

CC: Jingjing Wu <jingjing.wu@intel.com>

Can you please add maintainer(s) to CC when sending a patch, mail traffic in dpdk.org is keep increasing and it is hard to follow. Adding maintainer to the CC helps a lot to the maintainer.

I guess everybody knows, but just as a reminder, maintainer list kept in
file: http://dpdk.org/browse/dpdk/tree/MAINTAINERS


Thanks,
ferruh

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt
  2016-10-13  6:07 [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt Qiming Yang
  2016-10-13  6:07 ` [dpdk-dev] [PATCH 2/2] net/i40e: fix VF bonded device link down Qiming Yang
  2016-10-19 10:56 ` [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt Ferruh Yigit
@ 2016-10-24  9:32 ` Wu, Jingjing
  2 siblings, 0 replies; 6+ messages in thread
From: Wu, Jingjing @ 2016-10-24  9:32 UTC (permalink / raw)
  To: Yang, Qiming, dev; +Cc: Yang, Qiming



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiming Yang
> Sent: Thursday, October 13, 2016 2:07 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>
> Subject: [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt
> 
> Previously, link status interrupt in i40e is achieved by checking
> LINK_STAT_CHANGE_MASK in PFINT_ICR0 register which is provided only
> for diagnostic use. Instead, drivers need to get the link status
> change notification by using LSE (Link Status Event).
> 
> This patch enables LSE and calls LSC callback when the event is
> received. This patch also removes the processing on
> LINK_STAT_CHANGE_MASK.

Good description! Thanks!
> Fixes: 5c9222058df7 ("i40e: move to drivers/net/")

Acked-by: Jingjing Wu <jingjing.wu@intel.com> with minor comment:
The commit 5c9222058df7 ("i40e: move to drivers/net/") is just a moving i40e PMD driver code to current folder. It is not exactly that introduced the issue.
Maybe what you are looking for is 4861cde46116 ("i40e: new poll mode driver")


Thanks
Jingjing

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH 2/2] net/i40e: fix VF bonded device link down
  2016-10-13  6:07 ` [dpdk-dev] [PATCH 2/2] net/i40e: fix VF bonded device link down Qiming Yang
@ 2016-10-24  9:50   ` Wu, Jingjing
  0 siblings, 0 replies; 6+ messages in thread
From: Wu, Jingjing @ 2016-10-24  9:50 UTC (permalink / raw)
  To: Yang, Qiming, dev; +Cc: Yang, Qiming



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Qiming Yang
> Sent: Thursday, October 13, 2016 2:07 PM
> To: dev@dpdk.org
> Cc: Yang, Qiming <qiming.yang@intel.com>
> Subject: [dpdk-dev] [PATCH 2/2] net/i40e: fix VF bonded device link down
> 
> Originally, using DPDK as host driver, when VF bonded device
> uses I40E_VIRTCHNL_OP_GET_LINK_STAT to query PF the link status,

If VF device is used as slave of a bond device, it will be polled periodically though alarm. Interrupt is involved here.
And then VF will send I40E_VIRTCHNL_OP_GET_LINK_STAT message to PF to get the status. The response is handled by interrupt callback. Interrupt is involved here again. That's what bond device cannot bring up.

> This patch uses PF to notify link status instead of VF query.
This patch changes like that remove I40E_VIRTCHNL_OP_GET_LINK_STAT message, link status in VF driver is updated when PF driver notify it, and VF stores the links status locally. VF driver just returns the local status when being required.

> Fixes: 5c9222058df7 ("i40e: move to drivers/net/")
The same comments as your previous patch. It is not exact commit introduced this issue.

Thanks
Jingjing

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-10-24  9:50 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-13  6:07 [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt Qiming Yang
2016-10-13  6:07 ` [dpdk-dev] [PATCH 2/2] net/i40e: fix VF bonded device link down Qiming Yang
2016-10-24  9:50   ` Wu, Jingjing
2016-10-19 10:56 ` [dpdk-dev] [PATCH 1/2] net/i40e: fix link status change interrupt Ferruh Yigit
2016-10-24  2:21   ` Yang, Qiming
2016-10-24  9:32 ` Wu, Jingjing

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).