Netdev List
 help / color / mirror / Atom feed
* [PATCH] tbf: stop wanton destruction of children (v2)
From: Stephen Hemminger @ 2010-05-15  0:38 UTC (permalink / raw)
  To: Patrick McHardy; +Cc: David Miller, netdev
In-Reply-To: <4BEC2936.2060800@trash.net>

Several netem users use TBF for rate control. But every time the parameters
of TBF are changed it destroys the child qdisc, requiring reconfigation.
Better to just keep child qdisc and just notify it of changed limit.

Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>


--- a/net/sched/sch_tbf.c	2010-05-14 15:04:56.297095729 -0700
+++ b/net/sched/sch_tbf.c	2010-05-14 15:09:57.296733332 -0700
@@ -273,7 +273,11 @@ static int tbf_change(struct Qdisc* sch,
 	if (max_size < 0)
 		goto done;
 
-	if (qopt->limit > 0) {
+	if (q->qdisc != &noop_qdisc) {
+		err = fifo_set_limit(q->qdisc, qopt->limit);
+		if (err)
+			goto done;
+	} else if (qopt->limit > 0) {
 		child = fifo_create_dflt(sch, &bfifo_qdisc_ops, qopt->limit);
 		if (IS_ERR(child)) {
 			err = PTR_ERR(child);

^ permalink raw reply

* Re: mmotm 2010-05-11 - dies in pm_qos_update_request()
From: mark gross @ 2010-05-14 23:43 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Valdis.Kletnieks, Andrew Morton, David S. Miller, linux-kernel,
	e1000-devel, netdev
In-Reply-To: <201005140024.50279.rjw@sisk.pl>

On Fri, May 14, 2010 at 12:24:50AM +0200, Rafael J. Wysocki wrote:
> On Thursday 13 May 2010, Valdis.Kletnieks@vt.edu wrote:
> > On Wed, 12 May 2010 23:07:20 +0200, "Rafael J. Wysocki" said:
> > > On Wednesday 12 May 2010, Valdis.Kletnieks@vt.edu wrote:
> > > > On Tue, 11 May 2010 18:21:22 PDT, akpm@linux-foundation.org said:
> > > > > The mm-of-the-moment snapshot 2010-05-11-18-20 has been uploaded to
> > > > > 
> > > > >    http://userweb.kernel.org/~akpm/mmotm/
> > > > 
> > > > Dell Latitude E6500, x86_64 kernel.
> > > > 
> > > > Died a horrid death at boot in the e1000e driver.  Seems to be
> > > > something in linux-next.patch. Didn't get a netconsole trace for obvious
> > > > reasons...
> > > > 
> > > > Copied-by-hand traceback:
> > > > pm_qos_update_request()+0x22
> > > > e1000_configure+0x478
> > > > e1000_open_device+0xee
> > > > ? _raw_notifier_call_chain+0xf
> > > > __dev_open+0xec
> > > > dev_open+0x1b
> > > > netpoll_setup+0x28b
> > > > init_netconsole+0xbc
> > > > 
> > > > I suspect this commit:
> > > > 
> > > > commit 23606cf5d1192c2b17912cb2ef6e62f9b11de133
> > > > Author: Rafael J. Wysocki <rjw@sisk.pl>
> > > > Date:   Sun Mar 14 14:35:17 2010 +0000
> > > > 
> > > >     e1000e / PCI / PM: Add basic runtime PM support (rev. 4)
> > > 
> > > No, I don't think so.  I'm running -rc6 with this patch applied on a box with
> > > e1000e and it works just fine.
> > > 
> > > Please try to revert this one instead:
> > > 
> > > http://git.kernel.org/?p=linux/kernel/git/rafael/suspend-2.6.git;a=patch;h=ed77134bfccf5e75b6cbadab268e559dbe6a4ebb
> > 
> > Confirming - reverting that patch and doing the build fixup results in a
> > kernel that doesn't blow up in the e1000e driver...
> 
> Then I guess there's an initializations problem somewhere.
> 
> Mark, any chance to look into that any time soon?  If we don't resolve this
> before the merge window opens, I'm afraid I'll have to revert that commit
> from my tree.
>

I'll look at it right away.

I think I have an e1000e in my home box.

--mgross

 

^ permalink raw reply

* Re: [net-next-2.6 V7 PATCH 1/2] Add netlink support for virtual port management (was iovnl)
From: Chris Wright @ 2010-05-14 23:43 UTC (permalink / raw)
  To: Williams, Mitch A
  Cc: Patrick McHardy, Chris Wright, Scott Feldman, Arnd Bergmann,
	davem@davemloft.net, shemminger@vyatta.com,
	netdev@vger.kernel.org
In-Reply-To: <EA929A9653AAE14F841771FB1DE5A1365FF39989D4@rrsmsx501.amr.corp.intel.com>

Hi Mitch,

* Williams, Mitch A (mitch.a.williams@intel.com) wrote:
> I'd really like to find a way to fix this, instead of having the functionality disabled.

I've got some patches that are close to complete.  I'll post them
shortly.  I'm able to query link from ip at this point.

thanks,
-chris

^ permalink raw reply

* Re: [PATCH 2.6.34-rc6] net: Improve ks8851 snl transmit performance
From: Bill Fink @ 2010-05-14 23:22 UTC (permalink / raw)
  To: Ha, Tristram
  Cc: Arce, Abraham, Ben Dooks, David Miller, netdev, linux-kernel,
	Jan, Sebastien
In-Reply-To: <14385191E87B904DBD836449AA30269D021A66@MORGANITE.micrel.com>

On Wed, 12 May 2010, Ha, Tristram wrote:

> I use a web browser to send patches through my company's e-mail system.  The message is composed by cut and paste, so it may not conform to Linux standard.
> 
> The latest nuttcp default size for UDP is 1500 bytes, rather than 8192 bytes.  In my case, the transmit performance improves from 10 Mbps to 11.  Have you tried TCP?

Just a nuttcp correction.  The default unicast UDP buflen in any
recent nuttcp is the largest power of 2 less than the MSS of the
control connection.  This means that the default UDP buflen for a
1500 byte MTU link is 1024, while for 9000 byte jumbo capable
networks it would be 8192.  This was done to avoid IP fragmentation
by default in most common scenarios (can be overridden by explicitly
setting the buflen with the "-l" nuttcp option).

						-Bill



> -----Original Message-----
> From: Arce, Abraham [mailto:x0066660@ti.com]
> Sent: Thu 5/6/2010 10:02 PM
> To: Ha, Tristram; Ben Dooks
> Cc: David Miller; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; Jan, Sebastien
> Subject: RE: [PATCH 2.6.34-rc6] net: Improve ks8851 snl transmit performance
>  
> Hi,
> 
> [snip]
> 
> > There is a driver option no_tx_opt so that the driver can revert to original
> > implementation.  This allows user to verify if the transmit performance
> > actually improves.
> 
> Should we limit patch description to 80 characters also?
> 
> > Signed-off-by: Tristram Ha <Tristram.Ha@micrel.com>
> > ---
> > This replaces the [patch 01/13] patch I submitted and was objected by David.
> > 
> > Other users with Micrel KSZ8851 SNL chip please verify the transmit
> > performance does improve or not.
> 
> Tested-by: Abraham Arce <x0066660@ti.com>
> 
> Executing some nuttcp scenarios:
> 
> - Without Patch -
> 
> # /testsuites/ethernet/bin/nuttcp -u -i -Ri50m <serverip>
>  1.2676 MB /   1.00 sec =   10.6330 Mbps     0 /  1298 ~drop/pkt  0.00 ~%loss
>  1.2705 MB /   1.00 sec =   10.6579 Mbps     0 /  1301 ~drop/pkt  0.00 ~%loss
>  1.2686 MB /   1.00 sec =   10.6414 Mbps     0 /  1299 ~drop/pkt  0.00 ~%loss
>  1.2695 MB /   1.00 sec =   10.6496 Mbps     0 /  1300 ~drop/pkt  0.00 ~%loss
>  1.2695 MB /   1.00 sec =   10.6496 Mbps     0 /  1300 ~drop/pkt  0.00 ~%loss
>  1.2686 MB /   1.00 sec =   10.6414 Mbps     0 /  1299 ~drop/pkt  0.00 ~%loss
>  1.2686 MB /   1.00 sec =   10.6414 Mbps     0 /  1299 ~drop/pkt  0.00 ~%loss
>  1.2646 MB /   1.00 sec =   10.6086 Mbps     0 /  1295 ~drop/pkt  0.00 ~%loss
>  1.2686 MB /   1.00 sec =   10.6412 Mbps     0 /  1299 ~drop/pkt  0.00 ~%loss
>  1.2695 MB /   1.00 sec =   10.6498 Mbps     0 /  1300 ~drop/pkt  0.00 ~%loss
> 
> 12.7314 MB /  10.02 sec =   10.6637 Mbps 4 %TX 0 %RX 0 / 13037 drop/pkt 0.00 %loss
> 
> - With Patch -
> 
> # /testsuites/ethernet/bin/nuttcp -u -i -Ri50m 10.87.231.229
>     1.2891 MB /   1.00 sec =   10.8133 Mbps     0 /  1320 ~drop/pkt  0.00 ~%loss
>     1.2900 MB /   1.00 sec =   10.8217 Mbps     0 /  1321 ~drop/pkt  0.00 ~%loss
>     1.2900 MB /   1.00 sec =   10.8217 Mbps     0 /  1321 ~drop/pkt  0.00 ~%loss
>     1.2910 MB /   1.00 sec =   10.8298 Mbps     0 /  1322 ~drop/pkt  0.00 ~%loss
>     1.2910 MB /   1.00 sec =   10.8299 Mbps     0 /  1322 ~drop/pkt  0.00 ~%loss
>     1.2900 MB /   1.00 sec =   10.8216 Mbps     0 /  1321 ~drop/pkt  0.00 ~%loss
>     1.2900 MB /   1.00 sec =   10.8216 Mbps     0 /  1321 ~drop/pkt  0.00 ~%loss
>     1.2891 MB /   1.00 sec =   10.8135 Mbps     0 /  1320 ~drop/pkt  0.00 ~%loss
>     1.2900 MB /   1.00 sec =   10.8216 Mbps     0 /  1321 ~drop/pkt  0.00 ~%loss
>     1.2910 MB /   1.00 sec =   10.8298 Mbps     0 /  1322 ~drop/pkt  0.00 ~%loss
> 
>    12.9492 MB /  10.02 sec =   10.8461 Mbps 4 %TX 0 %RX 0 / 13260 drop/pkt 0.00
> %loss
> 
> Also simulated heavy transmission consisting of 40 processes executed in parallel:
> 
>  - 20 ping instances using packet size of 32768
>  - 20 dd instances creating a 50MB file each under the nfs rootfs
> 
> If any specific test scenario/application is required please do let me know...
> 
> Best Regards
> Abraham

^ permalink raw reply

* Re: TSC unstable due to TSC halts in idle?
From: Suresh Siddha @ 2010-05-14 23:15 UTC (permalink / raw)
  To: john stultz
  Cc: Jesper Dangaard Brouer, Thomas Gleixner,
	linux-kernel@vger.kernel.org, netdev, Brown, Len, Li, Shaohua,
	hawk@comx.dk
In-Reply-To: <AANLkTimeXEaUnzSBuilbQLHj_oho3ia9EgyPGpB7vJRI@mail.gmail.com>

On Fri, 2010-05-14 at 15:28 -0700, john stultz wrote:
> On Fri, May 14, 2010 at 1:13 PM, Jesper Dangaard Brouer <hawk@diku.dk> wrote:
> > I want to know, if its safe to enable the TSC clocksource, when the
> > kernel reports:
> >  "Marking TSC unstable due to TSC halts in idle"
> >
> > The system selects HPET (in current_clocksource), but I can still see
> > TSC as an available clocksource (in
> > /sys/devices/system/clocksource/clocksource0/available_clocksource).
> >
> > Is it safe to enable TSC manually (by changing current_clocksource)?
> > (my workload is 10Git/s routing, cannot survive with a slow clock)
> >
> >
> > Any trick to avoid this? (e.g. kernel config setting, or a /sys/ setting
> > which changes the minimum P-state?)
> 
> Might try booting with the max-cstate=1 option.

Jesper mentioned that it is xeon 5550. TSC's for that processor doesn't
stop in idle. Perhaps he is using an older kernel which doesn't detect
this fact. Jesper, more recent kernels should be able to use TSC as the
clocksource.

thanks,
suresh

^ permalink raw reply

* Weird TCP retransmit behaviour in recent kernels
From: Michael Smith @ 2010-05-14 23:01 UTC (permalink / raw)
  To: netdev

Hi,

I'm struggling with TCP sessions stalling when Windows XP SP2 clients
connect to a SUSE Linux Enterprise 11 server (kernel 2.6.27.x). The
problem doesn't occur with kernel 2.6.18.8 on the server, and I'm
wondering if something's changed since then in the retransmit logic.

It seems like when consecutive packets are lost, the SLES11
server retransmits the first packet when the timeout fires. The client
ACKs, but the server doesn't retransmit the next lost packet; instead,
it sends a couple more new packets, which don't get ACKed. The new
packets don't show up in Wireshark - either something in the network is
dropping them, or maybe Windows doesn't forward them to WinPcap because
there's a hole in the sequence. The timeout fires again after double
the time, and the second packet is retransmitted and ACKed, then
more brand new packets are sent out. The transfer quickly grinds to a
halt.

There's a WAN and VPN between the clients and the server. HTTP downloads
from the server stall at various points depending on the client. The
point at which the connection stalls seems to be dependent on latency.
For example, if the RTT to the client is 12 ms, the connection might
usually stall after 120 KB; if it's 20 ms, it might stall at 1200 KB.

The problem doesn't occur when a Windows client talks to a Windows
server.  When a Linux client talks to the SLES11 server, the connection
doesn't stall completely but slows to a crawl (~3 KB/sec, as opposed to
typical 50-200 KB/sec).

I was able to work around the problem for most clients by locking the
TCP congestion window to a maximum of 6 on the SLES11 server. Some sites
are pathologically bad and the connection stalls unless I lock the
congestion window to 1 (!!).

I've put up a couple of sample traces from a pathological site where
the problem shows up with cwnd locked to 3:

http://www.hurts.ca/sles11.router.pcap.gz - view from the server's firewall
http://www.hurts.ca/sles11.windows.pcap.gz - view from a client PC

On the firewall, you can see the problem around packets 93-104. The server
sends sequence 66781, 68041, 69301; retransmits 66781, gets an ACK, then
sends 70561, 71821; retransmits 68041, gets an ACK, then sends 73081,
74341, and so on. On the client, the "future" sequence packets after
the ACK never show up in Wireshark.

I've tried all of the obvious things:
- disabling TCP segment/checksum offloading functions on client and server;
- disabling SACK;
- trying all available congestion control algorithms on SLES11
   (cubic, reno, veno, illinois);
- turning off anti-virus on the client.

The only 100% reliable workaround seems to be to proxy the connections
through a kernel 2.6.18.8 machine on the same subnet. It seems like
the problem exists with a vanilla 2.6.31 kernel, too.

Has anyone seen something like this before? Any ideas where to go next? 
I'm pretty sure there's nothing strange in the network - just plain old 
Cisco routers and site-to-site VPNs.

Thanks,
Mike

PS: the frame with the HTTP request will show as having a bad checksum
because I hand edited the IP in the Host: header, poorly. Also, the
transfer recovered briefly about 231 seconds in - I couldn't figure out
why, but the SLES11 server finally filled in the sequence hole for a
bit.

^ permalink raw reply

* Re: TSC unstable due to TSC halts in idle?
From: john stultz @ 2010-05-14 22:28 UTC (permalink / raw)
  To: Jesper Dangaard Brouer
  Cc: Thomas Gleixner, linux-kernel, netdev, Len Brown, Shaohua Li,
	hawk
In-Reply-To: <Pine.LNX.4.64.1005142208290.6220@ask.diku.dk>

On Fri, May 14, 2010 at 1:13 PM, Jesper Dangaard Brouer <hawk@diku.dk> wrote:
> I want to know, if its safe to enable the TSC clocksource, when the
> kernel reports:
>  "Marking TSC unstable due to TSC halts in idle"
>
> The system selects HPET (in current_clocksource), but I can still see
> TSC as an available clocksource (in
> /sys/devices/system/clocksource/clocksource0/available_clocksource).
>
> Is it safe to enable TSC manually (by changing current_clocksource)?
> (my workload is 10Git/s routing, cannot survive with a slow clock)
>
>
> Any trick to avoid this? (e.g. kernel config setting, or a /sys/ setting
> which changes the minimum P-state?)

Might try booting with the max-cstate=1 option.

thanks
-john

^ permalink raw reply

* Re: loosing IPMI-card by loading netconsole
From: Tejun Heo @ 2010-05-14 22:24 UTC (permalink / raw)
  To: Matt Mackall
  Cc: Ronciak, John, Henning Fehrmann, Kirsher, Jeffrey T,
	Brandeburg, Jesse, Allan, Bruce W, Waskiewicz Jr, Peter P,
	netdev@vger.kernel.org, Carsten Aulbert
In-Reply-To: <1273875413.15067.1348.camel@calx>

Hello,

On 05/15/2010 12:16 AM, Matt Mackall wrote:
>> Yeap, sure, it would be effective but I kind of want to leave
>> bisection as the last resort.  Bisection is a somewhat painful process
>> especially when the machine isn't right next to you and someone who
>> has overall knowledge can often identify the problem much easier with
>> appropriate debugging info.
> 
> Well nothing jumps to mind in the netpoll/netconsole code and I haven't
> heard any similar reports. My guess is it's something obscure, so I
> think the sooner you start bisecting... Even one or two tests will get
> us a lot closer to figuring out what changed in the last 1.5 years.

I see.  I was hoping it would ring a bell to someone.  We'll probably
try to provide the info Jesse asked and if that doesn't lead anywhere
start bisecting.

Thank you.

-- 
tejun

^ permalink raw reply

* Re: loosing IPMI-card by loading netconsole
From: Tejun Heo @ 2010-05-14 22:18 UTC (permalink / raw)
  To: Brandeburg, Jesse
  Cc: e1000-devel, netdev@vger.kernel.org, Allan, Bruce W,
	Henning Fehrmann, Ronciak, John, Kirsher, Jeffrey T, Matt Mackall,
	Carsten Aulbert
In-Reply-To: <1273857641.3057.20.camel@localhost.localdomain>

Hello,

On 05/14/2010 07:20 PM, "Brandeburg, Jesse" wrote:
> On Fri, 2010-05-14 at 09:27 -0700, Tejun Heo wrote:
> We've actually had quite a few problems like this over the years, so I'm
> not quite so surprised to hear about something like this.
> 
> Its easy to break the reception of IPMI packets because there are a
> couple of registers that if not correctly configured during all points
> of driver lifetime (probe only, administratively down, up)

Heh... yeah, haven't heard a lot of kind things about IPMI.  IPMI
which shares the usual network connection seems like a good idea on
paper but doesn't really seem to work as expected.  It is a pretty
backward idea to have low level debugging / management mechanism
depending on all the highlevel stuff working correctly.

> One thing that would really help us is to see the stats from ethtool -S
> ethX when interface is up, and not receiving IPMI
>  
> The other "smoking gun" indicator is the output of the register dump
> tool called ethregs that we have posted at sourceforge.  Please gather
> registers for the card in question before and after loading netconsole.
> 
> http://prdownloads.sf.net/e1000/ethregs-1.7.2.tar.gz

I think Henning would be able to provide these info early next week.

>> On 2.6.27.39, netconsole + IPMI works fine.  On 2.6.32.7, as soon as
>> netconsole is loaded, IPMI stops working.  Unloading netconsole
>> doesn't revive IPMI but detaching the driver from the controller does.
>> In both cases, usual networking works fine.
> 
> I think that "loading netconsole" means bringing the interface "UP", in
> this case, is this correct?

Nope, modprobing the netconsole module.  Usual networking and IPMI
work happily together until netconsole is loaded.  The weird thing tho
is that Henning is reporting that IPMI is using different interface
from the netconsole.  No idea how they could interfact.

> To ask another way: Is network traffic active on the interface in
> question before netconsole is loaded?

So, yeap.

Thanks.

-- 
tejun

------------------------------------------------------------------------------

_______________________________________________
E1000-devel mailing list
E1000-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel&#174; Ethernet, visit http://communities.intel.com/community/wired

^ permalink raw reply

* Re: loosing IPMI-card by loading netconsole
From: Matt Mackall @ 2010-05-14 22:16 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Ronciak, John, Henning Fehrmann, Kirsher, Jeffrey T,
	Brandeburg, Jesse, Allan, Bruce W, Waskiewicz Jr, Peter P,
	netdev@vger.kernel.org, Carsten Aulbert
In-Reply-To: <4BEDCA89.7000707@kernel.org>

On Sat, 2010-05-15 at 00:11 +0200, Tejun Heo wrote:
> Hello,
> 
> On 05/14/2010 07:37 PM, Matt Mackall wrote:
> >> On 2.6.27.39, netconsole + IPMI works fine.  On 2.6.32.7, as soon as
> >> netconsole is loaded, IPMI stops working.  Unloading netconsole
> >> doesn't revive IPMI but detaching the driver from the controller does.
> >> In both cases, usual networking works fine.
> > 
> > Looks like a job for bisect.
> 
> Yeap, sure, it would be effective but I kind of want to leave
> bisection as the last resort.  Bisection is a somewhat painful process
> especially when the machine isn't right next to you and someone who
> has overall knowledge can often identify the problem much easier with
> appropriate debugging info.

Well nothing jumps to mind in the netpoll/netconsole code and I haven't
heard any similar reports. My guess is it's something obscure, so I
think the sooner you start bisecting... Even one or two tests will get
us a lot closer to figuring out what changed in the last 1.5 years.

-- 
Mathematics is the supreme nostalgia of our time.



^ permalink raw reply

* Re: loosing IPMI-card by loading netconsole
From: Tejun Heo @ 2010-05-14 22:11 UTC (permalink / raw)
  To: Matt Mackall
  Cc: Ronciak, John, Henning Fehrmann, Kirsher, Jeffrey T,
	Brandeburg, Jesse, Allan, Bruce W, Waskiewicz Jr, Peter P,
	netdev@vger.kernel.org, Carsten Aulbert
In-Reply-To: <1273858653.15067.1335.camel@calx>

Hello,

On 05/14/2010 07:37 PM, Matt Mackall wrote:
>> On 2.6.27.39, netconsole + IPMI works fine.  On 2.6.32.7, as soon as
>> netconsole is loaded, IPMI stops working.  Unloading netconsole
>> doesn't revive IPMI but detaching the driver from the controller does.
>> In both cases, usual networking works fine.
> 
> Looks like a job for bisect.

Yeap, sure, it would be effective but I kind of want to leave
bisection as the last resort.  Bisection is a somewhat painful process
especially when the machine isn't right next to you and someone who
has overall knowledge can often identify the problem much easier with
appropriate debugging info.

Thanks.

-- 
tejun

^ permalink raw reply

* [patch 2.6.35 25/25] wimax/i2400m: driver defaults to firmware v1.5 for i6x60 devices
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

Firmware is available in the linux-firmware package.

Signed-off-by: Inaky Perez-Gonzalez <inaky@linux.intel.com>
---
 drivers/net/wimax/i2400m/usb.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wimax/i2400m/usb.c b/drivers/net/wimax/i2400m/usb.c
index f456807..16341ff 100644
--- a/drivers/net/wimax/i2400m/usb.c
+++ b/drivers/net/wimax/i2400m/usb.c
@@ -788,3 +788,4 @@ MODULE_DESCRIPTION("Driver for USB based Intel Wireless WiMAX Connection 2400M "
 		   "(5x50 & 6050)");
 MODULE_LICENSE("GPL");
 MODULE_FIRMWARE(I2400MU_FW_FILE_NAME_v1_5);
+MODULE_FIRMWARE(I6050U_FW_FILE_NAME_v1_5);
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 24/25] wimax/i2400m: driver defaults to firmware v1.5 for i5x50 devices
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

Updates the i2400m driver to default to firmware versions v1.5 for the
Intel Wireless WiMAX Connection 5150 and 5350 devices.

Firmware available in linux-firmware.

Signed-off-by: Inaky Perez-Gonzalez <inaky@linux.intel.com>
---
 drivers/net/wimax/i2400m/usb.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wimax/i2400m/usb.c b/drivers/net/wimax/i2400m/usb.c
index 6fd8cf5..f456807 100644
--- a/drivers/net/wimax/i2400m/usb.c
+++ b/drivers/net/wimax/i2400m/usb.c
@@ -82,6 +82,8 @@ MODULE_PARM_DESC(debug,
 
 /* Our firmware file name */
 static const char *i2400mu_bus_fw_names_5x50[] = {
+#define I2400MU_FW_FILE_NAME_v1_5 "i2400m-fw-usb-1.5.sbcf"
+	I2400MU_FW_FILE_NAME_v1_5,
 #define I2400MU_FW_FILE_NAME_v1_4 "i2400m-fw-usb-1.4.sbcf"
 	I2400MU_FW_FILE_NAME_v1_4,
 	NULL,
@@ -785,4 +787,4 @@ MODULE_AUTHOR("Intel Corporation <linux-wimax@intel.com>");
 MODULE_DESCRIPTION("Driver for USB based Intel Wireless WiMAX Connection 2400M "
 		   "(5x50 & 6050)");
 MODULE_LICENSE("GPL");
-MODULE_FIRMWARE(I2400MU_FW_FILE_NAME_v1_4);
+MODULE_FIRMWARE(I2400MU_FW_FILE_NAME_v1_5);
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 22/25] wimax: wimax_msg_alloc() returns ERR_PTR not null
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Dan Carpenter
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Dan Carpenter <error27@gmail.com>

wimax_msg_alloc() returns an ERR_PTR and not null.  I changed it to test
for ERR_PTR instead of null.  I also added a check in front of the
kfree() because kfree() can handle null but not ERR_PTR.

Signed-off-by: Dan Carpenter <error27@gmail.com>
---
 drivers/net/wimax/i2400m/rx.c |    9 ++++-----
 1 files changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wimax/i2400m/rx.c b/drivers/net/wimax/i2400m/rx.c
index 66f968a..0004c68 100644
--- a/drivers/net/wimax/i2400m/rx.c
+++ b/drivers/net/wimax/i2400m/rx.c
@@ -300,17 +300,16 @@ void i2400m_rx_ctl_ack(struct i2400m *i2400m,
 		d_printf(1, dev, "Huh? waiter for command reply cancelled\n");
 		goto error_waiter_cancelled;
 	}
-	if (ack_skb == NULL) {
+	if (IS_ERR(ack_skb))
 		dev_err(dev, "CMD/GET/SET ack: cannot allocate SKB\n");
-		i2400m->ack_skb = ERR_PTR(-ENOMEM);
-	} else
-		i2400m->ack_skb = ack_skb;
+	i2400m->ack_skb = ack_skb;
 	spin_unlock_irqrestore(&i2400m->rx_lock, flags);
 	complete(&i2400m->msg_completion);
 	return;
 
 error_waiter_cancelled:
-	kfree_skb(ack_skb);
+	if (!IS_ERR(ack_skb))
+		kfree_skb(ack_skb);
 error_no_waiter:
 	spin_unlock_irqrestore(&i2400m->rx_lock, flags);
 	return;
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 23/25] wimax/i2400m: Move module params to other file so they can be static
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Prasanna S Panchamukhi
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Prasanna S Panchamukhi <prasannax.s.panchamukhi@intel.com>

This patch moves the module parameters to the file where they
can be avoided to be global and allow them to be static.

The module param : idle_mode_disabled and power_save_disabled
are moved from  driver.c to control.c. Also these module parameters
are declared to be static as they are not required to be global anymore.
The module param : rx_reorder_disabled is moved from driver.c file to
rx.c file. Also this parameter is declated as static as it is not
required to be global anymore.

Signed-off-by: Prasanna S Panchamukhi<prasannax.s.panchamukhi@intel.com>
---
 drivers/net/wimax/i2400m/control.c |   15 +++++++++++++++
 drivers/net/wimax/i2400m/driver.c  |   19 -------------------
 drivers/net/wimax/i2400m/i2400m.h  |    6 ------
 drivers/net/wimax/i2400m/rx.c      |    5 +++++
 4 files changed, 20 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wimax/i2400m/control.c b/drivers/net/wimax/i2400m/control.c
index 6180772..0c1aa88 100644
--- a/drivers/net/wimax/i2400m/control.c
+++ b/drivers/net/wimax/i2400m/control.c
@@ -83,6 +83,21 @@
 #define D_SUBMODULE control
 #include "debug-levels.h"
 
+static int i2400m_idle_mode_disabled;/* 0 (idle mode enabled) by default */
+module_param_named(idle_mode_disabled, i2400m_idle_mode_disabled, int, 0644);
+MODULE_PARM_DESC(idle_mode_disabled,
+		 "If true, the device will not enable idle mode negotiation "
+		 "with the base station (when connected) to save power.");
+
+/* 0 (power saving enabled) by default */
+static int i2400m_power_save_disabled;
+module_param_named(power_save_disabled, i2400m_power_save_disabled, int, 0644);
+MODULE_PARM_DESC(power_save_disabled,
+		 "If true, the driver will not tell the device to enter "
+		 "power saving mode when it reports it is ready for it. "
+		 "False by default (so the device is told to do power "
+		 "saving).");
+
 int i2400m_passive_mode;	/* 0 (passive mode disabled) by default */
 module_param_named(passive_mode, i2400m_passive_mode, int, 0644);
 MODULE_PARM_DESC(passive_mode,
diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
index 39cf96a..66bdb5d 100644
--- a/drivers/net/wimax/i2400m/driver.c
+++ b/drivers/net/wimax/i2400m/driver.c
@@ -75,25 +75,6 @@
 #include "debug-levels.h"
 
 
-int i2400m_idle_mode_disabled;	/* 0 (idle mode enabled) by default */
-module_param_named(idle_mode_disabled, i2400m_idle_mode_disabled, int, 0644);
-MODULE_PARM_DESC(idle_mode_disabled,
-		 "If true, the device will not enable idle mode negotiation "
-		 "with the base station (when connected) to save power.");
-
-int i2400m_rx_reorder_disabled;	/* 0 (rx reorder enabled) by default */
-module_param_named(rx_reorder_disabled, i2400m_rx_reorder_disabled, int, 0644);
-MODULE_PARM_DESC(rx_reorder_disabled,
-		 "If true, RX reordering will be disabled.");
-
-int i2400m_power_save_disabled;	/* 0 (power saving enabled) by default */
-module_param_named(power_save_disabled, i2400m_power_save_disabled, int, 0644);
-MODULE_PARM_DESC(power_save_disabled,
-		 "If true, the driver will not tell the device to enter "
-		 "power saving mode when it reports it is ready for it. "
-		 "False by default (so the device is told to do power "
-		 "saving).");
-
 static char i2400m_debug_params[128];
 module_param_string(debug, i2400m_debug_params, sizeof(i2400m_debug_params),
 		    0644);
diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index 1babc55..fa74777 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -885,7 +885,6 @@ extern int i2400m_rx(struct i2400m *, struct sk_buff *);
 extern struct i2400m_msg_hdr *i2400m_tx_msg_get(struct i2400m *, size_t *);
 extern void i2400m_tx_msg_sent(struct i2400m *);
 
-extern int i2400m_power_save_disabled;
 
 /*
  * Utility functions
@@ -992,10 +991,5 @@ extern int i2400m_barker_db_init(const char *);
 extern void i2400m_barker_db_exit(void);
 
 
-/* Module parameters */
-
-extern int i2400m_idle_mode_disabled;
-extern int i2400m_rx_reorder_disabled;
-
 
 #endif /* #ifndef __I2400M_H__ */
diff --git a/drivers/net/wimax/i2400m/rx.c b/drivers/net/wimax/i2400m/rx.c
index 0004c68..c835ae8 100644
--- a/drivers/net/wimax/i2400m/rx.c
+++ b/drivers/net/wimax/i2400m/rx.c
@@ -155,6 +155,11 @@
 #define D_SUBMODULE rx
 #include "debug-levels.h"
 
+static int i2400m_rx_reorder_disabled;	/* 0 (rx reorder enabled) by default */
+module_param_named(rx_reorder_disabled, i2400m_rx_reorder_disabled, int, 0644);
+MODULE_PARM_DESC(rx_reorder_disabled,
+		 "If true, RX reordering will be disabled.");
+
 struct i2400m_report_hook_args {
 	struct sk_buff *skb_rx;
 	const struct i2400m_l3l4_hdr *l3l4_hdr;
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 21/25] wimax: checking ERR_PTR vs null
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Dan Carpenter
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Dan Carpenter <error27@gmail.com>

stch_skb is allocated with wimax_gnl_re_state_change_alloc().  That
function returns ERR_PTRs on failure and doesn't return NULL.

Signed-off-by: Dan Carpenter <error27@gmail.com>
---
 net/wimax/stack.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/wimax/stack.c b/net/wimax/stack.c
index 1ed65db..62b1a66 100644
--- a/net/wimax/stack.c
+++ b/net/wimax/stack.c
@@ -315,7 +315,7 @@ void __wimax_state_change(struct wimax_dev *wimax_dev, enum wimax_st new_state)
 		BUG();
 	}
 	__wimax_state_set(wimax_dev, new_state);
-	if (stch_skb)
+	if (!IS_ERR(stch_skb))
 		wimax_gnl_re_state_change_send(wimax_dev, stch_skb, header);
 out:
 	d_fnend(3, dev, "(wimax_dev %p new_state %u [old %u]) = void\n",
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 20/25] wimax/i2400m: USB specific TX queue's minimum buffer room required for new message
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>

This patch specifies the TX queue's buffer room required by the
USB bus driver while allocating header space for a new message.
Please refer the documentation in the code.

Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
 drivers/net/wimax/i2400m/usb.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wimax/i2400m/usb.c b/drivers/net/wimax/i2400m/usb.c
index d8c4d64..6fd8cf5 100644
--- a/drivers/net/wimax/i2400m/usb.c
+++ b/drivers/net/wimax/i2400m/usb.c
@@ -467,6 +467,13 @@ int i2400mu_probe(struct usb_interface *iface,
 	usb_set_intfdata(iface, i2400mu);
 
 	i2400m->bus_tx_block_size = I2400MU_BLK_SIZE;
+	/*
+	 * Room required in the Tx queue for USB message to accommodate
+	 * a smallest payload while allocating header space is 16 bytes.
+	 * Adding this room  for the new tx message increases the
+	 * possibilities of including any payload with size <= 16 bytes.
+	 */
+	i2400m->bus_tx_room_min = I2400MU_BLK_SIZE;
 	i2400m->bus_pl_size_max = I2400MU_PL_SIZE_MAX;
 	i2400m->bus_setup = NULL;
 	i2400m->bus_dev_start = i2400mu_bus_dev_start;
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 18/25] wimax/i2400m: reserve additional space in the TX queue's buffer while allocating space for a new message header
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>

Increase the possibilities of including at least one payload by reserving
some additional space in the TX queue while allocating TX queue's space
for new message header. Please refer the documentation in the code for details.

Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
 drivers/net/wimax/i2400m/i2400m.h |    6 ++++++
 drivers/net/wimax/i2400m/tx.c     |   11 ++++++++++-
 2 files changed, 16 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index b8c7dbf..1babc55 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -242,6 +242,11 @@ struct i2400m_barker_db;
  *     so we have a tx_blk_size variable that the bus layer sets to
  *     tell the engine how much of that we need.
  *
+ * @bus_tx_room_min: [fill] Minimum room required while allocating
+ *     TX queue's buffer space for message header. SDIO requires
+ *     224 bytes and USB 16 bytes. Refer bus specific driver code
+ *     for details.
+ *
  * @bus_pl_size_max: [fill] Maximum payload size.
  *
  * @bus_setup: [optional fill] Function called by the bus-generic code
@@ -573,6 +578,7 @@ struct i2400m {
 	wait_queue_head_t state_wq;	/* Woken up when on state updates */
 
 	size_t bus_tx_block_size;
+	size_t bus_tx_room_min;
 	size_t bus_pl_size_max;
 	unsigned bus_bm_retries;
 
diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index 609f1ca..3f819ef 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -563,8 +563,17 @@ void i2400m_tx_new(struct i2400m *i2400m)
 	struct i2400m_msg_hdr *tx_msg;
 	bool try_head = 0;
 	BUG_ON(i2400m->tx_msg != NULL);
+	/*
+	 * In certain situations, TX queue might have enough space to
+	 * accommodate the new message header I2400M_TX_PLD_SIZE, but
+	 * might not have enough space to accommodate the payloads.
+	 * Adding bus_tx_room_min padding while allocating a new TX message
+	 * increases the possibilities of including at least one payload of the
+	 * size <= bus_tx_room_min.
+	 */
 try_head:
-	tx_msg = i2400m_tx_fifo_push(i2400m, I2400M_TX_PLD_SIZE, 0, try_head);
+	tx_msg = i2400m_tx_fifo_push(i2400m, I2400M_TX_PLD_SIZE,
+				     i2400m->bus_tx_room_min, try_head);
 	if (tx_msg == NULL)
 		goto out;
 	else if (tx_msg == TAIL_FULL) {
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 19/25] wimax/i2400m: SDIO specific TX queue's minimum buffer room for new message
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>

This patch specifies the TX queue's minimum buffer room required to
accommodate one smallest SDIO payload.
Please refer the documentation in the code.

Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
 drivers/net/wimax/i2400m/sdio.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/net/wimax/i2400m/sdio.c b/drivers/net/wimax/i2400m/sdio.c
index 7632f80..9bfc26e 100644
--- a/drivers/net/wimax/i2400m/sdio.c
+++ b/drivers/net/wimax/i2400m/sdio.c
@@ -483,6 +483,13 @@ int i2400ms_probe(struct sdio_func *func,
 	sdio_set_drvdata(func, i2400ms);
 
 	i2400m->bus_tx_block_size = I2400MS_BLK_SIZE;
+	/*
+	 * Room required in the TX queue for SDIO message to accommodate
+	 * a smallest payload while allocating header space is 224 bytes,
+	 * which is the smallest message size(the block size 256 bytes)
+	 * minus the smallest message header size(32 bytes).
+	 */
+	i2400m->bus_tx_room_min = I2400MS_BLK_SIZE - I2400M_PL_ALIGN * 2;
 	i2400m->bus_pl_size_max = I2400MS_PL_SIZE_MAX;
 	i2400m->bus_setup = i2400ms_bus_setup;
 	i2400m->bus_dev_start = i2400ms_bus_dev_start;
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 16/25] wimax i2400m: fix race condition while accessing rx_roq by using kref count
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>

This patch fixes the race condition when one thread tries to destroy
the memory allocated for rx_roq, while another thread still happen
to access rx_roq.
Such a race condition occurs when i2400m-sdio kernel module gets
unloaded, destroying the memory allocated for rx_roq while rx_roq
is accessed by i2400m_rx_edata(), as explained below:
$thread1                                $thread2
$ void i2400m_rx_edata()                $
$Access rx_roq[]                        $
$roq = &i2400m->rx_roq[ro_cin]          $
$ i2400m_roq_[reset/queue/update_ws]    $
$                                       $ void i2400m_rx_release();
$                                       $kfree(rx->roq);
$                                       $rx->roq = NULL;
$Oops! rx_roq is NULL

This patch fixes the race condition using refcount approach.

Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
 drivers/net/wimax/i2400m/i2400m.h |   12 +++++++--
 drivers/net/wimax/i2400m/rx.c     |   44 ++++++++++++++++++++++++++++++++----
 2 files changed, 48 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wimax/i2400m/i2400m.h b/drivers/net/wimax/i2400m/i2400m.h
index 7a9c2c5..b8c7dbf 100644
--- a/drivers/net/wimax/i2400m/i2400m.h
+++ b/drivers/net/wimax/i2400m/i2400m.h
@@ -412,7 +412,7 @@ struct i2400m_barker_db;
  *
  * @tx_size_max: biggest TX message sent.
  *
- * @rx_lock: spinlock to protect RX members
+ * @rx_lock: spinlock to protect RX members and rx_roq_refcount.
  *
  * @rx_pl_num: total number of payloads received
  *
@@ -436,6 +436,10 @@ struct i2400m_barker_db;
  *     delivered. Then the driver can release them to the host. See
  *     drivers/net/i2400m/rx.c for details.
  *
+ * @rx_roq_refcount: refcount rx_roq. This refcounts any access to
+ *     rx_roq thus preventing rx_roq being destroyed when rx_roq
+ *     is being accessed. rx_roq_refcount is protected by rx_lock.
+ *
  * @rx_reports: reports received from the device that couldn't be
  *     processed because the driver wasn't still ready; when ready,
  *     they are pulled from here and chewed.
@@ -597,10 +601,12 @@ struct i2400m {
 		tx_num, tx_size_acc, tx_size_min, tx_size_max;
 
 	/* RX stuff */
-	spinlock_t rx_lock;		/* protect RX state */
+	/* protect RX state and rx_roq_refcount */
+	spinlock_t rx_lock;
 	unsigned rx_pl_num, rx_pl_max, rx_pl_min,
 		rx_num, rx_size_acc, rx_size_min, rx_size_max;
-	struct i2400m_roq *rx_roq;	/* not under rx_lock! */
+	struct i2400m_roq *rx_roq;	/* access is refcounted */
+	struct kref rx_roq_refcount;	/* refcount access to rx_roq */
 	u8 src_mac_addr[ETH_HLEN];
 	struct list_head rx_reports;	/* under rx_lock! */
 	struct work_struct rx_report_ws;
diff --git a/drivers/net/wimax/i2400m/rx.c b/drivers/net/wimax/i2400m/rx.c
index fa2e11e..71b697f 100644
--- a/drivers/net/wimax/i2400m/rx.c
+++ b/drivers/net/wimax/i2400m/rx.c
@@ -917,6 +917,25 @@ void i2400m_roq_queue_update_ws(struct i2400m *i2400m, struct i2400m_roq *roq,
 
 
 /*
+ * This routine destroys the memory allocated for rx_roq, when no
+ * other thread is accessing it. Access to rx_roq is refcounted by
+ * rx_roq_refcount, hence memory allocated must be destroyed when
+ * rx_roq_refcount becomes zero. This routine gets executed when
+ * rx_roq_refcount becomes zero.
+ */
+void i2400m_rx_roq_destroy(struct kref *ref)
+{
+	unsigned itr;
+	struct i2400m *i2400m
+			= container_of(ref, struct i2400m, rx_roq_refcount);
+	for (itr = 0; itr < I2400M_RO_CIN + 1; itr++)
+		__skb_queue_purge(&i2400m->rx_roq[itr].queue);
+	kfree(i2400m->rx_roq[0].log);
+	kfree(i2400m->rx_roq);
+	i2400m->rx_roq = NULL;
+}
+
+/*
  * Receive and send up an extended data packet
  *
  * @i2400m: device descriptor
@@ -969,6 +988,7 @@ void i2400m_rx_edata(struct i2400m *i2400m, struct sk_buff *skb_rx,
 	unsigned ro_needed, ro_type, ro_cin, ro_sn;
 	struct i2400m_roq *roq;
 	struct i2400m_roq_data *roq_data;
+	unsigned long flags;
 
 	BUILD_BUG_ON(ETH_HLEN > sizeof(*hdr));
 
@@ -1007,7 +1027,16 @@ void i2400m_rx_edata(struct i2400m *i2400m, struct sk_buff *skb_rx,
 		ro_cin = (reorder >> I2400M_RO_CIN_SHIFT) & I2400M_RO_CIN;
 		ro_sn = (reorder >> I2400M_RO_SN_SHIFT) & I2400M_RO_SN;
 
+		spin_lock_irqsave(&i2400m->rx_lock, flags);
 		roq = &i2400m->rx_roq[ro_cin];
+		if (roq == NULL) {
+			kfree_skb(skb);	/* rx_roq is already destroyed */
+			spin_unlock_irqrestore(&i2400m->rx_lock, flags);
+			goto error;
+		}
+		kref_get(&i2400m->rx_roq_refcount);
+		spin_unlock_irqrestore(&i2400m->rx_lock, flags);
+
 		roq_data = (struct i2400m_roq_data *) &skb->cb;
 		roq_data->sn = ro_sn;
 		roq_data->cs = cs;
@@ -1034,6 +1063,10 @@ void i2400m_rx_edata(struct i2400m *i2400m, struct sk_buff *skb_rx,
 		default:
 			dev_err(dev, "HW BUG? unknown reorder type %u\n", ro_type);
 		}
+
+		spin_lock_irqsave(&i2400m->rx_lock, flags);
+		kref_put(&i2400m->rx_roq_refcount, i2400m_rx_roq_destroy);
+		spin_unlock_irqrestore(&i2400m->rx_lock, flags);
 	}
 	else
 		i2400m_net_erx(i2400m, skb, cs);
@@ -1344,6 +1377,7 @@ int i2400m_rx_setup(struct i2400m *i2400m)
 			__i2400m_roq_init(&i2400m->rx_roq[itr]);
 			i2400m->rx_roq[itr].log = &rd[itr];
 		}
+		kref_init(&i2400m->rx_roq_refcount);
 	}
 	return 0;
 
@@ -1357,12 +1391,12 @@ error_roq_alloc:
 /* Tear down the RX queue and infrastructure */
 void i2400m_rx_release(struct i2400m *i2400m)
 {
+	unsigned long flags;
+
 	if (i2400m->rx_reorder) {
-		unsigned itr;
-		for(itr = 0; itr < I2400M_RO_CIN + 1; itr++)
-			__skb_queue_purge(&i2400m->rx_roq[itr].queue);
-		kfree(i2400m->rx_roq[0].log);
-		kfree(i2400m->rx_roq);
+		spin_lock_irqsave(&i2400m->rx_lock, flags);
+		kref_put(&i2400m->rx_roq_refcount, i2400m_rx_roq_destroy);
+		spin_unlock_irqrestore(&i2400m->rx_lock, flags);
 	}
 	/* at this point, nothing can be received... */
 	i2400m_report_hook_flush(i2400m);
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 17/25] wimax/i2400m: fix incorrect handling of type 2 and 3 RX messages
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>

According to Intel Wimax i3200, i5x50 and i6x60 device specification documents,
the host driver must not reset the device if the normalized sequence numbers
are greater than 1023 for type 2 and type 3 RX messages.
This patch removes the code that incorrectly used to reset the device.

Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
 drivers/net/wimax/i2400m/rx.c |   51 +++++++++++++++++++++--------------------
 1 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/drivers/net/wimax/i2400m/rx.c b/drivers/net/wimax/i2400m/rx.c
index 71b697f..66f968a 100644
--- a/drivers/net/wimax/i2400m/rx.c
+++ b/drivers/net/wimax/i2400m/rx.c
@@ -743,12 +743,12 @@ unsigned __i2400m_roq_update_ws(struct i2400m *i2400m, struct i2400m_roq *roq,
 	unsigned new_nws, nsn_itr;
 
 	new_nws = __i2400m_roq_nsn(roq, sn);
-	if (unlikely(new_nws >= 1024) && d_test(1)) {
-		dev_err(dev, "SW BUG? __update_ws new_nws %u (sn %u ws %u)\n",
-			new_nws, sn, roq->ws);
-		WARN_ON(1);
-		i2400m_roq_log_dump(i2400m, roq);
-	}
+	/*
+	 * For type 2(update_window_start) rx messages, there is no
+	 * need to check if the normalized sequence number is greater 1023.
+	 * Simply insert and deliver all packets to the host up to the
+	 * window start.
+	 */
 	skb_queue_walk_safe(&roq->queue, skb_itr, tmp_itr) {
 		roq_data_itr = (struct i2400m_roq_data *) &skb_itr->cb;
 		nsn_itr = __i2400m_roq_nsn(roq, roq_data_itr->sn);
@@ -890,26 +890,27 @@ void i2400m_roq_queue_update_ws(struct i2400m *i2400m, struct i2400m_roq *roq,
 		  i2400m, roq, skb, sn);
 	len = skb_queue_len(&roq->queue);
 	nsn = __i2400m_roq_nsn(roq, sn);
+	/*
+	 * For type 3(queue_update_window_start) rx messages, there is no
+	 * need to check if the normalized sequence number is greater 1023.
+	 * Simply insert and deliver all packets to the host up to the
+	 * window start.
+	 */
 	old_ws = roq->ws;
-	if (unlikely(nsn >= 1024)) {
-		dev_err(dev, "SW BUG? queue_update_ws nsn %u (sn %u ws %u)\n",
-			nsn, sn, roq->ws);
-		i2400m_roq_log_dump(i2400m, roq);
-		i2400m_reset(i2400m, I2400M_RT_WARM);
-	} else {
-		/* if the queue is empty, don't bother as we'd queue
-		 * it and inmediately unqueue it -- just deliver it */
-		if (len == 0) {
-			struct i2400m_roq_data *roq_data;
-			roq_data = (struct i2400m_roq_data *) &skb->cb;
-			i2400m_net_erx(i2400m, skb, roq_data->cs);
-		}
-		else
-			__i2400m_roq_queue(i2400m, roq, skb, sn, nsn);
-		__i2400m_roq_update_ws(i2400m, roq, sn + 1);
-		i2400m_roq_log_add(i2400m, roq, I2400M_RO_TYPE_PACKET_WS,
-				   old_ws, len, sn, nsn, roq->ws);
-	}
+	/* If the queue is empty, don't bother as we'd queue
+	 * it and immediately unqueue it -- just deliver it.
+	 */
+	if (len == 0) {
+		struct i2400m_roq_data *roq_data;
+		roq_data = (struct i2400m_roq_data *) &skb->cb;
+		i2400m_net_erx(i2400m, skb, roq_data->cs);
+	} else
+		__i2400m_roq_queue(i2400m, roq, skb, sn, nsn);
+
+	__i2400m_roq_update_ws(i2400m, roq, sn + 1);
+	i2400m_roq_log_add(i2400m, roq, I2400M_RO_TYPE_PACKET_WS,
+			   old_ws, len, sn, nsn, roq->ws);
+
 	d_fnend(2, dev, "(i2400m %p roq %p skb %p sn %u) = void\n",
 		i2400m, roq, skb, sn);
 	return;
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 15/25] wimax/i2400m: increase tx queue length from 5 to 20 [v1]
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>

This patch increases the tx_queue_len to 20 so as to
minimize the jitter in the throughput.

Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
 drivers/net/wimax/i2400m/netdev.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wimax/i2400m/netdev.c b/drivers/net/wimax/i2400m/netdev.c
index fc3754a..7d7b5ef 100644
--- a/drivers/net/wimax/i2400m/netdev.c
+++ b/drivers/net/wimax/i2400m/netdev.c
@@ -88,7 +88,11 @@ enum {
 	 * might take to get out of IDLE / negotiate it with the base
 	 * station. We add 1sec for good measure. */
 	I2400M_TX_TIMEOUT = 21 * HZ,
-	I2400M_TX_QLEN = 5,
+	/*
+	 * Experimentation has determined that, 20 to be a good value
+	 * for minimizing the jitter in the throughput.
+	 */
+	I2400M_TX_QLEN = 20,
 };
 
 
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 14/25] wimax/i2400m: fix system freeze caused by an infinite loop [v1]
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>

This patch fixes an infinite loop caused by i2400m_tx_fifo_push() due
to a corner case where there is no tail space in the TX FIFO.
Please refer the documentation in the code for details.

Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
 drivers/net/wimax/i2400m/tx.c |   65 +++++++++++++++++++++++++++++++++++++---
 1 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index 101550a..609f1ca 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -341,6 +341,14 @@ size_t __i2400m_tx_tail_room(struct i2400m *i2400m)
  * @padding: ensure that there is at least this many bytes of free
  *     contiguous space in the fifo. This is needed because later on
  *     we might need to add padding.
+ * @try_head: specify either to allocate head room or tail room space
+ *     in the TX FIFO. This boolean is required to avoids a system hang
+ *     due to an infinite loop caused by i2400m_tx_fifo_push().
+ *     The caller must always try to allocate tail room space first by
+ *     calling this routine with try_head = 0. In case if there
+ *     is not enough tail room space but there is enough head room space,
+ *     (i2400m_tx_fifo_push() returns TAIL_FULL) try to allocate head
+ *     room space, by calling this routine again with try_head = 1.
  *
  * Returns:
  *
@@ -372,6 +380,48 @@ size_t __i2400m_tx_tail_room(struct i2400m *i2400m)
  * fail and return TAIL_FULL and let the caller figure out if we wants to
  * skip the tail room and try to allocate from the head.
  *
+ * There is a corner case, wherein i2400m_tx_new() can get into
+ * an infinite loop calling i2400m_tx_fifo_push().
+ * In certain situations, tx_in would have reached on the top of TX FIFO
+ * and i2400m_tx_tail_room() returns 0, as described below:
+ *
+ * N  ___________ tail room is zero
+ *   |<-  IN   ->|
+ *   |           |
+ *   |           |
+ *   |           |
+ *   |   data    |
+ *   |<-  OUT  ->|
+ *   |           |
+ *   |           |
+ *   | head room |
+ * 0  -----------
+ * During such a time, where tail room is zero in the TX FIFO and if there
+ * is a request to add a payload to TX FIFO, which calls:
+ * i2400m_tx()
+ *         ->calls i2400m_tx_close()
+ *         ->calls i2400m_tx_skip_tail()
+ *         goto try_new;
+ *         ->calls i2400m_tx_new()
+ *                    |----> [try_head:]
+ *     infinite loop  |     ->calls i2400m_tx_fifo_push()
+ *                    |                if (tail_room < needed)
+ *                    |                   if (head_room => needed)
+ *                    |                       return TAIL_FULL;
+ *                    |<----  goto try_head;
+ *
+ * i2400m_tx() calls i2400m_tx_close() to close the message, since there
+ * is no tail room to accommodate the payload and calls
+ * i2400m_tx_skip_tail() to skip the tail space. Now i2400m_tx() calls
+ * i2400m_tx_new() to allocate space for new message header calling
+ * i2400m_tx_fifo_push() that returns TAIL_FULL, since there is no tail space
+ * to accommodate the message header, but there is enough head space.
+ * The i2400m_tx_new() keeps re-retrying by calling i2400m_tx_fifo_push()
+ * ending up in a loop causing system freeze.
+ *
+ * This corner case is avoided by using a try_head boolean,
+ * as an argument to i2400m_tx_fifo_push().
+ *
  * Note:
  *
  *     Assumes i2400m->tx_lock is taken, and we use that as a barrier
@@ -380,7 +430,8 @@ size_t __i2400m_tx_tail_room(struct i2400m *i2400m)
  *     pop data off the queue
  */
 static
-void *i2400m_tx_fifo_push(struct i2400m *i2400m, size_t size, size_t padding)
+void *i2400m_tx_fifo_push(struct i2400m *i2400m, size_t size,
+			  size_t padding, bool try_head)
 {
 	struct device *dev = i2400m_dev(i2400m);
 	size_t room, tail_room, needed_size;
@@ -395,7 +446,7 @@ void *i2400m_tx_fifo_push(struct i2400m *i2400m, size_t size, size_t padding)
 	}
 	/* Is there space at the tail? */
 	tail_room = __i2400m_tx_tail_room(i2400m);
-	if (tail_room < needed_size) {
+	if (!try_head && tail_room < needed_size) {
 		/*
 		 * If the tail room space is not enough to push the message
 		 * in the TX FIFO, then there are two possibilities:
@@ -510,14 +561,16 @@ void i2400m_tx_new(struct i2400m *i2400m)
 {
 	struct device *dev = i2400m_dev(i2400m);
 	struct i2400m_msg_hdr *tx_msg;
+	bool try_head = 0;
 	BUG_ON(i2400m->tx_msg != NULL);
 try_head:
-	tx_msg = i2400m_tx_fifo_push(i2400m, I2400M_TX_PLD_SIZE, 0);
+	tx_msg = i2400m_tx_fifo_push(i2400m, I2400M_TX_PLD_SIZE, 0, try_head);
 	if (tx_msg == NULL)
 		goto out;
 	else if (tx_msg == TAIL_FULL) {
 		i2400m_tx_skip_tail(i2400m);
 		d_printf(2, dev, "new TX message: tail full, trying head\n");
+		try_head = 1;
 		goto try_head;
 	}
 	memset(tx_msg, 0, I2400M_TX_PLD_SIZE);
@@ -591,7 +644,7 @@ void i2400m_tx_close(struct i2400m *i2400m)
 	aligned_size = ALIGN(tx_msg_moved->size, i2400m->bus_tx_block_size);
 	padding = aligned_size - tx_msg_moved->size;
 	if (padding > 0) {
-		pad_buf = i2400m_tx_fifo_push(i2400m, padding, 0);
+		pad_buf = i2400m_tx_fifo_push(i2400m, padding, 0, 0);
 		if (unlikely(WARN_ON(pad_buf == NULL
 				     || pad_buf == TAIL_FULL))) {
 			/* This should not happen -- append should verify
@@ -657,6 +710,7 @@ int i2400m_tx(struct i2400m *i2400m, const void *buf, size_t buf_len,
 	unsigned long flags;
 	size_t padded_len;
 	void *ptr;
+	bool try_head = 0;
 	unsigned is_singleton = pl_type == I2400M_PT_RESET_WARM
 		|| pl_type == I2400M_PT_RESET_COLD;
 
@@ -702,11 +756,12 @@ try_new:
 	/* So we have a current message header; now append space for
 	 * the message -- if there is not enough, try the head */
 	ptr = i2400m_tx_fifo_push(i2400m, padded_len,
-				  i2400m->bus_tx_block_size);
+				  i2400m->bus_tx_block_size, try_head);
 	if (ptr == TAIL_FULL) {	/* Tail is full, try head */
 		d_printf(2, dev, "pl append: tail full\n");
 		i2400m_tx_close(i2400m);
 		i2400m_tx_skip_tail(i2400m);
+		try_head = 1;
 		goto try_new;
 	} else if (ptr == NULL) {	/* All full */
 		result = -ENOSPC;
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 13/25] wimax/i2400m: modify i2400m_tx_fifo_push() to check for head room space in the TX FIFO [v1]
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>

This fixes i2400m_tx_fifo_push(); the check for having enough
space in the TX FIFO's tail was obscure and broken in certain
corner cases. The new check works in all cases and is way
clearer. Please refer the documentation in the code for details.

Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
 drivers/net/wimax/i2400m/tx.c |   16 ++++++++++++++--
 1 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index 1725f2b..101550a 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -396,8 +396,20 @@ void *i2400m_tx_fifo_push(struct i2400m *i2400m, size_t size, size_t padding)
 	/* Is there space at the tail? */
 	tail_room = __i2400m_tx_tail_room(i2400m);
 	if (tail_room < needed_size) {
-		if (i2400m->tx_out % I2400M_TX_BUF_SIZE
-		    < i2400m->tx_in % I2400M_TX_BUF_SIZE) {
+		/*
+		 * If the tail room space is not enough to push the message
+		 * in the TX FIFO, then there are two possibilities:
+		 * 1. There is enough head room space to accommodate
+		 * this message in the TX FIFO.
+		 * 2. There is not enough space in the head room and
+		 * in tail room of the TX FIFO to accommodate the message.
+		 * In the case (1), return TAIL_FULL so that the caller
+		 * can figure out, if the caller wants to push the message
+		 * into the head room space.
+		 * In the case (2), return NULL, indicating that the TX FIFO
+		 * cannot accommodate the message.
+		 */
+		if (room - tail_room >= needed_size) {
 			d_printf(2, dev, "fifo push %zu/%zu: tail full\n",
 				 size, padding);
 			return TAIL_FULL;	/* There might be head space */
-- 
1.6.6.1


^ permalink raw reply related

* [patch 2.6.35 12/25] wimax/i2400m: fix BUILD_BUG_ON() to use the maximum message size constant [v1]
From: Inaky Perez-Gonzalez @ 2010-05-14 21:45 UTC (permalink / raw)
  To: netdev, wimax; +Cc: Prasanna S. Panchamukhi
In-Reply-To: <cover.1273708027.git.inaky.perez-gonzalez@intel.com>

From: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>

The older method of computing the maximum PDU size relied
on a method that doesn't work when we prop the maximum
number of payloads up to the physical limit, and thus we kill
the whole computation and just verify that the constants are
congruent.

Signed-off-by: Prasanna S. Panchamukhi <prasannax.s.panchamukhi@intel.com>
---
 drivers/net/wimax/i2400m/tx.c |   20 +++++---------------
 1 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/drivers/net/wimax/i2400m/tx.c b/drivers/net/wimax/i2400m/tx.c
index a5002c8..1725f2b 100644
--- a/drivers/net/wimax/i2400m/tx.c
+++ b/drivers/net/wimax/i2400m/tx.c
@@ -287,19 +287,6 @@ enum {
 	 * documents, the maximum size of each message can be up to 16KiB.
 	 */
 	I2400M_TX_MSG_SIZE = 16384,
-	/*
-	 * 16 byte aligned MAX_MTU + 4 byte payload prefix.
-	 */
-	I2400M_MAX_MTU_ALIGN = 16,
-	I2400M_TX_PDU_SIZE = I2400M_MAX_MTU % I2400M_MAX_MTU_ALIGN
-	+ I2400M_MAX_MTU + sizeof(struct i2400m_pl_data_hdr),
-	 /*
-	  * 256 byte aligned toal size of 12 PDUs including msg header,
-	  */
-	I2400M_TX_PDU_ALIGN = 256,
-	I2400M_TX_PDU_TOTAL_SIZE = ((I2400M_TX_PDU_SIZE * I2400M_TX_PLD_MAX
-	+ sizeof(struct i2400m_msg_hdr))/I2400M_TX_PDU_ALIGN + 1)
-	* I2400M_TX_PDU_ALIGN * 2,
 };
 
 #define TAIL_FULL ((void *)~(unsigned long)NULL)
@@ -915,8 +902,11 @@ int i2400m_tx_setup(struct i2400m *i2400m)
 		goto error_kmalloc;
 	}
 
-	 /* Warn if the calculated buffer size exceeds I2400M_TX_BUF_SIZE. */
-	BUILD_BUG_ON(I2400M_TX_PDU_TOTAL_SIZE > I2400M_TX_BUF_SIZE);
+	/*
+	 * Fail the build if we can't fit at least two maximum size messages
+	 * on the TX FIFO [one being delivered while one is constructed].
+	 */
+	BUILD_BUG_ON(2 * I2400M_TX_MSG_SIZE > I2400M_TX_BUF_SIZE);
 	spin_lock_irqsave(&i2400m->tx_lock, flags);
 	i2400m->tx_sequence = 0;
 	i2400m->tx_in = 0;
-- 
1.6.6.1


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox