* Re: SFQ qdisc crashes with limit of 2 packets
From: Patrick McHardy @ 2007-09-18 19:15 UTC (permalink / raw)
To: Chuck Ebbert; +Cc: Netdev, Alexey Kuznetsov
In-Reply-To: <46F0117A.4060807@trash.net>
[-- Attachment #1: Type: text/plain, Size: 1570 bytes --]
Patrick McHardy wrote:
> Never mind, I found the reason. When enqueuing the packet, sfq_enqueue
> contains an off-by-one in the limit check (which IIRC is there for a
> reason, but I can't remember right now) and drops the packet again.
> dev_queue_xmit() calls qdisc_run() anyway and the empty qdisc is
> dequeued, which is not handled by SFQ.
>
> I see three possibilities to fix this (in my preferred order):
>
> 1) figure out why the off-by-one is there, if not needed remove
> 2) don't dequeue qdiscs even once if empty
> 3) check for NULL in sfq_dequeue
>
> So I'll try to remeber why the off-by-one is there ..
OK the off-by-one prevents an out-of-bounds array access, which
would cause a crash itself. Despite what I said above, sfq does
try to handle dequeues while empty, but forgets to update q->tail
when dropping the last packet from the only active queue, probably
because it wasn't expected that the queue length is too small to
queue even a single packet (and that really doesn't make much sense).
So one possibility for fixing this is to update q->tail in sfq_drop
when dropping the last packet, but that would still leave the qdisc
non-functional because of the off-by-one. I chose a different way:
cap the limit at SFQ_DEPTH-1 and remove the off-by-one, which should
have no effect on the max (still 127), but prevents the crash since
we can now queue at least a single packet and q->tail is properly
updated in sfq_dequeue().
CCed Alexey just to be safe, but I think the patch should be fine.
Signed-off-by: Patrick McHardy <kaber@trash.net>
[-- Attachment #2: y --]
[-- Type: text/plain, Size: 1347 bytes --]
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 9579573..cbf8089 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -270,7 +270,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc* sch)
q->tail = x;
}
}
- if (++sch->q.qlen < q->limit-1) {
+ if (++sch->q.qlen < q->limit) {
sch->bstats.bytes += skb->len;
sch->bstats.packets++;
return 0;
@@ -306,7 +306,7 @@ sfq_requeue(struct sk_buff *skb, struct Qdisc* sch)
q->tail = x;
}
}
- if (++sch->q.qlen < q->limit - 1) {
+ if (++sch->q.qlen < q->limit) {
sch->qstats.requeues++;
return 0;
}
@@ -391,10 +391,10 @@ static int sfq_change(struct Qdisc *sch, struct rtattr *opt)
q->quantum = ctl->quantum ? : psched_mtu(sch->dev);
q->perturb_period = ctl->perturb_period*HZ;
if (ctl->limit)
- q->limit = min_t(u32, ctl->limit, SFQ_DEPTH);
+ q->limit = min_t(u32, ctl->limit, SFQ_DEPTH - 1);
qlen = sch->q.qlen;
- while (sch->q.qlen >= q->limit-1)
+ while (sch->q.qlen >= q->limit)
sfq_drop(sch);
qdisc_tree_decrease_qlen(sch, qlen - sch->q.qlen);
@@ -423,7 +423,7 @@ static int sfq_init(struct Qdisc *sch, struct rtattr *opt)
q->dep[i+SFQ_DEPTH].next = i+SFQ_DEPTH;
q->dep[i+SFQ_DEPTH].prev = i+SFQ_DEPTH;
}
- q->limit = SFQ_DEPTH;
+ q->limit = SFQ_DEPTH - 1;
q->max_depth = 0;
q->tail = SFQ_DEPTH;
if (opt == NULL) {
^ permalink raw reply related
* Re: bnx2 dirver's firmware images
From: David Miller @ 2007-09-18 19:21 UTC (permalink / raw)
To: mchan; +Cc: vda.linux, linux-kernel, netdev
In-Reply-To: <1190145951.9540.230.camel@dell>
From: "Michael Chan" <mchan@broadcom.com>
Date: Tue, 18 Sep 2007 13:05:51 -0700
> The bnx2 firmware changes quite frequently. A new driver quite often
> requires new firmware to work correctly. Splitting them up makes things
> difficult for the user.
>
> The firmware in tg3 is a lot more mature and I don't expect it to
> change. I think tg3 is better suited for using request_firmware().
Like I said, I think neither should change and the driver should
be fully functional when built statically into the kernel.
:-)
^ permalink raw reply
* Re: bnx2 dirver's firmware images
From: David Miller @ 2007-09-18 19:20 UTC (permalink / raw)
To: hpa; +Cc: mchan, vda.linux, linux-kernel, netdev
In-Reply-To: <46F01BDE.6020902@zytor.com>
From: "H. Peter Anvin" <hpa@zytor.com>
Date: Tue, 18 Sep 2007 11:41:34 -0700
> David Miller wrote:
> >
> > I don't like it because it means people have to setup full initrd's
> > in order to do network booting with such network cards.
> >
>
> klibc could help with that, if there is interest in exploring that
> avenue again.
I appreciate the effort you put into klibc and the offer to
make initrd's easier to build.
But the point is that the initrd shouldn't be necessary in the first
place. There becomes zero point in building these drivers statically
into the kernel, which many of us do specifically to avoid module
loading, initrds, and all that fuss. Because the driver is totally
crippled even though it's been fully built into the main kernel image.
I mean, it's so incredibly stupid and makes kernel development that
much more difficult.
Every new dependency, be it requiring initrd or something else,
is one more barrier added to kernel development.
I really pine for the days where everything was so simple, and initrd
and modules were the odd ball cases, most developers built everything
into their kernel image.
^ permalink raw reply
* Re: [PATCH] phy: export phy_mii_ioctl
From: Jon Smirl @ 2007-09-18 19:17 UTC (permalink / raw)
To: Domen Puncer; +Cc: netdev, linuxppc-embedded, sven
In-Reply-To: <20070918151622.GD32628@nd47.coderock.org>
On 9/18/07, Domen Puncer <domen@coderock.org> wrote:
> More testing and getting it to work properly on Phytec pcm030 would
> be great.
I compiled it as a module:
CC [M] drivers/net/fec_mpc52xx/fec.o
drivers/net/fec_mpc52xx/fec.c:613: warning: 'mpc52xx_fec_mac_setup'
defined but not used
This code needs to be enclosed in "#ifndef MODULE". But why aren't you
using module_param() to make a string parameter and then copy it into
mpc52xx_fec_mac_addr[] if the parameter is not null?
If it is a module param you need to use
fec_mpc52xx_phy.mpc52xx-mac="xxxx" instead of just mpc52xx-mac. The
way it is not you can't use mpc52xx-mac when built as a module.
static int __init mpc52xx_fec_mac_setup(char *mac_address)
{
int i;
u64 val64;
val64 = simple_strtoull(mac_address, NULL, 16);
for (i = 0; i < 6; i++)
mpc52xx_fec_mac_addr[5-i] = val64 >> (i*8);
return 0;
}
__setup("mpc52xx-mac=", mpc52xx_fec_mac_setup);
--
Jon Smirl
jonsmirl@gmail.com
^ permalink raw reply
* Re: new NAPI interface broken
From: David Miller @ 2007-09-18 19:08 UTC (permalink / raw)
To: ossthema; +Cc: shemminger, netdev, themann, raisch
In-Reply-To: <200709181815.45954.ossthema@de.ibm.com>
From: Jan-Bernd Themann <ossthema@de.ibm.com>
Date: Tue, 18 Sep 2007 18:15:45 +0200
> One other thing I observed is that I can not unload the module as the
> ref counter of the eth device is too low. I haven't tracked down the
> source of this problem yet.
This is probably because of the resched device refcounting
bug that Roland Dreier just posted a fix for.
Look for subject "Fix refcounting problem with netif_rx_reschedule()"
I merge that in shortly to net-2.6.24 as well.
^ permalink raw reply
* Please pull 'iwlwifi' branch of wireless-2.6
From: John W. Linville @ 2007-09-18 18:50 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: jeff-o2qLIJkoznsdnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-wireless-u79uwXL29TY76Z2rM5mHXA
Jeff & Dave,
Here it is -- it's big, it's...well...beautiful in its own way...well,
at least it seems to work... :-)
There are some outstanding issues. The driver does more than it
probably should under the covers instead of in the stack, and the
issue of including headers with a "../../mac80211/..." path remains.
Still, I think it would be better to get this mainlined than to keep
it out of stream.
Thanks!
John
---
Patch available here:
http://www.kernel.org/pub/linux/kernel/people/linville/wireless-2.6/iwlwifi/0001-iwlwifi-add-iwlwifi-wireless-drivers.patch
---
The following changes since commit 0d4cbb5e7f60b2f1a4d8b7f6ea4cc264262c7a01:
Linus Torvalds (1):
Linux 2.6.23-rc6
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git iwlwifi
Zhu Yi (1):
iwlwifi: add iwlwifi wireless drivers
MAINTAINERS | 9 +
drivers/net/wireless/Kconfig | 129 +
drivers/net/wireless/Makefile | 12 +
drivers/net/wireless/iwl-3945-hw.h | 118 +
drivers/net/wireless/iwl-3945-rs.c | 979 ++++
drivers/net/wireless/iwl-3945-rs.h | 191 +
drivers/net/wireless/iwl-3945.c | 2290 +++++++++
drivers/net/wireless/iwl-3945.h | 41 +
drivers/net/wireless/iwl-4965-hw.h | 581 +++
drivers/net/wireless/iwl-4965-rs.c | 2118 ++++++++
drivers/net/wireless/iwl-4965-rs.h | 266 +
drivers/net/wireless/iwl-4965.c | 4719 ++++++++++++++++++
drivers/net/wireless/iwl-4965.h | 341 ++
drivers/net/wireless/iwl-channel.h | 161 +
drivers/net/wireless/iwl-commands.h | 1734 +++++++
drivers/net/wireless/iwl-debug.h | 149 +
drivers/net/wireless/iwl-eeprom.h | 336 ++
drivers/net/wireless/iwl-helpers.h | 255 +
drivers/net/wireless/iwl-hw.h | 537 ++
drivers/net/wireless/iwl-io.h | 470 ++
drivers/net/wireless/iwl-priv.h | 308 ++
drivers/net/wireless/iwl-prph.h | 229 +
drivers/net/wireless/iwl-spectrum.h | 91 +
drivers/net/wireless/iwl3945-base.c | 8732 ++++++++++++++++++++++++++++++++
drivers/net/wireless/iwl4965-base.c | 9323 +++++++++++++++++++++++++++++++++++
drivers/net/wireless/iwlwifi.h | 713 +++
26 files changed, 34832 insertions(+), 0 deletions(-)
create mode 100644 drivers/net/wireless/iwl-3945-hw.h
create mode 100644 drivers/net/wireless/iwl-3945-rs.c
create mode 100644 drivers/net/wireless/iwl-3945-rs.h
create mode 100644 drivers/net/wireless/iwl-3945.c
create mode 100644 drivers/net/wireless/iwl-3945.h
create mode 100644 drivers/net/wireless/iwl-4965-hw.h
create mode 100644 drivers/net/wireless/iwl-4965-rs.c
create mode 100644 drivers/net/wireless/iwl-4965-rs.h
create mode 100644 drivers/net/wireless/iwl-4965.c
create mode 100644 drivers/net/wireless/iwl-4965.h
create mode 100644 drivers/net/wireless/iwl-channel.h
create mode 100644 drivers/net/wireless/iwl-commands.h
create mode 100644 drivers/net/wireless/iwl-debug.h
create mode 100644 drivers/net/wireless/iwl-eeprom.h
create mode 100644 drivers/net/wireless/iwl-helpers.h
create mode 100644 drivers/net/wireless/iwl-hw.h
create mode 100644 drivers/net/wireless/iwl-io.h
create mode 100644 drivers/net/wireless/iwl-priv.h
create mode 100644 drivers/net/wireless/iwl-prph.h
create mode 100644 drivers/net/wireless/iwl-spectrum.h
create mode 100644 drivers/net/wireless/iwl3945-base.c
create mode 100644 drivers/net/wireless/iwl4965-base.c
create mode 100644 drivers/net/wireless/iwlwifi.h
--
John W. Linville
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org
^ permalink raw reply
* Re: bnx2 dirver's firmware images
From: H. Peter Anvin @ 2007-09-18 18:41 UTC (permalink / raw)
To: David Miller; +Cc: mchan, vda.linux, linux-kernel, netdev
In-Reply-To: <20070918.112337.74737433.davem@davemloft.net>
David Miller wrote:
>
> I don't like it because it means people have to setup full initrd's
> in order to do network booting with such network cards.
>
klibc could help with that, if there is interest in exploring that
avenue again.
-hpa
^ permalink raw reply
* Re: wrt Age Entry For IPv4 & IPv6 Route Table
From: David Miller @ 2007-09-18 18:36 UTC (permalink / raw)
To: varunc; +Cc: netdev, varuncha
In-Reply-To: <46EF85A8.5050301@linux.vnet.ibm.com>
From: Varun Chandramohan <varunc@linux.vnet.ibm.com>
Date: Tue, 18 Sep 2007 13:30:40 +0530
> Sorry for bothering you. I see that you are currently
> very busy. I just wanted to remind you that iam waiting for your
> feedback on the patch set. Just incase you want me to resend , please
> let me know. If this work is already in your queue, ignore this mail.
I don't have it in my inbox, and also in order to avoid confusion
on what is the latest revision, please repost your patch set
to netdev, thanks.
^ permalink raw reply
* Re: bnx2 dirver's firmware images
From: David Miller @ 2007-09-18 18:23 UTC (permalink / raw)
To: mchan; +Cc: vda.linux, linux-kernel, netdev
In-Reply-To: <1190141114.9540.216.camel@dell>
From: "Michael Chan" <mchan@broadcom.com>
Date: Tue, 18 Sep 2007 11:45:14 -0700
> On Tue, 2007-09-18 at 18:23 +0100, Denys Vlasenko wrote:
> > Do you have any plans to switch to request_firmware() interface,
> > which will allow you to avoid keeping firmware in unswappable kernel
> > memory and thus free ~80k?
>
> Matching the correct version of the firmware with the driver is the main
> issue.
>
> David, what's your opinion on this?
I don't like it because it means people have to setup full initrd's
in order to do network booting with such network cards.
But the days of my opinion mattering on that issue are long gone,
the momentum is just too greatly behind using request_firmware()
across the board, so there is no reason for bnx2 to be any different.
^ permalink raw reply
* NetXen driver causing slab corruption in -RT kernels
From: Vernon Mauery @ 2007-09-18 18:17 UTC (permalink / raw)
To: netdev; +Cc: LKML, Dhananjay Phadke
In doing some stress testing of the NetXen driver, I found that my machine was
dying in all sorts of weird ways. I saw several different crashes, BUG
messages in the TCP stack and some assert messages in the TCP stack as well.
I really didn't think that there could be six different bugs all at once in
the TCP/IP stack, so I started looking at possible memory corruption.
I first saw this on 2.6.16-rt22 with a backported netxen driver from 2.6.22.
I figured I should try the latest kernel, so I tried it on 2.6.23-rc6-git7
but could not trigger the slab corruption messages with CONFIG_DEBUG_SLAB, so
I figured the race must only exist in the -RT kernels. Next I tried
2.6.23-rc6-git7-rt1 (I applied patch-2.6.23-rc4-rt1 patch to 2.6.23-rc6-git7
and fixed the 5 failing hunks). After an hour or so, lots of slab corruption
messages showed up:
Slab corruption: size-2048 start=f40c4670, len=2048
Slab corruption: size-2048 start=f313cf48, len=2048
Redzone: 0x9f911029d74e35b/0x9f911029d74e35b.
Last user: [<c0166be4>](kfree+0x80/0x95)
010: 6b 6b 00 0e 1e 00 16 13 00 0e 1e 00 19 3d 08 00
020: 45 00 05 dc 92 ab 40 00 40 11 8a 5b 0a 02 02 03
030: 0a 02 02 04 80 0c 80 0d 05 c8 dc 39 00 00 00 00
040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Prev obj: start=f313c730, len=2048
Redzone: 0xd84156c5635688c0/0xd84156c5635688c0.
Last user: [<f8f06186>](netxen_post_rx_buffers_nodb+0x62/0x1f0 [netxen_nic])
000: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
010: 5a 5a 00 0e 1e 00 16 13 00 0e 1e 00 19 3d 08 00
Next obj: start=f313d760, len=2048
Redzone: 0xd84156c5635688c0/0xd84156c5635688c0.
Last user: [<f8f06186>](netxen_post_rx_buffers_nodb+0x62/0x1f0 [netxen_nic])
000: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
Slab corruption: size-2048 start=f395a6f0, len=2048
Redzone: 0x9f911029d74e35b/0x9f911029d74e35b.
Last user: [<c0166be4>](kfree+0x80/0x95)
010: 6b 6b 00 0e 1e 00 16 13 00 0e 1e 00 19 3d 08 00
020: 45 00 05 dc 92 ac 40 00 40 11 8a 5a 0a 02 02 03
030: 0a 02 02 04 80 0c 80 0d 05 c8 dc 39 00 00 00 00
040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Next obj: start=f395af08, len=2048
Redzone: 0xd84156c5635688c0/0xd84156c5635688c0.
Last user: [<f8f06186>](netxen_post_rx_buffers_nodb+0x62/0x1f0 [netxen_nic])
000: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
010: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
Redzone: 0x9f911029d74e35b/0x9f911029d74e35b.
Last user: [<c0166be4>](kfree+0x80/0x95)
010: 6b 6b 00 0e 1e 00 16 13 00 0e 1e 00 19 3d 08 00
020: 45 00 05 dc 92 aa 40 00 40 11 8a 5c 0a 02 02 03
030: 0a 02 02 04 80 0c 80 0d 05 c8 dc 39 00 00 00 00
040: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
050: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
060: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Next obj: start=f40c4e88, len=2048
Redzone: 0xd84156c5635688c0/0xd84156c5635688c0.
Last user: [<f8f06186>](netxen_post_rx_buffers_nodb+0x62/0x1f0 [netxen_nic])
000: 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a 5a
010: 5a 5a 00 0e 1e 00 16 13 00 0e 1e 00 19 3d 08 00
The stress test that I am running is basically a mixed bag of stuff I threw
together. It runs eight concurrent netperf TCP streams and two concurrent
UDP streams in both directions, (and on both 10GbE interfaces), ping -f in
both directions, some more disk/cpu loads in the background and a little bit
of NFS traffic thrown in for good measure.
--Vernon
^ permalink raw reply
* Re: bnx2 dirver's firmware images
From: Michael Chan @ 2007-09-18 19:09 UTC (permalink / raw)
To: Denys Vlasenko; +Cc: linux-kernel, David Miller, netdev
In-Reply-To: <200709181855.37598.vda.linux@googlemail.com>
On Tue, 2007-09-18 at 18:55 +0100, Denys Vlasenko wrote:
> On Tuesday 18 September 2007 19:45, Michael Chan wrote:
> > We can compress all the different sections of the firmware. Currently,
> > we only compress the biggest chunks and the rest are uncompressed.
>
> > These zeros should compress to almost nothing. But I agree that the
> > firmware is still big.
>
> You don't need to store and fetch zeros at all. You *know* that
> they are zeros, right? So do this:
>
> - REG_WR_IND(bp, offset, fw->bss[j]);
> + REG_WR_IND(bp, offset, 0);
>
Good point. We can do that. Looking at the file, there are other non-
zero data that can be compressed to save some more room.
^ permalink raw reply
* [ofa-general] [PATCH net-2.6.24] Fix documentation for dev_put()/dev_hold()
From: Roland Dreier @ 2007-09-18 18:04 UTC (permalink / raw)
To: davem; +Cc: netdev, general
In-Reply-To: <adawsunkg8i.fsf@cisco.com>
It looks like the comments for dev_put() and dev_hold() got reversed somehow.
Signed-off-by: Roland Dreier <rolandd@cisco.com>
---
include/linux/netdevice.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index be5fe05..239ae68 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1030,7 +1030,7 @@ extern int netdev_budget;
extern void netdev_run_todo(void);
/**
- * dev_put - get reference to device
+ * dev_put - release reference to device
* @dev: network device
*
* Hold reference to device to keep it from being freed.
@@ -1041,7 +1041,7 @@ static inline void dev_put(struct net_device *dev)
}
/**
- * dev_hold - release reference to device
+ * dev_hold - get reference to device
* @dev: network device
*
* Release reference to device to allow it to be freed.
^ permalink raw reply related
* Re: SFQ qdisc crashes with limit of 2 packets
From: Patrick McHardy @ 2007-09-18 17:57 UTC (permalink / raw)
To: Chuck Ebbert; +Cc: Netdev
In-Reply-To: <46F00B80.7050901@trash.net>
Patrick McHardy wrote:
> Chuck Ebbert wrote:
>
>>Limit of 1 is forbidden, crashes with 2, works with 3:
>>
>>>From disassembling sch_sfq.ko it seems that it is on line 360 of sch_sfq.c:
>> sch->qstats.backlog -= skb->len;
>>where "skb" is an invalid pointer:
>
>
>
> Is it a NULL pointer or something random?
Never mind, I found the reason. When enqueuing the packet, sfq_enqueue
contains an off-by-one in the limit check (which IIRC is there for a
reason, but I can't remember right now) and drops the packet again.
dev_queue_xmit() calls qdisc_run() anyway and the empty qdisc is
dequeued, which is not handled by SFQ.
I see three possibilities to fix this (in my preferred order):
1) figure out why the off-by-one is there, if not needed remove
2) don't dequeue qdiscs even once if empty
3) check for NULL in sfq_dequeue
So I'll try to remeber why the off-by-one is there ..
^ permalink raw reply
* [PATCH net-2.6.24] Fix refcounting problem with netif_rx_reschedule()
From: Roland Dreier @ 2007-09-18 17:58 UTC (permalink / raw)
To: davem; +Cc: Krishna Kumar, netdev, general
In-Reply-To: <20070918111803.1769.60619.sendpatchset@localhost.localdomain>
netif_rx_complete() takes a netdev parameter and does dev_put() on
that netdev, so netif_rx_reschedule() needs to also take a netdev
parameter and do dev_hold() on it to avoid reference counts from
getting becoming negative because of unbalanced dev_put()s.
This should fix the problem reported by Krishna Kumar
<krkumar2@in.ibm.com> with IPoIB waiting forever for netdev refcounts
to become 0 during module unload.
Signed-off-by: Roland Dreier <rolandd@cisco.com>
---
Dave, feel free to roll this up into earlier NAPI conversion patches
(assuming I'm understanding things correctly and this patch actually
makes sense!).
BTW, it looks like drivers/net/ibm_emac/ibm_emac_mal.c would not have
built in the current net-2.6.24 tree, since its call to
netif_rx_reschedule() was left with the netdev parameter. So that
file does not need to be touched in this patch.
drivers/infiniband/ulp/ipoib/ipoib_ib.c | 2 +-
drivers/net/arm/ep93xx_eth.c | 2 +-
drivers/net/ehea/ehea_main.c | 2 +-
drivers/net/ibmveth.c | 2 +-
include/linux/netdevice.h | 21 +++++++++++----------
5 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/infiniband/ulp/ipoib/ipoib_ib.c b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
index 6a2bff4..481e4b6 100644
--- a/drivers/infiniband/ulp/ipoib/ipoib_ib.c
+++ b/drivers/infiniband/ulp/ipoib/ipoib_ib.c
@@ -320,7 +320,7 @@ poll_more:
if (unlikely(ib_req_notify_cq(priv->cq,
IB_CQ_NEXT_COMP |
IB_CQ_REPORT_MISSED_EVENTS)) &&
- netif_rx_reschedule(napi))
+ netif_rx_reschedule(dev, napi))
goto poll_more;
}
diff --git a/drivers/net/arm/ep93xx_eth.c b/drivers/net/arm/ep93xx_eth.c
index f3858d1..7f016f3 100644
--- a/drivers/net/arm/ep93xx_eth.c
+++ b/drivers/net/arm/ep93xx_eth.c
@@ -309,7 +309,7 @@ poll_some_more:
}
spin_unlock_irq(&ep->rx_lock);
- if (more && netif_rx_reschedule(napi))
+ if (more && netif_rx_reschedule(dev, napi))
goto poll_some_more;
}
diff --git a/drivers/net/ehea/ehea_main.c b/drivers/net/ehea/ehea_main.c
index 4a5ab4a..9a499f4 100644
--- a/drivers/net/ehea/ehea_main.c
+++ b/drivers/net/ehea/ehea_main.c
@@ -636,7 +636,7 @@ static int ehea_poll(struct napi_struct *napi, int budget)
if (!cqe && !cqe_skb)
return 0;
- if (!netif_rx_reschedule(napi))
+ if (!netif_rx_reschedule(dev, napi))
return 0;
}
diff --git a/drivers/net/ibmveth.c b/drivers/net/ibmveth.c
index b8d7cec..b94f266 100644
--- a/drivers/net/ibmveth.c
+++ b/drivers/net/ibmveth.c
@@ -973,7 +973,7 @@ static int ibmveth_poll(struct napi_struct *napi, int budget)
netif_rx_complete(netdev, napi);
if (ibmveth_rxq_pending_buffer(adapter) &&
- netif_rx_reschedule(napi)) {
+ netif_rx_reschedule(netdev, napi)) {
lpar_rc = h_vio_signal(adapter->vdev->unit_address,
VIO_IRQ_DISABLE);
goto restart_poll;
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index be5fe05..0dbf185 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1198,16 +1198,6 @@ static inline u32 netif_msg_init(int debug_value, int default_msg_enable_bits)
return (1 << debug_value) - 1;
}
-/* Try to reschedule poll. Called by dev->poll() after netif_rx_complete(). */
-static inline int netif_rx_reschedule(struct napi_struct *n)
-{
- if (napi_schedule_prep(n)) {
- __napi_schedule(n);
- return 1;
- }
- return 0;
-}
-
/* Test if receive needs to be scheduled but only if up */
static inline int netif_rx_schedule_prep(struct net_device *dev,
struct napi_struct *napi)
@@ -1234,6 +1224,17 @@ static inline void netif_rx_schedule(struct net_device *dev,
__netif_rx_schedule(dev, napi);
}
+/* Try to reschedule poll. Called by dev->poll() after netif_rx_complete(). */
+static inline int netif_rx_reschedule(struct net_device *dev,
+ struct napi_struct *napi)
+{
+ if (napi_schedule_prep(napi)) {
+ __netif_rx_schedule(dev, napi);
+ return 1;
+ }
+ return 0;
+}
+
/* same as netif_rx_complete, except that local_irq_save(flags)
* has already been issued
*/
^ permalink raw reply related
* Re: bnx2 dirver's firmware images
From: Denys Vlasenko @ 2007-09-18 17:55 UTC (permalink / raw)
To: Michael Chan; +Cc: linux-kernel, davem, netdev
In-Reply-To: <1190141114.9540.216.camel@dell>
On Tuesday 18 September 2007 19:45, Michael Chan wrote:
> We can compress all the different sections of the firmware. Currently,
> we only compress the biggest chunks and the rest are uncompressed.
> These zeros should compress to almost nothing. But I agree that the
> firmware is still big.
You don't need to store and fetch zeros at all. You *know* that
they are zeros, right? So do this:
- REG_WR_IND(bp, offset, fw->bss[j]);
+ REG_WR_IND(bp, offset, 0);
--
vda
^ permalink raw reply
* Re: bnx2 dirver's firmware images
From: Michael Chan @ 2007-09-18 18:45 UTC (permalink / raw)
To: Denys Vlasenko; +Cc: linux-kernel, davem, netdev
In-Reply-To: <200709181823.26429.vda.linux@googlemail.com>
On Tue, 2007-09-18 at 18:23 +0100, Denys Vlasenko wrote:
> Hi Michael,
>
> In bnx2_fw.h I see the following:
>
> static u32 bnx2_RXP_b06FwBss[(0x13dc/4) + 1] = { 0x0 };
>
> static struct fw_info bnx2_rxp_fw_06 = {
> ...
> .bss = bnx2_RXP_b06FwBss,
> ...
> };
>
> I grepped for the usage of .bss member (grepped for '[.>]bss[^_]')
> and it is used only here:
>
> if (fw->bss) {
> int j;
>
> for (j = 0; j < (fw->bss_len/4); j++, offset += 4) {
> REG_WR_IND(bp, offset, fw->bss[j]);
> }
> }
>
> If I understand it correctly, you read zero words one by one from
> bnx2_RXP_b06FwBss and writing them into the card. This is very
> suboptimal usage of nearly 5k of kernel unswappable memory.
>
> Do you plan to fix it?
We can compress all the different sections of the firmware. Currently,
we only compress the biggest chunks and the rest are uncompressed.
These zeros should compress to almost nothing. But I agree that the
firmware is still big.
>
> Do you have any plans to switch to request_firmware() interface,
> which will allow you to avoid keeping firmware in unswappable kernel
> memory and thus free ~80k?
Matching the correct version of the firmware with the driver is the main
issue.
David, what's your opinion on this?
>
> $ size bnx2.o
> text data bss dec hex filename
> 52255 81551 6360 140166 22386 bnx2.o
> --
> vda
>
^ permalink raw reply
* Re: [ofa-general] Re: [PATCH 02/11] IB/ipoib: Notify the world before doing unregister
From: Roland Dreier @ 2007-09-18 17:42 UTC (permalink / raw)
To: Jay Vosburgh; +Cc: netdev, jgarzik, davem, general
In-Reply-To: <adaps0glve4.fsf@cisco.com>
> Maybe this new notification function should be in net/core/dev.c
> instead of exporting call_netdevice_notifiers()?
Or actually, does it work to add the call to the notifiers directly in
unregister_netdev() so that device drivers don't have to worry about it?
(And is the existing patch missing a call to notifiers in ipoib_vlan_delete()?)
- R.
^ permalink raw reply
* Re: SFQ qdisc crashes with limit of 2 packets
From: Patrick McHardy @ 2007-09-18 17:31 UTC (permalink / raw)
To: Chuck Ebbert; +Cc: Netdev
In-Reply-To: <46F0087A.3080104@redhat.com>
Chuck Ebbert wrote:
> Limit of 1 is forbidden, crashes with 2, works with 3:
>
>>From disassembling sch_sfq.ko it seems that it is on line 360 of sch_sfq.c:
> sch->qstats.backlog -= skb->len;
> where "skb" is an invalid pointer:
Is it a NULL pointer or something random?
^ permalink raw reply
* [ofa-general] Re: [PATCH] RDMA/CMA: Use neigh_event_send() to initiate neighbour discovery.
From: Steve Wise @ 2007-09-18 17:23 UTC (permalink / raw)
To: Sean Hefty; +Cc: netdev, rdreier, linux-kernel, general
In-Reply-To: <000101c7f568$9275b520$ff0da8c0@amr.corp.intel.com>
Once this is applied upstream, I can pull it back in to ofed-1.2.5 and
ofed-1.3.
Steve.
Sean Hefty wrote:
>> RDMA/CMA: Use neigh_event_send() to initiate neighbour discovery.
>>
>> Calling arp_send() to initiate neighbour discovery (ND) doesn't do the
>> full ND protocol. Namely, it doesn't handle retransmitting the arp
>> request if it is dropped. The function neigh_event_send() does all this.
>> Without doing full ND, rdma address resolution fails in the presence of
>> dropped arp bcast packets.
>>
>> Signed-off-by: Steve Wise <swise@opengridcomputing.com>
>
> Acked-by: Sean Hefty <sean.hefty@intel.com>
>
> Roland - can you please queue this up for 2.6.24?
^ permalink raw reply
* [ofa-general] Re: InfiniBand/RDMA merge plans for 2.6.24
From: Michael S. Tsirkin @ 2007-09-18 17:22 UTC (permalink / raw)
To: Roland Dreier; +Cc: netdev, Michael S. Tsirkin, linux-kernel, general
In-Reply-To: <adafy1blwok.fsf@cisco.com>
> Quoting Roland Dreier <rdreier@cisco.com>:
> Subject: Re: InfiniBand/RDMA merge plans for 2.6.24
>
> > Roland, could you merge the common TX CQ patch please?
> > It actually fixes a real problem.
>
> Yes, I will, but it collides with the net-2.6.24 NAPI rework I think,
> so it may not go in until a few days after the merge window.
>
> Have you verified that the patch cures the interrupt overload issues?
Yes.
--
MST
^ permalink raw reply
* SFQ qdisc crashes with limit of 2 packets
From: Chuck Ebbert @ 2007-09-18 17:18 UTC (permalink / raw)
To: Netdev
Limit of 1 is forbidden, crashes with 2, works with 3:
https://bugzilla.redhat.com/show_bug.cgi?id=219895
=========
If the defect is produced at a console (as in ctrl-alt-f<0-6>) a kernel stack
trace can be seen the moment "ping" is invoked. Since the stack trace is not
written to the /var/log/messages here's part of it (manually copied):
syscall_call(()
sys_socketcall()
sys_sendmsg()
sock_sendmsg()
inet_sendmsg()
raw_sendmsg()
ip_push_pending_frames()
ip_output()
neigh_resolve_output()
dev_queue_xmit()
__qdisc_run()
The location given in __qdisc_run() is 0x30/0x19b. The value given for EIP is
sfq_dequeue+0xf6/0x179 in the sch_sfq module.
>From disassembling sch_sfq.ko it seems that it is on line 360 of sch_sfq.c:
sch->qstats.backlog -= skb->len;
where "skb" is an invalid pointer:
net/sched/sch_sfq.c:360
194: ff 4d 28 decl 0x28(%ebp)
197: 8b 14 24 mov (%esp),%edx
19a: 8b 42 60 mov 0x60(%edx),%eax ** crash **
19d: 29 45 58 sub %eax,0x58(%ebp)
^ permalink raw reply
* Re: [PATCH]: New SO_BINDTODEVICE fix.
From: Ben Greear @ 2007-09-18 17:13 UTC (permalink / raw)
To: David Miller; +Cc: netdev, kaber
In-Reply-To: <20070914.164243.41634621.davem@davemloft.net>
David Miller wrote:
> Ok, I changed my mind and decided to retain the optlen==0
> intended behavior. It fell out of fixing the small
> string length case.
>
> This is likely what I'll push to Linus and later -stable
> as a fix for this stuff.
I manually patched this into 2.6.20 and ran some tests.
My test program is here:
http://www.candelatech.com/oss/bind_test.c
First, I could not detect any packets routed in-correctly with the
old code that did not release the route. I'm not sure if my
test is bogus or if the old code somehow managed to trigger
a new route lookup anyway.
On failure, I expected the first un-bind to not actually work
and that I would continue to see the second set of 5 packets on
the previously bound interface. I did not see these packets, so
the un-bind worked.
I did test that the new code works with passing "" and 0 length
argument to the un-bind. As far as I can tell, it now matches
the man page and all is well.
Since I could not reproduce a functional error, and you can work around
the man-page mismatch by passing a 4+ byte string of /0 to un-bind, this
may not be required for stable, but it should not hurt.
Thanks,
Ben
>
> Thanks.
>
> commit 4878809f711981a602cc562eb47994fc81ea0155
> Author: David S. Miller <davem@sunset.davemloft.net>
> Date: Fri Sep 14 16:41:03 2007 -0700
>
> [NET]: Fix two issues wrt. SO_BINDTODEVICE.
>
> 1) Comments suggest that setting optlen to zero will unbind
> the socket from whatever device it might be attached to. This
> hasn't been the case since at least 2.2.x because the first thing
> this function does is return -EINVAL if 'optlen' is less than
> sizeof(int).
>
> This check also means that passing in a two byte string doesn't
> work so well. It's almost as if this code was testing with "eth?"
> patterned strings and nothing else :-)
>
> Fix this by breaking the logic of this facility out into a
> seperate function which validates optlen more appropriately.
>
> The optlen==0 and small string cases now work properly.
>
> 2) We should reset the cached route of the socket after we have made
> the device binding changes, not before.
>
> Reported by Ben Greear.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> diff --git a/net/core/sock.c b/net/core/sock.c
> index cfed7d4..190de61 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c
> @@ -362,6 +362,61 @@ struct dst_entry *sk_dst_check(struct sock *sk, u32 cookie)
> }
> EXPORT_SYMBOL(sk_dst_check);
>
> +static int sock_bindtodevice(struct sock *sk, char __user *optval, int optlen)
> +{
> + int ret = -ENOPROTOOPT;
> +#ifdef CONFIG_NETDEVICES
> + char devname[IFNAMSIZ];
> + int index;
> +
> + /* Sorry... */
> + ret = -EPERM;
> + if (!capable(CAP_NET_RAW))
> + goto out;
> +
> + ret = -EINVAL;
> + if (optlen < 0)
> + goto out;
> +
> + /* Bind this socket to a particular device like "eth0",
> + * as specified in the passed interface name. If the
> + * name is "" or the option length is zero the socket
> + * is not bound.
> + */
> + if (optlen > IFNAMSIZ - 1)
> + optlen = IFNAMSIZ - 1;
> + memset(devname, 0, sizeof(devname));
> +
> + ret = -EFAULT;
> + if (copy_from_user(devname, optval, optlen))
> + goto out;
> +
> + if (devname[0] == '\0') {
> + index = 0;
> + } else {
> + struct net_device *dev = dev_get_by_name(devname);
> +
> + ret = -ENODEV;
> + if (!dev)
> + goto out;
> +
> + index = dev->ifindex;
> + dev_put(dev);
> + }
> +
> + lock_sock(sk);
> + sk->sk_bound_dev_if = index;
> + sk_dst_reset(sk);
> + release_sock(sk);
> +
> + ret = 0;
> +
> +out:
> +#endif
> +
> + return ret;
> +}
> +
> /*
> * This is meant for all protocols to use and covers goings on
> * at the socket level. Everything here is generic.
> @@ -390,6 +445,9 @@ int sock_setsockopt(struct socket *sock, int level, int optname,
> }
> #endif
>
> + if (optname == SO_BINDTODEVICE)
> + return sock_bindtodevice(sk, optval, optlen);
> +
> if (optlen < sizeof(int))
> return -EINVAL;
>
> @@ -578,54 +636,6 @@ set_rcvbuf:
> ret = sock_set_timeout(&sk->sk_sndtimeo, optval, optlen);
> break;
>
> -#ifdef CONFIG_NETDEVICES
> - case SO_BINDTODEVICE:
> - {
> - char devname[IFNAMSIZ];
> -
> - /* Sorry... */
> - if (!capable(CAP_NET_RAW)) {
> - ret = -EPERM;
> - break;
> - }
> -
> - /* Bind this socket to a particular device like "eth0",
> - * as specified in the passed interface name. If the
> - * name is "" or the option length is zero the socket
> - * is not bound.
> - */
> -
> - if (!valbool) {
> - sk->sk_bound_dev_if = 0;
> - } else {
> - if (optlen > IFNAMSIZ - 1)
> - optlen = IFNAMSIZ - 1;
> - memset(devname, 0, sizeof(devname));
> - if (copy_from_user(devname, optval, optlen)) {
> - ret = -EFAULT;
> - break;
> - }
> -
> - /* Remove any cached route for this socket. */
> - sk_dst_reset(sk);
> -
> - if (devname[0] == '\0') {
> - sk->sk_bound_dev_if = 0;
> - } else {
> - struct net_device *dev = dev_get_by_name(devname);
> - if (!dev) {
> - ret = -ENODEV;
> - break;
> - }
> - sk->sk_bound_dev_if = dev->ifindex;
> - dev_put(dev);
> - }
> - }
> - break;
> - }
> -#endif
> -
> -
> case SO_ATTACH_FILTER:
> ret = -EINVAL;
> if (optlen == sizeof(struct sock_fprog)) {
> -
> 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
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* [ofa-general] Re: InfiniBand/RDMA merge plans for 2.6.24
From: Roland Dreier @ 2007-09-18 17:18 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, general
In-Reply-To: <20070918163433.GL2050@mellanox.co.il>
> Roland, could you merge the common TX CQ patch please?
> It actually fixes a real problem.
Yes, I will, but it collides with the net-2.6.24 NAPI rework I think,
so it may not go in until a few days after the merge window.
Have you verified that the patch cures the interrupt overload issues?
^ permalink raw reply
* Re: why does tcp_v[46]_conn_request not inc MIB stats
From: Rick Jones @ 2007-09-18 17:14 UTC (permalink / raw)
To: Andi Kleen; +Cc: David Miller, sri, netdev
In-Reply-To: <p73wsuow7t0.fsf@bingen.suse.de>
Andi Kleen wrote:
> David Miller <davem@davemloft.net> writes:
>
>
>>This is limiting embryonic mini-socket creation. The listen overflow
>>should only increment when the 3-way handshake completion is aborted
>>because the listening socket limit is exceeded, which is entirely
>>different from the embryonic limit.
>
>
> That's true, but I think Rick has a point in that there should
> be some sort of (different) counter counting this. In general
> I believe every point in the stack who drops a packet should
> have a statistics counter so that it can be later diagnosed.
>
> Rick, the best way to get such a counter in is to just send a patch,
> don't ask for it.
I will try to work something up after cleaning-up the other listenq
patch to remove the /proc/net/tcp[6] stuff, and an iproute2 patch I have
to allow setting rto_min, rtt and rttvar using tc-esque units.
rick jones
^ permalink raw reply
* RE: e1000 driver and samba
From: Tantilov, Emil S @ 2007-09-18 17:04 UTC (permalink / raw)
To: L F, Florian Weimer
Cc: Urs Thuermann, Bill Fink, Brandeburg, Jesse, Kok, Auke-jan H,
James Chapman, netdev
In-Reply-To: <780b6f780709180932j5944b927p44ace42833e0230b@mail.gmail.com>
I ran a test here in the lab and was not able to reproduce the issue
you're describing. My setup is with 2 systems connected through a
100Mbit switch. Client runs Windows XP and the server is Linux RHEL5
with updated samba 3.0.26a (latest I could download from samba.org).
I was able to copy 1GB zip file back and forth and also decompress it
from/to the shared drive without data corruption.
Have you looked into the data corruption - what is the difference
between the files? If the files have the same size you should be able to
see the diff with hexdump for example.
Could you provide your samba config?
Thanks,
Emil
-----Original Message-----
From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
On Behalf Of L F
Sent: Tuesday, September 18, 2007 9:32 AM
To: Florian Weimer
Cc: Urs Thuermann; Bill Fink; Brandeburg, Jesse; Kok, Auke-jan H; James
Chapman; netdev@vger.kernel.org
Subject: Re: e1000 driver and samba
This is the latest ethtool -S :
beehive:~# ethtool -S eth4
NIC statistics:
rx_packets: 33491526
tx_packets: 41410384
rx_bytes: 28384277429
tx_bytes: 46178788616
rx_broadcast: 3144
tx_broadcast: 2068
rx_multicast: 79
tx_multicast: 0
rx_errors: 0
tx_errors: 0
tx_dropped: 0
multicast: 79
collisions: 0
rx_length_errors: 0
rx_over_errors: 0
rx_crc_errors: 0
rx_frame_errors: 0
rx_no_buffer_count: 0
rx_missed_errors: 0
tx_aborted_errors: 0
tx_carrier_errors: 0
tx_fifo_errors: 0
tx_heartbeat_errors: 0
tx_window_errors: 0
tx_abort_late_coll: 0
tx_deferred_ok: 36256
tx_single_coll_ok: 0
tx_multi_coll_ok: 0
tx_timeout_count: 0
tx_restart_queue: 0
rx_long_length_errors: 0
rx_short_length_errors: 0
rx_align_errors: 0
tx_tcp_seg_good: 0
tx_tcp_seg_failed: 0
rx_flow_control_xon: 37420
rx_flow_control_xoff: 37420
tx_flow_control_xon: 0
tx_flow_control_xoff: 0
rx_long_byte_count: 28384277429
rx_csum_offload_good: 33478553
rx_csum_offload_errors: 0
rx_header_split: 0
alloc_rx_buff_failed: 0
tx_smbus: 0
rx_smbus: 0
dropped_smbus: 0
There have been no further deferred frames, in accordance to what was
posted above. However, as Urs very correctly points out, the problem
is still very much there. I am waiting for new and improved cables
(third batch) to see if that makes a difference.
I am also wondering if, perhaps, a layer3 managed switch with port
statistics may prove helpful.
To answer Bill's questions, memtest+ 1.70 was run overnight, it
totalled some fifty passes with no errors. I know that some maintain
that memtest is not a definitive RAM test, but it certainly catches
most problems.
As to the disk side of the equation, that is of course a possible
concern and I will be running a second, more intensive batch of tests.
However, the fact that virtualbox (running on the server) reports no
errors despite running through significant amounts of data (10GB+,
passing through samba via the tap device that is bridged to the
problematic ethernet) every day makes me fairly confident that the
disk side of the equation is not where the problem lies.
LF
-
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
^ 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