[U-Boot] commit 620776d causes TFTP error: 'Unsupported option(s) requested' (8)

Hi,
With latest u-boot/master, TFTP is seriously broken.
=> tftp 100000 bzImage Speed: 100, full duplex Using pch_gbe device TFTP from server 10.10.0.8; our IP address is 10.10.0.100; sending through gateway 10.10.0.1 Filename 'bzImage'. Load address: 0x100000 Loading: T TFTP error: 'Unsupported option(s) requested' (8) Starting again
git bisect shows the following commit broke the TFTP
commit 620776d734e4b126c407f636bda825a594a17723 Author: Pavel Machek pavel@denx.de Date: Tue Aug 18 14:34:26 2015 +0200
tftp: adjust settings to be suitable for 100Mbit ethernet
Adjust timouts and retry counts to be suitable for loaded ethernet network. With 5 seconds timeout, 10 retries maximum, tftp is impossible even on local network with single full-speed TCP connection.
100msec timeout should be suitable for most networks tftp is used on, that is local ethernets. Timeout count really needs to be way higher, as lost packets are normal when TCP is running over the same network.
Enforce 10msec minimum.
Signed-off-by: Pavel Machek pavel@denx.de Acked-by: Joe Hershberger joe.hershberger@ni.com
Can we get this fixed ASAP? Thanks,
Regards, Bin

Hi Bin,
On Mon, Aug 24, 2015 at 9:25 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi,
With latest u-boot/master, TFTP is seriously broken.
=> tftp 100000 bzImage Speed: 100, full duplex Using pch_gbe device TFTP from server 10.10.0.8; our IP address is 10.10.0.100; sending through gateway 10.10.0.1 Filename 'bzImage'. Load address: 0x100000 Loading: T TFTP error: 'Unsupported option(s) requested' (8) Starting again
I'm guessing you are having an issue that the TFTP server you are using for some reason does not allow a timeout as small as 10 ms. What server are you testing against? Can you experiment and find the lowest that it accepts? Maybe there is a compromise we can reach that still works with most (all?) servers and also improves the behavior in a lossy environment.
git bisect shows the following commit broke the TFTP
commit 620776d734e4b126c407f636bda825a594a17723 Author: Pavel Machek pavel@denx.de Date: Tue Aug 18 14:34:26 2015 +0200
tftp: adjust settings to be suitable for 100Mbit ethernet Adjust timouts and retry counts to be suitable for loaded ethernet network. With 5 seconds timeout, 10 retries maximum, tftp is impossible even on local network with single full-speed TCP connection. 100msec timeout should be suitable for most networks tftp is used on, that is local ethernets. Timeout count really needs to be way higher, as lost packets are normal when TCP is running over the same network. Enforce 10msec minimum. Signed-off-by: Pavel Machek <pavel@denx.de> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
Can we get this fixed ASAP? Thanks,
Hopefully with a little more information about your case we can get this resolved quickly.
-Joe

Hi Joe,
On Tue, Aug 25, 2015 at 11:42 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Mon, Aug 24, 2015 at 9:25 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi,
With latest u-boot/master, TFTP is seriously broken.
=> tftp 100000 bzImage Speed: 100, full duplex Using pch_gbe device TFTP from server 10.10.0.8; our IP address is 10.10.0.100; sending through gateway 10.10.0.1 Filename 'bzImage'. Load address: 0x100000 Loading: T TFTP error: 'Unsupported option(s) requested' (8) Starting again
I'm guessing you are having an issue that the TFTP server you are using for some reason does not allow a timeout as small as 10 ms. What server are you testing against? Can you experiment and find the lowest that it accepts? Maybe there is a compromise we can reach that still works with most (all?) servers and also improves the behavior in a lossy environment.
I am using a CentOS server, with /etc/xinetd.d/tftp configuration below.
service tftp { socket_type = dgram protocol = udp wait = yes user = root server = /usr/sbin/in.tftpd server_args = -s /tftpboot disable = no per_source = 11 cps = 100 2 flags = IPv4 }
I don't see an entry to change timeout settings.
git bisect shows the following commit broke the TFTP
commit 620776d734e4b126c407f636bda825a594a17723 Author: Pavel Machek pavel@denx.de Date: Tue Aug 18 14:34:26 2015 +0200
tftp: adjust settings to be suitable for 100Mbit ethernet Adjust timouts and retry counts to be suitable for loaded ethernet network. With 5 seconds timeout, 10 retries maximum, tftp is impossible even on local network with single full-speed TCP connection. 100msec timeout should be suitable for most networks tftp is used on, that is local ethernets. Timeout count really needs to be way higher, as lost packets are normal when TCP is running over the same network. Enforce 10msec minimum. Signed-off-by: Pavel Machek <pavel@denx.de> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
Can we get this fixed ASAP? Thanks,
Hopefully with a little more information about your case we can get this resolved quickly.
Regards, Bin

