From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Warren Date: Thu, 11 Mar 2010 15:08:27 -0800 Subject: [U-Boot] [PATCH] TFTP: allow for adjustable retransmission timout In-Reply-To: <20100311224421.0E098E22AD1@gemini.denx.de> References: <1256115432-31992-1-git-send-email-wd@denx.de> <1263768953-18819-1-git-send-email-wd@denx.de> <20100311224421.0E098E22AD1@gemini.denx.de> Message-ID: <4B9977EB.7020702@gmail.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Wolfgang, On 3/11/2010 2:44 PM, Wolfgang Denk wrote: > Dear Ben, > > In message<1263768953-18819-1-git-send-email-wd@denx.de> I wrote: > >> So far, TFTP negotiated a fixed retransmission timeout of 5 seconds. >> In some cases (busy networks, slow TFTP servers) this caused very >> slow transfers. Add new environment variable "tftptimeout" allows to >> set this timeout. Lowering this value may make downloads succeed >> faster in networks with high packet loss rates or with unreliable >> TFTP servers. >> >> Signed-off-by: Wolfgang Denk >> Cc: Ben Warren >> --- >> I submitted this patch for RFC in October, but never received any >> feedback. Let's see if it goes in or gets rejected... >> >> README | 19 ++++++++++++++++--- >> net/tftp.c | 22 +++++++++++++++++----- >> 2 files changed, 33 insertions(+), 8 deletions(-) >> >> diff --git a/README b/README >> index 22e35c3..13bad41 100644 >> --- a/README >> +++ b/README >> @@ -2982,7 +2982,9 @@ environment. As long as you don't save the environment you are >> working with an in-memory copy. In case the Flash area containing the >> environment is erased by accident, a default environment is provided. >> >> -Some configuration options can be set using Environment Variables: >> +Some configuration options can be set using Environment Variables. >> + >> +List of environment variables (most likely not complete): >> >> baudrate - see CONFIG_BAUDRATE >> >> @@ -3094,7 +3096,7 @@ Some configuration options can be set using Environment Variables: >> available network interfaces. >> It just stays at the currently selected interface. >> >> - netretry - When set to "no" each network operation will >> + netretry - When set to "no" each network operation will >> either succeed or fail without retrying. >> When set to "once" the network operation will >> fail when all the available network interfaces >> @@ -3110,7 +3112,18 @@ Some configuration options can be set using Environment Variables: >> tftpdstport - If this is set, the value is used for TFTP's UDP >> destination port instead of the Well Know Port 69. >> >> - vlan - When set to a value< 4095 the traffic over >> + tftpblocksize - Block size to use for TFTP transfers; if not set, >> + we use the TFTP server's default block size >> + >> + tftptimeout - Retransmission timeout for TFTP packets (in milli- >> + seconds, minimum value is 1000 = 1 second). Defines >> + when a packet is considered to be lost so it has to >> + be retransmitted. The default is 5000 = 5 seconds. >> + Lowering this value may make downloads succeed >> + faster in networks with high packet loss rates or >> + with unreliable TFTP servers. >> + >> + vlan - When set to a value< 4095 the traffic over >> Ethernet is encapsulated/received over 802.1q >> VLAN tagged frames. >> >> diff --git a/net/tftp.c b/net/tftp.c >> index a02463b..3f402d1 100644 >> --- a/net/tftp.c >> +++ b/net/tftp.c >> @@ -211,7 +211,7 @@ TftpSend (void) >> pkt += 5 /*strlen("octet")*/ + 1; >> strcpy ((char *)pkt, "timeout"); >> pkt += 7 /*strlen("timeout")*/ + 1; >> - sprintf((char *)pkt, "%lu", TIMEOUT / 1000); >> + sprintf((char *)pkt, "%lu", TftpTimeoutMSecs / 1000); >> debug("send option \"timeout %s\"\n", (char *)pkt); >> pkt += strlen((char *)pkt) + 1; >> #ifdef CONFIG_TFTP_TSIZE >> @@ -413,7 +413,6 @@ TftpHandler (uchar * pkt, unsigned dest, unsigned src, unsigned len) >> } >> >> TftpLastBlock = TftpBlock; >> - TftpTimeoutMSecs = TIMEOUT; >> TftpTimeoutCountMax = TIMEOUT_COUNT; >> NetSetTimeout (TftpTimeoutMSecs, TftpTimeout); >> >> @@ -528,10 +527,24 @@ TftpStart (void) >> { >> char *ep; /* Environment pointer */ >> >> - /* Allow the user to choose tftpblocksize */ >> + /* >> + * Allow the user to choose TFTP blocksize and timeout. >> + * TFTP protocol has a minimal timeout of 1 second. >> + */ >> if ((ep = getenv("tftpblocksize")) != NULL) >> TftpBlkSizeOption = simple_strtol(ep, NULL, 10); >> - debug("tftp block size is %i\n", TftpBlkSizeOption); >> + >> + if ((ep = getenv("tftptimeout")) != NULL) >> + TftpTimeoutMSecs = simple_strtol(ep, NULL, 10); >> + >> + if (TftpTimeoutMSecs< 1000) { >> + printf("TFTP timeout (%ld ms) too low, " >> + "set minimum = 1000 ms\n); >> + TftpTimeoutMSecs = 1000; >> + } >> + >> + debug("TFTP blocksize = %i, timeout = %ld ms\n", >> + TftpBlkSizeOption, TftpTimeoutMSecs); >> >> TftpServerIP = NetServerIP; >> if (BootFile[0] == '\0') { >> @@ -588,7 +601,6 @@ TftpStart (void) >> >> puts ("Loading: *\b"); >> >> - TftpTimeoutMSecs = TftpRRQTimeoutMSecs; >> TftpTimeoutCountMax = TftpRRQTimeoutCountMax; >> >> NetSetTimeout (TftpTimeoutMSecs, TftpTimeout); >> -- >> > What's the status of this patch? Is it on your queue? > So sorry - I thought I'd already taken care of this. Please feel free to add my SOB and apply directly. Signed-off-by: Ben Warren regards, Ben