* Re: [PATCH 2.6.13-rc1 8/17] bonding: SYSFS INTERFACE (large)
From: Mitch Williams @ 2005-07-06 18:37 UTC (permalink / raw)
To: Dmitry Torokhov; +Cc: netdev, Radheka Godse, fubar, bonding-devel
In-Reply-To: <200507020030.03635.dtor_core@ameritech.net>
On Sat, 2 Jul 2005, Dmitry Torokhov wrote:
>
> Couple of comments:
[snip]
> > +
> > +static struct class *netdev_class;
> > +/*--------------------------- Data Structures -----------------------------*/
> > +
> > +/* Bonding sysfs lock. Why can't we just use the subsytem lock?
> > + * Because kobject_register tries to acquire the subsystem lock. If
> > + * we already hold the lock (which we would if the user was creating
> > + * a new bond through the sysfs interface), we deadlock.
> > + */
> > +
> > +struct rw_semaphore bonding_rwsem;
>
> klists were just added to the kernel proper. Does this sentiment still
> holds true?
Thanks for reviewing this patch, Dmitry. We appreciate your efforts, and
we'll make the changes you pointed out.
In this case, we hold the lock on access to all bonding-owned sysfs files,
because it's possible for changes to one file to alter the contents and/or
presence of another file. Consider:
1) process 'foo' opens /sys/class/net/bond1/mode
2) process 'bar' opens /sys/class/net/bonding_masters
3) process 'bar' writes to bonding_masters and removes bond1
4) process 'foo' tries to write
5) Boom. Or rather, oops.
Thus, we have this lock. I don't think that klists will help here.
-Mitch
^ permalink raw reply
* Re: controlling ARP Proxy scope?
From: Henrik Nordstrom @ 2005-07-06 15:32 UTC (permalink / raw)
To: Zdenek Radouch; +Cc: netdev, linux-net
In-Reply-To: <3u3gb7$1npg80@smtp05.mrf.mail.rcn.net>
On Wed, 6 Jul 2005, Zdenek Radouch wrote:
> Well, how do I tell it that I want to proxy for all machines on the
> 192.168.13.128/29 net attached to eth0.5, but not for any of
> the machines on 192.168.2.0/24 attached to eth0.6 ?
For whom going where?
What is eth0.5? A tagged 802.1q vlan on eth0, or something else? (i.e. is
it a interface of it's own according to ip link show, or just an alias?)
> It just occured to me that if I misunderstood the semantics, the setup
> may be wrong. I assumed that turning the proxy arp on on interface X
> would make the interface X answer (proxy) the ARP queries.
> Is that correct?
Yes.
>>> It is equally mind boggling to me how this could ever work with a stack
>>> allowing source-based routing, that is, a stack allowing coexistence of
>>> multiple, possibly conflicting routing tables.
>>
>> Why not?
>
> Because in rule-based routing, a table entries are only valid when the
> corresponding
> rule hits, based on the source address. In the absence of a source address
> as is the case of an ARP request, how could you possibly determine which
> of the routing tables should be consulted to decide whether the ARP query
> should be answered?
ARP requests do have valid source addresses, at least the normal ARP
queries. The duplicate IP check ARP requests and a few other obscure cases
don't.
> I agree that you wouldn't want to enter discrete addresses.
> But it could be a simple command using the standard subnet notation:
>
> arp_proxy --add 10.1.2.0/24 [11:22:33:44:55:66]
> (optional Eth address for 3rd party proxy ARP).
Which to my experience is all automatic from the routing table. But I do
remember some slight complications in source routed networks but nothing
major. Before arp_ignore existed I often used source routing as a
substitute for keeping proxy-ARP at bay but I don't remember the exact
details.
I only experienced problems when there was other IP networks on the same
Ethernet but not known to the box. In this case I had to enable the
arp_ignore sysctl to stop answering "other" ARP queries for networks not
defined on the same Ethernet interface. I also used the arp_announce to
keep the source address of ARP responses under control.
In addition in a setup which involved stations with IP addresses identical
to that of one of my own other Ethernet interfaces I had to hack the
kernel slightly to make it possible to ignore ARP duplicate IP check
packets on the relevant interfaces. But this is very special setup
involving NAT and other nastinesses which shouldn't be encountered in any
reasonably normal network situation.
Regards
Henrik
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Thomas Graf @ 2005-07-06 12:42 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev
In-Reply-To: <42CB2B84.50702@cosmosbay.com>
* Eric Dumazet <42CB2B84.50702@cosmosbay.com> 2005-07-06 02:53
A short recap after some coffee and sleep:
The inital issue you brought up which you backed up with numbers
is probably the cause of multiple wrong branch predictions due
to the fact that I wrote skb = dequeue(); if (skb) which was
assumed to be likely by the compiler. In your patch you fixed
this with !skb_queue_empty() which fixed this wrong prediction
and also acts as a little optimization due to skb_queue_empty()
being really simple to implement for the compiler. The patch
I posted should result in almost the same result, despite of
the additional wrong branch prediction for the loop it always
has one wrong prediction which is when we hit a non-empty queue.
In your unrolled version you could optimize it even more by
taking advantage that prio=2 is the most likely non-empty queue
so you could change the check to likely and save a wrong branch
prediction for the common case at the cost of a branch misprediction
if all queues are empty.
The patch I posted results in something like this:
pfifo_fast_dequeue:
pushl %ebx
xorl %ecx, %ecx
movl 8(%esp), %ebx
leal 128(%ebx), %edx
.L129:
movl (%edx), %eax
cmpl %edx, %eax
jne .L132 ; if (!skb_queue_empty())
incl %ecx
addl $20, %edx
cmpl $2, %ecx
jle .L129 ; end of loop
xorl %eax, %eax ; all queues empty
.L117:
popl %ebx
ret
I regard the miss here as acceptable for the increased flexibility
we get. It can be optimized with a loop unrolling but my opinion
is to try and avoid it if possible.
Now your second thought is quite interesting, although it heavly
depends on the fact that prio=2 is the most often used band. It
will be interesting to see some numbers.
> static struct sk_buff *
> pfifo_fast_dequeue(struct Qdisc* qdisc)
> {
> struct sk_buff_head *list = qdisc_priv(qdisc);
> struct sk_buff_head *best = NULL;
>
> list += 2;
> if (!skb_queue_empty(list))
> best = list;
> list--;
> if (!skb_queue_empty(list))
> best = list;
> list--;
> if (!skb_queue_empty(list))
> best = list;
Here is what I mean, a likely() should be even better.
> if (best) {
> qdisc->q.qlen--;
> return __skb_dequeue(best);
> }
> return NULL;
> }
^ permalink raw reply
* Re: controlling ARP Proxy scope?
From: Zdenek Radouch @ 2005-07-06 4:32 UTC (permalink / raw)
To: Henrik Nordstrom; +Cc: netdev, linux-net
In-Reply-To: <Pine.LNX.4.61.0507060405300.26027@filer.marasystems.com>
At 04:20 AM 7/6/05 +0200, Henrik Nordstrom wrote:
>On Tue, 5 Jul 2005, Zdenek Radouch wrote:
>
>>> proxy_arp simply ARPs if there is a route for the requested destination
>>> going out on another interface than where the ARP was seen.
>>
>> In my case, the proxy replies to a request seen on the very same interface
>> to which the route points to.
>
>Are you really sure on this? This part has always worked fine for me with
>Linux proxy-arp and a large variety of different kernels.
Yes, I am very sure about it. Of all attached networks, only one is a 10.*
network, and the address answered, 10.1.2.1 happens to be the main
router/gateway
to the rest of the world. My machine (with proxy arp) answers
incorrectly the ARP requests (for 10.1.2.1) from other machines on the
10.1.2.* subnet. Once it steals the role of the gateway, my machine then
forwards the packets to its own default gateway which is behind the proxied
machines.
As a result of this mess, I see foreign traffic diverted through my proxy
server
via a private network to another gateway on the other side.
>
>> I find the idea to proxy based on routing tables quite questionable.
>
>So do I. The manual proxy-arp entries method suits me much better, but is
>a pain due to lack of range support (probably why it got removed in 2.4)
>
>> It may work is some pretty trivial cases, but will very obviously fail
>> with a more complex configuration.
>
>Haven't managed to find a single situation not solveable yet.. and this
>involves pretty complex configurations.. I don't remember which of the
>sysctls mentioned earlier did the trick, but once enabled things starts to
>behave quite sanely even when there is multiple foreign networks
>unexpectedly carried on the same Ethernet. IIRC the settings I settled for
>was
>
> arp_ignore = 1
> arp_announce = 1
>
>> I have seven or eight networks attached to the node, and I certainly do
>> not want to proxy for every single address one may find in the routing
>> tables.
>
>Then don't.
Well, how do I tell it that I want to proxy for all machines on the
192.168.13.128/29 net attached to eth0.5, but not for any of
the machines on 192.168.2.0/24 attached to eth0.6 ?
It just occured to me that if I misunderstood the semantics, the setup
may be wrong. I assumed that turning the proxy arp on on interface X
would make the interface X answer (proxy) the ARP queries.
Is that correct?
(The alternative would be that turning the bit on interface X would
make all other interfaces answer on behalf of X).
>
>> It is equally mind boggling to me how this could ever work with a stack
>> allowing source-based routing, that is, a stack allowing coexistence of
>> multiple, possibly conflicting routing tables.
>
>Why not?
Because in rule-based routing, a table entries are only valid when the
corresponding
rule hits, based on the source address. In the absence of a source address
as is the case of an ARP request, how could you possibly determine which
of the routing tables should be consulted to decide whether the ARP query
should be answered?
>
>> Sounds to me like I am going to have to rewrite the module. It needs to be
>> configured manually
>
>Well, for most setups it does work automagically. Just bring up the
>interfaces with the same IP, route the network out on the "main" interface
>having most hosts and host (or subnet) route the other out the other
>interface. ARP then follows automatically.
>
>But in messy networks or when your routing table is not correct then
>sysctls is needed to restrict when to respond to stop you from responding
>to ARP requests to outside/foreign networks.
>
>Probably isn't very hard to bring back the support for published proxy-arp
>entries if needed. But without range support it's a pain to maitain in
>most setups requiring proxy-arp as you then need an ARP entry for every
>"other" station on each interface involved in proxy-arp, meaning that if
>you proxy-arp a /24 network then you need 253 proxy-arp entries (one per
>station, defining which interface it belongs on). In the normal situation
>that you only act as a proxy-arp gateway for less than a handful stations
>this is a significant administrative overhead compared to just configuring
>routing which is required anyway.
I agree that you wouldn't want to enter discrete addresses.
But it could be a simple command using the standard subnet notation:
arp_proxy --add 10.1.2.0/24 [11:22:33:44:55:66]
(optional Eth address for 3rd party proxy ARP).
Regards
-Zdenek
^ permalink raw reply
* Re: controlling ARP Proxy scope?
From: Henrik Nordstrom @ 2005-07-06 2:20 UTC (permalink / raw)
To: Zdenek Radouch; +Cc: netdev, linux-net
In-Reply-To: <3u3gb7$1no73u@smtp05.mrf.mail.rcn.net>
On Tue, 5 Jul 2005, Zdenek Radouch wrote:
>> proxy_arp simply ARPs if there is a route for the requested destination
>> going out on another interface than where the ARP was seen.
>
> In my case, the proxy replies to a request seen on the very same interface
> to which the route points to.
Are you really sure on this? This part has always worked fine for me with
Linux proxy-arp and a large variety of different kernels.
> I find the idea to proxy based on routing tables quite questionable.
So do I. The manual proxy-arp entries method suits me much better, but is
a pain due to lack of range support (probably why it got removed in 2.4)
> It may work is some pretty trivial cases, but will very obviously fail
> with a more complex configuration.
Haven't managed to find a single situation not solveable yet.. and this
involves pretty complex configurations.. I don't remember which of the
sysctls mentioned earlier did the trick, but once enabled things starts to
behave quite sanely even when there is multiple foreign networks
unexpectedly carried on the same Ethernet. IIRC the settings I settled for
was
arp_ignore = 1
arp_announce = 1
> I have seven or eight networks attached to the node, and I certainly do
> not want to proxy for every single address one may find in the routing
> tables.
Then don't.
> It is equally mind boggling to me how this could ever work with a stack
> allowing source-based routing, that is, a stack allowing coexistence of
> multiple, possibly conflicting routing tables.
Why not?
> Sounds to me like I am going to have to rewrite the module. It needs to be
> configured manually
Well, for most setups it does work automagically. Just bring up the
interfaces with the same IP, route the network out on the "main" interface
having most hosts and host (or subnet) route the other out the other
interface. ARP then follows automatically.
But in messy networks or when your routing table is not correct then
sysctls is needed to restrict when to respond to stop you from responding
to ARP requests to outside/foreign networks.
Probably isn't very hard to bring back the support for published proxy-arp
entries if needed. But without range support it's a pain to maitain in
most setups requiring proxy-arp as you then need an ARP entry for every
"other" station on each interface involved in proxy-arp, meaning that if
you proxy-arp a /24 network then you need 253 proxy-arp entries (one per
station, defining which interface it belongs on). In the normal situation
that you only act as a proxy-arp gateway for less than a handful stations
this is a significant administrative overhead compared to just configuring
routing which is required anyway.
Regards
Henrik
^ permalink raw reply
* Re: controlling ARP Proxy scope?
From: Zdenek Radouch @ 2005-07-06 1:55 UTC (permalink / raw)
To: Henrik Nordstrom; +Cc: netdev, linux-net
In-Reply-To: <Pine.LNX.4.61.0507022316310.26953@filer.marasystems.com>
At 11:21 PM 7/2/05 +0200, Henrik Nordstrom wrote:
>On Fri, 1 Jul 2005, Zdenek Radouch wrote:
>
>> So, left with only a binary flag in /proc, and network definition on the
>> interface,
>> I assumed (perhaps naively) that the arp would proxy only for the addresses
>> within the subnet defined for the interface (on which the proxy arp is
>> turned on).
>> However, that does not seem to be the case.
>
>You may be able to tune this with either arp_filter or arp_ignore.
Unfortunately I can't. Not without adding more code to what is quite
obviously a bunch of kludgy patches for an ill-conceived ARP proxy design.
>
>> I have an interface with address 10.1.2.219 and mask 255.255.255.248 with
>> proxy arp turned on on this interface, and the machine is responding
>> (I see that with tcpdump) to arp requests for address 10.1.2.1, i.e.,
>> an address outside of the proxy interface's subnet.
>
>Correct.
>
>> Can anyone explain the behavior?
>
>proxy_arp simply ARPs if there is a route for the requested destination
>going out on another interface than where the ARP was seen.
In my case, the proxy replies to a request seen on the very same interface
to which the route points to. That's wrong no matter how you look at it;
this is a route to which this node will be routing, i.e., this node will be
ARPing
for this route address - it itself should not reply to such requests, nor
could
it ever successfully do so.
I find the idea to proxy based on routing tables quite questionable. It
may work
is some pretty trivial cases, but will very obviously fail with a more complex
configuration. I have seven or eight networks attached to the node, and I
certainly do not want to proxy for every single address one may find in the
routing tables.
It is equally mind boggling to me how this could ever work with a stack
allowing
source-based routing, that is, a stack allowing coexistence of multiple,
possibly
conflicting routing tables.
Sounds to me like I am going to have to rewrite the module. It needs to be
configured manually - the notion that it could work automagically, without
external configuration is quite unrealistic, as one can see from the code
in arp_filter and arp_ignore.
Thanks for the pointers.
-Zdenek
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Eric Dumazet @ 2005-07-06 1:09 UTC (permalink / raw)
To: Thomas Graf; +Cc: David S. Miller, netdev
In-Reply-To: <20050706010200.GU16076@postel.suug.ch>
Thomas Graf a écrit :
> I think you got me wrong, the whole point of this qdisc
> is to prioritize which means that we cannot dequeue from
> prio 1 as long as the queue in prio 0 is not empty.
if prio 0 is not empty, then the last
if (!skb_queue_empty(list))
best = list;
will set 'best' to the prio 0 list, and we dequeue the packet on this prio 0 list, not on prio 1 or prio 2.
>
> If you have no traffic at all for prio=0 and prio=1 then
> the best solution is to replace the qdisc on the device
> with a simple fifo.
Yes sure, but I know that already. Unfortunatly I have some trafic on prio=1 and prio=0 (about 5 %)
Thank you
Eric
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Thomas Graf @ 2005-07-06 1:07 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev
In-Reply-To: <42CB2E24.6010303@cosmosbay.com>
* Eric Dumazet <42CB2E24.6010303@cosmosbay.com> 2005-07-06 03:04
> Thomas Graf a écrit :
>
> >>Maybe we can rewrite the whole thing without branches, examining prio
> >>from PFIFO_FAST_BANDS-1 down to 0, at least for modern cpu with
> >>conditional mov (cmov)
> >
> >
> >This would break the whole thing, the qdisc is supposed to try and
> >dequeue from the highest priority queue (prio=0) first.
> >
> >
>
> I still dequeue a packet from the highest priority queue.
Ahh... sorry, I misread your patch, interesting idea. I'll
be waiting for your numbers.
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Eric Dumazet @ 2005-07-06 1:04 UTC (permalink / raw)
To: Thomas Graf; +Cc: David S. Miller, netdev
In-Reply-To: <20050706005140.GT16076@postel.suug.ch>
Thomas Graf a écrit :
>>Maybe we can rewrite the whole thing without branches, examining prio from
>>PFIFO_FAST_BANDS-1 down to 0, at least for modern cpu with conditional mov
>>(cmov)
>
>
> This would break the whole thing, the qdisc is supposed to try and
> dequeue from the highest priority queue (prio=0) first.
>
>
I still dequeue a packet from the highest priority queue.
But nothing prevents us to look the three queues in the reverse order, if you can avoid the conditional branches.
No memory penalty, since most of time we were looking at the three queues anyway, and the 3 sk_buff_head are in the same cache line.
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Thomas Graf @ 2005-07-06 1:02 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev
In-Reply-To: <42CB2B84.50702@cosmosbay.com>
* Eric Dumazet <42CB2B84.50702@cosmosbay.com> 2005-07-06 02:53
> (Sorry, still using 2.6.12, but the idea remains)
I think you got me wrong, the whole point of this qdisc
is to prioritize which means that we cannot dequeue from
prio 1 as long as the queue in prio 0 is not empty.
If you have no traffic at all for prio=0 and prio=1 then
the best solution is to replace the qdisc on the device
with a simple fifo.
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Eric Dumazet @ 2005-07-06 0:53 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Thomas Graf, David S. Miller, netdev
In-Reply-To: <42CB2698.2080904@cosmosbay.com>
Eric Dumazet a écrit :
>
>
> Maybe we can rewrite the whole thing without branches, examining prio
> from PFIFO_FAST_BANDS-1 down to 0, at least for modern cpu with
> conditional mov (cmov)
>
> struct sk_buff_head *best = NULL;
> struct sk_buff_head *list = qdisc_priv(qdisc)+PFIFO_FAST_BANDS-1;
> if (skb_queue_empty(list)) best = list ;
> list--;
> if (skb_queue_empty(list)) best = list ;
> list--;
> if (skb_queue_empty(list)) best = list ;
> if (best != NULL) {
> qdisc->q.qlen--;
> return __qdisc_dequeue_head(qdisc, best);
> }
>
> This version should have one branch.
> I will test this after some sleep :)
> See you
> Eric
>
>
(Sorry, still using 2.6.12, but the idea remains)
static struct sk_buff *
pfifo_fast_dequeue(struct Qdisc* qdisc)
{
struct sk_buff_head *list = qdisc_priv(qdisc);
struct sk_buff_head *best = NULL;
list += 2;
if (!skb_queue_empty(list))
best = list;
list--;
if (!skb_queue_empty(list))
best = list;
list--;
if (!skb_queue_empty(list))
best = list;
if (best) {
qdisc->q.qlen--;
return __skb_dequeue(best);
}
return NULL;
}
At least the compiler output seems promising :
0000000000000550 <pfifo_fast_dequeue>:
550: 48 8d 97 f0 00 00 00 lea 0xf0(%rdi),%rdx
557: 31 c9 xor %ecx,%ecx
559: 48 8d 87 c0 00 00 00 lea 0xc0(%rdi),%rax
560: 48 39 97 f0 00 00 00 cmp %rdx,0xf0(%rdi)
567: 48 0f 45 ca cmovne %rdx,%rcx
56b: 48 8d 97 d8 00 00 00 lea 0xd8(%rdi),%rdx
572: 48 39 97 d8 00 00 00 cmp %rdx,0xd8(%rdi)
579: 48 0f 45 ca cmovne %rdx,%rcx
57d: 48 39 87 c0 00 00 00 cmp %rax,0xc0(%rdi)
584: 48 0f 45 c8 cmovne %rax,%rcx
588: 31 c0 xor %eax,%eax
58a: 48 85 c9 test %rcx,%rcx
58d: 74 32 je 5c1 <pfifo_fast_dequeue+0x71> // one conditional branch
58f: ff 4f 40 decl 0x40(%rdi)
592: 48 8b 11 mov (%rcx),%rdx
595: 48 39 ca cmp %rcx,%rdx
598: 74 27 je 5c1 <pfifo_fast_dequeue+0x71> // never taken branch : always predicted OK
59a: 48 89 d0 mov %rdx,%rax
59d: 48 8b 12 mov (%rdx),%rdx
5a0: ff 49 10 decl 0x10(%rcx)
5a3: 48 c7 40 10 00 00 00 movq $0x0,0x10(%rax)
5aa: 00
5ab: 48 89 4a 08 mov %rcx,0x8(%rdx)
5af: 48 89 11 mov %rdx,(%rcx)
5b2: 48 c7 40 08 00 00 00 movq $0x0,0x8(%rax)
5b9: 00
5ba: 48 c7 00 00 00 00 00 movq $0x0,(%rax)
5c1: 90 nop
5c2: c3 retq
I Will post tomorrow some profiling results.
Eric
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Thomas Graf @ 2005-07-06 0:51 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev
In-Reply-To: <42CB2698.2080904@cosmosbay.com>
* Eric Dumazet <42CB2698.2080904@cosmosbay.com> 2005-07-06 02:32
> Hum... shouldnt it be :
>
> + skb = __qdisc_dequeue_head(qdisc, list + prio);
>
Correct.
> Anyway, the branches misprediction come from the fact that most of packets
> are queued in the prio=2 list.
>
> So each time this function is called, a non unrolled version has to pay 2
> to 5 branches misprediction.
>
> if ((!skb_queue_empty(list + prio)) /* branch not taken, mispredict when
> prio=0 */
The !expr implies an unlikely so the prediction should be right and
equal to your unrolling version.
> Maybe we can rewrite the whole thing without branches, examining prio from
> PFIFO_FAST_BANDS-1 down to 0, at least for modern cpu with conditional mov
> (cmov)
This would break the whole thing, the qdisc is supposed to try and
dequeue from the highest priority queue (prio=0) first.
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Eric Dumazet @ 2005-07-06 0:32 UTC (permalink / raw)
To: Thomas Graf; +Cc: David S. Miller, netdev
In-Reply-To: <20050705234104.GR16076@postel.suug.ch>
Thomas Graf a écrit :
> I still think we can fix this performance issue without manually
> unrolling the loop or we should at least try to. In the end gcc
> should notice the constant part of the loop and move it out so
> basically the only difference should the additional prio++ and
> possibly a failing branch prediction.
>
> What about this? I'm still not sure where exactly all the time
> is lost so this is a shot in the dark.
>
> Index: net-2.6/net/sched/sch_generic.c
> ===================================================================
> --- net-2.6.orig/net/sched/sch_generic.c
> +++ net-2.6/net/sched/sch_generic.c
> @@ -330,10 +330,11 @@ static struct sk_buff *pfifo_fast_dequeu
> {
> int prio;
> struct sk_buff_head *list = qdisc_priv(qdisc);
> + struct sk_buff *skb;
>
> - for (prio = 0; prio < PFIFO_FAST_BANDS; prio++, list++) {
> - struct sk_buff *skb = __qdisc_dequeue_head(qdisc, list);
> - if (skb) {
> + for (prio = 0; prio < PFIFO_FAST_BANDS; prio++) {
> + if (!skb_queue_empty(list + prio)) {
> + skb = __qdisc_dequeue_head(qdisc, list);
> qdisc->q.qlen--;
> return skb;
> }
>
>
Hum... shouldnt it be :
+ skb = __qdisc_dequeue_head(qdisc, list + prio);
?
Anyway, the branches misprediction come from the fact that most of packets are queued in the prio=2 list.
So each time this function is called, a non unrolled version has to pay 2 to 5 branches misprediction.
if ((!skb_queue_empty(list + prio)) /* branch not taken, mispredict when prio=0 */
if ((!skb_queue_empty(list + prio)) /* branch not taken, mispredict when prio=1 */
if ((!skb_queue_empty(list + prio)) /* branch taken (or not if queue is really empty), mispredict when prio=2 */
Maybe we can rewrite the whole thing without branches, examining prio from PFIFO_FAST_BANDS-1 down to 0, at least for modern cpu with
conditional mov (cmov)
struct sk_buff_head *best = NULL;
struct sk_buff_head *list = qdisc_priv(qdisc)+PFIFO_FAST_BANDS-1;
if (skb_queue_empty(list)) best = list ;
list--;
if (skb_queue_empty(list)) best = list ;
list--;
if (skb_queue_empty(list)) best = list ;
if (best != NULL) {
qdisc->q.qlen--;
return __qdisc_dequeue_head(qdisc, best);
}
This version should have one branch.
I will test this after some sleep :)
See you
Eric
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Thomas Graf @ 2005-07-05 23:55 UTC (permalink / raw)
To: David S. Miller; +Cc: dada1, netdev
In-Reply-To: <20050705.164503.104035718.davem@davemloft.net>
* David S. Miller <20050705.164503.104035718.davem@davemloft.net> 2005-07-05 16:45
> From: Thomas Graf <tgraf@suug.ch>
> Date: Wed, 6 Jul 2005 01:41:04 +0200
>
> > I still think we can fix this performance issue without manually
> > unrolling the loop or we should at least try to. In the end gcc
> > should notice the constant part of the loop and move it out so
> > basically the only difference should the additional prio++ and
> > possibly a failing branch prediction.
>
> But the branch prediction is where I personally think a lot
> of the lossage is coming from. These can cost upwards of 20
> or 30 processor cycles, easily. That's getting close to the
> cost of a L2 cache miss.
Absolutely. I think what happens is that we produce predicion
failures due to the logic within qdisc_dequeue_head(), I
cannot back this up with numbers though.
> I see the difficulties with this change now, why don't we revisit
> this some time in the future?
Fine with me.
Eric, the patch I just posted should result in the same branch
prediction as your loop unrolling. The only additional overhead
we still have is the list + prio thing and an additional conditional
jump to do the loop. If you have the cycles etc. it would be nice
to compare it with your numbers.
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: David S. Miller @ 2005-07-05 23:45 UTC (permalink / raw)
To: tgraf; +Cc: dada1, netdev
In-Reply-To: <20050705234104.GR16076@postel.suug.ch>
From: Thomas Graf <tgraf@suug.ch>
Date: Wed, 6 Jul 2005 01:41:04 +0200
> I still think we can fix this performance issue without manually
> unrolling the loop or we should at least try to. In the end gcc
> should notice the constant part of the loop and move it out so
> basically the only difference should the additional prio++ and
> possibly a failing branch prediction.
But the branch prediction is where I personally think a lot
of the lossage is coming from. These can cost upwards of 20
or 30 processor cycles, easily. That's getting close to the
cost of a L2 cache miss.
I see the difficulties with this change now, why don't we revisit
this some time in the future?
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Thomas Graf @ 2005-07-05 23:41 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev
In-Reply-To: <42CB14B2.5090601@cosmosbay.com>
* Eric Dumazet <42CB14B2.5090601@cosmosbay.com> 2005-07-06 01:16
> Oh well, I was unaware of last changes in 2.6.13-rc1 :(
Ok, this clarifies a lot for me, I was under the impression
you knew about these changes.
> Given the fact that the PFIFO_FAST_BANDS macro was introduced, I wonder if
> the patch should be this one or not...
> Should we assume PFIFO_FAST_BANDS will stay at 3 or what ?
It is very unlikely to change within mainline but the idea behind
it is to allow it be changed at compile time.
I still think we can fix this performance issue without manually
unrolling the loop or we should at least try to. In the end gcc
should notice the constant part of the loop and move it out so
basically the only difference should the additional prio++ and
possibly a failing branch prediction.
What about this? I'm still not sure where exactly all the time
is lost so this is a shot in the dark.
Index: net-2.6/net/sched/sch_generic.c
===================================================================
--- net-2.6.orig/net/sched/sch_generic.c
+++ net-2.6/net/sched/sch_generic.c
@@ -330,10 +330,11 @@ static struct sk_buff *pfifo_fast_dequeu
{
int prio;
struct sk_buff_head *list = qdisc_priv(qdisc);
+ struct sk_buff *skb;
- for (prio = 0; prio < PFIFO_FAST_BANDS; prio++, list++) {
- struct sk_buff *skb = __qdisc_dequeue_head(qdisc, list);
- if (skb) {
+ for (prio = 0; prio < PFIFO_FAST_BANDS; prio++) {
+ if (!skb_queue_empty(list + prio)) {
+ skb = __qdisc_dequeue_head(qdisc, list);
qdisc->q.qlen--;
return skb;
}
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Eric Dumazet @ 2005-07-05 23:16 UTC (permalink / raw)
To: David S. Miller, tgraf; +Cc: netdev
In-Reply-To: <20050705.143548.28788459.davem@davemloft.net>
[-- Attachment #1: Type: text/plain, Size: 921 bytes --]
David S. Miller a écrit :
> From: Thomas Graf <tgraf@suug.ch>
> Date: Tue, 5 Jul 2005 23:33:55 +0200
>
>
>>* David S. Miller <20050705.142210.14973612.davem@davemloft.net> 2005-07-05 14:22
>>
>>>So I'll apply the original unrolling patch for now.
>>
>>The patch must be changed to use __qdisc_dequeue_head() instead of
>>__skb_dequeue() or we screw up the backlog.
>
>
> Ok, good thing the patch didn't apply correctly anyways :)
>
>
Oh well, I was unaware of last changes in 2.6.13-rc1 :(
Given the fact that the PFIFO_FAST_BANDS macro was introduced, I wonder if the patch should be this one or not...
Should we assume PFIFO_FAST_BANDS will stay at 3 or what ?
[NET] : unroll a small loop in pfifo_fast_dequeue(). Compiler generates better code.
oprofile says this function uses now 0.29% instead of 1.22 %, on a x86_64 target.
Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
[-- Attachment #2: patch.sch_generic --]
[-- Type: text/plain, Size: 1015 bytes --]
--- linux-2.6.13-rc1/net/sched/sch_generic.c 2005-07-06 00:46:53.000000000 +0200
+++ linux-2.6.13-rc1-ed/net/sched/sch_generic.c 2005-07-06 01:05:04.000000000 +0200
@@ -328,18 +328,31 @@
static struct sk_buff *pfifo_fast_dequeue(struct Qdisc* qdisc)
{
- int prio;
struct sk_buff_head *list = qdisc_priv(qdisc);
- for (prio = 0; prio < PFIFO_FAST_BANDS; prio++, list++) {
- struct sk_buff *skb = __qdisc_dequeue_head(qdisc, list);
- if (skb) {
- qdisc->q.qlen--;
- return skb;
+#if PFIFO_FAST_BANDS == 3
+ for (;;) {
+ if (!skb_queue_empty(list))
+ break;
+ list++;
+ if (!skb_queue_empty(list))
+ break;
+ list++;
+ if (!skb_queue_empty(list))
+ break;
+ return NULL;
}
- }
-
- return NULL;
+#else
+ int prio;
+ for (prio = 0;; list++) {
+ if (!skb_queue_empty(list))
+ break;
+ if (++prio == PFIFO_FAST_BANDS)
+ return NULL;
+ }
+#endif
+ qdisc->q.qlen--;
+ return __qdisc_dequeue_head(qdisc, list);
}
static int pfifo_fast_requeue(struct sk_buff *skb, struct Qdisc* qdisc)
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: David S. Miller @ 2005-07-05 21:35 UTC (permalink / raw)
To: tgraf; +Cc: dada1, netdev
In-Reply-To: <20050705213355.GM16076@postel.suug.ch>
From: Thomas Graf <tgraf@suug.ch>
Date: Tue, 5 Jul 2005 23:33:55 +0200
> * David S. Miller <20050705.142210.14973612.davem@davemloft.net> 2005-07-05 14:22
> > So I'll apply the original unrolling patch for now.
>
> The patch must be changed to use __qdisc_dequeue_head() instead of
> __skb_dequeue() or we screw up the backlog.
Ok, good thing the patch didn't apply correctly anyways :)
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Thomas Graf @ 2005-07-05 21:33 UTC (permalink / raw)
To: David S. Miller; +Cc: dada1, netdev
In-Reply-To: <20050705.142210.14973612.davem@davemloft.net>
* David S. Miller <20050705.142210.14973612.davem@davemloft.net> 2005-07-05 14:22
> So I'll apply the original unrolling patch for now.
The patch must be changed to use __qdisc_dequeue_head() instead of
__skb_dequeue() or we screw up the backlog.
^ permalink raw reply
* tg3 polling extension
From: Qinghua(Kevin) Ye @ 2005-07-05 21:32 UTC (permalink / raw)
To: netdev; +Cc: click
Hi, all,
I am thinking about extending the tg3 driver to support Click Polling. Click has its patch on linux kernel, and adds some callback functions and polling variable to the net_device struct(I did this work on 2.6.11 kernel, and on AMD opteron):
netdev->polling = 0; // Check if click polling is enabled or not
netdev->rx_poll = tg3_rx_poll;
// Click calls rx_poll whenever PollDevice is scheduled. rx_poll
returns a list of sk_buff's that has been sitting in the NICs receive
buffer. rx_poll should NULL the sk_buff's send to Click.
netdev->rx_refill = tg3_rx_refill;
// If rx_poll returned any sk_buff's PollDevice then calls rx_refill
and passes a new list of sk_buff's to the driver. rx_refill should use
this list to update the drivers receive buffer and receive
descriptors.
netdev->tx_clean = tg3_tx_clean;
// Whenever ToDevice is scheduled it calls tx_clean to clean already
send sk_buffs from the NICs transmit queue. tx_clean should return
this list.
netdev->tx_queue = tg3_tx_pqueue;
//ToDevice then calls tx_queue and passes an sk_buff to the driver for
transmission.
netdev->tx_eob = tg3_tx_eob;
/tx_eob should set the TX Descriptor Tail
netdev->tx_start = tg3_tx_start;
//tc_start calls tx_eob - Anyone more info on this ?
netdev->poll_off = tg3_poll_off;
//disabling click polling and enabling the NICs irq
netdev->poll_on = tg3_poll_on;
//disabling the NICs irq and enabling click polling
Based on this extension, I wrote these functions as follows. However, it works not well. For uni-process, it does work, but can only send thousands of packet if there is no packet coming in, no matter how many packets supposed to send out. I debugged it, seems that the problem is on the tg3_tx_clean side. It cannot clean the packet buffer on time. So the interface will be timeout, and reseted again and again. However, once there are many packets comming, the tx can send more packets out comparative to the number of packets comming in.
Another problem is: When I start two processes to deal with two interface, the system will crash after process tens of thousands packets. Sometimes I got this message in syslog:
Jun 30 13:00:42 node13 kernel: Oops: 0000 [1] SMP
Jun 30 13:00:42 node13 kernel: CPU 0
Jun 30 13:00:42 node13 kernel: Modules linked in: click proclikefs tg3 nfs lockd sunrpc ib_mthca lp button autofs ib_ipoib ib_sa ib_mad ib_core ohci1394 ieee1394 floppy parport_pc parport usbcore ide_disk ide_core
Jun 30 13:00:42 node13 kernel: Pid: 4300, comm: kclick Tainted: GF 2.6.11.6-click
Jun 30 13:00:42 node13 kernel: RIP: 0010:[<ffffffff881dfc14>] <ffffffff881dfc14>{:click:_ZN10PollDevice8run_taskEv+196}
Jun 30 13:00:42 node13 kernel: RSP: 0018:ffff81007ee65e48 EFLAGS: 00010297
Jun 30 13:00:42 node13 kernel: RAX: 000000000000006b RBX: 0000000000000000 RCX: 0000000000000002
Jun 30 13:00:42 node13 kernel: RDX: 0000000000000740 RSI: ffff81007e681340 RDI: 0000000000000000
Jun 30 13:00:42 node13 kernel: RBP: ffff81007e681340 R08: 0000000000000000 R09: 0000000000000000
Jun 30 13:00:42 node13 kernel: R10: 0000000000000690 R11: 0000000000000000 R12: ffff8100e605ce00
Jun 30 13:00:42 node13 kernel: R13: 0000000000000000 R14: 0000000000000001 R15: ffff81007ee65e58
Jun 30 13:00:42 node13 kernel: FS: 00002aaaaae044c0(0000) GS:ffffffff8047a980(0000) knlGS:0000000000000000
Jun 30 13:00:42 node13 kernel: CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
Jun 30 13:00:42 node13 kernel: CR2: 0000000000000000 CR3: 00000000e616c000 CR4: 00000000000006e0
Jun 30 13:00:42 node13 kernel: Process kclick (pid: 4300, threadinfo ffff81007ee64000, task ffff81007fc190d0)
Jun 30 13:00:42 node13 kernel: Stack: 0000000000000000 0000000200000000 0000000042c4415a 0000000000082509
Jun 30 13:00:42 node13 kernel: ffff8100e60899c0 ffff8100e605cea0 0000000000000006 0000000000000046
Jun 30 13:00:42 node13 kernel: ffff8100e60899c0 0000000000000000
Jun 30 13:00:42 node13 kernel: Call Trace:<ffffffff881a420d>{:click:_ZN12RouterThread6driverEv+973}
Jun 30 13:00:42 node13 kernel: <ffffffff8820324a>{:click:_Z11click_schedPv+170} <ffffffff8010ef9f>{child_rip+8}
Jun 30 13:00:42 node13 kernel: <ffffffff882031a0>{:click:_Z11click_schedPv+0} <ffffffff8010ef97>{child_rip+0}
Jun 30 13:00:42 node13 kernel:
Jun 30 13:00:42 node13 kernel:
Jun 30 13:00:42 node13 kernel: Code: 4d 8b 6d 00 48 c7 03 00 00 00 00 4d 85 ed 74 10 41 0f 18 4d
Jun 30 13:00:42 node13 kernel: RIP <ffffffff881dfc14>{:click:_ZN10PollDevice8run_taskEv+196} RSP <ffff81007ee65e48>
Jun 30 13:00:42 node13 kernel: CR2: 0000000000000000
Any idea will be appreciated! Thanks.
Qinghua
Extended functions:
static struct sk_buff *
tg3_rx_poll(struct net_device *dev, int *want)
{
struct tg3 *tp=dev->priv;
int budget = *want;
struct sk_buff *skb_head = 0, *skb_last = 0;
int skb_size;
u32 rx_rcb_ptr;
u16 hw_idx, sw_idx;
int received;
int has_jumbo=0;
struct tg3_hw_status *sblk = tp->hw_status;
/* process the reset request to avoid reenable ints*/
if(tp->reset == 1){
tp->reset = 0;
tg3_reset_task(tp);
spin_lock(&tp->lock);
tg3_disable_ints(tp);
spin_unlock(&tp->lock);
}
*want = 0;
rx_rcb_ptr=tp->rx_rcb_ptr;
hw_idx = tp->hw_status->idx[0].rx_producer;
/*
* We need to order the read of hw_idx and the read of
* the opaque cookie.
*/
rmb();
spin_lock(&tp->lock);
sw_idx = rx_rcb_ptr % TG3_RX_RCB_RING_SIZE(tp);
received = 0;
while (sw_idx != hw_idx && budget > 0) {
struct tg3_rx_buffer_desc *desc = &tp->rx_rcb[sw_idx];
unsigned int len;
struct sk_buff *skb;
dma_addr_t dma_addr;
u32 opaque_key, desc_idx, *post_ptr;
desc_idx = desc->opaque & RXD_OPAQUE_INDEX_MASK;
opaque_key = desc->opaque & RXD_OPAQUE_RING_MASK;
if (opaque_key == RXD_OPAQUE_RING_STD) {
if ((desc->err_vlan & RXD_ERR_MASK) != 0 &&
(desc->err_vlan != RXD_ERR_ODD_NIBBLE_RCVD_MII)) { rx_rcb_ptr++;
sw_idx = rx_rcb_ptr % TG3_RX_RCB_RING_SIZE(tp);
continue;
}
dma_addr = pci_unmap_addr(&tp->rx_std_buffers[desc_idx], mapping);
skb = tp->rx_std_buffers[desc_idx].skb;
if(!skb){
rx_rcb_ptr++;
sw_idx = rx_rcb_ptr % TG3_RX_RCB_RING_SIZE(tp);
continue;
}
skb_size = RX_PKT_BUF_SZ;
} else if (opaque_key == RXD_OPAQUE_RING_JUMBO) {
/* Qinghua: We do not need jumbo packet, just take as error*/
post_ptr = &tp->rx_jumbo_ptr;
tg3_recycle_rx(tp, opaque_key,
desc_idx, *post_ptr);
*post_ptr++;
has_jumbo=1;
rx_rcb_ptr++;
sw_idx = rx_rcb_ptr % TG3_RX_RCB_RING_SIZE(tp);
continue;
}
else {
/* other case, do nothing */
rx_rcb_ptr++;
sw_idx = rx_rcb_ptr % TG3_RX_RCB_RING_SIZE(tp);
continue;
}
/* get the length of packet */
len = ((desc->idx_len & RXD_LEN_MASK) >> RXD_LEN_SHIFT) -
4; /*omit crc */
pci_unmap_single(tp->pdev, dma_addr,
skb_size - tp->rx_offset,
PCI_DMA_FROMDEVICE);
tp->rx_std_buffers[desc_idx].skb=NULL;
skb_put(skb, len);
//skb->protocol = eth_type_trans(skb, tp->dev);
skb_pull(skb,dev->hard_header_len);
/* process checksum */
if ((tp->tg3_flags & TG3_FLAG_RX_CHECKSUMS) &&
(desc->type_flags & RXD_FLAG_TCPUDP_CSUM) &&
(((desc->ip_tcp_csum & RXD_TCPCSUM_MASK)
>> RXD_TCPCSUM_SHIFT) == 0xffff))
skb->ip_summed = CHECKSUM_UNNECESSARY;
else
skb->ip_summed = CHECKSUM_NONE;
/* let Click process it *
#if TG3_VLAN_TAG_USED
if (tp->vlgrp != NULL &&
desc->type_flags & RXD_FLAG_VLAN) {
tg3_vlan_rx(tp, skb,
desc->err_vlan & RXD_VLAN_MASK);
} else
#endif
netif_receive_skb(skb, skb->protocol, 0);
*/
if (received == 0) {
skb_head = skb;
skb_last = skb;
skb_last->next = NULL;
} else {
skb_last->next = skb;
skb->next = NULL;
skb_last = skb;
}
tp->dev->last_rx = jiffies;
received++;
budget--;
rx_rcb_ptr++;
sw_idx = rx_rcb_ptr % TG3_RX_RCB_RING_SIZE(tp);
}
/* ACK the status ring. */
if(tp->rx_rcb_ptr!=rx_rcb_ptr){
tp->rx_rcb_ptr = rx_rcb_ptr;
tw32_rx_mbox(MAILBOX_RCVRET_CON_IDX_0 + TG3_64BIT_REG_LOW,(rx_rcb_ptr % TG3_RX_RCB_RING_SIZE(tp)));
mmiowb();
}
/* Refill RX ring(s). */
if (has_jumbo) {
sw_idx = tp->rx_jumbo_ptr % TG3_RX_JUMBO_RING_SIZE;
tw32_rx_mbox(MAILBOX_RCV_JUMBO_PROD_IDX + TG3_64BIT_REG_LOW,
sw_idx);
mmiowb();
}
*want = received;
spin_unlock(&tp->lock);
return skb_head;
}
int
tg3_rx_refill(struct net_device *dev, struct sk_buff **skbs)//, int rcb_idx)
{
struct tg3 *tp=dev->priv;
struct tg3_rx_buffer_desc *desc;
struct ring_info *map;
dma_addr_t mapping;
u16 sw_idx;
int nfilled = 0, last_filled = -1;
struct sk_buff *skb_list;
u32 rcb_idx,post_idx;
u32 rx_rcb_ptr=tp->rx_rcb_ptr-1;
rmb();
if(skbs == 0){
return (rcb_idx-post_idx)%TG3_RX_RING_SIZE;
}
sw_idx=rx_rcb_ptr%TG3_RX_RCB_RING_SIZE(tp);
desc=&tp->rx_rcb[sw_idx];
post_idx=tp->rx_std_ptr;
rcb_idx=desc->opaque&RXD_OPAQUE_INDEX_MASK;
/* if this is not a std ring index, or the ring is full, return */
if(((desc->opaque&RXD_OPAQUE_RING_MASK)!=RXD_OPAQUE_RING_STD)||((rcb_idx-post_idx)%TG3_RX_RING_SIZE==0)){
return 0;
}
spin_lock(&tp->lock);
map = &tp->rx_std_buffers[post_idx];
skb_list = *skbs;
while(((rcb_idx - post_idx)%TG3_RX_RING_SIZE!=0)&& skb_list){
struct sk_buff *skb = skb_list;
if(map->skb!=NULL) {
post_idx = (post_idx + 1) % TG3_RX_RING_SIZE;
map = &tp->rx_std_buffers[post_idx];
continue;
}
skb_list = skb_list->next;
desc = &tp->rx_std[post_idx];
skb->dev = dev;
mapping = pci_map_single(tp->pdev, skb->data,
RX_PKT_BUF_SZ - tp->rx_offset,
PCI_DMA_FROMDEVICE);
map->skb = skb;
pci_unmap_addr_set(map, mapping, mapping);
desc->addr_hi = ((u64)mapping >> 32);
desc->addr_lo = ((u64)mapping & 0xffffffff);
last_filled = post_idx;
post_idx = (post_idx + 1) % TG3_RX_RING_SIZE;
nfilled++;
map = &tp->rx_std_buffers[post_idx];
}
/* Return the unsed list of skb */
*skbs = skb_list;
/* Refill RX ring(s). */
//dest_idx = &tp->rx_std_ptr % TG3_RX_RING_SIZE;
if(post_idx!=tp->rx_std_ptr){
tp->rx_std_ptr = post_idx ;
tw32_rx_mbox(MAILBOX_RCV_STD_PROD_IDX + TG3_64BIT_REG_LOW,post_idx);
mmiowb();
}
spin_unlock(&tp->lock);
return (rcb_idx-post_idx)%TG3_RX_RING_SIZE;
}
static int
tg3_poll_on(struct net_device *dev)
{
struct tg3 *tp = dev->priv;
unsigned long flags;
if (!dev->polling) {
spin_lock_irqsave(&tp->lock,flags);
dev->polling = 2;
tp->reset = 0;
tg3_disable_ints(tp);
spin_unlock_irqrestore(&tp->lock,flags);
}
//printk("tg3_poll_on\n");
return 0;
}
static int
tg3_poll_off(struct net_device *dev)
{
struct tg3 *tp = dev->priv;
unsigned long flags;
printk("tg3_poll_off:begin\n");
if (dev->polling > 0) {
dev->polling = 0;
spin_lock(&tp->lock);
tg3_restart_ints(tp);
spin_unlock(&tp->lock);
//printk("tg3_poll_off\n");
}
printk("tg3_poll_off:end\n");
return 0;
}
static int
tg3_tx_eob(struct net_device *dev)
{
struct tg3 *tp = dev->priv;
struct tg3_hw_status *sblk = tp->hw_status;
// handle link change and other phy events
if (sblk->status & SD_STATUS_UPDATED) {
spin_lock(&tp->lock);
tw32_mailbox(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW,
0x00000001);
tr32(MAILBOX_INTERRUPT_0 + TG3_64BIT_REG_LOW);
sblk->status &= ~SD_STATUS_UPDATED;
spin_unlock(&tp->lock);
}
if (!(tp->tg3_flags &
(TG3_FLAG_USE_LINKCHG_REG |
TG3_FLAG_POLL_SERDES))) {
if (sblk->status & SD_STATUS_LINK_CHG) {
spin_lock(&tp->lock);
sblk->status = SD_STATUS_UPDATED |
(sblk->status & ~SD_STATUS_LINK_CHG);
tg3_setup_phy(tp, 0);
spin_unlock(&tp->lock);
}
}
spin_lock(&tp->tx_lock);
/* Packets are ready, update Tx producer idx local and on card. */
tw32_tx_mbox((MAILBOX_SNDHOST_PROD_IDX_0 + TG3_64BIT_REG_LOW), tp->tx_prod);
mmiowb();
spin_unlock(&tp->tx_lock);
return 0;
}
static int
tg3_tx_start(struct net_device *dev)
{
//printk("tg3_tx_start\n");
tg3_tx_eob(dev);
return 0;
}
static int
tg3_tx_pqueue(struct net_device *netdev, struct sk_buff *skb)
{
//return tg3_start_xmit(skb,netdev);
struct tg3 *tp = netdev_priv(netdev);
dma_addr_t mapping;
unsigned int i;
u32 len, entry, base_flags, mss;
int would_hit_hwbug;
unsigned long flags;
len = skb_headlen(skb);
/* No BH disabling for tx_lock here. We are running in BH disabled
* context and TX reclaim runs via tp->poll inside of a software
* interrupt. Rejoice!
*
* Actually, things are not so simple. If we are to take a hw
* IRQ here, we can deadlock, consider:
*
* CPU1 CPU2
* tg3_start_xmit
* take tp->tx_lock
* tg3_timer
* take tp->lock
* tg3_interrupt
* spin on tp->lock
* spin on tp->tx_lock
*
* So we really do need to disable interrupts when taking
* tx_lock here.
*/
if (!spin_trylock(&tp->tx_lock)) {
return NETDEV_TX_LOCKED;
}
/* This is a hard error, log it. */
if (unlikely(TX_BUFFS_AVAIL(tp) <= (skb_shinfo(skb)->nr_frags + 1))) {
netif_stop_queue(netdev);
spin_unlock(&tp->tx_lock);
printk(KERN_ERR PFX "%s: BUG! Tx Ring full when queue awake!\n",
netdev->name);
return NETDEV_TX_BUSY;
}
entry = tp->tx_prod;
base_flags = 0;
if (skb->ip_summed == CHECKSUM_HW)
base_flags |= TXD_FLAG_TCPUDP_CSUM;
#if TG3_TSO_SUPPORT != 0
mss = 0;
if (skb->len > (tp->dev->mtu + ETH_HLEN) &&
(mss = skb_shinfo(skb)->tso_size) != 0) {
int tcp_opt_len, ip_tcp_len;
tcp_opt_len = ((skb->h.th->doff - 5) * 4);
ip_tcp_len = (skb->nh.iph->ihl * 4) + sizeof(struct tcphdr);
base_flags |= (TXD_FLAG_CPU_PRE_DMA |
TXD_FLAG_CPU_POST_DMA);
skb->nh.iph->check = 0;
skb->nh.iph->tot_len = ntohs(mss + ip_tcp_len + tcp_opt_len);
if (tp->tg3_flags2 & TG3_FLG2_HW_TSO) {
skb->h.th->check = 0;
base_flags &= ~TXD_FLAG_TCPUDP_CSUM;
}
else {
skb->h.th->check =
~csum_tcpudp_magic(skb->nh.iph->saddr,
skb->nh.iph->daddr,
0, IPPROTO_TCP, 0);
}
if ((tp->tg3_flags2 & TG3_FLG2_HW_TSO) ||
(GET_ASIC_REV(tp->pci_chip_rev_id) == ASIC_REV_5705)) {
if (tcp_opt_len || skb->nh.iph->ihl > 5) {
int tsflags;
tsflags = ((skb->nh.iph->ihl - 5) +
(tcp_opt_len >> 2));
mss |= (tsflags << 11);
}
} else {
if (tcp_opt_len || skb->nh.iph->ihl > 5) {
int tsflags;
tsflags = ((skb->nh.iph->ihl - 5) +
(tcp_opt_len >> 2));
base_flags |= tsflags << 12;
}
}
}
#else
mss = 0;
#endif
#if TG3_VLAN_TAG_USED
if (tp->vlgrp != NULL && vlan_tx_tag_present(skb))
base_flags |= (TXD_FLAG_VLAN |
(vlan_tx_tag_get(skb) << 16));
#endif
/* Queue skb data, a.k.a. the main skb fragment. */
mapping = pci_map_single(tp->pdev, skb->data, len, PCI_DMA_TODEVICE);
tp->tx_buffers[entry].skb = skb;
pci_unmap_addr_set(&tp->tx_buffers[entry], mapping, mapping);
would_hit_hwbug = 0;
if (tg3_4g_overflow_test(mapping, len))
would_hit_hwbug = entry + 1;
tg3_set_txd(tp, entry, mapping, len, base_flags,
(skb_shinfo(skb)->nr_frags == 0) | (mss << 1));
entry = NEXT_TX(entry);
/* Now loop through additional data fragments, and queue them. */
if (skb_shinfo(skb)->nr_frags > 0) {
unsigned int i, last;
last = skb_shinfo(skb)->nr_frags - 1;
for (i = 0; i <= last; i++) {
skb_frag_t *frag = &skb_shinfo(skb)->frags[i];
len = frag->size;
mapping = pci_map_page(tp->pdev,
frag->page,
frag->page_offset,
len, PCI_DMA_TODEVICE);
tp->tx_buffers[entry].skb = NULL;
pci_unmap_addr_set(&tp->tx_buffers[entry], mapping, mapping);
if (tg3_4g_overflow_test(mapping, len)) {
/* Only one should match. */
if (would_hit_hwbug)
BUG();
would_hit_hwbug = entry + 1;
}
if (tp->tg3_flags2 & TG3_FLG2_HW_TSO)
tg3_set_txd(tp, entry, mapping, len,
base_flags, (i == last)|(mss << 1));
else
tg3_set_txd(tp, entry, mapping, len,
base_flags, (i == last));
entry = NEXT_TX(entry);
}
}
if (would_hit_hwbug) {
u32 last_plus_one = entry;
u32 start;
unsigned int len = 0;
would_hit_hwbug -= 1;
entry = entry - 1 - skb_shinfo(skb)->nr_frags;
entry &= (TG3_TX_RING_SIZE - 1);
start = entry;
i = 0;
while (entry != last_plus_one) {
if (i == 0)
len = skb_headlen(skb);
else
len = skb_shinfo(skb)->frags[i-1].size;
if (entry == would_hit_hwbug)
break;
i++;
entry = NEXT_TX(entry);
}
/* If the workaround fails due to memory/mapping
* failure, silently drop this packet.
*/
if (tigon3_4gb_hwbug_workaround(tp, skb,
entry, len,
last_plus_one,
&start, mss))
goto queue_out;
entry = start;
}
tp->tx_prod = entry;
if (TX_BUFFS_AVAIL(tp) <= (MAX_SKB_FRAGS + 1))
netif_stop_queue(netdev);
queue_out:
spin_unlock(&tp->tx_lock);
netdev->trans_start = jiffies;
return NETDEV_TX_OK;
}
static struct sk_buff *
tg3_tx_poll(struct tg3 *tp)
{
u32 hw_idx = tp->hw_status->idx[0].tx_consumer;
u32 sw_idx = tp->tx_cons;
struct sk_buff *skb_head,*skb_last;
skb_head=skb_last=0;
while (sw_idx != hw_idx) {
struct tx_ring_info *ri = &tp->tx_buffers[sw_idx];
struct sk_buff *skb = ri->skb;
int i;
if (unlikely(skb == NULL))
BUG();
pci_unmap_single(tp->pdev,
pci_unmap_addr(ri, mapping),
skb_headlen(skb),
PCI_DMA_TODEVICE);
ri->skb = NULL;
sw_idx = NEXT_TX(sw_idx);
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
if (unlikely(sw_idx == hw_idx))
BUG();
ri = &tp->tx_buffers[sw_idx];
if (unlikely(ri->skb != NULL))
BUG();
pci_unmap_page(tp->pdev,
pci_unmap_addr(ri, mapping),
skb_shinfo(skb)->frags[i].size,
PCI_DMA_TODEVICE);
sw_idx = NEXT_TX(sw_idx);
}
if (skb_head == 0) {
skb_head = skb;
skb_last = skb;
skb_last->next = NULL;
} else {
skb_last->next = skb;
skb->next = NULL;
skb_last = skb;
}
// dev_kfree_skb_irq(skb);
}
tp->tx_cons = sw_idx;
if (netif_queue_stopped(tp->dev) &&
(TX_BUFFS_AVAIL(tp) > TG3_TX_WAKEUP_THRESH))
netif_wake_queue(tp->dev);
return skb_head;
}
static struct sk_buff *
tg3_tx_clean(struct net_device *netdev)
{
struct tg3 *tp = netdev_priv(netdev);
struct sk_buff *skb_head,*skb_last;
u32 hw_idx = tp->hw_status->idx[0].tx_consumer;
u32 sw_idx = tp->tx_cons;
skb_head=skb_last=0;
rmb();
spin_lock(&tp->lock);
if(tp->hw_status->idx[0].tx_consumer == tp->tx_cons){
spin_unlock(&tp->lock);
return NULL;
}
spin_lock(&tp->tx_lock);
// skb_head = tg3_tx_poll(tp);
while (sw_idx != hw_idx) {
struct tx_ring_info *ri = &tp->tx_buffers[sw_idx];
struct sk_buff *skb = ri->skb;
int i;
if (unlikely(skb == NULL))
BUG();
pci_unmap_single(tp->pdev,
pci_unmap_addr(ri, mapping),
skb_headlen(skb),
PCI_DMA_TODEVICE);
ri->skb = NULL;
sw_idx = NEXT_TX(sw_idx);
for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
if (unlikely(sw_idx == hw_idx))
BUG();
ri = &tp->tx_buffers[sw_idx];
if (unlikely(ri->skb != NULL))
BUG();
pci_unmap_page(tp->pdev,
pci_unmap_addr(ri, mapping),
skb_shinfo(skb)->frags[i].size,
PCI_DMA_TODEVICE);
sw_idx = NEXT_TX(sw_idx);
}
if (skb_head == 0) {
skb_head = skb;
skb_last = skb;
skb_last->next = NULL;
} else {
skb_last->next = skb;
skb->next = NULL;
skb_last = skb;
}
// dev_kfree_skb_irq(skb);
}
tp->tx_cons = sw_idx;
if (netif_queue_stopped(tp->dev) &&
(TX_BUFFS_AVAIL(tp) > TG3_TX_WAKEUP_THRESH))
netif_wake_queue(tp->dev);
//return skb_head;
/* the end of tx_poll function */
spin_unlock(&tp->tx_lock);
spin_unlock(&tp->lock);
return skb_head;
}
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: David S. Miller @ 2005-07-05 21:26 UTC (permalink / raw)
To: dada1; +Cc: netdev
In-Reply-To: <42CA390C.9000801@cosmosbay.com>
Eric, I've told you this before many times. Please do something
so that your email client does not corrupt the patches. Once again,
your email client turned all the tab characters into spaces and
thus made the patch unusable.
Even though you used an attachment, the tab-->space transformation
still happened somehow.
Please fix this, and make a serious mental note to prevent this
somehow in the future, thanks a lot.
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: David S. Miller @ 2005-07-05 21:22 UTC (permalink / raw)
To: tgraf; +Cc: dada1, netdev
In-Reply-To: <20050705173411.GK16076@postel.suug.ch>
From: Thomas Graf <tgraf@suug.ch>
Date: Tue, 5 Jul 2005 19:34:11 +0200
> Do as you wish, I don't feel like argueing about micro optimizations.
I bet the performance gain really comes from the mispredicted
branches in the loop.
For loops of fixed duration, say, 5 or 6 iterations or less, it
totally defeats the branch prediction logic in most processors.
By the time the chip moves the I-cache branch state to "likely"
the loop has ended and we eat a mispredict.
I think the original patch is OK, hand unrolling the loop in
the C code. Adding -funroll-loops to the CFLAGS has lots of
implications, and in particular the embedded folks might not
be happy with some things that result from that.
So I'll apply the original unrolling patch for now.
^ permalink raw reply
* Msi-X on Opterons?
From: Leonid Grossman @ 2005-07-05 18:31 UTC (permalink / raw)
To: netdev; +Cc: netdev, 'Raghavendra Koushik'
Did anyone had any luck getting MSI-X working on Opteron platforms?
The newer 8132-based systems support MSI-X, but we could not get it working
with our card there.
It works fine on Xeon systems, I wonder if the implementation is
Xeon-centric...
^ permalink raw reply
* Re: [PATCH] loop unrolling in net/sched/sch_generic.c
From: Thomas Graf @ 2005-07-05 17:34 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David S. Miller, netdev
In-Reply-To: <42CAAE2F.5070807@cosmosbay.com>
* Eric Dumazet <42CAAE2F.5070807@cosmosbay.com> 2005-07-05 17:58
> I'm using vanilla 2.6.12 : no -funroll-loop in it. Maybe in your tree, not
> on 99.9% of 2.6.12 trees.
>
> Are you suggesting everybody should use this compiler flag ?
> I dont know about loopback optimization, I am not involved with this stuff,
> maybe you think I'm another guy ?
>
> It seems to me you give unrelated arguments.
Do as you wish, I don't feel like argueing about micro optimizations.
^ permalink raw reply
* Re: [TG3]: About hw coalescing infrastructure.
From: Eric Dumazet @ 2005-07-05 16:14 UTC (permalink / raw)
To: David S. Miller; +Cc: mchan, netdev
In-Reply-To: <20050704.160034.55511095.davem@davemloft.net>
David S. Miller a écrit :
> SMP system? What platform and 570x chip revision?
>
dual opterons 248,
Ethernet controller: Broadcom Corporation NetXtreme BCM5702 Gigabit Ethernet (rev 02)
> Does the stock driver in 2.6.12 hang as well?
Stock driver in 2.6.11 or 2.6.12 cause hangs (crashes ?) too, but no kernel mesages are logged on disk.
I think this is crashing in cache_grow() (slab code), not sure because I dont have access to the console.
The 2.6.13-rc1 tg3 driver seems to survive (running for 18 hours now), and the cpu used by network activity
was cut down :)
rx-usecs: 500
rx-frames: 30
rx-usecs-irq: 500
rx-frames-irq: 20
tx-usecs: 490
tx-frames: 53
tx-usecs-irq: 490
tx-frames-irq: 5
About 1200 IRQ/second now, with 30000 recv packs/s and 25000 xmit pack/s
Thank you
Eric
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox