From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2AD4543EA2; Thu, 18 Apr 2024 15:40:09 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D1D244068A; Thu, 18 Apr 2024 15:40:08 +0200 (CEST) Received: from mail-pf1-f173.google.com (mail-pf1-f173.google.com [209.85.210.173]) by mails.dpdk.org (Postfix) with ESMTP id B5EBD40042 for ; Thu, 18 Apr 2024 15:40:06 +0200 (CEST) Received: by mail-pf1-f173.google.com with SMTP id d2e1a72fcca58-6ed01c63657so841185b3a.2 for ; Thu, 18 Apr 2024 06:40:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; t=1713447606; x=1714052406; darn=dpdk.org; h=cc:to:subject:message-id:date:from:mime-version:from:to:cc:subject :date:message-id:reply-to; bh=Gf0zTZBQ1Pc8h923OH0YhICWhdTJ9l8ZhXLHuFQfNBg=; b=gznKrcWcalSepo391S6+86rn577DpV2FM4NXPCKq4P93BJF8+IulQuQ/hk6DcVeMsc nOGPqTz+YjtcAn4VpQAoxzEjFncKyIctUw9DpbktYhuc7GP55nPzO9tk2ik45cugGwqA rXVIVraDPlKHIbLIs9+KshFjFgE3v0gW03DkzzYQjXNVxKKGVQNsAoT9xwihDHsEiDPU uLRsEVvrYxywPIK790cPLy0ly8DQvlPKsI8yOPnlq3EmSYdY76EEMaN8ABa4EQZLY8tM 4iQzp0UVnVO5kA/LJlgStMceIw9rawzEavaqIIcmr7Brsew4fYFXLcXkEq5gFQ+tAdM0 rN+Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1713447606; x=1714052406; h=cc:to:subject:message-id:date:from:mime-version:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=Gf0zTZBQ1Pc8h923OH0YhICWhdTJ9l8ZhXLHuFQfNBg=; b=IE+vd8m5IVuQ5S+hokYsZVTc3MNTmoTZp85n2W0r7Xh7jvZJ6ctU7/VOA+MkQozavu mX24lNOJAfJS6TIkEdH1J1kUgkJiaclIKQNQ6YYJNlhBiX26tBVNpR+0yT0COmAK5EvD xCsTR1wgC8UcDq0aSiYwUrn9YPB4aLNPj2P1LhYauSYh5UzU98hnbzQS+XufAZvBhJz2 fg5Ev2dWKE0L+DY/1KCHT4kGyQXZXggo7sRtuyoeSN6hTh6NPitTjKOUJvqjSsNgPLaG I4M2dSn3LmGkX0kKjjh+7hjzHNsaYKPo2IgQ//bArrETIj9kUzU0uzuxghazsD/lWlJp MweQ== X-Gm-Message-State: AOJu0Yx80UdWcdzwN7aYT68k/8hNTQzJ6F8Iy//81k74XlgpWTZYWExd 3KHffXC6qrTh64a2JZHEBq00FX+UM387wd/LUjhCOOoSsqbuVWsEF6nzqKFKB9xheOqPZniBt8v ClPHXA4+KQ1jxP8dXOOt1rIyjMTJ/H9b2XzQ3Vcg4i/NISXfp29s= X-Google-Smtp-Source: AGHT+IH7UOqp8utGqt0iw0vS4EOmT2+cG1leQdNxUfgdsTEsuOvm90Rc38aLbdGmRjTm/qfitHpLzfTAnHfE0iuJ7wE= X-Received: by 2002:a05:6a00:9295:b0:6ed:de70:5ef8 with SMTP id jw21-20020a056a00929500b006edde705ef8mr3645210pfb.6.1713447605752; Thu, 18 Apr 2024 06:40:05 -0700 (PDT) MIME-Version: 1.0 From: Edwin Brossette Date: Thu, 18 Apr 2024 15:39:54 +0200 Message-ID: Subject: net/ixgbe: all interrupt disabled after enabling lsc on X552/X557-AT card To: dev@dpdk.org Cc: Olivier Matz Content-Type: multipart/alternative; boundary="000000000000e1f2e306165f1b6d" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org --000000000000e1f2e306165f1b6d Content-Type: text/plain; charset="UTF-8" Hello, I have found a bug in the ixgbe pmd, concerning this model of nic in particular: -> Intel Corporation Ethernet Connection X552/X557-AT 10GBASE-T I was able to detect this bug after enabling the use of Link state change interrupt in the card via the following flag: - port->rte_conf.intr_conf.lsc = 1; With these interrupts enabled on this particular network card, there is a chance on every link-up that all interrupts get disabled, as the interrupt mask is unintentionally set to 0. This happens because there are two interrupts that are triggered on a link status change on this model of nic. See related code here: https://git.dpdk.org/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c?id=c99e1db87798c2cd2c0ac94c7676e961eedffc87#n4552 Whenever an lsc interrupt is triggered, we enter this ixgbe_dev_interrupt_get_status() function. Bit 20 (IXGBE_EICR_LSC ) and 25 (IXGBE_EICR_GPI_SDP0_X550EM_x ) of the Extendend Interrupt Cause Register (EICR) will be set to '1' on a link status change. When the interrupt is handled in the ixgbe_dev_interrupt_delayed_handler() function, if the IXGBE_FLAG_NEED_LINK_UPDATE flag is raised (happens when EICR_LSC bit is raised in eicr), an alarm for a delayed handler will be programmed. The following code poses issue: https://git.dpdk.org/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c?id=c99e1db87798c2cd2c0ac94c7676e961eedffc87#n4736 intr->mask = intr->mask_original; intr->mask_original = 0; Sometimes, it's possible to see a first interrupt with bit IXGBE_EICR_LSC raised, quickly followed by a second interrupt with both bit IXGBE_EICR_LSC and bit IXGBE_EICR_GPI_SDP0_X550EM_x being raised. In this case, the flag IXGBE_FLAG_NEED_LINK_UPDATE will be raised in both cases and the delayed handler will be programmed to be ran twice. This will lead to intr->mask_original being set to 0 after the first delayed_handler is done, and then intr->mask being also set to 0 after the second time it's run. This effectively disables all interrupts from working on the device, which is particularly painful since we no longer poll for link state on our side, having switched to interrupt-based link state handling. My suggestion to solve this issue is to avoid creating an alarm for ixgbe_dev_interrupt_delayed_handler() if there is already one pending. I do this by checking for bit IXGBE_EIMS_LSC in intr->mask. I no longer see this issue after pushing in my local repository. I will suggest a patch by responding to this mail. I can show a way to reproduce bellow with test pmd. I have added some custom logs into the code for my own convenience: ===================================== 1) start testpmd: PCI-SLOT IFNAME MAC-ADDRESS KMOD MAX SPEED CURRENT SPEED DEVICE 0000:03:00.0 ntfp1 0c:c4:7a:75:8f:ec ixgbe 1x2.5 GT/s PCIe 1x2.5 GT/s PCIe Intel Corporation Ethernet Connection X552/X557-AT 10GBASE-T 0000:03:00.1 ntfp2 0c:c4:7a:75:8f:ed ixgbe 1x2.5 GT/s PCIe 1x2.5 GT/s PCIe Intel Corporation Ethernet Connection X552/X557-AT 10GBASE-T sudo dpdk-hugepages.py --setup 2G; dpdk-devbind --bind=vfio-pci 0000:03:00.0 dpdk-devbind --bind=vfio-pci 0000:03:00.1 dpdk-testpmd -a 0000:03:00.0 -a 0000:03:00.1 -- -i --rxq=2 --txq=2 --coremask=0x0c --total-num-mbufs=250000 EAL: Detected CPU lcores: 16 EAL: Detected NUMA nodes: 1 EAL: Detected static linkage of DPDK EAL: Multi-process socket /var/run/dpdk/rte/mp_socket EAL: Selected IOVA mode 'PA' EAL: VFIO support initialized EAL: Using IOMMU type 8 (No-IOMMU) EAL: Probe PCI driver: net_ixgbe (8086:15ad) device: 0000:03:00.0 (socket -1) EAL: Probe PCI driver: net_ixgbe (8086:15ad) device: 0000:03:00.1 (socket -1) Interactive-mode selected previous number of forwarding cores 1 - changed to number of configured cores 2 Warning: NUMA should be configured manually by using --port-numa-config and --ring-numa-config parameters along with --numa. testpmd: create a new mbuf pool : n=250000, size=2176, socket=0 testpmd: preferred mempool ops selected: ring_mp_mc Configuring Port 0 (socket 0) Port 0: 0C:C4:7A:75:8F:EC Configuring Port 1 (socket 0) Port 1: 0C:C4:7A:75:8F:ED Checking link statuses... Done 2) To reproduce, simply repeatedly set link up and down. At one point there should be something like this: testpmd> set link-up port 0 ixgbe_dev_interrupt_get_status(): <><><><>><>: eicr register: 100000 <---------------------- IXGBE_EICR_LSC ixgbe_dev_interrupt_get_status(): <><><><>><>: set IXGBE_FLAG_NEED_LINK_UPDATE ixgbe_dev_interrupt_get_status(): <><><><>><>: current flags: flags = 1 ixgbe_dev_interrupt_action(): <><><><>><>: current flags (1): flags = 1 ixgbe_dev_interrupt_action(): <><><><>><>: current flags (2): flags = 1 ixgbe_dev_interrupt_action(): <><><><>><>: current flags (3): flags = 1 ixgbe_dev_interrupt_action(): <><><><>><>: for dev: 0x557c0cd844c0 ixgbe_dev_interrupt_action(): <><><><>><>: saved intr: mask_original = 36700160 <------------ original interrupt mask ixgbe_dev_interrupt_action(): <><><><>><>: current intr: mask = 35651584 <------------ interrupt mask with lsc disabled ixgbe_dev_interrupt_get_status(): <><><><>><>: eicr register: 2100000 <---------------------- IXGBE_EICR_LSC and IXGBE_EICR_GPI_SDP0_X550EM_x ixgbe_dev_interrupt_get_status(): <><><><>><>: set IXGBE_FLAG_NEED_LINK_UPDATE ixgbe_dev_interrupt_get_status(): <><><><>><>: current flags: flags = 5 ixgbe_dev_interrupt_action(): <><><><>><>: current flags (1): flags = 5 ixgbe_dev_interrupt_action(): <><><><>><>: current flags (2): flags = 5 ixgbe_dev_interrupt_action(): <><><><>><>: current flags (3): flags = 1 ixgbe_dev_interrupt_action(): <><><><>><>: for dev: 0x557c0cd844c0 ixgbe_dev_interrupt_action(): <><><><>><>: saved intr: mask_original = 35651584 <------------ original mask is lost here ixgbe_dev_interrupt_action(): <><><><>><>: current intr: mask = 35651584 Port 0: link state change event ixgbe_dev_interrupt_delayed_handler(): <><><><>><>: mask restored: 35651584 testpmd> set link-down port 0ixgbe_dev_interrupt_delayed_handler(): <><><><>><>: mask restored: 0 <--- all interrupts are disabled 3) At this point, link state can no longer be updated: testpmd> show port info 0 ********************* Infos for port 0 ********************* MAC address: 0C:C4:7A:75:8F:EC Device name: 0000:03:00.0 Driver name: net_ixgbe Firmware-version: 0x800001cf Devargs: Connect to socket: 0 memory allocation on the socket: 0 Link status: up <---------------------- Link speed: 10 Gbps Link duplex: full-duplex Autoneg status: On MTU: 1500 testpmd> set link-down port 0 testpmd> show port info ********************* Infos for port 0 ********************* MAC address: 0C:C4:7A:75:8F:EC Device name: 0000:03:00.0 Driver name: net_ixgbe Firmware-version: 0x800001cf Devargs: Connect to socket: 0 memory allocation on the socket: 0 Link status: up <------------- should not be up after link down. Link speed: 10 Gbps Link duplex: full-duplex Autoneg status: On MTU: 1500 --000000000000e1f2e306165f1b6d Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Hello,

