Hi Lijian, Yes, that seems like a good idea to me.
There is a VPP patch that I added for DPDK 19.08 to deal with the same issue at build/external/patches/dpdk_19.08/0001-ixgbe-fix-link-state-timing-issue-on-fiber-ports.patch. It looks like my patch may not apply cleanly to DPDK 20.02 because they changed how the task to update link status is managed (rte_eal_alarm -> thread). When you submit your patch in gerrit, you are welcome to add me as a reviewer. I reported a bug to DPDK when I saw this issue - https://bugs.dpdk.org/show_bug.cgi?id=388. If you need a "Fixes:" tag for your patch when you submit it to upstream DPDK, you could reference that bug ID. Thanks, -Matt On Wed, Mar 18, 2020 at 9:12 AM Lijian Zhang <lijian.zh...@arm.com> wrote: > Hi Maintainers, > > > > Currently CSIT performance testing failed on Taishan servers due to > interface link down issue. > > > > This issue is observed with X520-2 NICs on FD.io lab Taishan server. After > VPP booting up and bringing up the interfaces with command ‘set interface > state <interface> up’, it still shows link down status from the command > ‘show hardware-interfaces’. However, the hardware link status is actually > up. dpdk_process() cannot get the hardware link status correctly via > rte_eth_link_get_nowait(). > > > > In ixgbe_dev_link_update_share(), if the media type is fiber and the link > is down, a flag (IXGBE_FLAG_NEED_LINK_CONFIG) is set. A callback to > ixgbe_dev_setup_link_alarm_handler() is scheduled trying to set up the link > and clear the flag afterwards. > > > > If the device is started or stopped before the flag is cleared, the > scheduled callback is canceled. This causes the flag to remain set and > subsequent calls to ixgbe_dev_link_update_share() return without trying to > retrieve the link state because the flag is set. > > * 4242 if (intr->flags & IXGBE_FLAG_NEED_LINK_CONFIG)* > > * 4243 return rte_eth_linkstatus_set(dev, &link);* > > > > When the callback is canceled by either interface start or stop > operations, in ixgbe_dev_cancel_link_thread(), after cancelling the > callback/thread, unset the flag on the device to avoid this condition. > > > > The code change to dpdk-20.02 to fix this issue is as below. > > Can I put a patch to vpp patch directory > (build/external/patches/dpdk_20.02/) to fix the issue temporarily, and > meanwhile we will upstream a corresponding fixing patch in DPDK project? > > > > *diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c > b/drivers/net/ixgbe/ixgbe_ethdev.c* > > *index 23b3f5b0c..aa882cb8b 100644* > > *--- a/drivers/net/ixgbe/ixgbe_ethdev.c* > > *+++ b/drivers/net/ixgbe/ixgbe_ethdev.c* > > *@@ -4147,11 +4147,14 @@ static void* > > *ixgbe_dev_cancel_link_thread(struct rte_eth_dev *dev)* > > *{* > > * struct ixgbe_adapter *ad = dev->data->dev_private;* > > *+ struct ixgbe_interrupt *intr =* > > *+ IXGBE_DEV_PRIVATE_TO_INTR(dev->data->dev_private);* > > * void *retval;* > > > > * if (rte_atomic32_read(&ad->link_thread_running)) {* > > * pthread_cancel(ad->link_thread_tid);* > > * pthread_join(ad->link_thread_tid, &retval);* > > *+ intr->flags &= ~IXGBE_FLAG_NEED_LINK_CONFIG;* > > * rte_atomic32_clear(&ad->link_thread_running);* > > * }* > > *}* > > *@@ -4262,8 +4265,8 @@ ixgbe_dev_link_update_share(struct rte_eth_dev > *dev,* > > * if (link_up == 0) {* > > * if (ixgbe_get_media_type(hw) == ixgbe_media_type_fiber) {* > > *- intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;* > > * if > (rte_atomic32_test_and_set(&ad->link_thread_running)) {* > > *+ intr->flags |= > IXGBE_FLAG_NEED_LINK_CONFIG;* > > * if > (rte_ctrl_thread_create(&ad->link_thread_tid,* > > * "ixgbe-link-handler",* > > * NULL,* > > Thanks. > > > >
-=-=-=-=-=-=-=-=-=-=-=- Links: You receive all messages sent to this group. View/Reply Online (#15812): https://lists.fd.io/g/vpp-dev/message/15812 Mute This Topic: https://lists.fd.io/mt/72049580/21656 Group Owner: vpp-dev+ow...@lists.fd.io Unsubscribe: https://lists.fd.io/g/vpp-dev/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-