[v2] net/ixgbe: fix link state timing issue on fiber ports

Message ID 1588933707-4672-1-git-send-email-phil.yang@arm.com (mailing list archive)
State Accepted, archived
Delegated to: xiaolong ye
Headers
Series [v2] net/ixgbe: fix link state timing issue on fiber ports |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-nxp-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-testing fail Testing issues

Commit Message

Phil Yang May 8, 2020, 10:28 a.m. UTC
  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_thread_handler() is scheduled which should
try to set up the link and clear the flag afterwards. This flag works
as a guard variable between threads.

To avoid potential race condition between threads, set the
IXGBE_FLAG_NEED_LINK_CONFIG flag only when there is no link thread
running.

Fixes: 819d0d1d57f1 ("net/ixgbe: fix blocking system events")
Cc: stable@dpdk.org

Bugzilla ID: 388

Signed-off-by: Phil Yang <phil.yang@arm.com>
Reviewed-by: Lijian Zhang <lijian.zhang@arm.com>
---
v2:
1. rebase code.

 drivers/net/ixgbe/ixgbe_ethdev.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
  

Comments

Xiaolong Ye May 11, 2020, 2:49 a.m. UTC | #1
On 05/08, Phil Yang wrote:
>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_thread_handler() is scheduled which should
>try to set up the link and clear the flag afterwards. This flag works
>as a guard variable between threads.
>
>To avoid potential race condition between threads, set the
>IXGBE_FLAG_NEED_LINK_CONFIG flag only when there is no link thread
>running.
>
>Fixes: 819d0d1d57f1 ("net/ixgbe: fix blocking system events")
>Cc: stable@dpdk.org
>
>Bugzilla ID: 388
>
>Signed-off-by: Phil Yang <phil.yang@arm.com>
>Reviewed-by: Lijian Zhang <lijian.zhang@arm.com>
>---
>v2:
>1. rebase code.
>
> drivers/net/ixgbe/ixgbe_ethdev.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
>index aa1e8aa..dc19f0c 100644
>--- a/drivers/net/ixgbe/ixgbe_ethdev.c
>+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
>@@ -4276,9 +4276,13 @@ 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;
> 			ixgbe_dev_wait_setup_link_complete(dev, 0);
> 			if (rte_atomic32_test_and_set(&ad->link_thread_running)) {
>+				/* To avoid race condition between threads, set
>+				 * the IXGBE_FLAG_NEED_LINK_CONFIG flag only
>+				 * when there is no link thread running.
>+				 */
>+				intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
> 				if (rte_ctrl_thread_create(&ad->link_thread_tid,
> 					"ixgbe-link-handler",
> 					NULL,
>-- 
>2.7.4
>

Acked-by: Xiaolong Ye <xiaolong.ye@intel.com>

Applied to dpdk-next-net-intel, Thanks.
  

Patch

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index aa1e8aa..dc19f0c 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -4276,9 +4276,13 @@  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;
 			ixgbe_dev_wait_setup_link_complete(dev, 0);
 			if (rte_atomic32_test_and_set(&ad->link_thread_running)) {
+				/* To avoid race condition between threads, set
+				 * the IXGBE_FLAG_NEED_LINK_CONFIG flag only
+				 * when there is no link thread running.
+				 */
+				intr->flags |= IXGBE_FLAG_NEED_LINK_CONFIG;
 				if (rte_ctrl_thread_create(&ad->link_thread_tid,
 					"ixgbe-link-handler",
 					NULL,