I ha= ve found a bug in the ixgbe pmd, concerning this model of nic in particula= r:
=C2=A0=C2=A0=C2=A0=C2=A0 -> Intel Corporation Ethernet Conn= ection X552/X557-AT 10GBASE-T

I was able to detect= this bug after enabling the use of Link state change interrupt in the card= via the following flag:
=C2=A0=C2=A0=C2=A0 - port->rte_conf.i= ntr_conf.lsc =3D 1;

With these interrupts enabled on this particular network card, there is a=20 chance on every link-up that all interrupts get disabled, as the=20 interrupt mask is unintentionally set to 0.
This happens=20 because there are two interrupts=20 that are triggered on a link status change on this model of nic. See relate= d code here:
<= br>
Whenever an lsc interrupt is triggered, we enter this i= xgbe_dev_interrupt_get_status() function.
Bit 20 (IX= GBE_EICR_LSC ) and 25 (IXGBE_EICR_GPI_SDP0_X550EM_x ) o= f the Extendend Interrupt Cause Register (EICR) will be set to '1' = on a link status change.
When the interrupt is handled in the ixgbe_dev_interrupt_delayed_handler() function, if the IXGB= E_FLAG_NEED_LINK_UPDATE flag is raised (happens when EICR_LSC bit is= raised in eicr),=C2=A0 an alarm for a delayed handler will be programmed.<= /div>

