[PATCH] net: hisilicon: Fix a BUG trigered by wrong bytes_compl

[PATCH] net: hisilicon: Fix a BUG trigered by wrong bytes_compl

[Jiangfeng Xiao]

The trace is as follow:
kernel BUG at lib/dynamic_queue_limits.c:26!
Internal error: Oops - BUG: 0 [#1] SMP ARM
Modules linked in: hip04_eth
CPU: 0 PID: 2003 Comm: tDblStackPcap0 Tainted: G           O L  4.4.197 #1
Hardware name: Hisilicon A15
task: c3637668 task.stack: de3bc000
PC is at dql_completed+0x18/0x154
LR is at hip04_tx_reclaim+0x110/0x174 [hip04_eth]
pc : [<c041abfc>]    lr : [<bf0003a8>]    psr: 800f0313
sp : de3bdc2c  ip : 00000000  fp : c020fb10
r10: 00000000  r9 : c39b4224  r8 : 00000001
r7 : 00000046  r6 : c39b4000  r5 : 0078f392  r4 : 0078f392
r3 : 00000047  r2 : 00000000  r1 : 00000046  r0 : df5d5c80
Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
Control: 32c5387d  Table: 1e189b80  DAC: 55555555
Process tDblStackPcap0 (pid: 2003, stack limit = 0xde3bc190)
Stack: (0xde3bdc2c to 0xde3be000)
[<c041abfc>] (dql_completed) from [<bf0003a8>] (hip04_tx_reclaim+0x110/0x174 [hip04_eth])
[<bf0003a8>] (hip04_tx_reclaim [hip04_eth]) from [<bf0012c0>] (hip04_rx_poll+0x20/0x388 [hip04_eth])
[<bf0012c0>] (hip04_rx_poll [hip04_eth]) from [<c04c8d9c>] (net_rx_action+0x120/0x374)
[<c04c8d9c>] (net_rx_action) from [<c021eaf4>] (__do_softirq+0x218/0x318)
[<c021eaf4>] (__do_softirq) from [<c021eea0>] (irq_exit+0x88/0xac)
[<c021eea0>] (irq_exit) from [<c0240130>] (msa_irq_exit+0x11c/0x1d4)
[<c0240130>] (msa_irq_exit) from [<c0267ba8>] (__handle_domain_irq+0x110/0x148)
[<c0267ba8>] (__handle_domain_irq) from [<c0201588>] (gic_handle_irq+0xd4/0x118)
[<c0201588>] (gic_handle_irq) from [<c0558360>] (__irq_svc+0x40/0x58)
Exception stack(0xde3bdde0 to 0xde3bde28)
dde0: 00000000 00008001 c3637668 00000000 00000000 a00f0213 dd3627a0 c0af6380
de00: c086d380 a00f0213 c0a22a50 de3bde6c 00000002 de3bde30 c0558138 c055813c
de20: 600f0213 ffffffff
[<c0558360>] (__irq_svc) from [<c055813c>] (_raw_spin_unlock_irqrestore+0x44/0x54)
Kernel panic - not syncing: Fatal exception in interrupt

Pre-modification code:
int hip04_mac_start_xmit(struct sk_buff *skb, struct net_device *ndev)
{
[...]
[1]	priv->tx_head = TX_NEXT(tx_head);
[2]	count++;
[3]	netdev_sent_queue(ndev, skb->len);
[...]
}
An rx interrupt occurs if hip04_mac_start_xmit just executes to the line 2,
tx_head has been updated, but corresponding 'skb->len' has not been
added to dql_queue.

And then
hip04_mac_interrupt->__napi_schedule->hip04_rx_poll->hip04_tx_reclaim

In hip04_tx_reclaim, because tx_head has been updated,
bytes_compl will plus an additional "skb-> len"
which has not been added to dql_queue. And then
trigger the BUG_ON(bytes_compl > num_queued - dql->num_completed).

To solve the problem described above, we put
"netdev_sent_queue(ndev, skb->len);"
before
"priv->tx_head = TX_NEXT(tx_head);"

In addition, I adjusted the position of "count++;"
to make the code more readable.

Signed-off-by: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
---
 drivers/net/ethernet/hisilicon/hip04_eth.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
index 3e9b6d5..085476d 100644
--- a/drivers/net/ethernet/hisilicon/hip04_eth.c
+++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
@@ -543,9 +543,8 @@ static void hip04_start_tx_timer(struct hip04_priv *priv)
 	skb_tx_timestamp(skb);
 
 	hip04_set_xmit_desc(priv, phys);
-	priv->tx_head = TX_NEXT(tx_head);
-	count++;
 	netdev_sent_queue(ndev, skb->len);
+	priv->tx_head = TX_NEXT(tx_head);
 
 	stats->tx_bytes += skb->len;
 	stats->tx_packets++;
@@ -553,6 +552,7 @@ static void hip04_start_tx_timer(struct hip04_priv *priv)
 	/* Ensure tx_head update visible to tx reclaim */
 	smp_wmb();
 
+	count++;
 	/* queue is getting full, better start cleaning up now */
 	if (count >= priv->tx_coalesce_frames) {
 		if (napi_schedule_prep(&priv->napi)) {
-- 
1.8.5.6

Re: [PATCH] net: hisilicon: Fix a BUG trigered by wrong bytes_compl

[David Miller]

From: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
Date: Sat, 14 Dec 2019 15:23:02 +0800

> In addition, I adjusted the position of "count++;"
> to make the code more readable.

This is an unrelated cleanup and should not be done in the same change
as your bug fix, submit this if you like for net-next after net has next
been merged into net-next.

> 
> Signed-off-by: Jiangfeng Xiao <xiaojiangfeng@huawei.com>

You need to provide an appropriate Fixes: tag.

Re: [PATCH] net: hisilicon: Fix a BUG trigered by wrong bytes_compl

[Jiangfeng Xiao]

On 2019/12/18 5:42, David Miller wrote:
> From: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
> Date: Sat, 14 Dec 2019 15:23:02 +0800
> 
>> In addition, I adjusted the position of "count++;"
>> to make the code more readable.
> 
> This is an unrelated cleanup and should not be done in the same change
> as your bug fix, submit this if you like for net-next after net has next
> been merged into net-next.
> 
>>
>> Signed-off-by: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
> 
> You need to provide an appropriate Fixes: tag.
> 
> .
I will provide an appropriate Fixes: tag in v2, thanks for your guidance.

Re: [PATCH] net: hisilicon: Fix a BUG trigered by wrong bytes_compl

[Jiangfeng Xiao]

On 2019/12/18 9:40, Jiangfeng Xiao wrote:
> 
> 
> On 2019/12/18 5:42, David Miller wrote:
>> From: Jiangfeng Xiao <xiaojiangfeng@huawei.com>
>> Date: Sat, 14 Dec 2019 15:23:02 +0800
>>
>>> In addition, I adjusted the position of "count++;"
>>> to make the code more readable.
>>
>> This is an unrelated cleanup and should not be done in the same change
>> as your bug fix, submit this if you like for net-next after net has next
>> been merged into net-next.
>>
I'm so sorry. I didn't notice this issue in v2. Please ignore my v2. I will fix this issue in v3.