* Re: TCP SACK issue, hung connection, tcpdump included
From: Darryl L. Miles @ 2007-07-31 5:03 UTC (permalink / raw)
To: linux-kernel; +Cc: Netdev
In-Reply-To: <20070729160721.GA31276@1wt.eu>
I've been able to capture a tcpdump from both ends during the problem
and its my belief there is a bug in 2.6.20.1 (at the client side) in
that it issues a SACK option for an old sequence which the current
window being advertised is beyond it. This is the most concerning issue
as the integrity of the sequence numbers doesn't seem right (to my
limited understanding anyhow).
There is another concern of why the SERVER performed a retransmission in
the first place, when the tcpdump shows the ack covering it has been seen.
I have made available the full dumps at:
http://darrylmiles.org/snippets/lkml/20070731/
There are some changes in 2.6.22 that appear to affect TCP SACK handling
does this fix a known issue ?
This sequence is interesting from the client side:
03:58:56.419034 IP SERVER.ssh > CLIENT.43726: . 26016:27464(1448) ack
4239 win 2728 <nop,nop,timestamp 16345815 819458859> # S1
03:58:56.419100 IP CLIENT.43726 > SERVER.ssh: . ack 27464 win 501
<nop,nop,timestamp 819458884 16345815> # C1
03:58:56.422019 IP SERVER.ssh > CLIENT.43726: P 27464:28176(712) ack
4239 win 2728 <nop,nop,timestamp 16345815 819458859> # S2
03:58:56.422078 IP CLIENT.43726 > SERVER.ssh: . ack 28176 win 501
<nop,nop,timestamp 819458884 16345815> # C2
The above 4 packets look as expect to me. Then we suddenly see a
retransmission of 26016:27464.
03:58:56.731597 IP SERVER.ssh > CLIENT.43726: . 26016:27464(1448) ack
4239 win 2728 <nop,nop,timestamp 16346128 819458864> # S3
So the client instead of discarding the retransmission of duplicate
segment, issues a SACK.
03:58:56.731637 IP CLIENT.43726 > SERVER.ssh: . ack 28176 win 501
<nop,nop,timestamp 819458962 16345815,nop,nop,sack sack 1 {26016:27464}
> # C3
In response to this the server is confused ??? It responds to
sack{26016:27464} but the client is also saying "wnd 28176". Wouldn't
the server expect "wnd < 26016" to there is a segment to retransmit ?
03:58:57.322800 IP SERVER.ssh > CLIENT.43726: . 26016:27464(1448) ack
4239 win 2728 <nop,nop,timestamp 16346718 819458864> # S4
Now viewed from the server side:
03:58:56.365655 IP SERVER.ssh > CLIENT.43726: . 26016:27464(1448) ack
4239 win 2728 <nop,nop,timestamp 16345815 819458859> # S1
03:58:56.365662 IP SERVER.ssh > CLIENT.43726: P 27464:28176(712) ack
4239 win 2728 <nop,nop,timestamp 16345815 819458859> # S2
03:58:56.374633 IP CLIENT.43726 > SERVER.ssh: . ack 24144 win 488
<nop,nop,timestamp 819458861 16345731> # propagation delay
03:58:56.381630 IP CLIENT.43726 > SERVER.ssh: . ack 25592 win 501
<nop,nop,timestamp 819458863 16345734> # propagation delay
03:58:56.384503 IP CLIENT.43726 > SERVER.ssh: . ack 26016 win 501
<nop,nop,timestamp 819458864 16345734> # propagation delay
03:58:56.462583 IP CLIENT.43726 > SERVER.ssh: . ack 27464 win 501
<nop,nop,timestamp 819458884 16345815> # C1
03:58:56.465707 IP CLIENT.43726 > SERVER.ssh: . ack 28176 win 501
<nop,nop,timestamp 819458884 16345815> # C2
The above packets just as expected.
03:58:56.678546 IP SERVER.ssh > CLIENT.43726: . 26016:27464(1448) ack
4239 win 2728 <nop,nop,timestamp 16346128 819458864> # S3
I guess the above packet is indeed a retransmission of "# S1" but why
was it retransmitted, when we can clearly see "# C1" above acks this
segment ? It is not even as if the retransmission escaped before the
kernel had time to process the ack, as 200ms elapsed. CONCERN NUMBER TWO
03:58:56.774778 IP CLIENT.43726 > SERVER.ssh: . ack 28176 win 501
<nop,nop,timestamp 819458962 16345815,nop,nop,sack sack 1 {26016:27464}
> # C3
CONCERN NUMBER ONE, why in response to that escaped retransmission was a
SACK the appropriate response ? When at the time the client sent the
SACK it had received all data upto 28176, a fact it continues to
advertise in the "# C3" packet above.
There is nothing wrong is the CLIENT expecting to see a retransmission
of that segment at this point in time that is an expected circumstance.
03:58:57.269529 IP SERVER.ssh > CLIENT.43726: . 26016:27464(1448) ack
4239 win 2728 <nop,nop,timestamp 16346718 819458864> # S4
Darryl
^ permalink raw reply
* Re: [PATCH net-2.6 1/2] [TCP]: Fix ratehalving with bidirectional flows
From: Ilpo Järvinen @ 2007-07-31 4:59 UTC (permalink / raw)
To: David Miller; +Cc: Netdev
In-Reply-To: <20070730.194946.08653937.davem@davemloft.net>
[-- Attachment #1: Type: TEXT/PLAIN, Size: 1131 bytes --]
On Mon, 30 Jul 2007, David Miller wrote:
> From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
> Date: Mon, 30 Jul 2007 20:18:45 +0300 (EEST)
>
> > Actually, the ratehalving seems to work too well, as cwnd is
> > reduced on every second ACK even though the packets in flight
> > remains unchanged. Recoveries in a bidirectional flows suffer
> > quite badly because of this, both NewReno and SACK are affected.
> >
> > After this patch, rate halving is performed for ACK only if
> > packets in flight was supposedly changed too.
> >
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
>
> This fix looks great, and I see no potential problems with
> Reno after doing a bit of auditing.
>
> Applied and I'll push to -stable too.
I think it's probably good to add tp->snd_una != prior_snd_una
check there too... It's not going to make a large difference,
mostly just to be conservative when skb collapse stuff got done
(and maybe to annoy cheaters too though I couldn't at this point
figure out how they could abuse it)...
...I think I can come up with that on Wednesday, so please hold
stable push until that.
--
i.
^ permalink raw reply
* [PATCH] SCTP: drop SACK if ctsn is not less than the next tsn of assoc
From: Wei Yongjun @ 2007-07-31 4:44 UTC (permalink / raw)
To: lksctp-developers, netdev
In-Reply-To: <18087.57737.908842.337891@zeus.sw.starentnetworks.com>
If SCTP data sender received a SACK which contains Cumulative TSN Ack is
not less than the Cumulative TSN Ack Point, and if this Cumulative TSN
Ack is not used by the data sender, SCTP data sender still accept this
SACK , and next SACK which send correctly to DATA sender be dropped,
because it is less than the new Cumulative TSN Ack Point.
After received this SACK, data will be retrans again and again even if
correct SACK is received.
So I think this SACK must be dropped to let data transmit correctly.
Following is the tcpdump of my test. And patch in this mail can avoid
this problem.
02:19:38.233278 sctp (1) [INIT] [init tag: 1250461886] [rwnd: 54784] [OS: 10] [MIS: 65535] [init TSN: 217114040]
02:19:39.782160 sctp (1) [INIT ACK] [init tag: 1] [rwnd: 54784] [OS: 100] [MIS: 65535] [init TSN: 100]
02:19:39.798583 sctp (1) [COOKIE ECHO]
02:19:40.082125 sctp (1) [COOKIE ACK]
02:19:40.097859 sctp (1) [DATA] (B)(E) [TSN: 217114040] [SID: 0] [SSEQ 0] [PPID 0xf192090b]
02:19:40.100162 sctp (1) [DATA] (B)(E) [TSN: 217114041] [SID: 0] [SSEQ 1] [PPID 0x3e467007]
02:19:40.100779 sctp (1) [DATA] (B)(E) [TSN: 217114042] [SID: 0] [SSEQ 2] [PPID 0x11b12a0a]
02:19:40.101200 sctp (1) [DATA] (B)(E) [TSN: 217114043] [SID: 0] [SSEQ 3] [PPID 0x30e7d979]
02:19:40.561147 sctp (1) [SACK] [cum ack 217114040] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:40.568498 sctp (1) [DATA] (B)(E) [TSN: 217114044] [SID: 0] [SSEQ 4] [PPID 0x251ff86f]
02:19:40.569308 sctp (1) [DATA] (B)(E) [TSN: 217114045] [SID: 0] [SSEQ 5] [PPID 0xe5d5da5d]
02:19:40.700584 sctp (1) [SACK] [cum ack 290855864] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:40.701562 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
02:19:40.701567 sctp (1) [DATA] (B)(E) [TSN: 217114047] [SID: 0] [SSEQ 7] [PPID 0xca47e645]
02:19:40.701569 sctp (1) [DATA] (B)(E) [TSN: 217114048] [SID: 0] [SSEQ 8] [PPID 0x6c0ea150]
02:19:40.701576 sctp (1) [DATA] (B)(E) [TSN: 217114049] [SID: 0] [SSEQ 9] [PPID 0x9cc1994f]
02:19:40.701585 sctp (1) [DATA] (B)(E) [TSN: 217114050] [SID: 0] [SSEQ 10] [PPID 0xb1df4129]
02:19:41.098201 sctp (1) [SACK] [cum ack 217114041] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:41.283257 sctp (1) [SACK] [cum ack 217114042] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:41.457217 sctp (1) [SACK] [cum ack 217114043] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:41.691528 sctp (1) [SACK] [cum ack 217114044] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:41.849636 sctp (1) [SACK] [cum ack 217114045] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:41.975473 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
02:19:42.021229 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:42.196495 sctp (1) [SACK] [cum ack 217114047] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:42.424319 sctp (1) [SACK] [cum ack 217114048] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:42.586924 sctp (1) [SACK] [cum ack 217114049] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:42.744810 sctp (1) [SACK] [cum ack 217114050] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:42.965536 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:43.106385 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
02:19:43.218969 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:45.374101 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
02:19:45.489258 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:49.830116 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
02:19:49.984577 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
02:19:58.760300 sctp (1) [DATA] (B)(E) [TSN: 217114046] [SID: 0] [SSEQ 6] [PPID 0x87d8b423]
02:19:58.931690 sctp (1) [SACK] [cum ack 217114046] [a_rwnd 54784] [#gap acks 0] [#dup tsns 0]
Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
--- net/sctp/sm_statefuns.c.orig 2007-07-29 18:11:01.000000000 -0400
+++ net/sctp/sm_statefuns.c 2007-07-29 18:14:49.000000000 -0400
@@ -2880,6 +2880,15 @@ sctp_disposition_t sctp_sf_eat_sack_6_2(
return SCTP_DISPOSITION_DISCARD;
}
+ /* If Cumulative TSN Ack is not less than the Cumulative TSN
+ * Ack which will be send in the next data, drop the SACK.
+ */
+ if (!TSN_lt(ctsn, asoc->next_tsn)) {
+ SCTP_DEBUG_PRINTK("ctsn %x\n", ctsn);
+ SCTP_DEBUG_PRINTK("next_tsn %x\n", asoc->next_tsn);
+ return SCTP_DISPOSITION_DISCARD;
+ }
+
/* Return this SACK for further processing. */
sctp_add_cmd_sf(commands, SCTP_CMD_PROCESS_SACK, SCTP_SACKH(sackh));
^ permalink raw reply
* Re: RFC: on [ab]use of skb->cb by VLAN code
From: Ben Greear @ 2007-07-31 4:30 UTC (permalink / raw)
To: David Miller; +Cc: hadi, kaber, netdev, mcarlson
In-Reply-To: <20070730.190621.13770511.davem@davemloft.net>
David Miller wrote:
> From: jamal <hadi@cyberus.ca>
> Date: Mon, 30 Jul 2007 22:02:04 -0400
>
>
>> I came across the issue because i used cb in batching to store transient
>> state which is used between qdisc dequeueing and hardware enqueueing
>> (looked and smelled legit to me).
>>
>
> Right, dequeue->device should be OK and doesn't work because of
> the VLAN issue.
>
So, shall we add a new field to the skb in order to get the info out of cb?
Looks like a single 32-bit field would be sufficient.
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* RE: [PATCH 0/4][RFC] lro: Generic Large Receive Offload for TCP traffic
From: Leonid Grossman @ 2007-07-31 3:58 UTC (permalink / raw)
To: Andrew Gallatin, Linas Vepstas
Cc: Jan-Bernd Themann, netdev, Thomas Klein, Jeff Garzik,
Jan-Bernd Themann, linux-kernel, linux-ppc, Christoph Raisch,
Marcus Eder, Stefan Roscher, David Miller
In-Reply-To: <46AE232F.3080409@myri.com>
> -----Original Message-----
> From: netdev-owner@vger.kernel.org [mailto:netdev-
> owner@vger.kernel.org] On Behalf Of Andrew Gallatin
> Sent: Monday, July 30, 2007 10:43 AM
> To: Linas Vepstas
> Cc: Jan-Bernd Themann; netdev; Thomas Klein; Jeff Garzik; Jan-Bernd
> Themann; linux-kernel; linux-ppc; Christoph Raisch; Marcus Eder;
Stefan
> Roscher; David Miller
> Subject: Re: [PATCH 0/4][RFC] lro: Generic Large Receive Offload for
> TCP traffic
>
>
> Here is a quick reply before something more official can
> be written up:
>
> Linas Vepstas wrote:
>
> > -- what is LRO?
>
> Large Receive Offload
>
> > -- Basic principles of operation?
>
> LRO is analogous to a receive side version of TSO. The NIC (or
> driver) merges several consecutive segments from the same connection,
> fixing up checksums, etc. Rather than up to 45 separate 1500 byte
> frames (meaning up to 45 trips through the network stack), the driver
> merges them into one 65212 byte frame. It currently works only
> with TCP over IPv4.
>
> LRO was, AFAIK, first though of by Neterion. They had a paper about
> it at OLS2005.
> http://www.linuxinsight.com/files/ols2005/grossman-reprint.pdf
>
> > -- Can I use it in my driver?
>
> Yes, it can be used in any driver.
>
> > -- Does my hardware have to have some special feature before I can
> use it?
>
> No.
Ditto. LRO hw assists (or full LRO offload, meaning that the merge
happens in the ASIC but the merge criteria are set by the host) are
quite beneficial, especially as the number of LRO connections gets very
large (Neterion has IP around fw/hw LRO and will release full hw LRO
offload in the next ASIC), but as Andrew indicated sw-only LRO
implementation can be done for any NIC and provides good results -
especially for non-jumbo workloads.
>
> > -- What sort of performance improvement does it provide?
Throughput?
> > Latency? CPU usage? How does it affect DMA allocation? Does it
> > improve only a certain type of traffic (large/small packets,
> etc.)
>
> The benefit is directly proportional to the packet rate.
>
> See my reply to the previous RFC for performance information. The
> executive summary is that for the myri10ge 10GbE driver on low end
> hardware with 1500b frames, I've seen it increase throughput by a
> factor of nearly 2.5x, while at the same time reducing CPU utilization
> by 17%. The affect for jumbo frames is less dramatic, but still
> impressive (1.10x, 14% CPU reduction)
>
> You can achieve better speedups if your driver receives into
> high-order pages.
>
> > -- Example code? What's the API? How should my driver use it?
>
> The 3/4 in this patch showed an example of converting a driver
> to use LRO for skb based receive buffers. I'm working on
> a patch for myri10ge that shows how you would use it in a driver
> which receives into pages.
>
> Cheers,
>
> Drew
> -
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] [NET]: fix multicast list when cloning sockets
From: Arnaldo Carvalho de Melo @ 2007-07-31 3:00 UTC (permalink / raw)
To: David Miller; +Cc: fleitner, dlstevens, netdev
In-Reply-To: <20070730.190144.26963470.davem@davemloft.net>
On 7/30/07, David Miller <davem@davemloft.net> wrote:
> From: Flavio Leitner <fleitner@redhat.com>
> Date: Mon, 30 Jul 2007 13:04:48 -0300
>
> >
> > The sock_copy() function uses memcpy() to clone the socket
> > including the struct ip_mc_socklist *mc_list pointer.
> >
> > The ip_mc_drop_socket() function is called when socket is closed
> > to free these objects leaving the other sockets cloned from the
> > same master socket with invalid pointers.
> >
> > This patch sets mc_list of cloned socket to NULL.
> >
> > Signed-off-by: Flavio Leitner <fleitner@redhat.com>
>
> Allowing non-datagram sockets to end up with a non-NULL inet->mc_list
> in the first place is a bug.
>
> Multicast subscriptions cannot even be used with TCP and DCCP, which
> are the only two users of these connection oriented socket functions.
>
> The first thing that TCP and DCCP do, in fact, for input packet
> processing is drop the packet if it is not unicast.
>
> Therefore the fix really is for the inet layer to reject multicast
> subscription requests on sockets for which that absolutely does not
> make sense. There is no reason these functions in
> inet_connection_sock.c should need to be mindful of multicast
> state. :-)
Well, we can add a BUG_ON there then 8)
Flavio, take a look at do_ip_setsockopt in net/ipv4/ip_sockglue.c, in
the IP_{ADD,DROP}_MEMBERSHIP labels.
Don't forget IPV6 (net/ipv6/ipv6_sockglue.c)
- Arnaldo
^ permalink raw reply
* Re: [PATCH net-2.6 0/2] [TCP]: Fix bidirectional brokeness
From: David Miller @ 2007-07-31 2:54 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev
In-Reply-To: <Pine.LNX.4.64.0707301947210.8788@kivilampi-30.cs.helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Mon, 30 Jul 2007 20:16:54 +0300 (EEST)
> These will generate you some hassle when you rebase tcp-2.6. Btw, you
> forgot to push tcp-2.6 out last time though I could assume it's
> state... :-) In case you're going to now push it out, could you please
> drop "[TCP]: Remove num_acked>0 checks from cong.ctrl mods pkts_acked"
> from it as it seems to be out of place in tcp-2.6, I can resubmit it
> to net-2.6.24 when you open it (unless you want to put it to net-2.6
> directly as it's rather trivial one).
It has been in the back of my mind to take care of the tcp-2.6
tree and rebase it for you and others.
I'll take a stab at that soon hopefully.
Thanks for reminding me.
^ permalink raw reply
* Re: [PATCH net-2.6 2/2] [TCP]: Bidir flow must not disregard SACK blocks for lost marking
From: David Miller @ 2007-07-31 2:53 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev
In-Reply-To: <Pine.LNX.4.64.0707302019110.8788@kivilampi-30.cs.helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Mon, 30 Jul 2007 20:19:40 +0300 (EEST)
> It's possible that new SACK blocks that should trigger new LOST
> markings arrive with new data (which previously made is_dupack
> false). In addition, I think this fixes a case where we get
> a cumulative ACK with enough SACK blocks to trigger the fast
> recovery (is_dupack would be false there too).
>
> I'm not completely pleased with this solution because readability
> of the code is somewhat questionable as 'is_dupack' in SACK case
> is no longer about dupacks only but would mean something like
> 'lost_marker_work_todo' too... But because of Eifel stuff done
> in CA_Recovery, the FLAG_DATA_SACKED check cannot be placed to
> the if statement which seems attractive solution. Nevertheless,
> I didn't like adding another variable just for that either... :-)
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
I've applied this as well.
I don't mind the complex conditionals so much in loss
handling, they are almost inevitable. However I believe
they could be simplified as a lot of pieces of code ask
similar if not identical questions.
We could ask several of these things up-front, regardless
of path we will take (reno, DSACK, reorder, FRTO, etc.)
and pass the answers along in a bitmask. We do that to
some extent already with how we analyze the retransmit
queue at the beginning of ACK processing.
^ permalink raw reply
* Re: [TULIP] Need new maintainer
From: david @ 2007-07-31 2:52 UTC (permalink / raw)
To: Valerie Henson
Cc: Kyle McMartin, netdev, linux-kernel, tulip-users, Grant Grundler,
Jeff Garzik
In-Reply-To: <20070731015710.GA4196@rainbow>
On Mon, 30 Jul 2007, Valerie Henson wrote:
> On Mon, Jul 30, 2007 at 03:31:58PM -0400, Kyle McMartin wrote:
>> On Mon, Jul 30, 2007 at 01:04:13PM -0600, Valerie Henson wrote:
>>> The Tulip network driver needs a new maintainer! I no longer have
>>> time to maintain the Tulip network driver and I'm stepping down. Jeff
>>> Garzik would be happy to get volunteers.
>>>
>>
>> Since I already take care of a major consumer of these devices (parisc,
>> which pretty much all have tulip) I'm willing to take care of this.
>> Alternately, Grant is probably willing.
>
> And I coulda handed you a suitcase full of cards and I missed my
> chance!
>
> It's fine by me, although Jeff is the final arbiter.
>
> Thanks!
>
> -VAL
when the maintainership gets settled send me a message and I can send you
a couple of different cards (including a d-link 4-port card)
David Lang
^ permalink raw reply
* Re: [PATCH net-2.6 1/2] [TCP]: Fix ratehalving with bidirectional flows
From: David Miller @ 2007-07-31 2:49 UTC (permalink / raw)
To: ilpo.jarvinen; +Cc: netdev
In-Reply-To: <Pine.LNX.4.64.0707302017290.8788@kivilampi-30.cs.helsinki.fi>
From: "Ilpo_Järvinen" <ilpo.jarvinen@helsinki.fi>
Date: Mon, 30 Jul 2007 20:18:45 +0300 (EEST)
> Actually, the ratehalving seems to work too well, as cwnd is
> reduced on every second ACK even though the packets in flight
> remains unchanged. Recoveries in a bidirectional flows suffer
> quite badly because of this, both NewReno and SACK are affected.
>
> After this patch, rate halving is performed for ACK only if
> packets in flight was supposedly changed too.
>
> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi>
This fix looks great, and I see no potential problems with
Reno after doing a bit of auditing.
Applied and I'll push to -stable too.
^ permalink raw reply
* Re: fscked clock sources revisited
From: jamal @ 2007-07-31 2:14 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Robert.Olsson, shemminger, kaber
In-Reply-To: <20070730.183750.77058266.davem@davemloft.net>
On Mon, 2007-30-07 at 18:37 -0700, David Miller wrote:
> Relatively speaking, the acpi_pm numbers are at least consistent and
> about as much as so as jiffies. Jiffies numbers are also possibly
> better, at least in part, because of the decreased accuracy and errors
> propagating.
I am not sure jiffies gives innacurate results. It gives the best
performance on the dual xeon i tested on, all the time. Very consistent.
On its accuracy: I was able to validate it With pktgen generating
traffic and some external hardware tool capturing the interpacket gaps
etc.
> tsc acts as expected, since every time your cpu changes power
> management state (which it is going to do dynamically) the TSC
> rates change and thus the accuracy goes into outer-space.
Robert was saying he had even more bizare results with opteron given the
NUMA nature.
> There really isn't much that can be done by any of this. These issues
> exist because of hardware limitations, nobody bothered to build
> x86/x86_64 systems with a system wide TICK register that is both
> impervious to cpu frequence scaling and also cheap to access.
>
> So the above is what we basically have to live with :-)
That is a bummer. I am going to test with hpet when i get the chance
and perhaps turn off all the other sources if nothing good comes out; i
need my numbers ;->
cheers,
jamal
^ permalink raw reply
* Re: fscked clock sources revisited
From: Arjan van de Ven @ 2007-07-31 2:07 UTC (permalink / raw)
To: hadi
Cc: netdev, David Miller, Robert.Olsson, Stephen Hemminger,
Patrick McHardy
In-Reply-To: <1185844980.5162.23.camel@localhost>
On Mon, 2007-07-30 at 21:23 -0400, jamal wrote:
> On Mon, 2007-30-07 at 21:17 -0400, jamal wrote:
>
> > Actually iirc, hpet is not even enabled in the
> > kernel -
>
> Sorry, i lied - the config file is on my laptop - it is enabled.
is it also on in the bios? (and if not, can you grab the patches to
force enable it instead?)
(they're in the -hrt patchkit and iirc also in -mm)
--
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org
^ permalink raw reply
* Re: fscked clock sources revisited
From: Arjan van de Ven @ 2007-07-31 2:03 UTC (permalink / raw)
To: David Miller; +Cc: hadi, netdev, Robert.Olsson, shemminger, kaber
In-Reply-To: <20070730.183750.77058266.davem@davemloft.net>
On Mon, 2007-07-30 at 18:37 -0700, David Miller wrote:
> There really isn't much that can be done by any of this. These issues
> exist because of hardware limitations, nobody bothered to build
> x86/x86_64 systems with a system wide TICK register that is both
> impervious to cpu frequence scaling and also cheap to access.
actually the tsc is clock speed invariant on pretty much all cpus these
days. The problem is that it stops in deeper idle states... (and that it
may be out of sync on an multi-socket system) not that it keeps changing
frequency ;(
(so it still sucks, just differently)
^ permalink raw reply
* Re: RFC: on [ab]use of skb->cb by VLAN code
From: David Miller @ 2007-07-31 2:06 UTC (permalink / raw)
To: hadi; +Cc: greearb, kaber, netdev, mcarlson
In-Reply-To: <1185847324.5162.31.camel@localhost>
From: jamal <hadi@cyberus.ca>
Date: Mon, 30 Jul 2007 22:02:04 -0400
> I came across the issue because i used cb in batching to store transient
> state which is used between qdisc dequeueing and hardware enqueueing
> (looked and smelled legit to me).
Right, dequeue->device should be OK and doesn't work because of
the VLAN issue.
^ permalink raw reply
* Re: [PATCH RFC]: napi_struct V4
From: David Miller @ 2007-07-31 2:03 UTC (permalink / raw)
To: rdreier; +Cc: netdev, shemminger, jgarzik, hadi, rusty
In-Reply-To: <adamyxdpbzg.fsf@cisco.com>
From: Roland Dreier <rdreier@cisco.com>
Date: Mon, 30 Jul 2007 19:01:55 -0700
> Are you saying that netif_rx_reschedule() itself is racy?
Nevermind, you're right.
I'll see if I can resurrect something akin to netif_rx_reschedule()
when I get back to the NAPI patches. Probably what I'll do is let
the ->poll() handler make sure to accumulate 'rx' work counts if
it loops again on netif_rx_reschedule() instead of dorking with
the quota and budget in that helper function.
^ permalink raw reply
* Re: RFC: on [ab]use of skb->cb by VLAN code
From: jamal @ 2007-07-31 2:02 UTC (permalink / raw)
To: David Miller; +Cc: greearb, kaber, netdev, mcarlson
In-Reply-To: <20070730.183337.27784025.davem@davemloft.net>
On Mon, 2007-30-07 at 18:33 -0700, David Miller wrote:
> I understand the concern, but how much qdisc stuff can possibly
> happen between those two ->hard_start_xmit() calls and do we
> want to support that in any way anyways?
>From a quick glance only netem seems to use it in the fast path (in a
legit way)
Theoretically, you could have many generations (i.e parents and
children, grandchildren etc) of netdevices stacked on top of each other
each with qdiscs. In a simple example: dont know how well these days
Vlans->bonding->somehardwarenetdevice works.
Redirect will could also result in a graph of unrelated netdevices (and
it is fair game to trample on cb anywhere along the path)
I came across the issue because i used cb in batching to store transient
state which is used between qdisc dequeueing and hardware enqueueing
(looked and smelled legit to me).
> The only alternative I see is to add more things to struct sk_buff
> and that's usually very unpopular :-)
I know ;-> Thats why i asked the question.
cheers,
jamal
^ permalink raw reply
* Re: [PATCH RFC]: napi_struct V4
From: Roland Dreier @ 2007-07-31 2:01 UTC (permalink / raw)
To: David Miller; +Cc: netdev, shemminger, jgarzik, hadi, rusty
In-Reply-To: <20070730.154854.07455239.davem@davemloft.net>
> It is not a feature of these changes.
> These races exist in the current code, it is a bug fix.
Not sure I follow. What is the race in the current code? Are you
saying that netif_rx_reschedule() itself is racy? Otherwise the code
in ipoib looks OK to me.
- R.
^ permalink raw reply
* Re: [PATCH] [NET]: fix multicast list when cloning sockets
From: David Miller @ 2007-07-31 2:01 UTC (permalink / raw)
To: fleitner; +Cc: dlstevens, acme, netdev
In-Reply-To: <20070730160448.GA4002@redhat.com>
From: Flavio Leitner <fleitner@redhat.com>
Date: Mon, 30 Jul 2007 13:04:48 -0300
>
> The sock_copy() function uses memcpy() to clone the socket
> including the struct ip_mc_socklist *mc_list pointer.
>
> The ip_mc_drop_socket() function is called when socket is closed
> to free these objects leaving the other sockets cloned from the
> same master socket with invalid pointers.
>
> This patch sets mc_list of cloned socket to NULL.
>
> Signed-off-by: Flavio Leitner <fleitner@redhat.com>
Allowing non-datagram sockets to end up with a non-NULL inet->mc_list
in the first place is a bug.
Multicast subscriptions cannot even be used with TCP and DCCP, which
are the only two users of these connection oriented socket functions.
The first thing that TCP and DCCP do, in fact, for input packet
processing is drop the packet if it is not unicast.
Therefore the fix really is for the inet layer to reject multicast
subscription requests on sockets for which that absolutely does not
make sense. There is no reason these functions in
inet_connection_sock.c should need to be mindful of multicast
state. :-)
^ permalink raw reply
* Re: [TULIP] Need new maintainer
From: Valerie Henson @ 2007-07-31 1:57 UTC (permalink / raw)
To: Kyle McMartin
Cc: netdev, linux-kernel, tulip-users, Grant Grundler, Jeff Garzik
In-Reply-To: <20070730193158.GA13808@fattire.cabal.ca>
On Mon, Jul 30, 2007 at 03:31:58PM -0400, Kyle McMartin wrote:
> On Mon, Jul 30, 2007 at 01:04:13PM -0600, Valerie Henson wrote:
> > The Tulip network driver needs a new maintainer! I no longer have
> > time to maintain the Tulip network driver and I'm stepping down. Jeff
> > Garzik would be happy to get volunteers.
> >
>
> Since I already take care of a major consumer of these devices (parisc,
> which pretty much all have tulip) I'm willing to take care of this.
> Alternately, Grant is probably willing.
And I coulda handed you a suitcase full of cards and I missed my
chance!
It's fine by me, although Jeff is the final arbiter.
Thanks!
-VAL
^ permalink raw reply
* Re: [PATCH 2.6.23-rc1] PPPOL2TP: Add CONFIG_INET Kconfig dependency
From: David Miller @ 2007-07-31 1:52 UTC (permalink / raw)
To: jchapman; +Cc: netdev, toralf.foerster
In-Reply-To: <200707301205.l6UC5dmR026629@quickie.katalix.com>
From: James Chapman <jchapman@katalix.com>
Date: Mon, 30 Jul 2007 13:05:39 +0100
> [PPPOL2TP]: Add CONFIG_INET Kconfig dependency.
>
> PPPOL2TP uses UDP so it obviously depends on CONFIG_INET.
>
> Signed-off-by: James Chapman <jchapman@katalix.com>
Fair enough, applied, thanks James.
^ permalink raw reply
* [PATCH]: Fix sk_buff page offsets and lengths.
From: David Miller @ 2007-07-31 1:50 UTC (permalink / raw)
To: netdev; +Cc: sfr, shemminger
Stephen Rothwell pointed out to me that the skb_frag_struct
is broken on platforms using 64K or larger page sizes, it
even generates warnings when (for example) the myri10ge driver
tries to assign PAGE_SIZE into frag->size.
I've thus increased page offset and size to __u32 in the patch below.
I made this change much to even my own chagrin, but this is the
most direct fix and the ifdefs we could put here are both ugly
and also not something that we do with struct scatterlist so
no reason to do it in a place like this.
Actually, the cost on 64-bit is zero because there existed 4 bytes of
alignment padding for skb_frag_struct because of the page pointer.
On 32-bit the cost is up to 64-bytes :-/
Stephen, this opens up the doors a bit for the scatterlist work
you wanted to do in sk_buff.
commit 051c14dbc588590e0a165dda0305c7c1b9ce7fb0
Author: David S. Miller <davem@sunset.davemloft.net>
Date: Mon Jul 30 18:47:03 2007 -0700
[NET]: Page offsets and lengths need to be __u32.
Based upon a report from Stephen Rothwell.
Signed-off-by: David S. Miller <davem@davemloft.net>
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index ce25643..93c27f7 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -134,8 +134,8 @@ typedef struct skb_frag_struct skb_frag_t;
struct skb_frag_struct {
struct page *page;
- __u16 page_offset;
- __u16 size;
+ __u32 page_offset;
+ __u32 size;
};
/* This data is invariant across clones and lives at
^ permalink raw reply related
* Re: fscked clock sources revisited
From: David Miller @ 2007-07-31 1:37 UTC (permalink / raw)
To: hadi; +Cc: netdev, Robert.Olsson, shemminger, kaber
In-Reply-To: <1185844239.5162.17.camel@localhost>
From: jamal <hadi@cyberus.ca>
Date: Mon, 30 Jul 2007 21:10:39 -0400
> acpi_pm: 108, 110, 111, 91, 108
> tsc: 143, 108, 161, 129, 108
> jiffies: 132, 138, 132, 146, 150
>
> jiffies produces better results than tsc which produces better results
> than acpi_pm.
Relatively speaking, the acpi_pm numbers are at least consistent and
about as much as so as jiffies. Jiffies numbers are also possibly
better, at least in part, because of the decreased accuracy and errors
propagating.
tsc acts as expected, since every time your cpu changes power
management state (which it is going to do dynamically) the TSC
rates change and thus the accuracy goes into outer-space.
There really isn't much that can be done by any of this. These issues
exist because of hardware limitations, nobody bothered to build
x86/x86_64 systems with a system wide TICK register that is both
impervious to cpu frequence scaling and also cheap to access.
So the above is what we basically have to live with :-)
^ permalink raw reply
* Re: RFC: on [ab]use of skb->cb by VLAN code
From: David Miller @ 2007-07-31 1:33 UTC (permalink / raw)
To: hadi; +Cc: greearb, kaber, netdev, mcarlson
In-Reply-To: <1185843556.5162.8.camel@localhost>
From: jamal <hadi@cyberus.ca>
Date: Mon, 30 Jul 2007 20:59:16 -0400
> This seems like a bad use since there can be a lot of things between
> a real hardware driver and something that sets a vlan tag (qdiscs come
> to mind).
I understand the concern, but how much qdisc stuff can possibly
happen between those two ->hard_start_xmit() calls and do we
want to support that in any way anyways?
The only alternative I see is to add more things to struct sk_buff
and that's usually very unpopular :-)
^ permalink raw reply
* Re: [PATCH 1/7] Preparatory refactoring part 1.
From: Corey Hickey @ 2007-07-31 1:26 UTC (permalink / raw)
To: Patrick McHardy; +Cc: netdev
In-Reply-To: <46ADECC9.7010303@trash.net>
Patrick McHardy wrote:
>> -static int
>> -sfq_enqueue(struct sk_buff *skb, struct Qdisc* sch)
>> +static void sfq_q_enqueue(struct sk_buff *skb, struct sfq_sched_data *q, unsigned int end)
>
>
> Please make sure to break at 80 chars and to keep the style
> in this file consistent (newline before function name).
Ok. For what it's worth, though, most of the original functions in the
file don't have a newline before the function name. Omitting the newline
would thus make the new/changed functions more consistent with the
rest of the file. I don't have a preference either way, so unless you
change your mind I'll put the newline back in..
>> {
>> - struct sfq_sched_data *q = qdisc_priv(sch);
>> unsigned hash = sfq_hash(q, skb);
>> sfq_index x;
>>
>> @@ -256,8 +257,12 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc* sch)
>> q->ht[hash] = x = q->dep[SFQ_DEPTH].next;
>> q->hash[x] = hash;
>> }
>> - sch->qstats.backlog += skb->len;
>
> Why not keep this instead of having both callers do it?
My idea was to have all the sfq_q_* functions operate on "struct
sfq_sched_data" and have no knowledge of the "struct Qdisc". I did this
in order to be able to use the new functions in sfq_change() when the
temporary sfq_sched_data doesn't have a parent Qdisc.
There's probably a better way, and I am of course open to suggestions,
but what I did made sense to me.
>> - __skb_queue_tail(&q->qs[x], skb);
>> +
>> + if (end == SFQ_TAIL)
>> + __skb_queue_tail(&q->qs[x], skb);
>> + else
>> + __skb_queue_head(&q->qs[x], skb);
>> +
>> sfq_inc(q, x);
>> if (q->qs[x].qlen == 1) { /* The flow is new */
>> if (q->tail == SFQ_DEPTH) { /* It is the first flow */
>> @@ -270,12 +275,21 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc* sch)
>> q->tail = x;
>> }
>> }
>> +}
>> +
>> +static int
>> +sfq_enqueue(struct sk_buff *skb, struct Qdisc* sch)
>> +{
>> + struct sfq_sched_data *q = qdisc_priv(sch);
>
> newline please.
Ok.
>> + sfq_q_enqueue(skb, q, SFQ_TAIL);
>> + sch->qstats.backlog += skb->len;
>> if (++sch->q.qlen < q->limit-1) {
>> sch->bstats.bytes += skb->len;
>> sch->bstats.packets++;
>> return 0;
>> }
>>
>> + sch->qstats.drops++;
>
> sfq_drop already increments this.
You're right, of course. When I look at the original sfq_requeue(),
though, I see that same line right before sfq_drop(). That's probably
why it ended up in my patch. Is that a bug? The original sfq_enqueue()
doesn't have that line.
http://git.kernel.org/?p=linux/kernel/git/jgarzik/netdev-2.6.git;a=blob;f=net/sched/sch_sfq.c
line 314
>> sfq_drop(sch);
>> return NET_XMIT_CN;
>> }
>> @@ -284,28 +298,8 @@ static int
>> sfq_requeue(struct sk_buff *skb, struct Qdisc* sch)
>> {
>> struct sfq_sched_data *q = qdisc_priv(sch);
>
> newline please
Ok.
>> -static struct sk_buff *
>> -sfq_dequeue(struct Qdisc* sch)
>> +static struct sk_buff *sfq_q_dequeue(struct sfq_sched_data *q)
>
>
> Keep style consistent please.
Ok.
Thank you for the review. I'll address the rest of your comments when I
have time later.
-Corey
^ permalink raw reply
* Re: fscked clock sources revisited
From: jamal @ 2007-07-31 1:23 UTC (permalink / raw)
To: Arjan van de Ven
Cc: netdev, David Miller, Robert.Olsson, Stephen Hemminger,
Patrick McHardy
In-Reply-To: <1185844629.5162.20.camel@localhost>
On Mon, 2007-30-07 at 21:17 -0400, jamal wrote:
> Actually iirc, hpet is not even enabled in the
> kernel -
Sorry, i lied - the config file is on my laptop - it is enabled.
cheers,
jamal
^ 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