Skip to contentSkip to conversation

net: atl1c: add BQL support

PATCH Applied Ron Angeles 3 participants 5 messages Aug 12, 2015 – Aug 13, 2015

Subsystems (3)
Networking Network drivers Atlx Ethernet drivers
lore
Share & tools
lore

Ron Angeles

author
#0 lore raw

PATCH 1/1 net: atl1c: add BQL support

This BQL implementation is mostly derived from its related driver, alx. Tested on AR8131 (rev c0) [1969:1063]. Saturated a 100mbps link with 5 concurrent runs of netperf. Ping latency dropped from 14ms to 3ms.

Signed-off-by Ron Angeles <[email protected]>
---
 drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 10 ++++++++++
 1 file changed, 10 insertions(+)
drivers/net/ethernet/atheros/atl1c/atl1c_main.c +10 -0
index 932bd18..2795d6d 100644
@@ -874,6 +874,8 @@ static void atl1c_clean_tx_ring(struct atl1c_adapter *adapter,
 		atl1c_clean_buffer(pdev, buffer_info);
 	}
 
+	netdev_reset_queue(adapter->netdev);
+
 	/* Zero out Tx-buffers */
 	memset(tpd_ring->desc, 0, sizeof(struct atl1c_tpd_desc) *
 		ring_count);

