Skip to contentSkip to conversation

ixgbe port stop failed after launch testpmd

Bug 1249 [email protected] 5 participants 7 messages Jun 12, 2023 – Jun 14, 2023

lore
Share & tools
lore

[email protected]

author Acked-by Morten Brørup
#0 lore raw

https://bugs.dpdk.org/show_bug.cgi?id=1249

            Bug ID: 1249
           Summary: [dpdk-23.07] ixgbe port stop failed after launch
                    testpmd
           Product: DPDK
           Version: 23.07
          Hardware: All
                OS: All
            Status: UNCONFIRMED
          Severity: normal
          Priority: Normal
         Component: testpmd
          Assignee: [email protected]
          Reporter: [email protected]

Target Milestone: ---

Environment
OS: Ubuntu 22.04.2 LTS/5.15.0-71-generic
DPDK version: 8a2de735318b38ba93a041cb2f209889bde02689
Compiler: gcc version 11.3.0
NIC hardware: 82599ES 10-Gigabit SFI/SFP+ Network Connection 10fb NIC driver: ixgbe-5.18.11
NIC firmware: 0x000161bf

Test Setup

1. bind 1 port to vfio-pci ./usertools/dpdk-devbind.py -b vfio-pci 0000:b1:00.0

2. launch testpmd x86_64-native-linuxapp-gcc/app/dpdk-testpmd -l 1-2 -n 4 -- -i

3.testpmd>port stop 0
testpmd> show port info all

Show the output from the previous commands
testpmd>port stop 0
Stopping ports...
Checking link statuses...
ixgbe_dev_link_update_share(): Other link thread is running now! Done
testpmd> show port info all
ixgbe_dev_wait_setup_link_complete(): IXGBE link thread not complete too long time!
need kill to quit testpmd.

Expected Result
port stop normal.

Regression
Is this issue a regression: (Y/N)Y

Version the regression was introduced: Specify git id if known. commit e90baf6b82f66c880c7e7f8f77fc534980165808

Author: Tyler Retzlaff <[email protected]>
Date:   Tue Jun 6 14:45:45 2023 -0700

net/ixgbe: replace legacy atomics with GCC builtin atomics

Replace the use of rte_atomic.h types and functions, instead use GCC supplied C++11 memory model builtins.

Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Morten Brørup <[email protected]>
Signature · 2 lines
You are receiving this mail because:
You are the assignee for the bug.
#1 lore raw

Hello Tyler,

(I could not assign this bz to you, please register to bugs.dpdk.org)

Quoted 56 lines · On Mon, Jun 12, 2023 at 9:29 AM <[email protected]> wrote:

Bug ID 1249
Summary [dpdk-23.07] ixgbe port stop failed after launch testpmd Product DPDK
Version 23.07
Hardware All
OS All
Status UNCONFIRMED
Severity normal
Priority Normal
Component testpmd
Assignee [email protected]
Reporter [email protected]
Target Milestone ---

Environment
OS: Ubuntu 22.04.2 LTS/5.15.0-71-generic
DPDK version: 8a2de735318b38ba93a041cb2f209889bde02689
Compiler: gcc version 11.3.0
NIC hardware: 82599ES 10-Gigabit SFI/SFP+ Network Connection 10fb NIC driver: ixgbe-5.18.11
NIC firmware: 0x000161bf

Test Setup

1. bind 1 port to vfio-pci ./usertools/dpdk-devbind.py -b vfio-pci 0000:b1:00.0

2. launch testpmd x86_64-native-linuxapp-gcc/app/dpdk-testpmd -l 1-2 -n 4 -- -i

3.testpmd>port stop 0
testpmd> show port info all

Show the output from the previous commands
testpmd>port stop 0
Stopping ports...
Checking link statuses...
ixgbe_dev_link_update_share(): Other link thread is running now! Done
testpmd> show port info all
ixgbe_dev_wait_setup_link_complete(): IXGBE link thread not complete too long time!
need kill to quit testpmd.

Expected Result
port stop normal.

Regression
Is this issue a regression: (Y/N)Y

Version the regression was introduced: Specify git id if known. commit e90baf6b82f66c880c7e7f8f77fc534980165808

Author: Tyler Retzlaff <[email protected]>
Date:   Tue Jun 6 14:45:45 2023 -0700
Quoted 7 lines

net/ixgbe: replace legacy atomics with GCC builtin atomics

Replace the use of rte_atomic.h types and functions, instead use GCC supplied C++11 memory model builtins.

Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Morten Brørup <[email protected]>

I suspect this change broke handling of multiple ports by converting a counter atomic to a simple boolean.
Can you have a look?

Thanks.

Signature · 1 line
David Marchand
#2 lore raw

Hi David,

I'm out of office until Thursday so I am not able to properly look right now.

If there has been a broad regression here could I ask for your help in either reverting it until I can look at it properly or if you see the obvious problem fixing it.

There were multiple instances of flags being converted to bool in the series it may be that they all need to be backed out.

Very disappointing if I missed something here.

Sorry for the trouble.

Quoted 78 lines · David Marchand

Hello Tyler,

(I could not assign this bz to you, please register to bugs.dpdk.org)

Quoted 56 lines · On Mon, Jun 12, 2023 at 9:29 AM <[email protected]> wrote:

