Skip to contentSkip to conversation

tg3: Add skb->xmit_more support

PATCH Applied Prashant Sreedharan 5 participants 6 messages Oct 13, 2014 – Oct 14, 2014

Subsystems (3)
Networking Network drivers Broadcom TG3 gigabit Ethernet driver
lore
Share & tools
lore

Prashant Sreedharan

author
#0 lore raw

PATCH 1/1 tg3: Add skb->xmit_more support

Ring TX doorbell only if xmit_more is not set or the queue is stopped.

Suggested-by Daniel Borkmann <[email protected]>
Signed-off-by Prashant Sreedharan <[email protected]>
Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/tg3.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)
drivers/net/ethernet/broadcom/tg3.c +6 -4
index ba49948..dbb41c1 100644
@@ -8099,9 +8099,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Sync BD data before updating mailbox */
 	wmb();
 
-	/* Packets are ready, update Tx producer idx local and on card. */
-	tw32_tx_mbox(tnapi->prodmbox, entry);
-
 	tnapi->tx_prod = entry;
 	if (unlikely(tg3_tx_avail(tnapi) <= (MAX_SKB_FRAGS + 1))) {
 		netif_tx_stop_queue(txq);

@@ -8116,7 +8113,12 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			netif_tx_wake_queue(txq);
 	}
 
-	mmiowb();
+	if (!skb->xmit_more || netif_xmit_stopped(txq)) {
+		/* Packets are ready, update Tx producer idx on card. */
+		tw32_tx_mbox(tnapi->prodmbox, entry);
+		mmiowb();
+	}
+
 	return NETDEV_TX_OK;
 
 dma_error:
Signature · 1 line
1.7.1
#1 lore raw
Quoted 8 lines · Prashant Sreedharan

Ring TX doorbell only if xmit_more is not set or the queue is stopped.

Suggested-by Daniel Borkmann <[email protected]>
Signed-off-by Prashant Sreedharan <[email protected]>
Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/tg3.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

Have you noticed any performance change ?

I did the patch for bnx2x but got no real difference...

Thanks

#2 lore raw
Quoted 10 lines · Eric Dumazet
Quoted 8 lines · Prashant Sreedharan

Ring TX doorbell only if xmit_more is not set or the queue is stopped.

Suggested-by Daniel Borkmann <[email protected]>
Signed-off-by Prashant Sreedharan <[email protected]>
Signed-off-by Michael Chan <[email protected]>
---
  drivers/net/ethernet/broadcom/tg3.c |   10 ++++++----
  1 files changed, 6 insertions(+), 4 deletions(-)
Quoted 3 lines

Have you noticed any performance change ?

I did the patch for bnx2x but got no real difference...

If ringing the doorbell is just a pio write, it may just get lost in the noise. If though the programming model (or some sort of defect workaround) of the NIC requires a pio read of some sort when ringing the doorbell...

rick

#3 lore raw
In reply to Prashant Sreedharan
Quoted 24 lines · Prashant Sreedharan

Ring TX doorbell only if xmit_more is not set or the queue is stopped.

Suggested-by Daniel Borkmann <[email protected]>
Signed-off-by Prashant Sreedharan <[email protected]>
Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/tg3.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)
drivers/net/ethernet/broadcom/tg3.c +0 -3
index ba49948..dbb41c1 100644
@@ -8099,9 +8099,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Sync BD data before updating mailbox */
 	wmb();
 
-	/* Packets are ready, update Tx producer idx local and on card. */
-	tw32_tx_mbox(tnapi->prodmbox, entry);
-
 	tnapi->tx_prod = entry;
 	if (unlikely(tg3_tx_avail(tnapi) <= (MAX_SKB_FRAGS + 1))) {
 		netif_tx_stop_queue(txq);

drivers/net/ethernet/broadcom/tg3.c static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev) · line 8118

Quoted 7 lines
drivers/net/ethernet/broadcom/tg3.c +3 -1
@@ -8116,7 +8113,12 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			netif_tx_wake_queue(txq);
 	}
 
-	mmiowb();
+	if (!skb->xmit_more || netif_xmit_stopped(txq)) {
+		/* Packets are ready, update Tx producer idx on card. */
+		tw32_tx_mbox(tnapi->prodmbox, entry);

I think you need to swap the test, i.e.

netif_xmit_stopped() || xmit_more

Otherwise, hw may not be aware of further packets wainting and queue might not be restarted?

#4 lore raw
Quoted 28 lines · Florian Westphal
Quoted 27 lines · Prashant Sreedharan

Ring TX doorbell only if xmit_more is not set or the queue is stopped.

Suggested-by Daniel Borkmann <[email protected]>
Signed-off-by Prashant Sreedharan <[email protected]>
Signed-off-by Michael Chan <[email protected]>
---
 drivers/net/ethernet/broadcom/tg3.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)
drivers/net/ethernet/broadcom/tg3.c +0 -3
index ba49948..dbb41c1 100644
@@ -8099,9 +8099,6 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 	/* Sync BD data before updating mailbox */
 	wmb();
 
-	/* Packets are ready, update Tx producer idx local and on card. */
-	tw32_tx_mbox(tnapi->prodmbox, entry);
-
 	tnapi->tx_prod = entry;
 	if (unlikely(tg3_tx_avail(tnapi) <= (MAX_SKB_FRAGS + 1))) {
 		netif_tx_stop_queue(txq);

@@ -8116,7 +8113,12 @@ static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev)
 			netif_tx_wake_queue(txq);
 	}
Quoted 6 lines

drivers/net/ethernet/broadcom/tg3.c static netdev_tx_t tg3_start_xmit(struct sk_buff *skb, struct net_device *dev) · line 8118

Quoted 4 lines
drivers/net/ethernet/broadcom/tg3.c +3 -1
-	mmiowb();
+	if (!skb->xmit_more || netif_xmit_stopped(txq)) {
+		/* Packets are ready, update Tx producer idx on card. */
+		tw32_tx_mbox(tnapi->prodmbox, entry);

I think you need to swap the test, i.e.

Never mind, sorry for the noise.