Hi Bin,
On Mon, Aug 24, 2015 at 11:05 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Tue, Aug 25, 2015 at 11:42 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Mon, Aug 24, 2015 at 9:25 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi,
With latest u-boot/master, TFTP is seriously broken.
=> tftp 100000 bzImage Speed: 100, full duplex Using pch_gbe device TFTP from server 10.10.0.8; our IP address is 10.10.0.100; sending through gateway 10.10.0.1 Filename 'bzImage'. Load address: 0x100000 Loading: T TFTP error: 'Unsupported option(s) requested' (8) Starting again
I'm guessing you are having an issue that the TFTP server you are using for some reason does not allow a timeout as small as 10 ms. What server are you testing against? Can you experiment and find the lowest that it accepts? Maybe there is a compromise we can reach that still works with most (all?) servers and also improves the behavior in a lossy environment.
I am using a CentOS server, with /etc/xinetd.d/tftp configuration below.
service tftp { socket_type = dgram protocol = udp wait = yes user = root server = /usr/sbin/in.tftpd server_args = -s /tftpboot disable = no per_source = 11 cps = 100 2 flags = IPv4 }
I don't see an entry to change timeout settings.
I didn't mean to change the server settings; I meant can you adjust the U-Boot code until your server doesn't reject the option?
git bisect shows the following commit broke the TFTP
commit 620776d734e4b126c407f636bda825a594a17723 Author: Pavel Machek pavel@denx.de Date: Tue Aug 18 14:34:26 2015 +0200
tftp: adjust settings to be suitable for 100Mbit ethernet Adjust timouts and retry counts to be suitable for loaded ethernet network. With 5 seconds timeout, 10 retries maximum, tftp is impossible even on local network with single full-speed TCP connection. 100msec timeout should be suitable for most networks tftp is used on, that is local ethernets. Timeout count really needs to be way higher, as lost packets are normal when TCP is running over the same network. Enforce 10msec minimum. Signed-off-by: Pavel Machek <pavel@denx.de> Acked-by: Joe Hershberger <joe.hershberger@ni.com>
Can we get this fixed ASAP? Thanks,
Hopefully with a little more information about your case we can get this resolved quickly.
Regards, Bin

Hi Joe,
On Tue, Aug 25, 2015 at 12:24 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Mon, Aug 24, 2015 at 11:05 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Tue, Aug 25, 2015 at 11:42 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Mon, Aug 24, 2015 at 9:25 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi,
With latest u-boot/master, TFTP is seriously broken.
=> tftp 100000 bzImage Speed: 100, full duplex Using pch_gbe device TFTP from server 10.10.0.8; our IP address is 10.10.0.100; sending through gateway 10.10.0.1 Filename 'bzImage'. Load address: 0x100000 Loading: T TFTP error: 'Unsupported option(s) requested' (8) Starting again
I'm guessing you are having an issue that the TFTP server you are using for some reason does not allow a timeout as small as 10 ms. What server are you testing against? Can you experiment and find the lowest that it accepts? Maybe there is a compromise we can reach that still works with most (all?) servers and also improves the behavior in a lossy environment.
I am using a CentOS server, with /etc/xinetd.d/tftp configuration below.
service tftp { socket_type = dgram protocol = udp wait = yes user = root server = /usr/sbin/in.tftpd server_args = -s /tftpboot disable = no per_source = 11 cps = 100 2 flags = IPv4 }
I don't see an entry to change timeout settings.
I didn't mean to change the server settings; I meant can you adjust the U-Boot code until your server doesn't reject the option?
I would like to revert this commit before we find a solution. Even if I find a proper value to get tftp work again in my network environment, we don't know if this commit breaks someone else's board. After all, this timeout value has been there for years (?), and I believe it is a safe value for all the boards that are actively maintained.
[snip]
Regards, Bin