The following code poses issue:
intr->mask =3D in=
tr->mask_original;
intr->mask_original =3D 0;
Sometimes, it's po= ssible to see a first interrupt with bit IXGBE_EICR_LSC raised= , quickly followed by a second interrupt with both bit IXGBE_EICR_LSC= and bit IXGBE_EICR_GPI_SDP0_X550EM_x being raised.
In this case, the flag IXGBE_FLAG_NEED_LINK_UPDATE will= be raised in both cases and the delayed handler will be programmed to be r= an twice.
This will lead to intr->mask_original b= eing set to 0 after the first delayed_handler is done, and then intr-= >mask being also set to 0 after the second time it's run.
This effectively disables all interrupts from working on the device, which=20 is particularly painful since we no longer poll for link state on our=20 side, having switched to interrupt-based link state handling.
My suggestion to solve this issue is to avoid creating an alarm= for ixgbe_dev_interrupt_delayed_handler() if there is already= one pending. I do this by checking for bit IXGBE_EIMS_LSC=C2=A0 in i= ntr->mask.
I no longer see this issue after pushing= in my local repository. I will suggest a patch by responding to this mail.=

I can show a way to reproduce bellow with tes= t pmd. I have added some custom logs into the code for my own convenience: =

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D

=C2=A0=C2=A0=C2=A0 1) start testpmd:

<= div>PCI-SLOT =C2=A0 =C2=A0 =C2=A0IFNAME =C2=A0MAC-ADDRESS =C2=A0 =C2=A0 =C2= =A0 =C2=A0KMOD =C2=A0 MAX SPEED =C2=A0 =C2=A0 =C2=A0 =C2=A0CURRENT SPEED = =C2=A0 =C2=A0DEVICE
0000:03:00.0 =C2=A0ntfp1 =C2=A0 0c:c4:7a:75:8f:ec =C2=A0ixgbe =C2=A01x2.5 GT/s PCIe =C2= =A01x2.5 GT/s PCIe=20 =C2=A0Intel Corporation Ethernet Connection X552/X557-AT 10GBASE-T
0000:= 03:00.1 =C2=A0ntfp2 =C2=A0 0c:c4:7a:75:8f:ed =C2=A0ixgbe =C2=A01x2.5 GT/s PCIe =C2= =A01x2.5 GT/s PCIe=20 =C2=A0Intel Corporation Ethernet Connection X552/X557-AT 10GBASE-T

sudo dpdk-hugepages.py --setup 2G;
dpdk-devbind = --bind=3Dvfio-pci 0000:03:00.0
dpdk-devbind --bind=3Dvfio-pci 0000:03:00= .1

=C2=A0dpdk-testpmd -a 0000:03:00.0 -a 0000:03:0= 0.1 -- -i --rxq=3D2 --txq=3D2 --coremask=3D0x0c --total-num-mbufs=3D250000<= br>EAL: Detected CPU lcores: 16
EAL: Detected NUMA nodes: 1
EAL: Dete= cted static linkage of DPDK
EAL: Multi-process socket /var/run/dpdk/rte/= mp_socket
EAL: Selected IOVA mode 'PA'
EAL: VFIO support init= ialized
EAL: Using IOMMU type 8 (No-IOMMU)
EAL: Probe PCI driver: net= _ixgbe (8086:15ad) device: 0000:03:00.0 (socket -1)
EAL: Probe PCI drive= r: net_ixgbe (8086:15ad) device: 0000:03:00.1 (socket -1)
Interactive-mo= de selected
previous number of forwarding cores 1 - changed to number of= configured cores 2
Warning: NUMA should be configured manually by using= --port-numa-config and --ring-numa-config parameters along with --numa.testpmd: create a new mbuf pool <mb_pool_0>: n=3D250000, size=3D2176= , socket=3D0
testpmd: preferred mempool ops selected: ring_mp_mc
Conf= iguring Port 0 (socket 0)
Port 0: 0C:C4:7A:75:8F:EC
Configuring Port = 1 (socket 0)
Port 1: 0C:C4:7A:75:8F:ED
Checking link statuses...
D= one


