netdev - RE: [net,1/1] net: stmmac: Set OWN bit last in dwmac4_set_rx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID:
 <NTZPR01MB1018A388BD187A1CB38833B3F8BB2@NTZPR01MB1018.CHNPR01.prod.partner.outlook.cn>
Date: Sat, 10 Aug 2024 02:58:50 +0000
From: EnDe Tan <ende.tan@...rfivetech.com>
To: Andrew Lunn <andrew@...n.ch>, Tan En De <endeneer@...il.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"alexandre.torgue@...s.st.com" <alexandre.torgue@...s.st.com>,
	"joabreu@...opsys.com" <joabreu@...opsys.com>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	"mcoquelin.stm32@...il.com" <mcoquelin.stm32@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Leyfoon Tan
	<leyfoon.tan@...rfivetech.com>
Subject: RE: [net,1/1] net: stmmac: Set OWN bit last in dwmac4_set_rx_owner()

 Hi Andrew, thanks for taking a look at my patch.

> Are you seeing things going wrong with real hardware, or is this just code
> review? If this is a real problem, please add a description of what the user
> would see.
>
> Does this need to be backported in stable?

On my FPGA, after running iperf3 for a while, the GMAC somehow got stuck,
as shown by 0.00 bits/sec, for example:
```
iperf3 -t 6000 -c 192.168.xxx.xxx -i 10 -P 2 -l 128
...
[  5] 220.00-230.00 sec  2.04 MBytes  1.71 Mbits/sec    3   1.41 KBytes
[  7] 220.00-230.00 sec  2.04 MBytes  1.71 Mbits/sec    3   1.41 KBytes
[SUM] 220.00-230.00 sec  4.07 MBytes  3.42 Mbits/sec    6
- - - - - - - - - - - - - - - - - - - - - - - - -
[  5] 230.00-240.01 sec  0.00 Bytes  0.00 bits/sec    2   1.41 KBytes
[  7] 230.00-240.01 sec  0.00 Bytes  0.00 bits/sec    2   1.41 KBytes
[SUM] 230.00-240.01 sec  0.00 Bytes  0.00 bits/sec    4
```

Used devmem to check registers:
0x780 (Rx_Packets_Count_Good_Bad)
- The count was incrementing, so packets were still received.
0x114C (DMA_CH0_Current_App_RxDesc)
- Value was changing, so DMA did update the RX descriptor address pointer.
0x1160 (DMA_CH0_Status).
- Receive Buffer Unavailable RBU = 0.
- Receive Interrupt RI = 1 (stuck at 1).

which led me to suspect that there was missed RX interrupt.
I then came across dwmac4_set_rx_owner() function, and saw the possibility of 
missed interrupt (when DMA sees OWN bit before INT_ON_COMPLETION bit).

However, even with this patch, the problem persists on my FPGA.
Therefore, I'd treat this patch as a code review, as I can't provide a concrete proof
that this patch fixes any real hardware.

> Is the problem here that RDES3_INT_ON_COMPLETION_EN is added after the
> RDES3_OWN above has hit the hardware, so it gets ignored?
> 
> It seems like it would be better to calculate the value in a local variable, and
> then assign to p->des3 once.

I didn't use local variable because I worry about CPU out-of-order execution. 
For example,
```
local_var = (INT_ON_COMPLETION | OWN)
des3 |= local_var
```
CPU optimization might result in this
```
des3 |= INT_ON_COMPLETION
des3 |= OWN
```
or worst, out of order like this
```
des3 |= OWN
des3 |= INT_ON_COMPLETION
```
which could cause missing interrupt.

Regards,
Tan En De