2020-06-09 09:48:26

by Biao Huang (黄彪)

[permalink] [raw]
Subject: [PATCH] net: stmmac: Fix RX Coalesce IOC always true issue

Currently rx_count_frames in stmmac_rx_refill always 0, which leads to
use_rx_wd false, and IOC bit of rx_desc3 true forever. Fix it.

Fixes: 6fa9d691b91ac ("net: stmmac: Prevent divide-by-zero")
Signed-off-by: Biao Huang <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index e6898fd5223f..87b529743fd0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -3607,8 +3607,7 @@ static inline void stmmac_rx_refill(struct stmmac_priv *priv, u32 queue)
stmmac_refill_desc3(priv, rx_q, p);

rx_q->rx_count_frames++;
- rx_q->rx_count_frames += priv->rx_coal_frames;
- if (rx_q->rx_count_frames > priv->rx_coal_frames)
+ if (rx_q->rx_count_frames >= priv->rx_coal_frames)
rx_q->rx_count_frames = 0;

use_rx_wd = !priv->rx_coal_frames;
--
2.18.0


2020-06-09 14:24:15

by Jose Abreu

[permalink] [raw]
Subject: RE: [PATCH] net: stmmac: Fix RX Coalesce IOC always true issue

From: Biao Huang <[email protected]>
Date: Jun/09/2020, 10:41:33 (UTC+00:00)

> - rx_q->rx_count_frames += priv->rx_coal_frames;
> - if (rx_q->rx_count_frames > priv->rx_coal_frames)
> + if (rx_q->rx_count_frames >= priv->rx_coal_frames)

This is no right. If you want to RX IC bit to not always be set you need
to change coalesce parameters using ethtool.

---
Thanks,
Jose Miguel Abreu

2020-06-10 01:35:30

by Biao Huang (黄彪)

[permalink] [raw]
Subject: Re: [PATCH] net: stmmac: Fix RX Coalesce IOC always true issue

On Tue, 2020-06-09 at 14:20 +0000, Jose Abreu wrote:
> From: Biao Huang <[email protected]>
> Date: Jun/09/2020, 10:41:33 (UTC+00:00)
>
> > - rx_q->rx_count_frames += priv->rx_coal_frames;
> > - if (rx_q->rx_count_frames > priv->rx_coal_frames)
> > + if (rx_q->rx_count_frames >= priv->rx_coal_frames)
>
> This is no right. If you want to RX IC bit to not always be set you need
> to change coalesce parameters using ethtool.

let's take look at these lines:

1. rx_q->rx_count_frames += priv->rx_coal_frames;
2. if (rx_q->rx_count_frames > priv->rx_coal_frames)
3. rx_q->rx_count_frames = 0;

before the if condition(line 2 above), there is "rx_q->rx_count_frames
+= priv->rx_coal_frames"(line 1 above) sentence; so the if condition
always true, and the assignment rx_q->rx_count_frames = 0 happens.

Here is the result on our platform, all desc3[30] = 1; you can also
check it on your platform. no matter what rx-frames you set with
"ethtool -C eth0 rx-frames", desc3[30] always true.

# ethtool -c eth0
Coalesce parameters for eth0:
Adaptive RX: off TX: off
stats-block-usecs: 0
sample-interval: 0
pkt-rate-low: 0
pkt-rate-high: 0

rx-usecs: 301
rx-frames: 25
rx-usecs-irq: 0
rx-frames-irq: 0

#cd /sys/kernel/debug/stmmaceth/eth0
# cat descriptors_status
RX Queue 0:
Descriptor ring:
0 [0x5038b000]: 0xa8541a82 0x0 0x0 0xc1000000

1 [0x5038b010]: 0xa8541202 0x0 0x0 0xc1000000

2 [0x5038b020]: 0xa8540982 0x0 0x0 0xc1000000

3 [0x5038b030]: 0xa8540102 0x0 0x0 0xc1000000

4 [0x5038b040]: 0xa658f802 0x0 0x0 0xc1000000

5 [0x5038b050]: 0xa658ef82 0x0 0x0 0xc1000000

6 [0x5038b060]: 0xa658e702 0x0 0x0 0xc1000000

7 [0x5038b070]: 0xa658de82 0x0 0x0 0xc1000000

8 [0x5038b080]: 0xa658d602 0x0 0x0 0xc1000000

9 [0x5038b090]: 0xa658cd82 0x0 0x0 0xc1000000

10 [0x5038b0a0]: 0xa658c502 0x0 0x0 0xc1000000

11 [0x5038b0b0]: 0xa658bc82 0x0 0x0 0xc1000000

12 [0x5038b0c0]: 0xa658b402 0x0 0x0 0xc1000000

13 [0x5038b0d0]: 0xa658ab82 0x0 0x0 0xc1000000

14 [0x5038b0e0]: 0xa658a302 0x0 0x0 0xc1000000

15 [0x5038b0f0]: 0xa6589a82 0x0 0x0 0xc1000000

16 [0x5038b100]: 0xa6589202 0x0 0x0 0xc1000000

17 [0x5038b110]: 0xa6588982 0x0 0x0 0xc1000000

18 [0x5038b120]: 0xa6588102 0x0 0x0 0xc1000000

19 [0x5038b130]: 0xa46df802 0x0 0x0 0xc1000000

20 [0x5038b140]: 0xa46def82 0x0 0x0 0xc1000000

21 [0x5038b150]: 0xa46de702 0x0 0x0 0xc1000000

22 [0x5038b160]: 0xa46dde82 0x0 0x0 0xc1000000

23 [0x5038b170]: 0xa46dd602 0x0 0x0 0xc1000000

24 [0x5038b180]: 0xa46dcd82 0x0 0x0 0xc1000000

25 [0x5038b190]: 0xa46dc502 0x0 0x0 0xc1000000

26 [0x5038b1a0]: 0xa46dbc82 0x0 0x0 0xc1000000

27 [0x5038b1b0]: 0xa46db402 0x0 0x0 0xc1000000

28 [0x5038b1c0]: 0xa46dab82 0x0 0x0 0xc1000000

29 [0x5038b1d0]: 0xa46da302 0x0 0x0 0xc1000000

30 [0x5038b1e0]: 0xa46d9a82 0x0 0x0 0xc1000000
...

> ---
> Thanks,
> Jose Miguel Abreu