Hi,
i've same problem.
Running tftp-hpa on Linux Mint 17.
cat /etc/default/tftpd-hpa # /etc/default/tftpd-hpa RUN_DAEMON="yes" TFTP_USERNAME="tftp" TFTP_DIRECTORY="/tftpboot/tseries" TFTP_ADDRESS="0.0.0.0:69" TFTP_OPTIONS="-l -s"
best regards, Hannes
On 25.08.2015 09:40, Bin Meng wrote:
Hi Joe,
On Tue, Aug 25, 2015 at 12:24 PM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Mon, Aug 24, 2015 at 11:05 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Joe,
On Tue, Aug 25, 2015 at 11:42 AM, Joe Hershberger joe.hershberger@gmail.com wrote:
Hi Bin,
On Mon, Aug 24, 2015 at 9:25 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi,
With latest u-boot/master, TFTP is seriously broken.
=> tftp 100000 bzImage Speed: 100, full duplex Using pch_gbe device TFTP from server 10.10.0.8; our IP address is 10.10.0.100; sending through gateway 10.10.0.1 Filename 'bzImage'. Load address: 0x100000 Loading: T TFTP error: 'Unsupported option(s) requested' (8) Starting again
I'm guessing you are having an issue that the TFTP server you are using for some reason does not allow a timeout as small as 10 ms. What server are you testing against? Can you experiment and find the lowest that it accepts? Maybe there is a compromise we can reach that still works with most (all?) servers and also improves the behavior in a lossy environment.
I am using a CentOS server, with /etc/xinetd.d/tftp configuration below.
service tftp { socket_type = dgram protocol = udp wait = yes user = root server = /usr/sbin/in.tftpd server_args = -s /tftpboot disable = no per_source = 11 cps = 100 2 flags = IPv4 }
I don't see an entry to change timeout settings.
I didn't mean to change the server settings; I meant can you adjust the U-Boot code until your server doesn't reject the option?
I would like to revert this commit before we find a solution. Even if I find a proper value to get tftp work again in my network environment, we don't know if this commit breaks someone else's board. After all, this timeout value has been there for years (?), and I believe it is a safe value for all the boards that are actively maintained.
[snip]
Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot

On Tue 2015-08-25 14:13:17, Hannes Schmelzer wrote:
Hi,
i've same problem.
Running tftp-hpa on Linux Mint 17.
cat /etc/default/tftpd-hpa # /etc/default/tftpd-hpa RUN_DAEMON="yes" TFTP_USERNAME="tftp" TFTP_DIRECTORY="/tftpboot/tseries" TFTP_ADDRESS="0.0.0.0:69" TFTP_OPTIONS="-l -s"
Yes. Please try patch I mailed half an hour ago... it should fix that. (Actually here it is again, without changelog).
Sorry for the trouble, Pavel
Date: Tue, 25 Aug 2015 13:44:25 +0200 From: Pavel Machek pavel@denx.de To: Bin Meng bmeng.cn@gmail.com Cc: Joe Hershberger joe.hershberger@ni.com, Tom Rini trini@konsulko.com, U-Boot Mailing List u-boot@lists.denx.de Subject: [PATCH] Change default tftp timeout to be rfc-compliant
diff --git a/net/tftp.c b/net/tftp.c index 18ce84c..e919638 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -18,8 +18,9 @@
/* Well known TFTP port # */ #define WELL_KNOWN_PORT 69 -/* Millisecs to timeout for lost pkt */ -#define TIMEOUT 100UL +/* Millisecs to timeout for lost pkt. Anything below 1000msec is against RFC, and + some servers will refuse it. */ +#define TIMEOUT 1000UL #ifndef CONFIG_NET_RETRY_COUNT /* # of timeouts before giving up */ # define TIMEOUT_COUNT 1000

