* Re: NETDEV WATCHDOG: eth2: transmit timed out with 3c905C-TX
From: Steffen Klassert @ 2006-06-06 19:13 UTC (permalink / raw)
To: Marco Berizzi; +Cc: netdev
In-Reply-To: <BAY103-F31CD06F5FF6568FC37AFEAB2950@phx.gbl>
On Tue, Jun 06, 2006 at 11:12:45AM +0200, Marco Berizzi wrote:
>
> I have moved this damn pc from the remote to my site and I have
> placed it in production environment with 2.6.17-rc5
> No problem after 24 hours (on the remote side the problem was
> arising after a couple of hours). I have modprobed 3c59x with
> debug=4. I see only these kind of messages (are they fine?):
>
> Jun 5 14:31:25 Pleiadi kernel: eth2: vortex_error(), status=0x8081
> Jun 5 14:31:40 Pleiadi last message repeated 3 times
> Jun 5 14:31:47 Pleiadi kernel: eth2: vortex_error(), status=0x8281
> Jun 5 14:31:47 Pleiadi kernel: eth2: Media selection timer tick happened,
> Autonegotiate.
> Jun 5 14:31:47 Pleiadi kernel: dev->watchdog_timeo=1250
> Jun 5 14:31:47 Pleiadi kernel: eth2: MII transceiver has status 782d.
> Jun 5 14:31:47 Pleiadi kernel: eth2: Media selection timer finished,
> Autonegotiate.
> Jun 5 14:31:51 Pleiadi kernel: eth2: vortex_error(), status=0x8081
> Jun 5 14:32:03 Pleiadi last message repeated 2 times
> Jun 5 14:32:10 Pleiadi kernel: eth2: vortex_error(), status=0x8481
> Jun 5 14:32:15 Pleiadi kernel: eth2: vortex_error(), status=0x8081
> Jun 5 14:32:46 Pleiadi last message repeated 7 times
This is ok, just normal operation of the NIC.
>
> The only relevant change, between the remote and my site, is a
> different ethernet switch where the 3c905C is connected to.
> Could it be an issue?
Well, I think it can. Problems with a switch are mostly related
to the autonegotiation of the media type and full/half-duplex.
But in your case the autonegotiation seems to be ok
(mii-tool/ethtool output). More specific information you can
get with the mii-diag and vortex-diag tools. You can find
these tools at http://www.scyld.com/ethercard_diag.html
There are problems with a cisco switch documented in
Documentation/networking/vortex.txt for example.
Steffen
^ permalink raw reply
* TCP Westwood+ patches
From: Stephen Hemminger @ 2006-06-06 18:53 UTC (permalink / raw)
To: Luca De Cicco, David S. Miller; +Cc: netdev, Saverio Mascolo
In-Reply-To: <20060606184152.6d97a193@localhost>
I cleaned these up and put them in a git tree. Luco if you want to be the
maintainer of this, then the correct way is to send a patch to the MAINTAINER
file.
-------
The following changes since commit bd6673d239e7041bc3f81f8d6d0242b7bf6dd62f:
Andreas Schwab:
[CONNECTOR]: Fix warning in cn_queue.c
are found in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/shemminger/tcp-2.6.18.git#tcp
Luca De Cicco:
TCP Westwood: comment fixes
TCP Westwood: bandwidth filter startup
TCP Westwood: reset RTT min after FRTO
Stephen Hemminger:
TCP Westwood: fix first sample
net/ipv4/tcp_westwood.c | 62 ++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 55 insertions(+), 7 deletions(-)
diff --git a/net/ipv4/tcp_westwood.c b/net/ipv4/tcp_westwood.c
index 62a96b7..4247da1 100644
--- a/net/ipv4/tcp_westwood.c
+++ b/net/ipv4/tcp_westwood.c
@@ -1,7 +1,24 @@
/*
- * TCP Westwood+
+ * TCP Westwood+: end-to-end bandwidth estimation for TCP
*
- * Angelo Dell'Aera: TCP Westwood+ support
+ * Angelo Dell'Aera: author of the first version of TCP Westwood+ in Linux 2.4
+ *
+ * Support at http://c3lab.poliba.it/index.php/Westwood
+ * Main references in literature:
+ *
+ * - Mascolo S, Casetti, M. Gerla et al.
+ * "TCP Westwood: bandwidth estimation for TCP" Proc. ACM Mobicom 2001
+ *
+ * - A. Grieco, s. Mascolo
+ * "Performance evaluation of New Reno, Vegas, Westwood+ TCP" ACM Computer
+ * Comm. Review, 2004
+ *
+ * - A. Dell'Aera, L. Grieco, S. Mascolo.
+ * "Linux 2.4 Implementation of Westwood+ TCP with Rate-Halving :
+ * A Performance Evaluation Over the Internet" (ICC 2004), Paris, June 2004
+ *
+ * Westwood+ employs end-to-end bandwidth measurement to set cwnd and
+ * ssthresh after packet loss. The probing phase is as the original Reno.
*/
#include <linux/config.h>
@@ -23,6 +40,7 @@ struct westwood {
u32 rtt;
u32 rtt_min; /* minimum observed RTT */
u8 first_ack; /* flag which infers that this is the first ack */
+ u8 reset_rtt_min; /* Reset RTT min to next RTT sample*/
};
@@ -50,6 +68,7 @@ static void tcp_westwood_init(struct soc
w->bw_est = 0;
w->accounted = 0;
w->cumul_ack = 0;
+ w->reset_rtt_min = 1;
w->rtt_min = w->rtt = TCP_WESTWOOD_INIT_RTT;
w->rtt_win_sx = tcp_time_stamp;
w->snd_una = tcp_sk(sk)->snd_una;
@@ -65,10 +84,16 @@ static inline u32 westwood_do_filter(u32
return (((7 * a) + b) >> 3);
}
-static inline void westwood_filter(struct westwood *w, u32 delta)
+static void westwood_filter(struct westwood *w, u32 delta)
{
- w->bw_ns_est = westwood_do_filter(w->bw_ns_est, w->bk / delta);
- w->bw_est = westwood_do_filter(w->bw_est, w->bw_ns_est);
+ /* If the filter is empty fill it with the first sample of bandwidth */
+ if (w->bw_ns_est == 0 && w->bw_est == 0) {
+ w->bw_ns_est = w->bk / delta;
+ w->bw_est = w->bw_ns_est;
+ } else {
+ w->bw_ns_est = westwood_do_filter(w->bw_ns_est, w->bk / delta);
+ w->bw_est = westwood_do_filter(w->bw_est, w->bw_ns_est);
+ }
}
/*
@@ -93,7 +118,7 @@ static void westwood_update_window(struc
struct westwood *w = inet_csk_ca(sk);
s32 delta = tcp_time_stamp - w->rtt_win_sx;
- /* Initialise w->snd_una with the first acked sequence number in order
+ /* Initialize w->snd_una with the first acked sequence number in order
* to fix mismatch between tp->snd_una and w->snd_una for the first
* bandwidth sample
*/
@@ -119,6 +144,16 @@ static void westwood_update_window(struc
}
}
+static inline void update_rtt_min(struct westwood *w)
+{
+ if (w->reset_rtt_min) {
+ w->rtt_min = w->rtt;
+ w->reset_rtt_min = 0;
+ } else
+ w->rtt_min = min(w->rtt, w->rtt_min);
+}
+
+
/*
* @westwood_fast_bw
* It is called when we are in fast path. In particular it is called when
@@ -134,7 +169,7 @@ static inline void westwood_fast_bw(stru
w->bk += tp->snd_una - w->snd_una;
w->snd_una = tp->snd_una;
- w->rtt_min = min(w->rtt, w->rtt_min);
+ update_rtt_min(w);
}
/*
@@ -191,7 +226,7 @@ static void tcp_westwood_event(struct so
{
struct tcp_sock *tp = tcp_sk(sk);
struct westwood *w = inet_csk_ca(sk);
-
+
switch(event) {
case CA_EVENT_FAST_ACK:
westwood_fast_bw(sk);
@@ -203,12 +238,14 @@ static void tcp_westwood_event(struct so
case CA_EVENT_FRTO:
tp->snd_ssthresh = tcp_westwood_bw_rttmin(sk);
+ /* Update RTT_min when next ack arrives */
+ w->reset_rtt_min = 1;
break;
case CA_EVENT_SLOW_ACK:
westwood_update_window(sk);
w->bk += westwood_acked_count(sk);
- w->rtt_min = min(w->rtt, w->rtt_min);
+ update_rtt_min(w);
break;
default:
^ permalink raw reply related
* [ PATCH 2.6.17-rc6 1/1] udp.c: counting InDatagrams which are never delivered
From: Gerrit Renker @ 2006-06-06 18:25 UTC (permalink / raw)
To: netdev; +Cc: kaber, jmorris
This problem involves MIB counter inaccuracies triggered by
failed UDP checksums.
Problem: ip_local_deliver_finish calls udp_rcv, which calls
udp_queue_rcv_skb.
Unless the sk_filter is set, the checksum of the incoming
UDP datagram is not verified. If there are no other problems
InDatagrams (UDP_MIB_INDATAGRAMS) is then incremented.
Now, if udp_recvmsg is called as a handler for incoming
UDP datagrams, the checksum is verified for the first time
(unless sk_filter was set) and if the checksum fails, the
`goto csum_copy_err' leads to forcibly removing the datagram
_and_ incrementing InErrors (UDP_MIB_INERRORS).
Issue: When problem occurs in the manner described, the datagram
is counted twice: once as InDatagram and once as InErrors.
RFC 2013 defines InDatagrams as counter for delivered datagrams;
these datagrams are counted but never delivered.
How to reproduce: Send UDP datagrams with checksums enabled, use middlebox
which corrupts part of the traffic (e.g. bit errors / NetEm) and
use /proc/net/snmp to watch the counters. The sum of InErrors,
NoPorts and InDatagrams exceeds the real number of sent datagrams
by the number of datagrams which were counted twice and forcibly
removed by udp_recvmsg.
Non-occurrence: The problem does not occur if the sender disabled
UDP checksums (zero field; allowed for IPv4, but not for IPv6),
since then the checksum code returns success.
Fix: Move the `UDP_INC_STATS_BH(UDP_MIB_INDATAGRAMS)' statement from
udp_queue_rcv_skb to udp_recvmsg. Now InDatagrams only counts those
datagrams which were really delivered (as per RFC 2013).
Please CC: any correspondence to gerrit@erg.abdn.ac.uk
Signed-off-by: <gerrit@erg.abdn.ac.uk>
---
diff -Nurp a/net/ipv4/udp.c b/net/ipv4/udp.c
--- a/net/ipv4/udp.c 2006-06-06 17:01:26.000000000 +0100
+++ b/net/ipv4/udp.c 2006-06-06 23:39:45.000000000 +0100
@@ -823,6 +823,7 @@ try_again:
goto out_free;
sock_recv_timestamp(msg, sk, skb);
+ UDP_INC_STATS_BH(UDP_MIB_INDATAGRAMS);
/* Copy the address. */
if (sin)
@@ -1032,7 +1033,6 @@ static int udp_queue_rcv_skb(struct sock
kfree_skb(skb);
return -1;
}
- UDP_INC_STATS_BH(UDP_MIB_INDATAGRAMS);
return 0;
}
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Jeff Moyer @ 2006-06-06 17:42 UTC (permalink / raw)
To: Auke Kok
Cc: Garzik, Jeff, Neil Horman, Mitch Williams, netdev,
Brandeburg, Jesse, Kok, Auke, mpm
In-Reply-To: <4485BCA2.5070904@intel.com>
==> Regarding Re: [PATCH 1/2] e1000: fix netpoll with NAPI; Auke Kok <auke-jan.h.kok@intel.com> adds:
auke-jan.h.kok> Jeff Moyer wrote:
>> ==> Regarding Re: [PATCH 1/2] e1000: fix netpoll with NAPI; Auke Kok <auke-jan.h.kok@intel.com> adds:
auke-jan.h.kok> Neil Horman wrote:
>>>> On Tue, Jun 06, 2006 at 09:39:25AM -0700, Mitch Williams wrote:
>>>>> On Tue, 2006-06-06 at 09:52 -0400, Neil Horman wrote:
auke-jan.h.kok> [snip]
>>
>>>>> However, just for the sake of correctness (and paranoia), I'll whip up
>>>>> another patch that does this check.
>>>>>
>>>> Thanks for the quick feedback!
>>>> Regards
>>>> Neil
>>>>
>>>>> Jeff, please do not commit this patch.
auke-jan.h.kok> Jeff,
auke-jan.h.kok> I've popped the patch off from our gitserver, so you can
>> pull the two
auke-jan.h.kok> outstanding patches while we revamp this one.
>> Would you please send patches to this list? I'd certainly like to review
>> them. I don't think the problem needs solving in the e1000 driver. I
>> think it is an issue that should be handled properly by netpoll.
auke-jan.h.kok> ???
auke-jan.h.kok> that message was directed to Jeff Garzik, perhaps that was too confusing.
I figured it was directed at Jeff G.
auke-jan.h.kok> They were sent here in the first place:
auke-jan.h.kok> http://marc.theaimsgroup.com/?l=linux-netdev&m=114954878711789&w=2
Thanks for the pointer.
As you noted, e1000 now uses a separate device for polling. Netpoll should
be able to deal with this, though. I'd like to solicit mpm's input on
this, as I'm having difficulties coming up with a clean solution.
For some background, the netpoll_poll_lock calls were introduced to prevent
recursion in a device driver's ->poll routine. By essentially calling the
poll routine from the poll_controller routine, you are no longer prevented
from such recursion.
It would be best if the poll_controller routine was kept simple. It should
do the equivalent of the interrupt processing portion of the work, and
leave the delivery to the network stack for a call to the ->poll routine.
Solving this at the netpoll layer will be a bit difficult, since we have no
insight into the driver design (as your driver illustrates). Up until now,
our model worked well.
We could, potentially, walk the list of devices scheduled for a poll much
the same as net_rx_action does. However, we don't want to process work
pending on other network adapters, only the one associated with the netpoll
net device. I can think of at least one way to make this distinction, but
it feels too much like a hack.
Matt, any ideas on this?
-Jeff
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Auke Kok @ 2006-06-06 17:34 UTC (permalink / raw)
To: Jeff Moyer
Cc: Garzik, Jeff, Neil Horman, Mitch Williams, netdev,
Brandeburg, Jesse, Kok, Auke, mpm
In-Reply-To: <x494pyyw5sr.fsf@segfault.boston.redhat.com>
Jeff Moyer wrote:
> ==> Regarding Re: [PATCH 1/2] e1000: fix netpoll with NAPI; Auke Kok <auke-jan.h.kok@intel.com> adds:
>
> auke-jan.h.kok> Neil Horman wrote:
>>> On Tue, Jun 06, 2006 at 09:39:25AM -0700, Mitch Williams wrote:
>>>> On Tue, 2006-06-06 at 09:52 -0400, Neil Horman wrote:
>
>
> auke-jan.h.kok> [snip]
>
>
>>>> However, just for the sake of correctness (and paranoia), I'll whip up
>>>> another patch that does this check.
>>>>
>>> Thanks for the quick feedback!
>>> Regards
>>> Neil
>>>
>>>> Jeff, please do not commit this patch.
>
> auke-jan.h.kok> Jeff,
>
> auke-jan.h.kok> I've popped the patch off from our gitserver, so you can pull the two
> auke-jan.h.kok> outstanding patches while we revamp this one.
>
> Would you please send patches to this list? I'd certainly like to review
> them. I don't think the problem needs solving in the e1000 driver. I
> think it is an issue that should be handled properly by netpoll.
???
that message was directed to Jeff Garzik, perhaps that was too confusing.
They were sent here in the first place:
http://marc.theaimsgroup.com/?l=linux-netdev&m=114954878711789&w=2
Cheers,
Auke
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Jeff Moyer @ 2006-06-06 17:30 UTC (permalink / raw)
To: Auke Kok
Cc: Garzik, Jeff, Neil Horman, Mitch Williams, netdev,
Brandeburg, Jesse, Kok, Auke, mpm
In-Reply-To: <4485B8EC.4090603@intel.com>
==> Regarding Re: [PATCH 1/2] e1000: fix netpoll with NAPI; Auke Kok <auke-jan.h.kok@intel.com> adds:
auke-jan.h.kok> Neil Horman wrote:
>> On Tue, Jun 06, 2006 at 09:39:25AM -0700, Mitch Williams wrote:
>>> On Tue, 2006-06-06 at 09:52 -0400, Neil Horman wrote:
auke-jan.h.kok> [snip]
>>> However, just for the sake of correctness (and paranoia), I'll whip up
>>> another patch that does this check.
>>>
>> Thanks for the quick feedback!
>> Regards
>> Neil
>>
>>> Jeff, please do not commit this patch.
auke-jan.h.kok> Jeff,
auke-jan.h.kok> I've popped the patch off from our gitserver, so you can pull the two
auke-jan.h.kok> outstanding patches while we revamp this one.
Would you please send patches to this list? I'd certainly like to review
them. I don't think the problem needs solving in the e1000 driver. I
think it is an issue that should be handled properly by netpoll.
-Jeff
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Jeff Moyer @ 2006-06-06 17:29 UTC (permalink / raw)
To: Mitch Williams
Cc: Neil Horman, Kok, Auke, Garzik, Jeff, netdev, Brandeburg, Jesse,
Kok, Auke, mpm
In-Reply-To: <1149611965.13635.19.camel@strongmad>
==> Regarding Re: [PATCH 1/2] e1000: fix netpoll with NAPI; Mitch Williams <mitch.a.williams@intel.com> adds:
mitch> On Tue, 2006-06-06 at 09:52 -0400, Neil Horman wrote:
>> I've been speaking about this fix with a Jeff Moyer, and we've come up with some
>> concerns regarding its implementation. Specifically the call to
adapter-> clean_rx in the case of the e1000 driver is rather a layering
>> violation in the netpoll code, in the sense that the function pointed to by clean_rx
>> is functionality that is nominally used by the dev->poll method. In fact in
>> this case, it would appear possible since dev->poll is called under the
>> poll_lock, but dev->poll_controller is not, that is is possible to have cpus in
>> a system executing in e1000_clean_rx_irq_[ps] at the same time leading to data
>> corruption:
>>
>> CPU0:
>> netpoll_poll_dev
dev-> poll_controller (e1000_netpoll)
adapter-> clean_rx (e1000_clean_rx_irq)
>>
>> CPU1:
>> napi_poll
dev-> poll (e1000_clean)
>> e1000_clean_rx_irq
mitch> Hmmm. You may have a point. I don't think a spinlock is required, but
mitch> we do need to check if the poll is already scheduled on another CPU,
mitch> like netpoll does in poll_napi().
mitch> In practice, of course, we never see this. The only netpoll client in
mitch> the kernel is netconsole, which doesn't do receives. A few Major
mitch> Distros use netdump, which does do receives, but only after the system
mitch> has crashed. In that case, all other CPUs are stopped anyway.
Don't forget about driver/net/kgdb_eth.c, which is in-tree, and does sends
and receives.
-Jeff
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Auke Kok @ 2006-06-06 17:18 UTC (permalink / raw)
To: Garzik, Jeff
Cc: Neil Horman, Mitch Williams, netdev, Brandeburg, Jesse, Kok, Auke,
jmoyer, mpm
In-Reply-To: <20060606170513.GB21969@hmsreliant.homelinux.net>
Neil Horman wrote:
> On Tue, Jun 06, 2006 at 09:39:25AM -0700, Mitch Williams wrote:
>> On Tue, 2006-06-06 at 09:52 -0400, Neil Horman wrote:
[snip]
>> However, just for the sake of correctness (and paranoia), I'll whip up
>> another patch that does this check.
>>
> Thanks for the quick feedback!
>
> Regards
> Neil
>
>> Jeff, please do not commit this patch.
Jeff,
I've popped the patch off from our gitserver, so you can pull the two
outstanding patches while we revamp this one.
Auke
^ permalink raw reply
* [PATCH 5/5] skge: version 1.6
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20060606171110.592587000@zqx3.pdx.osdl.net>
[-- Attachment #1: skge-1.6.patch --]
[-- Type: text/plain, Size: 338 bytes --]
Update version string.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- skge-2.6.orig/drivers/net/skge.c
+++ skge-2.6/drivers/net/skge.c
@@ -44,7 +44,7 @@
#include "skge.h"
#define DRV_NAME "skge"
-#define DRV_VERSION "1.5"
+#define DRV_VERSION "1.6"
#define PFX DRV_NAME " "
#define DEFAULT_TX_RING_SIZE 128
--
^ permalink raw reply
* [PATCH 3/5] skge: transmit complete via IRQ not NAPI
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20060606171110.592587000@zqx3.pdx.osdl.net>
[-- Attachment #1: skge-txirq.patch --]
[-- Type: text/plain, Size: 9856 bytes --]
The transmit side code has a number of ring problems that caused some
of the Bugzilla reports. Rather than trying to fix the details, it is safer
to rewrite the code that handles transmit completion and freeing.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- skge-2.6.orig/drivers/net/skge.c
+++ skge-2.6/drivers/net/skge.c
@@ -2303,21 +2303,20 @@ static int skge_xmit_frame(struct sk_buf
{
struct skge_port *skge = netdev_priv(dev);
struct skge_hw *hw = skge->hw;
- struct skge_ring *ring = &skge->tx_ring;
struct skge_element *e;
struct skge_tx_desc *td;
int i;
u32 control, len;
u64 map;
+ unsigned long flags;
skb = skb_padto(skb, ETH_ZLEN);
if (!skb)
return NETDEV_TX_OK;
- if (!spin_trylock(&skge->tx_lock)) {
+ if (!spin_trylock_irqsave(&skge->tx_lock, flags))
/* Collision - tell upper layer to requeue */
return NETDEV_TX_LOCKED;
- }
if (unlikely(skge_avail(&skge->tx_ring) < skb_shinfo(skb)->nr_frags + 1)) {
if (!netif_queue_stopped(dev)) {
@@ -2326,12 +2325,13 @@ static int skge_xmit_frame(struct sk_buf
printk(KERN_WARNING PFX "%s: ring full when queue awake!\n",
dev->name);
}
- spin_unlock(&skge->tx_lock);
+ spin_unlock_irqrestore(&skge->tx_lock, flags);
return NETDEV_TX_BUSY;
}
- e = ring->to_use;
+ e = skge->tx_ring.to_use;
td = e->desc;
+ BUG_ON(td->control & BMU_OWN);
e->skb = skb;
len = skb_headlen(skb);
map = pci_map_single(hw->pdev, skb->data, len, PCI_DMA_TODEVICE);
@@ -2372,8 +2372,10 @@ static int skge_xmit_frame(struct sk_buf
frag->size, PCI_DMA_TODEVICE);
e = e->next;
- e->skb = NULL;
+ e->skb = skb;
tf = e->desc;
+ BUG_ON(tf->control & BMU_OWN);
+
tf->dma_lo = map;
tf->dma_hi = (u64) map >> 32;
pci_unmap_addr_set(e, mapaddr, map);
@@ -2390,56 +2392,68 @@ static int skge_xmit_frame(struct sk_buf
skge_write8(hw, Q_ADDR(txqaddr[skge->port], Q_CSR), CSR_START);
- if (netif_msg_tx_queued(skge))
+ if (unlikely(netif_msg_tx_queued(skge)))
printk(KERN_DEBUG "%s: tx queued, slot %td, len %d\n",
- dev->name, e - ring->start, skb->len);
+ dev->name, e - skge->tx_ring.start, skb->len);
- ring->to_use = e->next;
+ skge->tx_ring.to_use = e->next;
if (skge_avail(&skge->tx_ring) <= TX_LOW_WATER) {
pr_debug("%s: transmit queue full\n", dev->name);
netif_stop_queue(dev);
}
- mmiowb();
- spin_unlock(&skge->tx_lock);
+ spin_unlock_irqrestore(&skge->tx_lock, flags);
dev->trans_start = jiffies;
return NETDEV_TX_OK;
}
-static void skge_tx_complete(struct skge_port *skge, struct skge_element *last)
+
+/* Free resources associated with this reing element */
+static void skge_tx_free(struct skge_port *skge, struct skge_element *e,
+ u32 control)
{
struct pci_dev *pdev = skge->hw->pdev;
- struct skge_element *e;
- for (e = skge->tx_ring.to_clean; e != last; e = e->next) {
- struct sk_buff *skb = e->skb;
- int i;
+ BUG_ON(!e->skb);
- e->skb = NULL;
+ /* skb header vs. fragment */
+ if (control & BMU_STF)
pci_unmap_single(pdev, pci_unmap_addr(e, mapaddr),
- skb_headlen(skb), PCI_DMA_TODEVICE);
+ pci_unmap_len(e, maplen),
+ PCI_DMA_TODEVICE);
+ else
+ pci_unmap_page(pdev, pci_unmap_addr(e, mapaddr),
+ pci_unmap_len(e, maplen),
+ PCI_DMA_TODEVICE);
- for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
- e = e->next;
- pci_unmap_page(pdev, pci_unmap_addr(e, mapaddr),
- skb_shinfo(skb)->frags[i].size,
- PCI_DMA_TODEVICE);
- }
+ if (control & BMU_EOF) {
+ if (unlikely(netif_msg_tx_done(skge)))
+ printk(KERN_DEBUG PFX "%s: tx done slot %td\n",
+ skge->netdev->name, e - skge->tx_ring.start);
- dev_kfree_skb(skb);
+ dev_kfree_skb_any(e->skb);
}
- skge->tx_ring.to_clean = e;
+ e->skb = NULL;
}
+/* Free all buffers in transmit ring */
static void skge_tx_clean(struct skge_port *skge)
{
+ struct skge_element *e;
+ unsigned long flags;
- spin_lock_bh(&skge->tx_lock);
- skge_tx_complete(skge, skge->tx_ring.to_use);
+ spin_lock_irqsave(&skge->tx_lock, flags);
+ for (e = skge->tx_ring.to_clean; e != skge->tx_ring.to_use; e = e->next) {
+ struct skge_tx_desc *td = e->desc;
+ skge_tx_free(skge, e, td->control);
+ td->control = 0;
+ }
+
+ skge->tx_ring.to_clean = e;
netif_wake_queue(skge->netdev);
- spin_unlock_bh(&skge->tx_lock);
+ spin_unlock_irqrestore(&skge->tx_lock, flags);
}
static void skge_tx_timeout(struct net_device *dev)
@@ -2665,32 +2679,28 @@ resubmit:
return NULL;
}
-static void skge_tx_done(struct skge_port *skge)
+/* Free all buffers in Tx ring which are no longer owned by device */
+static void skge_txirq(struct net_device *dev)
{
+ struct skge_port *skge = netdev_priv(dev);
struct skge_ring *ring = &skge->tx_ring;
- struct skge_element *e, *last;
+ struct skge_element *e;
+
+ rmb();
spin_lock(&skge->tx_lock);
- last = ring->to_clean;
for (e = ring->to_clean; e != ring->to_use; e = e->next) {
struct skge_tx_desc *td = e->desc;
if (td->control & BMU_OWN)
break;
- if (td->control & BMU_EOF) {
- last = e->next;
- if (unlikely(netif_msg_tx_done(skge)))
- printk(KERN_DEBUG PFX "%s: tx done slot %td\n",
- skge->netdev->name, e - ring->start);
- }
+ skge_tx_free(skge, e, td->control);
}
+ skge->tx_ring.to_clean = e;
- skge_tx_complete(skge, last);
-
- skge_write8(skge->hw, Q_ADDR(txqaddr[skge->port], Q_CSR), CSR_IRQ_CL_F);
-
- if (skge_avail(&skge->tx_ring) > TX_LOW_WATER)
+ if (netif_queue_stopped(skge->netdev)
+ && skge_avail(&skge->tx_ring) > TX_LOW_WATER)
netif_wake_queue(skge->netdev);
spin_unlock(&skge->tx_lock);
@@ -2705,8 +2715,6 @@ static int skge_poll(struct net_device *
int to_do = min(dev->quota, *budget);
int work_done = 0;
- skge_tx_done(skge);
-
for (e = ring->to_clean; prefetch(e->next), work_done < to_do; e = e->next) {
struct skge_rx_desc *rd = e->desc;
struct sk_buff *skb;
@@ -2738,10 +2746,12 @@ static int skge_poll(struct net_device *
return 1; /* not done */
netif_rx_complete(dev);
- mmiowb();
- hw->intr_mask |= skge->port == 0 ? (IS_R1_F|IS_XA1_F) : (IS_R2_F|IS_XA2_F);
+ spin_lock_irq(&hw->hw_lock);
+ hw->intr_mask |= rxirqmask[skge->port];
skge_write32(hw, B0_IMSK, hw->intr_mask);
+ mmiowb();
+ spin_unlock_irq(&hw->hw_lock);
return 0;
}
@@ -2871,8 +2881,10 @@ static void skge_extirq(void *arg)
}
mutex_unlock(&hw->phy_mutex);
+ spin_lock_irq(&hw->hw_lock);
hw->intr_mask |= IS_EXT_REG;
skge_write32(hw, B0_IMSK, hw->intr_mask);
+ spin_unlock_irq(&hw->hw_lock);
}
static irqreturn_t skge_intr(int irq, void *dev_id, struct pt_regs *regs)
@@ -2885,54 +2897,68 @@ static irqreturn_t skge_intr(int irq, vo
if (status == 0)
return IRQ_NONE;
+ spin_lock(&hw->hw_lock);
+ status &= hw->intr_mask;
if (status & IS_EXT_REG) {
hw->intr_mask &= ~IS_EXT_REG;
schedule_work(&hw->phy_work);
}
- if (status & (IS_R1_F|IS_XA1_F)) {
- skge_write8(hw, Q_ADDR(Q_R1, Q_CSR), CSR_IRQ_CL_F);
- hw->intr_mask &= ~(IS_R1_F|IS_XA1_F);
- netif_rx_schedule(hw->dev[0]);
+ if (status & IS_XA1_F) {
+ skge_write8(hw, Q_ADDR(Q_XA1, Q_CSR), CSR_IRQ_CL_F);
+ skge_txirq(hw->dev[0]);
}
- if (status & (IS_R2_F|IS_XA2_F)) {
- skge_write8(hw, Q_ADDR(Q_R2, Q_CSR), CSR_IRQ_CL_F);
- hw->intr_mask &= ~(IS_R2_F|IS_XA2_F);
- netif_rx_schedule(hw->dev[1]);
+ if (status & IS_R1_F) {
+ skge_write8(hw, Q_ADDR(Q_R1, Q_CSR), CSR_IRQ_CL_F);
+ hw->intr_mask &= ~IS_R1_F;
+ netif_rx_schedule(hw->dev[0]);
}
- if (likely((status & hw->intr_mask) == 0))
- return IRQ_HANDLED;
+ if (status & IS_PA_TO_TX1)
+ skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_TX1);
if (status & IS_PA_TO_RX1) {
struct skge_port *skge = netdev_priv(hw->dev[0]);
+
++skge->net_stats.rx_over_errors;
skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_RX1);
}
- if (status & IS_PA_TO_RX2) {
- struct skge_port *skge = netdev_priv(hw->dev[1]);
- ++skge->net_stats.rx_over_errors;
- skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_RX2);
- }
-
- if (status & IS_PA_TO_TX1)
- skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_TX1);
-
- if (status & IS_PA_TO_TX2)
- skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_TX2);
if (status & IS_MAC1)
skge_mac_intr(hw, 0);
- if (status & IS_MAC2)
- skge_mac_intr(hw, 1);
+ if (hw->dev[1]) {
+ if (status & IS_XA2_F) {
+ skge_write8(hw, Q_ADDR(Q_XA2, Q_CSR), CSR_IRQ_CL_F);
+ skge_txirq(hw->dev[1]);
+ }
+
+ if (status & IS_R2_F) {
+ skge_write8(hw, Q_ADDR(Q_R2, Q_CSR), CSR_IRQ_CL_F);
+ hw->intr_mask &= ~IS_R2_F;
+ netif_rx_schedule(hw->dev[1]);
+ }
+
+ if (status & IS_PA_TO_RX2) {
+ struct skge_port *skge = netdev_priv(hw->dev[1]);
+ ++skge->net_stats.rx_over_errors;
+ skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_RX2);
+ }
+
+ if (status & IS_PA_TO_TX2)
+ skge_write16(hw, B3_PA_CTRL, PA_CLR_TO_TX2);
+
+ if (status & IS_MAC2)
+ skge_mac_intr(hw, 1);
+ }
if (status & IS_HW_ERR)
skge_error_irq(hw);
skge_write32(hw, B0_IMSK, hw->intr_mask);
+ spin_unlock(&hw->hw_lock);
return IRQ_HANDLED;
}
@@ -3083,6 +3109,7 @@ static int skge_reset(struct skge_hw *hw
else
hw->ram_size = t8 * 4096;
+ spin_lock_init(&hw->hw_lock);
hw->intr_mask = IS_HW_ERR | IS_EXT_REG | IS_PORT_1;
if (hw->ports > 1)
hw->intr_mask |= IS_PORT_2;
@@ -3389,7 +3416,11 @@ static void __devexit skge_remove(struct
dev0 = hw->dev[0];
unregister_netdev(dev0);
+ spin_lock_irq(&hw->hw_lock);
+ hw->intr_mask = 0;
skge_write32(hw, B0_IMSK, 0);
+ spin_unlock_irq(&hw->hw_lock);
+
skge_write16(hw, B0_LED, LED_STAT_OFF);
skge_write8(hw, B0_CTST, CS_RST_SET);
--- skge-2.6.orig/drivers/net/skge.h
+++ skge-2.6/drivers/net/skge.h
@@ -2388,6 +2388,7 @@ struct skge_ring {
struct skge_hw {
void __iomem *regs;
struct pci_dev *pdev;
+ spinlock_t hw_lock;
u32 intr_mask;
struct net_device *dev[2];
--
^ permalink raw reply
* [PATCH 2/5] skge: TX low water mark definition
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20060606171110.592587000@zqx3.pdx.osdl.net>
[-- Attachment #1: skge-lowater.patch --]
[-- Type: text/plain, Size: 1430 bytes --]
Consolidate all usage of ring low water mark to one value.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- skge-2.6.orig/drivers/net/skge.c
+++ skge-2.6/drivers/net/skge.c
@@ -50,6 +50,7 @@
#define DEFAULT_TX_RING_SIZE 128
#define DEFAULT_RX_RING_SIZE 512
#define MAX_TX_RING_SIZE 1024
+#define TX_LOW_WATER (MAX_SKB_FRAGS + 1)
#define MAX_RX_RING_SIZE 4096
#define RX_COPY_THRESHOLD 128
#define RX_BUF_SIZE 1536
@@ -401,7 +402,7 @@ static int skge_set_ring_param(struct ne
int err;
if (p->rx_pending == 0 || p->rx_pending > MAX_RX_RING_SIZE ||
- p->tx_pending < MAX_SKB_FRAGS+1 || p->tx_pending > MAX_TX_RING_SIZE)
+ p->tx_pending < TX_LOW_WATER || p->tx_pending > MAX_TX_RING_SIZE)
return -EINVAL;
skge->rx_ring.count = p->rx_pending;
@@ -2394,7 +2395,7 @@ static int skge_xmit_frame(struct sk_buf
dev->name, e - ring->start, skb->len);
ring->to_use = e->next;
- if (skge_avail(&skge->tx_ring) <= MAX_SKB_FRAGS + 1) {
+ if (skge_avail(&skge->tx_ring) <= TX_LOW_WATER) {
pr_debug("%s: transmit queue full\n", dev->name);
netif_stop_queue(dev);
}
@@ -2689,7 +2690,7 @@ static void skge_tx_done(struct skge_por
skge_write8(skge->hw, Q_ADDR(txqaddr[skge->port], Q_CSR), CSR_IRQ_CL_F);
- if (skge_avail(&skge->tx_ring) > MAX_SKB_FRAGS + 1)
+ if (skge_avail(&skge->tx_ring) > TX_LOW_WATER)
netif_wake_queue(skge->netdev);
spin_unlock(&skge->tx_lock);
--
^ permalink raw reply
* [PATCH 0/5] skge: new version 1.6
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
Go back to handling transmit completion via IRQ not NAPI. This works
better and is cleaner. Handle PHY via work queue, not tasklet.
--
^ permalink raw reply
* [PATCH 4/5] skge: dont allow bad hardware address from ROM
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20060606171110.592587000@zqx3.pdx.osdl.net>
[-- Attachment #1: skge-badaddr.patch --]
[-- Type: text/plain, Size: 709 bytes --]
Sometimes boards don't reset properly, and the address read out of the
EEPROM is zero. Stop the insanity before the device gets registered.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- skge-2.6.orig/drivers/net/skge.c
+++ skge-2.6/drivers/net/skge.c
@@ -3362,6 +3362,14 @@ static int __devinit skge_probe(struct p
if ((dev = skge_devinit(hw, 0, using_dac)) == NULL)
goto err_out_led_off;
+ if (!is_valid_ether_addr(dev->dev_addr)) {
+ printk(KERN_ERR PFX "%s: bad (zero?) ethernet address in rom\n",
+ pci_name(pdev));
+ err = -EIO;
+ goto err_out_free_netdev;
+ }
+
+
err = register_netdev(dev);
if (err) {
printk(KERN_ERR PFX "%s: cannot register net device\n",
--
^ permalink raw reply
* [PATCH 1/5] skge: use workq for PHY handling
From: Stephen Hemminger @ 2006-06-06 17:11 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20060606171110.592587000@zqx3.pdx.osdl.net>
[-- Attachment #1: skge-phy-workq.patch --]
[-- Type: text/plain, Size: 5675 bytes --]
Since accessing the PHY can take 100's of usecs, use a work queue to
allow spinning in outside of soft/hard irq.
Signed-off-by: Stephen Hemminger <shemminger@osdl.org>
--- skge-2.6.orig/drivers/net/skge.c
+++ skge-2.6/drivers/net/skge.c
@@ -603,7 +603,7 @@ static void skge_led(struct skge_port *s
struct skge_hw *hw = skge->hw;
int port = skge->port;
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
if (hw->chip_id == CHIP_ID_GENESIS) {
switch (mode) {
case LED_MODE_OFF:
@@ -663,7 +663,7 @@ static void skge_led(struct skge_port *s
PHY_M_LED_MO_RX(MO_LED_ON));
}
}
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
}
/* blink LED's for finding board */
@@ -2038,7 +2038,7 @@ static void skge_phy_reset(struct skge_p
netif_stop_queue(skge->netdev);
netif_carrier_off(skge->netdev);
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
if (hw->chip_id == CHIP_ID_GENESIS) {
genesis_reset(hw, port);
genesis_mac_init(hw, port);
@@ -2046,7 +2046,7 @@ static void skge_phy_reset(struct skge_p
yukon_reset(hw, port);
yukon_init(hw, port);
}
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
}
/* Basic MII support */
@@ -2067,12 +2067,12 @@ static int skge_ioctl(struct net_device
/* fallthru */
case SIOCGMIIREG: {
u16 val = 0;
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
if (hw->chip_id == CHIP_ID_GENESIS)
err = __xm_phy_read(hw, skge->port, data->reg_num & 0x1f, &val);
else
err = __gm_phy_read(hw, skge->port, data->reg_num & 0x1f, &val);
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
data->val_out = val;
break;
}
@@ -2081,14 +2081,14 @@ static int skge_ioctl(struct net_device
if (!capable(CAP_NET_ADMIN))
return -EPERM;
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
if (hw->chip_id == CHIP_ID_GENESIS)
err = xm_phy_write(hw, skge->port, data->reg_num & 0x1f,
data->val_in);
else
err = gm_phy_write(hw, skge->port, data->reg_num & 0x1f,
data->val_in);
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
break;
}
return err;
@@ -2191,12 +2191,12 @@ static int skge_up(struct net_device *de
goto free_rx_ring;
/* Initialize MAC */
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
if (hw->chip_id == CHIP_ID_GENESIS)
genesis_mac_init(hw, port);
else
yukon_mac_init(hw, port);
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
/* Configure RAMbuffers */
chunk = hw->ram_size / ((hw->ports + 1)*2);
@@ -2847,16 +2847,16 @@ static void skge_error_irq(struct skge_h
}
/*
- * Interrupt from PHY are handled in tasklet (soft irq)
+ * Interrupt from PHY are handled in work queue
* because accessing phy registers requires spin wait which might
* cause excess interrupt latency.
*/
-static void skge_extirq(unsigned long data)
+static void skge_extirq(void *arg)
{
- struct skge_hw *hw = (struct skge_hw *) data;
+ struct skge_hw *hw = arg;
int port;
- spin_lock(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
for (port = 0; port < hw->ports; port++) {
struct net_device *dev = hw->dev[port];
struct skge_port *skge = netdev_priv(dev);
@@ -2868,7 +2868,7 @@ static void skge_extirq(unsigned long da
bcom_phy_intr(skge);
}
}
- spin_unlock(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
hw->intr_mask |= IS_EXT_REG;
skge_write32(hw, B0_IMSK, hw->intr_mask);
@@ -2886,7 +2886,7 @@ static irqreturn_t skge_intr(int irq, vo
if (status & IS_EXT_REG) {
hw->intr_mask &= ~IS_EXT_REG;
- tasklet_schedule(&hw->ext_tasklet);
+ schedule_work(&hw->phy_work);
}
if (status & (IS_R1_F|IS_XA1_F)) {
@@ -2957,7 +2957,7 @@ static int skge_set_mac_address(struct n
if (!is_valid_ether_addr(addr->sa_data))
return -EADDRNOTAVAIL;
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
memcpy(dev->dev_addr, addr->sa_data, ETH_ALEN);
memcpy_toio(hw->regs + B2_MAC_1 + port*8,
dev->dev_addr, ETH_ALEN);
@@ -2970,7 +2970,7 @@ static int skge_set_mac_address(struct n
gma_set_addr(hw, port, GM_SRC_ADDR_1L, dev->dev_addr);
gma_set_addr(hw, port, GM_SRC_ADDR_2L, dev->dev_addr);
}
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
return 0;
}
@@ -3150,14 +3150,14 @@ static int skge_reset(struct skge_hw *hw
skge_write32(hw, B0_IMSK, hw->intr_mask);
- spin_lock_bh(&hw->phy_lock);
+ mutex_lock(&hw->phy_mutex);
for (i = 0; i < hw->ports; i++) {
if (hw->chip_id == CHIP_ID_GENESIS)
genesis_reset(hw, i);
else
yukon_reset(hw, i);
}
- spin_unlock_bh(&hw->phy_lock);
+ mutex_unlock(&hw->phy_mutex);
return 0;
}
@@ -3305,8 +3305,8 @@ static int __devinit skge_probe(struct p
}
hw->pdev = pdev;
- spin_lock_init(&hw->phy_lock);
- tasklet_init(&hw->ext_tasklet, skge_extirq, (unsigned long) hw);
+ mutex_init(&hw->phy_mutex);
+ INIT_WORK(&hw->phy_work, skge_extirq, hw);
hw->regs = ioremap_nocache(pci_resource_start(pdev, 0), 0x4000);
if (!hw->regs) {
@@ -3392,7 +3392,7 @@ static void __devexit skge_remove(struct
skge_write16(hw, B0_LED, LED_STAT_OFF);
skge_write8(hw, B0_CTST, CS_RST_SET);
- tasklet_kill(&hw->ext_tasklet);
+ flush_scheduled_work();
free_irq(pdev->irq, hw);
pci_release_regions(pdev);
--- skge-2.6.orig/drivers/net/skge.h
+++ skge-2.6/drivers/net/skge.h
@@ -2399,9 +2399,8 @@ struct skge_hw {
u32 ram_size;
u32 ram_offset;
u16 phy_addr;
-
- struct tasklet_struct ext_tasklet;
- spinlock_t phy_lock;
+ struct work_struct phy_work;
+ struct mutex phy_mutex;
};
enum {
--
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Neil Horman @ 2006-06-06 17:05 UTC (permalink / raw)
To: Mitch Williams
Cc: Kok, Auke, Garzik, Jeff, netdev, Brandeburg, Jesse, Kok, Auke,
jmoyer, mpm
In-Reply-To: <1149611965.13635.19.camel@strongmad>
On Tue, Jun 06, 2006 at 09:39:25AM -0700, Mitch Williams wrote:
> On Tue, 2006-06-06 at 09:52 -0400, Neil Horman wrote:
> > I've been speaking about this fix with a Jeff Moyer, and we've come up with some
> > concerns regarding its implementation. Specifically the call to
> > adapter->clean_rx in the case of the e1000 driver is rather a layering
> > violation in the netpoll code, in the sense that the function pointed to by clean_rx
> > is functionality that is nominally used by the dev->poll method. In fact in
> > this case, it would appear possible since dev->poll is called under the
> > poll_lock, but dev->poll_controller is not, that is is possible to have cpus in
> > a system executing in e1000_clean_rx_irq_[ps] at the same time leading to data
> > corruption:
> >
> > CPU0:
> > netpoll_poll_dev
> > dev->poll_controller (e1000_netpoll)
> > adapter->clean_rx (e1000_clean_rx_irq)
> >
> > CPU1:
> > napi_poll
> > dev->poll (e1000_clean)
> > e1000_clean_rx_irq
>
> Hmmm. You may have a point. I don't think a spinlock is required, but
> we do need to check if the poll is already scheduled on another CPU,
> like netpoll does in poll_napi().
>
> In practice, of course, we never see this. The only netpoll client in
> the kernel is netconsole, which doesn't do receives. A few Major
> Distros use netdump, which does do receives, but only after the system
> has crashed. In that case, all other CPUs are stopped anyway.
>
You are probably right, this is a rare case, if it ever happens at all, but I
think there is (at least as Jeff explained it to me) a corner case, where
netconsole on transmit, notes an exhaustion of tx descriptors, and in response
calls the poll controller method of the driver to clean up and make some of
those descriptors available:
printk
release_console_sem
call_console_drivers
netconsole.c:write_msg
netpoll_send_udp
netpoll_send_skb
if (netif_queue_stopped(np->dev)) <--- out of descriptors ?
netpoll_poll(np); <--- trigger a poll to clean up
If this is being done at the same time as a napi_poll on another cpu, we have a
real set of conditions under which a corruption can occur.
> However, just for the sake of correctness (and paranoia), I'll whip up
> another patch that does this check.
>
Thanks for the quick feedback!
Regards
Neil
> Jeff, please do not commit this patch.
>
> -Mitch
>
> NB: The root of this problem is that e1000 uses a dummy netdev to
> schedule NAPI polling. Since netpoll doesn't know about the dummy
> netdev, it checks the "real" e1000 netdev struct to see if it's
> scheduled for polling. Since this is never the case, netpoll fails when
> e1000 is configured to use NAPI. Hence, this patch.
>
> Why is the dummy netdev in place? To support multi-queue RX. Our PCIe
> adapters support this, but the kernel's not _quite_ ready yet.
> Hopefully, the VJ net channel stuff will enable this feature.
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
/***************************************************
*Neil Horman
*Software Engineer
*gpg keyid: 1024D / 0x92A74FA1 - http://pgp.mit.edu
***************************************************/
^ permalink raw reply
* Re: [PATCH] TCP changes for 2.6.18
From: Stephen Hemminger @ 2006-06-06 16:46 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20060605.165859.104035231.davem@davemloft.net>
On Mon, 05 Jun 2006 16:58:59 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Stephen Hemminger <shemminger@osdl.org>
> Date: Mon, 5 Jun 2006 11:03:31 -0700
>
> > Dave, please consider adding these for the 2.6.18.
>
> I'll pick these into my tree by extracting out the patches.
> Your tree wasn't based upon the net-2.6.18 tree, so if I just
> pull it into mine I'll get all the upstream changes since I
> cut the net-2.6.18 which at the current time I don't want :)
>
I will rebase it on your tree today.
> BTW, it seems we now have at least 3 instances "VEGAS + stuff"
> and thus the core vegas logic is duplicated that many times.
> Would be nice to have some core vegas infrastructure in the
> generic congestion avoidance layer at some point.
Agreed, and the vegas stuff needs some work as well. It should
have more smoothing like Westwood+
^ permalink raw reply
* [PATCH]TCP Westwood+
From: Luca De Cicco @ 2006-06-06 16:41 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev, David S. Miller, Saverio Mascolo
In-Reply-To: <20060605093240.5d68e117@dxpl.pdx.osdl.net>
[-- Attachment #1: Type: text/plain, Size: 801 bytes --]
Dear all,
As requested you find attached patches (they should be orghogonal) to
tcp_westwood.c. Changes are listed below:
* westwood_comments.diff: Updated comments pointing to essential papers
about TCP Westwood
* westwood_bugfix.diff: Fixes a subtle bug that caused the first sample
to be wrong
* westwood_faster_filter.diff: The bandwidth estimate filter is now
initialized with the first bandwidth sample in order to have better
performances in the case of small file transfers.
* westwood_rtt_min_reset.diff: RTT_min is updated each time a timeout
event occurs (in order to cope with hard handovers in wireless
scenarios such as UMTS). A new inline function, update_rtt_min has been
added.
Signed-off-by: Luca De Cicco <ldecicco@gmail.com>
Best Regards,
Luca De Cicco
Politecnico di Bari
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: westwood_bugfix.diff --]
[-- Type: text/x-patch; name=westwood_bugfix.diff, Size: 1073 bytes --]
Index: ipv4/tcp_westwood.c
===================================================================
--- ipv4.orig/tcp_westwood.c 2006-06-06 18:08:03.000000000 +0200
+++ ipv4/tcp_westwood.c 2006-06-06 18:09:44.000000000 +0200
@@ -22,6 +22,7 @@
u32 accounted;
u32 rtt;
u32 rtt_min; /* minimum observed RTT */
+ u16 first_ack; /* flag which infers that this is the first ack */
};
@@ -52,6 +53,7 @@
w->rtt_min = w->rtt = TCP_WESTWOOD_INIT_RTT;
w->rtt_win_sx = tcp_time_stamp;
w->snd_una = tcp_sk(sk)->snd_una;
+ w->first_ack = 1;
}
/*
@@ -184,6 +186,15 @@
{
struct tcp_sock *tp = tcp_sk(sk);
struct westwood *w = inet_csk_ca(sk);
+
+ /* Initialise w->snd_una with the first acked sequence number in oder
+ * to fix mismatch between tp->snd_una and w->snd_una for the first
+ * bandwidth sample
+ */
+ if(w->first_ack && (event == CA_EVENT_FAST_ACK||event == CA_EVENT_SLOW_ACK)) {
+ w->snd_una = tp->snd_una;
+ w->first_ack = 0;
+ }
switch(event) {
case CA_EVENT_FAST_ACK:
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: westwood_comments.diff --]
[-- Type: text/x-patch; name=westwood_comments.diff, Size: 1432 bytes --]
Index: ipv4/tcp_westwood.c
===================================================================
--- ipv4.orig/tcp_westwood.c 2006-06-06 17:56:13.000000000 +0200
+++ ipv4/tcp_westwood.c 2006-06-06 17:57:13.000000000 +0200
@@ -1,9 +1,29 @@
/*
- * TCP Westwood+
+ * TCP Westwood+: end-to-end bandwidth estimation for TCP
*
- * Angelo Dell'Aera: TCP Westwood+ support
+ * Angelo Dell'Aera: author of the first version of TCP Westwood+ in Linux 2.4
+ * Luca De Cicco: current support of Westwood+ and author of the last patch
+ * with: updated RTT_min and initial bandwidth estimate
+ *
+ * Support at http://c3lab.poliba.it/index.php/Westwood
+ * Main references in literature:
+ *
+ * - Mascolo S, Casetti, M. Gerla et al.
+ * "TCP Westwood: bandwidth estimation for TCP" Proc. ACM Mobicom 2001
+ *
+ * - A. Grieco, s. Mascolo
+ * "Performance evaluation of New Reno, Vegas, Westwood+ TCP" ACM Computer
+ * Comm. Review, 2004
+ *
+ * - A. Dell'Aera, L. Grieco, S. Mascolo.
+ * "Linux 2.4 Implementation of Westwood+ TCP with Rate-Halving :
+ * A Performance Evaluation Over the Internet" (ICC 2004), Paris, June 2004
+ *
+ * Westwood+ employs end-to-end bandwdidth measurement to set cwnd and
+ * ssthresh after packet loss. The probing phase is as the original Reno.
*/
+
#include <linux/config.h>
#include <linux/mm.h>
#include <linux/module.h>
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: westwood_faster_filter.diff --]
[-- Type: text/x-patch; name=westwood_faster_filter.diff, Size: 799 bytes --]
Index: ipv4/tcp_westwood.c
===================================================================
--- ipv4.orig/tcp_westwood.c 2006-06-06 18:14:50.000000000 +0200
+++ ipv4/tcp_westwood.c 2006-06-06 18:16:49.000000000 +0200
@@ -65,8 +65,17 @@
static inline void westwood_filter(struct westwood *w, u32 delta)
{
- w->bw_ns_est = westwood_do_filter(w->bw_ns_est, w->bk / delta);
- w->bw_est = westwood_do_filter(w->bw_est, w->bw_ns_est);
+ /*
+ * If the filter is empty fill it with the first sample of bandwidth
+ */
+ if (w->bw_ns_est==0 && w->bw_est==0)
+ {
+ w->bw_ns_est = w->bk / delta;
+ w->bw_est = w->bw_ns_est ;
+ } else {
+ w->bw_ns_est = westwood_do_filter(w->bw_ns_est, w->bk / delta);
+ w->bw_est = westwood_do_filter(w->bw_est, w->bw_ns_est);
+ }
}
/*
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #5: westwood_rtt_min_reset.diff --]
[-- Type: text/x-patch; name=westwood_rtt_min_reset.diff, Size: 1820 bytes --]
Index: ipv4/tcp_westwood.c
===================================================================
--- ipv4.orig/tcp_westwood.c 2006-06-06 18:10:22.000000000 +0200
+++ ipv4/tcp_westwood.c 2006-06-06 18:13:07.000000000 +0200
@@ -15,6 +15,7 @@
struct westwood {
u32 bw_ns_est; /* first bandwidth estimation..not too smoothed 8) */
u32 bw_est; /* bandwidth estimate */
+ u16 reset_rtt_min; /* Please reset RTT min to RTT sample*/
u32 rtt_win_sx; /* here starts a new evaluation... */
u32 bk;
u32 snd_una; /* used for evaluating the number of acked bytes */
@@ -50,6 +51,7 @@
w->bw_est = 0;
w->accounted = 0;
w->cumul_ack = 0;
+ w->reset_rtt_min = 1;
w->rtt_min = w->rtt = TCP_WESTWOOD_INIT_RTT;
w->rtt_win_sx = tcp_time_stamp;
w->snd_una = tcp_sk(sk)->snd_una;
@@ -110,6 +112,19 @@
}
}
+static inline void update_rtt_min(struct sock *sk)
+{
+ struct westwood *w = inet_csk_ca(sk);
+
+ if (w->reset_rtt_min) {
+ w->rtt_min = w->rtt;
+ w->reset_rtt_min = 0;
+ } else {
+ w->rtt_min = min(w->rtt, w->rtt_min);
+ }
+}
+
+
/*
* @westwood_fast_bw
* It is called when we are in fast path. In particular it is called when
@@ -125,7 +140,7 @@
w->bk += tp->snd_una - w->snd_una;
w->snd_una = tp->snd_una;
- w->rtt_min = min(w->rtt, w->rtt_min);
+ update_rtt_min(sk);
}
/*
@@ -207,12 +222,14 @@
case CA_EVENT_FRTO:
tp->snd_ssthresh = westwood_bw_rttmin(sk);
+ /* Please update RTT_min when next ack arrives */
+ w->reset_rtt_min = 1;
break;
case CA_EVENT_SLOW_ACK:
westwood_update_window(sk);
w->bk += westwood_acked_count(sk);
- w->rtt_min = min(w->rtt, w->rtt_min);
+ update_rtt_min(sk);
break;
default:
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Mitch Williams @ 2006-06-06 16:39 UTC (permalink / raw)
To: Neil Horman
Cc: Kok, Auke, Garzik, Jeff, netdev, Brandeburg, Jesse, Kok, Auke,
jmoyer, mpm
In-Reply-To: <20060606135217.GA21969@hmsreliant.homelinux.net>
On Tue, 2006-06-06 at 09:52 -0400, Neil Horman wrote:
> I've been speaking about this fix with a Jeff Moyer, and we've come up with some
> concerns regarding its implementation. Specifically the call to
> adapter->clean_rx in the case of the e1000 driver is rather a layering
> violation in the netpoll code, in the sense that the function pointed to by clean_rx
> is functionality that is nominally used by the dev->poll method. In fact in
> this case, it would appear possible since dev->poll is called under the
> poll_lock, but dev->poll_controller is not, that is is possible to have cpus in
> a system executing in e1000_clean_rx_irq_[ps] at the same time leading to data
> corruption:
>
> CPU0:
> netpoll_poll_dev
> dev->poll_controller (e1000_netpoll)
> adapter->clean_rx (e1000_clean_rx_irq)
>
> CPU1:
> napi_poll
> dev->poll (e1000_clean)
> e1000_clean_rx_irq
Hmmm. You may have a point. I don't think a spinlock is required, but
we do need to check if the poll is already scheduled on another CPU,
like netpoll does in poll_napi().
In practice, of course, we never see this. The only netpoll client in
the kernel is netconsole, which doesn't do receives. A few Major
Distros use netdump, which does do receives, but only after the system
has crashed. In that case, all other CPUs are stopped anyway.
However, just for the sake of correctness (and paranoia), I'll whip up
another patch that does this check.
Jeff, please do not commit this patch.
-Mitch
NB: The root of this problem is that e1000 uses a dummy netdev to
schedule NAPI polling. Since netpoll doesn't know about the dummy
netdev, it checks the "real" e1000 netdev struct to see if it's
scheduled for polling. Since this is never the case, netpoll fails when
e1000 is configured to use NAPI. Hence, this patch.
Why is the dummy netdev in place? To support multi-queue RX. Our PCIe
adapters support this, but the kernel's not _quite_ ready yet.
Hopefully, the VJ net channel stuff will enable this feature.
^ permalink raw reply
* Re: 2.6.17-rc5-mm3-lockdep -
From: Stefan Richter @ 2006-06-06 16:39 UTC (permalink / raw)
To: Valdis.Kletnieks
Cc: Jiri Slaby, Andrew Morton, arjan, mingo, linux-kernel,
linux1394-devel, netdev
In-Reply-To: <4485798B.4030007@s5r6.in-berlin.de>
>>Valdis.Kletnieks@vt.edu napsal(a):
...
>>>[ 464.687000] [ BUG: illegal lock usage! ]
>>>[ 464.687000] ----------------------------
>>>[ 464.687000] illegal {in-hardirq-W} -> {hardirq-on-W} usage.
>>>[ 464.687000] id/2700 [HC0[0]:SC0[1]:HE1:SE0] takes:
>>>[ 464.687000] (&list->lock){++..}, at: [<c0351a07>] unix_stream_connect+0x334/0x408
>>>[ 464.687000] {in-hardirq-W} state was registered at:
>>>[ 464.687000] [<c012dd45>] lockdep_acquire+0x67/0x7f
>>>[ 464.687000] [<c0383f11>] _spin_lock_irqsave+0x30/0x3f
>>>[ 464.687000] [<c02fa93f>] skb_dequeue+0x18/0x49
>>>[ 464.687000] [<f086b7f1>] hpsb_bus_reset+0x5e/0xa2 [ieee1394]
>>>[ 464.687000] [<f0887007>] ohci_irq_handler+0x370/0x726 [ohci1394]
>>>[ 464.687000] [<c013f9a8>] handle_IRQ_event+0x1d/0x52
>>>[ 464.687000] [<c0140bc4>] handle_level_irq+0x97/0xe3
>>>[ 464.687000] [<c01045d0>] do_IRQ+0x8b/0xaf
>>>[ 464.687000] irq event stamp: 2964
>>>[ 464.687000] hardirqs last enabled at (2963): [<c0384223>] _spin_unlock_irqrestore+0x3b/0x6d
>>>[ 464.687000] hardirqs last disabled at (2962): [<c0383ef5>] _spin_lock_irqsave+0x14/0x3f
>>>[ 464.687000] softirqs last enabled at (2956): [<c0119da0>] __do_softirq+0x9d/0xa5
>>>[ 464.687000] softirqs last disabled at (2964): [<c0383f6b>] _spin_lock_bh+0x10/0x3a
>>>[ 464.687000]
>>>[ 464.687000] other info that might help us debug this:
>>>[ 464.687000] 1 locks held by id/2700:
>>>[ 464.687000] #0: (&u->lock){--..}, at: [<c03517bb>] unix_stream_connect+0xe8/0x408
>>>[ 464.687000]
>>>[ 464.687000] stack backtrace:
>>>[ 464.687000] [<c01032d6>] show_trace_log_lvl+0x64/0x125
>>>[ 464.687000] [<c0103865>] show_trace+0x1b/0x20
>>>[ 464.687000] [<c010391c>] dump_stack+0x1f/0x24
>>>[ 464.687000] [<c012bfb1>] print_usage_bug+0x1a8/0x1b4
>>>[ 464.687000] [<c012c6c7>] mark_lock+0x2ba/0x4e5
>>>[ 464.687000] [<c012d2b8>] __lockdep_acquire+0x476/0xa91
>>>[ 464.687000] [<c012dd45>] lockdep_acquire+0x67/0x7f
>>>[ 464.687000] [<c0383f87>] _spin_lock_bh+0x2c/0x3a
>>>[ 464.687000] [<c0351a07>] unix_stream_connect+0x334/0x408
>>>[ 464.687000] [<c02f7236>] sys_connect+0x6e/0xa3
>>>[ 464.687000] [<c02f79da>] sys_socketcall+0x96/0x190
>>>[ 464.687000] [<c03845e2>] sysenter_past_esp+0x63/0xa1
On second look it doesn't seem to be a problem of the ieee1394 stack but
rather of underlying skb and net infrastructure.
BTW, the locking in -mm's net/unix/af_unix.c::unix_stream_connect()
differs a bit from stock unix_stream_connect(). I see spin_lock_bh() in
2.6.17-rc5-mm3 where 2.6.17-rc5 has spin_lock().
(added Cc: netdev)
--
Stefan Richter
-=====-=-==- -==- --==-
http://arcgraph.de/sr/
^ permalink raw reply
* Re: [PATCH] hush noisy ieee80211 CCMP printks
From: Jason Lunz @ 2006-06-06 15:02 UTC (permalink / raw)
To: Jouni Malinen; +Cc: linville, netdev
In-Reply-To: <20060606034138.GB9568@jm.kir.nu>
On Mon, Jun 05, 2006 at 08:41:38PM -0700, Jouni Malinen wrote:
> Do you happen to have a wireless sniffer that you could use to capture
> the frames? It would be interesting to see whether such a capture log
> could be mapped into the dropped frames shown in the kernel debug log.
I don't know. Would booting something like knoppix on another laptop
with some flavor of Intel wireless do? The only two laptops I have
regular access to are the bcm43xx one in question and another Intel one
that ordinarily runs winxp.
> Would you be interested in testing this with net/d80211 code and
> wireless-dev.git?
sure, I can do that. I'm fairly clumsy with git, so it may take a while.
Jason
^ permalink raw reply
* Re: [PATCH 1/1] LSM-IPsec SELinux Authorize (with minor fix)
From: Xiaolan Zhang @ 2006-06-06 14:55 UTC (permalink / raw)
To: James Morris
Cc: netdev, davem, chrisw, herbert, sds, tjaeger, latten,
Serge E Hallyn, George Wilson, czhang.us
In-Reply-To: <Pine.LNX.4.64.0606060133220.9787@d.namei>
Singned-off-by: Catherine Zhang <cxzhang@watson.ibm.com>
James, is this enough or do I need to modify the original patch to add the
above line? The code was taken from various pieces of patches originally
from Trent and merged/modified by me. Let me know what else I need to do.
thanks,
Catherine
James Morris <jmorris@namei.org> wrote on 06/06/2006 01:37:04 AM:
> On Tue, 6 Jun 2006, Catherine Zhang wrote:
>
> > Minor fix per James' comment.
>
> Can you also add a Signed-off-by line?
>
> I can't recall if you were the original author. If not, we also need a
> From line (per Documentation/SubmittingPatches).
>
>
> Thanks,
>
> --
> James Morris
> <jmorris@namei.org>
^ permalink raw reply
* Re: [PATCH 1/2] e1000: fix netpoll with NAPI
From: Neil Horman @ 2006-06-06 13:52 UTC (permalink / raw)
To: Kok, Auke; +Cc: Garzik, Jeff, netdev, Brandeburg, Jesse, Kok, Auke, jmoyer, mpm
In-Reply-To: <20060605231125.12584.17039.stgit@gitlost.site>
On Mon, Jun 05, 2006 at 04:11:25PM -0700, Kok, Auke wrote:
>
> Netpoll was broken due to the earlier addition of multiqueue.
>
> Signed-off-by: Mitch Williams <mitch.a.williams@intel.com>
> Signed-off-by: Auke Kok <auke-jan.h.kok@intel.com>
> ---
>
> drivers/net/e1000/e1000_main.c | 9 ++++++++-
> 1 files changed, 8 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/e1000/e1000_main.c b/drivers/net/e1000/e1000_main.c
> index ed15fca..7103a0e 100644
> --- a/drivers/net/e1000/e1000_main.c
> +++ b/drivers/net/e1000/e1000_main.c
> @@ -4629,10 +4629,17 @@ static void
> e1000_netpoll(struct net_device *netdev)
> {
> struct e1000_adapter *adapter = netdev_priv(netdev);
> +#ifdef CONFIG_E1000_NAPI
> + int budget = 0;
> +
> + disable_irq(adapter->pdev->irq);
> + e1000_clean_tx_irq(adapter, adapter->tx_ring);
> + adapter->clean_rx(adapter, adapter->rx_ring, &budget, netdev->weight);
> +#else
> +
I've been speaking about this fix with a Jeff Moyer, and we've come up with some
concerns regarding its implementation. Specifically the call to
adapter->clean_rx in the case of the e1000 driver is rather a layering
violation in the netpoll code, in the sense that the function pointed to by clean_rx
is functionality that is nominally used by the dev->poll method. In fact in
this case, it would appear possible since dev->poll is called under the
poll_lock, but dev->poll_controller is not, that is is possible to have cpus in
a system executing in e1000_clean_rx_irq_[ps] at the same time leading to data
corruption:
CPU0:
netpoll_poll_dev
dev->poll_controller (e1000_netpoll)
adapter->clean_rx (e1000_clean_rx_irq)
CPU1:
napi_poll
dev->poll (e1000_clean)
e1000_clean_rx_irq
I'm not sure what the right fix is here. A spinlock in e1000_clean_rx_irq[_ps]
would seem to be an easy and direct fix, but I don't know that thats the best
solution.
Something I don't understand is why the call to adapter->clean_rx is
required in the first place when the napi_poll routine calls it itself directly.
All other drivers schedule a napi poll and receive frames via that path. My
guess is that it has to do with the fact that we schedule polls on the device in
the polling_netdev array, rather than the actual registered netdev. Looking at
the driver code I note that while an entire array is allocated for polling
netdevs, we only ever use entry 0. Would it make sense to just remove the the
polling_netdev array and use the registered device like all the other drivers.
I expect that would eliminate the need for this patch as well.
Regards
Neil
> disable_irq(adapter->pdev->irq);
> e1000_intr(adapter->pdev->irq, netdev, NULL);
> e1000_clean_tx_irq(adapter, adapter->tx_ring);
> -#ifndef CONFIG_E1000_NAPI
> adapter->clean_rx(adapter, adapter->rx_ring);
> #endif
> enable_irq(adapter->pdev->irq);
>
>
>
> --
> Auke Kok <auke-jan.h.kok@intel.com>
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
/***************************************************
*Neil Horman
*Software Engineer
*gpg keyid: 1024D / 0x92A74FA1 - http://pgp.mit.edu
***************************************************/
^ permalink raw reply
* Re: [PATCH 2/3] pci: bcm43xx avoid pci_find_device
From: Michael Buesch @ 2006-06-06 12:58 UTC (permalink / raw)
To: Jiri Slaby
Cc: Greg KH, Linux Kernel Mailing List, linux-pci, jgarzik, netdev,
mb, st3, linville
In-Reply-To: <20060605205309.GA31061@kroah.com>
On Monday 05 June 2006 22:53, Greg KH wrote:
> On Mon, Jun 05, 2006 at 10:20:07PM +0200, Jiri Slaby wrote:
> > bcm43xx avoid pci_find_device
> >
> > Change pci_find_device to safer pci_get_device with support for more
> > devices.
> >
> > Signed-off-by: Jiri Slaby <jirislaby@gmail.com>
> >
> > ---
> > commit 4b73c16f5411d97360d5f26f292ffddeb670ff75
> > tree 6e43c8bd02498eb1ceec6bdc64277fa8408da9e2
> > parent d59f9ea8489749f59cd0c7333a4784cab964daa8
> > author Jiri Slaby <ku@bellona.localdomain> Mon, 05 Jun 2006 22:01:03 +0159
> > committer Jiri Slaby <ku@bellona.localdomain> Mon, 05 Jun 2006 22:01:03 +0159
> >
> > drivers/net/wireless/bcm43xx/bcm43xx_main.c | 21 ++++++++++++++++-----
> > 1 files changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_main.c b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> > index 22b8fa6..d1a9975 100644
> > --- a/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> > +++ b/drivers/net/wireless/bcm43xx/bcm43xx_main.c
> > @@ -2133,6 +2133,13 @@ out:
> > return err;
> > }
> >
> > +#ifdef CONFIG_BCM947XX
> > +static struct pci_device_id bcm43xx_47xx_ids[] = {
> > + { PCI_DEVICE(PCI_VENDOR_ID_BROADCOM, 0x4324) },
> > + { 0 }
> > +};
> > +#endif
> > +
> > static int bcm43xx_initialize_irq(struct bcm43xx_private *bcm)
> > {
> > int res;
> > @@ -2142,11 +2149,15 @@ static int bcm43xx_initialize_irq(struct
> > bcm->irq = bcm->pci_dev->irq;
> > #ifdef CONFIG_BCM947XX
> > if (bcm->pci_dev->bus->number == 0) {
> > - struct pci_dev *d = NULL;
> > - /* FIXME: we will probably need more device IDs here... */
> > - d = pci_find_device(PCI_VENDOR_ID_BROADCOM, 0x4324, NULL);
> > - if (d != NULL) {
> > - bcm->irq = d->irq;
> > + struct pci_dev *d;
> > + struct pci_device_id *id;
> > + for (id = bcm43xx_47xx_ids; id->vendor; id++) {
> > + d = pci_get_device(id->vendor, id->device, NULL);
> > + if (d != NULL) {
> > + bcm->irq = d->irq;
> > + pci_dev_put(d);
> > + break;
> > + }
>
> This will not work if you have more than one of the same devices in the
> system.
>
> Well, the original code will not either :(
>
> Why not just use the proper pci interface? Why poke around in another
> pci device to steal an irq, when that irq might not even be valid?
> (irqs are not valid until pci_enable_device() is called on them...)
Ok, if someone really wants to have this patch in mainline.
Signed-off-by: Michael Buesch <mb@bu3sch.de>
But the whole purpose of this patch is really questionable.
* This code is only compiled for the OpenWRT Router kernel.
This kernel does not use softmac, but dscape stack.
So nobody will ever actually use this code.
One could even argue, if the code should be removed from
the softmac driver, but I think openwrt people use it for
some kind of hacking, testing, whatever.
It's not so much code, so I don't care. It does not add
maintainance work.
* Do we really need to increment some reference counters?
I mean, we are asking for a bus here. This bus is not
hotpluggable or something and will never go away. Either it
is soldered on the board or not. That is what we test here.
So, Jiri, if you really want to have this patch upstream, you
have my Signed-off-by. If nobody else cares, it will get lost
in the deep black netdev hole. ;)
--
Greetings Michael.
^ permalink raw reply
* Re: 2.6.17-rc5-mm3
From: Mel Gorman @ 2006-06-06 10:57 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-kernel, netdev
In-Reply-To: <20060605115456.c2b1e156.akpm@osdl.org>
On Mon, 5 Jun 2006, Andrew Morton wrote:
> On Mon, 5 Jun 2006 18:56:37 +0100
> mel@csn.ul.ie (Mel Gorman) wrote:
>
>>
>> I am seeing more networking-related funniness with 2.6.17-rc5-mm3 on the
>> same machine previously fixed by git-net-llc-fix.patch. The console log is
>> below. I've done no investigation work in case it's a known problem.
>
> It's not a known problem, afaik.
>
>> ...
>> Starting anacron: [ OK ]
>> Starting atd: [ OK ]
>> Starting Avahi daemon: [ OK ]
>> Starting cups-config-daemon: [ OK ]
>> Starting HAL daemon: [ OK ]
>> Fedora Core release 5 (Bordeaux)
>> Kernel 2.6.17-rc5-mm2-autokern1 on an x86_64
Bah, I'm a spanner. The patches I was testing were rebased to the latest
-mm, but the kernel version they were then tested on was not changed. This
was probably the LLC bug with a different shaped error and the first set
of tests are passing with -mm3. Sorry for the noise.
--
Mel Gorman
Part-time Phd Student Linux Technology Center
University of Limerick IBM Dublin Software Lab
^ permalink raw reply
* Re: [NET]: Add netif_tx_lock
From: Herbert Xu @ 2006-06-06 10:22 UTC (permalink / raw)
To: David Miller; +Cc: mchan, jgarzik, netdev
In-Reply-To: <20060605.213250.85688925.davem@davemloft.net>
On Mon, Jun 05, 2006 at 09:32:50PM -0700, David Miller wrote:
>
> IPOIB is going to BUG() with this change. Because now, in their
> multicast code, you're going to local_bh_disable() via
> netif_tx_unlock() with hw IRQs disabled which is illegal.
>
> It shows a bug here in the locking of the IPOIB driver.
You had me woried there.
> We need to think about this change some more. :)
I thought about it a bit more and I'm not worried anymore :)
Notice that the patch does netif_tx_lock/netif_tx_unlock
for IB instead of netif_tx_lock_bh/netif_tx_unlock_bh.
So there is no BH enabling going on at all.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox