Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] usbnet: Do not implement ethtool get_link() if link state is unknown
From: Ben Hutchings @ 2009-11-04  3:20 UTC (permalink / raw)
  To: David Miller; +Cc: david-b, greg, jacmet, steve.glendinning, netdev
In-Reply-To: <20091103.020444.120631587.davem@davemloft.net>

[-- Attachment #1: Type: text/plain, Size: 868 bytes --]

On Tue, 2009-11-03 at 02:04 -0800, David Miller wrote:
> From: David Brownell <david-b@pacbell.net>
> Date: Tue, 3 Nov 2009 02:41:27 -0700
> 
> > On Tuesday 03 November 2009, David Miller wrote:
> >> All kidding aside, I think the alternative is for the USB network
> >> driver to call ethtool_op_get_link() if it cannot determine the
> >> link state in hardware.
> > 
> > There's usbnet_get_link() which does just that.  But
> > there may be some ancient debris confusing things.
> 
> It's perfect, and Ben's patch is completely unnecessary.

I don't see how it's 'perfect' since it reports the link as up where it
is really unknown.  Still, this is a fairly minor bug.

Ben.

-- 
Ben Hutchings
The generation of random numbers is too important to be left to chance.
                                                            - Robert Coveyou

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: HTB accuracy on 10GbE
From: Ryousei Takano @ 2009-11-04  3:45 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: Patrick McHardy, Linux Netdev List, takano-ryousei
In-Reply-To: <b30d1c3b0911031913k2c922935vc9605b76ac43168f@mail.gmail.com>

On Wed, Nov 4, 2009 at 12:13 PM, Ryousei Takano <ryousei@gmail.com> wrote:
> Hi Patrick and Stephen,
>
> Thanks for your comments.
>
> I retried on the newer kernel and iproute2, and added the experimental result
> on my page.  Please see 'Experimental result 2':
>    http://code.google.com/p/pspacer/wiki/HTBon10GbE
>
> The accuracy improves compared with the previous experiment.
> The difference reduces from +810 Mbps to +430 Mbps.
> It is because the timer resolution improves from 1 usec to 1/64 usec.
> But it is not perfect.
>
Oops, not 1/64 usec but 1/16 usec.


> Best regards,
> Ryousei Takano
>
>
> On Tue, Nov 3, 2009 at 5:53 AM, Stephen Hemminger <shemminger@vyatta.com> wrote:
>> On Mon, 02 Nov 2009 16:43:42 +0100
>> Patrick McHardy <kaber@trash.net> wrote:
>>
>>> Ryousei Takano wrote:
>>> > Hi Stephen and all,
>>> >
>>> > I have observed a HTB accuracy problem on the Linux kernel 2.6.30 and
>>> > the Myri-10G 10 GbE NIC.
>>> > HTB can control the transmission rate at Gigabit speed, however it can
>>> > not work well at 10 Gigabit speed.
>>> >
>>> > I asked Stephen this problem at Japan Linux Symposium.  He mentioned a
>>> > HTB bug related to the timer granularity.
>>> > I want to know what is happen, and what should be do for fixing it.
>>> >
>>> > Any comments and suggestions will be welcome.
>>> >
>>> > For more detail, please see the following page:
>>> > http://code.google.com/p/pspacer/wiki/HTBon10GbE
>>>
>>> This is not an easy problem to fix. Userspace, the kernel and the
>>> netlink API use 32 bit for timing related values, which is too small
>>> to use more than microsecond resolution. All of them need to be
>>> converted to use bigger types, additionally some kind of compatibility
>>> handling to deal with old iproute versions still using microsecond
>>> resolution is required.
>>
>> The existing API is a legacy mish-mash. The field is limited to 32 bits,
>> but it might be possible to use a finer scale.
>>
>> Maybe if kernel advertised finer resolution through /proc/net/psched
>> then table could be finer grained. This would maintain compatibility
>> between kernel and user space. You would need to have new kernel and
>> new iproute to get nanosecond resolution but older combinations would
>> still work.
>>
>> The downside is that by using nanosecond resolution the rates are upper
>> bounded at 4.2seconds / packet.
>>
>>
>

^ permalink raw reply

* Re: HTB accuracy on 10GbE
From: Eric Dumazet @ 2009-11-04  5:03 UTC (permalink / raw)
  To: Ryousei Takano
  Cc: Stephen Hemminger, Patrick McHardy, Linux Netdev List,
	takano-ryousei
In-Reply-To: <b30d1c3b0911031913k2c922935vc9605b76ac43168f@mail.gmail.com>

Ryousei Takano a écrit :
> Hi Patrick and Stephen,
> 
> Thanks for your comments.
> 
> I retried on the newer kernel and iproute2, and added the experimental result
> on my page.  Please see 'Experimental result 2':
>     http://code.google.com/p/pspacer/wiki/HTBon10GbE
> 
> The accuracy improves compared with the previous experiment.
> The difference reduces from +810 Mbps to +430 Mbps.
> It is because the timer resolution improves from 1 usec to 1/64 usec.
> But it is not perfect.
> 

Hmm, do you know part of the error comes from the user tool itself ?

If you check iperf results at sender and receiver you'll see different
values, sender lies a bit.

Tried here on a Gbit link (I dont have 10Gbe yet)

$ ./iperf.bench.sh
.100 104
.200 206
.300 307
.400 413
.500 515
.600 610
.700 715
.800 822
.900 913
1.000 945

while on receiver :
[  4]  0.0- 5.3 sec  62.8 MBytes    100 Mbits/sec
[  5]  0.0- 5.1 sec    123 MBytes    202 Mbits/sec
[  4]  0.0- 5.1 sec    183 MBytes    303 Mbits/sec
[  5]  0.0- 5.1 sec    246 MBytes    409 Mbits/sec
[  4]  0.0- 5.0 sec    307 MBytes    511 Mbits/sec
[  5]  0.0- 5.0 sec    364 MBytes    607 Mbits/sec
[  4]  0.0- 5.0 sec    427 MBytes    711 Mbits/sec
[  5]  0.0- 5.0 sec    490 MBytes    818 Mbits/sec
[  4]  0.0- 5.0 sec    545 MBytes    909 Mbits/sec
[  5]  0.0- 5.0 sec    565 MBytes    941 Mbits/sec


You might use longer intervals to reduce this error (10 secs instead of 5 secs)

$./iperf.bench.sh
.100 102
.200 204
.300 305
.400 410
.500 513
.600 608
.700 713
.800 820
.900 911
1.000 943

^ permalink raw reply

* Re: HTB accuracy on 10GbE
From: Eric Dumazet @ 2009-11-04  5:27 UTC (permalink / raw)
  To: Ryousei Takano
  Cc: Stephen Hemminger, Patrick McHardy, Linux Netdev List,
	takano-ryousei
In-Reply-To: <4AF10B1D.4050604@gmail.com>

Eric Dumazet a écrit :
> 
> Hmm, do you know part of the error comes from the user tool itself ?
> 
> If you check iperf results at sender and receiver you'll see different
> values, sender lies a bit.
> 
> Tried here on a Gbit link (I dont have 10Gbe yet)
> 
> $ ./iperf.bench.sh
> .100 104
> .200 206
> .300 307
> .400 413
> .500 515
> .600 610
> .700 715
> .800 822
> .900 913
> 1.000 945
> 
(that was with standard 1500 MTU)

Now, with 9000 MTU and 50 seconds samples (instead of 5 s) I get :

$ ./iperf.bench.sh
.100 101
.200 200
.300 301
.400 401
.500 500
.600 601
.700 700
.800 803
.900 903
1.000 991

Not too bad :)

