* [git patches] net driver fixes
From: Jeff Garzik @ 2007-10-03 18:40 UTC (permalink / raw)
To: Andrew Morton, Linus Torvalds; +Cc: netdev, LKML
sky2 is really the only important fix, the others are trivial.
Please pull from 'upstream-linus' branch of
master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git upstream-linus
to receive the following updates:
drivers/net/sky2.c | 3 ---
drivers/net/wireless/bcm43xx/bcm43xx_wx.c | 2 +-
net/ieee80211/softmac/ieee80211softmac_wx.c | 2 +-
3 files changed, 2 insertions(+), 5 deletions(-)
Joe Perches (1):
bcm43xx: Correct printk with PFX before KERN_
Richard Knutsson (1):
softmac: Fix compiler-warning
Stephen Hemminger (1):
sky2: jumbo frame regression fix
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 162489b..ea117fc 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -2163,9 +2163,6 @@ static struct sk_buff *sky2_receive(struct net_device *dev,
sky2->rx_next = (sky2->rx_next + 1) % sky2->rx_pending;
prefetch(sky2->rx_ring + sky2->rx_next);
- if (length < ETH_ZLEN || length > sky2->rx_data_size)
- goto len_error;
-
/* This chip has hardware problems that generates bogus status.
* So do only marginal checking and expect higher level protocols
* to handle crap frames.
diff --git a/drivers/net/wireless/bcm43xx/bcm43xx_wx.c b/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
index d6d9413..6acfdc4 100644
--- a/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
+++ b/drivers/net/wireless/bcm43xx/bcm43xx_wx.c
@@ -444,7 +444,7 @@ static int bcm43xx_wx_set_xmitpower(struct net_device *net_dev,
u16 maxpower;
if ((data->txpower.flags & IW_TXPOW_TYPE) != IW_TXPOW_DBM) {
- printk(PFX KERN_ERR "TX power not in dBm.\n");
+ printk(KERN_ERR PFX "TX power not in dBm.\n");
return -EOPNOTSUPP;
}
diff --git a/net/ieee80211/softmac/ieee80211softmac_wx.c b/net/ieee80211/softmac/ieee80211softmac_wx.c
index 442b987..5742dc8 100644
--- a/net/ieee80211/softmac/ieee80211softmac_wx.c
+++ b/net/ieee80211/softmac/ieee80211softmac_wx.c
@@ -114,7 +114,7 @@ check_assoc_again:
sm->associnfo.associating = 1;
/* queue lower level code to do work (if necessary) */
schedule_delayed_work(&sm->associnfo.work, 0);
-out:
+
mutex_unlock(&sm->associnfo.mutex);
return 0;
^ permalink raw reply related
* [ofa-general] Re: InfiniBand/RDMA merge plans for 2.6.24
From: Shirley Ma @ 2007-10-03 18:43 UTC (permalink / raw)
To: Roland Dreier; +Cc: netdev, linux-kernel, general, netdev-owner
In-Reply-To: <ada1wcxnev5.fsf@cisco.com>
Roland Dreier <rdreier@cisco.com> wrote on 09/17/2007 02:47:42 PM:
> > > IPoIB CM handles this properly by gathering together single pages
in
> > > skbs' fragment lists.
>
> > Then can we reuse IPoIB CM code here?
>
> Yes, if possible, refactoring things so that the rx skb allocation
> code becomes common between CM and non-CM would definitely make sense.
IPoIB-CM rx skb allocation is not generic to be used by UD, it allocates
more buffers than needed if mtu is not 64K, and doesn't query the real
max_num_sg from the device. I am thinking to have a generic skb allocation
in IPoIB based on matrix of (ipoib-mtu-size, page-size, max_num_sg,
head-size).
Thanks
Shirley
^ permalink raw reply
* [PATCH] Fix rose.ko oops on unload
From: Alexey Dobriyan @ 2007-10-03 18:54 UTC (permalink / raw)
To: ralf, davem; +Cc: netdev, pidoux
Quick'n'dirty fix to 100% oops on "rmmod rose". Do you want me to
properly unwind everything before .24?
-----------
Commit a3d384029aa304f8f3f5355d35f0ae274454f7cd aka
"[AX.25]: Fix unchecked rose_add_loopback_neigh uses"
transformed rose_loopback_neigh var into statically allocated one.
However, on unload it will be kfree's which can't work.
Steps to reproduce:
modprobe rose
rmmod rose
BUG: unable to handle kernel NULL pointer dereference at virtual address 00000008
printing eip:
c014c664
*pde = 00000000
Oops: 0000 [#1]
PREEMPT DEBUG_PAGEALLOC
Modules linked in: rose ax25 fan ufs loop usbhid rtc snd_intel8x0 snd_ac97_codec ehci_hcd ac97_bus uhci_hcd thermal usbcore button processor evdev sr_mod cdrom
CPU: 0
EIP: 0060:[<c014c664>] Not tainted VLI
EFLAGS: 00210086 (2.6.23-rc9 #3)
EIP is at kfree+0x48/0xa1
eax: 00000556 ebx: c1734aa0 ecx: f6a5e000 edx: f7082000
esi: 00000000 edi: f9a55d20 ebp: 00200287 esp: f6a5ef28
ds: 007b es: 007b fs: 0000 gs: 0033 ss: 0068
Process rmmod (pid: 1823, ti=f6a5e000 task=f7082000 task.ti=f6a5e000)
Stack: f9a55d20 f9a5200c 00000000 00000000 00000000 f6a5e000 f9a5200c f9a55a00
00000000 bf818cf0 f9a51f3f f9a55a00 00000000 c0132c60 65736f72 00000000
f69f9630 f69f9528 c014244a f6a4e900 00200246 f7082000 c01025e6 00000000
Call Trace:
[<f9a5200c>] rose_rt_free+0x1d/0x49 [rose]
[<f9a5200c>] rose_rt_free+0x1d/0x49 [rose]
[<f9a51f3f>] rose_exit+0x4c/0xd5 [rose]
[<c0132c60>] sys_delete_module+0x15e/0x186
[<c014244a>] remove_vma+0x40/0x45
[<c01025e6>] sysenter_past_esp+0x8f/0x99
[<c012bacf>] trace_hardirqs_on+0x118/0x13b
[<c01025b6>] sysenter_past_esp+0x5f/0x99
=======================
Code: 05 03 1d 80 db 5b c0 8b 03 25 00 40 02 00 3d 00 40 02 00 75 03 8b 5b 0c 8b 73 10 8b 44 24 18 89 44 24 04 9c 5d fa e8 77 df fd ff <8b> 56 08 89 f8 e8 84 f4 fd ff e8 bd 32 06 00 3b 5c 86 60 75 0f
EIP: [<c014c664>] kfree+0x48/0xa1 SS:ESP 0068:f6a5ef28
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
include/net/rose.h | 2 +-
net/rose/rose_loopback.c | 4 ++--
net/rose/rose_route.c | 15 ++++++++++-----
3 files changed, 13 insertions(+), 8 deletions(-)
--- a/include/net/rose.h
+++ b/include/net/rose.h
@@ -188,7 +188,7 @@ extern void rose_kick(struct sock *);
extern void rose_enquiry_response(struct sock *);
/* rose_route.c */
-extern struct rose_neigh rose_loopback_neigh;
+extern struct rose_neigh *rose_loopback_neigh;
extern const struct file_operations rose_neigh_fops;
extern const struct file_operations rose_nodes_fops;
extern const struct file_operations rose_routes_fops;
--- a/net/rose/rose_loopback.c
+++ b/net/rose/rose_loopback.c
@@ -79,7 +79,7 @@ static void rose_loopback_timer(unsigned long param)
skb_reset_transport_header(skb);
- sk = rose_find_socket(lci_o, &rose_loopback_neigh);
+ sk = rose_find_socket(lci_o, rose_loopback_neigh);
if (sk) {
if (rose_process_rx_frame(sk, skb) == 0)
kfree_skb(skb);
@@ -88,7 +88,7 @@ static void rose_loopback_timer(unsigned long param)
if (frametype == ROSE_CALL_REQUEST) {
if ((dev = rose_dev_get(dest)) != NULL) {
- if (rose_rx_call_request(skb, dev, &rose_loopback_neigh, lci_o) == 0)
+ if (rose_rx_call_request(skb, dev, rose_loopback_neigh, lci_o) == 0)
kfree_skb(skb);
} else {
kfree_skb(skb);
--- a/net/rose/rose_route.c
+++ b/net/rose/rose_route.c
@@ -45,7 +45,7 @@ static DEFINE_SPINLOCK(rose_neigh_list_lock);
static struct rose_route *rose_route_list;
static DEFINE_SPINLOCK(rose_route_list_lock);
-struct rose_neigh rose_loopback_neigh;
+struct rose_neigh *rose_loopback_neigh;
/*
* Add a new route to a node, and in the process add the node and the
@@ -362,7 +362,12 @@ out:
*/
void rose_add_loopback_neigh(void)
{
- struct rose_neigh *sn = &rose_loopback_neigh;
+ struct rose_neigh *sn;
+
+ rose_loopback_neigh = kmalloc(sizeof(struct rose_neigh), GFP_KERNEL);
+ if (!rose_loopback_neigh)
+ return;
+ sn = rose_loopback_neigh;
sn->callsign = null_ax25_address;
sn->digipeat = NULL;
@@ -417,13 +422,13 @@ int rose_add_loopback_node(rose_address *address)
rose_node->mask = 10;
rose_node->count = 1;
rose_node->loopback = 1;
- rose_node->neighbour[0] = &rose_loopback_neigh;
+ rose_node->neighbour[0] = rose_loopback_neigh;
/* Insert at the head of list. Address is always mask=10 */
rose_node->next = rose_node_list;
rose_node_list = rose_node;
- rose_loopback_neigh.count++;
+ rose_loopback_neigh->count++;
out:
spin_unlock_bh(&rose_node_list_lock);
@@ -454,7 +459,7 @@ void rose_del_loopback_node(rose_address *address)
rose_remove_node(rose_node);
- rose_loopback_neigh.count--;
+ rose_loopback_neigh->count--;
out:
spin_unlock_bh(&rose_node_list_lock);
^ permalink raw reply
* Re: [PATCH] sky2: jumbo frame regression fix
From: Ian Kumlien @ 2007-10-03 18:28 UTC (permalink / raw)
To: Bill Davidsen; +Cc: Stephen Hemminger, Jeff Garzik, Linux-kernel, netdev
In-Reply-To: <4703D9B6.109@tmr.com>
[-- Attachment #1: Type: text/plain, Size: 1053 bytes --]
On ons, 2007-10-03 at 14:04 -0400, Bill Davidsen wrote:
> Ian Kumlien wrote:
> > On tis, 2007-10-02 at 18:02 -0700, Stephen Hemminger wrote:
> >> Remove unneeded check that caused problems with jumbo frame sizes.
> >> The check was recently added and is wrong.
> >> When using jumbo frames the sky2 driver does fragmentation, so
> >> rx_data_size is less than mtu.
> >
> > Confirmed working.
> >
> > Now running with 9k mtu with no errors, =)
>
> Have you verified that you are actually getting jumbo packets out of the
> NIC? I had one machine which did standard packets silently using sky2
> and jumbo using sk98lin. I was looking for something else with tcpdump
> and got one of those WTF moments when I saw all the tiny packets.
20:27:06.542461 IP pi.local > blue.local: ICMP echo request, id 27173, seq 42, length 8008
20:27:06.543136 IP blue.local > pi.local: ICMP echo reply, id 27173, seq 42, length 8008
That should solve it for us, right? =)
--
Ian Kumlien <pomac () vapor ! com> -- http://pomac.netswarm.net
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] Fix rose.ko oops on unload
From: Jeff Garzik @ 2007-10-03 19:04 UTC (permalink / raw)
To: Alexey Dobriyan; +Cc: ralf, davem, netdev, pidoux
In-Reply-To: <20071003185413.GA16242@martell.zuzino.mipt.ru>
Alexey Dobriyan wrote:
> Quick'n'dirty fix to 100% oops on "rmmod rose". Do you want me to
> properly unwind everything before .24?
> -----------
> Commit a3d384029aa304f8f3f5355d35f0ae274454f7cd aka
> "[AX.25]: Fix unchecked rose_add_loopback_neigh uses"
> transformed rose_loopback_neigh var into statically allocated one.
> However, on unload it will be kfree's which can't work.
I'm definitely missing something... assuming your patch is applied,
where is the kfree() for rose_loopback_neigh ?
Jeff
^ permalink raw reply
* Re: [PATCH] Fix rose.ko oops on unload
From: Alexey Dobriyan @ 2007-10-03 19:21 UTC (permalink / raw)
To: Jeff Garzik; +Cc: ralf, davem, netdev, pidoux
In-Reply-To: <4703E7B4.2090309@garzik.org>
On Wed, Oct 03, 2007 at 03:04:20PM -0400, Jeff Garzik wrote:
> Alexey Dobriyan wrote:
> >Quick'n'dirty fix to 100% oops on "rmmod rose". Do you want me to
> >properly unwind everything before .24?
> >-----------
> >Commit a3d384029aa304f8f3f5355d35f0ae274454f7cd aka
> >"[AX.25]: Fix unchecked rose_add_loopback_neigh uses"
> >transformed rose_loopback_neigh var into statically allocated one.
> >However, on unload it will be kfree's which can't work.
>
> I'm definitely missing something... assuming your patch is applied,
> where is the kfree() for rose_loopback_neigh ?
AFAICS, it will be glued to rose_neigh_list in rose_add_loopback_neigh().
On unload, found and rose_remove_neigh() will free it.
^ permalink raw reply
* Re: lockdep report from bonding.
From: Andy Gospodarek @ 2007-10-03 20:50 UTC (permalink / raw)
To: Dave Jones; +Cc: netdev
In-Reply-To: <20071003170514.GA32239@redhat.com>
On Wed, Oct 03, 2007 at 01:05:14PM -0400, Dave Jones wrote:
> Reported by a Fedora user this morning.
>
> Ethernet Channel Bonding Driver: v3.1.3 (June 13, 2007)
> bonding: MII link monitoring set to 100 ms
> ADDRCONF(NETDEV_UP): bond0: link is not ready
> bonding: bond0: Adding slave eth0.
> e100: eth0: e100_watchdog: link up, 100Mbps, full-duplex
> bonding: bond0: making interface eth0 the new active one.
> bonding: bond0: enslaving eth0 as an active interface with an up link.
>
> =================================
> [ INFO: inconsistent lock state ]
> 2.6.23-0.214.rc8.git2.fc8 #1
> ---------------------------------
> inconsistent {softirq-on-W} -> {in-softirq-W} usage.
> events/1/10 [HC0[0]:SC1[1]:HE1:SE0] takes:
> (&(bond_info->tx_hashtbl_lock)){-+..}, at: [<f8ad154c>] tlb_clear_slave+0x1d/0x9a [bonding]
> {softirq-on-W} state was registered at:
> [<c0449fb0>] __lock_acquire+0x4ff/0xc67
> [<c044ab92>] lock_acquire+0x7b/0x9e
> [<c0633050>] _spin_lock+0x2e/0x58
> [<f8ad293a>] bond_alb_initialize+0x64/0x18e [bonding]
> [<f8acf25f>] bond_open+0x33/0x178 [bonding]
> [<c05ceb36>] dev_open+0x31/0x6c
> [<c05ccc8d>] dev_change_flags+0xa3/0x156
> [<c060d579>] devinet_ioctl+0x207/0x50e
> [<c060dc27>] inet_ioctl+0x86/0xa4
> [<c05c2e62>] sock_ioctl+0x1ac/0x1c9
> [<c04942a2>] do_ioctl+0x22/0x68
> [<c0494531>] vfs_ioctl+0x249/0x25c
> [<c049458d>] sys_ioctl+0x49/0x64
> [<c040522e>] syscall_call+0x7/0xb
> [<ffffffff>] 0xffffffff
> irq event stamp: 40878
> hardirqs last enabled at (40878): [<c0633474>] _spin_unlock_irq+0x22/0x2f
> hardirqs last disabled at (40877): [<c063339d>] _spin_lock_irq+0x19/0x67
> softirqs last enabled at (40872): [<c05e6fcf>] rt_run_flush+0x6e/0x97
> softirqs last disabled at (40873): [<c04075d4>] do_softirq+0x74/0xf7
>
> other info that might help us debug this:
> 3 locks held by events/1/10:
> #0: (rtnl_mutex){--..}, at: [<c0631c31>] mutex_lock+0x21/0x24
> #1: (&bond->lock){-.-+}, at: [<f8ad25ed>] bond_alb_monitor+0x16/0x26e [bonding]
> #2: (&bond->curr_slave_lock){..-+}, at: [<f8ad2680>] bond_alb_monitor+0xa9/0x26e [bonding]
>
> stack backtrace:
> [<c0406463>] show_trace_log_lvl+0x1a/0x2f
> [<c0406e4d>] show_trace+0x12/0x14
> [<c0406e65>] dump_stack+0x16/0x18
> [<c0448856>] print_usage_bug+0x141/0x14b
> [<c04490dc>] mark_lock+0x12f/0x472
> [<c0449f38>] __lock_acquire+0x487/0xc67
> [<c044ab92>] lock_acquire+0x7b/0x9e
> [<c0633050>] _spin_lock+0x2e/0x58
> [<f8ad154c>] tlb_clear_slave+0x1d/0x9a [bonding]
> [<f8ad269a>] bond_alb_monitor+0xc3/0x26e [bonding]
> [<c043541b>] run_timer_softirq+0x127/0x18f
> [<c0432a21>] __do_softirq+0x78/0xff
> [<c04075d4>] do_softirq+0x74/0xf7
> =======================
> ADDRCONF(NETDEV_CHANGE): bond0: link becomes ready
> bonding: bond0: Adding slave eth1.
>
This isn't surprising and rears it's head every once in a while. It
probably becomes more apparent on faster, multiprocessor systems. The
big bonding-workqueue conversion patch that Jay and I have been testing
for a while should resolve this one too (since it moves the monitoring
out of softirq context *and* it switches the hashtbl locks to _bh ones
along with a bunch of other changes).
^ permalink raw reply
* Re: tcp bw in 2.6
From: Larry McVoy @ 2007-10-03 21:23 UTC (permalink / raw)
To: Pekka Pietikainen; +Cc: lm, David Miller, torvalds, herbert, wscott, netdev
In-Reply-To: <20071003211321.GA13583@ee.oulu.fi>
> A few notes to the discussion. I've seen one e1000 "bug" that ended up being
> a crappy AMD pre-opteron SMP chipset with a totally useless PCI bus
> implementation, which limited performance quite a bit-totally depending on
> what you plugged in and in which slot. 10e milk-and-bread-store
> 32/33 gige nics actually were better than server-class e1000's
> in those, but weren't that great either.
That could well be my problem, this is a dual processor (not core) athlon
(not opteron) tyan motherboard if I recall correctly.
> Check your interrupt rates for the interface. You shouldn't be getting
> anywhere near 1 interrupt/packet. If you are, something is badly wrong :).
The acks (because I'm sending) are about 1.5 packets/interrupt.
When this box is receiving it's moving about 3x ass much data
and has a _lower_ (absolute, not per packet) interrupt load.
--
---
Larry McVoy lm at bitmover.com http://www.bitkeeper.com
^ permalink raw reply
* Re: tcp bw in 2.6
From: Pekka Pietikainen @ 2007-10-03 21:13 UTC (permalink / raw)
To: lm, David Miller, torvalds, herbert, wscott, netdev
In-Reply-To: <20071002212132.GF29944@bitmover.com>
On Tue, Oct 02, 2007 at 02:21:32PM -0700, Larry McVoy wrote:
> More data, sky2 works fine (really really fine, like 79MB/sec) between
> Linux dylan.bitmover.com 2.6.18.1 #5 SMP Mon Oct 23 17:36:00 PDT 2006 i686
> Linux steele 2.6.20-16-generic #2 SMP Sun Sep 23 18:31:23 UTC 2007 x86_64
>
> So this is looking like a e1000 bug. I'll try to upgrade the kernel on
> the ia64 box and see what happens.
A few notes to the discussion. I've seen one e1000 "bug" that ended up being
a crappy AMD pre-opteron SMP chipset with a totally useless PCI bus
implementation, which limited performance quite a bit-totally depending on
what you plugged in and in which slot. 10e milk-and-bread-store
32/33 gige nics actually were better than server-class e1000's
in those, but weren't that great either.
A few things worth trying out is using recv(.., MSG_TRUNC ) on the receiver,
that tests the theoretical sender maximum performance much better (but memory
bandwidth vs. GigE is much higher these days than it was in 2001 so maybe
not that useful anymore).
Check your interrupt rates for the interface. You shouldn't be getting
anywhere near 1 interrupt/packet. If you are, something is badly wrong :).
Running getsockopt(...TCP_INFO) every few secs on the socket and printing
that out can be useful too. That gives you both sides' idea on what the
tcp windows etc. are.
My favourite tool is a home-made thing called yantt btw.
( http://www.ee.oulu.fi/~pp/yantt.tgz . Needs lots of cleanup love,
it mucks with the window sizes by default, since in the 2.4 days you really
had to do that to get any kind of performance and the help text is wrong.
But it's pretty easy to hack to try out new ideas, use
sendfile/MSG_TRUNC/TCP_INFO etc.
Netperf is the kitchen sink of network benchmark tools. But trying out a few
tiny things with it is not fun at all, I tried and quickly decided to
write my own tool for my master's thesis work ;-)
Oh. Don't measure CPU usage with top. Use a cyclesoaker (google for
cyclesoak, I included akpm's with yantt) :-)
And yes. TCP stacks do have bugs, especially when things get outside the
equipment most people have. Having a dedicated transatlantic 2.5Gbps
connection found a really fun one a long time ago ;)
--
Pekka Pietikainen
^ permalink raw reply
* Re: tcp bw in 2.6
From: Pekka Pietikainen @ 2007-10-03 21:50 UTC (permalink / raw)
To: lm, David Miller, torvalds, herbert, wscott, netdev
In-Reply-To: <20071003212358.GF6183@bitmover.com>
On Wed, Oct 03, 2007 at 02:23:58PM -0700, Larry McVoy wrote:
> > A few notes to the discussion. I've seen one e1000 "bug" that ended up being
> > a crappy AMD pre-opteron SMP chipset with a totally useless PCI bus
> > implementation, which limited performance quite a bit-totally depending on
> > what you plugged in and in which slot. 10e milk-and-bread-store
> > 32/33 gige nics actually were better than server-class e1000's
> > in those, but weren't that great either.
>
> That could well be my problem, this is a dual processor (not core) athlon
> (not opteron) tyan motherboard if I recall correctly.
If it's AMD760/768MPX, here's some relevant discussion:
http://lkml.org/lkml/2002/7/18/292
http://www.ussg.iu.edu/hypermail/linux/kernel/0307.1/1109.html
http://www.ussg.iu.edu/hypermail/linux/kernel/0307.1/1154.html
http://www.ussg.iu.edu/hypermail/linux/kernel/0307.1/1212.html
http://forums.2cpu.com/showthread.php?s=&threadid=31211
>
> > Check your interrupt rates for the interface. You shouldn't be getting
> > anywhere near 1 interrupt/packet. If you are, something is badly wrong :).
>
> The acks (because I'm sending) are about 1.5 packets/interrupt.
> When this box is receiving it's moving about 3x ass much data
> and has a _lower_ (absolute, not per packet) interrupt load.
Probably not a problem then, since those acks probably cover many
sent packets. Current interrupt mitigation schemes are pretty
dynamic, balancing between latency and bulk performance so the acks
might be fine (thousands vs. tens of thousands/sec)
--
Pekka Pietikainen
^ permalink raw reply
* Re: Please pull 'upstream-davem' branch of wireless-2.6
From: David Miller @ 2007-10-03 21:52 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20071003141051.GA5790-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
From: "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>
Date: Wed, 3 Oct 2007 10:10:51 -0400
> So I'm not sure what happened for you. But I think it must have been
> some other anomaly.
Ok, I'll take some detailed notes next time it happens so we can
figure out why :-)
^ permalink raw reply
* Re: [PATCH] net: fix race in process_backlog
From: David Miller @ 2007-10-03 21:58 UTC (permalink / raw)
To: a.p.zijlstra; +Cc: linux-kernel, netdev, shemminger, jdike
In-Reply-To: <1191426293.5599.27.camel@lappy>
From: Peter Zijlstra <a.p.zijlstra@chello.nl>
Date: Wed, 03 Oct 2007 17:44:53 +0200
> Index: linux-2.6/net/core/dev.c
> ===================================================================
> --- linux-2.6.orig/net/core/dev.c
> +++ linux-2.6/net/core/dev.c
> @@ -2095,11 +2095,11 @@ static int process_backlog(struct napi_s
>
> local_irq_disable();
> skb = __skb_dequeue(&queue->input_pkt_queue);
> - local_irq_enable();
> if (!skb) {
> - napi_complete(napi);
> + __napi_complete(napi);
> break;
> }
> + local_irq_enable();
What re-enables interrupts in the !skb path?
^ permalink raw reply
* Re: [PATCH] net: fix race in process_backlog
From: Stephen Hemminger @ 2007-10-03 22:05 UTC (permalink / raw)
To: David Miller; +Cc: a.p.zijlstra, linux-kernel, netdev, jdike
In-Reply-To: <20071003.145807.111205634.davem@davemloft.net>
On Wed, 03 Oct 2007 14:58:07 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date: Wed, 03 Oct 2007 17:44:53 +0200
>
> > Index: linux-2.6/net/core/dev.c
> > ===================================================================
> > --- linux-2.6.orig/net/core/dev.c
> > +++ linux-2.6/net/core/dev.c
> > @@ -2095,11 +2095,11 @@ static int process_backlog(struct napi_s
> >
> > local_irq_disable();
> > skb = __skb_dequeue(&queue->input_pkt_queue);
> > - local_irq_enable();
> > if (!skb) {
> > - napi_complete(napi);
> > + __napi_complete(napi);
> > break;
> > }
> > + local_irq_enable();
>
> What re-enables interrupts in the !skb path?
This looks like a better fix. the irq_enable is needed in both cases.
--- a/net/core/dev.c 2007-09-27 07:19:10.000000000 -0700
+++ b/net/core/dev.c 2007-10-03 15:03:54.000000000 -0700
@@ -2077,12 +2077,14 @@ static int process_backlog(struct napi_s
local_irq_disable();
skb = __skb_dequeue(&queue->input_pkt_queue);
- local_irq_enable();
if (!skb) {
- napi_complete(napi);
+ __napi_complete(napi);
+ local_irq_enable();
break;
}
+ local_irq_enable();
+
dev = skb->dev;
netif_receive_skb(skb);
--
Stephen Hemminger <shemminger@linux-foundation.org>
^ permalink raw reply
* Re: [git patches] net driver updates
From: David Miller @ 2007-10-03 22:33 UTC (permalink / raw)
To: jeff; +Cc: netdev, linux-kernel
In-Reply-To: <20071003183916.GA28437@havoc.gtf.org>
From: Jeff Garzik <jeff@garzik.org>
Date: Wed, 3 Oct 2007 14:39:16 -0400
>
> Normally I wait a day or two between pushes, to queue up patches and
> also to avoid annoying my upstream :) But this includes a couple fixes
> I felt should be upstreamed sooner rather than later.
>
> Please pull from 'upstream' branch of
> master.kernel.org:/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git upstream
Pulled, thanks Jeff!
^ permalink raw reply
* Re: [PATCH v4] qe: miscellaneous code improvements and fixes to the QE library
From: Kumar Gala @ 2007-10-03 22:41 UTC (permalink / raw)
To: Timur Tabi; +Cc: Stephen Hemminger, linuxppc-dev, netdev
In-Reply-To: <4703D8B9.5080006@freescale.com>
On Oct 3, 2007, at 1:00 PM, Timur Tabi wrote:
> Stephen Hemminger wrote:
>
>> Separate the changes into individual patches to allow for better
>> comment/review
>> and bisection in case of regression.
>
> That would be too difficult. Some of the changes are single lines,
> and this patch has already been approved -- I just cross-posted to
> netdev because I made a few ucc_geth changes that can't be
> docoupled from the powerpc changes. A series of 18 patches would
> just be convoluted.
Normally I would agree, but at this point I'm not going to gripe too
much about it.
- k
^ permalink raw reply
* [PATCH] r8169: revert part of 6dccd16b7c2703e8bbf8bca62b5cf248332afbe2
From: Francois Romieu @ 2007-10-03 22:42 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev, jeti, David Miller
The 8169/8110SC currently announces itself as:
[...]
eth0: RTL8169sc/8110sc at 0x........, ..:..:..:..:..:.., XID 18000000 IRQ ..
^^^^^^^^
It uses RTL_GIGA_MAC_VER_05 and this part of the changeset can cut
its performance by a factor of 2~2.5 as reported by Timo.
(the driver includes code just before the hunk to write the ChipCmd
register when mac_version == RTL_GIGA_MAC_VER_0[1-4])
Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
Cc: Timo Jantunen <jeti@welho.com>
---
drivers/net/r8169.c | 16 +++++++++++++---
1 files changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index c921ec3..c76dd29 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -1918,7 +1918,11 @@ static void rtl_hw_start_8169(struct net_device *dev)
rtl_set_rx_max_size(ioaddr);
- rtl_set_rx_tx_config_registers(tp);
+ if ((tp->mac_version == RTL_GIGA_MAC_VER_01) ||
+ (tp->mac_version == RTL_GIGA_MAC_VER_02) ||
+ (tp->mac_version == RTL_GIGA_MAC_VER_03) ||
+ (tp->mac_version == RTL_GIGA_MAC_VER_04))
+ rtl_set_rx_tx_config_registers(tp);
tp->cp_cmd |= rtl_rw_cpluscmd(ioaddr) | PCIMulRW;
@@ -1941,6 +1945,14 @@ static void rtl_hw_start_8169(struct net_device *dev)
rtl_set_rx_tx_desc_registers(tp, ioaddr);
+ if ((tp->mac_version != RTL_GIGA_MAC_VER_01) &&
+ (tp->mac_version != RTL_GIGA_MAC_VER_02) &&
+ (tp->mac_version != RTL_GIGA_MAC_VER_03) &&
+ (tp->mac_version != RTL_GIGA_MAC_VER_04)) {
+ RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
+ rtl_set_rx_tx_config_registers(tp);
+ }
+
RTL_W8(Cfg9346, Cfg9346_Lock);
/* Initially a 10 us delay. Turned it into a PCI commit. - FR */
@@ -1955,8 +1967,6 @@ static void rtl_hw_start_8169(struct net_device *dev)
/* Enable all known interrupts by setting the interrupt mask. */
RTL_W16(IntrMask, tp->intr_event);
-
- RTL_W8(ChipCmd, CmdTxEnb | CmdRxEnb);
}
static void rtl_hw_start_8168(struct net_device *dev)
--
1.5.3.2
^ permalink raw reply related
* Re: [PATCH] net: fix race in process_backlog
From: David Miller @ 2007-10-03 23:39 UTC (permalink / raw)
To: shemminger; +Cc: a.p.zijlstra, linux-kernel, netdev, jdike
In-Reply-To: <20071003150519.1fd206a8@freepuppy.rosehill>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Wed, 3 Oct 2007 15:05:19 -0700
> On Wed, 03 Oct 2007 14:58:07 -0700 (PDT)
> David Miller <davem@davemloft.net> wrote:
>
> > From: Peter Zijlstra <a.p.zijlstra@chello.nl>
> > Date: Wed, 03 Oct 2007 17:44:53 +0200
> >
> > > Index: linux-2.6/net/core/dev.c
> > > ===================================================================
> > > --- linux-2.6.orig/net/core/dev.c
> > > +++ linux-2.6/net/core/dev.c
> > > @@ -2095,11 +2095,11 @@ static int process_backlog(struct napi_s
> > >
> > > local_irq_disable();
> > > skb = __skb_dequeue(&queue->input_pkt_queue);
> > > - local_irq_enable();
> > > if (!skb) {
> > > - napi_complete(napi);
> > > + __napi_complete(napi);
> > > break;
> > > }
> > > + local_irq_enable();
> >
> > What re-enables interrupts in the !skb path?
>
> This looks like a better fix. the irq_enable is needed in both cases.
Yep, applied, thanks Peter and Stephen.
^ permalink raw reply
* Re: [patch 3/3] git-net: sctp build fix (not for applying)
From: David Miller @ 2007-10-03 23:45 UTC (permalink / raw)
To: vladislav.yasevich; +Cc: akpm, netdev
In-Reply-To: <47039E3F.9020305@hp.com>
From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Wed, 03 Oct 2007 09:50:55 -0400
> akpm@linux-foundation.org wrote:
> > From: Andrew Morton <akpm@linux-foundation.org>
> >
> > net/sctp/sm_statetable.c:551: error: 'sctp_sf_tabort_8_4_8' undeclared here (not in a function)
> >
>
> Andrew, is the a result of the merge of net-2.6.24 with net-2.6?
Actually, it is a result of merging with Linus's tree since your SCTP
bits were there already, that's why Andrew hit this.
> That's the only way I see this happening.
Right.
I'll resolve this cleanly as I rebase net-2.6.24 today.
^ permalink raw reply
* net-2.6.24 rebased
From: David Miller @ 2007-10-04 5:07 UTC (permalink / raw)
To: netdev; +Cc: akpm
Available as usual at:
kernel.org:/pub/scm/linux/kernel/git/davem/net-2.6.24.git
I resolved the SCTP and network driver conflicts that come standard
with a rebase to Linus's current tree.
We're up to 700 changesets and an 8.7 MB patch, w00t!
I've been using it for an hour or so on my workstation so something
works. It also passes an allmodconfig build on sparc64.
Either later tonight or some time tomorrow I'll start hitting the
patch backlog in my inbox.
Thanks.
^ permalink raw reply
* Re: Blackfin Ethernet MAC driver compile error
From: Bryan Wu @ 2007-10-04 9:12 UTC (permalink / raw)
To: Kalle Pokki; +Cc: linux-kernel, netdev, bryan.wu
In-Reply-To: <a425f86c0710020430o3507eeb2veb104dc936f230dd@mail.gmail.com>
On Tue, 2007-10-02 at 19:30 +0800, Kalle Pokki wrote:
> The Blackfin Ethernet MAC driver does not compile. It seems the driver
> is
> missing some pinmux defines.
>
> CC drivers/net/bfin_mac.o
> drivers/net/bfin_mac.c: In function 'setup_pin_mux':
> drivers/net/bfin_mac.c:275: error: 'P_MII0' undeclared (first use in
> this function)
> drivers/net/bfin_mac.c:275: error: (Each undeclared identifier is
> reported only once
> drivers/net/bfin_mac.c:275: error: for each function it appears in.)
> drivers/net/bfin_mac.c:279: error: implicit declaration of function
> 'peripheral_request_list'
> drivers/net/bfin_mac.c:285: error: implicit declaration of function
> 'peripheral_free_list'
> make[2]: *** [drivers/net/bfin_mac.o] Error 1
> make[1]: *** [drivers/net] Error 2
> make: *** [drivers] Error 2
>
Sorry for missing the pinmux patches.
After Linus's git-pull, it should be fixed in the latest Linus mainline
git tree.
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=66b1f1a982bf4dbad9fa0de25b8d95c4936f05c4
Thanks a lot
-Bryan
^ permalink raw reply
* Re: Blackfin Ethernet MAC driver compile error
From: Bryan Wu @ 2007-10-04 9:20 UTC (permalink / raw)
To: Robin Getz; +Cc: Kalle Pokki, linux-kernel, netdev, bryan.wu
In-Reply-To: <200710021136.38514.rgetz@blackfin.uclinux.org>
On Tue, 2007-10-02 at 23:36 +0800, Robin Getz wrote:
> On Tue 2 Oct 2007 07:30, Kalle Pokki pondered:
> > The Blackfin Ethernet MAC driver does not compile. It seems the
> driver is
> > missing some pinmux defines.
> >
> > CC drivers/net/bfin_mac.o
> > drivers/net/bfin_mac.c: In function 'setup_pin_mux':
> > drivers/net/bfin_mac.c:275: error: 'P_MII0' undeclared (first use
> in
> > this function)
> > drivers/net/bfin_mac.c:275: error: (Each undeclared identifier is
> > reported only once
> > drivers/net/bfin_mac.c:275: error: for each function it appears in.)
>
> Unfortunately, Bryan is out for the week, so he can't update things..
I am here, although it is National Day holiday in China and I am not in
office.
>
> The updates to include/asm-blackfin/mach-bf537/portmux.h seem to have
> been
> missing from Bryan's git tree, and therefore were not pushed to
> Linus's tree.
>
Yes, it was not merged to Linus's tree almost one month ago.
But now Linus git-pull it from my git-tree
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=66b1f1a982bf4dbad9fa0de25b8d95c4936f05c4
> Sigh. - Sorry about that.
>
> When Bryan gets back, he will update things, but for now, this might
> work. (I
> don't know if there are any other interdependencies).
>
Sorry again, some important core API changes should be merged before
drivers.
>
> --- portmux.h 2007-10-02 11:18:34.000000000 -0400
> +++ ./linux-2.6.x/include/asm-blackfin/mach-bf537/portmux.h
> 2007-08-18
> 19:37:22.000000000 -0400
> @@ -99,11 +99,44 @@
> #define P_SPORT0_DRPRI (P_DEFINED | P_IDENT(PORT_PJ8) | P_FUNCT(0))
> #define P_SPORT0_TSCLK (P_DEFINED | P_IDENT(PORT_PJ9) | P_FUNCT(0))
> #define P_SPORT0_TFS (P_DEFINED | P_IDENT(PORT_PJ10) | P_FUNCT(0))
> -#define P_SPORT0_DTPRI (P_DEFINED | P_IDENT(PORT_PJ11) | P_FUNCT(1))
> +#define P_SPORT0_DTPRI (P_DEFINED | P_IDENT(PORT_PJ11) | P_FUNCT(0))
> #define P_CAN0_RX (P_DEFINED | P_IDENT(PORT_PJ4) | P_FUNCT(1))
> #define P_CAN0_TX (P_DEFINED | P_IDENT(PORT_PJ5) | P_FUNCT(1))
> #define P_SPI0_SSEL3 (P_DEFINED | P_IDENT(PORT_PJ10) | P_FUNCT(1))
> #define P_SPI0_SSEL2 (P_DEFINED | P_IDENT(PORT_PJ11) | P_FUNCT(1))
> #define P_SPI0_SSEL7 (P_DEFINED | P_IDENT(PORT_PJ5) | P_FUNCT(2))
>
> -#endif /* _MACH_PORTMUX_H_ */
> +#define P_MII0 {\
> + P_MII0_ETxD0, \
> + P_MII0_ETxD1, \
> + P_MII0_ETxD2, \
> + P_MII0_ETxD3, \
> + P_MII0_ETxEN, \
> + P_MII0_TxCLK, \
> + P_MII0_PHYINT, \
> + P_MII0_COL, \
> + P_MII0_ERxD0, \
> + P_MII0_ERxD1, \
> + P_MII0_ERxD2, \
> + P_MII0_ERxD3, \
> + P_MII0_ERxDV, \
> + P_MII0_ERxCLK, \
> + P_MII0_ERxER, \
> + P_MII0_CRS, \
> + P_MDC, \
> + P_MDIO, 0}
> +
> +
> +#define P_RMII0 {\
> + P_MII0_ETxD0, \
> + P_MII0_ETxD1, \
> + P_MII0_ETxEN, \
> + P_MII0_ERxD0, \
> + P_MII0_ERxD1, \
> + P_MII0_ERxER, \
> + P_RMII0_REF_CLK, \
> + P_RMII0_MDINT, \
> + P_RMII0_CRS_DV, \
> + P_MDC, \
> + P_MDIO, 0}
> +#endif /* _MACH_PORTMUX_H_ */
>
> > drivers/net/bfin_mac.c:279: error: implicit declaration of function
> > 'peripheral_request_list'
> > drivers/net/bfin_mac.c:285: error: implicit declaration of function
> > 'peripheral_free_list'
>
> Arg.
>
> This was in an arch/blackfin/kernel/bfin_gpio.c update that hasn't
> seem to
> made it either.
>
> In the future - we will make sure driver submissions are done in the
> correct
> order - so the proper infrastructure is in the tree before the driver
> is
> submitted. - Sorry.
>
I will try my best to keep them in order or fix it as soon as possible,
-:)))
[!snip!]
>
Thanks
-Bryan
>
^ permalink raw reply
* Re: [PATCH net-2.6.24 0/3]: More TCP fixes
From: Ilpo Järvinen @ 2007-10-04 10:13 UTC (permalink / raw)
To: Cedric Le Goater; +Cc: David Miller, Netdev
In-Reply-To: <4703B16C.2000106@fr.ibm.com>
On Wed, 3 Oct 2007, Cedric Le Goater wrote:
> Cedric Le Goater wrote:
> >
> > Below are the messages I got on 2) right after running ketchup (which does
> > a wget www.kernel.org)
Oops, those tcp_fragment WARNINGs in the other mail were due to bug in
the debug patch as it called verify too early in there (before queue was
adjusted, no wonder it finds state inconsistent at that point, fixed that)...
...So please discard all old debug patches, they're all broken in this
respect... :-(
> > not a warning on 1) with your extra verbose patch.
>
> bummer, I got this one on 1) :(
>
> WARNING: at /home/legoater/linux/net-2.6.24.git/net/ipv4/tcp_input.c:2325 tcp_fastretrans_alert()
> Call Trace:
> <IRQ> [<ffffffff8022ddb6>] __wake_up+0x1f/0x4c
> [<ffffffff803fd9d3>] tcp_ack+0xcee/0x18ac
> [<ffffffff80400764>] tcp_rcv_established+0x61f/0x6df
...I just wonder why that's the first place where it occurs... Can you try
the debug patch below (fixed verify place in tcp_fragment/collapse, added
some of them to narrow it down, and handled GSO more user friendly way in
the printout). Put it on top of those three patches (mm should be fine :-)).
...I wish the verify triggers way before the fastretrans trap (for some
reason it didn't do that in the quoted trace, maybe I had some verifys
missing in that old patch or something)...
--
i.
include/net/tcp.h | 3 +
net/ipv4/tcp_input.c | 41 +++++++++++++++---
net/ipv4/tcp_ipv4.c | 113 +++++++++++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_output.c | 4 +-
4 files changed, 154 insertions(+), 7 deletions(-)
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 991ccdc..54a0d91 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -43,6 +43,9 @@
#include <linux/seq_file.h>
+extern void tcp_verify_fackets(struct sock *sk);
+extern void tcp_print_queue(struct sock *sk);
+
extern struct inet_hashinfo tcp_hashinfo;
extern atomic_t tcp_orphan_count;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 87c9ef5..7707b1d 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -1140,7 +1140,7 @@ static int tcp_check_dsack(struct tcp_sock *tp, struct sk_buff *ack_skb,
return dup_sack;
}
-static int
+int
tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_una)
{
const struct inet_connection_sock *icsk = inet_csk(sk);
@@ -1159,6 +1159,9 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
int i;
int first_sack_index;
+ if (WARN_ON(!tp->sacked_out && tp->fackets_out))
+ tcp_print_queue(sk);
+
if (!tp->sacked_out) {
if (WARN_ON(tp->fackets_out))
tp->fackets_out = 0;
@@ -1421,6 +1424,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
}
}
}
+ tcp_verify_fackets(sk);
/* Check for lost retransmit. This superb idea is
* borrowed from "ratehalving". Event "C".
@@ -1633,13 +1637,14 @@ void tcp_enter_frto(struct sock *sk)
tcp_set_ca_state(sk, TCP_CA_Disorder);
tp->high_seq = tp->snd_nxt;
tp->frto_counter = 1;
+ tcp_verify_fackets(sk);
}
/* Enter Loss state after F-RTO was applied. Dupack arrived after RTO,
* which indicates that we should follow the traditional RTO recovery,
* i.e. mark everything lost and do go-back-N retransmission.
*/
-static void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag)
+void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag)
{
struct tcp_sock *tp = tcp_sk(sk);
struct sk_buff *skb;
@@ -1676,6 +1681,7 @@ static void tcp_enter_frto_loss(struct sock *sk, int allowed_segments, int flag)
}
}
tcp_verify_left_out(tp);
+ tcp_verify_fackets(sk);
tp->snd_cwnd = tcp_packets_in_flight(tp) + allowed_segments;
tp->snd_cwnd_cnt = 0;
@@ -1754,6 +1760,7 @@ void tcp_enter_loss(struct sock *sk, int how)
}
}
tcp_verify_left_out(tp);
+ tcp_verify_fackets(sk);
tp->reordering = min_t(unsigned int, tp->reordering,
sysctl_tcp_reordering);
@@ -2309,7 +2316,7 @@ static void tcp_mtup_probe_success(struct sock *sk, struct sk_buff *skb)
* It does _not_ decide what to send, it is made in function
* tcp_xmit_retransmit_queue().
*/
-static void
+void
tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
{
struct inet_connection_sock *icsk = inet_csk(sk);
@@ -2318,13 +2325,20 @@ tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
int do_lost = is_dupack || ((flag&FLAG_DATA_SACKED) &&
(tp->fackets_out > tp->reordering));
+ tcp_verify_fackets(sk);
+
/* Some technical things:
* 1. Reno does not count dupacks (sacked_out) automatically. */
- if (!tp->packets_out)
+ if (!tp->packets_out) {
+ WARN_ON(tcp_is_sack(tp) && tp->sacked_out);
tp->sacked_out = 0;
+ }
- if (WARN_ON(!tp->sacked_out && tp->fackets_out))
+ if (WARN_ON(!tp->sacked_out && tp->fackets_out)) {
+ printk(KERN_ERR "TCP %d\n", tcp_is_reno(tp));
+ tcp_print_queue(sk);
tp->fackets_out = 0;
+ }
/* Now state machine starts.
* A. ECE, hence prohibit cwnd undoing, the reduction is required. */
@@ -2334,6 +2348,8 @@ tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
/* B. In all the states check for reneging SACKs. */
if (tp->sacked_out && tcp_check_sack_reneging(sk))
return;
+
+ tcp_verify_fackets(sk);
/* C. Process data loss notification, provided it is valid. */
if ((flag&FLAG_DATA_LOST) &&
@@ -2390,6 +2406,8 @@ tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
}
}
+ tcp_verify_fackets(sk);
+
/* F. Process state. */
switch (icsk->icsk_ca_state) {
case TCP_CA_Recovery:
@@ -2405,6 +2423,7 @@ tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
if (!tcp_try_undo_loss(sk)) {
tcp_moderate_cwnd(tp);
tcp_xmit_retransmit_queue(sk);
+ tcp_verify_fackets(sk);
return;
}
if (icsk->icsk_ca_state != TCP_CA_Open)
@@ -2423,6 +2442,7 @@ tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
if (!tcp_time_to_recover(sk)) {
tcp_try_to_open(sk, flag);
+ tcp_verify_fackets(sk);
return;
}
@@ -2434,6 +2454,7 @@ tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
/* Restores the reduction we did in tcp_mtup_probe() */
tp->snd_cwnd++;
tcp_simple_retransmit(sk);
+ tcp_verify_fackets(sk);
return;
}
@@ -2460,11 +2481,13 @@ tcp_fastretrans_alert(struct sock *sk, int pkts_acked, int flag)
tp->snd_cwnd_cnt = 0;
tcp_set_ca_state(sk, TCP_CA_Recovery);
}
+ tcp_verify_fackets(sk);
if (do_lost || tcp_head_timedout(sk))
tcp_update_scoreboard(sk);
tcp_cwnd_down(sk, flag);
tcp_xmit_retransmit_queue(sk);
+ tcp_verify_fackets(sk);
}
/* Read draft-ietf-tcplw-high-performance before mucking
@@ -2573,7 +2596,7 @@ static u32 tcp_tso_acked(struct sock *sk, struct sk_buff *skb)
* is before the ack sequence we can discard it as it's confirmed to have
* arrived at the other end.
*/
-static int tcp_clean_rtx_queue(struct sock *sk, s32 *seq_rtt_p)
+int tcp_clean_rtx_queue(struct sock *sk, s32 *seq_rtt_p)
{
struct tcp_sock *tp = tcp_sk(sk);
const struct inet_connection_sock *icsk = inet_csk(sk);
@@ -2695,6 +2718,8 @@ static int tcp_clean_rtx_queue(struct sock *sk, s32 *seq_rtt_p)
ca_ops->pkts_acked(sk, pkts_acked, rtt_us);
}
}
+ tcp_verify_fackets(sk);
+
#if FASTRETRANS_DEBUG > 0
BUG_TRAP((int)tp->sacked_out >= 0);
@@ -3011,6 +3036,7 @@ static int tcp_ack(struct sock *sk, struct sk_buff *skb, int flag)
tcp_ca_event(sk, CA_EVENT_SLOW_ACK);
}
+ tcp_verify_fackets(sk);
/* We passed data and got it acked, remove any soft error
* log. Something worked...
*/
@@ -3031,6 +3057,8 @@ static int tcp_ack(struct sock *sk, struct sk_buff *skb, int flag)
if (tp->frto_counter)
frto_cwnd = tcp_process_frto(sk, flag);
+ tcp_verify_fackets(sk);
+
if (tcp_ack_is_dubious(sk, flag)) {
/* Advance CWND, if state allows this. */
if ((flag & FLAG_DATA_ACKED) && !frto_cwnd &&
@@ -3040,6 +3068,7 @@ static int tcp_ack(struct sock *sk, struct sk_buff *skb, int flag)
} else {
if ((flag & FLAG_DATA_ACKED) && !frto_cwnd)
tcp_cong_avoid(sk, ack, prior_in_flight, 1);
+ tcp_verify_fackets(sk);
}
if ((flag & FLAG_FORWARD_PROGRESS) || !(flag&FLAG_NOT_DUP))
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 7fed0a6..6a450a7 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -108,6 +108,119 @@ struct inet_hashinfo __cacheline_aligned tcp_hashinfo = {
.lhash_wait = __WAIT_QUEUE_HEAD_INITIALIZER(tcp_hashinfo.lhash_wait),
};
+void tcp_print_queue(struct sock *sk)
+{
+ struct tcp_sock *tp = tcp_sk(sk);
+ struct sk_buff *skb;
+ char s[50+1];
+ char p[50+1];
+ int idx = 0, i;
+ u32 hs = tp->highest_sack;
+
+ if (!tp->sacked_out)
+ hs = tp->snd_una;
+
+ tcp_for_write_queue(skb, sk) {
+ if (skb == tcp_send_head(sk))
+ break;
+
+ for (i = 0; i < tcp_skb_pcount(skb); i++) {
+ if (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED) {
+ if (i)
+ s[idx] = 'g';
+ else if (tcp_skb_pcount(skb) > 1)
+ s[idx] = 'G';
+ else if (skb->len < tp->mss_cache)
+ s[idx] = 's';
+ else
+ s[idx] = 'S';
+ } else {
+ if (i)
+ s[idx] = '-';
+ else
+ s[idx] = '+';
+ }
+ p[idx] = ' ';
+ if (!i) {
+ if ((TCP_SKB_CB(skb)->seq == hs) && (tp->fastpath_skb_hint == skb))
+ p[idx] = 'x';
+ else if (tp->fastpath_skb_hint == skb)
+ p[idx] = 'f';
+ else if (TCP_SKB_CB(skb)->seq == hs)
+ p[idx] = 'h';
+ }
+
+ if (++idx >= 50) {
+ s[idx] = 0;
+ p[idx] = 0;
+ printk(KERN_ERR "TCP wq(s) %s\n", s);
+ printk(KERN_ERR "TCP wq(i) %s\n", p);
+ idx = 0;
+ }
+ }
+ }
+ if (idx) {
+ s[idx] = '<';
+ s[idx+1] = 0;
+ p[idx] = '<';
+ p[idx+1] = 0;
+ printk(KERN_ERR "TCP wq(s) %s\n", s);
+ printk(KERN_ERR "TCP wq(i) %s\n", p);
+ }
+ printk(KERN_ERR "s%u f%u (%u) p%u seq: su%u hs%u sn%u (%u)\n",
+ tp->sacked_out, tp->fackets_out, tp->fastpath_cnt_hint,
+ tp->packets_out,
+ tp->snd_una, tp->highest_sack, tp->snd_nxt,
+ ((tp->fastpath_skb_hint == NULL) ? 0 :
+ TCP_SKB_CB(tp->fastpath_skb_hint)->seq));
+}
+
+void tcp_verify_fackets(struct sock *sk)
+{
+ struct tcp_sock *tp = tcp_sk(sk);
+ struct sk_buff *skb;
+ u32 fackets = 0;
+ int hisack_valid = 0;
+ int err = 0;
+
+ if (tcp_is_reno(tp))
+ return;
+
+ if (!tp->sacked_out) {
+ if (WARN_ON(tp->fackets_out))
+ err = 1;
+ else if (tp->fastpath_skb_hint == NULL)
+ return;
+ }
+
+ /* ...expensive processing here... */
+ tcp_for_write_queue(skb, sk) {
+ if (skb == tcp_send_head(sk))
+ break;
+
+ if (tp->sacked_out && (TCP_SKB_CB(skb)->seq == tp->highest_sack)) {
+ hisack_valid = 1;
+ if (WARN_ON(tp->fackets_out != fackets + tcp_skb_pcount(skb)))
+ err = 1;
+ }
+
+ if (skb == tp->fastpath_skb_hint)
+ if (WARN_ON(fackets != tp->fastpath_cnt_hint))
+ err = 1;
+
+ if (WARN_ON((fackets > tp->fackets_out) && (TCP_SKB_CB(skb)->sacked & TCPCB_SACKED_ACKED)))
+ err = 1;
+
+ fackets += tcp_skb_pcount(skb);
+ }
+
+ if (WARN_ON(tp->sacked_out && !hisack_valid))
+ err = 1;
+
+ if (err)
+ tcp_print_queue(sk);
+}
+
static int tcp_v4_get_port(struct sock *sk, unsigned short snum)
{
return inet_csk_get_port(&tcp_hashinfo, sk, snum,
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c
index 5329675..8a2917f 100644
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -778,6 +778,7 @@ int tcp_fragment(struct sock *sk, struct sk_buff *skb, u32 len, unsigned int mss
/* Link BUFF into the send queue. */
skb_header_release(buff);
tcp_insert_write_queue_after(skb, buff, sk);
+ tcp_verify_fackets(sk);
return 0;
}
@@ -1688,7 +1689,7 @@ u32 __tcp_select_window(struct sock *sk)
}
/* Attempt to collapse two adjacent SKB's during retransmission. */
-static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int mss_now)
+void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int mss_now)
{
struct tcp_sock *tp = tcp_sk(sk);
struct sk_buff *next_skb = tcp_write_queue_next(sk, skb);
@@ -1767,6 +1768,7 @@ static void tcp_retrans_try_collapse(struct sock *sk, struct sk_buff *skb, int m
}
sk_stream_free_skb(sk, next_skb);
+ tcp_verify_fackets(sk);
}
}
--
1.5.0.6
^ permalink raw reply related
* [RFC][PATCH 1/2] TCP: fix lost retransmit detection
From: TAKANO Ryousei @ 2007-10-04 9:45 UTC (permalink / raw)
To: netdev; +Cc: y-kodama
This patch allows to detect loss of retransmitted packets more
accurately by using the highest end sequence number among SACK
blocks. Before the retransmission queue is scanned, the highest
end sequence number (high_end_seq) is retrieved, and this value
is compared with the ack_seq of each packet.
Signed-off-by: Ryousei Takano <takano-ryousei@aist.go.jp>
Signed-off-by: Yuetsu Kodama <y-kodama@aist.go.jp>
---
net/ipv4/tcp_input.c | 14 +++++++++++---
1 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bbad2cd..12db4b3 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -978,6 +978,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
int cached_fack_count;
int i;
int first_sack_index;
+ __u32 high_end_seq;
if (!tp->sacked_out)
tp->fackets_out = 0;
@@ -1012,6 +1013,14 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (before(TCP_SKB_CB(ack_skb)->ack_seq, prior_snd_una - tp->max_window))
return 0;
+ /* Retrieve the highest end_seq among SACK blocks. */
+ high_end_seq = ntohl(sp[0].end_seq);
+ for (i = 1; i < num_sacks; i++) {
+ __u32 end_seq = ntohl(sp[i].end_seq);
+ if (after(end_seq, high_end_seq))
+ high_end_seq = end_seq;
+ }
+
/* SACK fastpath:
* if the only SACK change is the increase of the end_seq of
* the first block then only apply that SACK block
@@ -1161,9 +1170,8 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
}
if ((sacked&TCPCB_SACKED_RETRANS) &&
- after(end_seq, TCP_SKB_CB(skb)->ack_seq) &&
- (!lost_retrans || after(end_seq, lost_retrans)))
- lost_retrans = end_seq;
+ after(high_end_seq, TCP_SKB_CB(skb)->ack_seq))
+ lost_retrans = high_end_seq;
if (!in_sack)
continue;
--
1.5.2.4
^ permalink raw reply related
* [RFC][PATCH 2.6.23-rc9 0/2] detection of loss of retransmitted packets
From: TAKANO Ryousei @ 2007-10-04 9:44 UTC (permalink / raw)
To: netdev; +Cc: y-kodama
Hi all,
We found a performance problem which occurs in heavy packet loss
conditions. It seems there is a problem in detecting loss of
retransmitted packets.
In the retransmission queue, status of sent packets are registered.
When a packet is retransmitted, it is so marked, and snd_nxt (sequence
number of the next new (non-retransmission) packet to be sent) at that
moment is registered as ack_seq. A retransmitted packet is lost if it
is not SACKed, and its ack_seq is smaller than the sequence number of
any SACKed packet.
An ACK packet can have up to three SACK blocks. A SACK block has a
"start sequence number (start_seq)" and an "end sequence number
(end_seq)" of received packets. In the current implementation of
tcp_sacktag_write_queue(), if an ACK packet has multiple SACK blocks,
the SACK blocks are sorted by the start_seq in an ascending order, and
processed in the order. For scoreboarding packets in retransmission
queue, the queue is scanned from the the snd_una (the lowest sequence
number of not yet ACKed packets) to the end_seq of the SACK block. To
optimize the scanning process, the next SACK block is processed not
from the snd_una but from the end_seq of the previously processed SACK
block. In the current implementation, for detecting the loss of
retransmitted packets, the ack_seq of a retransmitted packet is
compared with the end_seq of each SACK block during the scoreboarding.
Therefore, a retransmitted packet which ack_seq is smaller than the
end_seq of the last SACK block but larger than that of the currently
being processed SACK block can not be detected as lost.
Such undetected loss may eventually cause an RTO and performance may
be degraded.
PATCH #1 fixes this problem by comparing the the ack_seq with the
largest end_seq of the SACK blocks.
In addition, some of SACK blocks in an ACK packet may be already
reported in preceding ACK packets. PATCH #2 optimizes processing by
skipping such already reported SACK blocks. Usually, only the first
SACK block of an ACK packet is the new one to be processed.
Therefore, in most cases, applying PATCH #2 also solves the problem.
However, to ensure accurate processing in case there are multiple
new SACK blocks in an ACK packet, PATCH #2 should be applied in
conjunction with PATCH #1.
The experimental network is as follows:
Node A ----> Router -------> Delay -------> Node B
(Policing rate: emulator
500Mbps) (RTT: 20ms)
You can find the detail of our experimental setting at
http://projects.gtrc.aist.go.jp/gnet/sack-bug.html
We transferred 1 GByte of data from Node A to Node B for ten times.
Here is the performance comparison of the cases with and without
these patches.
Ave. goodput Ave. RTO
2.6.22 376 Mbps 26
PATCH#1 481 Mbps 0
PATCH#2 483 Mbps 0
In the vanilla kernel, several RTOs (TCPTimeouts + TCPSackRecoveryFail)
occur. On the other hand, our patches eliminate RTOs and improve the
average goodput by 28%.
Any comments and ideas would be appreciated.
Regards,
Ryousei Takano
^ permalink raw reply
* [RFC][PATCH 2/2] TCP: skip processing cached SACK blocks
From: TAKANO Ryousei @ 2007-10-04 9:44 UTC (permalink / raw)
To: netdev; +Cc: y-kodama
This patch allows to process only newly reported SACK blocks at the
sender side. An ACK packet contains up to three SACK blocks, and some
of them may be already reported and processed blocks. This patch
prevents processing of such already processed SACK blocks.
Signed-off-by: Ryousei Takano <takano-ryousei@aist.go.jp>
Signed-off-by: Yuetsu Kodama <y-kodama@aist.go.jp>
---
net/ipv4/tcp_input.c | 24 ++++++++++++++++++++++++
1 files changed, 24 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bbad2cd..9615fc9 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -978,6 +978,7 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
int cached_fack_count;
int i;
int first_sack_index;
+ u8 sack_block_skip[4] = {0,0,0,0};
if (!tp->sacked_out)
tp->fackets_out = 0;
@@ -1012,6 +1013,21 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (before(TCP_SKB_CB(ack_skb)->ack_seq, prior_snd_una - tp->max_window))
return 0;
+ /* Skip processing cached SACK blocks. */
+ for (i = 0; i < num_sacks; i++) {
+ __be32 start_seq = sp[i].start_seq;
+ __be32 end_seq = sp[i].end_seq;
+ int j;
+
+ for (j = 0; j < ARRAY_SIZE(tp->recv_sack_cache); j++) {
+ if ((tp->recv_sack_cache[j].start_seq == start_seq) &&
+ (tp->recv_sack_cache[j].end_seq == end_seq)) {
+ sack_block_skip[i] = 1;
+ break;
+ }
+ }
+ }
+
/* SACK fastpath:
* if the only SACK change is the increase of the end_seq of
* the first block then only apply that SACK block
@@ -1051,11 +1067,16 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
if (after(ntohl(sp[j].start_seq),
ntohl(sp[j+1].start_seq))){
struct tcp_sack_block_wire tmp;
+ u8 sbtmp;
tmp = sp[j];
sp[j] = sp[j+1];
sp[j+1] = tmp;
+ sbtmp = sack_block_skip[j];
+ sack_block_skip[j] = sack_block_skip[j+1];
+ sack_block_skip[j+1] = sbtmp;
+
/* Track where the first SACK block goes to */
if (j == first_sack_index)
first_sack_index = j+1;
@@ -1083,6 +1104,9 @@ tcp_sacktag_write_queue(struct sock *sk, struct sk_buff *ack_skb, u32 prior_snd_
int fack_count;
int dup_sack = (found_dup_sack && (i == first_sack_index));
+ if (sack_block_skip[i])
+ continue;
+
skb = cached_skb;
fack_count = cached_fack_count;
--
1.5.2.4
^ permalink raw reply related
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