[dpdk-dev,1/2] net/i40e: fix link status change interrupt
Commit Message
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(-)
Comments
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
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
> -----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
@@ -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);