| Message ID | Pine.LNX.4.64.0810221244390.7072@wrl-59.cs.helsinki.fi |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Il giorno Wed, 22 Oct 2008 13:00:01 +0300 (EEST) "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> ha scritto: [...] > > It would be nice for Aldo to check what the result will be with my > second patch (only) using sack=1,ts=0,wscale=0. I guess it does but > it's a bit unclear if nop's in front help or not (having the patch > below should anyway help also in that case as the mss option gets > moved before it anyway). in order to avoid misunderstandings, i list herebelow my actions: i've used on paperino the kernel 2.6.27.2 patched with ilpo's 21.10.2008 patch (see please comment 47 in http://bugzilla.kernel.org/show_bug.cgi?id=11721, and NOT with the patch contained in comment 58, right?) i've modified the following files: echo 0 > /proc/sys/net/ipv4/tcp_window_scaling echo 0 > /proc/sys/net/ipv4/tcp_timestamps echo 1 > /proc/sys/net/ipv4/tcp_sack the results are that i CAN navigate (w3m kernel.org) and update my system (apt-get update in debian). should it be of any help to ilpo, i attach the usual tcpdump on the wan eth of topolino (my home server/gw) ciao! aldo [...]
Il giorno Wed, 22 Oct 2008 15:09:40 +0300 (EEST) "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> ha scritto: [...] > So only thing that was problem in your case is that sackOK option (or > SACK_PERM in code notation) couldn't begin TCP options. With any > other option as the first one, including nop, it does work. Thanks > for all the testing you made and reporting it in the first place. i thank you for your efforts in solving this problem! it has been a real pleasere for me to cooperate with you! all the best to you all [...] > Thanks again. > i thank you! :-) aldo -- 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
From: "Ilpo Järvinen" <ilpo.jarvinen@helsinki.fi> Date: Wed, 22 Oct 2008 13:00:01 +0300 (EEST) > [PATCH] tcp: Restore ordering of TCP options for the sake of inter-operability > > This is not our bug! Sadly some devices cannot cope with the change > of TCP option ordering which was a result of the recent rewrite of > the option code (not that there was some particular reason steming > from the rewrite for the reordering) though any ordering of TCP > options is perfectly legal. Thus we restore the original ordering > to allow interoperability with/through such broken devices and add > some warning about this trap. Since the reordering just happened > without any particular reason, this change shouldn't cost us > anything. > > There are already couple of known failure reports (within close > proximity of the last release), so the problem might be more > wide-spread than a single device. And other reports which may > be due to the same problem though the symptoms were less obvious. > Analysis of one of the case revealed (with very high probability) > that sack capability cannot be negotiated as the first option > (SYN never got a response). > > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> > Reported-by: Aldo Maggi <sentiniate@tiscali.it> > Tested-by: Aldo Maggi <sentiniate@tiscali.it> Applied and I'll queue up for -stable, thanks! -- 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
On Sat, Oct 25, 2008 at 10:15:15PM +0200, Aldo Maggi wrote: > Il giorno Tue, 21 Oct 2008 07:49:54 +0000 > Jarek Poplawski <jarkao2@gmail.com> ha scritto: > > > On Tue, Oct 21, 2008 at 09:27:21AM +0200, Aldo Maggi wrote: > > ... > > > as soon as i've time i'll replace the modem and run some tests. > > just to let you know! > i have changed my modem with a new one. > the bug has disappeared with no modification of the tcp files. > this shows that the problem was due to my old zyxel. > > ciao! > aldo > Aldo, I think it's a very useful information, so I forward this to the people. Thanks again, Jarek P. -- 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
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index de54f02..e4c5ac9 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -362,6 +362,17 @@ struct tcp_out_options { __u32 tsval, tsecr; /* need to include OPTION_TS */ }; +/* Beware: Something in the Internet is very sensitive to the ordering of + * TCP options, we learned this through the hard way, so be careful here. + * Luckily we can at least blame others for their non-compliance but from + * inter-operatibility perspective it seems that we're somewhat stuck with + * the ordering which we have been using if we want to keep working with + * those broken things (not that it currently hurts anybody as there isn't + * particular reason why the ordering would need to be changed). + * + * At least SACK_PERM as the first option is known to lead to a disaster + * (but it may well be that other scenarios fail similarly). + */ static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp, const struct tcp_out_options *opts, __u8 **md5_hash) { @@ -376,6 +387,12 @@ static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp, *md5_hash = NULL; } + if (unlikely(opts->mss)) { + *ptr++ = htonl((TCPOPT_MSS << 24) | + (TCPOLEN_MSS << 16) | + opts->mss); + } + if (likely(OPTION_TS & opts->options)) { if (unlikely(OPTION_SACK_ADVERTISE & opts->options)) { *ptr++ = htonl((TCPOPT_SACK_PERM << 24) | @@ -392,12 +409,6 @@ static void tcp_options_write(__be32 *ptr, struct tcp_sock *tp, *ptr++ = htonl(opts->tsecr); } - if (unlikely(opts->mss)) { - *ptr++ = htonl((TCPOPT_MSS << 24) | - (TCPOLEN_MSS << 16) | - opts->mss); - } - if (unlikely(OPTION_SACK_ADVERTISE & opts->options && !(OPTION_TS & opts->options))) { *ptr++ = htonl((TCPOPT_NOP << 24) |