* Re: [PATCH] tcp: fix ICMP-RTO war
From: Ilpo Järvinen @ 2010-01-27 12:36 UTC (permalink / raw)
To: Damian Lukowski
Cc: Denys Fedoryshchenko, Netdev, David Miller, Alexey Kuznetsov
In-Reply-To: <4B5D8ABB.8030906@tvk.rwth-aachen.de>
On Mon, 25 Jan 2010, Damian Lukowski wrote:
> considering Denys' latest tests, I think we should bound
> at TCP_RTO_MIN inside __tcp_set_rto().
> Look at the following piece:
> > [ 604.193389] rto: 200 (0 >> 3 + 0, 32) time: 304193 sent: 304091 pen: 1 307291 rem: 98
> > [ 604.193518] lower bound violation: 0 code 1 sk_state 1
> > [ 604.193589] rto: 200 (0 >> 3 + 0, 31) time: 304193 sent: 304091 pen: 1 304291 rem: 98
> > [ 604.193706] lower bound violation: 0 code 1 sk_state 1
> > [ 604.193776] rto: 200 (0 >> 3 + 0, 30) time: 304193 sent: 304091 pen: 1 304291 rem: 98
> > [ 607.341327] lower bound violation: 0 code 1 sk_state 1
> > [ 607.341412] rto: 200 (0 >> 3 + 0, 33) time: 307341 sent: 307091 pen: 1 310291 rem: 0
>
> We have a burst of three incoming ICMPs, not triggering retransmissions because
> of rem > 0. Nevertheless, there is an increase of icsk_backoff by four
> within 3100ms, with no ICMPs in between.
> For me, this is explainable by the broken mdev/rtt issue together with
> bursty ICMP replies.
Unless they are for a different connection? We might have to print sk (%p)
in all those printouts to be sure which maps to which. If a peer becomes
unreachable, it may well have multiple connections open (this was a
proxy, iirc?).
--
i.
^ permalink raw reply
* Re: [PATCH] tcp: fix ICMP-RTO war
From: Ilpo Järvinen @ 2010-01-27 12:41 UTC (permalink / raw)
To: David Miller; +Cc: damian, denys, Netdev, Alexey Kuznetsov
In-Reply-To: <20100125.234557.00486980.davem@davemloft.net>
On Mon, 25 Jan 2010, David Miller wrote:
> From: Damian Lukowski <damian@tvk.rwth-aachen.de>
> Date: Mon, 25 Jan 2010 16:07:43 +0100
>
> > @@ -530,7 +530,11 @@ static inline void tcp_bound_rto(const struct sock *sk)
> >
> > static inline u32 __tcp_set_rto(const struct tcp_sock *tp)
> > {
> > - return (tp->srtt >> 3) + tp->rttvar;
> > + u32 rto = (tp->srtt >> 3) + tp->rttvar;
> > + if (unlikely(rto < TCP_RTO_MIN))
> > + return TCP_RTO_MIN;
> > + else
> > + return rto;
> > }
>
> The min RTO is now a runtime variable, TCP_RTO_MIN is merely the
> default, so we should use tcp_rto_min() for obtaining that value.
>
> And if we make this change, we might want to delete the comment in
> tcp_set_rto() which claims:
>
> /* NOTE: clamping at TCP_RTO_MIN is not required, current algo
> * guarantees that rto is higher.
> */
> tcp_bound_rto(sk);
>
> And we have shown here at least one case where that is not true.
> :-)
I went through some history, it seems that this comment about the lower
bound originates from Alexey [1]:
commit 893e1302654a9bcdc0e7d9ac95159657a8f5e0e8
Author: davem <davem>
Date: Wed Dec 13 04:10:12 2000 +0000
Fix numerous RTO bugs.
Withdraw CWR when DSACKs are seen.
Do not allow recvmsg on unbound socket, it is
senseless.
All from Alexey.
> I've looked at Denys's traces and your analysis, and I still
> can't figure out who the true culprit is that lets us get into
> such a state that RTO is evaluated so low...
--
i.
[1] http://git.kernel.org/?p=linux/kernel/git/davem/netdev-vger-cvs.git;a=commit;h=893e1302654a9bcdc0e7d9ac95159657a8f5e0e8
^ permalink raw reply
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: Jarek Poplawski @ 2010-01-27 12:43 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127.042336.52877358.davem@davemloft.net>
On Wed, Jan 27, 2010 at 04:23:36AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Wed, 27 Jan 2010 12:17:46 +0000
>
> > If there is nothing better I still think this patch is right, except
> > better subject (Fix impossible NULL dereference in ... ;-).
>
> Sorry, I'm not applying it, just like I didn't apply Krishna's similar
> patch he posted earlier today.
I've understood your decision and I'm OK with that; I'm only not sure
there is a better way (comments?) to make this code readable.
Jarek P.
^ permalink raw reply
* Re: CBQ broken in 2.6
From: Anton Ivanov @ 2010-01-27 12:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100127.035635.173712880.davem@davemloft.net>
Thanks it is indeed sch_cbq.c,
I have been going through the code in the meantime and found a place
where cl->tparent can be modified further down in the routine in
question. There is an invocation of cbq_set_lss() around the end of
cbq_change_class() which can do that.
I am rebuilding the kernel for my CBQ box with a few printks at the
moment to see if it modified there or not.
In any case here is the tell-tale symptom:
class cbq 1:16 parent 1: leaf 76: rate 5600Kbit (bounded,isolated) prio
2
Sent 162051 bytes 925 pkt (dropped 0, overlimits 0 requeues 0)
rate 0bit 0pps backlog 0b 0p requeues 0
borrowed 201 overactions 0 avgidle 78 undertime 0
That is a bounded class. Its borrowed should be always 0 no matter what.
That is basically a broken CBQ implementation.
An elementary network test suite shows the same result - it is being
allowed to borrow.
I am happy to send the whole config if necessary if someone wants to
look at it.
Brgds,
[snip]
--
Understanding is a three-edged sword:
your side, their side, and the truth. --Kosh Naranek
A. R. Ivanov
E-mail: anton.ivanov@kot-begemot.co.uk
WWW: http://www.kot-begemot.co.uk/
^ permalink raw reply
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: David Miller @ 2010-01-27 13:14 UTC (permalink / raw)
To: jarkao2; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127124331.GB15611@ff.dom.local>
From: Jarek Poplawski <jarkao2@gmail.com>
Date: Wed, 27 Jan 2010 12:43:31 +0000
> I'm only not sure there is a better way (comments?) to make this
> code readable.
You can make it more readable and even validate the invariant at the
same time by adding some kind of inline function if you wish.
static inline struct dst_entry *rtable_to_dst_entry(struct rtable *rt)
{
BUILD_BUG_ON(rt != &rt->u.dst);
/* Because "->u.dst" is the first member of struct rtable,
* this dereference is safe even when rt is NULL.
*/
return &rt->u.dst;
}
Something like that.
^ permalink raw reply
* Re: [PATCH] l2tp: Fix a UDP socket reference count bug in the pppol2tp driver
From: James Chapman @ 2010-01-27 13:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20100123.015511.83860202.davem@davemloft.net>
David Miller wrote:
> From: James Chapman <jchapman@katalix.com>
> Date: Thu, 21 Jan 2010 16:10:09 +0000
>
>> The bug can cause a kernel stack trace when a tunnel socket is closed.
>>
>> WARNING: at include/net/sock.h:435 udp_lib_unhash+0x117/0x120()
>> Pid: 1086, comm: openl2tpd Not tainted 2.6.33-rc1 #8
>> Call Trace:
>
> This fix doesn't look right at all.
>
> You grab one reference in connect() and then drop a reference
> every single recvmsg() call.
No, one ref is grabbed when the UDP socket is prepared for L2TP. Another
ref is grabbed while processing a skb in the receive path.
> recvmsg() calls to connect() would be many to one, so I can't
> see how this reference counting scheme could possibly work.
Perhaps you missed the sock_hold() in pppol2tp_sock_to_tunnel(), which
is called for every received skb in pppol2tp_recv_core()?
When userspace closes all session sockets in the tunnel, including the
special tunnel pppol2tp socket which has session_id==0, the ref on the
UDP tunnel socket is dropped, which allows it to be released.
> Why don't you describe the exact sequence of events that lead
> to the trace, so we can figure out how to correct this
> properly?
A way to reproduce the issue is to prepare the UDP socket for L2TP (by
opening a tunnel pppol2tp socket) and then close it before any L2TP
sessions are added to it. The sequence is
Create UDP socket
Create tunnel pppol2tp socket to prepare UDP socket for L2TP
pppol2tp_connect: session_id=0, peer_session_id=0
L2TP SCCRP control frame received (tunnel_id==0)
pppol2tp_recv_core: sock_hold()
pppol2tp_recv_core: sock_put
L2TP ZLB control frame received (tunnel_id=nnn)
pppol2tp_recv_core: sock_hold()
pppol2tp_recv_core: sock_put
Close tunnel management socket
pppol2tp_release: session_id=0, peer_session_id=0
Close UDP socket
udp_lib_close: BUG
The addition of sock_hold() in pppol2tp_connect() solves the problem.
For data frames, two sock_put() calls were added to plug a refcnt leak
per received data frame. The ref that is grabbed at the top of
pppol2tp_recv_core() must always be released, but this wasn't done for
accepted data frames or data frames discarded because of bad UDP
checksums. This leak meant that any UDP socket that had passed L2TP data
traffic (i.e. L2TP data frames, not just L2TP control frames) using
pppol2tp would not be released by the kernel.
Does the above help?
--
James Chapman
Katalix Systems Ltd
http://www.katalix.com
Catalysts for your Embedded Linux software development
^ permalink raw reply
* Re: Network QoS support in applications
From: Benny Amorsen @ 2010-01-27 13:24 UTC (permalink / raw)
To: Dunc; +Cc: Kalle Valo, David Miller, kaber, netdev, linux-wireless
In-Reply-To: <4B5EF5DF.2070005@lemonia.org>
Dunc <dunc@lemonia.org> writes:
> If applications set the QoS values, the who's to stop someone (for
> example) writing a bittorrent client that marks all packets for the
> highest priority as if they were VoIP or something?
Nothing, but nothing stops them from writing a bittorrent client which
does "optimistic ACK" either. Yet noone seems to bother.
Also, bittorrent marked as EF could easily get hit by a "don't queue
voice packets, just drop instead" policy. Late voice packets are useless
and might as well be dropped, but TCP streams like bittorrent react
badly to packet loss.
All in all there is little incentive for people to game the system.
Should it happen anyway, network administrators have plentiful tools for
fixing it, up to and including ip link set dev whatever down.
/Benny
^ permalink raw reply
* Re: [PATCH] ipv6: Fix possible NULL dereference in icmp6_dst_alloc
From: Jarek Poplawski @ 2010-01-27 13:28 UTC (permalink / raw)
To: David Miller; +Cc: shemminger, djohnson, sakkiped, netdev
In-Reply-To: <20100127.051410.74328462.davem@davemloft.net>
On Wed, Jan 27, 2010 at 05:14:10AM -0800, David Miller wrote:
> From: Jarek Poplawski <jarkao2@gmail.com>
> Date: Wed, 27 Jan 2010 12:43:31 +0000
>
> > I'm only not sure there is a better way (comments?) to make this
> > code readable.
>
> You can make it more readable and even validate the invariant at the
> same time by adding some kind of inline function if you wish.
>
> static inline struct dst_entry *rtable_to_dst_entry(struct rtable *rt)
> {
> BUILD_BUG_ON(rt != &rt->u.dst);
>
> /* Because "->u.dst" is the first member of struct rtable,
> * this dereference is safe even when rt is NULL.
> */
> return &rt->u.dst;
> }
>
> Something like that.
Yes, it looks the most readable to me.
Thanks,
Jarek P.
^ permalink raw reply
* Re: [PATCH 2/4] ar7: implement clock API
From: Ralf Baechle @ 2010-01-27 13:54 UTC (permalink / raw)
To: Florian Fainelli; +Cc: linux-mips, Wim Van Sebroeck, netdev, David Miller
In-Reply-To: <201001270910.06224.florian@openwrt.org>
On Wed, Jan 27, 2010 at 09:10:06AM +0100, Florian Fainelli wrote:
> This patch makes the ar7 clock code implement the
> Linux clk API. Drivers using the various clocks
> available in the SoC are updated accordingly.
>
> Signed-off-by: Florian Fainelli <florian@openwrt.org>
> Acked-by: Wim Van Sebroeck <wim@iguana.be>
Thanks; queued for 2.6.34.
Ralf
^ permalink raw reply
* Re: [PATCH] tcp: fix ICMP-RTO war
From: Denys Fedoryshchenko @ 2010-01-27 13:56 UTC (permalink / raw)
To: Ilpo Järvinen, Netdev, Damian Lukowski, David Miller
On Wednesday 27 January 2010 14:36:18 you wrote:
> Unless they are for a different connection? We might have to print sk (%p)
> in all those printouts to be sure which maps to which. If a peer becomes
> unreachable, it may well have multiple connections open (this was a
> proxy, iirc?).
>
Ok i will try to do that today.
Most probably different connections, on this proxy i have 10-15k established
connections at peak time.
Also about estimating rtt, maybe there is something wrong in idea. Just what
will happen if ip visible for proxy have multiple people behind? It can be
small router for hotspot and 10-20 people behind it, some with very large
rtt, some with very small.
And some of them getting disconnected (out of range from wireless, for
example), and thats most probably why host unreachable sent... at same time
proxy have established and running tcp connections with other people on same
router (means for proxy same ip).
Sorry, again to Ilpo Järvinen, by default i have reply, instead of reply all.
^ permalink raw reply
* [PATCH] sctp: IPsec rules are ineffective with ipv6
From: Nicolas Dichtel @ 2010-01-27 14:12 UTC (permalink / raw)
To: netdev, Vlad Yasevich, linux-sctp
[-- Attachment #1: Type: text/plain, Size: 165 bytes --]
xfrm_lookup() is missing in sctp_v6_xmit(), add it.
Signed-off-by: Junwei Zhang <junwei.zhang@6wind.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
[-- Attachment #2: x.diff --]
[-- Type: text/x-diff, Size: 708 bytes --]
diff --git a/net/sctp/ipv6.c b/net/sctp/ipv6.c
index cc50fbe..f24e23c 100644
--- a/net/sctp/ipv6.c
+++ b/net/sctp/ipv6.c
@@ -197,8 +197,10 @@ out:
static int sctp_v6_xmit(struct sk_buff *skb, struct sctp_transport *transport)
{
struct sock *sk = skb->sk;
+ struct dst_entry *dst = skb_dst(skb);
struct ipv6_pinfo *np = inet6_sk(sk);
struct flowi fl;
+ int err;
memset(&fl, 0, sizeof(fl));
@@ -231,6 +233,9 @@ static int sctp_v6_xmit(struct sk_buff *skb, struct sctp_transport *transport)
if (!(transport->param_flags & SPP_PMTUD_ENABLE))
skb->local_df = 1;
+ if ((err = xfrm_lookup(sock_net(sk), &dst, &fl, sk, 0)) < 0)
+ return err;
+
return ip6_xmit(sk, skb, &fl, np->opt, 0);
}
^ permalink raw reply related
* occasional netdev watchdog timeouts with sungem on Apple G5
From: Mikael Pettersson @ 2010-01-27 14:21 UTC (permalink / raw)
To: netdev; +Cc: linux-kernel
For a few months now I've been getting very occasional netdev
watchdog timeouts on the built-in sungem ethernet in my G5. I
can't say exactly when it started, but probably post-2.6.30.
The kernel always recovers so it's not a big problem, but I find it
a bit alarming since I never see this with any other ethernet driver.
dmesg from 2.6.33-rc4:
sungem.c:v0.98 8/24/03 David S. Miller (davem@redhat.com)
PHY ID: 2062e0, addr: 1
eth0: Sun GEM (PCI) 10/100/1000BaseT Ethernet 00:0a:95:aa:66:50
eth0: Found BCM5421-K2 PHY
...
eth0: Link is up at 100 Mbps, full-duplex.
...
eth0: Pause is enabled (rxfifo: 10240 off: 7168 on: 5632)
...
NETDEV WATCHDOG: eth0 (gem): transmit queue 0 timed out
------------[ cut here ]------------
Badness at net/sched/sch_generic.c:255
NIP: c0000000002812ac LR: c0000000002812a8 CTR: 0000000000000001
REGS: c0000001f3513750 TRAP: 0700 Not tainted (2.6.33-rc4)
MSR: 9000000000029032 <EE,ME,CE,IR,DR> CR: 28000484 XER: 20000000
TASK = c0000001f6390000[2856] 'sshd' THREAD: c0000001f3510000 CPU: 1
GPR00: c0000000002812a8 c0000001f35139d0 c000000000457ed0 000000000000003e
GPR04: 0000000000000000 ffffffffffffffff 0000000000000000 000000000001ffff
GPR08: c0000000004788b8 c0000000003c8f08 c0000000004788b8 0000000008bb9000
GPR12: 0000000028000482 c000000000474680 0000000020103670 00000000806595c4
GPR16: 00000000631b65ac 0000000000000100 c00000000044a740 c00000000045d300
GPR20: c0000001f60d5020 c0000001f60d5420 c0000001f60d5820 ffffffffffffffff
GPR24: 0000000000000001 0000000000000000 0000000000000000 0000000000000002
GPR28: 0000000000000001 0000000000000000 c000000000412258 c0000001f4750000
NIP [c0000000002812ac] .dev_watchdog+0x1d0/0x310
LR [c0000000002812a8] .dev_watchdog+0x1cc/0x310
Call Trace:
[c0000001f35139d0] [c0000000002812a8] .dev_watchdog+0x1cc/0x310 (unreliable)
[c0000001f3513ad0] [c000000000054890] .run_timer_softirq+0x1c4/0x2a0
[c0000001f3513bd0] [c00000000004e870] .__do_softirq+0xf0/0x1d0
[c0000001f3513ca0] [c00000000000bbc0] .do_softirq+0x50/0x94
[c0000001f3513d20] [c00000000004e5c0] .irq_exit+0x54/0xb0
[c0000001f3513da0] [c00000000001be6c] .timer_interrupt+0xb4/0xe0
[c0000001f3513e30] [c000000000003610] decrementer_common+0x110/0x180
Instruction dump:
41fe0040 38810070 7fe3fb78 38a00040 4bfe96b1 60000000 7fa6eb78 7fe4fb78
7c651b78 e87e8028 48054ee5 60000000 <0fe00000> e93e8020 38000001 98090000
eth0: transmit timed out, resetting
eth0: TX_STATE[003ffc05:00000001:0000001f]
eth0: RX_STATE[0100c805:00000001:00000021]
eth0: Link is up at 1000 Mbps, full-duplex.
eth0: Pause is enabled (rxfifo: 10240 off: 7168 on: 5632)
eth0: transmit timed out, resetting
eth0: TX_STATE[00000001:00000000:00000001]
eth0: RX_STATE[00064801:00000000:00000001]
eth0: Link is up at 100 Mbps, full-duplex.
eth0: Pause is enabled (rxfifo: 10240 off: 7168 on: 5632)
eth0: transmit timed out, resetting
eth0: TX_STATE[003ffc05:00000001:0000001f]
eth0: RX_STATE[0100c805:00000001:00000021]
eth0: Link is up at 100 Mbps, full-duplex.
eth0: Pause is enabled (rxfifo: 10240 off: 7168 on: 5632)
lspci -v:
0000:f0:0b.0 Host bridge: Apple Computer Inc. U3 AGP
Flags: bus master, 66MHz, medium devsel, latency 16
Capabilities: <access denied>
0000:f0:10.0 VGA compatible controller: nVidia Corporation NV34 [GeForce FX 5200 Ultra] (rev a1) (prog-if 00 [VGA controller])
Subsystem: nVidia Corporation Unknown device 0010
Flags: bus master, 66MHz, medium devsel, latency 16, IRQ 48
Memory at a1000000 (32-bit, non-prefetchable) [size=16M]
Memory at a8000000 (32-bit, prefetchable) [size=128M]
Expansion ROM at a0000000 [disabled] [size=128K]
Capabilities: <access denied>
0001:00:00.0 Host bridge: Apple Computer Inc. Unknown device 004a
Flags: bus master, fast devsel, latency 0
Capabilities: <access denied>
0001:00:01.0 PCI bridge: Advanced Micro Devices [AMD] AMD-8131 PCI-X Bridge (rev 12) (prog-if 00 [Normal decode])
Flags: bus master, 66MHz, medium devsel, latency 32
Bus: primary=00, secondary=06, subordinate=06, sec-latency=32
Memory behind bridge: 90000000-900fffff
Capabilities: <access denied>
0001:00:02.0 PCI bridge: Advanced Micro Devices [AMD] AMD-8131 PCI-X Bridge (rev 12) (prog-if 00 [Normal decode])
Flags: bus master, 66MHz, medium devsel, latency 32
Bus: primary=00, secondary=07, subordinate=07, sec-latency=32
Capabilities: <access denied>
0001:00:03.0 PCI bridge: Apple Computer Inc. K2 HT-PCI Bridge (prog-if 00 [Normal decode])
Flags: bus master, fast devsel, latency 0
Bus: primary=00, secondary=01, subordinate=01, sec-latency=32
Memory behind bridge: 80000000-800fffff
Prefetchable memory behind bridge: 00000000-000fffff
Capabilities: <access denied>
0001:00:04.0 PCI bridge: Apple Computer Inc. K2 HT-PCI Bridge (prog-if 00 [Normal decode])
Flags: bus master, fast devsel, latency 0
Bus: primary=00, secondary=02, subordinate=02, sec-latency=32
Memory behind bridge: 80100000-801fffff
Prefetchable memory behind bridge: 00000000-000fffff
Capabilities: <access denied>
0001:00:05.0 PCI bridge: Apple Computer Inc. K2 HT-PCI Bridge (prog-if 00 [Normal decode])
Flags: bus master, fast devsel, latency 0
Bus: primary=00, secondary=03, subordinate=03, sec-latency=32
Memory behind bridge: 80200000-802fffff
Prefetchable memory behind bridge: 00000000-000fffff
Capabilities: <access denied>
0001:00:06.0 PCI bridge: Apple Computer Inc. K2 HT-PCI Bridge (prog-if 00 [Normal decode])
Flags: bus master, fast devsel, latency 0
Bus: primary=00, secondary=04, subordinate=04, sec-latency=32
Memory behind bridge: 80300000-805fffff
Prefetchable memory behind bridge: 00000000-000fffff
Capabilities: <access denied>
0001:00:07.0 PCI bridge: Apple Computer Inc. K2 HT-PCI Bridge (prog-if 00 [Normal decode])
Flags: bus master, fast devsel, latency 0
Bus: primary=00, secondary=05, subordinate=05, sec-latency=32
Memory behind bridge: 80600000-806fffff
Prefetchable memory behind bridge: 00000000-000fffff
Capabilities: <access denied>
0001:01:07.0 Class ff00: Apple Computer Inc. K2 KeyLargo Mac/IO (rev 20)
Flags: bus master, medium devsel, latency 16
Memory at 80000000 (32-bit, non-prefetchable) [size=512K]
0001:01:08.0 USB Controller: Apple Computer Inc. K2 KeyLargo USB (prog-if 10 [OHCI])
Flags: bus master, medium devsel, latency 16, IRQ 27
Memory at 80081000 (32-bit, non-prefetchable) [size=4K]
0001:01:09.0 USB Controller: Apple Computer Inc. K2 KeyLargo USB (prog-if 10 [OHCI])
Flags: bus master, medium devsel, latency 16, IRQ 28
Memory at 80080000 (32-bit, non-prefetchable) [size=4K]
0001:02:0b.0 USB Controller: NEC Corporation USB (rev 43) (prog-if 10 [OHCI])
Subsystem: NEC Corporation Hama USB 2.0 CardBus
Flags: bus master, medium devsel, latency 16, IRQ 63
Memory at 80102000 (32-bit, non-prefetchable) [size=4K]
Capabilities: <access denied>
0001:02:0b.1 USB Controller: NEC Corporation USB (rev 43) (prog-if 10 [OHCI])
Subsystem: NEC Corporation Hama USB 2.0 CardBus
Flags: bus master, medium devsel, latency 16, IRQ 63
Memory at 80101000 (32-bit, non-prefetchable) [size=4K]
Capabilities: <access denied>
0001:02:0b.2 USB Controller: NEC Corporation USB 2.0 (rev 04) (prog-if 20 [EHCI])
Subsystem: NEC Corporation USB 2.0
Flags: bus master, medium devsel, latency 16, IRQ 63
Memory at 80100000 (32-bit, non-prefetchable) [size=256]
Capabilities: <access denied>
0001:03:0d.0 Class ff00: Apple Computer Inc. K2 ATA/100
Flags: bus master, medium devsel, latency 32, IRQ 39
Memory at 80204000 (32-bit, non-prefetchable) [size=16K]
0001:03:0e.0 FireWire (IEEE 1394): Apple Computer Inc. K2 FireWire (prog-if 10 [OHCI])
Subsystem: Apple Computer Inc. Unknown device 5811
Flags: bus master, medium devsel, latency 248, IRQ 40
Memory at 80200000 (32-bit, non-prefetchable) [size=4K]
Capabilities: <access denied>
0001:04:0f.0 Ethernet controller: Apple Computer Inc. K2 GMAC (Sun GEM)
Flags: bus master, 66MHz, slow devsel, latency 16, IRQ 41
Memory at 80400000 (32-bit, non-prefetchable) [size=2M]
Expansion ROM at 80300000 [disabled] [size=1M]
0001:05:0c.0 IDE interface: Broadcom K2 SATA (prog-if 8f [Master SecP SecO PriP PriO])
Subsystem: Broadcom K2 SATA
Flags: bus master, medium devsel, latency 16, IRQ 16
I/O ports at <unassigned> [disabled]
I/O ports at <unassigned> [disabled]
I/O ports at <unassigned> [disabled]
I/O ports at <unassigned> [disabled]
I/O ports at <unassigned> [disabled]
Memory at 80600000 (32-bit, non-prefetchable) [size=8K]
0001:06:03.0 Ethernet controller: Broadcom Corporation NetXtreme BCM5701 Gigabit Ethernet (rev 15)
Subsystem: Compaq Computer Corporation NC7770 Gigabit Server Adapter (PCI-X, 10/100/1000-T)
Flags: bus master, 66MHz, medium devsel, latency 16, IRQ 53
Memory at 90010000 (64-bit, non-prefetchable) [size=64K]
[virtual] Expansion ROM at 90000000 [disabled] [size=64K]
Capabilities: <access denied>
Just FYI, in case a pattern emerges etc.
/Mikael
^ permalink raw reply
* [PATCH 1/3] sky2: Factor out code to calculate packet sizes
From: Mike McCormack @ 2010-01-27 15:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Move code to calculate receive threshold and packet size out of
sky2_rx_start() so that is can be called from elsewhere easily.
Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
drivers/net/sky2.c | 57 +++++++++++++++++++++++++++++++++------------------
1 files changed, 37 insertions(+), 20 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 0b47c7f..2061eb8 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1064,6 +1064,40 @@ static inline struct sky2_rx_le *sky2_next_rx(struct sky2_port *sky2)
return le;
}
+static unsigned sky2_get_rx_threshold(struct sky2_port* sky2)
+{
+ unsigned size;
+
+ /* Space needed for frame data + headers rounded up */
+ size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
+
+ /* Stopping point for hardware truncation */
+ return (size - 8) / sizeof(u32);
+}
+
+static unsigned sky2_get_rx_data_size(struct sky2_port* sky2)
+{
+ struct rx_ring_info *re;
+ unsigned size;
+
+ /* Space needed for frame data + headers rounded up */
+ size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
+
+ sky2->rx_nfrags = size >> PAGE_SHIFT;
+ BUG_ON(sky2->rx_nfrags > ARRAY_SIZE(re->frag_addr));
+
+ /* Compute residue after pages */
+ size -= sky2->rx_nfrags << PAGE_SHIFT;
+
+ /* Optimize to handle small packets and headers */
+ if (size < copybreak)
+ size = copybreak;
+ if (size < ETH_HLEN)
+ size = ETH_HLEN;
+
+ return size;
+}
+
/* Build description to hardware for one receive segment */
static void sky2_rx_add(struct sky2_port *sky2, u8 op,
dma_addr_t map, unsigned len)
@@ -1337,7 +1371,7 @@ static int sky2_rx_start(struct sky2_port *sky2)
struct sky2_hw *hw = sky2->hw;
struct rx_ring_info *re;
unsigned rxq = rxqaddr[sky2->port];
- unsigned i, size, thresh;
+ unsigned i, thresh;
sky2->rx_put = sky2->rx_next = 0;
sky2_qset(hw, rxq);
@@ -1358,25 +1392,7 @@ static int sky2_rx_start(struct sky2_port *sky2)
if (!(hw->flags & SKY2_HW_NEW_LE))
rx_set_checksum(sky2);
- /* Space needed for frame data + headers rounded up */
- size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
-
- /* Stopping point for hardware truncation */
- thresh = (size - 8) / sizeof(u32);
-
- sky2->rx_nfrags = size >> PAGE_SHIFT;
- BUG_ON(sky2->rx_nfrags > ARRAY_SIZE(re->frag_addr));
-
- /* Compute residue after pages */
- size -= sky2->rx_nfrags << PAGE_SHIFT;
-
- /* Optimize to handle small packets and headers */
- if (size < copybreak)
- size = copybreak;
- if (size < ETH_HLEN)
- size = ETH_HLEN;
-
- sky2->rx_data_size = size;
+ sky2->rx_data_size = sky2_get_rx_data_size(sky2);
/* Fill Rx ring */
for (i = 0; i < sky2->rx_pending; i++) {
@@ -1401,6 +1417,7 @@ static int sky2_rx_start(struct sky2_port *sky2)
* the register is limited to 9 bits, so if you do frames > 2052
* you better get the MTU right!
*/
+ thresh = sky2_get_rx_threshold(sky2);
if (thresh > 0x1ff)
sky2_write32(hw, SK_REG(sky2->port, RX_GMF_CTRL_T), RX_TRUNC_OFF);
else {
--
1.5.6.5
^ permalink raw reply related
* [PATCH 2/3] sky2: Allocate initial skbs in sky2_alloc_buffers
From: Mike McCormack @ 2010-01-27 15:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
Allocating everything in one place means there's a single point
of failure in sky2_up, and sky2_rx_start can no longer fail.
This also fixes a memory leak in the case that sky2_rx_start
fails in the middle of allocating skbs, since any allocated
skbs will not be free'd in sky2_up's failure path.
Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
drivers/net/sky2.c | 42 +++++++++++++++++++++++-------------------
1 files changed, 23 insertions(+), 19 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index 2061eb8..a967912 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1358,7 +1358,7 @@ static inline void sky2_rx_update(struct sky2_port *sky2, unsigned rxq)
}
/*
- * Allocate and setup receiver buffer pool.
+ * Setup receiver buffer pool.
* Normal case this ends up creating one list element for skb
* in the receive ring. Worst case if using large MTU and each
* allocation falls on a different 64 bit region, that results
@@ -1392,22 +1392,10 @@ static int sky2_rx_start(struct sky2_port *sky2)
if (!(hw->flags & SKY2_HW_NEW_LE))
rx_set_checksum(sky2);
- sky2->rx_data_size = sky2_get_rx_data_size(sky2);
- /* Fill Rx ring */
+ /* submit Rx ring */
for (i = 0; i < sky2->rx_pending; i++) {
re = sky2->rx_ring + i;
-
- re->skb = sky2_rx_alloc(sky2);
- if (!re->skb)
- goto nomem;
-
- if (sky2_rx_map_skb(hw->pdev, re, sky2->rx_data_size)) {
- dev_kfree_skb(re->skb);
- re->skb = NULL;
- goto nomem;
- }
-
sky2_rx_submit(sky2, re);
}
@@ -1453,14 +1441,12 @@ static int sky2_rx_start(struct sky2_port *sky2)
return 0;
-nomem:
- sky2_rx_clean(sky2);
- return -ENOMEM;
}
static int sky2_alloc_buffers(struct sky2_port *sky2)
{
struct sky2_hw *hw = sky2->hw;
+ unsigned i;
/* must be power of 2 */
sky2->tx_le = pci_alloc_consistent(hw->pdev,
@@ -1486,6 +1472,24 @@ static int sky2_alloc_buffers(struct sky2_port *sky2)
if (!sky2->rx_ring)
goto nomem;
+ sky2->rx_data_size = sky2_get_rx_data_size(sky2);
+
+ /* Fill Rx ring */
+ for (i = 0; i < sky2->rx_pending; i++) {
+ struct rx_ring_info *re = sky2->rx_ring + i;
+
+ re->skb = sky2_rx_alloc(sky2);
+ if (!re->skb)
+ goto nomem;
+
+ if (sky2_rx_map_skb(hw->pdev, re, sky2->rx_data_size)) {
+ dev_kfree_skb(re->skb);
+ re->skb = NULL;
+ goto nomem;
+ }
+ }
+
+
return 0;
nomem:
return -ENOMEM;
@@ -1495,6 +1499,8 @@ static void sky2_free_buffers(struct sky2_port *sky2)
{
struct sky2_hw *hw = sky2->hw;
+ sky2_rx_clean(sky2);
+
if (sky2->rx_le) {
pci_free_consistent(hw->pdev, RX_LE_BYTES,
sky2->rx_le, sky2->rx_le_map);
@@ -1953,8 +1959,6 @@ static int sky2_down(struct net_device *dev)
/* Free any pending frames stuck in HW queue */
sky2_tx_complete(sky2, sky2->tx_prod);
- sky2_rx_clean(sky2);
-
sky2_free_buffers(sky2);
return 0;
--
1.5.6.5
^ permalink raw reply related
* [PATCH 3/3] sky2: Remove failure cases for sky2_rx_start
From: Mike McCormack @ 2010-01-27 15:05 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: netdev
sky2_rx_start() can no longer fail, so remove redundant code pathes.
Signed-off-by: Mike McCormack <mikem@ring3k.org>
---
drivers/net/sky2.c | 24 ++++++------------------
1 files changed, 6 insertions(+), 18 deletions(-)
diff --git a/drivers/net/sky2.c b/drivers/net/sky2.c
index a967912..6ea660f 100644
--- a/drivers/net/sky2.c
+++ b/drivers/net/sky2.c
@@ -1366,7 +1366,7 @@ static inline void sky2_rx_update(struct sky2_port *sky2, unsigned rxq)
* One element is used for checksum enable/disable, and one
* extra to avoid wrap.
*/
-static int sky2_rx_start(struct sky2_port *sky2)
+static void sky2_rx_start(struct sky2_port *sky2)
{
struct sky2_hw *hw = sky2->hw;
struct rx_ring_info *re;
@@ -1392,7 +1392,6 @@ static int sky2_rx_start(struct sky2_port *sky2)
if (!(hw->flags & SKY2_HW_NEW_LE))
rx_set_checksum(sky2);
-
/* submit Rx ring */
for (i = 0; i < sky2->rx_pending; i++) {
re = sky2->rx_ring + i;
@@ -1437,10 +1436,6 @@ static int sky2_rx_start(struct sky2_port *sky2)
sky2_write32(hw, Q_ADDR(txqaddr[sky2->port], Q_TEST),
TBMU_TEST_HOME_ADD_FIX_EN | TBMU_TEST_ROUTING_ADD_FIX_EN);
}
-
-
-
- return 0;
}
static int sky2_alloc_buffers(struct sky2_port *sky2)
@@ -1590,9 +1585,7 @@ static int sky2_up(struct net_device *dev)
sky2_set_vlan_mode(hw, port, sky2->vlgrp != NULL);
#endif
- err = sky2_rx_start(sky2);
- if (err)
- goto err_out;
+ sky2_rx_start(sky2);
/* Enable interrupts from phy/mac for port */
imask = sky2_read32(hw, B0_IMSK);
@@ -2200,7 +2193,6 @@ static int sky2_change_mtu(struct net_device *dev, int new_mtu)
struct sky2_port *sky2 = netdev_priv(dev);
struct sky2_hw *hw = sky2->hw;
unsigned port = sky2->port;
- int err;
u16 ctl, mode;
u32 imask;
@@ -2246,21 +2238,17 @@ static int sky2_change_mtu(struct net_device *dev, int new_mtu)
sky2_write8(hw, RB_ADDR(rxqaddr[port], RB_CTRL), RB_ENA_OP_MD);
- err = sky2_rx_start(sky2);
+ sky2_rx_start(sky2);
sky2_write32(hw, B0_IMSK, imask);
sky2_read32(hw, B0_Y2_SP_LISR);
napi_enable(&hw->napi);
- if (err)
- dev_close(dev);
- else {
- gma_write16(hw, port, GM_GP_CTRL, ctl);
+ gma_write16(hw, port, GM_GP_CTRL, ctl);
- netif_wake_queue(dev);
- }
+ netif_wake_queue(dev);
- return err;
+ return 0;
}
/* For small just reuse existing skb for next receive */
--
1.5.6.5
^ permalink raw reply related
* Re: [PATCH] tcp: fix ICMP-RTO war
From: Alexey Kuznetsov @ 2010-01-27 14:14 UTC (permalink / raw)
To: Ilpo J?rvinen; +Cc: David Miller, damian, denys, Netdev
In-Reply-To: <alpine.DEB.2.00.1001260957540.17103@melkinpaasi.cs.helsinki.fi>
Hello!
> I went through some history, it seems that this comment about the lower
> bound originates from Alexey [1]:
Yes, I even remember why it was done.
Actual comment starts with "If rtt variance happened to be less 50msec...".
It was observed that sometimes due to high burstiness of ACKs
(found originally when linux commuinitected to solaris tcp stack,
which issued huge burst of acks each 50msec. I think he data can be found
in netdev archives), rtt deviation tends to collapse to 0 during such burst,
rto was calculated as plain rtt and obviously we falled to retransmit.
So, the algorithm was repaired. First, to avoid glitches in the future
mdev was clamped to 50ms (rttvar, contribution to rto, is 200msec).
That's what the comment is about.
Second, Linux used to sample rtt each ack, not each rtt (which is technically
main source of the problem). Instead of decreasing sampling rate
I used different sampling rates for growing and dropping rtt:
it grows with rate of arriving acks and drops each rtt i.e. rto grows
quickly, but drops slowly.
Alexey
^ permalink raw reply
* Re: 0% cpu usasge after fresh boot or net restart but 10% CPU if kernel flush route cache
From: Eric Dumazet @ 2010-01-27 15:26 UTC (permalink / raw)
To: cold cold; +Cc: netdev
In-Reply-To: <41ac0f9e1001260858o7d2a6a6dgb37ecfba5c325932@mail.gmail.com>
Le mardi 26 janvier 2010 à 18:58 +0200, cold cold a écrit :
> HI,
>
>
> i have expiriance some CPU usage spikes up to 10% on each four cpus
> after the first kernel route cache flush.
> After mashine start first 20 min CPU is 0%si 300Mbits/s full duplex
> and arount 100k pps forwarded traffic, without any firewall, just
> plain routing.
>
> route -n |wc -l
> 34
>
> ip route show cache | wc -l
> 2140842
>
> cat /proc/sys/net/ipv4/route/secret_interval
> 600
> cat /proc/sys/net/ipv4/route/max_size
> 33554432
>
> after kernel flush i got 10% on all CPUs for 5-6 mins. It's not from
> rebuilding route cashe becouse after
> fresh boot or network restrat there is no CPU usage until kernel flush
> route cache.
> I try to play with rhash_entries= 300000 to 2000000 same result.
If you have one million dst entries to flush, it takes some time.
You could try to not increase the rhash_entries
(or keep it low, say 131072)
but tune /proc/sys/net/ipv4/route settings.
Try to reduce gc_elasticity from 8 to 2
Try to reduce gc_interval from 60 to 1
Important thing to consider is to irq affinities (so that one cpu
handles network interrupts, to minimize cache ping poings )
^ permalink raw reply
* Re: Hang: 2.6.32.4 sky2/DMAR (was [PATCH] sky2: Fix WARNING: at lib/dma-debug.c:902 check_sync)
From: Michael Breuer @ 2010-01-27 15:34 UTC (permalink / raw)
To: Jarek Poplawski
Cc: David Miller, Stephen Hemminger, akpm, flyboy, linux-kernel,
netdev, Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <20100123232133.GA3487@del.dom.local>
On 01/23/2010 06:21 PM, Jarek Poplawski wrote:
> On Fri, Jan 22, 2010 at 06:50:21PM -0500, Michael Breuer wrote:
>
>> When the packets were dropped, there was a different sequence in the
>> log - DISCOVER/OFFER repeated. The "normal" is that the sequence
>> appeared correct and complete - DISCOVER/OFFER/REQUEST/ACK - or
>> INFORM/ACK (vs. INFORM repeatedly sans ACK) as the case may be.
>>
> Anyway, I'd be intersted if the switch matters here.
>
> Plus one more test: could you try to load sky2 with the parameter:
> "copybreak=1" (the rest as in any recent test, which gave you dmar
> errors; any switch).
>
> Thanks,
> Jarek P.
>
Ok - now up 80+ hours with copybreak=1. I'm going to redo w/o copybreak
to confirm that I haven't inadvertently fixed something. However, given
that it might be copybreak-related, I looked at sky2.c again and I'm
wondering about the copybreak max size in sky2_rx_start:
size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
/* Stopping point for hardware truncation */
thresh = (size - 8) / sizeof(u32);
sky2->rx_nfrags = size >> PAGE_SHIFT;
BUG_ON(sky2->rx_nfrags > ARRAY_SIZE(re->frag_addr));
/* Compute residue after pages */
size -= sky2->rx_nfrags << PAGE_SHIFT;
/* Optimize to handle small packets and headers */
if (size < copybreak)
size = copybreak;
if (size < ETH_HLEN)
size = ETH_HLEN;
Why would increasing size to copybreak be valid here?
Guessing a bit as I'm not sure about rx_nfrags, but if I read this
correctly, if size is ever less than copybreak it's because there isn't
enough space left for anything larger. If so, wouldn't increasing size
potentially corrupt something? I'd further guess that the resulting
condition manifests sooner (or at least with a more visible effect) when
using DMAR.
In any event, why "copybreak" as the minimum buffer size? I'd suggest
that if it isn't possible to allocate at least MTU + overhead that
sky2_rx_start ought to be delayed until there is room.
^ permalink raw reply
* [PATCH 2.6.33-rc5] drivers/net: ks8851_mll ethernet network driver
From: Choi, David @ 2010-01-27 16:03 UTC (permalink / raw)
To: davem; +Cc: netdev
Hello David Miller,
I fix a bug in ks8851_mll driver, which has existed since 2.6.32-rc6.
>From : David J. Choi <david.choi@micrel.com>
Fix a bug that the data pointers in the interrupt handler are set wrong, which is related with the 5th parameter of request_irq().
Signed-off-by : David J. Choi <david.choi@micrel.com>
---
--- linux-2.6.33-rc5/drivers/net/ks8851_mll.c.orig 2010-01-26 17:36:51.000000000 -0800
+++ linux-2.6.33-rc5/drivers/net/ks8851_mll.c 2010-01-26 17:34:34.000000000 -0800
@@ -854,8 +854,8 @@ static void ks_update_link_status(struct
static irqreturn_t ks_irq(int irq, void *pw)
{
- struct ks_net *ks = pw;
- struct net_device *netdev = ks->netdev;
+ struct net_device *netdev = pw;
+ struct ks_net *ks = netdev_priv(netdev);
u16 status;
/*this should be the first in IRQ handler */
---
^ permalink raw reply
* Re: Network QoS support in applications
From: Olaf van der Spek @ 2010-01-27 16:18 UTC (permalink / raw)
To: Kalle Valo; +Cc: netdev, linux-wireless
In-Reply-To: <87k4v5nuej.fsf@purkki.valot.fi>
On Tue, Jan 26, 2010 at 9:27 AM, Kalle Valo <kalle.valo@iki.fi> wrote:
> I would like to clear up all this by and I'm willing to write a
> document for application developers about network QoS. But I need help
> to understand what's the proper way to mark different QoS
> prioritities.
Maybe it shouldn't be done by applications, unless the streams of an
app use different priorities.
It might be useful to look at configuration of networked apps in a
broader sense, including stuff like to what IP addresses and ports an
app should bind. Maybe even to what Unix sockets or other transports.
Being able to configure this in one central place is way easier then
having to dive into the conf file of each individual application.
In this central place the QoS stuff could be configured too.
As this doesn't require application support, it's probably easier to
implement. It also provides more flexibility and maybe even more
security.
Olaf
^ permalink raw reply
* Re: [PATCH 2.6.33-rc5] drivers/net: ks8851_mll ethernet network driver
From: Ben Hutchings @ 2010-01-27 16:28 UTC (permalink / raw)
To: davem; +Cc: Choi, David, netdev
In-Reply-To: <C43529A246480145B0A6D0234BDB0F0D02129C@MELANITE.micrel.com>
On Wed, 2010-01-27 at 08:03 -0800, Choi, David wrote:
> Hello David Miller,
>
> I fix a bug in ks8851_mll driver, which has existed since 2.6.32-rc6.
>
> From : David J. Choi <david.choi@micrel.com>
>
> Fix a bug that the data pointers in the interrupt handler are set wrong, which is related with the 5th parameter of request_irq().
>
> Signed-off-by : David J. Choi <david.choi@micrel.com>
This should go to stable as well since the driver will be useless
without this.
Ben.
--
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: Hang: 2.6.32.4 sky2/DMAR (was [PATCH] sky2: Fix WARNING: at lib/dma-debug.c:902 check_sync)
From: Stephen Hemminger @ 2010-01-27 16:50 UTC (permalink / raw)
To: Michael Breuer
Cc: Jarek Poplawski, David Miller, akpm, flyboy, linux-kernel, netdev,
Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <4B605D1B.60402@majjas.com>
On Wed, 27 Jan 2010 10:34:51 -0500
Michael Breuer <mbreuer@majjas.com> wrote:
> On 01/23/2010 06:21 PM, Jarek Poplawski wrote:
> > On Fri, Jan 22, 2010 at 06:50:21PM -0500, Michael Breuer wrote:
> >
> >> When the packets were dropped, there was a different sequence in the
> >> log - DISCOVER/OFFER repeated. The "normal" is that the sequence
> >> appeared correct and complete - DISCOVER/OFFER/REQUEST/ACK - or
> >> INFORM/ACK (vs. INFORM repeatedly sans ACK) as the case may be.
> >>
> > Anyway, I'd be intersted if the switch matters here.
> >
> > Plus one more test: could you try to load sky2 with the parameter:
> > "copybreak=1" (the rest as in any recent test, which gave you dmar
> > errors; any switch).
> >
> > Thanks,
> > Jarek P.
> >
> Ok - now up 80+ hours with copybreak=1. I'm going to redo w/o copybreak
> to confirm that I haven't inadvertently fixed something. However, given
> that it might be copybreak-related, I looked at sky2.c again and I'm
> wondering about the copybreak max size in sky2_rx_start:
>
> size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
>
> /* Stopping point for hardware truncation */
> thresh = (size - 8) / sizeof(u32);
>
> sky2->rx_nfrags = size >> PAGE_SHIFT;
> BUG_ON(sky2->rx_nfrags > ARRAY_SIZE(re->frag_addr));
>
> /* Compute residue after pages */
> size -= sky2->rx_nfrags << PAGE_SHIFT;
>
> /* Optimize to handle small packets and headers */
> if (size < copybreak)
> size = copybreak;
> if (size < ETH_HLEN)
> size = ETH_HLEN;
>
>
> Why would increasing size to copybreak be valid here?
>
> Guessing a bit as I'm not sure about rx_nfrags, but if I read this
> correctly, if size is ever less than copybreak it's because there isn't
> enough space left for anything larger. If so, wouldn't increasing size
> potentially corrupt something? I'd further guess that the resulting
> condition manifests sooner (or at least with a more visible effect) when
> using DMAR.
>
> In any event, why "copybreak" as the minimum buffer size? I'd suggest
> that if it isn't possible to allocate at least MTU + overhead that
> sky2_rx_start ought to be delayed until there is room.
This code is where driver decides how much data will be received in skb
data area and the remaining data spills over into skb frags.
Copybreak is the threshold so that packets less than size are copied
to a new skb. The code doing the copying there assumes the data is
totally contained in the skb (not in frags). The size increase there
is to make sure that assumption is always true. I suppose you
could do something perverse like setting copybreak really huge
and confuse driver, but that is a user error.
^ permalink raw reply
* Re: Hang: 2.6.32.4 sky2/DMAR (was [PATCH] sky2: Fix WARNING: at lib/dma-debug.c:902 check_sync)
From: Michael Breuer @ 2010-01-27 16:57 UTC (permalink / raw)
To: Stephen Hemminger
Cc: Jarek Poplawski, David Miller, akpm, flyboy, linux-kernel, netdev,
Michael Chan, Don Fry, Francois Romieu, Matt Carlson
In-Reply-To: <20100127085049.5b5048e9@nehalam>
On 1/27/2010 11:50 AM, Stephen Hemminger wrote:
> On Wed, 27 Jan 2010 10:34:51 -0500
> Michael Breuer<mbreuer@majjas.com> wrote:
>
>
>> On 01/23/2010 06:21 PM, Jarek Poplawski wrote:
>>
>>> On Fri, Jan 22, 2010 at 06:50:21PM -0500, Michael Breuer wrote:
>>>
>>>
>>>> When the packets were dropped, there was a different sequence in the
>>>> log - DISCOVER/OFFER repeated. The "normal" is that the sequence
>>>> appeared correct and complete - DISCOVER/OFFER/REQUEST/ACK - or
>>>> INFORM/ACK (vs. INFORM repeatedly sans ACK) as the case may be.
>>>>
>>>>
>>> Anyway, I'd be intersted if the switch matters here.
>>>
>>> Plus one more test: could you try to load sky2 with the parameter:
>>> "copybreak=1" (the rest as in any recent test, which gave you dmar
>>> errors; any switch).
>>>
>>> Thanks,
>>> Jarek P.
>>>
>>>
>> Ok - now up 80+ hours with copybreak=1. I'm going to redo w/o copybreak
>> to confirm that I haven't inadvertently fixed something. However, given
>> that it might be copybreak-related, I looked at sky2.c again and I'm
>> wondering about the copybreak max size in sky2_rx_start:
>>
>> size = roundup(sky2->netdev->mtu + ETH_HLEN + VLAN_HLEN, 8);
>>
>> /* Stopping point for hardware truncation */
>> thresh = (size - 8) / sizeof(u32);
>>
>> sky2->rx_nfrags = size>> PAGE_SHIFT;
>> BUG_ON(sky2->rx_nfrags> ARRAY_SIZE(re->frag_addr));
>>
>> /* Compute residue after pages */
>> size -= sky2->rx_nfrags<< PAGE_SHIFT;
>>
>> /* Optimize to handle small packets and headers */
>> if (size< copybreak)
>> size = copybreak;
>> if (size< ETH_HLEN)
>> size = ETH_HLEN;
>>
>>
>> Why would increasing size to copybreak be valid here?
>>
>> Guessing a bit as I'm not sure about rx_nfrags, but if I read this
>> correctly, if size is ever less than copybreak it's because there isn't
>> enough space left for anything larger. If so, wouldn't increasing size
>> potentially corrupt something? I'd further guess that the resulting
>> condition manifests sooner (or at least with a more visible effect) when
>> using DMAR.
>>
>> In any event, why "copybreak" as the minimum buffer size? I'd suggest
>> that if it isn't possible to allocate at least MTU + overhead that
>> sky2_rx_start ought to be delayed until there is room.
>>
> This code is where driver decides how much data will be received in skb
> data area and the remaining data spills over into skb frags.
> Copybreak is the threshold so that packets less than size are copied
> to a new skb. The code doing the copying there assumes the data is
> totally contained in the skb (not in frags). The size increase there
> is to make sure that assumption is always true. I suppose you
> could do something perverse like setting copybreak really huge
> and confuse driver, but that is a user error.
>
>
Ok - but I'm wondering under what circumstances size would be <
copybreak in the first place after computing the residue. If size ends
up being unreasonably small, is simply increasing the number to whatever
copybreak is correct? Assuming my testing is correct, then the crash
I've been experiencing when using dmar (only) seems related to the value
of copybreak. I don't think the other use (skb reuse) is the issue (but
hey, I could have missed something). The crash occurs when copybreak is
the default of 128, didn't happen when I set copybreak to 1.
^ permalink raw reply
* Re: [PATCH 2/3] sky2: Allocate initial skbs in sky2_alloc_buffers
From: Stephen Hemminger @ 2010-01-27 17:00 UTC (permalink / raw)
To: Mike McCormack; +Cc: netdev
In-Reply-To: <4B605648.7020705@ring3k.org>
On Thu, 28 Jan 2010 00:05:44 +0900
Mike McCormack <mikem@ring3k.org> wrote:
> Allocating everything in one place means there's a single point
> of failure in sky2_up, and sky2_rx_start can no longer fail.
>
> This also fixes a memory leak in the case that sky2_rx_start
> fails in the middle of allocating skbs, since any allocated
> skbs will not be free'd in sky2_up's failure path.
I may incorporate the cleanup, but there is no leak.
if rx_start fails, it already called rx_clean.
^ permalink raw reply
* Re: [PATCH 1/2] syncookies: print synflood warning if syn queue is full
From: Olaf van der Spek @ 2010-01-27 17:01 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev
In-Reply-To: <20091208.130959.138311725.davem@davemloft.net>
On Tue, Dec 8, 2009 at 10:09 PM, David Miller <davem@davemloft.net> wrote:
> From: Olaf van der Spek <olafvdspek@gmail.com>
> Date: Tue, 8 Dec 2009 15:47:59 +0100
>
>> On Fri, Oct 16, 2009 at 8:49 PM, Florian Westphal <fw@strlen.de> wrote:
>>> Always print a warning if the syn queue is full, just like
>>> the tcp/ipv6 code does.
>>>
>>> The "want_cookie" define is no longer needed -- gcc
>>> removes the relevant branches in the CONFIG_SYN_COOKIES=n case.
>>>
>>> Signed-off-by: Florian Westphal <fw@strlen.de>
>>
>> Any comments?
>
> You patch isn't even in patchwork any more, so for one thing
> it's definitely not in my queue any more.
Florian?
^ 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