Home » Mailing list archives » linux-kernel » 2008 » September » 25

[PATCH] net: remove LLTX in atl2 driver

Previous thread: [GIT]: Networking by David Miller on Wednesday, September 24, 2008 - 4:34 pm. (1 message)

Next thread: [patch] pci: document the pcie_aspm kernel parameter by Chuck Ebbert on Wednesday, September 24, 2008 - 5:40 pm. (2 messages)
[view original message]
From: Kevin Hao
Subject: [PATCH] net: remove LLTX in atl2 driver
Date: Wednesday, September 24, 2008 - 5:35 pm

When NETIF_F_LLTX is set, the atlx driver will use a private lock.
But in recent kernels this implementation seems redundant and
can cause problems where AF_PACKET sees things twice. Since
NETIF_F_LLTX is marked as deprecated and shouldn't be used in
new driver, this patch removes NETIF_F_LLTX and adds a mmiowb
before sending packet.

Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
---
 drivers/net/atlx/atl2.c |   24 +-----------------------
 drivers/net/atlx/atl2.h |    1 -
 2 files changed, 1 insertions(+), 24 deletions(-)

diff --git a/drivers/net/atlx/atl2.c b/drivers/net/atlx/atl2.c
index b2995ac..3df4ee1 100644
--- a/drivers/net/atlx/atl2.c
+++ b/drivers/net/atlx/atl2.c
@@ -116,7 +116,6 @@ static int __devinit atl2_sw_init(struct atl2_adapter *adapter)
 	hw->max_frame_size = adapter->netdev->mtu;
 
 	spin_lock_init(&adapter->stats_lock);
-	spin_lock_init(&adapter->tx_lock);
 
 	set_bit(__ATL2_DOWN, &adapter->flags);
 
@@ -749,11 +748,7 @@ static void atl2_down(struct atl2_adapter *adapter)
 	 * reschedule our watchdog timer */
 	set_bit(__ATL2_DOWN, &adapter->flags);
 
-#ifdef NETIF_F_LLTX
-	netif_stop_queue(netdev);
-#else
 	netif_tx_disable(netdev);
-#endif
 
 	/* reset MAC to disable all RX/TX */
 	atl2_reset_hw(&adapter->hw);
@@ -829,7 +824,6 @@ static inline int TxdFreeBytes(struct atl2_adapter *adapter)
 static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 {
 	struct atl2_adapter *adapter = netdev_priv(netdev);
-	unsigned long flags;
 	struct tx_pkt_header *txph;
 	u32 offset, copy_len;
 	int txs_unused;
@@ -845,16 +839,6 @@ static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 		return NETDEV_TX_OK;
 	}
 
-#ifdef NETIF_F_LLTX
-	local_irq_save(flags);
-	if (!spin_trylock(&adapter->tx_lock)) {
-		/* Collision - tell upper layer to requeue */
-		local_irq_restore(flags);
-		return NETDEV_TX_LOCKED;
-	}
-#else
-	spin_lock_irqsave(&adapter->tx_lock, flags);
-#endif
 	txs_unused = ...
[ message continues ]
[view original message]
From: Chris Snook
Subject: Re: [PATCH] net: remove LLTX in atl2 driver
Date: Thursday, September 25, 2008 - 1:30 pm

Can you explain a bit more concretely the problem this solves, and your testing? 
  Ultimately we'll want to merge this code with the atl1 code, so we need to be 
confident that we can and should make the same change there.


--


[ message continues ]
[view original message]
From: Jeff Garzik
Subject: Re: [PATCH] net: remove LLTX in atl2 driver
Date: Thursday, September 25, 2008 - 1:55 pm

Not directly addressing your question, but LLTX is indeed deprecated and 
in general we want to move away from it.

	Jeff



--


[ message continues ]
[view original message]
From: Chris Snook
Subject: Re: [PATCH] net: remove LLTX in atl2 driver
Date: Thursday, September 25, 2008 - 3:34 pm

I'm all in favor of removing legacy cruft.  I want to know a little more about 
it so we can remove it from the atl1 code as well.  Ultimately most of this will 
be shared code.

-- Chris
--


[ message continues ]
[view original message]
From: Jay Cliburn
Subject: Re: [PATCH] net: remove LLTX in atl2 driver
Date: Thursday, September 25, 2008 - 4:58 pm

On Thu, 25 Sep 2008 18:34:29 -0400

Yes, LLTX can (and should) be removed from the atl1 code.  I had an
exchange with Herbert Xu and David Miller on LKML a few weeks ago, and
they gave me a few tips on how to do it, but I haven't yet had the
time.  What takes an experienced netdev warrior about a half hour to
implement unfortunately takes me days of after-dayjob time as I track
through all the calls that that are affected and test the modified
driver.

Now back to atl2...  I'd like to hear from Kevin also whether he tested
the LLTX removal.  I'm building the driver with his patch as we speak,
and I'll test it myself this weekend, but Kevin I'd still like to know
if you tested the patch.

Thanks,
Jay
--


[ message continues ]
[view original message]
From: Kevin Hao
Subject: Re: [PATCH] net: remove LLTX in atl2 driver
Date: Thursday, September 25, 2008 - 6:20 pm

Yes, I have tested this driver on a Eee PC. It works well.

Thanks,
--


[ message continues ]
[view original message]
From: Chris Snook
Subject: Re: [PATCH] net: remove LLTX in atl2 driver
Date: Thursday, September 25, 2008 - 6:24 pm

Thanks, please mention this with your patch submissions.

-- Chris
--


[ message continues ]
[view original message]
From: Kevin Hao
Subject: [PATCH] net: remove LLTX in atl2 driver
Date: Thursday, September 25, 2008 - 7:20 pm

When NETIF_F_LLTX is set, the atlx driver will use a private lock.
But in recent kernels this implementation seems redundant and
can cause problems where AF_PACKET sees things twice. Since
NETIF_F_LLTX is marked as deprecated and shouldn't be used in
new driver, this patch removes NETIF_F_LLTX and adds a mmiowb
before sending packet. I have tested this driver on a Eee PC.
It works well.

Signed-off-by: Kevin Hao <kexin.hao@windriver.com>
Acked-by: Jay Cliburn <jacliburn@bellsouth.net>
---
 drivers/net/atlx/atl2.c |   24 +-----------------------
 drivers/net/atlx/atl2.h |    1 -
 2 files changed, 1 insertions(+), 24 deletions(-)

diff --git a/drivers/net/atlx/atl2.c b/drivers/net/atlx/atl2.c
index b2995ac..3df4ee1 100644
--- a/drivers/net/atlx/atl2.c
+++ b/drivers/net/atlx/atl2.c
@@ -116,7 +116,6 @@ static int __devinit atl2_sw_init(struct atl2_adapter *adapter)
 	hw->max_frame_size = adapter->netdev->mtu;
 
 	spin_lock_init(&adapter->stats_lock);
-	spin_lock_init(&adapter->tx_lock);
 
 	set_bit(__ATL2_DOWN, &adapter->flags);
 
@@ -749,11 +748,7 @@ static void atl2_down(struct atl2_adapter *adapter)
 	 * reschedule our watchdog timer */
 	set_bit(__ATL2_DOWN, &adapter->flags);
 
-#ifdef NETIF_F_LLTX
-	netif_stop_queue(netdev);
-#else
 	netif_tx_disable(netdev);
-#endif
 
 	/* reset MAC to disable all RX/TX */
 	atl2_reset_hw(&adapter->hw);
@@ -829,7 +824,6 @@ static inline int TxdFreeBytes(struct atl2_adapter *adapter)
 static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 {
 	struct atl2_adapter *adapter = netdev_priv(netdev);
-	unsigned long flags;
 	struct tx_pkt_header *txph;
 	u32 offset, copy_len;
 	int txs_unused;
@@ -845,16 +839,6 @@ static int atl2_xmit_frame(struct sk_buff *skb, struct net_device *netdev)
 		return NETDEV_TX_OK;
 	}
 
-#ifdef NETIF_F_LLTX
-	local_irq_save(flags);
-	if (!spin_trylock(&adapter->tx_lock)) {
-		/* Collision - tell upper layer to requeue */
-		local_irq_restore(flags);
-		return ...
[ message continues ]
[view original message]
From: Jay Cliburn
Subject: Re: [PATCH] net: remove LLTX in atl2 driver
Date: Thursday, September 25, 2008 - 6:24 pm

On Thu, 25 Sep 2008 08:35:22 +0800

Seems to work fine with large file (3.5 GiB) two-way scp and two-way
iperf runs.

[root@osprey ~]# ifconfig eth1
eth1      Link encap:Ethernet  HWaddr 00:13:74:00:5C:38  
          inet addr:192.168.1.33  Bcast:192.168.1.255  Mask:255.255.255.0
          inet6 addr: fe80::213:74ff:fe00:5c38/64 Scope:Link
          UP BROADCAST RUNNING MULTICAST  MTU:1492  Metric:1
          RX packets:10054352 errors:0 dropped:0 overruns:0 frame:1
          TX packets:12364268 errors:0 dropped:0 overruns:0 carrier:6
          collisions:0 txqueuelen:1000 
          RX bytes:11990462653 (11.1 GiB)  TX bytes:12351847745 (11.5 GiB)
          Memory:dbfc0000-dc000000 

--


[ message continues ]
Previous thread: [GIT]: Networking by David Miller on Wednesday, September 24, 2008 - 4:34 pm. (1 message)

Next thread: [patch] pci: document the pcie_aspm kernel parameter by Chuck Ebbert on Wednesday, September 24, 2008 - 5:40 pm. (2 messages)