^ permalink raw reply

* Re: 2.6.32-rc5-mmotm1101 - kernel BUG at net/ipv4/tcp_input.c:3707!
From: Gilad Ben-Yossef @ 2009-11-04  6:27 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: Valdis.Kletnieks, Eric Dumazet, Andrew Morton, linux-kernel,
	netdev, Ori Finkelman
In-Reply-To: <Pine.LNX.4.64.0911032330090.3488@melkinkari.cs.Helsinki.FI>

Ilpo Järvinen wrote:

> On Tue, 3 Nov 2009, Gilad Ben-Yossef wrote:
>
>   
>> Eric Dumazet wrote:
>>
>>     
>>> Valdis.Kletnieks@vt.edu a écrit :
>>>   
>>>       
>>>> Seen right after I started 'fetchmail'.  Reproducible - 3 out of 3.
>>>> I'll bisect this tonight if nobody jumps up and yells they know what it
>>>> is...
>>>>     
>>>>         
>> Bah... this is most probably my fault. Sorry about that.
>>
>> Can you please try the patch in the next email?
>>
>> But also, can you please send me the route table in effect when this happened
>> and the fetchmail command line/config (removing any passwords or account
>> details of course)? I want to understand better when this happens.
>>     
>
> According to the stacktrace, it came from ipv6 side which doesn't have any 
> null checking what so ever atm (you only handled ipv4 correctly). ...You 
> should be a bit more careful next time when adding any BUG_ONs...
>   
I agree, but for my defense I should add this was not just plain 
carelessness, I believed
that the dst_entry cannot be NULL at that location. That was obviously 
wrong. :-(

Gilad





-- 
Gilad Ben-Yossef
Chief Coffee Drinker & CTO
Codefidence Ltd.

Web:   http://codefidence.com
Cell:  +972-52-8260388
Skype: gilad_codefidence
Tel:   +972-8-9316883 ext. 201
Fax:   +972-8-9316884
Email: gilad@codefidence.com

Check out our Open Source technology and training blog - http://tuxology.net

	"The biggest risk you can take it is to take no risk."
		-- Mark Zuckerberg and probably others

^ permalink raw reply

* Re: 2.6.32-rc5-mmotm1101 - kernel BUG at net/ipv4/tcp_input.c:3707!
From: Gilad Ben-Yossef @ 2009-11-04  6:38 UTC (permalink / raw)
  To: Valdis.Kletnieks
  Cc: Ilpo Järvinen, Eric Dumazet, Andrew Morton, linux-kernel,
	netdev, Ori Finkelman
In-Reply-To: <7916.1257300093@turing-police.cc.vt.edu>

Valdis.Kletnieks@vt.edu wrote:

> On Tue, 03 Nov 2009 23:34:19 +0200, Ilpo JÀrvinen said:
>
>   
>> On Tue, 3 Nov 2009, Gilad Ben-Yossef wrote:
>>
>>     
>>> Eric Dumazet wrote:
>>>
>>>       
>>>> Valdis.Kletnieks@vt.edu a écrit :
>>>>   
>>>>         
>>>>> Seen right after I started 'fetchmail'.  Reproducible - 3 out of 3.
>>>>> I'll bisect this tonight if nobody jumps up and yells they know what it
>>>>> is...
>>>>>     
>>>>>           
>>> Bah... this is most probably my fault. Sorry about that.
>>>
>>> Can you please try the patch in the next email?
>>>       
>
> Tried while at home, machine panic'ed.  No netconsole here at the moment, sorry.
>   

Ok, thanks.That is ... strange.

I didn't manage to recreate this here with a simple IPv6 set and netcat
as server client but I will try further, but if there is any way to send me
the crash location that would be a big help. Thanks.
>   
>
> Why was this blowing chunks in the IPv6 when I was making an IPv4 connection
> then? I just noticed that. My fetchmail can't make an IPv6 TCP connection to
> our IMAP server because the server isn't v6-enabled yet.  And although I
> contact our DNS over IPv6, but that's UDP not TCP.
>
>   
I don't think the chunk blowing occurred due to the connection to the IMAP
server. That codes deals with incoming SYNs. I guess it happened when 
fetchmail
tried to connect to the local mail daemon and this should be happening 
over the loopback interface...


Gilad


-- 
Gilad Ben-Yossef
Chief Coffee Drinker & CTO
Codefidence Ltd.

Web:   http://codefidence.com
Cell:  +972-52-8260388
Skype: gilad_codefidence
Tel:   +972-8-9316883 ext. 201
Fax:   +972-8-9316884
Email: gilad@codefidence.com

Check out our Open Source technology and training blog - http://tuxology.net

	"The biggest risk you can take it is to take no risk."
		-- Mark Zuckerberg and probably others

^ permalink raw reply

* REGRESSION: On 2.6.32-rc5 the firmware hangs, and the nic is unusable.
From: Eric W. Biederman @ 2009-11-04  7:40 UTC (permalink / raw)
  To: Dhananjay Phadke; +Cc: netdev


On 2.6.31.5 I get:
ethtool -i eth0
driver: netxen_nic
version: 4.0.30
firmware-version: 4.0.305
bus-info: 0000:06:00.0

And everything works except bonding.

On 2.6.32-rc5 I dhcp I get an IP
there is a sanity test ping.

Then the driver reports a firmware hang and
the interface goes down and I am dead in the water.

I'm a bit frustrated with this as it seems with each kernel release
the driver gets a little bit less usable.

Have I given you enough information to track this down?

Eric

^ permalink raw reply

* Re: HTB accuracy on 10GbE
From: Ryousei Takano @ 2009-11-04  8:19 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Stephen Hemminger, Patrick McHardy, Linux Netdev List,
	takano-ryousei
In-Reply-To: <4AF110CE.8070701@gmail.com>

Hi Eric,

On Wed, Nov 4, 2009 at 2:27 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Eric Dumazet a écrit :
>>
>> Hmm, do you know part of the error comes from the user tool itself ?
>>
>> If you check iperf results at sender and receiver you'll see different
>> values, sender lies a bit.
>>
>> Tried here on a Gbit link (I dont have 10Gbe yet)
>>
>> $ ./iperf.bench.sh
>> .100 104
>> .200 206
>> .300 307
>> .400 413
>> .500 515
>> .600 610
>> .700 715
>> .800 822
>> .900 913
>> 1.000 945
>>
> (that was with standard 1500 MTU)
>
> Now, with 9000 MTU and 50 seconds samples (instead of 5 s) I get :
>
> $ ./iperf.bench.sh
> .100 101
> .200 200
> .300 301
> .400 401
> .500 500
> .600 601
> .700 700
> .800 803
> .900 903
> 1.000 991
>
> Not too bad :)
>

I tried iperf with 60 seconds samples. I got the almost same result.

Here is the result:
      sender	receiver
1.000 1.00	1.00
2.000 2.01	2.01
3.000 3.03	3.02
4.000 4.07	4.07
5.000 5.05	5.05
6.000 6.16	6.16
7.000 7.22	7.22
8.000 8.15	8.15
9.000 9.23	9.23
9.900 9.69	9.69

Best regards,
Ryousei Takano

^ permalink raw reply

* Re: [PATCH] sch_htb.c consume the classes's tokens bellow the HTB_CAN_SEND level
From: Jarek Poplawski @ 2009-11-04  8:28 UTC (permalink / raw)
  To: Changli Gao; +Cc: Jamal Hadi Salim, devik, netdev
In-Reply-To: <412e6f7f0911031753m4af1467fn1b0326bdf17fe48b@mail.gmail.com>

On Wed, Nov 04, 2009 at 09:53:52AM +0800, Changli Gao wrote:
> On Wed, Nov 4, 2009 at 7:00 AM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> > On Tue, Nov 03, 2009 at 09:18:49PM +0800, Changli Gao wrote:
> >> On Tue, Nov 3, 2009 at 6:05 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> >> >
> >> > The ceil specification is controlled only by ctokens, which are always
> >> > updated, so no such risk.
> >> >
> >> Nevertheless, updating tokens is necessary too.
> >
> > If it's really necessary you should present some test case fixed by
> > your patch, I guess.
> >
> > In the meantime let's consider what could be broken:
> > class 1:1 (parent) rate 10 packets/sec
> > class 1:2 rate 5 packets/sec ceil 10 packets/sec
> > class 1:3 rate 5 packets/sec ceil 10 packets/sec
> >
> > class 1:2 doesn't use all its rate, and sends every other second
> >        (in even seconds)
> > class 1:3 sends 10 packets during the first second, so with your
> >        patch it will use its tokens for 2 seconds
> > class 1:2 uses its rate in the second second..., so class 1:1
> >        can't lend anything
> > class 1:3 can only borrow, so it won't be able to send during
> >        this second anything
> >
> > So, the effect would be class 1:3 sending every odd second 10 packets
> > while every even second - nothing...
> 
> class 1:3 can send, as its parent rate is 10, but class 1:2 only uses
> half of it, and class 1:1 is still in HTB_CAN_SEND mode.
> 
> The result is, hasn't any difference with or without my patch :
> class 1:1 sends 10 packets in odd seconds, and 5 packets in even seconds.

I guess you meant class 1:3, and there is a difference: it sends 5
packets in even seconds only if it manages to borrow from 1:1, but
it's not _guaranteed_ at all. In this particular case it's quite
probable class 1:2 will send 10 packets in even seconds instead, or
with some finer borrowing control it could be: class 1:2 8 packets,
class 1:3 2 packets, as well.

