* [PATCH] Avoid reusing the same TCP port number
@ 2014-07-01 11:53 Valentin Dornauer
2014-07-03 18:41 ` Andrey Borzenkov
0 siblings, 1 reply; 3+ messages in thread
From: Valentin Dornauer @ 2014-07-01 11:53 UTC (permalink / raw)
To: grub-devel; +Cc: fritsch
[-- Attachment #1.1: Type: text/plain, Size: 1202 bytes --]
Hi!
Attached is another patch from our internal repository. I am not
certain that it will work under any circumstances and it has only
been tested on x86 hardware, so please use with care!
We had some very annoying issues with quick consecutive reboots,
as GRUB would reuse the same TCP port number without properly closing
the connection (especially when a boot fails). The remote host
(Apache web server) would then discard the new connection and booting
from HTTP would fail.
I've included excerpts from the original commit message to further
clarify the problem:
Even if a grub boot succeeds, the TCP connection for the last
HTTP request stays in state LAST_ACK for some reason. If the
next reboot happens before the web server discards that connection,
this will lead to the server not responding to the new SYN on
the same port number.
Make the initial port number depend on the RTC time. Increase
it by 80 for every second, wrapping around after 512 seconds.
The 80 is used because grub opens around 3 connections for every
file. Use & 511 instead of modulo because of linking problems.
Original patch by Stefan Fritsch <fritsch@genua.de>.
- Valentin
[-- Attachment #1.2: 0001-Avoid-reusing-the-same-TCP-port-number.patch --]
[-- Type: application/octet-stream, Size: 2740 bytes --]
From accf41f8b2b71b03fbd3c7cd5774d44a83874f4a Mon Sep 17 00:00:00 2001
From: Valentin Dornauer <valentin@unimplemented.org>
Date: Tue, 1 Jul 2014 11:54:00 +0200
Subject: [PATCH] Avoid reusing the same TCP port number
Make the initial port number depend on the RTC time. Increase it by 80
for every second, wrapping around after 512 seconds.
* grub-core/net/tcp.c (grub_net_tcp_open): Calculate in_port based
on RTC time to avoid problems with stale open TCP connections on the
remote host.
Original patch by Stefan Fritsch <fritsch@genua.de>.
---
ChangeLog | 6 ++++++
grub-core/net/tcp.c | 21 ++++++++++++++++++++-
2 files changed, 26 insertions(+), 1 deletion(-)
diff --git a/ChangeLog b/ChangeLog
index 5109c5a..61a01d2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+2014-07-01 Valentin Dornauer <valentin@unimplemented.org>
+
+ * grub-core/net/tcp.c (grub_net_tcp_open): Calculate in_port based
+ on RTC time to avoid problems with stale open TCP connections on the
+ remote host.
+
2014-06-26 Colin Watson <cjwatson@ubuntu.com>
* docs/grub-dev.texi (Finding your way around): The build system no
diff --git a/grub-core/net/tcp.c b/grub-core/net/tcp.c
index 2077f55..b125e6d 100644
--- a/grub-core/net/tcp.c
+++ b/grub-core/net/tcp.c
@@ -22,6 +22,7 @@
#include <grub/net/netbuff.h>
#include <grub/time.h>
#include <grub/priority_queue.h>
+#include <grub/datetime.h>
#define TCP_SYN_RETRANSMISSION_TIMEOUT GRUB_NET_INTERVAL
#define TCP_SYN_RETRANSMISSION_COUNT GRUB_NET_TRIES
@@ -553,13 +554,31 @@ grub_net_tcp_open (char *server,
struct grub_net_network_level_interface *inf;
grub_net_network_level_address_t gateway;
grub_net_tcp_socket_t socket;
- static grub_uint16_t in_port = 21550;
+ static grub_uint16_t in_port = 0;
struct grub_net_buff *nb;
struct tcphdr *tcph;
int i;
grub_uint8_t *nbd;
grub_net_link_level_address_t ll_target_addr;
+ if (in_port == 0)
+ {
+ struct grub_datetime datetime;
+ unsigned int seconds;
+ in_port = 21550;
+
+ /*
+ * Try to not use the same initial port number on consecutive quick reboots,
+ * in particular avoid collisions within the usual TIME_WAIT time of 120s.
+ * Increase the initial port number by 80 for every RTC second mod 512.
+ * This means the max for in_port is 21550 + 511 * 80 == 62430.
+ */
+ if (grub_get_datetime (&datetime) == 0)
+ {
+ seconds = ((datetime.hour * 60 ) + datetime.minute ) * 60 + datetime.second;
+ in_port += (seconds & 511) * 80;
+ }
+ }
err = grub_net_resolve_address (server, &addr);
if (err)
return NULL;
--
1.7.10.4
[-- Attachment #1.3: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: Message signed with OpenPGP using GPGMail --]
[-- Type: application/pgp-signature, Size: 841 bytes --]
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Avoid reusing the same TCP port number
2014-07-01 11:53 [PATCH] Avoid reusing the same TCP port number Valentin Dornauer
@ 2014-07-03 18:41 ` Andrey Borzenkov
2015-01-13 14:41 ` Valentin Dornauer
0 siblings, 1 reply; 3+ messages in thread
From: Andrey Borzenkov @ 2014-07-03 18:41 UTC (permalink / raw)
To: The development of GNU GRUB; +Cc: valentin, fritsch
[-- Attachment #1: Type: text/plain, Size: 1579 bytes --]
В Tue, 1 Jul 2014 13:53:49 +0200
Valentin Dornauer <valentin@unimplemented.org> пишет:
> Hi!
> Attached is another patch from our internal repository. I am not
> certain that it will work under any circumstances and it has only
> been tested on x86 hardware, so please use with care!
>
> We had some very annoying issues with quick consecutive reboots,
> as GRUB would reuse the same TCP port number without properly closing
> the connection (especially when a boot fails). The remote host
> (Apache web server) would then discard the new connection and booting
> from HTTP would fail.
>
>
> I've included excerpts from the original commit message to further
> clarify the problem:
>
> Even if a grub boot succeeds, the TCP connection for the last
> HTTP request stays in state LAST_ACK for some reason. If the
> next reboot happens before the web server discards that connection,
> this will lead to the server not responding to the new SYN on
> the same port number.
>
> Make the initial port number depend on the RTC time. Increase
> it by 80 for every second, wrapping around after 512 seconds.
> The 80 is used because grub opens around 3 connections for every
> file. Use & 511 instead of modulo because of linking problems.
>
> Original patch by Stefan Fritsch <fritsch@genua.de>.
>
> - Valentin
>
May be you can simply use grub_get_time_ms()? After all, we are not
interested in exact time, just in some pseudo-random distribution. It
should have less overhead than grub_get_datetime().
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Avoid reusing the same TCP port number
2014-07-03 18:41 ` Andrey Borzenkov
@ 2015-01-13 14:41 ` Valentin Dornauer
0 siblings, 0 replies; 3+ messages in thread
From: Valentin Dornauer @ 2015-01-13 14:41 UTC (permalink / raw)
To: Andrey Borzenkov; +Cc: grub-devel
Hello Andrey,
On Thu, Jul 03, 2014 at 10:41:25PM +0400, Andrey Borzenkov wrote:
> May be you can simply use grub_get_time_ms()? After all, we are not
> interested in exact time, just in some pseudo-random distribution. It
> should have less overhead than grub_get_datetime().
I finally got around to trying out your suggestion concerning the
TCP source port "randomization". The main problem I see with using
the TSC's value instead of the current time of day (in seconds)
is that it will have approximately the same value every time the
machine is rebooted and reaches the point where the first file is
loaded. This means that clashes are way more likely than with the
RTC based solution, at least for the first file. Little or no network
jitter could mean even more collisions.
Do you think it's still worth the effort to change the previously
proposed [1] patch?
Thanks
Valentin
[1] http://lists.gnu.org/archive/html/grub-devel/2014-07/msg00000.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2015-01-13 14:42 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-07-01 11:53 [PATCH] Avoid reusing the same TCP port number Valentin Dornauer
2014-07-03 18:41 ` Andrey Borzenkov
2015-01-13 14:41 ` Valentin Dornauer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).