On 2013/11/27 12:35, Eric Dumazet wrote:

On Wed, 2013-11-27 at 12:07 +0800, Yang Yingliang wrote:

quoted

Using iperf to send packets(GSO mode is on), a bug is triggered:

quoted

When a skb has frags, bytes_compl plus skb->len nr_frags times in cp_tx().
It's not the correct value(actually, it should plus skb->len once) and it
will trigger the BUG_ON(bytes_compl > num_queued - dql->num_completed).Use
txd->opts1 which stores the real length of a frag in cp_start_xmit() to
calculate bytes_compl.
pkts_compl also has a wrong value, fix it too.

It's introduced by commit 871f0d4c("8139cp: enable bql").

Signed-off-by: Yang Yingliang <redacted>
---
 drivers/net/ethernet/realtek/8139cp.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/realtek/8139cp.c b/drivers/net/ethernet/realtek/8139cp.c
index f2a2128..3b837f0 100644
--- a/drivers/net/ethernet/realtek/8139cp.c
+++ b/drivers/net/ethernet/realtek/8139cp.c

@@ -664,7 +664,7 @@ static void cp_tx (struct cp_private *cp)
 	while (tx_tail != tx_head) {
 		struct cp_desc *txd = cp->tx_ring + tx_tail;
 		struct sk_buff *skb;
-		u32 status;
+		u32 status, flags;
 
 		rmb();
 		status = le32_to_cpu(txd->opts1);

@@ -678,8 +678,10 @@ static void cp_tx (struct cp_private *cp)
 				 le32_to_cpu(txd->opts1) & 0xffff,
 				 PCI_DMA_TODEVICE);
 
-		bytes_compl += skb->len;
-		pkts_compl++;
+		flags = le32_to_cpu(txd->opts1);

Why not using 'status' here, its the same value.

quoted

+		bytes_compl += (flags & 0xffff);
+		if (flags & FirstFrag)
+			pkts_compl++;
 
 		if (status & LastFrag) {
 			if (status & (TxError | TxFIFOUnder)) {

I dont know, this seems a bit convoluted and dangerous, as
cp_start_xmit() did :

netdev_sent_queue(dev, skb->len);

I tried to let pkts_compl equals sum of every frags' length.
Your way looks more simpler. I'll post an updated version of
the patch as you suggested.

Thanks!