| From e5bc56211e3dfb06929ce7cd523ad2d87f2fe4f2 Mon Sep 17 00:00:00 2001 |
| From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> |
| Date: Thu, 23 Oct 2008 14:06:35 -0700 |
| Subject: tcp: Restore ordering of TCP options for the sake of inter-operability |
| MIME-Version: 1.0 |
| Content-Type: text/plain; charset=utf-8 |
| Content-Transfer-Encoding: 8bit |
| |
| From: Ilpo Järvinen <ilpo.jarvinen@helsinki.fi> |
| |
| [ Upstream commit fd6149d332973bafa50f03ddb0ea9513e67f4517 ] |
| |
| 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> |
| Signed-off-by: David S. Miller <davem@davemloft.net> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> |
| |
| --- |
| net/ipv4/tcp_output.c | 23 +++++++++++++++++------ |
| 1 file changed, 17 insertions(+), 6 deletions(-) |
| |
| --- a/net/ipv4/tcp_output.c |
| +++ b/net/ipv4/tcp_output.c |
| @@ -357,6 +357,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) { |
| @@ -371,6 +382,12 @@ static void tcp_options_write(__be32 *pt |
| *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) | |
| @@ -387,12 +404,6 @@ static void tcp_options_write(__be32 *pt |
| *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) | |