Hi Pavel,
this works for me.
many thanks. I will looking for some patch coming soon.
best regards, Hannes
On 25.08.2015 14:19, Pavel Machek wrote:
On Tue 2015-08-25 14:13:17, Hannes Schmelzer wrote:
Hi,
i've same problem.
Running tftp-hpa on Linux Mint 17.
cat /etc/default/tftpd-hpa # /etc/default/tftpd-hpa RUN_DAEMON="yes" TFTP_USERNAME="tftp" TFTP_DIRECTORY="/tftpboot/tseries" TFTP_ADDRESS="0.0.0.0:69" TFTP_OPTIONS="-l -s"
Yes. Please try patch I mailed half an hour ago... it should fix that. (Actually here it is again, without changelog).
Sorry for the trouble, Pavel
Date: Tue, 25 Aug 2015 13:44:25 +0200 From: Pavel Machek pavel@denx.de To: Bin Meng bmeng.cn@gmail.com Cc: Joe Hershberger joe.hershberger@ni.com, Tom Rini trini@konsulko.com, U-Boot Mailing List u-boot@lists.denx.de Subject: [PATCH] Change default tftp timeout to be rfc-compliant
diff --git a/net/tftp.c b/net/tftp.c index 18ce84c..e919638 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -18,8 +18,9 @@
/* Well known TFTP port # */ #define WELL_KNOWN_PORT 69 -/* Millisecs to timeout for lost pkt */ -#define TIMEOUT 100UL +/* Millisecs to timeout for lost pkt. Anything below 1000msec is against RFC, and
- some servers will refuse it. */
+#define TIMEOUT 1000UL #ifndef CONFIG_NET_RETRY_COUNT /* # of timeouts before giving up */ # define TIMEOUT_COUNT 1000

Hi Pavel,
On Tue, 2015-08-25 at 14:19 +0200, Pavel Machek wrote:
On Tue 2015-08-25 14:13:17, Hannes Schmelzer wrote:
Hi,
i've same problem.
Running tftp-hpa on Linux Mint 17.
cat /etc/default/tftpd-hpa # /etc/default/tftpd-hpa RUN_DAEMON="yes" TFTP_USERNAME="tftp" TFTP_DIRECTORY="/tftpboot/tseries" TFTP_ADDRESS="0.0.0.0:69" TFTP_OPTIONS="-l -s"
Yes. Please try patch I mailed half an hour ago... it should fix that. (Actually here it is again, without changelog).
Sorry for the trouble, Pavel
I'm wondering if there's a chance to fix that obvious regression anytime soon?
We're approaching v2015.10 release and with today's master all my boards fail on tftp load with mentioned: -------------------->8------------------- # tftp Speed: 100, full duplex Using ethernet@f000a000 device TFTP from server 10.225.15.67; our IP address is 10.225.15.11 Filename 'uImage'. Load address: 0x82000000 Loading: T T TFTP error: 'Unsupported option(s) requested' (8) -------------------->8-------------------
If there's no proper fix for now then we do need to revert http://git.denx.de/?p=u-boot.git;a=commit;h=620776d734e4b126c407f636bda825a5...
-Alexey

Hi Alexey,
On Thu, Sep 10, 2015 at 5:26 PM, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Pavel,
On Tue, 2015-08-25 at 14:19 +0200, Pavel Machek wrote:
On Tue 2015-08-25 14:13:17, Hannes Schmelzer wrote:
Hi,
i've same problem.
Running tftp-hpa on Linux Mint 17.
cat /etc/default/tftpd-hpa # /etc/default/tftpd-hpa RUN_DAEMON="yes" TFTP_USERNAME="tftp" TFTP_DIRECTORY="/tftpboot/tseries" TFTP_ADDRESS="0.0.0.0:69" TFTP_OPTIONS="-l -s"
Yes. Please try patch I mailed half an hour ago... it should fix that. (Actually here it is again, without changelog).
Sorry for the trouble, Pavel
I'm wondering if there's a chance to fix that obvious regression anytime soon?
This is already in u-boot-x86/master here: http://git.denx.de/?p=u-boot/u-boot-x86.git;a=commit;h=527ba17bfd058bd87d5ed...
We're approaching v2015.10 release and with today's master all my boards fail on tftp load with mentioned:
Sounds like Simon is soon sending a pull request... http://u-boot.10912.n7.nabble.com/Any-more-patches-for-x86-td227547.html
Cheers, -Joe

