patches for DPDK stable branches
 help / color / mirror / Atom feed
* [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt
@ 2016-11-17 11:12 Qiming Yang
  2016-11-17 11:12 ` [dpdk-stable] [PATCH 2/2] net/i40e: fix VF bonded device link down Qiming Yang
  0 siblings, 1 reply; 12+ messages in thread
From: Qiming Yang @ 2016-11-17 11:12 UTC (permalink / raw)
  To: stable; +Cc: yuanhan.liu, Qiming Yang

[ backported from upstream commit b52fe009edc166bfce205c8ad931190c7d3f2d5f ]

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: 4861cde46116 ("i40e: new poll mode driver")

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.7.4

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

* [dpdk-stable] [PATCH 2/2] net/i40e: fix VF bonded device link down
  2016-11-17 11:12 [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt Qiming Yang
@ 2016-11-17 11:12 ` Qiming Yang
  2016-11-17 12:02   ` Yuanhan Liu
  2016-11-17 12:03   ` Yuanhan Liu
  0 siblings, 2 replies; 12+ messages in thread
From: Qiming Yang @ 2016-11-17 11:12 UTC (permalink / raw)
  To: stable; +Cc: yuanhan.liu, Qiming Yang

[ backported from upstream commit f2c6036c6455099cbadfb9f0a0a3deb3320211c3 ]

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

This patch removes I40E_VIRTCHNL_OP_GET_LINK_STAT
message. Link status in VF driver will be updated when PF driver
notify it, and VF stores this link status locally. VF driver just
returns the local status when being required.

Fixes: 4861cde46116 ("i40e: new poll mode driver")

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..a5585bd 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 =
+		(enum i40e_aq_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.7.4

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

* Re: [dpdk-stable] [PATCH 2/2] net/i40e: fix VF bonded device link down
  2016-11-17 11:12 ` [dpdk-stable] [PATCH 2/2] net/i40e: fix VF bonded device link down Qiming Yang
@ 2016-11-17 12:02   ` Yuanhan Liu
  2016-11-17 12:03   ` Yuanhan Liu
  1 sibling, 0 replies; 12+ messages in thread
From: Yuanhan Liu @ 2016-11-17 12:02 UTC (permalink / raw)
  To: Qiming Yang; +Cc: stable

On Thu, Nov 17, 2016 at 07:12:23PM +0800, Qiming Yang wrote:
> [ backported from upstream commit f2c6036c6455099cbadfb9f0a0a3deb3320211c3 ]

The commit is wrong: the upstream commit is f4668a33efe5311301e10bc6e3709a2eccb446ad.

Fixed and applied to dpdk stable branc 16.07.

Thanks.

	--yliu

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

* Re: [dpdk-stable] [PATCH 2/2] net/i40e: fix VF bonded device link down
  2016-11-17 11:12 ` [dpdk-stable] [PATCH 2/2] net/i40e: fix VF bonded device link down Qiming Yang
  2016-11-17 12:02   ` Yuanhan Liu
@ 2016-11-17 12:03   ` Yuanhan Liu
  1 sibling, 0 replies; 12+ messages in thread
From: Yuanhan Liu @ 2016-11-17 12:03 UTC (permalink / raw)
  To: Qiming Yang; +Cc: stable

On Thu, Nov 17, 2016 at 07:12:23PM +0800, Qiming Yang wrote:
> [ backported from upstream commit f2c6036c6455099cbadfb9f0a0a3deb3320211c3 ]

Again, the commit is wrong: the upstream commit is bb6722fb5c0eae07ab3a6566298df2d6028e1cf9.

Fixed and applied to dpdk stable branc 16.07.

Thanks.

        --yliu

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

* Re: [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt
  2016-11-23  3:00         ` Yuanhan Liu
@ 2016-11-23  8:46           ` Yang, Qiming
  0 siblings, 0 replies; 12+ messages in thread
From: Yang, Qiming @ 2016-11-23  8:46 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: stable

I meet some problem when I clone the code.
[root@dpdk7 dpdk-stable-v2]# git clone http://dpdk.org/git/dpdk-stable
Cloning into 'dpdk-stable'...
remote: Counting objects: 52952, done.
remote: Compressing objects: 100% (11583/11583), done.
error: RPC failed; curl 56 Recv failure: Connection reset by peer
fatal: The remote end hung up unexpectedly
fatal: early EOF
fatal: index-pack failed
Can you give me the whole commit number of my patches?

-----Original Message-----
From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] 
Sent: Wednesday, November 23, 2016 11:00 AM
To: Yang, Qiming <qiming.yang@intel.com>
Cc: stable@dpdk.org
Subject: Re: [PATCH 1/2] net/i40e: fix link status change interrupt

On Wed, Nov 23, 2016 at 02:47:15AM +0000, Yang, Qiming wrote:
> I can't find my commits in the current 16.07 stable branch, so I don't know how to write the fix line.

The two I applied from you are:

    718d247 net/i40e: fix VF bonded device link down
    0b02249 net/i40e: fix link status change interrupt

	--yliu

> If the old patches didn't apply, can I send v2 patch set?
> 
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Wednesday, November 23, 2016 9:39 AM
> To: Yang, Qiming <qiming.yang@intel.com>
> Cc: stable@dpdk.org
> Subject: Re: [PATCH 1/2] net/i40e: fix link status change interrupt
> 
> On Wed, Nov 23, 2016 at 01:03:29AM +0000, Yang, Qiming wrote:
> > Yes, I fixed the build errors with ICC. Sorry, I don't know them have already been applied.
> 
> I think I have made a reply to your original patches, that I have applied them.
> 
> > I'll send another patch for bug fix.
> > Also need to add fix line? 
> 
> Yes. And please submit it to stable mailing list only.
> 
> Thanks.
> 
> 	--yliu
> > 
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Tuesday, November 22, 2016 8:33 PM
> > To: Yang, Qiming <qiming.yang@intel.com>
> > Cc: stable@dpdk.org
> > Subject: Re: [PATCH 1/2] net/i40e: fix link status change interrupt
> > 
> > On Tue, Nov 22, 2016 at 06:37:23PM +0800, Qiming Yang wrote:
> > > [ backported from upstream commit
> > > b52fe009edc166bfce205c8ad931190c7d3f2d5f ]
> > 
> > Do you meant to fix the build errors with ICC? The two patches have already been applied, meaning you should send patches on top of the current 16.07 stable branch to fix the build issue.
> > 
> > 	--yliu
> > 
> > > 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: 4861cde46116 ("i40e: new poll mode driver")
> > > 
> > > 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.7.4

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

* Re: [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt
  2016-11-23  2:47       ` Yang, Qiming
@ 2016-11-23  3:00         ` Yuanhan Liu
  2016-11-23  8:46           ` Yang, Qiming
  0 siblings, 1 reply; 12+ messages in thread
From: Yuanhan Liu @ 2016-11-23  3:00 UTC (permalink / raw)
  To: Yang, Qiming; +Cc: stable

On Wed, Nov 23, 2016 at 02:47:15AM +0000, Yang, Qiming wrote:
> I can't find my commits in the current 16.07 stable branch, so I don't know how to write the fix line.

The two I applied from you are:

    718d247 net/i40e: fix VF bonded device link down
    0b02249 net/i40e: fix link status change interrupt

	--yliu

> If the old patches didn't apply, can I send v2 patch set?
> 
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] 
> Sent: Wednesday, November 23, 2016 9:39 AM
> To: Yang, Qiming <qiming.yang@intel.com>
> Cc: stable@dpdk.org
> Subject: Re: [PATCH 1/2] net/i40e: fix link status change interrupt
> 
> On Wed, Nov 23, 2016 at 01:03:29AM +0000, Yang, Qiming wrote:
> > Yes, I fixed the build errors with ICC. Sorry, I don't know them have already been applied.
> 
> I think I have made a reply to your original patches, that I have applied them.
> 
> > I'll send another patch for bug fix.
> > Also need to add fix line? 
> 
> Yes. And please submit it to stable mailing list only.
> 
> Thanks.
> 
> 	--yliu
> > 
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> > Sent: Tuesday, November 22, 2016 8:33 PM
> > To: Yang, Qiming <qiming.yang@intel.com>
> > Cc: stable@dpdk.org
> > Subject: Re: [PATCH 1/2] net/i40e: fix link status change interrupt
> > 
> > On Tue, Nov 22, 2016 at 06:37:23PM +0800, Qiming Yang wrote:
> > > [ backported from upstream commit
> > > b52fe009edc166bfce205c8ad931190c7d3f2d5f ]
> > 
> > Do you meant to fix the build errors with ICC? The two patches have already been applied, meaning you should send patches on top of the current 16.07 stable branch to fix the build issue.
> > 
> > 	--yliu
> > 
> > > 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: 4861cde46116 ("i40e: new poll mode driver")
> > > 
> > > 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.7.4

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

* Re: [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt
  2016-11-23  1:38     ` Yuanhan Liu
@ 2016-11-23  2:47       ` Yang, Qiming
  2016-11-23  3:00         ` Yuanhan Liu
  0 siblings, 1 reply; 12+ messages in thread
From: Yang, Qiming @ 2016-11-23  2:47 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: stable

I can't find my commits in the current 16.07 stable branch, so I don't know how to write the fix line.
If the old patches didn't apply, can I send v2 patch set?

-----Original Message-----
From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] 
Sent: Wednesday, November 23, 2016 9:39 AM
To: Yang, Qiming <qiming.yang@intel.com>
Cc: stable@dpdk.org
Subject: Re: [PATCH 1/2] net/i40e: fix link status change interrupt

On Wed, Nov 23, 2016 at 01:03:29AM +0000, Yang, Qiming wrote:
> Yes, I fixed the build errors with ICC. Sorry, I don't know them have already been applied.

I think I have made a reply to your original patches, that I have applied them.

> I'll send another patch for bug fix.
> Also need to add fix line? 

Yes. And please submit it to stable mailing list only.

Thanks.

	--yliu
> 
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com]
> Sent: Tuesday, November 22, 2016 8:33 PM
> To: Yang, Qiming <qiming.yang@intel.com>
> Cc: stable@dpdk.org
> Subject: Re: [PATCH 1/2] net/i40e: fix link status change interrupt
> 
> On Tue, Nov 22, 2016 at 06:37:23PM +0800, Qiming Yang wrote:
> > [ backported from upstream commit
> > b52fe009edc166bfce205c8ad931190c7d3f2d5f ]
> 
> Do you meant to fix the build errors with ICC? The two patches have already been applied, meaning you should send patches on top of the current 16.07 stable branch to fix the build issue.
> 
> 	--yliu
> 
> > 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: 4861cde46116 ("i40e: new poll mode driver")
> > 
> > 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.7.4

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

* Re: [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt
  2016-11-23  1:03   ` Yang, Qiming
@ 2016-11-23  1:38     ` Yuanhan Liu
  2016-11-23  2:47       ` Yang, Qiming
  0 siblings, 1 reply; 12+ messages in thread
From: Yuanhan Liu @ 2016-11-23  1:38 UTC (permalink / raw)
  To: Yang, Qiming; +Cc: stable

On Wed, Nov 23, 2016 at 01:03:29AM +0000, Yang, Qiming wrote:
> Yes, I fixed the build errors with ICC. Sorry, I don't know them have already been applied.

I think I have made a reply to your original patches, that I have
applied them.

> I'll send another patch for bug fix.
> Also need to add fix line? 

Yes. And please submit it to stable mailing list only.

Thanks.

	--yliu
> 
> -----Original Message-----
> From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] 
> Sent: Tuesday, November 22, 2016 8:33 PM
> To: Yang, Qiming <qiming.yang@intel.com>
> Cc: stable@dpdk.org
> Subject: Re: [PATCH 1/2] net/i40e: fix link status change interrupt
> 
> On Tue, Nov 22, 2016 at 06:37:23PM +0800, Qiming Yang wrote:
> > [ backported from upstream commit 
> > b52fe009edc166bfce205c8ad931190c7d3f2d5f ]
> 
> Do you meant to fix the build errors with ICC? The two patches have already been applied, meaning you should send patches on top of the current 16.07 stable branch to fix the build issue.
> 
> 	--yliu
> 
> > 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: 4861cde46116 ("i40e: new poll mode driver")
> > 
> > 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.7.4

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

* Re: [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt
  2016-11-22 12:33 ` Yuanhan Liu
@ 2016-11-23  1:03   ` Yang, Qiming
  2016-11-23  1:38     ` Yuanhan Liu
  0 siblings, 1 reply; 12+ messages in thread
From: Yang, Qiming @ 2016-11-23  1:03 UTC (permalink / raw)
  To: Yuanhan Liu; +Cc: stable

Yes, I fixed the build errors with ICC. Sorry, I don't know them have already been applied. I'll send another patch for bug fix.
Also need to add fix line? 

-----Original Message-----
From: Yuanhan Liu [mailto:yuanhan.liu@linux.intel.com] 
Sent: Tuesday, November 22, 2016 8:33 PM
To: Yang, Qiming <qiming.yang@intel.com>
Cc: stable@dpdk.org
Subject: Re: [PATCH 1/2] net/i40e: fix link status change interrupt

On Tue, Nov 22, 2016 at 06:37:23PM +0800, Qiming Yang wrote:
> [ backported from upstream commit 
> b52fe009edc166bfce205c8ad931190c7d3f2d5f ]

Do you meant to fix the build errors with ICC? The two patches have already been applied, meaning you should send patches on top of the current 16.07 stable branch to fix the build issue.

	--yliu

> 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: 4861cde46116 ("i40e: new poll mode driver")
> 
> 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.7.4

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

* Re: [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt
  2016-11-22 10:37 Qiming Yang
@ 2016-11-22 12:33 ` Yuanhan Liu
  2016-11-23  1:03   ` Yang, Qiming
  0 siblings, 1 reply; 12+ messages in thread
From: Yuanhan Liu @ 2016-11-22 12:33 UTC (permalink / raw)
  To: Qiming Yang; +Cc: stable

On Tue, Nov 22, 2016 at 06:37:23PM +0800, Qiming Yang wrote:
> [ backported from upstream commit b52fe009edc166bfce205c8ad931190c7d3f2d5f ]

Do you meant to fix the build errors with ICC? The two patches have
already been applied, meaning you should send patches on top of the
current 16.07 stable branch to fix the build issue.

	--yliu

> 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: 4861cde46116 ("i40e: new poll mode driver")
> 
> 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.7.4

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

* [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt
@ 2016-11-22 10:37 Qiming Yang
  2016-11-22 12:33 ` Yuanhan Liu
  0 siblings, 1 reply; 12+ messages in thread
From: Qiming Yang @ 2016-11-22 10:37 UTC (permalink / raw)
  To: stable; +Cc: yuanhan.liu, Qiming Yang

[ backported from upstream commit b52fe009edc166bfce205c8ad931190c7d3f2d5f ]

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: 4861cde46116 ("i40e: new poll mode driver")

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.7.4

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

* [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt
@ 2016-11-17 12:08 Qiming Yang
  0 siblings, 0 replies; 12+ messages in thread
From: Qiming Yang @ 2016-11-17 12:08 UTC (permalink / raw)
  To: stable; +Cc: yuanhan.liu, Qiming Yang

[ backported from upstream commit f4668a33efe5311301e10bc6e3709a2eccb446ad ]

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: 4861cde46116 ("i40e: new poll mode driver")

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.7.4

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

end of thread, other threads:[~2016-11-23  8:46 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-17 11:12 [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt Qiming Yang
2016-11-17 11:12 ` [dpdk-stable] [PATCH 2/2] net/i40e: fix VF bonded device link down Qiming Yang
2016-11-17 12:02   ` Yuanhan Liu
2016-11-17 12:03   ` Yuanhan Liu
2016-11-17 12:08 [dpdk-stable] [PATCH 1/2] net/i40e: fix link status change interrupt Qiming Yang
2016-11-22 10:37 Qiming Yang
2016-11-22 12:33 ` Yuanhan Liu
2016-11-23  1:03   ` Yang, Qiming
2016-11-23  1:38     ` Yuanhan Liu
2016-11-23  2:47       ` Yang, Qiming
2016-11-23  3:00         ` Yuanhan Liu
2016-11-23  8:46           ` Yang, Qiming

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).