=C2=A0=C2=A0=C2=A0 2) To reprod= uce, simply repeatedly set link up and down. At one point there should be s= omething like this:

testpmd> set link-up port 0=
ixgbe_dev_interrupt_get_status(): <><><><&g= t;><>: eicr register: 100000=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 <---------------------- IXGBE_EICR_LSC
ixgbe_dev_interrupt_get_status(): <><>= ;<><>><>: set IXGBE_FLAG_NEED_LINK_UPDATE
ixgbe_dev= _interrupt_get_status(): <><><><>><>: curr= ent flags: flags =3D 1
ixgbe_dev_interrupt_action(): <><><= ;><>><>: current flags (1): flags =3D 1
ixgbe_dev_inte= rrupt_action(): <><><><>><>: current flags= (2): flags =3D 1
ixgbe_dev_interrupt_action(): <><><>= <>><>: current flags (3): flags =3D 1
ixgbe_dev_interrupt= _action(): <><><><>><>: for dev: 0x557c0cd= 844c0
ixgbe_dev_interrupt_action(): <><><><>><>: saved intr: mask_original =3D 36700160=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 <------------ original inter= rupt mask
ixgbe_dev_interrupt_action(): <><><><>><>: current intr: mask =3D=20 35651584=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 <------------ interrupt mask with l= sc=20 disabled
ixgbe_dev_interrupt_get_status(): <><><><&= gt;><>: eicr register: 2100000=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 <---------------------- IXGBE_EICR_= LSC and IXGBE_EICR_GPI_SDP0_X550EM_x
ixgbe_dev_inter= rupt_get_status(): <><><><>><>: set IXGBE_= FLAG_NEED_LINK_UPDATE
ixgbe_dev_interrupt_get_status(): <><>= <><>><>: current flags: flags =3D 5
ixgbe_dev_inter= rupt_action(): <><><><>><>: current flags = (1): flags =3D 5
ixgbe_dev_interrupt_action(): <><><>&= lt;>><>: current flags (2): flags =3D 5
ixgbe_dev_interrupt_= action(): <><><><>><>: current flags (3): = flags =3D 1
ixgbe_dev_interrupt_action(): <><><><&g= t;><>: for dev: 0x557c0cd844c0
ixgbe_dev_interrupt_action(): <><><><>><>: saved intr: mask_original =3D 35651584=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 <------------ original mask = is lost here
ixgbe_dev_interrupt_action(): <><><><&= gt;><>: current intr: mask =3D 35651584

Port 0: link state = change event
ixgbe_dev_interrupt_delayed_handler(): <><><= ><>><>: mask restored: 35651584
testpmd> set link-d= own port 0ixgbe_dev_interrupt_delayed_handler(): <><><>&l= t;>><>: mask restored: 0=C2=A0=C2=A0 <--- all interrupts are= disabled

=C2=A0=C2=A0=C2=A0 3) At this point,= link state can no longer be updated:

testpmd> = show port info 0

********************* Infos for port 0 =C2=A0******= ***************
MAC address: 0C:C4:7A:75:8F:EC
Device name: 0000:03:0= 0.0
Driver name: net_ixgbe
Firmware-version: 0x800001cf
Devargs: <= br>Connect to socket: 0
memory allocation on the socket: 0
Link statu= s: up=C2=A0=C2=A0=C2=A0=C2=A0 <----------------------
Link speed: 10 = Gbps
Link duplex: full-duplex
Autoneg status: On
MTU: 1500

testpmd> set link-down port 0
testpmd> show port= info

********************* Infos for port 0 =C2=A0****************= *****
MAC address: 0C:C4:7A:75:8F:EC
Device name: 0000:03:00.0
Dri= ver name: net_ixgbe
Firmware-version: 0x800001cf
Devargs:
Connect= to socket: 0
memory allocation on the socket: 0
Link status: up=C2= =A0=C2=A0=C2=A0 <------------- should not be up after link down.
Link= speed: 10 Gbps
Link duplex: full-duplex
Autoneg status: On
MTU: 1= 500


--000000000000e1f2e306165f1b6d--