[Bug 1249] [dpdk-23.07] ixgbe port stop failed after launch testpmd

[Bug 1249] [dpdk-23.07] ixgbe port stop failed after launch testpmd

[bugzilla]

[-- Attachment #1: Type: text/plain, Size: 2009 bytes --]

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: dev@dpdk.org
          Reporter: linglix.chen@intel.com
  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 <roretzla@linux.microsoft.com>
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 <roretzla@linux.microsoft.com>
    Acked-by: Morten Brørup <mb@smartsharesystems.com>

-- 
You are receiving this mail because:
You are the assignee for the bug.

[-- Attachment #2: Type: text/html, Size: 4072 bytes --]

Re: [Bug 1249] [dpdk-23.07] ixgbe port stop failed after launch testpmd

[David Marchand]

Hello Tyler,

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

On Mon, Jun 12, 2023 at 9:29 AM <bugzilla@dpdk.org> 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 dev@dpdk.org
> Reporter linglix.chen@intel.com
> 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 <roretzla@linux.microsoft.com>
> 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 <roretzla@linux.microsoft.com>
>     Acked-by: Morten Brørup <mb@smartsharesystems.com>

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


Thanks.

-- 
David Marchand

Re: [Bug 1249] [dpdk-23.07] ixgbe port stop failed after launch testpmd

[Tyler Retzlaff]

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.


On Mon, Jun 12, 2023 at 09:43:23AM +0200, David Marchand wrote:
> Hello Tyler,
> 
> (I could not assign this bz to you, please register to bugs.dpdk.org)
> 
> On Mon, Jun 12, 2023 at 9:29 AM <bugzilla@dpdk.org> 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 dev@dpdk.org
> > Reporter linglix.chen@intel.com
> > 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 <roretzla@linux.microsoft.com>
> > 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 <roretzla@linux.microsoft.com>
> >     Acked-by: Morten Brørup <mb@smartsharesystems.com>
> 
> I suspect this change broke handling of multiple ports by converting a
> counter atomic to a simple boolean.
> Can you have a look?
> 
> 
> Thanks.
> 
> -- 
> David Marchand

Re: [Bug 1249] [dpdk-23.07] ixgbe port stop failed after launch testpmd

[David Marchand]

Hello,

On Mon, Jun 12, 2023 at 6:32 PM Tyler Retzlaff
<roretzla@linux.microsoft.com> wrote:
> 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.


-- 
David Marchand

RE: [Bug 1249] [dpdk-23.07] ixgbe port stop failed after launch testpmd

[Yang, 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.

Qiming

> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Tuesday, June 13, 2023 5:51 PM
> To: Tyler Retzlaff <roretzla@linux.microsoft.com>; Yang, Qiming
> <qiming.yang@intel.com>; Wu, Wenjun1 <wenjun1.wu@intel.com>
> Cc: dev@dpdk.org; Morten Brørup <mb@smartsharesystems.com>
> Subject: Re: [Bug 1249] [dpdk-23.07] ixgbe port stop failed after launch
> testpmd
> 
> Hello,
> 
> On Mon, Jun 12, 2023 at 6:32 PM Tyler Retzlaff
> <roretzla@linux.microsoft.com> wrote:
> > 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.
> 
> 
> --
> David Marchand

Re: [Bug 1249] [dpdk-23.07] ixgbe port stop failed after launch testpmd

[David Marchand]

On Wed, Jun 14, 2023 at 8:57 AM Yang, Qiming <qiming.yang@intel.com> wrote:
>
> 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?


-- 
David Marchand

RE: [Bug 1249] [dpdk-23.07] ixgbe port stop failed after launch testpmd

[Morten Brørup]

> From: David Marchand [mailto:david.marchand@redhat.com]
> Sent: Wednesday, 14 June 2023 09.14
> 
> On Wed, Jun 14, 2023 at 8:57 AM Yang, Qiming <qiming.yang@intel.com> wrote:
> >
> > 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.

> 
> Performance impact is not a problem for control path code, is it?
> 
> 
> --
> David Marchand