Hi,
On 10 September 2015 at 15:26, Alexey Brodkin Alexey.Brodkin@synopsys.com wrote:
Hi Pavel,
On Tue, 2015-08-25 at 14:19 +0200, Pavel Machek wrote:
On Tue 2015-08-25 14:13:17, Hannes Schmelzer wrote:
Hi,
i've same problem.
Running tftp-hpa on Linux Mint 17.
cat /etc/default/tftpd-hpa # /etc/default/tftpd-hpa RUN_DAEMON="yes" TFTP_USERNAME="tftp" TFTP_DIRECTORY="/tftpboot/tseries" TFTP_ADDRESS="0.0.0.0:69" TFTP_OPTIONS="-l -s"
Yes. Please try patch I mailed half an hour ago... it should fix that. (Actually here it is again, without changelog).
Sorry for the trouble, Pavel
I'm wondering if there's a chance to fix that obvious regression anytime soon?
We're approaching v2015.10 release and with today's master all my boards fail on tftp load with mentioned: -------------------->8------------------- # tftp Speed: 100, full duplex Using ethernet@f000a000 device TFTP from server 10.225.15.67; our IP address is 10.225.15.11 Filename 'uImage'. Load address: 0x82000000 Loading: T T TFTP error: 'Unsupported option(s) requested' (8) -------------------->8-------------------
If there's no proper fix for now then we do need to revert http://git.denx.de/?p=u-boot.git;a=commit;h=620776d734e4b126c407f636bda825a5...
Please test on u-boot-x86/master - I just sent a pull request.
Regards, Simon

On Tue 2015-08-25 10:25:35, Bin Meng wrote:
Hi,
With latest u-boot/master, TFTP is seriously broken.
Sorry about that.
=> tftp 100000 bzImage Speed: 100, full duplex Using pch_gbe device TFTP from server 10.10.0.8; our IP address is 10.10.0.100; sending through gateway 10.10.0.1 Filename 'bzImage'. Load address: 0x100000 Loading: T TFTP error: 'Unsupported option(s) requested' (8) Starting again
git bisect shows the following commit broke the TFTP
I was not intentionally changing the packets being sent.
Could you try to capture tcpdump, including contents of packet, ideally in both working and broken sessions?
Could you try setting
-#define TIMEOUT 5000UL +#define TIMEOUT 100UL
in net/tftp.c to something like 1000UL?
Thanks, Pavel

