Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH net-next] fib: remove a useless synchronize_rcu() call
From: David Miller @ 2010-10-16 18:55 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1286980984.3876.679.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 13 Oct 2010 16:43:04 +0200

> fib_nl_delrule() calls synchronize_rcu() for no apparent reason,
> while rtnl is held.
> 
> I suspect it was done to avoid an atomic_inc_not_zero() in
> fib_rules_lookup(), which commit 7fa7cb7109d07 added anyway.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] fib_trie: use fls() instead of open coded loop
From: David Miller @ 2010-10-16 18:55 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev, Robert.Olsson
In-Reply-To: <1286988971.3876.974.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 13 Oct 2010 18:56:11 +0200

> fib_table_lookup() might use fls() to speedup an open coded loop.
> 
> Noticed while doing a profile analysis.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] fib: avoid false sharing on fib_table_hash
From: David Miller @ 2010-10-16 18:56 UTC (permalink / raw)
  To: eric.dumazet; +Cc: netdev
In-Reply-To: <1286994123.3876.1137.camel@edumazet-laptop>

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Wed, 13 Oct 2010 20:22:03 +0200

> While doing profile analysis, I found fib_hash_table was sometime in a
> cache line shared by a possibly often written kernel structure.
> 
> (CONFIG_IP_ROUTE_MULTIPATH || !CONFIG_IPV6_MULTIPLE_TABLES)
> 
> It's hard to detect because not easily reproductible.
> 
> Make sure we allocate a full cache line to keep this shared in all cpus
> caches.
> 
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied.

^ permalink raw reply

* Re: [PATCH] via-velocity: forced 1000 Mbps mode support.
From: David Miller @ 2010-10-16 18:56 UTC (permalink / raw)
  To: romieu; +Cc: davidlv.linux, netdev, DavidLv, ShirleyHu, AndersMa, rseguier
In-Reply-To: <20101013192605.GA5157@electric-eye.fr.zoreil.com>

From: Francois Romieu <romieu@fr.zoreil.com>
Date: Wed, 13 Oct 2010 21:26:05 +0200

> Full duplex only. Half duplex 1000 Mbps is not supported.
> 
> Signed-off-by: David Lv <DavidLv@viatech.com.cn>
> Acked-by: Francois Romieu <romieu@fr.zoreil.com>
> Tested-by: Seguier Regis <rseguier@e-teleport.net>

Applied.

^ permalink raw reply

* Re: [PATCH net-next] tipc: cleanup function namespace
From: David Miller @ 2010-10-16 18:56 UTC (permalink / raw)
  To: shemminger; +Cc: paul.gortmaker, nhorman, netdev, allan.stephens
In-Reply-To: <20101013162035.0c2e8123@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 13 Oct 2010 16:20:35 -0700

> Do some cleanups of TIPC based on make namespacecheck
>   1. Don't export unused symbols
>   2. Eliminate dead code
>   3. Make functions and variables local
>   4. Rename buf_acquire to tipc_buf_acquire since it is used in several files
> 
> Compile tested only.
> This make break out of tree kernel modules that depend on TIPC routines.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

I really think we can and should do this now, so I've
applied this to net-next-2.6

Thanks everyone.

^ permalink raw reply

* Re: [PATCH net-next 1/2]: stmmac: make ethtool functions local
From: David Miller @ 2010-10-16 18:57 UTC (permalink / raw)
  To: shemminger; +Cc: peppe.cavallaro, deepak.sikri, netdev
In-Reply-To: <20101013175031.41c29349@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 13 Oct 2010 17:50:31 -0700

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

Applied.

^ permalink raw reply

* Re: [PATCH net-next 2/2] stmmac: make function tables const
From: David Miller @ 2010-10-16 18:57 UTC (permalink / raw)
  To: shemminger; +Cc: peppe.cavallaro, deepak.sikri, netdev
In-Reply-To: <20101013175125.6f6a64db@nehalam>

From: Stephen Hemminger <shemminger@vyatta.com>
Date: Wed, 13 Oct 2010 17:51:25 -0700

> These tables only contain function pointers.
> 
> Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>

Applied.

^ permalink raw reply

* Re: [PATCH -next] net: move MII outside of NET_ETHERNET, fix kconfig warning
From: David Miller @ 2010-10-16 18:58 UTC (permalink / raw)
  To: randy.dunlap; +Cc: linux-kernel, netdev, akpm, jgarzik
In-Reply-To: <20101013181859.fc3a96fd.randy.dunlap@oracle.com>

From: Randy Dunlap <randy.dunlap@oracle.com>
Date: Wed, 13 Oct 2010 18:18:59 -0700

> From: Randy Dunlap <randy.dunlap@oracle.com>
> 
> We have USB, PCMCIA, and gigabit ethernet drivers that select
> MII even though NET_ETHERNET is not enabled, so make MII not
> be dependent on NET_ETHERNET.  It is still dependent on NET
> and NETDEVICES.
> 
> Fixes kconfig unmet dependency warning (shortened, was very long string):
 ...
> Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
> Acked-by: Jeff Garzik <jgarzik@pobox.com> [2006-NOV-30]

Applied, and I added your infiniband patch too.

Thanks Randy.

^ permalink raw reply