@@ -1551,6 +1553,7 @@ static bool atl1c_clean_tx_irq(struct atl1c_adapter *adapter,
 	u16 next_to_clean = atomic_read(&tpd_ring->next_to_clean);
 	u16 hw_next_to_clean;
 	u16 reg;
+	unsigned int total_bytes = 0, total_packets = 0;
 
 	reg = type == atl1c_trans_high ? REG_TPD_PRI1_CIDX : REG_TPD_PRI0_CIDX;
 

@@ -1558,12 +1561,18 @@ static bool atl1c_clean_tx_irq(struct atl1c_adapter *adapter,
 
 	while (next_to_clean != hw_next_to_clean) {
 		buffer_info = &tpd_ring->buffer_info[next_to_clean];
+		if (buffer_info->skb) {
+			total_bytes += buffer_info->skb->len;
+			total_packets++;
+		}
 		atl1c_clean_buffer(pdev, buffer_info);
 		if (++next_to_clean == tpd_ring->count)
 			next_to_clean = 0;
 		atomic_set(&tpd_ring->next_to_clean, next_to_clean);
 	}
 
+	netdev_completed_queue(adapter->netdev, total_packets, total_bytes);
+
 	if (netif_queue_stopped(adapter->netdev) &&
 			netif_carrier_ok(adapter->netdev)) {
 		netif_wake_queue(adapter->netdev);

@@ -2256,6 +2265,7 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
 		spin_unlock_irqrestore(&adapter->tx_lock, flags);
 		dev_kfree_skb_any(skb);
 	} else {
+		netdev_sent_queue(adapter->netdev, skb->len);
 		atl1c_tx_queue(adapter, skb, tpd, type);
 		spin_unlock_irqrestore(&adapter->tx_lock, flags);
 	}
Signature · 1 line
2.4.6
#1 lore raw

Ron Angeles <[email protected]> :

Quoted 3 lines

This BQL implementation is mostly derived from its related driver, alx. Tested on AR8131 (rev c0) [1969:1063]. Saturated a 100mbps link with 5 concurrent runs of netperf. Ping latency dropped from 14ms to 3ms.

Could you use some time to test the attached experimental stuff as well ?

Signature · 1 line
Ueimor
Quoted 1 line

From 3f5bcfcc93ecc8634e5da95cc58a751e39a1ec9f Mon Sep 17 00:00:00 2001

Message-Id: <3f5bcfcc93ecc8634e5da95cc58a751e39a1ec9f.1439418772.git.romieu@fr.zoreil.com>
From: Francois Romieu <[email protected]>
Date: Sun, 19 Apr 2015 00:23:51 +0200
Subject: [PATCH 1/5] atl1c: use return value where argument pointer makes no
sense.
X-Organisation: Land of Sunshine Inc.

Signed-off-by Francois Romieu <[email protected]>
---
 drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)
drivers/net/ethernet/atheros/atl1c/atl1c_main.c +8 -13
index 932bd18..282ec17 100644
@@ -56,8 +56,7 @@ static int atl1c_stop_mac(struct atl1c_hw *hw);
 static void atl1c_disable_l0s_l1(struct atl1c_hw *hw);
 static void atl1c_set_aspm(struct atl1c_hw *hw, u16 link_speed);
 static void atl1c_start_mac(struct atl1c_adapter *adapter);
-static void atl1c_clean_rx_irq(struct atl1c_adapter *adapter,
-		   int *work_done, int work_to_do);
+static int atl1c_clean_rx_irq(struct atl1c_adapter *adapter, int budget);
 static int atl1c_up(struct atl1c_adapter *adapter);
 static void atl1c_down(struct atl1c_adapter *adapter);
 static int atl1c_reset_mac(struct atl1c_hw *hw);

@@ -1787,11 +1786,10 @@ static void atl1c_clean_rfd(struct atl1c_rfd_ring *rfd_ring,
 	rfd_ring->next_to_clean = rfd_index;
 }
 
-static void atl1c_clean_rx_irq(struct atl1c_adapter *adapter,
-		   int *work_done, int work_to_do)
+static int atl1c_clean_rx_irq(struct atl1c_adapter *adapter, int budget)
 {
 	u16 rfd_num, rfd_index;
-	u16 count = 0;
+	u16 count;
 	u16 length;
 	struct pci_dev *pdev = adapter->pdev;
 	struct net_device *netdev  = adapter->netdev;

@@ -1801,9 +1799,7 @@ static void atl1c_clean_rx_irq(struct atl1c_adapter *adapter,
 	struct atl1c_recv_ret_status *rrs;
 	struct atl1c_buffer *buffer_info;
 
-	while (1) {
-		if (*work_done >= work_to_do)
-			break;
+	for (count = 0; count < budget; count++) {
 		rrs = ATL1C_RRD_DESC(rrd_ring, rrd_ring->next_to_clean);
 		if (likely(RRS_RXD_IS_VALID(rrs->word3))) {
 			rfd_num = (rrs->word0 >> RRS_RX_RFD_CNT_SHIFT) &

@@ -1857,12 +1853,11 @@ rrs_checked:
 			__vlan_hwaccel_put_tag(skb, htons(ETH_P_8021Q), vlan);
 		}
 		netif_receive_skb(skb);
-
-		(*work_done)++;
-		count++;
 	}
 	if (count)
 		atl1c_alloc_rx_buffer(adapter);
+
+	return count;
 }
 
 /**

@@ -1875,10 +1870,10 @@ static int atl1c_clean(struct napi_struct *napi, int budget)
 	int work_done = 0;
 
 	/* Keep link state information with original netdev */
-	if (!netif_carrier_ok(adapter->netdev))
+	if (unlikely(!netif_carrier_ok(adapter->netdev)))
 		goto quit_polling;
 	/* just enable one RXQ */
-	atl1c_clean_rx_irq(adapter, &work_done, budget);
+	work_done = atl1c_clean_rx_irq(adapter, budget);
 
 	if (work_done < budget) {
 quit_polling:
Signature · 1 line
2.4.3
Quoted 1 line

From c1eb6bb4817ed04155e829df435264be675aeab4 Mon Sep 17 00:00:00 2001

Message-Id: <c1eb6bb4817ed04155e829df435264be675aeab4.1439418772.git.romieu@fr.zoreil.com>
In-Reply-To: <3f5bcfcc93ecc8634e5da95cc58a751e39a1ec9f.1439418772.git.romieu@fr.zoreil.com>
References: <3f5bcfcc93ecc8634e5da95cc58a751e39a1ec9f.1439418772.git.romieu@fr.zoreil.com>
From: Francois Romieu <[email protected]>
Date: Sun, 19 Apr 2015 00:45:09 +0200
Subject: [PATCH 2/5] atl1c: highlight normal code path in receive poller
atl1c_clean_rx_irq.
X-Organisation: Land of Sunshine Inc.

Signed-off-by Francois Romieu <[email protected]>
---
 drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 46 +++++++++++--------------
 1 file changed, 20 insertions(+), 26 deletions(-)
drivers/net/ethernet/atheros/atl1c/atl1c_main.c +20 -26

66 lines not on this page · raw message

Signature · 1 line
2.4.3
Quoted 1 line

From 60b6eef77b3256e67c1ba1970df8d8488d76164a Mon Sep 17 00:00:00 2001

Message-Id: <60b6eef77b3256e67c1ba1970df8d8488d76164a.1439418772.git.romieu@fr.zoreil.com>
In-Reply-To: <3f5bcfcc93ecc8634e5da95cc58a751e39a1ec9f.1439418772.git.romieu@fr.zoreil.com>
References: <3f5bcfcc93ecc8634e5da95cc58a751e39a1ec9f.1439418772.git.romieu@fr.zoreil.com>
From: Francois Romieu <[email protected]>
Date: Sat, 2 May 2015 22:33:09 +0200
Subject: [PATCH 3/5] atl1c: remove useless forward declaration and useless
variable.
X-Organisation: Land of Sunshine Inc.

Signed-off-by Francois Romieu <[email protected]>
---
 drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)
drivers/net/ethernet/atheros/atl1c/atl1c_main.c +2 -6
index 35ea3ec..4150118 100644
@@ -56,7 +56,6 @@ static int atl1c_stop_mac(struct atl1c_hw *hw);
 static void atl1c_disable_l0s_l1(struct atl1c_hw *hw);
 static void atl1c_set_aspm(struct atl1c_hw *hw, u16 link_speed);
 static void atl1c_start_mac(struct atl1c_adapter *adapter);
-static int atl1c_clean_rx_irq(struct atl1c_adapter *adapter, int budget);
 static int atl1c_up(struct atl1c_adapter *adapter);
 static void atl1c_down(struct atl1c_adapter *adapter);
 static int atl1c_reset_mac(struct atl1c_hw *hw);

@@ -1689,7 +1688,6 @@ static int atl1c_alloc_rx_buffer(struct atl1c_adapter *adapter)
 	struct pci_dev *pdev = adapter->pdev;
 	struct atl1c_buffer *buffer_info, *next_info;
 	struct sk_buff *skb;
-	void *vir_addr = NULL;
 	u16 num_alloc = 0;
 	u16 rfd_next_to_use, next_next;
 	struct atl1c_rx_free_desc *rfd_desc;

@@ -1716,13 +1714,11 @@ static int atl1c_alloc_rx_buffer(struct atl1c_adapter *adapter)
 		 * this will result in a 16 byte aligned IP header after
 		 * the 14 byte MAC header is removed
 		 */
-		vir_addr = skb->data;
 		ATL1C_SET_BUFFER_STATE(buffer_info, ATL1C_BUFFER_BUSY);
 		buffer_info->skb = skb;
 		buffer_info->length = adapter->rx_buffer_len;
-		mapping = pci_map_single(pdev, vir_addr,
-						buffer_info->length,
-						PCI_DMA_FROMDEVICE);
+		mapping = pci_map_single(pdev, skb->data, buffer_info->length,
+					 PCI_DMA_FROMDEVICE);
 		if (unlikely(pci_dma_mapping_error(pdev, mapping))) {
 			dev_kfree_skb(skb);
 			buffer_info->skb = NULL;
Signature · 1 line
2.4.3
Quoted 1 line

From a4897d6d64d5e880dd3a6f30b17c3cb1c02a244e Mon Sep 17 00:00:00 2001

Message-Id: <a4897d6d64d5e880dd3a6f30b17c3cb1c02a244e.1439418772.git.romieu@fr.zoreil.com>
In-Reply-To: <3f5bcfcc93ecc8634e5da95cc58a751e39a1ec9f.1439418772.git.romieu@fr.zoreil.com>
References: <3f5bcfcc93ecc8634e5da95cc58a751e39a1ec9f.1439418772.git.romieu@fr.zoreil.com>
From: Francois Romieu <[email protected]>
Date: Thu, 13 Aug 2015 00:15:48 +0200
Subject: [PATCH 4/5] atl1c: move tx packet completion to NAPI context.
X-Organisation: Land of Sunshine Inc.

Signed-off-by Francois Romieu <[email protected]>
---
 drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 57 +++++++++++++++++++++----
 1 file changed, 49 insertions(+), 8 deletions(-)
drivers/net/ethernet/atheros/atl1c/atl1c_main.c +49 -8

103 lines not on this page · raw message

Signature · 1 line
2.4.3
Quoted 1 line

From defdd6f990b188a819902d4f3f36b956c2e63d9b Mon Sep 17 00:00:00 2001

Message-Id: <defdd6f990b188a819902d4f3f36b956c2e63d9b.1439418772.git.romieu@fr.zoreil.com>
In-Reply-To: <3f5bcfcc93ecc8634e5da95cc58a751e39a1ec9f.1439418772.git.romieu@fr.zoreil.com>
References: <3f5bcfcc93ecc8634e5da95cc58a751e39a1ec9f.1439418772.git.romieu@fr.zoreil.com>
From: Francois Romieu <[email protected]>
Date: Thu, 13 Aug 2015 00:21:04 +0200
Subject: [PATCH 5/5] atl1c: remove tx_lock.
X-Organisation: Land of Sunshine Inc.

Being only used in atl1c_xmit_frame makes it a good candidate for removal, especially as tx completion now happens in softirq context.

TODO: check the details / elaborate...

Signed-off-by Francois Romieu <[email protected]>
---
 drivers/net/ethernet/atheros/atl1c/atl1c.h      |  3 +--
 drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 11 -----------
 2 files changed, 1 insertion(+), 13 deletions(-)
drivers/net/ethernet/atheros/atl1c/atl1c.h +1 -2
index b9203d9..c46b489 100644
@@ -488,7 +488,7 @@ struct atl1c_tpd_ring {
 	dma_addr_t dma;		/* descriptor ring physical address */
 	u16 size;		/* descriptor ring length in bytes */
 	u16 count;		/* number of descriptors in the ring */
-	u16 next_to_use; 	/* this is protectd by adapter->tx_lock */
+	u16 next_to_use;
 	atomic_t next_to_clean;
 	struct atl1c_buffer *buffer_info;
 };

@@ -542,7 +542,6 @@ struct atl1c_adapter {
 	u16 link_duplex;
 
 	spinlock_t mdio_lock;
-	spinlock_t tx_lock;
 	atomic_t irq_sem;
 
 	struct work_struct common_task;
drivers/net/ethernet/atheros/atl1c/atl1c_main.c +0 -11
index 5624b58..63b65e3 100644
@@ -823,7 +823,6 @@ static int atl1c_sw_init(struct atl1c_adapter *adapter)
 	atl1c_set_rxbufsize(adapter, adapter->netdev);
 	atomic_set(&adapter->irq_sem, 1);
 	spin_lock_init(&adapter->mdio_lock);
-	spin_lock_init(&adapter->tx_lock);
 	set_bit(__AT_DOWN, &adapter->flags);
 
 	return 0;

@@ -2228,7 +2227,6 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
 					  struct net_device *netdev)
 {
 	struct atl1c_adapter *adapter = netdev_priv(netdev);
-	unsigned long flags;
 	u16 tpd_req = 1;
 	struct atl1c_tpd_desc *tpd;
 	enum atl1c_trans_queue type = atl1c_trans_normal;

@@ -2239,16 +2237,10 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
 	}
 
 	tpd_req = atl1c_cal_tpd_req(skb);
-	if (!spin_trylock_irqsave(&adapter->tx_lock, flags)) {
-		if (netif_msg_pktdata(adapter))
-			dev_info(&adapter->pdev->dev, "tx locked\n");
-		return NETDEV_TX_LOCKED;
-	}
 
 	if (atl1c_tpd_avail(adapter, type) < tpd_req) {
 		/* no enough descriptor, just stop queue */
 		netif_stop_queue(netdev);
-		spin_unlock_irqrestore(&adapter->tx_lock, flags);
 		return NETDEV_TX_BUSY;
 	}
 

@@ -2256,7 +2248,6 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
 
 	/* do TSO and check sum */
 	if (atl1c_tso_csum(adapter, skb, &tpd, type) != 0) {
-		spin_unlock_irqrestore(&adapter->tx_lock, flags);
 		dev_kfree_skb_any(skb);
 		return NETDEV_TX_OK;
 	}

@@ -2279,11 +2270,9 @@ static netdev_tx_t atl1c_xmit_frame(struct sk_buff *skb,
 			   "tx-skb droppted due to dma error\n");
 		/* roll back tpd/buffer */
 		atl1c_tx_rollback(adapter, tpd, type);
-		spin_unlock_irqrestore(&adapter->tx_lock, flags);
 		dev_kfree_skb_any(skb);
 	} else {
 		atl1c_tx_queue(adapter, skb, tpd, type);
-		spin_unlock_irqrestore(&adapter->tx_lock, flags);
 	}
 
 	return NETDEV_TX_OK;
Signature · 1 line
2.4.3
author
#2 lore raw
Quoted 7 lines · Francois Romieu

Ron Angeles <[email protected]> :

Quoted 3 lines

This BQL implementation is mostly derived from its related driver, alx. Tested on AR8131 (rev c0) [1969:1063]. Saturated a 100mbps link with 5 concurrent runs of netperf. Ping latency dropped from 14ms to 3ms.

Could you use some time to test the attached experimental stuff as well ?

I only have one of these NICs and it is running on something critical throughout the weekdays. I can try running these when I have time on a weekend. Is there any particular way you would suggest testing? Would an overnight smoke test with netperf suffice?

-Ron

#3 lore raw

Ron Angeles <[email protected]> :
[...]

Quoted 3 lines

I only have one of these NICs and it is running on something critical throughout the weekdays. I can try running these when I have time on a weekend.

Thanks, it would be perfect: I can't promise it won't crash.

Quoted 2 lines

Is there any particular way you would suggest testing? Would an overnight smoke test with netperf suffice?

Anything that changes timing, packet size, and plays with different contexts (link change, retrieve stats, etc.).

I'll send an updated patchkit tomorrow. The xmit part is a bit different from what I'm used to.

Signature · 1 line
Ueimor