> class 1:2 sends 5 packets in even seconds.
> class 1:1 (parent) sends 10 packets in every second.
> 
> Let's think this case in another way: which class sends packets in
> even seconds first, class 1:2 or class 1:3.
> With my patch, as 1:3 in HTB_MAY_BORROW mode, and 1:2 in HTB_CAN_SEND
> mode, so 1:2 sends all its 5 packets first.
> Without my patch, as 1:2 and 1:3 are both in HTB_CAN_SEND mode, the
> sequence is undetermined. In other word, 1:2 and 1:3 are treated
> fairly, and it isn't fair for 1:2, because 1:2 sends nothing in odd
> seconds, and has no deficit in rate as 1:3.

The token bucket (cl->buffer) is just to account distinctly when a
class is entitled to send and when it actually does send within its
rate. The fairness is controlled by classes itself with HTB_CAN_SEND
state. Using this bucket to account for borrowed sending deprives us
of this precise information. "The fairness" would be controlled by
priorities of borrowing instead (see above).

Regards,
Jarek P.

^ permalink raw reply

* Re: [PATCH] usbnet: Do not implement ethtool get_link() if link state is unknown
From: David Miller @ 2009-11-04  8:36 UTC (permalink / raw)
  To: ben; +Cc: david-b, greg, jacmet, steve.glendinning, netdev
In-Reply-To: <1257304800.3136.452.camel@localhost>

From: Ben Hutchings <ben@decadent.org.uk>
Date: Wed, 04 Nov 2009 03:20:00 +0000

> I don't see how it's 'perfect' since it reports the link as up where it
> is really unknown.  Still, this is a fairly minor bug.

That is the policy we enforce for all drivers, especially virtual
ones.  If you can't tell, you specify that the link is up.

Otherwise automated tools like NetworkManager et al. will not even
attempt to bring do DHCP and bring the network device online.

^ permalink raw reply

* Re: [announce] new rt2800 drivers for Ralink wireless & project tree
From: Ingo Molnar @ 2009-11-04  8:37 UTC (permalink / raw)
  To: Ivo van Doorn
  Cc: Bartlomiej Zolnierkiewicz, linux-wireless, linux-kernel, netdev,
	Randy Dunlap, Luis Correia, John W. Linville, Johannes Berg,
	Jarek Poplawski, Pekka Enberg, David Miller
In-Reply-To: <200911032200.04516.IvDoorn@gmail.com>


* Ivo van Doorn <ivdoorn@gmail.com> wrote:

> >       MAINTAINERS: add rt2800 entry
> 
> I see you decided to take over the maintainership? Doesn't that need 
> the current maintainer to move away, or was this part of the "going 
> over other peoples head" plan?
>
> [...]
>
> These are too much (and too big) patches for me to review at once, 
> I'll look at them later.

Frankly, having read through the recent discussions related to the 
rt2800pci/usb drivers, the subtle (and largely undeserved) group 
violence and abuse you are inflicting on Bart is stomach-turning.

The non-working rt2800pci driver has been pending in your private tree 
for how long, 1.5 _years_?

Look at the diffstat of Bart's driver:

   15 files changed, 4036 insertions(+), 7158 deletions(-)

He reduced your 5.2 KLOC non-working driver into a 1.8 KLOC _working_ 
driver.

And _still_ your complaint about Bart's series is that he updated the 
MAINTAINERS entry and added an entry for rt2800? Heck _sure_ he should 
update it, he is the one doing the hard work of trying to bring it to 
users, trying to clean up a messy driver space, trying to turn crap into 
gold.

The thing is, if you dont have the time or interest to listen to and act 
upon review feedback, be constructive about it and fix (obvious) 
structural problems in your rt2800 code, you should just step aside and 
let Bart maintain what he is apparently more capable of maintaining than 
you are.

What you are doing here is a thinly veiled land-grab: you did a minimal 
token driver for rt2800 that doesnt work, kept it in your private tree 
for _1.5 years_, and the moment someone _else_ came along and did 
something better and more functional in drivers/staging/, you discovered 
your sudden interest for it and moved the crappy driver upstream at 
lightning's speed (it is already in net-next AFAICS, despite negative 
test and review feedback) - ignoring and throwing away all the work that 
Bart has done.

Such behavior wouldnt fly in _any_ other Linux subsystem, but apparently 
there is one set of rules for upstream kernel maintainers and then 
there's another, different set of rules for upstream wireless driver 
maintainers.

Really, you should listen to contrary opinion and _you_ should work 
_hard_ to integrate Bart socially and open up your close circle of 
wireless insiders instead of fighting his 'outsider' contributions every 
which way. We dont care if people are rough, express displeasure and 
show strong opinion about crappy code - but the moment you are 
_excluding_ capable people and playing petty office politics (like you 
are very clearly doing it with Bart here) everyone loses.

Guys, show some minimal amount of honesty, openness and critical 
thinking please ...

	Ingo

^ permalink raw reply

* Re: [Bugme-new] [Bug 14427] New: ipv6 forward cause strange route
From: Pekka Savola @ 2009-11-04  8:01 UTC (permalink / raw)
  To: YOSHIFUJI Hideaki
  Cc: Andrew Morton, green, netdev, bugzilla-daemon, bugme-daemon,
	davem
In-Reply-To: <4AF011A6.1070903@linux-ipv6.org>

On Tue, 3 Nov 2009, YOSHIFUJI Hideaki wrote:
>>  RFC 2526: Reserved IPv6 Subnet Anycast Addresses
>>  RFC 3627: Use of /127 Prefix Length Between Routers Considered Harmful
>
> I should say "Subnet-router anycast address" and
> RFC3513: Internet Protocol Version 6 (IPv6) Addressing
> Architecture".

FWIW, please note that commercial vendors haven't implemented this 
very extensively, and there's also some recent activity arguing this 
is not very useful:

http://tools.ietf.org/html/draft-kohno-ipv6-prefixlen-p2p-00

We'll likely get better idea how folks react to this during the next 
week's IETF.

-- 
Pekka Savola                 "You each name yourselves king, yet the
Netcore Oy                    kingdom bleeds."
Systems. Networks. Security. -- George R.R. Martin: A Clash of Kings

^ permalink raw reply

* Re: [PATCH] sch_htb.c consume the classes's tokens bellow the HTB_CAN_SEND level
From: Changli Gao @ 2009-11-04  9:16 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: Jamal Hadi Salim, devik, netdev
In-Reply-To: <20091104082808.GA6224@ff.dom.local>

On Wed, Nov 4, 2009 at 4:28 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
>> >
>> > If it's really necessary you should present some test case fixed by
>> > your patch, I guess.
>> >
>> > In the meantime let's consider what could be broken:
>> > class 1:1 (parent) rate 10 packets/sec
>> > class 1:2 rate 5 packets/sec ceil 10 packets/sec
>> > class 1:3 rate 5 packets/sec ceil 10 packets/sec
>> >
>> > class 1:2 doesn't use all its rate, and sends every other second
>> >        (in even seconds)
>> > class 1:3 sends 10 packets during the first second, so with your
>> >        patch it will use its tokens for 2 seconds
>> > class 1:2 uses its rate in the second second..., so class 1:1
>> >        can't lend anything
>> > class 1:3 can only borrow, so it won't be able to send during
>> >        this second anything
>> >
>> > So, the effect would be class 1:3 sending every odd second 10 packets
>> > while every even second - nothing...
>>
>> class 1:3 can send, as its parent rate is 10, but class 1:2 only uses
>> half of it, and class 1:1 is still in HTB_CAN_SEND mode.
>>
>> The result is, hasn't any difference with or without my patch :
>> class 1:1 sends 10 packets in odd seconds, and 5 packets in even seconds.
>
> I guess you meant class 1:3.

You are right. :)

> and there is a difference: it sends 5
> packets in even seconds only if it manages to borrow from 1:1, but
> it's not _guaranteed_ at all. In this particular case it's quite
> probable class 1:2 will send 10 packets in even seconds instead, or
> with some finer borrowing control it could be: class 1:2 8 packets,
> class 1:3 2 packets, as well.

It is just correct. You focus on 1 second fairness, while I focus on 2
seconds fairness.

>
>> class 1:2 sends 5 packets in even seconds.
>> class 1:1 (parent) sends 10 packets in every second.
>>
>> Let's think this case in another way: which class sends packets in
>> even seconds first, class 1:2 or class 1:3.
>> With my patch, as 1:3 in HTB_MAY_BORROW mode, and 1:2 in HTB_CAN_SEND
>> mode, so 1:2 sends all its 5 packets first.
>> Without my patch, as 1:2 and 1:3 are both in HTB_CAN_SEND mode, the
>> sequence is undetermined. In other word, 1:2 and 1:3 are treated
>> fairly, and it isn't fair for 1:2, because 1:2 sends nothing in odd
>> seconds, and has no deficit in rate as 1:3.
>
> The token bucket (cl->buffer) is just to account distinctly when a
> class is entitled to send and when it actually does send within its
> rate. The fairness is controlled by classes itself with HTB_CAN_SEND
> state. Using this bucket to account for borrowed sending deprives us
> of this precise information. "The fairness" would be controlled by
> priorities of borrowing instead (see above).
>
The token bucket and ctoken bucket both use cl->mbuffer to control
rate granularities. If we don't account token bucket when the
corresponding class in HTB_MAY_BORROW mode, the cl->mbuffer will
become useless.


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* [Patch] net: fix incorrect counting in __scm_destroy()
From: Amerigo Wang @ 2009-11-04 10:04 UTC (permalink / raw)
  To: linux-kernel; +Cc: netdev, Amerigo Wang, David S. Miller


It seems that in __scm_destroy() we forgot to decrease
the ->count after fput(->fp[i]), this may cause some
problem when we recursively call fput() again.

Signed-off-by: WANG Cong <amwang@redhat.com>
Cc: David S. Miller <davem@davemloft.net>

---
diff --git a/net/core/scm.c b/net/core/scm.c
index b7ba91b..fa53219 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -120,8 +120,10 @@ void __scm_destroy(struct scm_cookie *scm)
 				fpl = list_first_entry(&work_list, struct scm_fp_list, list);
 
 				list_del(&fpl->list);
-				for (i=fpl->count-1; i>=0; i--)
+				for (i = fpl->count-1; i >= 0; i--) {
 					fput(fpl->fp[i]);
+					fpl->count--;
+				}
 				kfree(fpl);
 			}
 

^ permalink raw reply related

* Re: [PATCH] e1000: the power down when running ifdown command
From: Naohiro Ooiwa @ 2009-11-04 10:23 UTC (permalink / raw)
  To: Stephen Hemminger
  Cc: jeffrey.t.kirsher, jesse.brandeburg, peter.p.waskiewicz.jr,
	john.ronciak, davem, Andrew Morton, netdev, svaidy, e1000-devel
In-Reply-To: <4AEE35D2.4000503@miraclelinux.com>

Naohiro Ooiwa wrote:
> Stephen Hemminger wrote:
>> On Sat, 31 Oct 2009 18:39:52 +0900
>> Naohiro Ooiwa <nooiwa@miraclelinux.com> wrote:
>>
>> Does this work with Wake On Lan? 
> 
> Yes, it works WOL.

Sorry, I made a mistake.
The WOL doesn't work when my patch applied to kernel.
I wasn't myself.

I consider the WOL and I will resent the patch.
Thank you for your point.


thanks,
Naohiro Ooiwa


> But I worry that my test is enough.
> 
> They are following:
>   - simple data transmission after ifdown;ifup.
>   - enable wol, ifup network device, system shutdown, and make sure wol work.
>   - enable wol, ifdown network device, system shutdown, and make sure wol work.
>   - while [ 0 ] ; do ifdown eth0 ; ifup eth0 ; done
>   - while [ 0 ] ; do modprobe e1000 ; rmmod e1000 ; done
> 
> 
>> @@ -1265,6 +1287,7 @@ static int e1000_open(struct net_device *netdev)
>>  		goto err_setup_rx;
>>
>>  	e1000_power_up_phy(adapter);
>> +	e1000_reset(adapter);
>>
>>  	adapter->mng_vlan_id = E1000_MNG_VLAN_NONE;
>>  	if ((hw->mng_cookie.status &
> 
> This code fix problem that e1000 driver doesn't work to auto-negotiation
> once in a while.
> Maybe, the cause is that set state to D0 just before it.
> I found it by repeat of ifup and ifdown.
> 
> If you find out other points and any necessary tests from my patch,
> please tell me. I will make sure them.
> 
> Thanks you.
> Naohiro Ooiwa
> 
> --
> 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

* Re: [PATCH] e1000: the power down when running ifdown command
From: Naohiro Ooiwa @ 2009-11-04 10:27 UTC (permalink / raw)
  To: Jeff Kirsher
  Cc: jesse.brandeburg, peter.p.waskiewicz.jr, john.ronciak, davem,
	Andrew Morton, netdev, svaidy, e1000-devel
In-Reply-To: <9929d2390911031337p6115fef0wc1b94e5141ead341@mail.gmail.com>

Jeff Kirsher wrote:
> 2009/11/3 Naohiro Ooiwa <nooiwa@miraclelinux.com>:
>> Jeff Kirsher wrote:
>>> 2009/10/31 Naohiro Ooiwa <nooiwa@miraclelinux.com>:
>>>
>>> I have added this patch to my tree for testing.  This patch requires a
>>> fair amount of regression testing, so once its passed testing I will
>>> push the patch to David/netdev.
>> I appreciate the marge your tree.
>> If there is anything I can do, please let me know.
>>
>> And I know this patch is good for e100 driver too.
>> I would really like to create patch for it.
>> How do you think about e100 driver.
>>
>>
>> Thanks,
>> Naohiro Ooiwa
>>
> 
> Patches are always welcome (referring to a e100 patch).

I am happy that you should say that.
I will try to create a patch for e100, e1000e, igb and ixgbe.
Before that, I should fix the following problems.


> As far as the e1000 patch goes, it has a number of issues which were
> found in testing.  Here are just a few problems we saw:
> 1. ethtool -t - crashes the system
> 2. ethtool eth0 - always shows link/speed as 1000/Full even when there
> is no cable
> 3. ethtool -s eth0 autoneg on/off - system hang. Sometimes a copper
> interface will show up as fiber after this.
> 4. ethtool -d/-S/-g etc - will corrupt the stats of the interface
> while doing ifup/down

Thank you for your tests.
Oh, My patch is full of problem.
The rest is my work.

I will resend the patch after test of all ethtool's options.
At that time, I will tell you contents of my tests.

And I sad WOL works on my patch in previous mail.
But WOL doesn't work. Sorry, I wasn't myself.
I will fix it too.

> 
> So it appears that more needs to be done to the driver to get this to
> work as expected.
> 
> NAK
> 








^ permalink raw reply

* Re: [Patch] net: fix incorrect counting in __scm_destroy()
From: Eric Dumazet @ 2009-11-04 10:29 UTC (permalink / raw)
  To: Amerigo Wang; +Cc: linux-kernel, netdev, David S. Miller
In-Reply-To: <20091104100717.4785.57149.sendpatchset@localhost.localdomain>

Amerigo Wang a écrit :
> It seems that in __scm_destroy() we forgot to decrease
> the ->count after fput(->fp[i]), this may cause some
> problem when we recursively call fput() again.
> 
> Signed-off-by: WANG Cong <amwang@redhat.com>
> Cc: David S. Miller <davem@davemloft.net>
> 
> ---
> diff --git a/net/core/scm.c b/net/core/scm.c
> index b7ba91b..fa53219 100644
> --- a/net/core/scm.c
> +++ b/net/core/scm.c
> @@ -120,8 +120,10 @@ void __scm_destroy(struct scm_cookie *scm)
>  				fpl = list_first_entry(&work_list, struct scm_fp_list, list);
>  
>  				list_del(&fpl->list);
> -				for (i=fpl->count-1; i>=0; i--)
> +				for (i = fpl->count-1; i >= 0; i--) {
>  					fput(fpl->fp[i]);
> +					fpl->count--;
> +				}
>  				kfree(fpl);
>  			}
>  

Hmm, your patch seems suspicious.

Are you fixing a real crash/bug, or is it something you discovered in a code review ?

Given we kfree(fpl) at the end of loop, we cannot recursively call __scm_destroy()
on same fpl, it would be a bug anyway ?

So you probably need something better, like testing fpl->list being not re-included
in current->scm_work_list before kfree() it 

^ permalink raw reply

* Re: [PATCH] sch_htb.c consume the classes's tokens bellow the HTB_CAN_SEND level
From: Jarek Poplawski @ 2009-11-04 10:42 UTC (permalink / raw)
  To: Changli Gao; +Cc: Jamal Hadi Salim, devik, netdev
In-Reply-To: <412e6f7f0911040116q6b25b705k83e5a45464698af1@mail.gmail.com>

On Wed, Nov 04, 2009 at 05:16:42PM +0800, Changli Gao wrote:
...
> It is just correct. You focus on 1 second fairness, while I focus on 2
> seconds fairness.

The whole example was very simplified, so it all would certainly
differ in time and real sends, especially with an interaction of
more classes. But, generally, main HTB algorithm seems to be quite
well tested against various fair and unfair cases, starting from
the author's examples:
http://luxik.cdi.cz/~devik/qos/htb/manual/userg.htm
so, I guess, this type of a bug would really show somewhere long time
ago.

> The token bucket and ctoken bucket both use cl->mbuffer to control
> rate granularities. If we don't account token bucket when the
> corresponding class in HTB_MAY_BORROW mode, the cl->mbuffer will
> become useless.

cl->mbuffer is only to limit some extreme effects, so more of an
exception, not a main tool of rate control. (It really should be
useless most of the time if classes don't stop sending and aren't
deprived of their full rate for really long time.)

Regards,
Jarek P.

^ permalink raw reply

* Re: [PATCH] sch_htb.c consume the classes's tokens bellow the HTB_CAN_SEND level
From: Martin Devera @ 2009-11-04 10:58 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: Changli Gao, Jamal Hadi Salim, netdev
In-Reply-To: <20091104104245.GB6224@ff.dom.local>

Jarek Poplawski wrote:
> On Wed, Nov 04, 2009 at 05:16:42PM +0800, Changli Gao wrote:
> ...
>> It is just correct. You focus on 1 second fairness, while I focus on 2
>> seconds fairness.
> 
> The whole example was very simplified, so it all would certainly
> differ in time and real sends, especially with an interaction of
> more classes. But, generally, main HTB algorithm seems to be quite
> well tested against various fair and unfair cases, starting from
> the author's examples:
> http://luxik.cdi.cz/~devik/qos/htb/manual/userg.htm
> so, I guess, this type of a bug would really show somewhere long time
> ago.
> 
>> The token bucket and ctoken bucket both use cl->mbuffer to control
>> rate granularities. If we don't account token bucket when the
>> corresponding class in HTB_MAY_BORROW mode, the cl->mbuffer will
>> become useless.
> 
> cl->mbuffer is only to limit some extreme effects, so more of an
> exception, not a main tool of rate control. (It really should be
> useless most of the time if classes don't stop sending and aren't
> deprived of their full rate for really long time.)

Hello,

yes you are right. If I remember correctly, I tried to charge "rate"
tokens up to root in early versions (which makes some sense) but problem
is with many interior classes stuck at extreme mbuffer value, losing
responsivity. Also it didn't played well with prio settings (class
stalls).
Bacause we don't charge "rate" now, some amount of burst can accumulate
in each interior class and decrease fairness at the same prio level a 
bit. However from measurments I did, these bursts of unfairness are
hidded by regular "bursting" of rate and ceil token buckets.

devik

^ permalink raw reply

* Re: [PATCHv7 3/3] vhost_net: a kernel-level virtio server
From: Andi Kleen @ 2009-11-04 11:08 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: netdev, virtualization, kvm, linux-kernel, mingo, linux-mm, akpm
In-Reply-To: <20091103172422.GD5591@redhat.com>

"Michael S. Tsirkin" <mst@redhat.com> writes:

Haven't really read the whole thing, just noticed something at a glance.

> +/* Expects to be always run from workqueue - which acts as
> + * read-size critical section for our kind of RCU. */
> +static void handle_tx(struct vhost_net *net)
> +{
> +	struct vhost_virtqueue *vq = &net->dev.vqs[VHOST_NET_VQ_TX];
> +	unsigned head, out, in, s;
> +	struct msghdr msg = {
> +		.msg_name = NULL,
> +		.msg_namelen = 0,
> +		.msg_control = NULL,
> +		.msg_controllen = 0,
> +		.msg_iov = vq->iov,
> +		.msg_flags = MSG_DONTWAIT,
> +	};
> +	size_t len, total_len = 0;
> +	int err, wmem;
> +	size_t hdr_size;
> +	struct socket *sock = rcu_dereference(vq->private_data);
> +	if (!sock)
> +		return;
> +
> +	wmem = atomic_read(&sock->sk->sk_wmem_alloc);
> +	if (wmem >= sock->sk->sk_sndbuf)
> +		return;
> +
> +	use_mm(net->dev.mm);

I haven't gone over all this code in detail, but that isolated reference count
use looks suspicious. What prevents the mm from going away before
you increment, if it's not the current one?

-Andi 

-- 
ak@linux.intel.com -- Speaking for myself only.

^ permalink raw reply

* Re: [PATCH] sch_htb.c consume the classes's tokens bellow the HTB_CAN_SEND level
From: Changli Gao @ 2009-11-04 11:21 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: Jamal Hadi Salim, devik, netdev
In-Reply-To: <20091104104245.GB6224@ff.dom.local>

On Wed, Nov 4, 2009 at 6:42 PM, Jarek Poplawski <jarkao2@gmail.com> wrote:
> On Wed, Nov 04, 2009 at 05:16:42PM +0800, Changli Gao wrote:
> ...
>> It is just correct. You focus on 1 second fairness, while I focus on 2
>> seconds fairness.
>
> The whole example was very simplified, so it all would certainly
> differ in time and real sends, especially with an interaction of
> more classes. But, generally, main HTB algorithm seems to be quite
> well tested against various fair and unfair cases, starting from
> the author's examples:
> http://luxik.cdi.cz/~devik/qos/htb/manual/userg.htm
> so, I guess, this type of a bug would really show somewhere long time
> ago.
>

This documentation is old, and after the last update 5.5.2002, there
were still bugs: http://luxik.cdi.cz/~devik/qos/htb/ .

>> The token bucket and ctoken bucket both use cl->mbuffer to control
>> rate granularities. If we don't account token bucket when the
>> corresponding class in HTB_MAY_BORROW mode, the cl->mbuffer will
>> become useless.
>
> cl->mbuffer is only to limit some extreme effects, so more of an
> exception, not a main tool of rate control. (It really should be
> useless most of the time if classes don't stop sending and aren't
> deprived of their full rate for really long time.)
>

I don't think so. Although a class's tokens may be negative, but its
ctokens may be positive. Charging its tokens is to prevent its cmode
from being changed to HTB_CAN_SEND from HTB_CANT_SEND directly.


-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: [PATCH] sch_htb.c consume the classes's tokens bellow the HTB_CAN_SEND level
From: Changli Gao @ 2009-11-04 11:30 UTC (permalink / raw)
  To: Martin Devera; +Cc: Jarek Poplawski, Jamal Hadi Salim, netdev
In-Reply-To: <4AF15E73.30806@cdi.cz>

On Wed, Nov 4, 2009 at 6:58 PM, Martin Devera <martin.devera@cdi.cz> wrote:
>
> Hello,
>
> yes you are right. If I remember correctly, I tried to charge "rate"
> tokens up to root in early versions (which makes some sense) but problem
> is with many interior classes stuck at extreme mbuffer value, losing
> responsivity.

It is strange. Would you like describe it more clearly?

> Also it didn't played well with prio settings (class
> stalls).

Was it the cause?

> Bacause we don't charge "rate" now, some amount of burst can accumulate
> in each interior class and decrease fairness at the same prio level a bit.
> However from measurments I did, these bursts of unfairness are
> hidded by regular "bursting" of rate and ceil token buckets.
>

One case for unfairness.



-- 
Regards,
Changli Gao(xiaosuo@gmail.com)

^ permalink raw reply

* Re: HTB accuracy on 10GbE
From: Eric Dumazet @ 2009-11-04 11:31 UTC (permalink / raw)
  To: Ryousei Takano
  Cc: Stephen Hemminger, Patrick McHardy, Linux Netdev List,
	takano-ryousei
In-Reply-To: <b30d1c3b0911040019q14a007d4lbd1b695db8884b27@mail.gmail.com>

Ryousei Takano a écrit :

> 
> I tried iperf with 60 seconds samples. I got the almost same result.
> 
> Here is the result:
>       sender	receiver
> 1.000 1.00	1.00
> 2.000 2.01	2.01
> 3.000 3.03	3.02
> 4.000 4.07	4.07
> 5.000 5.05	5.05
> 6.000 6.16	6.16
> 7.000 7.22	7.22
> 8.000 8.15	8.15
> 9.000 9.23	9.23
> 9.900 9.69	9.69
> 

One thing to consider is the estimation error in qdisc_l2t(), rate table has only 256 slots

static inline u32 qdisc_l2t(struct qdisc_rate_table* rtab, unsigned int pktlen)
{
	int slot = pktlen + rtab->rate.cell_align + rtab->rate.overhead;
	if (slot < 0)
		slot = 0;
	slot >>= rtab->rate.cell_log;
	if (slot > 255)
		return (rtab->data[255]*(slot >> 8) + rtab->data[slot & 0xFF]);
	return rtab->data[slot];
}


Maybe you can try changing class mtu to 40000 instead of 9000, and quantum to 60000 too

tc class add dev $DEV parent 1: classid 1:1 htb rate ${rate}mbit mtu 40000 quantum 60000

(because your tcp stack sends large buffers ( ~ 60000 bytes) as your NIC can offload tcp segmentation)


^ permalink raw reply

* Re: [PATCH] sch_htb.c consume the classes's tokens bellow the HTB_CAN_SEND level
From: Jarek Poplawski @ 2009-11-04 11:49 UTC (permalink / raw)
  To: Changli Gao; +Cc: Jamal Hadi Salim, devik, netdev
In-Reply-To: <412e6f7f0911040321o22c536fdid078f6d2225a90a0@mail.gmail.com>

On Wed, Nov 04, 2009 at 07:21:48PM +0800, Changli Gao wrote:
...
> > cl->mbuffer is only to limit some extreme effects, so more of an
> > exception, not a main tool of rate control. (It really should be
> > useless most of the time if classes don't stop sending and aren't
> > deprived of their full rate for really long time.)
> >
> 
> I don't think so. Although a class's tokens may be negative, but its
> ctokens may be positive. Charging its tokens is to prevent its cmode
> from being changed to HTB_CAN_SEND from HTB_CANT_SEND directly.

I think, you should really better show some tests proving your patch
is needed and doesn't affect a case I described, instead of trying to
discuss the meaninig of all HTB variables here.

Regards,
Jarek P.

^ permalink raw reply

* Re: [PATCH] sch_htb.c consume the classes's tokens bellow the  HTB_CAN_SEND level
From: Martin Devera @ 2009-11-04 11:51 UTC (permalink / raw)
  To: Changli Gao; +Cc: Jarek Poplawski, Jamal Hadi Salim, netdev
In-Reply-To: <412e6f7f0911040330uc5ebbcekfef57854fce501f@mail.gmail.com>

Changli Gao wrote:
> On Wed, Nov 4, 2009 at 6:58 PM, Martin Devera <martin.devera@cdi.cz> wrote:
>> Hello,
>>
>> yes you are right. If I remember correctly, I tried to charge "rate"
>> tokens up to root in early versions (which makes some sense) but problem
>> is with many interior classes stuck at extreme mbuffer value, losing
>> responsivity.
> 
> It is strange. Would you like describe it more clearly?

oops, forget it, I mixed it with different problem I remember :-)
The idea is to charge classe's tokens when it sends.
It always must be under ceil thus when it sends we can charge
ctokens (it must have some in order to be in CAN_SEND).
If it borrows, we don't charge it because doesn't send from
its own rate but rather from parent's rate. Thus charge
only parent. Because parent gave it rate as gift, free of charge,
it has it in addition to its own rate.


^ permalink raw reply


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