* Re: openvswitch/flow WAS ( Re: [rfc] Merging the Open vSwitch datapath
From: Jesse Gross @ 2010-10-16 19:33 UTC (permalink / raw)
  To: hadi; +Cc: Ben Pfaff, netdev, ovs-team
In-Reply-To: <1287228959.3664.72.camel@bigi>

On Sat, Oct 16, 2010 at 4:35 AM, jamal <hadi@cyberus.ca> wrote:
> On Fri, 2010-10-15 at 14:35 -0700, Jesse Gross wrote:
>
>>
>> You're right, at a high level, it appears that there is a bit of an
>> overlap between bridging, tc, and Open vSwitch.
>
> It looks like openvswitch rides on top of openflow, correct?
> earlier i was looking at  openflow/datapath but gleaning
> openvswitch/datapath it still looks conceptually the same
> at the lower level.

Yes, Open vSwitch supports the OpenFlow protocol.  However, the Open
vSwitch kernel portion is completely different from the OpenFlow
reference implementation datapath and in fact does not speak OpenFlow
at the kernel level.  You brought up the point of keeping the kernel
simple and making policy decisions in userspace.  I completely agree
and, in fact, that is the reason why Open vSwitch is designed the way
it is.

I think it might be helpful if I gave a high level overview of packet
processing:

When a packet is received it, the relevant fields from the packet are
extracted and matched against a hash table.  The most interesting part
is actually what happens when the packets don't match a hash entry:
they get sent up to userspace.  It is userspace that makes a policy
decision about the traffic and then pushes down a flow entry for
future packets to match.  Some of the things that those decisions can
be based on include: OpenFlow rules, wildcarded entries, normal L2
learning, etc.  From then on, packets in that flow can be processed on
the fast path in the kernel with minimal overhead, while still getting
the benefit of the knowledge of userspace.

So I think that we are actually in agreement on quite a number of
points: the kernel should be kept as simple as possible, the control
plane should be abstracted out and handled in userspace, and it should
be possible to map the control rules (from OpenFlow or anywhere
really) onto a simpler set of primitives for handling packets.

So with those goals in mind, here's what is needed:
1. Packet field extraction and classification.  Realistically speaking
a new, specialized classifier would probably be needed, as you
mention.
2. A mechanism to send/receive packets to/from userspace.  This is an
important component that Open vSwitch adds to the pipeline.  This will
probably expand in the future to suit different applications, like the
security processing that I talked about.
3. Output actions.  A few exist today, at least some new ones will
need to be added.

So in reality, all of major components of Open vSwitch are actually
not present in the kernel today.  I know the argument could be made
that certains parts can be replicated in different ways but that's
back to the simplicity point that I was making earlier.  The u32
classifier isn't well suited for these types of rules and neither is
pedit.  If we're going to add the needed components either way, let's
not make everyone's lives more complicated by mixing everything
together.

^ permalink raw reply

* Re: [Bugme-new] [Bug 19692] New: linux-2.6.36-rc5 crash with gianfar ethernet at full line rate traffic
From: Jarek Poplawski @ 2010-10-16 19:48 UTC (permalink / raw)
  To: emin ak
  Cc: Andrew Morton, netdev, bugzilla-daemon, bugme-daemon,
	Anton Vorontsov
In-Reply-To: <AANLkTinAj=9-RqNA6xKc_5ONz2hPMRGM8uJ2Xy7XJVdr@mail.gmail.com>

On Sat, Oct 16, 2010 at 02:14:01AM +0300, emin ak wrote:
> Hi Jarek.
> Sorry for delayed answer. As I promised, I have started the tests on
> Monday (in spite of dealing with The Monday Syndrome:)  I had applied
> your patch and after two billions packet (and approximatly four hours)
> passed kernel crashed with skb_over_panic error similar with first
> type. To ensure the patch is failed or not, I rerun the same test
> again. That time, surprisingly,  it did'nt crashed again for two days
> with same kernel. But this situation had occured before, I think
> sometimes because of randomness of the applied ethernet traffic and
> mostly because I cant apply all full line rate random traffic to my
> target device because of wrong test setup (switch / hardware packet
> generator settings etc..), it takes three or more day to crash the
> kernel and sometimes it never crashes. My device only and only crashes
> when I can apply full line rate random traffic. Before informing you
> and the list with (official) test results, I want to be sure with the
> truth of them. So that please let me apply more test to the target
> device for  a fews day more. After that I wish I'll came with good
> news!

Hi Emin,
Sorry for being impatient. Actually, waiting is no problem for me. I
simply didn't know how much this bug is reproducible, and was a bit
mislead by your forecast of one day result (and terrified btw seeing
words Monday and immediately side by side ;-) So, a few days or weeks,
no problem, it's all up to you.

On the other hand, it looks like there might be something else/more,
so I'd suggest to add this last skb_over_panic to bugzilla too.

Thanks,
Jarek P.

^ permalink raw reply

* Re: tbf/htb qdisc limitations
From: Jarek Poplawski @ 2010-10-16 20:58 UTC (permalink / raw)
  To: Bill Fink; +Cc: Eric Dumazet, Rick Jones, Steven Brudenell, netdev
In-Reply-To: <20101016005106.35e4cc8d.billfink@mindspring.com>

On Sat, Oct 16, 2010 at 12:51:06AM -0400, Bill Fink wrote:
> On Sat, 16 Oct 2010, Jarek Poplawski wrote:
> 
> > On Fri, Oct 15, 2010 at 05:37:46PM -0400, Bill Fink wrote:
> > ...
> > > i7test7% tc -s -d qdisc show dev eth2
> > > qdisc prio 1: root refcnt 33 bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
> > >  Sent 11028687119 bytes 1223828 pkt (dropped 293, overlimits 0 requeues 0) 
> > >  backlog 0b 0p requeues 0 
> > > qdisc tbf 10: parent 1:1 rate 8900Mbit burst 1112500b/64 mpu 0b lat 4295.0s 
> > >  Sent 11028687077 bytes 1223827 pkt (dropped 293, overlimits 593 requeues 0) 
> > >  backlog 0b 0p requeues 0 
> > > 
> > > I'm not sure how you can have so many dropped but not have
> > > any TCP retransmissions (or not show up as requeues).  But
> > > there's probably something basic I just don't understand
> > > about how all this stuff works.
> > 
> > Me either, but it seems higher "limit" might help with these drops.
> 
> You were of course correct about the higher limit helping.
> I finally upgraded the field system to 2.6.35, and did some
> testing on the real data path of interest, which has an RTT
> of about 29 ms.  I set up a rate limit of 8 Gbps using the
> following commands:
> 
> tc qdisc add dev eth2 root handle 1: prio
> tc qdisc add dev eth2 parent 1:1 handle 10: tbf rate 8000mbit limit 35000000 burst 20000 mtu 9000
> tc filter add dev eth2 protocol ip parent 1: prio 1 u32 match ip protocol 6 0xff match ip dst 192.168.1.23 flowid 10:1
> 
> hecn-i7sl1% nuttcp -T10 -i1 -w50m 192.168.1.23
>   676.3750 MB /   1.00 sec = 5673.4646 Mbps     0 retrans
>   948.5625 MB /   1.00 sec = 7957.1508 Mbps     0 retrans
>   948.8125 MB /   1.00 sec = 7959.5902 Mbps     0 retrans
>   948.3750 MB /   1.00 sec = 7955.5382 Mbps     0 retrans
>   949.0000 MB /   1.00 sec = 7960.6696 Mbps     0 retrans
>   948.7500 MB /   1.00 sec = 7958.7873 Mbps     0 retrans
>   948.6875 MB /   1.00 sec = 7958.0959 Mbps     0 retrans
>   948.6250 MB /   1.00 sec = 7957.4205 Mbps     0 retrans
>   948.7500 MB /   1.00 sec = 7958.7237 Mbps     0 retrans
>   948.4375 MB /   1.00 sec = 7956.3648 Mbps     0 retrans
> 
>  9270.5625 MB /  10.09 sec = 7707.7457 Mbps 24 %TX 36 %RX 0 retrans 29.38 msRTT
> 
> hecn-i7sl1% tc -s -d qdisc show dev eth2
> qdisc prio 1: root refcnt 33 bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
>  Sent 9779476756 bytes 1084943 pkt (dropped 0, overlimits 0 requeues 0) 
>  backlog 0b 0p requeues 0 
> qdisc tbf 10: parent 1:1 rate 8000Mbit burst 19000b/64 mpu 0b lat 35.0ms 
>  Sent 9779476756 bytes 1084943 pkt (dropped 0, overlimits 1831360 requeues 0) 
>  backlog 0b 0p requeues 0 
> 
> No drops!
> 
> BTW the effective rate limit seems to be a very coarse adjustment
> at these speeds.  I was seeing some data path issues at 8.9 Gbps
> so I tried setting slightly lower rates such as 8.8 Gbps, 8.7 Gbps,
> etc, but they still gave me an effective rate limit of about 8.9 Gbps.
> It wasn't until I got down to a setting of 8 Gbps that I actually
> got an effective rate limit of 8 Gbps.
> 
> Also the man page for tbf seems to be wrong/misleading about
> the burst parameter.  It states:
> 
> 	"If your buffer is too small, packets may be dropped because more
> 	tokens arrive per timer tick than fit in your bucket.  The minimum
> 	buffer size can be calculated by dividing the rate by HZ.
> 
> According to that, with a rate of 8 Gbps and HZ=1000, the minimum
> burst should be 1000000 bytes.  But my testing shows that a burst
> of just 20000 works just fine.  That's only 2 9000-byte packets
> or about 20 usec of traffic at the 8 Gbps rate.  Using too large
> a value for burst can actually be harmful as it allows the traffic
> to temporarily exceed the desired rate limit.

As I mentioned before, it could work, but your config is really on
the edge. Anyway, if lower than minimum buffer size is needed
something else is definitely wrong. (Btw, this size can matter less
with high resolution timers.) You could try if my iproute patch:
"tc_core: Use double in tc_core_time2tick()" (not merged) can help
here. While googling for this patch I found this page, which might be
interesting to you (besides the link to the thread with the patch at
the end, take 1 or 2, shouldn't matter):

http://code.google.com/p/pspacer/wiki/HTBon10GbE
 
If it doesn't help reconsider hfsc.

Thanks,
Jarek P.

^ permalink raw reply

* How to change delayed ACKs?
From: Fred . @ 2010-10-16 21:57 UTC (permalink / raw)
  To: linux-net

I play online games.
Someone told me about Leatrix Latency Fix.
http://www.wowinterface.com/downloads/info13581-LeatrixLatencyFix.html

ACKs are delayed. The latency fix works by changing setting to
decrease that delay.

On Windows it modifies 'TCPAckFrequency' in the registry.

On FreeBSD and Mac OS X;
sudo sysctl -w net.inet.tcp.delayed_ack=0

This key doesn't seem to exist on Linux.
error: "net.inet.tcp.delayed_ack" is an unknown key

Is there any way to reduce the delay of delayed ACKs in order to
increase performance in online gaming?

P.S im not subscribed to mailinglist

^ permalink raw reply

* r8169: high CPU load (softirq) bottlenecking throughput
From: Lasse Makholm @ 2010-10-16 21:54 UTC (permalink / raw)
  To: netdev

Hello netdev people,

I'm seeing what seems to be an unreasonably high CPU utilization from
the r8169 driver on an Intel Atom board.

Serving a 1GB blob repeatedly through Apache basically maxes out one
CPU core completely...

Here is a typical snapshot of the CPU load:

11:17:23 PM  CPU    %usr   %nice    %sys %iowait    %irq   %soft
%steal  %guest   %idle
11:17:24 PM  all    0.00    0.00    6.54    0.00    0.00   17.68
0.00    0.00   75.79
11:17:24 PM    0    0.00    0.00    0.00    0.00    0.00    0.00
0.00    0.00  100.00
11:17:24 PM    1    0.00    0.00    0.00    0.00    0.00    0.00
0.00    0.00  100.00
11:17:24 PM    2    0.00    0.00   27.00    0.00    0.00   73.00
0.00    0.00    0.00
11:17:24 PM    3    0.00    0.00    0.00    0.00    0.00    0.00
0.00    0.00  100.00

There is no disk IO while this is going on (the 1GB blob is cached),
and the machine is otherwise idle.

Throughput tops out at around 50MB/s but the receiving end and the
switch is easily able to get 100MB/s from another box.

The board is a D945GCLF2D with an Atom 330 (2 x 1.6 GHz), so
presumably it should be able to pump out 100MB/s easily...

For SSH-tunneled traffic, this is a disaster because both encryption
and I/O land on the same CPU... I'm currently maxing out at 10 - 20
MB/s with SSH-tunneled traffic

The kernel is currently 2.6.36-020636rc8-generic (Ubuntu daily
snapshot) but I've tried a handful of others between here and Ubuntu's
2.6.32-24....

lspci knows the following about the NIC:

makholm@korovyov:~$ lspci -vv -s 01:00.0
01:00.0 Ethernet controller: Realtek Semiconductor Co., Ltd.
RTL8111/8168B PCI Express Gigabit Ethernet controller (rev 02)
	Subsystem: Intel Corporation Device 0001
	Control: I/O+ Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr-
Stepping- SERR- FastB2B- DisINTx+
	Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort-
<TAbort- <MAbort- >SERR- <PERR- INTx-
	Latency: 0, Cache Line Size: 64 bytes
	Interrupt: pin A routed to IRQ 40
	Region 0: I/O ports at 1000 [size=256]
	Region 2: Memory at 88100000 (64-bit, non-prefetchable) [size=4K]
	Region 4: Memory at 88000000 (64-bit, prefetchable) [size=64K]
	Expansion ROM at 88020000 [disabled] [size=128K]
	Capabilities: <access denied>
	Kernel driver in use: r8169
	Kernel modules: r8169

makholm@korovyov:~$

I have a D-Link DGE-528T (also uses the r8169 driver) that shows
similar behavior in the Atom box. Trying that cards in an Athlon II
box also show a noticable CPU load by the r8169 driver, though the
Athlon obviously copes much better...

Any tips on how to improve the situation? I'd be happy to provide more
info and/or test some patches etc. if needed...

Thanks
/Lasse

^ permalink raw reply

* Re: "src" attribute ignored for IPv6 (preferred source address selection)
From: Daniel Roesen @ 2010-10-17  1:12 UTC (permalink / raw)
  To: netdev
In-Reply-To: <4C8215C8.8000908@httrack.com>

On Sat, Sep 04, 2010 at 11:47:52AM +0200, Xavier Roche wrote:
> In an attempt to change the source address in various conditions (network 
> prefix, outgoing protocol, user owner id, ..), it appeared that the src 
> attribute of an IPv6 route command (ip -6 route add .. src ..) was simply 
> ignored.
>
> The outgoing interface can be selected, but not the preferred source 
> address (see (1)), which remains always the same within an interface.
>
> If this a bug ? Or a known limitation of the kernel ?

Well, at least it was a known limitation six years ago, when I
discovered the same:

http://lkml.indiana.edu/hypermail/linux/kernel/0409.0/1768.html

Unfortunately I don't have the time (and most probably not the necessary
kernel knowhow) to hack this up myself, but I'm still interested. :-)

Best regards,
Daniel

-- 
CLUE-RIPE -- Jabber: dr@cluenet.de -- dr@IRCnet -- PGP: 0xA85C8AA0

^ permalink raw reply

* Re: tbf/htb qdisc limitations
From: Bill Fink @ 2010-10-17  1:24 UTC (permalink / raw)
  To: Jarek Poplawski; +Cc: Eric Dumazet, Rick Jones, Steven Brudenell, netdev
In-Reply-To: <20101016205824.GA2113@del.dom.local>

On Sat, 16 Oct 2010, Jarek Poplawski wrote:

> On Sat, Oct 16, 2010 at 12:51:06AM -0400, Bill Fink wrote:
> > On Sat, 16 Oct 2010, Jarek Poplawski wrote:
> > 
> > > On Fri, Oct 15, 2010 at 05:37:46PM -0400, Bill Fink wrote:
> > > ...
> > > > i7test7% tc -s -d qdisc show dev eth2
> > > > qdisc prio 1: root refcnt 33 bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
> > > >  Sent 11028687119 bytes 1223828 pkt (dropped 293, overlimits 0 requeues 0) 
> > > >  backlog 0b 0p requeues 0 
> > > > qdisc tbf 10: parent 1:1 rate 8900Mbit burst 1112500b/64 mpu 0b lat 4295.0s 
> > > >  Sent 11028687077 bytes 1223827 pkt (dropped 293, overlimits 593 requeues 0) 
> > > >  backlog 0b 0p requeues 0 
> > > > 
> > > > I'm not sure how you can have so many dropped but not have
> > > > any TCP retransmissions (or not show up as requeues).  But
> > > > there's probably something basic I just don't understand
> > > > about how all this stuff works.
> > > 
> > > Me either, but it seems higher "limit" might help with these drops.
> > 
> > You were of course correct about the higher limit helping.
> > I finally upgraded the field system to 2.6.35, and did some
> > testing on the real data path of interest, which has an RTT
> > of about 29 ms.  I set up a rate limit of 8 Gbps using the
> > following commands:
> > 
> > tc qdisc add dev eth2 root handle 1: prio
> > tc qdisc add dev eth2 parent 1:1 handle 10: tbf rate 8000mbit limit 35000000 burst 20000 mtu 9000
> > tc filter add dev eth2 protocol ip parent 1: prio 1 u32 match ip protocol 6 0xff match ip dst 192.168.1.23 flowid 10:1
> > 
> > hecn-i7sl1% nuttcp -T10 -i1 -w50m 192.168.1.23
> >   676.3750 MB /   1.00 sec = 5673.4646 Mbps     0 retrans
> >   948.5625 MB /   1.00 sec = 7957.1508 Mbps     0 retrans
> >   948.8125 MB /   1.00 sec = 7959.5902 Mbps     0 retrans
> >   948.3750 MB /   1.00 sec = 7955.5382 Mbps     0 retrans
> >   949.0000 MB /   1.00 sec = 7960.6696 Mbps     0 retrans
> >   948.7500 MB /   1.00 sec = 7958.7873 Mbps     0 retrans
> >   948.6875 MB /   1.00 sec = 7958.0959 Mbps     0 retrans
> >   948.6250 MB /   1.00 sec = 7957.4205 Mbps     0 retrans
> >   948.7500 MB /   1.00 sec = 7958.7237 Mbps     0 retrans
> >   948.4375 MB /   1.00 sec = 7956.3648 Mbps     0 retrans
> > 
> >  9270.5625 MB /  10.09 sec = 7707.7457 Mbps 24 %TX 36 %RX 0 retrans 29.38 msRTT
> > 
> > hecn-i7sl1% tc -s -d qdisc show dev eth2
> > qdisc prio 1: root refcnt 33 bands 3 priomap  1 2 2 2 1 2 0 0 1 1 1 1 1 1 1 1
> >  Sent 9779476756 bytes 1084943 pkt (dropped 0, overlimits 0 requeues 0) 
> >  backlog 0b 0p requeues 0 
> > qdisc tbf 10: parent 1:1 rate 8000Mbit burst 19000b/64 mpu 0b lat 35.0ms 
> >  Sent 9779476756 bytes 1084943 pkt (dropped 0, overlimits 1831360 requeues 0) 
> >  backlog 0b 0p requeues 0 
> > 
> > No drops!
> > 
> > BTW the effective rate limit seems to be a very coarse adjustment
> > at these speeds.  I was seeing some data path issues at 8.9 Gbps
> > so I tried setting slightly lower rates such as 8.8 Gbps, 8.7 Gbps,
> > etc, but they still gave me an effective rate limit of about 8.9 Gbps.
> > It wasn't until I got down to a setting of 8 Gbps that I actually
> > got an effective rate limit of 8 Gbps.
> > 
> > Also the man page for tbf seems to be wrong/misleading about
> > the burst parameter.  It states:
> > 
> > 	"If your buffer is too small, packets may be dropped because more
> > 	tokens arrive per timer tick than fit in your bucket.  The minimum
> > 	buffer size can be calculated by dividing the rate by HZ.
> > 
> > According to that, with a rate of 8 Gbps and HZ=1000, the minimum
> > burst should be 1000000 bytes.  But my testing shows that a burst
> > of just 20000 works just fine.  That's only 2 9000-byte packets
> > or about 20 usec of traffic at the 8 Gbps rate.  Using too large
> > a value for burst can actually be harmful as it allows the traffic
> > to temporarily exceed the desired rate limit.
> 
> As I mentioned before, it could work, but your config is really on
> the edge. Anyway, if lower than minimum buffer size is needed
> something else is definitely wrong. (Btw, this size can matter less
> with high resolution timers.) You could try if my iproute patch:
> "tc_core: Use double in tc_core_time2tick()" (not merged) can help
> here. While googling for this patch I found this page, which might be
> interesting to you (besides the link to the thread with the patch at
> the end, take 1 or 2, shouldn't matter):
> 
> http://code.google.com/p/pspacer/wiki/HTBon10GbE
>  
> If it doesn't help reconsider hfsc.

Thanks for the link.  From his results, it appears you can
get better accuracy by keeping TSO/GSO enabled and upping
the tc mtu parameter to 64000.  I will have to try that out.

For the very high bandwidth cases I tend to deal with, would
there be any advantage to further reducing the PSCHED_SHIFT
from its current value of 6?

					-Bill

^ permalink raw reply

* [PATCH resend] netfilter: fix kconfig unmet dependency warning
From: Randy Dunlap @ 2010-10-17  2:11 UTC (permalink / raw)
  To: netdev; +Cc: davem, netfilter-devel, Patrick McHardy

From: Randy Dunlap <randy.dunlap@oracle.com>

Fix netfilter kconfig unmet dependencies warning & spell out
"compatible" while there.

warning: (IP_NF_TARGET_TTL && NET && INET && NETFILTER && IP_NF_IPTABLES && NETFILTER_ADVANCED || IP6_NF_TARGET_HL && NET && INET && IPV6 && NETFILTER && IP6_NF_IPTABLES && NETFILTER_ADVANCED) selects NETFILTER_XT_TARGET_HL which has unmet direct dependencies ((IP_NF_MANGLE || IP6_NF_MANGLE) && NETFILTER_ADVANCED)

Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
 net/ipv4/netfilter/Kconfig |    4 ++--
 net/ipv6/netfilter/Kconfig |    4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

[orig. sent 2010-SEP-23]

--- linux-next-20100921.orig/net/ipv4/netfilter/Kconfig
+++ linux-next-20100921/net/ipv4/netfilter/Kconfig
@@ -324,10 +324,10 @@ config IP_NF_TARGET_ECN
 
 config IP_NF_TARGET_TTL
 	tristate '"TTL" target support'
-	depends on NETFILTER_ADVANCED
+	depends on NETFILTER_ADVANCED && IP_NF_MANGLE
 	select NETFILTER_XT_TARGET_HL
 	---help---
-	This is a backwards-compat option for the user's convenience
+	This is a backwards-compatible option for the user's convenience
 	(e.g. when running oldconfig). It selects
 	CONFIG_NETFILTER_XT_TARGET_HL.
 
--- linux-next-20100921.orig/net/ipv6/netfilter/Kconfig
+++ linux-next-20100921/net/ipv6/netfilter/Kconfig
@@ -132,10 +132,10 @@ config IP6_NF_MATCH_RT
 # The targets
 config IP6_NF_TARGET_HL
 	tristate '"HL" hoplimit target support'
-	depends on NETFILTER_ADVANCED
+	depends on NETFILTER_ADVANCED && IP6_NF_MANGLE
 	select NETFILTER_XT_TARGET_HL
 	---help---
-	This is a backwards-compat option for the user's convenience
+	This is a backwards-compatible option for the user's convenience
 	(e.g. when running oldconfig). It selects
 	CONFIG_NETFILTER_XT_TARGET_HL.
 
--

^ permalink raw reply

* Re: [PATCH 2/3] cxgb4: function namespace cleanup (v2)
From: Steve Wise @ 2010-10-17  3:30 UTC (permalink / raw)
  To: Dimitris Michailidis
  Cc: Stephen Hemminger, Divy Le Ray, David S. Miller, Casey Leedom,
	netdev
In-Reply-To: <4CB9434D.1060702@chelsio.com>


On 10/16/2010 1:16 AM, Dimitris Michailidis wrote:
> Stephen Hemminger wrote:
>> On Fri, 15 Oct 2010 18:11:42 -0700
>> Dimitris Michailidis <dm@chelsio.com> wrote:
>>
>>> Stephen Hemminger wrote:
>>>> Make functions only used in one file local.
>>>> Remove lots of dead code, relating to unsupported functions
>>>> in mainline driver like RSS, IPv6, and TCP offload.
>>> Thanks, this looks OK.  One exception, cxgb4_get_tcp_stats was 
>>> intended to be used by the rdma driver.  I see that driver doesn't 
>>> call it presently but if you don't mind can we give Steve a few 
>>> hours to tell us if he has any imminent plans to use it.  If he 
>>> doesn't offer to do something to use it for .37 it goes.
>>
>> The kernel source tree is not your development place holder tree.
>> At least #ifdef the code out for now.
>
> I am trying to protect Stephen Rothwell's time by checking that the IB 
> folks don't plan to add a call to this in their tree while we remove 
> the function in net-next.  There's supposed to be a call in the IB 
> driver.  I don't know why there isn't one or whether they are planning 
> to fix it for .37.  I see the potential for a linux-next conflict and 
> I am trying to avoid it.  #ifdef doesn't help, if it's not needed we 
> can remove it for good.

I'll add a patch this week to utilize the tcp stats.

Steve.


^ permalink raw reply

* Re: Documenting UNIX domain autobind
From: Tetsuo Handa @ 2010-10-17  5:28 UTC (permalink / raw)
  To: mtk.manpages; +Cc: netdev, mzxreary
In-Reply-To: <AANLkTimtTazMmDvQFgASsVGa=AYvPZqUaUmyg5p7VVY1@mail.gmail.com>

Michael Kerrisk wrote:
> I think this text correctly documents the technical details (but let
> me know if you see errors). What is lacking is an explanation of why
> this feature exists. Is someone able to explain where this feature is
> used and why?

What we can see is that unix_autobind() was added in Linux 2.1.15

  http://lxr.linux.no/linux-old+v2.1.15/net/unix/af_unix.c#L464
  addr->len = sprintf(addr->name->sun_path+1, "%08x", ordernum) + 1 + sizeof(short);

  49 * Differences from 2.0.0-11-... (ANK)
  50 *      Bug fixes and improvements.
  51 *              - client shutdown killed server socket.
  52 *              - removed all useless cli/sti pairs.
  53 *              - (suspicious!) not allow connect/send to connected not to us
  54 *                socket, return EPERM.
  55 *
  56 *      Semantic changes/extensions.
  57 *              - generic control message passing.
  58 *              - SCM_CREDENTIALS control message.
  59 *              - "Abstract" (not FS based) socket bindings.
  60 *                Abstract names are sequences of bytes (not zero terminated)
  61 *                started by 0, so that this name space does not intersect
  62 *                with BSD names.

and was changed to allow from 2^32 names to 2^20 names in Linux 2.3.15.

  http://lxr.linux.no/linux-old+v2.3.15/net/unix/af_unix.c#L514
  addr->len = sprintf(addr->name->sun_path+1, "%05x", ordernum) + 1 + sizeof(short);

I don't know the reason.

Regards.

^ permalink raw reply

* [PATCH 0/2] bonding: checkpatch.pl cleanups in bond_3ad.c
From: Bandan Das @ 2010-10-17  6:19 UTC (permalink / raw)
  To: David Miller; +Cc: LKML, NetDev, joe, Eric Dumazet, Bandan Das

Just some cleanups based on errors/warnings reported by checkpatch.pl.
There are still a ton of lines greater than 80 chars and I tried to fix 
a few that I came across.

Bandan Das (2):
  bonding: cleanup : add space around operators
  bonding: cleanup: remove braces from single block statements

 drivers/net/bonding/bond_3ad.c |  274 ++++++++++++++++++++--------------------
 1 files changed, 139 insertions(+), 135 deletions(-)

-- 
Bandan

^ permalink raw reply

* [PATCH 1/2] bonding: cleanup : add space around operators
From: Bandan Das @ 2010-10-17  6:19 UTC (permalink / raw)
  To: David Miller; +Cc: LKML, NetDev, joe, Eric Dumazet, Bandan Das
In-Reply-To: <1287296399-25142-1-git-send-email-bandan.das@stratus.com>

checkpatch.pl cleanup: Added spaces around operators at various places.
Also fixed some c99 style comments that I came across.

Signed-off-by: Bandan Das <bandan.das@stratus.com>
---
 drivers/net/bonding/bond_3ad.c |  111 ++++++++++++++++++++++++----------------
 1 files changed, 67 insertions(+), 44 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 079b9d1..6b06b00 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -93,7 +93,7 @@
 // compare MAC addresses
 #define MAC_ADDRESS_COMPARE(A, B) memcmp(A, B, ETH_ALEN)
 
-static struct mac_addr null_mac_addr = {{0, 0, 0, 0, 0, 0}};
+static struct mac_addr null_mac_addr = { { 0, 0, 0, 0, 0, 0 } };
 static u16 ad_ticks_per_sec;
 static const int ad_delta_in_ticks = (AD_TIMER_INTERVAL * HZ) / 1000;
 
@@ -329,7 +329,7 @@ static u16 __get_link_speed(struct port *port)
 	 * This is done in spite of the fact that the e100 driver reports 0 to be
 	 * compatible with MVT in the future.*/
 	if (slave->link != BOND_LINK_UP) {
-		speed=0;
+		speed = 0;
 	} else {
 		switch (slave->speed) {
 		case SPEED_10:
@@ -376,17 +376,17 @@ static u8 __get_duplex(struct port *port)
 	//  handling a special case: when the configuration starts with
 	// link down, it sets the duplex to 0.
 	if (slave->link != BOND_LINK_UP) {
-		retval=0x0;
+		retval = 0x0;
 	} else {
 		switch (slave->duplex) {
 		case DUPLEX_FULL:
-			retval=0x1;
+			retval = 0x1;
 			pr_debug("Port %d Received status full duplex update from adapter\n",
 				 port->actor_port_number);
 			break;
 		case DUPLEX_HALF:
 		default:
-			retval=0x0;
+			retval = 0x0;
 			pr_debug("Port %d Received status NOT full duplex update from adapter\n",
 				 port->actor_port_number);
 			break;
@@ -419,7 +419,7 @@ static inline void __initialize_port_locks(struct port *port)
  */
 static u16 __ad_timer_to_ticks(u16 timer_type, u16 par)
 {
-	u16 retval=0;	 //to silence the compiler
+	u16 retval = 0; /* to silence the compiler */
 
 	switch (timer_type) {
 	case AD_CURRENT_WHILE_TIMER:   // for rx machine usage
@@ -653,7 +653,7 @@ static void __update_ntt(struct lacpdu *lacpdu, struct port *port)
  */
 static void __attach_bond_to_agg(struct port *port)
 {
-	port=NULL; // just to satisfy the compiler
+	port = NULL; /* just to satisfy the compiler */
 	// This function does nothing since the parser/multiplexer of the receive
 	// and the parser/multiplexer of the aggregator are already combined
 }
@@ -668,7 +668,7 @@ static void __attach_bond_to_agg(struct port *port)
  */
 static void __detach_bond_from_agg(struct port *port)
 {
-	port=NULL; // just to satisfy the compiler
+	port = NULL; /* just to satisfy the compiler */
 	// This function does nothing sience the parser/multiplexer of the receive
 	// and the parser/multiplexer of the aggregator are already combined
 }
@@ -685,7 +685,9 @@ static int __agg_ports_are_ready(struct aggregator *aggregator)
 
 	if (aggregator) {
 		// scan all ports in this aggregator to verfy if they are all ready
-		for (port=aggregator->lag_ports; port; port=port->next_port_in_aggregator) {
+		for (port = aggregator->lag_ports;
+		     port;
+		     port = port->next_port_in_aggregator) {
 			if (!(port->sm_vars & AD_PORT_READY_N)) {
 				retval = 0;
 				break;
@@ -706,7 +708,8 @@ static void __set_agg_ports_ready(struct aggregator *aggregator, int val)
 {
 	struct port *port;
 
-	for (port=aggregator->lag_ports; port; port=port->next_port_in_aggregator) {
+	for (port = aggregator->lag_ports; port;
+	     port = port->next_port_in_aggregator) {
 		if (val) {
 			port->sm_vars |= AD_PORT_READY;
 		} else {
@@ -722,7 +725,7 @@ static void __set_agg_ports_ready(struct aggregator *aggregator, int val)
  */
 static u32 __get_agg_bandwidth(struct aggregator *aggregator)
 {
-	u32 bandwidth=0;
+	u32 bandwidth = 0;
 	u32 basic_speed;
 
 	if (aggregator->num_of_ports) {
@@ -744,7 +747,7 @@ static u32 __get_agg_bandwidth(struct aggregator *aggregator)
 			bandwidth = aggregator->num_of_ports * 10000;
 			break;
 		default:
-			bandwidth=0; // to silent the compilor ....
+			bandwidth = 0; /*to silence the compiler ....*/
 		}
 	}
 	return bandwidth;
@@ -1183,7 +1186,8 @@ static void ad_tx_machine(struct port *port)
 			}
 		}
 		// restart tx timer(to verify that we will not exceed AD_MAX_TX_IN_SECOND
-		port->sm_tx_timer_counter=ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
+		port->sm_tx_timer_counter =
+			ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
 	}
 }
 
@@ -1294,20 +1298,24 @@ static void ad_port_selection_logic(struct port *port)
 	// if the port is connected to other aggregator, detach it
 	if (port->aggregator) {
 		// detach the port from its former aggregator
-		temp_aggregator=port->aggregator;
-		for (curr_port=temp_aggregator->lag_ports; curr_port; last_port=curr_port, curr_port=curr_port->next_port_in_aggregator) {
+		temp_aggregator = port->aggregator;
+		for (curr_port = temp_aggregator->lag_ports; curr_port;
+		     last_port = curr_port,
+			     curr_port = curr_port->next_port_in_aggregator) {
 			if (curr_port == port) {
 				temp_aggregator->num_of_ports--;
 				if (!last_port) {// if it is the first port attached to the aggregator
-					temp_aggregator->lag_ports=port->next_port_in_aggregator;
+					temp_aggregator->lag_ports =
+						port->next_port_in_aggregator;
 				} else {// not the first port attached to the aggregator
-					last_port->next_port_in_aggregator=port->next_port_in_aggregator;
+					last_port->next_port_in_aggregator =
+						port->next_port_in_aggregator;
 				}
 
 				// clear the port's relations to this aggregator
 				port->aggregator = NULL;
-				port->next_port_in_aggregator=NULL;
-				port->actor_port_aggregator_identifier=0;
+				port->next_port_in_aggregator = NULL;
+				port->actor_port_aggregator_identifier = 0;
 
 				pr_debug("Port %d left LAG %d\n",
 					 port->actor_port_number,
@@ -1334,7 +1342,7 @@ static void ad_port_selection_logic(struct port *port)
 		// keep a free aggregator for later use(if needed)
 		if (!aggregator->lag_ports) {
 			if (!free_aggregator) {
-				free_aggregator=aggregator;
+				free_aggregator = aggregator;
 			}
 			continue;
 		}
@@ -1350,10 +1358,11 @@ static void ad_port_selection_logic(struct port *port)
 		   ) {
 			// attach to the founded aggregator
 			port->aggregator = aggregator;
-			port->actor_port_aggregator_identifier=port->aggregator->aggregator_identifier;
-			port->next_port_in_aggregator=aggregator->lag_ports;
+			port->actor_port_aggregator_identifier =
+				port->aggregator->aggregator_identifier;
+			port->next_port_in_aggregator = aggregator->lag_ports;
 			port->aggregator->num_of_ports++;
-			aggregator->lag_ports=port;
+			aggregator->lag_ports = port;
 			pr_debug("Port %d joined LAG %d(existing LAG)\n",
 				 port->actor_port_number,
 				 port->aggregator->aggregator_identifier);
@@ -1370,7 +1379,8 @@ static void ad_port_selection_logic(struct port *port)
 		if (free_aggregator) {
 			// assign port a new aggregator
 			port->aggregator = free_aggregator;
-			port->actor_port_aggregator_identifier=port->aggregator->aggregator_identifier;
+			port->actor_port_aggregator_identifier =
+				port->aggregator->aggregator_identifier;
 
 			// update the new aggregator's parameters
 			// if port was responsed from the end-user
@@ -1382,8 +1392,10 @@ static void ad_port_selection_logic(struct port *port)
 
 			port->aggregator->actor_admin_aggregator_key = port->actor_admin_port_key;
 			port->aggregator->actor_oper_aggregator_key = port->actor_oper_port_key;
-			port->aggregator->partner_system=port->partner_oper.system;
-			port->aggregator->partner_system_priority = port->partner_oper.system_priority;
+			port->aggregator->partner_system =
+				port->partner_oper.system;
+			port->aggregator->partner_system_priority =
+				port->partner_oper.system_priority;
 			port->aggregator->partner_oper_aggregator_key = port->partner_oper.key;
 			port->aggregator->receive_state = 1;
 			port->aggregator->transmit_state = 1;
@@ -1785,13 +1797,16 @@ static void ad_marker_info_send(struct port *port)
 	marker.requester_port = (((port->actor_port_number & 0xFF) << 8) |((u16)(port->actor_port_number & 0xFF00) >> 8));
 	marker.requester_system = port->actor_system;
 	// convert requester_port(u32) to Big Endian
-	marker.requester_transaction_id = (((++port->transaction_id & 0xFF) << 24) |((port->transaction_id & 0xFF00) << 8) |((port->transaction_id & 0xFF0000) >> 8) |((port->transaction_id & 0xFF000000) >> 24));
+	marker.requester_transaction_id =
+		(((++port->transaction_id & 0xFF) << 24)
+		 | ((port->transaction_id & 0xFF00) << 8)
+		 | ((port->transaction_id & 0xFF0000) >> 8)
+		 | ((port->transaction_id & 0xFF000000) >> 24));
 	marker.pad = 0;
 	marker.tlv_type_terminator = 0x00;
 	marker.terminator_length = 0x00;
-	for (index=0; index<90; index++) {
-		marker.reserved_90[index]=0;
-	}
+	for (index = 0; index < 90; index++)
+		marker.reserved_90[index] = 0;
 
 	// send the marker information
 	if (ad_marker_send(port, &marker) >= 0) {
@@ -1816,7 +1831,7 @@ static void ad_marker_info_received(struct bond_marker *marker_info,
 	//marker = *marker_info;
 	memcpy(&marker, marker_info, sizeof(struct bond_marker));
 	// change the marker subtype to marker response
-	marker.tlv_type=AD_MARKER_RESPONSE_SUBTYPE;
+	marker.tlv_type = AD_MARKER_RESPONSE_SUBTYPE;
 	// send the marker response
 
 	if (ad_marker_send(port, &marker) >= 0) {
@@ -1837,8 +1852,8 @@ static void ad_marker_info_received(struct bond_marker *marker_info,
 static void ad_marker_response_received(struct bond_marker *marker,
 	struct port *port)
 {
-	marker=NULL; // just to satisfy the compiler
-	port=NULL;  // just to satisfy the compiler
+	marker = NULL; /* just to satisfy the compiler */
+	port = NULL;  /* just to satisfy the compiler */
 	// DO NOTHING, SINCE WE DECIDED NOT TO IMPLEMENT THIS FEATURE FOR NOW
 }
 
@@ -2037,8 +2052,9 @@ void bond_3ad_unbind_slave(struct slave *slave)
 				new_aggregator->num_of_ports = aggregator->num_of_ports;
 
 				// update the information that is written on the ports about the aggregator
-				for (temp_port=aggregator->lag_ports; temp_port; temp_port=temp_port->next_port_in_aggregator) {
-					temp_port->aggregator=new_aggregator;
+				for (temp_port = aggregator->lag_ports; temp_port;
+				     temp_port = temp_port->next_port_in_aggregator) {
+					temp_port->aggregator = new_aggregator;
 					temp_port->actor_port_aggregator_identifier = new_aggregator->aggregator_identifier;
 				}
 
@@ -2071,7 +2087,9 @@ void bond_3ad_unbind_slave(struct slave *slave)
 	for (; temp_aggregator; temp_aggregator = __get_next_agg(temp_aggregator)) {
 		prev_port = NULL;
 		// search the port in the aggregator's related ports
-		for (temp_port=temp_aggregator->lag_ports; temp_port; prev_port=temp_port, temp_port=temp_port->next_port_in_aggregator) {
+		for (temp_port = temp_aggregator->lag_ports; temp_port;
+		     prev_port = temp_port,
+			     temp_port = temp_port->next_port_in_aggregator) {
 			if (temp_port == port) { // the aggregator found - detach the port from this aggregator
 				if (prev_port) {
 					prev_port->next_port_in_aggregator = temp_port->next_port_in_aggregator;
@@ -2079,7 +2097,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 					temp_aggregator->lag_ports = temp_port->next_port_in_aggregator;
 				}
 				temp_aggregator->num_of_ports--;
-				if (temp_aggregator->num_of_ports==0) {
+				if (temp_aggregator->num_of_ports == 0) {
 					select_new_active_agg = temp_aggregator->is_active;
 					// clear the aggregator
 					ad_clear_agg(temp_aggregator);
@@ -2094,7 +2112,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 			}
 		}
 	}
-	port->slave=NULL;
+	port->slave = NULL;
 }
 
 /**
@@ -2245,7 +2263,8 @@ void bond_3ad_adapter_speed_changed(struct slave *slave)
 	}
 
 	port->actor_admin_port_key &= ~AD_SPEED_KEY_BITS;
-	port->actor_oper_port_key=port->actor_admin_port_key |= (__get_link_speed(port) << 1);
+	port->actor_oper_port_key = port->actor_admin_port_key |=
+		(__get_link_speed(port) << 1);
 	pr_debug("Port %d changed speed\n", port->actor_port_number);
 	// there is no need to reselect a new aggregator, just signal the
 	// state machines to reinitialize
@@ -2262,7 +2281,7 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
 {
 	struct port *port;
 
-	port=&(SLAVE_AD_INFO(slave).port);
+	port = &(SLAVE_AD_INFO(slave).port);
 
 	// if slave is null, the whole port is not initialized
 	if (!port->slave) {
@@ -2272,7 +2291,8 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave)
 	}
 
 	port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS;
-	port->actor_oper_port_key=port->actor_admin_port_key |= __get_duplex(port);
+	port->actor_oper_port_key = port->actor_admin_port_key |=
+		__get_duplex(port);
 	pr_debug("Port %d changed duplex\n", port->actor_port_number);
 	// there is no need to reselect a new aggregator, just signal the
 	// state machines to reinitialize
@@ -2304,14 +2324,17 @@ void bond_3ad_handle_link_change(struct slave *slave, char link)
 	if (link == BOND_LINK_UP) {
 		port->is_enabled = true;
 		port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS;
-		port->actor_oper_port_key=port->actor_admin_port_key |= __get_duplex(port);
+		port->actor_oper_port_key = port->actor_admin_port_key |=
+			__get_duplex(port);
 		port->actor_admin_port_key &= ~AD_SPEED_KEY_BITS;
-		port->actor_oper_port_key=port->actor_admin_port_key |= (__get_link_speed(port) << 1);
+		port->actor_oper_port_key = port->actor_admin_port_key |=
+			(__get_link_speed(port) << 1);
 	} else {
 		/* link has failed */
 		port->is_enabled = false;
 		port->actor_admin_port_key &= ~AD_DUPLEX_KEY_BITS;
-		port->actor_oper_port_key= (port->actor_admin_port_key &= ~AD_SPEED_KEY_BITS);
+		port->actor_oper_port_key = (port->actor_admin_port_key &=
+					     ~AD_SPEED_KEY_BITS);
 	}
 	//BOND_PRINT_DBG(("Port %d changed link status to %s", port->actor_port_number, ((link == BOND_LINK_UP)?"UP":"DOWN")));
 	// there is no need to reselect a new aggregator, just signal the
-- 
1.7.2.3

^ permalink raw reply related

* [PATCH 2/2] bonding: cleanup: remove braces from single block statements
From: Bandan Das @ 2010-10-17  6:19 UTC (permalink / raw)
  To: David Miller; +Cc: LKML, NetDev, joe, Eric Dumazet, Bandan Das
In-Reply-To: <1287296399-25142-1-git-send-email-bandan.das@stratus.com>

checkpatch.pl cleanup : Remove braces from single statement
blocks.

Signed-off-by: Bandan Das <bandan.das@stratus.com>
---
 drivers/net/bonding/bond_3ad.c |  163 ++++++++++++++++++----------------------
 1 files changed, 72 insertions(+), 91 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 6b06b00..881914b 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -129,9 +129,8 @@ static void ad_marker_response_received(struct bond_marker *marker, struct port
  */
 static inline struct bonding *__get_bond_by_port(struct port *port)
 {
-	if (port->slave == NULL) {
+	if (port->slave == NULL)
 		return NULL;
-	}
 
 	return bond_get_bond_by_slave(port->slave);
 }
@@ -144,9 +143,8 @@ static inline struct bonding *__get_bond_by_port(struct port *port)
  */
 static inline struct port *__get_first_port(struct bonding *bond)
 {
-	if (bond->slave_cnt == 0) {
+	if (bond->slave_cnt == 0)
 		return NULL;
-	}
 
 	return &(SLAVE_AD_INFO(bond->first_slave).port);
 }
@@ -164,9 +162,8 @@ static inline struct port *__get_next_port(struct port *port)
 	struct slave *slave = port->slave;
 
 	// If there's no bond for this port, or this is the last slave
-	if ((bond == NULL) || (slave->next == bond->first_slave)) {
+	if ((bond == NULL) || (slave->next == bond->first_slave))
 		return NULL;
-	}
 
 	return &(SLAVE_AD_INFO(slave->next).port);
 }
@@ -183,9 +180,8 @@ static inline struct aggregator *__get_first_agg(struct port *port)
 	struct bonding *bond = __get_bond_by_port(port);
 
 	// If there's no bond for this port, or bond has no slaves
-	if ((bond == NULL) || (bond->slave_cnt == 0)) {
+	if ((bond == NULL) || (bond->slave_cnt == 0))
 		return NULL;
-	}
 
 	return &(SLAVE_AD_INFO(bond->first_slave).aggregator);
 }
@@ -203,9 +199,8 @@ static inline struct aggregator *__get_next_agg(struct aggregator *aggregator)
 	struct bonding *bond = bond_get_bond_by_slave(slave);
 
 	// If there's no bond for this aggregator, or this is the last slave
-	if ((bond == NULL) || (slave->next == bond->first_slave)) {
+	if ((bond == NULL) || (slave->next == bond->first_slave))
 		return NULL;
-	}
 
 	return &(SLAVE_AD_INFO(slave->next).aggregator);
 }
@@ -240,9 +235,8 @@ static inline void __enable_port(struct port *port)
 {
 	struct slave *slave = port->slave;
 
-	if ((slave->link == BOND_LINK_UP) && IS_UP(slave->dev)) {
+	if ((slave->link == BOND_LINK_UP) && IS_UP(slave->dev))
 		bond_set_slave_active_flags(slave);
-	}
 }
 
 /**
@@ -265,9 +259,8 @@ static inline u32 __get_agg_selection_mode(struct port *port)
 {
 	struct bonding *bond = __get_bond_by_port(port);
 
-	if (bond == NULL) {
+	if (bond == NULL)
 		return BOND_AD_STABLE;
-	}
 
 	return BOND_AD_INFO(bond).agg_select_mode;
 }
@@ -281,9 +274,8 @@ static inline int __check_agg_selection_timer(struct port *port)
 {
 	struct bonding *bond = __get_bond_by_port(port);
 
-	if (bond == NULL) {
+	if (bond == NULL)
 		return 0;
-	}
 
 	return BOND_AD_INFO(bond).agg_select_timer ? 1 : 0;
 }
@@ -328,9 +320,9 @@ static u16 __get_link_speed(struct port *port)
 	 * link down, it sets the speed to 0.
 	 * This is done in spite of the fact that the e100 driver reports 0 to be
 	 * compatible with MVT in the future.*/
-	if (slave->link != BOND_LINK_UP) {
+	if (slave->link != BOND_LINK_UP)
 		speed = 0;
-	} else {
+	else {
 		switch (slave->speed) {
 		case SPEED_10:
 			speed = AD_LINK_SPEED_BITMASK_10MBPS;
@@ -375,9 +367,9 @@ static u8 __get_duplex(struct port *port)
 
 	//  handling a special case: when the configuration starts with
 	// link down, it sets the duplex to 0.
-	if (slave->link != BOND_LINK_UP) {
+	if (slave->link != BOND_LINK_UP)
 		retval = 0x0;
-	} else {
+	else {
 		switch (slave->duplex) {
 		case DUPLEX_FULL:
 			retval = 0x1;
@@ -423,11 +415,10 @@ static u16 __ad_timer_to_ticks(u16 timer_type, u16 par)
 
 	switch (timer_type) {
 	case AD_CURRENT_WHILE_TIMER:   // for rx machine usage
-		if (par) {	      // for short or long timeout
+		if (par)
 			retval = (AD_SHORT_TIMEOUT_TIME*ad_ticks_per_sec); // short timeout
-		} else {
+		else
 			retval = (AD_LONG_TIMEOUT_TIME*ad_ticks_per_sec); // long timeout
-		}
 		break;
 	case AD_ACTOR_CHURN_TIMER:	    // for local churn machine
 		retval = (AD_CHURN_DETECTION_TIME*ad_ticks_per_sec);
@@ -519,11 +510,11 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 		port->actor_oper_port_state &= ~AD_STATE_DEFAULTED;
 
 		// set the partner sync. to on if the partner is sync. and the port is matched
-		if ((port->sm_vars & AD_PORT_MATCHED) && (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION)) {
+		if ((port->sm_vars & AD_PORT_MATCHED)
+		    && (lacpdu->actor_state & AD_STATE_SYNCHRONIZATION))
 			partner->port_state |= AD_STATE_SYNCHRONIZATION;
-		} else {
+		else
 			partner->port_state &= ~AD_STATE_SYNCHRONIZATION;
-		}
 	}
 }
 
@@ -710,11 +701,10 @@ static void __set_agg_ports_ready(struct aggregator *aggregator, int val)
 
 	for (port = aggregator->lag_ports; port;
 	     port = port->next_port_in_aggregator) {
-		if (val) {
+		if (val)
 			port->sm_vars |= AD_PORT_READY;
-		} else {
+		else
 			port->sm_vars &= ~AD_PORT_READY;
-		}
 	}
 }
 
@@ -838,9 +828,8 @@ static int ad_lacpdu_send(struct port *port)
 	int length = sizeof(struct lacpdu_header);
 
 	skb = dev_alloc_skb(length);
-	if (!skb) {
+	if (!skb)
 		return -ENOMEM;
-	}
 
 	skb->dev = slave->dev;
 	skb_reset_mac_header(skb);
@@ -879,9 +868,8 @@ static int ad_marker_send(struct port *port, struct bond_marker *marker)
 	int length = sizeof(struct bond_marker_header);
 
 	skb = dev_alloc_skb(length + 16);
-	if (!skb) {
+	if (!skb)
 		return -ENOMEM;
-	}
 
 	skb_reserve(skb, 16);
 
@@ -922,9 +910,10 @@ static void ad_mux_machine(struct port *port)
 	} else {
 		switch (port->sm_mux_state) {
 		case AD_MUX_DETACHED:
-			if ((port->sm_vars & AD_PORT_SELECTED) || (port->sm_vars & AD_PORT_STANDBY)) { // if SELECTED or STANDBY
+			if ((port->sm_vars & AD_PORT_SELECTED)
+			    || (port->sm_vars & AD_PORT_STANDBY))
+				/* if SELECTED or STANDBY */
 				port->sm_mux_state = AD_MUX_WAITING; // next state
-			}
 			break;
 		case AD_MUX_WAITING:
 			// if SELECTED == FALSE return to DETACH state
@@ -938,18 +927,18 @@ static void ad_mux_machine(struct port *port)
 			}
 
 			// check if the wait_while_timer expired
-			if (port->sm_mux_timer_counter && !(--port->sm_mux_timer_counter)) {
+			if (port->sm_mux_timer_counter
+			    && !(--port->sm_mux_timer_counter))
 				port->sm_vars |= AD_PORT_READY_N;
-			}
 
 			// in order to withhold the selection logic to check all ports READY_N value
 			// every callback cycle to update ready variable, we check READY_N and update READY here
 			__set_agg_ports_ready(port->aggregator, __agg_ports_are_ready(port->aggregator));
 
 			// if the wait_while_timer expired, and the port is in READY state, move to ATTACHED state
-			if ((port->sm_vars & AD_PORT_READY) && !port->sm_mux_timer_counter) {
+			if ((port->sm_vars & AD_PORT_READY)
+			    && !port->sm_mux_timer_counter)
 				port->sm_mux_state = AD_MUX_ATTACHED;	 // next state
-			}
 			break;
 		case AD_MUX_ATTACHED:
 			// check also if agg_select_timer expired(so the edable port will take place only after this timer)
@@ -1044,13 +1033,14 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 
 	// check if state machine should change state
 	// first, check if port was reinitialized
-	if (port->sm_vars & AD_PORT_BEGIN) {
-		port->sm_rx_state = AD_RX_INITIALIZE;		    // next state
-	}
+	if (port->sm_vars & AD_PORT_BEGIN)
+		/* next state */
+		port->sm_rx_state = AD_RX_INITIALIZE;
 	// check if port is not enabled
-	else if (!(port->sm_vars & AD_PORT_BEGIN) && !port->is_enabled && !(port->sm_vars & AD_PORT_MOVED)) {
-		port->sm_rx_state = AD_RX_PORT_DISABLED;	    // next state
-	}
+	else if (!(port->sm_vars & AD_PORT_BEGIN)
+		 && !port->is_enabled && !(port->sm_vars & AD_PORT_MOVED))
+		/* next state */
+		port->sm_rx_state = AD_RX_PORT_DISABLED;
 	// check if new lacpdu arrived
 	else if (lacpdu && ((port->sm_rx_state == AD_RX_EXPIRED) || (port->sm_rx_state == AD_RX_DEFAULTED) || (port->sm_rx_state == AD_RX_CURRENT))) {
 		port->sm_rx_timer_counter = 0; // zero timer
@@ -1072,13 +1062,16 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			// if no lacpdu arrived and no timer is on
 			switch (port->sm_rx_state) {
 			case AD_RX_PORT_DISABLED:
-				if (port->sm_vars & AD_PORT_MOVED) {
+				if (port->sm_vars & AD_PORT_MOVED)
 					port->sm_rx_state = AD_RX_INITIALIZE;	    // next state
-				} else if (port->is_enabled && (port->sm_vars & AD_PORT_LACP_ENABLED)) {
+				else if (port->is_enabled
+					 && (port->sm_vars
+					     & AD_PORT_LACP_ENABLED))
 					port->sm_rx_state = AD_RX_EXPIRED;	// next state
-				} else if (port->is_enabled && ((port->sm_vars & AD_PORT_LACP_ENABLED) == 0)) {
+				else if (port->is_enabled
+					 && ((port->sm_vars
+					      & AD_PORT_LACP_ENABLED) == 0))
 					port->sm_rx_state = AD_RX_LACP_DISABLED;    // next state
-				}
 				break;
 			default:    //to silence the compiler
 				break;
@@ -1094,11 +1087,10 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			 port->sm_rx_state);
 		switch (port->sm_rx_state) {
 		case AD_RX_INITIALIZE:
-			if (!(port->actor_oper_port_key & AD_DUPLEX_KEY_BITS)) {
+			if (!(port->actor_oper_port_key & AD_DUPLEX_KEY_BITS))
 				port->sm_vars &= ~AD_PORT_LACP_ENABLED;
-			} else {
+			else
 				port->sm_vars |= AD_PORT_LACP_ENABLED;
-			}
 			port->sm_vars &= ~AD_PORT_SELECTED;
 			__record_default(port);
 			port->actor_oper_port_state &= ~AD_STATE_EXPIRED;
@@ -1152,9 +1144,10 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			// verify that if the aggregator is enabled, the port is enabled too.
 			//(because if the link goes down for a short time, the 802.3ad will not
 			// catch it, and the port will continue to be disabled)
-			if (port->aggregator && port->aggregator->is_active && !__port_is_enabled(port)) {
+			if (port->aggregator
+			    && port->aggregator->is_active
+			    && !__port_is_enabled(port))
 				__enable_port(port);
-			}
 			break;
 		default:    //to silence the compiler
 			break;
@@ -1220,9 +1213,9 @@ static void ad_periodic_machine(struct port *port)
 			// If not expired, check if there is some new timeout parameter from the partner state
 			switch (port->sm_periodic_state) {
 			case AD_FAST_PERIODIC:
-				if (!(port->partner_oper.port_state & AD_STATE_LACP_TIMEOUT)) {
+				if (!(port->partner_oper.port_state
+				      & AD_STATE_LACP_TIMEOUT))
 					port->sm_periodic_state = AD_SLOW_PERIODIC;  // next state
-				}
 				break;
 			case AD_SLOW_PERIODIC:
 				if ((port->partner_oper.port_state & AD_STATE_LACP_TIMEOUT)) {
@@ -1241,11 +1234,11 @@ static void ad_periodic_machine(struct port *port)
 			port->sm_periodic_state = AD_FAST_PERIODIC;	 // next state
 			break;
 		case AD_PERIODIC_TX:
-			if (!(port->partner_oper.port_state & AD_STATE_LACP_TIMEOUT)) {
+			if (!(port->partner_oper.port_state
+			      & AD_STATE_LACP_TIMEOUT))
 				port->sm_periodic_state = AD_SLOW_PERIODIC;  // next state
-			} else {
+			else
 				port->sm_periodic_state = AD_FAST_PERIODIC;  // next state
-			}
 			break;
 		default:    //to silence the compiler
 			break;
@@ -1291,9 +1284,8 @@ static void ad_port_selection_logic(struct port *port)
 	int found = 0;
 
 	// if the port is already Selected, do nothing
-	if (port->sm_vars & AD_PORT_SELECTED) {
+	if (port->sm_vars & AD_PORT_SELECTED)
 		return;
-	}
 
 	// if the port is connected to other aggregator, detach it
 	if (port->aggregator) {
@@ -1321,9 +1313,8 @@ static void ad_port_selection_logic(struct port *port)
 					 port->actor_port_number,
 					 temp_aggregator->aggregator_identifier);
 				// if the aggregator is empty, clear its parameters, and set it ready to be attached
-				if (!temp_aggregator->lag_ports) {
+				if (!temp_aggregator->lag_ports)
 					ad_clear_agg(temp_aggregator);
-				}
 				break;
 			}
 		}
@@ -1341,9 +1332,8 @@ static void ad_port_selection_logic(struct port *port)
 
 		// keep a free aggregator for later use(if needed)
 		if (!aggregator->lag_ports) {
-			if (!free_aggregator) {
+			if (!free_aggregator)
 				free_aggregator = aggregator;
-			}
 			continue;
 		}
 		// check if current aggregator suits us
@@ -1384,11 +1374,11 @@ static void ad_port_selection_logic(struct port *port)
 
 			// update the new aggregator's parameters
 			// if port was responsed from the end-user
-			if (port->actor_oper_port_key & AD_DUPLEX_KEY_BITS) {// if port is full duplex
+			if (port->actor_oper_port_key & AD_DUPLEX_KEY_BITS)
+				/* if port is full duplex */
 				port->aggregator->is_individual = false;
-			} else {
+			else
 				port->aggregator->is_individual = true;
-			}
 
 			port->aggregator->actor_admin_aggregator_key = port->actor_admin_port_key;
 			port->aggregator->actor_oper_aggregator_key = port->actor_oper_port_key;
@@ -1716,9 +1706,8 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
 		port->actor_admin_port_state = AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
 		port->actor_oper_port_state  = AD_STATE_AGGREGATION | AD_STATE_LACP_ACTIVITY;
 
-		if (lacp_fast) {
+		if (lacp_fast)
 			port->actor_oper_port_state |= AD_STATE_LACP_TIMEOUT;
-		}
 
 		memcpy(&port->partner_admin, &tmpl, sizeof(tmpl));
 		memcpy(&port->partner_oper, &tmpl, sizeof(tmpl));
@@ -1947,9 +1936,8 @@ int bond_3ad_bind_slave(struct slave *slave)
 		port->actor_admin_port_key |= (__get_link_speed(port) << 1);
 		port->actor_oper_port_key = port->actor_admin_port_key;
 		// if the port is not full duplex, then the port should be not lacp Enabled
-		if (!(port->actor_oper_port_key & AD_DUPLEX_KEY_BITS)) {
+		if (!(port->actor_oper_port_key & AD_DUPLEX_KEY_BITS))
 			port->sm_vars &= ~AD_PORT_LACP_ENABLED;
-		}
 		// actor system is the bond's system
 		port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr;
 		// tx timer(to verify that no more than MAX_TX_IN_SECOND lacpdu's are sent in one second)
@@ -2021,9 +2009,10 @@ void bond_3ad_unbind_slave(struct slave *slave)
 			new_aggregator = __get_first_agg(port);
 			for (; new_aggregator; new_aggregator = __get_next_agg(new_aggregator)) {
 				// if the new aggregator is empty, or it is connected to our port only
-				if (!new_aggregator->lag_ports || ((new_aggregator->lag_ports == port) && !new_aggregator->lag_ports->next_port_in_aggregator)) {
+				if (!new_aggregator->lag_ports
+				    || ((new_aggregator->lag_ports == port)
+					&& !new_aggregator->lag_ports->next_port_in_aggregator))
 					break;
-				}
 			}
 			// if new aggregator found, copy the aggregator's parameters
 			// and connect the related lag_ports to the new aggregator
@@ -2061,9 +2050,8 @@ void bond_3ad_unbind_slave(struct slave *slave)
 				// clear the aggregator
 				ad_clear_agg(aggregator);
 
-				if (select_new_active_agg) {
+				if (select_new_active_agg)
 					ad_agg_selection_logic(__get_first_agg(port));
-				}
 			} else {
 				pr_warning("%s: Warning: unbinding aggregator, and could not find a new aggregator for its ports\n",
 					   slave->dev->master->name);
@@ -2091,11 +2079,10 @@ void bond_3ad_unbind_slave(struct slave *slave)
 		     prev_port = temp_port,
 			     temp_port = temp_port->next_port_in_aggregator) {
 			if (temp_port == port) { // the aggregator found - detach the port from this aggregator
-				if (prev_port) {
+				if (prev_port)
 					prev_port->next_port_in_aggregator = temp_port->next_port_in_aggregator;
-				} else {
+				else
 					temp_aggregator->lag_ports = temp_port->next_port_in_aggregator;
-				}
 				temp_aggregator->num_of_ports--;
 				if (temp_aggregator->num_of_ports == 0) {
 					select_new_active_agg = temp_aggregator->is_active;
@@ -2137,14 +2124,12 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 
 	read_lock(&bond->lock);
 
-	if (bond->kill_timers) {
+	if (bond->kill_timers)
 		goto out;
-	}
 
 	//check if there are any slaves
-	if (bond->slave_cnt == 0) {
+	if (bond->slave_cnt == 0)
 		goto re_arm;
-	}
 
 	// check if agg_select_timer timer after initialize is timed out
 	if (BOND_AD_INFO(bond).agg_select_timer && !(--BOND_AD_INFO(bond).agg_select_timer)) {
@@ -2177,9 +2162,8 @@ void bond_3ad_state_machine_handler(struct work_struct *work)
 		ad_tx_machine(port);
 
 		// turn off the BEGIN bit, since we already handled it
-		if (port->sm_vars & AD_PORT_BEGIN) {
+		if (port->sm_vars & AD_PORT_BEGIN)
 			port->sm_vars &= ~AD_PORT_BEGIN;
-		}
 	}
 
 re_arm:
@@ -2417,9 +2401,8 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 	 */
 	read_lock(&bond->lock);
 
-	if (!BOND_IS_OK(bond)) {
+	if (!BOND_IS_OK(bond))
 		goto out;
-	}
 
 	if (bond_3ad_get_active_agg_info(bond, &ad_info)) {
 		pr_debug("%s: Error: bond_3ad_get_active_agg_info failed\n",
@@ -2443,9 +2426,8 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 
 		if (agg && (agg->aggregator_identifier == agg_id)) {
 			slave_agg_no--;
-			if (slave_agg_no < 0) {
+			if (slave_agg_no < 0)
 				break;
-			}
 		}
 	}
 
@@ -2461,9 +2443,8 @@ int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev)
 		int slave_agg_id = 0;
 		struct aggregator *agg = SLAVE_AD_INFO(slave).port.aggregator;
 
-		if (agg) {
+		if (agg)
 			slave_agg_id = agg->aggregator_identifier;
-		}
 
 		if (SLAVE_IS_OK(slave) && agg && (slave_agg_id == agg_id)) {
 			res = bond_dev_queue_xmit(bond, skb, slave->dev);
-- 
1.7.2.3

^ permalink raw reply related

* Re: [RFC PATCH 0/9] ipvs network name space (netns) aware
From: Simon Horman @ 2010-10-17  6:47 UTC (permalink / raw)
  To: Hans Schillstrom
  Cc: lvs-devel, netdev, netfilter-devel, ja, wensong, daniel.lezcano
In-Reply-To: <201010081316.38317.hans.schillstrom@ericsson.com>

On Fri, Oct 08, 2010 at 01:16:36PM +0200, Hans Schillstrom wrote:
> This patch series adds network name space (netns) support to the LVS.
> 
> REVISION
> 
> This is version 1
> 
> OVERVIEW
> 
> The patch doesn't remove or add any functionality except for netns.
> For users that don't use network name space (netns) this patch is
> completely transparent.
> 
> No it's possible to run LVS in a Linux container (see lxc-tools)
> i.e.  a light weight virtualization. For example it's possible to run
> one or several lvs on a real server in their own network name spaces.
> >From the LVS point of view it looks like it runs on it's own machine.
> 
> IMPLEMENTATION
> Basic requirements for netns awareness
>  - Global variables has to be moved to dyn. allocated memory.
> 
> Most global variables now resides in a struct ipvs { } in netns/ip_vs.h.
> What is moved and what is not ?
> 
> Some cache aligned locks are still in global, module init params and some debug_level.
> 
> Algorithm files they are untouched.
> 
> QUESTIONS
> Drop rate in ip_vs_ctl per netns or grand total ?

My gut-feeling is that per netns makes more sense.

> Should more lock variables be moved (or less) ?

I'm unsure what you are asking here but I will make a general statement
about locking in IPVS: it needs work.

> 
> PATCH SET
> This patch set is based upon net-next-2.6 (2.6.36-rc3) from 4 oct 2010
> and [patch v4] ipvs: IPv6 tunnel mode
> 
> Note: ip_vs_xmit.c will not work without "[patch v4] ipvs: IPv6 tunnel mode"

Unfortunately the patches don't apply with the persistence engine
patches which were recently merged into nf-next-2.6 (although
"[patch v4.1 ]ipvs: IPv6 tunnel mode" is still unmerged).

I'm happy to work with you to make the required changes there.

(I realise those patches weren't merged when you made your post.
 But regardless, either your or me will need to update the patches).

Another issue is that your patches seem to be split in a way
where the build breaks along the way. E.g. after applying
patch 1, the build breaks. Could you please split things up
in a manner such that this doesn't happen. The reason being
that it breaks bisection.

Lastly, could you provide a unique subject for each patch.
I know its a bit tedious, but it does make a difference when
browsing the changelog.


^ permalink raw reply

* [RFC PATCH net-next] drivers/net/bonding/bond_3ad: Remove struct mac_addr and typedef
From: Joe Perches @ 2010-10-17  7:38 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: bonding-devel, netdev, LKML

Use more standard u8 name[ETH_ALEN];
Remove MAC_ADDRESS_COMPARE and use compare_ether_addr
Remove null_mac_addr and use is_zero_ether_addr and memset
Use memcpy for copying ether addresses

Size increases ~200 bytes so it's not great,
but it might be better for style consistency.

$ size drivers/net/bonding/*.o.*
   text	   data	    bss	    dec	    hex	filename
  25580	    776	   5800	  32156	   7d9c	drivers/net/bonding/bond_3ad.o.new
  25396	    776	   5808	  31980	   7cec	drivers/net/bonding/bond_3ad.o.old

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/net/bonding/bond_3ad.c |   50 ++++++++++++++++++---------------------
 drivers/net/bonding/bond_3ad.h |   20 ++++++---------
 2 files changed, 31 insertions(+), 39 deletions(-)

diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c
index 079b9d1..5a38eef 100644
--- a/drivers/net/bonding/bond_3ad.c
+++ b/drivers/net/bonding/bond_3ad.c
@@ -90,10 +90,6 @@
 #define     AD_LINK_SPEED_BITMASK_10000MBPS   0x10
 //endalloun
 
-// compare MAC addresses
-#define MAC_ADDRESS_COMPARE(A, B) memcmp(A, B, ETH_ALEN)
-
-static struct mac_addr null_mac_addr = {{0, 0, 0, 0, 0, 0}};
 static u16 ad_ticks_per_sec;
 static const int ad_delta_in_ticks = (AD_TIMER_INTERVAL * HZ) / 1000;
 
@@ -218,7 +214,7 @@ static inline struct aggregator *__get_next_agg(struct aggregator *aggregator)
  */
 static inline int __agg_has_partner(struct aggregator *agg)
 {
-	return !is_zero_ether_addr(agg->partner_system.mac_addr_value);
+	return !is_zero_ether_addr(agg->partner_system);
 }
 
 /**
@@ -478,7 +474,7 @@ static void __choose_matched(struct lacpdu *lacpdu, struct port *port)
 	// check if all parameters are alike
 	if (((ntohs(lacpdu->partner_port) == port->actor_port_number) &&
 	     (ntohs(lacpdu->partner_port_priority) == port->actor_port_priority) &&
-	     !MAC_ADDRESS_COMPARE(&(lacpdu->partner_system), &(port->actor_system)) &&
+	     !compare_ether_addr(lacpdu->partner_system, port->actor_system) &&
 	     (ntohs(lacpdu->partner_system_priority) == port->actor_system_priority) &&
 	     (ntohs(lacpdu->partner_key) == port->actor_oper_port_key) &&
 	     ((lacpdu->partner_state & AD_STATE_AGGREGATION) == (port->actor_oper_port_state & AD_STATE_AGGREGATION))) ||
@@ -510,7 +506,7 @@ static void __record_pdu(struct lacpdu *lacpdu, struct port *port)
 		// record the new parameter values for the partner operational
 		partner->port_number = ntohs(lacpdu->actor_port);
 		partner->port_priority = ntohs(lacpdu->actor_port_priority);
-		partner->system = lacpdu->actor_system;
+		memcpy(partner->system, lacpdu->actor_system, ETH_ALEN);
 		partner->system_priority = ntohs(lacpdu->actor_system_priority);
 		partner->key = ntohs(lacpdu->actor_key);
 		partner->port_state = lacpdu->actor_state;
@@ -568,7 +564,7 @@ static void __update_selected(struct lacpdu *lacpdu, struct port *port)
 		// check if any parameter is different
 		if (ntohs(lacpdu->actor_port) != partner->port_number ||
 		    ntohs(lacpdu->actor_port_priority) != partner->port_priority ||
-		    MAC_ADDRESS_COMPARE(&lacpdu->actor_system, &partner->system) ||
+		    compare_ether_addr(lacpdu->actor_system, partner->system) ||
 		    ntohs(lacpdu->actor_system_priority) != partner->system_priority ||
 		    ntohs(lacpdu->actor_key) != partner->key ||
 		    (lacpdu->actor_state & AD_STATE_AGGREGATION) != (partner->port_state & AD_STATE_AGGREGATION)) {
@@ -599,7 +595,7 @@ static void __update_default_selected(struct port *port)
 		// check if any parameter is different
 		if (admin->port_number != oper->port_number ||
 		    admin->port_priority != oper->port_priority ||
-		    MAC_ADDRESS_COMPARE(&admin->system, &oper->system) ||
+		    compare_ether_addr(admin->system, oper->system) ||
 		    admin->system_priority != oper->system_priority ||
 		    admin->key != oper->key ||
 		    (admin->port_state & AD_STATE_AGGREGATION)
@@ -629,7 +625,7 @@ static void __update_ntt(struct lacpdu *lacpdu, struct port *port)
 		// check if any parameter is different
 		if ((ntohs(lacpdu->partner_port) != port->actor_port_number) ||
 		    (ntohs(lacpdu->partner_port_priority) != port->actor_port_priority) ||
-		    MAC_ADDRESS_COMPARE(&(lacpdu->partner_system), &(port->actor_system)) ||
+		    compare_ether_addr(lacpdu->partner_system, port->actor_system) ||
 		    (ntohs(lacpdu->partner_system_priority) != port->actor_system_priority) ||
 		    (ntohs(lacpdu->partner_key) != port->actor_oper_port_key) ||
 		    ((lacpdu->partner_state & AD_STATE_LACP_ACTIVITY) != (port->actor_oper_port_state & AD_STATE_LACP_ACTIVITY)) ||
@@ -787,7 +783,7 @@ static inline void __update_lacpdu_from_port(struct port *port)
 	 */
 
 	lacpdu->actor_system_priority = htons(port->actor_system_priority);
-	lacpdu->actor_system = port->actor_system;
+	memcpy(lacpdu->actor_system, port->actor_system, ETH_ALEN);
 	lacpdu->actor_key = htons(port->actor_oper_port_key);
 	lacpdu->actor_port_priority = htons(port->actor_port_priority);
 	lacpdu->actor_port = htons(port->actor_port_number);
@@ -799,7 +795,7 @@ static inline void __update_lacpdu_from_port(struct port *port)
 	 */
 
 	lacpdu->partner_system_priority = htons(partner->system_priority);
-	lacpdu->partner_system = partner->system;
+	memcpy(lacpdu->partner_system, partner->system, ETH_ALEN);
 	lacpdu->partner_key = htons(partner->key);
 	lacpdu->partner_port_priority = htons(partner->port_priority);
 	lacpdu->partner_port = htons(partner->port_number);
@@ -1133,7 +1129,7 @@ static void ad_rx_machine(struct lacpdu *lacpdu, struct port *port)
 			break;
 		case AD_RX_CURRENT:
 			// detect loopback situation
-			if (!MAC_ADDRESS_COMPARE(&(lacpdu->actor_system), &(port->actor_system))) {
+			if (!compare_ether_addr(lacpdu->actor_system, port->actor_system)) {
 				// INFO_RECEIVED_LOOPBACK_FRAMES
 				pr_err("%s: An illegal loopback occurred on adapter (%s).\n"
 				       "Check the configuration to verify that all adapters are connected to 802.3ad compliant switch ports\n",
@@ -1340,11 +1336,11 @@ static void ad_port_selection_logic(struct port *port)
 		}
 		// check if current aggregator suits us
 		if (((aggregator->actor_oper_aggregator_key == port->actor_oper_port_key) && // if all parameters match AND
-		     !MAC_ADDRESS_COMPARE(&(aggregator->partner_system), &(port->partner_oper.system)) &&
+		     !compare_ether_addr(aggregator->partner_system, port->partner_oper.system) &&
 		     (aggregator->partner_system_priority == port->partner_oper.system_priority) &&
 		     (aggregator->partner_oper_aggregator_key == port->partner_oper.key)
 		    ) &&
-		    ((MAC_ADDRESS_COMPARE(&(port->partner_oper.system), &(null_mac_addr)) && // partner answers
+		    ((is_zero_ether_addr(port->partner_oper.system) && // partner answers
 		      !aggregator->is_individual)  // but is not individual OR
 		    )
 		   ) {
@@ -1382,7 +1378,7 @@ static void ad_port_selection_logic(struct port *port)
 
 			port->aggregator->actor_admin_aggregator_key = port->actor_admin_port_key;
 			port->aggregator->actor_oper_aggregator_key = port->actor_oper_port_key;
-			port->aggregator->partner_system=port->partner_oper.system;
+			memcpy(port->aggregator->partner_system, port->partner_oper.system, ETH_ALEN);
 			port->aggregator->partner_system_priority = port->partner_oper.system_priority;
 			port->aggregator->partner_oper_aggregator_key = port->partner_oper.key;
 			port->aggregator->receive_state = 1;
@@ -1636,7 +1632,7 @@ static void ad_clear_agg(struct aggregator *aggregator)
 		aggregator->is_individual = false;
 		aggregator->actor_admin_aggregator_key = 0;
 		aggregator->actor_oper_aggregator_key = 0;
-		aggregator->partner_system = null_mac_addr;
+		memset(aggregator->partner_system, 0, ETH_ALEN);
 		aggregator->partner_system_priority = 0;
 		aggregator->partner_oper_aggregator_key = 0;
 		aggregator->receive_state = 0;
@@ -1659,7 +1655,7 @@ static void ad_initialize_agg(struct aggregator *aggregator)
 	if (aggregator) {
 		ad_clear_agg(aggregator);
 
-		aggregator->aggregator_mac_address = null_mac_addr;
+		memset(aggregator->aggregator_mac_address, 0, ETH_ALEN);
 		aggregator->aggregator_identifier = 0;
 		aggregator->slave = NULL;
 	}
@@ -1695,7 +1691,7 @@ static void ad_initialize_port(struct port *port, int lacp_fast)
 	if (port) {
 		port->actor_port_number = 1;
 		port->actor_port_priority = 0xff;
-		port->actor_system = null_mac_addr;
+		memset(port->actor_system, 0, ETH_ALEN);
 		port->actor_system_priority = 0xffff;
 		port->actor_port_aggregator_identifier = 0;
 		port->ntt = false;
@@ -1754,7 +1750,7 @@ static void ad_enable_collecting_distributing(struct port *port)
  */
 static void ad_disable_collecting_distributing(struct port *port)
 {
-	if (port->aggregator && MAC_ADDRESS_COMPARE(&(port->aggregator->partner_system), &(null_mac_addr))) {
+	if (port->aggregator && is_zero_ether_addr(port->aggregator->partner_system)) {
 		pr_debug("Disabling port %d(LAG %d)\n",
 			 port->actor_port_number,
 			 port->aggregator->aggregator_identifier);
@@ -1875,14 +1871,14 @@ static u16 aggregator_identifier;
 void bond_3ad_initialize(struct bonding *bond, u16 tick_resolution, int lacp_fast)
 {
 	// check that the bond is not initialized yet
-	if (MAC_ADDRESS_COMPARE(&(BOND_AD_INFO(bond).system.sys_mac_addr),
-				bond->dev->dev_addr)) {
+	if (compare_ether_addr(BOND_AD_INFO(bond).system.sys_mac_addr,
+			       bond->dev->dev_addr)) {
 
 		aggregator_identifier = 0;
 
 		BOND_AD_INFO(bond).lacp_fast = lacp_fast;
 		BOND_AD_INFO(bond).system.sys_priority = 0xFFFF;
-		BOND_AD_INFO(bond).system.sys_mac_addr = *((struct mac_addr *)bond->dev->dev_addr);
+		memcpy(BOND_AD_INFO(bond).system.sys_mac_addr, bond->dev->dev_addr, ETH_ALEN);
 
 		// initialize how many times this module is called in one second(should be about every 100ms)
 		ad_ticks_per_sec = tick_resolution;
@@ -1936,7 +1932,7 @@ int bond_3ad_bind_slave(struct slave *slave)
 			port->sm_vars &= ~AD_PORT_LACP_ENABLED;
 		}
 		// actor system is the bond's system
-		port->actor_system = BOND_AD_INFO(bond).system.sys_mac_addr;
+		memcpy(port->actor_system, BOND_AD_INFO(bond).system.sys_mac_addr, ETH_ALEN);
 		// tx timer(to verify that no more than MAX_TX_IN_SECOND lacpdu's are sent in one second)
 		port->sm_tx_timer_counter = ad_ticks_per_sec/AD_MAX_TX_IN_SECOND;
 		port->aggregator = NULL;
@@ -1951,7 +1947,7 @@ int bond_3ad_bind_slave(struct slave *slave)
 
 		ad_initialize_agg(aggregator);
 
-		aggregator->aggregator_mac_address = *((struct mac_addr *)bond->dev->dev_addr);
+		memcpy(aggregator->aggregator_mac_address, bond->dev->dev_addr, ETH_ALEN);
 		aggregator->aggregator_identifier = (++aggregator_identifier);
 		aggregator->slave = slave;
 		aggregator->is_active = 0;
@@ -2027,7 +2023,7 @@ void bond_3ad_unbind_slave(struct slave *slave)
 				new_aggregator->is_individual = aggregator->is_individual;
 				new_aggregator->actor_admin_aggregator_key = aggregator->actor_admin_aggregator_key;
 				new_aggregator->actor_oper_aggregator_key = aggregator->actor_oper_aggregator_key;
-				new_aggregator->partner_system = aggregator->partner_system;
+				memcpy(new_aggregator->partner_system, aggregator->partner_system, ETH_ALEN);
 				new_aggregator->partner_system_priority = aggregator->partner_system_priority;
 				new_aggregator->partner_oper_aggregator_key = aggregator->partner_oper_aggregator_key;
 				new_aggregator->receive_state = aggregator->receive_state;
@@ -2371,7 +2367,7 @@ int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info)
 		ad_info->ports = aggregator->num_of_ports;
 		ad_info->actor_key = aggregator->actor_oper_aggregator_key;
 		ad_info->partner_key = aggregator->partner_oper_aggregator_key;
-		memcpy(ad_info->partner_system, aggregator->partner_system.mac_addr_value, ETH_ALEN);
+		memcpy(ad_info->partner_system, aggregator->partner_system, ETH_ALEN);
 		return 0;
 	}
 
diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h
index 2c46a154..648792c 100644
--- a/drivers/net/bonding/bond_3ad.h
+++ b/drivers/net/bonding/bond_3ad.h
@@ -37,10 +37,6 @@
 #define AD_LACP_SLOW 0
 #define AD_LACP_FAST 1
 
-typedef struct mac_addr {
-	u8 mac_addr_value[ETH_ALEN];
-} mac_addr_t;
-
 enum {
 	BOND_AD_STABLE = 0,
 	BOND_AD_BANDWIDTH = 1,
@@ -112,7 +108,7 @@ typedef struct lacpdu {
 	u8 tlv_type_actor_info;	      // = actor information(type/length/value)
 	u8 actor_information_length; // = 20
 	__be16 actor_system_priority;
-	struct mac_addr actor_system;
+	u8 actor_system[ETH_ALEN];
 	__be16 actor_key;
 	__be16 actor_port_priority;
 	__be16 actor_port;
@@ -121,7 +117,7 @@ typedef struct lacpdu {
 	u8 tlv_type_partner_info;     // = partner information
 	u8 partner_information_length;	 // = 20
 	__be16 partner_system_priority;
-	struct mac_addr partner_system;
+	u8 partner_system[ETH_ALEN];
 	__be16 partner_key;
 	__be16 partner_port_priority;
 	__be16 partner_port;
@@ -149,7 +145,7 @@ typedef struct bond_marker {
 	//  = 0x02  (marker response information)
 	u8 marker_length;	 //  = 0x16
 	u16 requester_port;	 //   The number assigned to the port by the requester
-	struct mac_addr requester_system;      //   The requester's system id
+	u8 requester_system[ETH_ALEN];      //   The requester's system id
 	u32 requester_transaction_id;	//   The transaction id allocated by the requester,
 	u16 pad;		 //  = 0
 	u8 tlv_type_terminator;	     //  = 0x00
@@ -175,12 +171,12 @@ struct port;
 
 // aggregator structure(43.4.5 in the 802.3ad standard)
 typedef struct aggregator {
-	struct mac_addr aggregator_mac_address;
+	u8 aggregator_mac_address[ETH_ALEN];
 	u16 aggregator_identifier;
 	bool is_individual;
 	u16 actor_admin_aggregator_key;
 	u16 actor_oper_aggregator_key;
-	struct mac_addr partner_system;
+	u8 partner_system[ETH_ALEN];
 	u16 partner_system_priority;
 	u16 partner_oper_aggregator_key;
 	u16 receive_state;		// BOOLEAN
@@ -193,7 +189,7 @@ typedef struct aggregator {
 } aggregator_t;
 
 struct port_params {
-	struct mac_addr system;
+	u8 system[ETH_ALEN];
 	u16 system_priority;
 	u16 key;
 	u16 port_number;
@@ -205,7 +201,7 @@ struct port_params {
 typedef struct port {
 	u16 actor_port_number;
 	u16 actor_port_priority;
-	struct mac_addr actor_system;	       // This parameter is added here although it is not specified in the standard, just for simplification
+	u8 actor_system[ETH_ALEN];       // This parameter is added here although it is not specified in the standard, just for simplification
 	u16 actor_system_priority;	 // This parameter is added here although it is not specified in the standard, just for simplification
 	u16 actor_port_aggregator_identifier;
 	bool ntt;
@@ -239,7 +235,7 @@ typedef struct port {
 // system structure
 struct ad_system {
 	u16 sys_priority;
-	struct mac_addr sys_mac_addr;
+	u8 sys_mac_addr[ETH_ALEN];
 };
 
 #ifdef __ia64__



^ permalink raw reply related

* [PATCH] net_sched: remove the unused parameter of qdisc_create_dflt()
From: Changli Gao @ 2010-10-16 23:04 UTC (permalink / raw)
  To: David S. Miller
  Cc: Jamal Hadi Salim, Stephen Hemminger, netdev, netem, Changli Gao

The first parameter dev isn't in use in qdisc_create_dflt().

Signed-off-by: Changli Gao <xiaosuo@gmail.com>
---
 include/net/sch_generic.h |    3 +--
 net/sched/sch_atm.c       |    5 ++---
 net/sched/sch_cbq.c       |   12 ++++++------
 net/sched/sch_drr.c       |    4 ++--
 net/sched/sch_dsmark.c    |    6 ++----
 net/sched/sch_fifo.c      |    3 +--
 net/sched/sch_generic.c   |   10 ++++------
 net/sched/sch_hfsc.c      |    8 +++-----
 net/sched/sch_htb.c       |    8 +++-----
 net/sched/sch_mq.c        |    2 +-
 net/sched/sch_multiq.c    |    3 +--
 net/sched/sch_netem.c     |    3 +--
 net/sched/sch_prio.c      |    2 +-
 13 files changed, 28 insertions(+), 41 deletions(-)
diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
index eda8808..ea1f8a8 100644
--- a/include/net/sch_generic.h
+++ b/include/net/sch_generic.h
@@ -328,8 +328,7 @@ extern void qdisc_destroy(struct Qdisc *qdisc);
 extern void qdisc_tree_decrease_qlen(struct Qdisc *qdisc, unsigned int n);
 extern struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
 				 struct Qdisc_ops *ops);
-extern struct Qdisc *qdisc_create_dflt(struct net_device *dev,
-				       struct netdev_queue *dev_queue,
+extern struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue,
 				       struct Qdisc_ops *ops, u32 parentid);
 extern void qdisc_calculate_pkt_len(struct sk_buff *skb,
 				   struct qdisc_size_table *stab);
diff --git a/net/sched/sch_atm.c b/net/sched/sch_atm.c
index 6318e11..2825407 100644
--- a/net/sched/sch_atm.c
+++ b/net/sched/sch_atm.c
@@ -275,8 +275,7 @@ static int atm_tc_change(struct Qdisc *sch, u32 classid, u32 parent,
 		goto err_out;
 	}
 	flow->filter_list = NULL;
-	flow->q = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
-				    &pfifo_qdisc_ops, classid);
+	flow->q = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops, classid);
 	if (!flow->q)
 		flow->q = &noop_qdisc;
 	pr_debug("atm_tc_change: qdisc %p\n", flow->q);
@@ -543,7 +542,7 @@ static int atm_tc_init(struct Qdisc *sch, struct nlattr *opt)
 	INIT_LIST_HEAD(&p->flows);
 	INIT_LIST_HEAD(&p->link.list);
 	list_add(&p->link.list, &p->flows);
-	p->link.q = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
+	p->link.q = qdisc_create_dflt(sch->dev_queue,
 				      &pfifo_qdisc_ops, sch->handle);
 	if (!p->link.q)
 		p->link.q = &noop_qdisc;
diff --git a/net/sched/sch_cbq.c b/net/sched/sch_cbq.c
index 28c01ef..eb76315 100644
--- a/net/sched/sch_cbq.c
+++ b/net/sched/sch_cbq.c
@@ -1379,9 +1379,9 @@ static int cbq_init(struct Qdisc *sch, struct nlattr *opt)
 	q->link.sibling = &q->link;
 	q->link.common.classid = sch->handle;
 	q->link.qdisc = sch;
-	if (!(q->link.q = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
-					    &pfifo_qdisc_ops,
-					    sch->handle)))
+	q->link.q = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops,
+				      sch->handle);
+	if (!q->link.q)
 		q->link.q = &noop_qdisc;
 
 	q->link.priority = TC_CBQ_MAXPRIO-1;
@@ -1623,7 +1623,7 @@ static int cbq_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
 	struct cbq_class *cl = (struct cbq_class*)arg;
 
 	if (new == NULL) {
-		new = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
+		new = qdisc_create_dflt(sch->dev_queue,
 					&pfifo_qdisc_ops, cl->common.classid);
 		if (new == NULL)
 			return -ENOBUFS;
@@ -1874,8 +1874,8 @@ cbq_change_class(struct Qdisc *sch, u32 classid, u32 parentid, struct nlattr **t
 	cl->R_tab = rtab;
 	rtab = NULL;
 	cl->refcnt = 1;
-	if (!(cl->q = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
-					&pfifo_qdisc_ops, classid)))
+	cl->q = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops, classid);
+	if (!cl->q)
 		cl->q = &noop_qdisc;
 	cl->common.classid = classid;
 	cl->tparent = parent;
diff --git a/net/sched/sch_drr.c b/net/sched/sch_drr.c
index b74046a..aa8b531 100644
--- a/net/sched/sch_drr.c
+++ b/net/sched/sch_drr.c
@@ -110,7 +110,7 @@ static int drr_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 	cl->refcnt	   = 1;
 	cl->common.classid = classid;
 	cl->quantum	   = quantum;
-	cl->qdisc	   = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
+	cl->qdisc	   = qdisc_create_dflt(sch->dev_queue,
 					       &pfifo_qdisc_ops, classid);
 	if (cl->qdisc == NULL)
 		cl->qdisc = &noop_qdisc;
@@ -218,7 +218,7 @@ static int drr_graft_class(struct Qdisc *sch, unsigned long arg,
 	struct drr_class *cl = (struct drr_class *)arg;
 
 	if (new == NULL) {
-		new = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
+		new = qdisc_create_dflt(sch->dev_queue,
 					&pfifo_qdisc_ops, cl->common.classid);
 		if (new == NULL)
 			new = &noop_qdisc;
diff --git a/net/sched/sch_dsmark.c b/net/sched/sch_dsmark.c
index 63d41f8..1d295d6 100644
--- a/net/sched/sch_dsmark.c
+++ b/net/sched/sch_dsmark.c
@@ -61,8 +61,7 @@ static int dsmark_graft(struct Qdisc *sch, unsigned long arg,
 		sch, p, new, old);
 
 	if (new == NULL) {
-		new = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
-					&pfifo_qdisc_ops,
+		new = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops,
 					sch->handle);
 		if (new == NULL)
 			new = &noop_qdisc;
@@ -384,8 +383,7 @@ static int dsmark_init(struct Qdisc *sch, struct nlattr *opt)
 	p->default_index = default_index;
 	p->set_tc_index = nla_get_flag(tb[TCA_DSMARK_SET_TC_INDEX]);
 
-	p->q = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
-				 &pfifo_qdisc_ops, sch->handle);
+	p->q = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops, sch->handle);
 	if (p->q == NULL)
 		p->q = &noop_qdisc;
 
diff --git a/net/sched/sch_fifo.c b/net/sched/sch_fifo.c
index 5948baf..4dfecb0 100644
--- a/net/sched/sch_fifo.c
+++ b/net/sched/sch_fifo.c
@@ -172,8 +172,7 @@ struct Qdisc *fifo_create_dflt(struct Qdisc *sch, struct Qdisc_ops *ops,
 	struct Qdisc *q;
 	int err = -ENOMEM;
 
-	q = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
-			      ops, TC_H_MAKE(sch->handle, 1));
+	q = qdisc_create_dflt(sch->dev_queue, ops, TC_H_MAKE(sch->handle, 1));
 	if (q) {
 		err = fifo_set_limit(q, limit);
 		if (err < 0) {
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index 3d57681..2a95272 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -574,10 +574,8 @@ errout:
 	return ERR_PTR(err);
 }
 
-struct Qdisc * qdisc_create_dflt(struct net_device *dev,
-				 struct netdev_queue *dev_queue,
-				 struct Qdisc_ops *ops,
-				 unsigned int parentid)
+struct Qdisc *qdisc_create_dflt(struct netdev_queue *dev_queue,
+				struct Qdisc_ops *ops, unsigned int parentid)
 {
 	struct Qdisc *sch;
 
@@ -682,7 +680,7 @@ static void attach_one_default_qdisc(struct net_device *dev,
 	struct Qdisc *qdisc;
 
 	if (dev->tx_queue_len) {
-		qdisc = qdisc_create_dflt(dev, dev_queue,
+		qdisc = qdisc_create_dflt(dev_queue,
 					  &pfifo_fast_ops, TC_H_ROOT);
 		if (!qdisc) {
 			printk(KERN_INFO "%s: activation failed\n", dev->name);
@@ -709,7 +707,7 @@ static void attach_default_qdiscs(struct net_device *dev)
 		dev->qdisc = txq->qdisc_sleeping;
 		atomic_inc(&dev->qdisc->refcnt);
 	} else {
-		qdisc = qdisc_create_dflt(dev, txq, &mq_qdisc_ops, TC_H_ROOT);
+		qdisc = qdisc_create_dflt(txq, &mq_qdisc_ops, TC_H_ROOT);
 		if (qdisc) {
 			qdisc->ops->attach(qdisc);
 			dev->qdisc = qdisc;
diff --git a/net/sched/sch_hfsc.c b/net/sched/sch_hfsc.c
index 4749609..069c62b 100644
--- a/net/sched/sch_hfsc.c
+++ b/net/sched/sch_hfsc.c
@@ -1088,7 +1088,7 @@ hfsc_change_class(struct Qdisc *sch, u32 classid, u32 parentid,
 	cl->refcnt    = 1;
 	cl->sched     = q;
 	cl->cl_parent = parent;
-	cl->qdisc = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
+	cl->qdisc = qdisc_create_dflt(sch->dev_queue,
 				      &pfifo_qdisc_ops, classid);
 	if (cl->qdisc == NULL)
 		cl->qdisc = &noop_qdisc;
@@ -1209,8 +1209,7 @@ hfsc_graft_class(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
 	if (cl->level > 0)
 		return -EINVAL;
 	if (new == NULL) {
-		new = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
-					&pfifo_qdisc_ops,
+		new = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops,
 					cl->cl_common.classid);
 		if (new == NULL)
 			new = &noop_qdisc;
@@ -1452,8 +1451,7 @@ hfsc_init_qdisc(struct Qdisc *sch, struct nlattr *opt)
 	q->root.cl_common.classid = sch->handle;
 	q->root.refcnt  = 1;
 	q->root.sched   = q;
-	q->root.qdisc = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
-					  &pfifo_qdisc_ops,
+	q->root.qdisc = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops,
 					  sch->handle);
 	if (q->root.qdisc == NULL)
 		q->root.qdisc = &noop_qdisc;
diff --git a/net/sched/sch_htb.c b/net/sched/sch_htb.c
index 87d9449..01b519d 100644
--- a/net/sched/sch_htb.c
+++ b/net/sched/sch_htb.c
@@ -1121,8 +1121,7 @@ static int htb_graft(struct Qdisc *sch, unsigned long arg, struct Qdisc *new,
 	if (cl->level)
 		return -EINVAL;
 	if (new == NULL &&
-	    (new = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
-				     &pfifo_qdisc_ops,
+	    (new = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops,
 				     cl->common.classid)) == NULL)
 		return -ENOBUFS;
 
@@ -1247,8 +1246,7 @@ static int htb_delete(struct Qdisc *sch, unsigned long arg)
 		return -EBUSY;
 
 	if (!cl->level && htb_parent_last_child(cl)) {
-		new_q = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
-					  &pfifo_qdisc_ops,
+		new_q = qdisc_create_dflt(sch->dev_queue, &pfifo_qdisc_ops,
 					  cl->parent->common.classid);
 		last_child = 1;
 	}
@@ -1377,7 +1375,7 @@ static int htb_change_class(struct Qdisc *sch, u32 classid,
 		/* create leaf qdisc early because it uses kmalloc(GFP_KERNEL)
 		   so that can't be used inside of sch_tree_lock
 		   -- thanks to Karlis Peisenieks */
-		new_q = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
+		new_q = qdisc_create_dflt(sch->dev_queue,
 					  &pfifo_qdisc_ops, classid);
 		sch_tree_lock(sch);
 		if (parent && !parent->level) {
diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
index fe91e50..ecc302f 100644
--- a/net/sched/sch_mq.c
+++ b/net/sched/sch_mq.c
@@ -56,7 +56,7 @@ static int mq_init(struct Qdisc *sch, struct nlattr *opt)
 
 	for (ntx = 0; ntx < dev->num_tx_queues; ntx++) {
 		dev_queue = netdev_get_tx_queue(dev, ntx);
-		qdisc = qdisc_create_dflt(dev, dev_queue, &pfifo_fast_ops,
+		qdisc = qdisc_create_dflt(dev_queue, &pfifo_fast_ops,
 					  TC_H_MAKE(TC_H_MAJ(sch->handle),
 						    TC_H_MIN(ntx + 1)));
 		if (qdisc == NULL)
diff --git a/net/sched/sch_multiq.c b/net/sched/sch_multiq.c
index 6ae2512..32690de 100644
--- a/net/sched/sch_multiq.c
+++ b/net/sched/sch_multiq.c
@@ -227,8 +227,7 @@ static int multiq_tune(struct Qdisc *sch, struct nlattr *opt)
 	for (i = 0; i < q->bands; i++) {
 		if (q->queues[i] == &noop_qdisc) {
 			struct Qdisc *child, *old;
-			child = qdisc_create_dflt(qdisc_dev(sch),
-						  sch->dev_queue,
+			child = qdisc_create_dflt(sch->dev_queue,
 						  &pfifo_qdisc_ops,
 						  TC_H_MAKE(sch->handle,
 							    i + 1));
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 4714ff1..e5593c0 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -538,8 +538,7 @@ static int netem_init(struct Qdisc *sch, struct nlattr *opt)
 
 	qdisc_watchdog_init(&q->watchdog, sch);
 
-	q->qdisc = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
-				     &tfifo_qdisc_ops,
+	q->qdisc = qdisc_create_dflt(sch->dev_queue, &tfifo_qdisc_ops,
 				     TC_H_MAKE(sch->handle, 1));
 	if (!q->qdisc) {
 		pr_debug("netem: qdisc create failed\n");
diff --git a/net/sched/sch_prio.c b/net/sched/sch_prio.c
index 0748fb1..b1c95bc 100644
--- a/net/sched/sch_prio.c
+++ b/net/sched/sch_prio.c
@@ -200,7 +200,7 @@ static int prio_tune(struct Qdisc *sch, struct nlattr *opt)
 	for (i=0; i<q->bands; i++) {
 		if (q->queues[i] == &noop_qdisc) {
 			struct Qdisc *child, *old;
-			child = qdisc_create_dflt(qdisc_dev(sch), sch->dev_queue,
+			child = qdisc_create_dflt(sch->dev_queue,
 						  &pfifo_qdisc_ops,
 						  TC_H_MAKE(sch->handle, i + 1));
 			if (child) {

^ permalink raw reply related

* Re: [PATCH] CAPI: Silence lockdep warning on get_capi_appl_by_nr usage
From: Peter Zijlstra @ 2010-10-17 10:25 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: David S. Miller, Linux Kernel Mailing List, i4ldeveloper,
	Linux Netdev List, Karsten Keil
In-Reply-To: <4CB9891B.4060606@web.de>

On Sat, 2010-10-16 at 13:14 +0200, Jan Kiszka wrote:
> +       WARN_ON_ONCE(!mutex_is_locked(&capi_controller_lock));

Note there is also lockdep_assert_held() which will actually validate
that the current task is the owner.

^ 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