Bug ID 1249
Summary [dpdk-23.07] ixgbe port stop failed after launch testpmd Product DPDK
Version 23.07
Hardware All
OS All
Status UNCONFIRMED
Severity normal
Priority Normal
Component testpmd
Assignee [email protected]
Reporter [email protected]
Target Milestone ---

Environment
OS: Ubuntu 22.04.2 LTS/5.15.0-71-generic
DPDK version: 8a2de735318b38ba93a041cb2f209889bde02689
Compiler: gcc version 11.3.0
NIC hardware: 82599ES 10-Gigabit SFI/SFP+ Network Connection 10fb NIC driver: ixgbe-5.18.11
NIC firmware: 0x000161bf

Test Setup

1. bind 1 port to vfio-pci ./usertools/dpdk-devbind.py -b vfio-pci 0000:b1:00.0

2. launch testpmd x86_64-native-linuxapp-gcc/app/dpdk-testpmd -l 1-2 -n 4 -- -i

3.testpmd>port stop 0
testpmd> show port info all

Show the output from the previous commands
testpmd>port stop 0
Stopping ports...
Checking link statuses...
ixgbe_dev_link_update_share(): Other link thread is running now! Done
testpmd> show port info all
ixgbe_dev_wait_setup_link_complete(): IXGBE link thread not complete too long time!
need kill to quit testpmd.

Expected Result
port stop normal.

Regression
Is this issue a regression: (Y/N)Y

Version the regression was introduced: Specify git id if known. commit e90baf6b82f66c880c7e7f8f77fc534980165808

Author: Tyler Retzlaff <[email protected]>
Date:   Tue Jun 6 14:45:45 2023 -0700
Quoted 7 lines

net/ixgbe: replace legacy atomics with GCC builtin atomics

Replace the use of rte_atomic.h types and functions, instead use GCC supplied C++11 memory model builtins.

Signed-off-by Tyler Retzlaff <[email protected]>
Acked-by Morten Brørup <[email protected]>

I suspect this change broke handling of multiple ports by converting a counter atomic to a simple boolean.
Can you have a look?

Thanks.

Signature · 1 line
David Marchand
#3 lore raw

Hello,

Quoted 6 lines · Tyler Retzlaff

If there has been a broad regression here could I ask for your help in either reverting it until I can look at it properly or if you see the obvious problem fixing it.

There were multiple instances of flags being converted to bool in the series it may be that they all need to be backed out.

I relooked at Tyler series, and I think the net/ixgbe patch is the only one that has an issue.

It converted:
- rte_atomic32_t link_thread_running;
+ bool link_thread_running;

IIUC, link_thread_running was used as a kind of refcount before Tyler change. So rather than revert, I suppose the fix is to move to a uint32_t (and rename the variable maybe).

ixgbe maintainers, please confirm.

Signature · 1 line
David Marchand
#4 lore raw

Hi, Tyler
May I know why you need to change the legacy atomics? rte_atomic32_xxx series action is performance optimized, I don't think the change is needed as no bug or other requirement.

Qiming

Quoted 36 lines

-----Original Message-----
From: David Marchand <[email protected]>
Sent: Tuesday, June 13, 2023 5:51 PM
To: Tyler Retzlaff <[email protected]>; Yang, Qiming <[email protected]>; Wu, Wenjun1 <[email protected]>
Cc: [email protected]; Morten Brørup <[email protected]>
Subject: Re: [Bug 1249] [dpdk-23.07] ixgbe port stop failed after launch testpmd

Hello,

Quoted 6 lines · Tyler Retzlaff

If there has been a broad regression here could I ask for your help in either reverting it until I can look at it properly or if you see the obvious problem fixing it.

There were multiple instances of flags being converted to bool in the series it may be that they all need to be backed out.

I relooked at Tyler series, and I think the net/ixgbe patch is the only one that has an issue.

It converted:

(quoted diff) +1 -1
-    rte_atomic32_t link_thread_running;
+    bool link_thread_running;

IIUC, link_thread_running was used as a kind of refcount before Tyler change. So rather than revert, I suppose the fix is to move to a uint32_t (and rename the variable maybe).

ixgbe maintainers, please confirm.

--
David Marchand

#5 lore raw
Quoted 3 lines · Qiming

Hi, Tyler
May I know why you need to change the legacy atomics? rte_atomic32_xxx series action is performance optimized, I don't think the change is needed as no bug or other requirement.

The deprecation of rte_atomic32_xxx atomics is not a new topic.

Performance impact is not a problem for control path code, is it?

Signature · 1 line
David Marchand
#6 lore raw
Quoted 11 lines

From: David Marchand [mailto:[email protected]]
Sent: Wednesday, 14 June 2023 09.14

Quoted 3 lines · Qiming

Hi, Tyler
May I know why you need to change the legacy atomics? rte_atomic32_xxx

series action is performance optimized, I don't think the change is needed as no bug or other requirement.

The deprecation of rte_atomic32_xxx atomics is not a new topic.

It would be nice if the roadmap web page mentioned that the legacy rte_atomicXX API is being phased out in favor of C11 atomics, and other long term directions for DPDK.

Quoted 6 lines

Performance impact is not a problem for control path code, is it?

--
David Marchand