Hi Pavel,
On Tue, Aug 25, 2015 at 3:12 PM, Pavel Machek pavel@denx.de wrote:
On Tue 2015-08-25 10:25:35, Bin Meng wrote:
Hi,
With latest u-boot/master, TFTP is seriously broken.
Sorry about that.
=> tftp 100000 bzImage Speed: 100, full duplex Using pch_gbe device TFTP from server 10.10.0.8; our IP address is 10.10.0.100; sending through gateway 10.10.0.1 Filename 'bzImage'. Load address: 0x100000 Loading: T TFTP error: 'Unsupported option(s) requested' (8) Starting again
git bisect shows the following commit broke the TFTP
I was not intentionally changing the packets being sent.
Could you try to capture tcpdump, including contents of packet, ideally in both working and broken sessions?
Yep, I will do that. I also tested a tftpd Windows server (http://tftpd32.jounin.net) and it looks that this Windows server is immune to this commit.
Could you try setting
-#define TIMEOUT 5000UL +#define TIMEOUT 100UL
in net/tftp.c to something like 1000UL?
Could you please elaborate more on what this commit is trying to fix, or improve? From the commit message, I don't quite understand "With 5 seconds timeout, 10 retries maximum, tftp is impossible even on local network with single full-speed TCP connection".
Regards, Bin

On Tue, Aug 25, 2015 at 4:58 PM, Bin Meng bmeng.cn@gmail.com wrote:
Hi Pavel,
On Tue, Aug 25, 2015 at 3:12 PM, Pavel Machek pavel@denx.de wrote:
On Tue 2015-08-25 10:25:35, Bin Meng wrote:
Hi,
With latest u-boot/master, TFTP is seriously broken.
Sorry about that.
=> tftp 100000 bzImage Speed: 100, full duplex Using pch_gbe device TFTP from server 10.10.0.8; our IP address is 10.10.0.100; sending through gateway 10.10.0.1 Filename 'bzImage'. Load address: 0x100000 Loading: T TFTP error: 'Unsupported option(s) requested' (8) Starting again
git bisect shows the following commit broke the TFTP
I was not intentionally changing the packets being sent.
Could you try to capture tcpdump, including contents of packet, ideally in both working and broken sessions?
Yep, I will do that. I also tested a tftpd Windows server (http://tftpd32.jounin.net) and it looks that this Windows server is immune to this commit.
Could you try setting
-#define TIMEOUT 5000UL +#define TIMEOUT 100UL
in net/tftp.c to something like 1000UL?
Could you please elaborate more on what this commit is trying to fix, or improve? From the commit message, I don't quite understand "With 5 seconds timeout, 10 retries maximum, tftp is impossible even on local network with single full-speed TCP connection".
OK, I am now pretty sure this commit should be reverted. This commit is a spec violation to RFC 2349 in which it defines the timeout minimum value should be 1.
With this commit change, we set timeout to 100ms which creates a wrong option timeout value = 0, when sending tftp read request to the server. The Linux xinitd server strictly follows the RFC and reports 'Unsupported option(s) requested' (8) while the Windows server is quite forgiving.
Regards, Bin

On Tue 2015-08-25 16:58:58, Bin Meng wrote:
Hi Pavel,
On Tue, Aug 25, 2015 at 3:12 PM, Pavel Machek pavel@denx.de wrote:
On Tue 2015-08-25 10:25:35, Bin Meng wrote:
Hi,
With latest u-boot/master, TFTP is seriously broken.
Sorry about that.
=> tftp 100000 bzImage Speed: 100, full duplex Using pch_gbe device TFTP from server 10.10.0.8; our IP address is 10.10.0.100; sending through gateway 10.10.0.1 Filename 'bzImage'. Load address: 0x100000 Loading: T TFTP error: 'Unsupported option(s) requested' (8) Starting again
git bisect shows the following commit broke the TFTP
I was not intentionally changing the packets being sent.
Could you try to capture tcpdump, including contents of packet, ideally in both working and broken sessions?
Yep, I will do that. I also tested a tftpd Windows server (http://tftpd32.jounin.net) and it looks that this Windows server is immune to this commit.
Could you try setting
-#define TIMEOUT 5000UL +#define TIMEOUT 100UL
in net/tftp.c to something like 1000UL?
Could you please elaborate more on what this commit is trying to fix, or improve? From the commit message, I don't quite understand "With 5 seconds timeout, 10 retries maximum, tftp is impossible even on local network with single full-speed TCP connection".
Well, with 10 retries maximum, I get enough packet loss on local network that tftp fails.
Now, longer timeout will make it slower, but not fail. So "max retries" is really critical.
If rfc says minimum is 1sec, we should not really default to 100msec, sorry about that. Can you try with 1sec if it works for you?
Thanks and best regards, Pavel

tftp timeout of 100msec gives good performance on local ethernet, but some servers (Centos) refuse to operate, and it is against RFC 2349.
This fixes regression caused by 620776d734e4b126c407f636bda825a594a17723 .
Signed-off-by: Pavel Machek pavel@denx.de
diff --git a/net/tftp.c b/net/tftp.c index 18ce84c..e919638 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -18,8 +18,9 @@
/* Well known TFTP port # */ #define WELL_KNOWN_PORT 69 -/* Millisecs to timeout for lost pkt */ -#define TIMEOUT 100UL +/* Millisecs to timeout for lost pkt. Anything below 1000msec is against RFC, and + some servers will refuse it. */ +#define TIMEOUT 1000UL #ifndef CONFIG_NET_RETRY_COUNT /* # of timeouts before giving up */ # define TIMEOUT_COUNT 1000

Hi Pavel, Joe,
On Tue, Aug 25, 2015 at 7:44 PM, Pavel Machek pavel@denx.de wrote:
tftp timeout of 100msec gives good performance on local ethernet, but some servers (Centos) refuse to operate, and it is against RFC 2349.
This fixes regression caused by 620776d734e4b126c407f636bda825a594a17723 .
This patch does not fix the issue properly. As the commit 620776d also changed the "<1000" test logic to "<10", which should not be. See my comments below.
Signed-off-by: Pavel Machek pavel@denx.de
diff --git a/net/tftp.c b/net/tftp.c index 18ce84c..e919638 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -18,8 +18,9 @@
/* Well known TFTP port # */ #define WELL_KNOWN_PORT 69 -/* Millisecs to timeout for lost pkt */ -#define TIMEOUT 100UL +/* Millisecs to timeout for lost pkt. Anything below 1000msec is against RFC, and
- some servers will refuse it. */
Nits: please use correct multi-line comment format.
+#define TIMEOUT 1000UL #ifndef CONFIG_NET_RETRY_COUNT /* # of timeouts before giving up */ # define TIMEOUT_COUNT 1000
--
I still would like to revert commit 620776d (IOW, apply my revert patch @ http://patchwork.ozlabs.org/patch/510389/). Then Pavel to submit a new patch to change only TIMEOUT_COUNT to something larger (I am still not convinced that we need change the retry count from 10 to 1000). Perhaps with a better comment in the codes to explain why a larger TIMEOUT_COUNT is needed.
Regards, Bin

On Tue 2015-08-25 21:03:26, Bin Meng wrote:
Hi Pavel, Joe,
On Tue, Aug 25, 2015 at 7:44 PM, Pavel Machek pavel@denx.de wrote:
tftp timeout of 100msec gives good performance on local ethernet, but some servers (Centos) refuse to operate, and it is against RFC 2349.
This fixes regression caused by 620776d734e4b126c407f636bda825a594a17723 .
This patch does not fix the issue properly. As the commit 620776d also changed the "<1000" test logic to "<10", which should not be. See my comments below.
Yes, I know.. and I'd like the test logic to stay. Some tftp servers can handle that, and performance is significantly better that way.
Best regards, Pavel
I still would like to revert commit 620776d (IOW, apply my revert patch @ http://patchwork.ozlabs.org/patch/510389/). Then Pavel to submit a new patch to change only TIMEOUT_COUNT to something larger (I am still not convinced that we need change the retry count from 10 to 1000). Perhaps with a better comment in the codes to explain why a larger TIMEOUT_COUNT is needed.
Regards, Bin

On Tue, Aug 25, 2015 at 04:32:48PM +0200, Pavel Machek wrote:
On Tue 2015-08-25 21:03:26, Bin Meng wrote:
Hi Pavel, Joe,
On Tue, Aug 25, 2015 at 7:44 PM, Pavel Machek pavel@denx.de wrote:
tftp timeout of 100msec gives good performance on local ethernet, but some servers (Centos) refuse to operate, and it is against RFC 2349.
This fixes regression caused by 620776d734e4b126c407f636bda825a594a17723 .
This patch does not fix the issue properly. As the commit 620776d also changed the "<1000" test logic to "<10", which should not be. See my comments below.
Yes, I know.. and I'd like the test logic to stay. Some tftp servers can handle that, and performance is significantly better that way.
Well, what does the RFC say we can and cannot do here?

On Tue 2015-08-25 10:49:10, Tom Rini wrote:
On Tue, Aug 25, 2015 at 04:32:48PM +0200, Pavel Machek wrote:
On Tue 2015-08-25 21:03:26, Bin Meng wrote:
Hi Pavel, Joe,
On Tue, Aug 25, 2015 at 7:44 PM, Pavel Machek pavel@denx.de wrote:
tftp timeout of 100msec gives good performance on local ethernet, but some servers (Centos) refuse to operate, and it is against RFC 2349.
This fixes regression caused by 620776d734e4b126c407f636bda825a594a17723 .
This patch does not fix the issue properly. As the commit 620776d also changed the "<1000" test logic to "<10", which should not be. See my comments below.
Yes, I know.. and I'd like the test logic to stay. Some tftp servers can handle that, and performance is significantly better that way.
Well, what does the RFC say we can and cannot do here?
According to RFC, we should not be putting 0 there.
Best regards, Pavel
http://www.rfc-base.org/txt/rfc-2349.txt
#secs The number of seconds to wait before retransmitting, specified in ASCII. Valid values range between "1" and "255" seconds, inclusive. This is a NULL-terminated field.
participants (7)
-
Alexey Brodkin
-
Bin Meng
-
Hannes Schmelzer
-
Joe Hershberger
-
Pavel Machek
-
Simon Glass
-
Tom Rini