| Message ID | 1440739559-16225-1-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
Hi, On 27 August 2015 at 23:25, Bin Meng <bmeng.cn@gmail.com> wrote: > Commit 620776d "tftp: adjust settings to be suitable for 100Mbit ethernet" > causes the following error message when trying to load a file using 'tftp' > command via a tftp server. > > TFTP error: 'Unsupported option(s) requested' (8) > > This is due to with commit 620776d changes, the tftp option 'timeout' > value is now set to zero which is an invalid value as per RFC2349 [1]. > Valid values range between "1" and "255" seconds, inclusive. With some > tftp servers that strictly implement the RFC requirement, it reports > such an error message. > > Revert commit 620776d for RFC compliance. > > [1] https://www.ietf.org/rfc/rfc2349.txt > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > Acked-by: Joe Hershberger <joe.hershberger@ni.com> > > --- > > Changes in v3: > - Drop e1000 build warning patch which is already applied > > Changes in v2: > - Rewrite the commit message to mention RFC2349 This series is assigned to me in patchwork. Since the series seems to consist of bug fixes and driver model conversion of a driver with no other users, I'm inclined to apply it now. Please let me know if you disagree. Regards, Simon
Hi Simon, On Sun, Aug 30, 2015 at 5:45 PM, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On 27 August 2015 at 23:25, Bin Meng <bmeng.cn@gmail.com> wrote: >> Commit 620776d "tftp: adjust settings to be suitable for 100Mbit ethernet" >> causes the following error message when trying to load a file using 'tftp' >> command via a tftp server. >> >> TFTP error: 'Unsupported option(s) requested' (8) >> >> This is due to with commit 620776d changes, the tftp option 'timeout' >> value is now set to zero which is an invalid value as per RFC2349 [1]. >> Valid values range between "1" and "255" seconds, inclusive. With some >> tftp servers that strictly implement the RFC requirement, it reports >> such an error message. >> >> Revert commit 620776d for RFC compliance. >> >> [1] https://www.ietf.org/rfc/rfc2349.txt >> >> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >> Acked-by: Joe Hershberger <joe.hershberger@ni.com> >> >> --- >> >> Changes in v3: >> - Drop e1000 build warning patch which is already applied >> >> Changes in v2: >> - Rewrite the commit message to mention RFC2349 > > This series is assigned to me in patchwork. Since the series seems to > consist of bug fixes and driver model conversion of a driver with no > other users, I'm inclined to apply it now. > > Please let me know if you disagree. I'm fine with that. -Joe
On 31 August 2015 at 08:38, Joe Hershberger <joe.hershberger@gmail.com> wrote: > Hi Simon, > > On Sun, Aug 30, 2015 at 5:45 PM, Simon Glass <sjg@chromium.org> wrote: >> Hi, >> >> On 27 August 2015 at 23:25, Bin Meng <bmeng.cn@gmail.com> wrote: >>> Commit 620776d "tftp: adjust settings to be suitable for 100Mbit ethernet" >>> causes the following error message when trying to load a file using 'tftp' >>> command via a tftp server. >>> >>> TFTP error: 'Unsupported option(s) requested' (8) >>> >>> This is due to with commit 620776d changes, the tftp option 'timeout' >>> value is now set to zero which is an invalid value as per RFC2349 [1]. >>> Valid values range between "1" and "255" seconds, inclusive. With some >>> tftp servers that strictly implement the RFC requirement, it reports >>> such an error message. >>> >>> Revert commit 620776d for RFC compliance. >>> >>> [1] https://www.ietf.org/rfc/rfc2349.txt >>> >>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>> Acked-by: Joe Hershberger <joe.hershberger@ni.com> >>> >>> --- >>> >>> Changes in v3: >>> - Drop e1000 build warning patch which is already applied >>> >>> Changes in v2: >>> - Rewrite the commit message to mention RFC2349 >> >> This series is assigned to me in patchwork. Since the series seems to >> consist of bug fixes and driver model conversion of a driver with no >> other users, I'm inclined to apply it now. >> >> Please let me know if you disagree. > > I'm fine with that. > > -Joe Applied to u-boot-x86, thanks!
Hello Bin, Am 28.08.2015 um 07:25 schrieb Bin Meng: > Commit 620776d "tftp: adjust settings to be suitable for 100Mbit ethernet" > causes the following error message when trying to load a file using 'tftp' > command via a tftp server. > > TFTP error: 'Unsupported option(s) requested' (8) > > This is due to with commit 620776d changes, the tftp option 'timeout' > value is now set to zero which is an invalid value as per RFC2349 [1]. > Valid values range between "1" and "255" seconds, inclusive. With some > tftp servers that strictly implement the RFC requirement, it reports > such an error message. > > Revert commit 620776d for RFC compliance. > > [1] https://www.ietf.org/rfc/rfc2349.txt > > Signed-off-by: Bin Meng <bmeng.cn@gmail.com> > Acked-by: Joe Hershberger <joe.hershberger@ni.com> Just tried current mainline on the smartweb board, and had the same issue. Your patch fixed it, thanks! Tested-by: Heiko Schocher <hs@denx.de> bye, Heiko > > --- > > Changes in v3: > - Drop e1000 build warning patch which is already applied > > Changes in v2: > - Rewrite the commit message to mention RFC2349 > > net/tftp.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/net/tftp.c b/net/tftp.c > index 18ce84c..89be32a 100644 > --- a/net/tftp.c > +++ b/net/tftp.c > @@ -19,10 +19,10 @@ > /* Well known TFTP port # */ > #define WELL_KNOWN_PORT 69 > /* Millisecs to timeout for lost pkt */ > -#define TIMEOUT 100UL > +#define TIMEOUT 5000UL > #ifndef CONFIG_NET_RETRY_COUNT > /* # of timeouts before giving up */ > -# define TIMEOUT_COUNT 1000 > +# define TIMEOUT_COUNT 10 > #else > # define TIMEOUT_COUNT (CONFIG_NET_RETRY_COUNT * 2) > #endif > @@ -711,10 +711,10 @@ void tftp_start(enum proto_t protocol) > if (ep != NULL) > timeout_ms = simple_strtol(ep, NULL, 10); > > - if (timeout_ms < 10) { > - printf("TFTP timeout (%ld ms) too low, set min = 10 ms\n", > + if (timeout_ms < 1000) { > + printf("TFTP timeout (%ld ms) too low, set min = 1000 ms\n", > timeout_ms); > - timeout_ms = 10; > + timeout_ms = 1000; > } > > debug("TFTP blocksize = %i, timeout = %ld ms\n", >
Hi, what about: https://patchwork.ozlabs.org/patch/510476/ best regards, Hannes On 08.09.2015 11:23, Heiko Schocher wrote: > Hello Bin, > > Am 28.08.2015 um 07:25 schrieb Bin Meng: >> Commit 620776d "tftp: adjust settings to be suitable for 100Mbit >> ethernet" >> causes the following error message when trying to load a file using >> 'tftp' >> command via a tftp server. >> >> TFTP error: 'Unsupported option(s) requested' (8) >> >> This is due to with commit 620776d changes, the tftp option 'timeout' >> value is now set to zero which is an invalid value as per RFC2349 [1]. >> Valid values range between "1" and "255" seconds, inclusive. With some >> tftp servers that strictly implement the RFC requirement, it reports >> such an error message. >> >> Revert commit 620776d for RFC compliance. >> >> [1] https://www.ietf.org/rfc/rfc2349.txt >> >> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >> Acked-by: Joe Hershberger <joe.hershberger@ni.com> > > Just tried current mainline on the smartweb board, and had the > same issue. Your patch fixed it, thanks! > > Tested-by: Heiko Schocher <hs@denx.de> > > bye, > Heiko >> >> --- >> >> Changes in v3: >> - Drop e1000 build warning patch which is already applied >> >> Changes in v2: >> - Rewrite the commit message to mention RFC2349 >> >> net/tftp.c | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/net/tftp.c b/net/tftp.c >> index 18ce84c..89be32a 100644 >> --- a/net/tftp.c >> +++ b/net/tftp.c >> @@ -19,10 +19,10 @@ >> /* Well known TFTP port # */ >> #define WELL_KNOWN_PORT 69 >> /* Millisecs to timeout for lost pkt */ >> -#define TIMEOUT 100UL >> +#define TIMEOUT 5000UL >> #ifndef CONFIG_NET_RETRY_COUNT >> /* # of timeouts before giving up */ >> -# define TIMEOUT_COUNT 1000 >> +# define TIMEOUT_COUNT 10 >> #else >> # define TIMEOUT_COUNT (CONFIG_NET_RETRY_COUNT * 2) >> #endif >> @@ -711,10 +711,10 @@ void tftp_start(enum proto_t protocol) >> if (ep != NULL) >> timeout_ms = simple_strtol(ep, NULL, 10); >> >> - if (timeout_ms < 10) { >> - printf("TFTP timeout (%ld ms) too low, set min = 10 ms\n", >> + if (timeout_ms < 1000) { >> + printf("TFTP timeout (%ld ms) too low, set min = 1000 ms\n", >> timeout_ms); >> - timeout_ms = 10; >> + timeout_ms = 1000; >> } >> >> debug("TFTP blocksize = %i, timeout = %ld ms\n", >> >
Hi Hannes, On Tue, Sep 8, 2015 at 6:13 PM, Hannes Schmelzer <hannes@schmelzer.or.at> wrote: > Hi, > > what about: > https://patchwork.ozlabs.org/patch/510476/ > > best regards, > Hannes > > The patch you mentioned does not completely revert the changes. It was a mix of partial revert plus previous tftp timeout changes. I believe Pavel will need to work out a proper patch to resolve the tftp timeout issue he sees in his network environment. But that is for the next release (v2016.01), not for this release. > > On 08.09.2015 11:23, Heiko Schocher wrote: >> >> Hello Bin, >> >> Am 28.08.2015 um 07:25 schrieb Bin Meng: >>> >>> Commit 620776d "tftp: adjust settings to be suitable for 100Mbit >>> ethernet" >>> causes the following error message when trying to load a file using >>> 'tftp' >>> command via a tftp server. >>> >>> TFTP error: 'Unsupported option(s) requested' (8) >>> >>> This is due to with commit 620776d changes, the tftp option 'timeout' >>> value is now set to zero which is an invalid value as per RFC2349 [1]. >>> Valid values range between "1" and "255" seconds, inclusive. With some >>> tftp servers that strictly implement the RFC requirement, it reports >>> such an error message. >>> >>> Revert commit 620776d for RFC compliance. >>> >>> [1] https://www.ietf.org/rfc/rfc2349.txt >>> >>> Signed-off-by: Bin Meng <bmeng.cn@gmail.com> >>> Acked-by: Joe Hershberger <joe.hershberger@ni.com> >> >> >> Just tried current mainline on the smartweb board, and had the >> same issue. Your patch fixed it, thanks! >> >> Tested-by: Heiko Schocher <hs@denx.de> >> >> bye, >> Heiko >>> >>> >>> --- >>> >>> Changes in v3: >>> - Drop e1000 build warning patch which is already applied >>> >>> Changes in v2: >>> - Rewrite the commit message to mention RFC2349 >>> >>> net/tftp.c | 10 +++++----- >>> 1 file changed, 5 insertions(+), 5 deletions(-) >>> >>> diff --git a/net/tftp.c b/net/tftp.c >>> index 18ce84c..89be32a 100644 >>> --- a/net/tftp.c >>> +++ b/net/tftp.c >>> @@ -19,10 +19,10 @@ >>> /* Well known TFTP port # */ >>> #define WELL_KNOWN_PORT 69 >>> /* Millisecs to timeout for lost pkt */ >>> -#define TIMEOUT 100UL >>> +#define TIMEOUT 5000UL >>> #ifndef CONFIG_NET_RETRY_COUNT >>> /* # of timeouts before giving up */ >>> -# define TIMEOUT_COUNT 1000 >>> +# define TIMEOUT_COUNT 10 >>> #else >>> # define TIMEOUT_COUNT (CONFIG_NET_RETRY_COUNT * 2) >>> #endif >>> @@ -711,10 +711,10 @@ void tftp_start(enum proto_t protocol) >>> if (ep != NULL) >>> timeout_ms = simple_strtol(ep, NULL, 10); >>> >>> - if (timeout_ms < 10) { >>> - printf("TFTP timeout (%ld ms) too low, set min = 10 ms\n", >>> + if (timeout_ms < 1000) { >>> + printf("TFTP timeout (%ld ms) too low, set min = 1000 ms\n", >>> timeout_ms); >>> - timeout_ms = 10; >>> + timeout_ms = 1000; >>> } >>> >>> debug("TFTP blocksize = %i, timeout = %ld ms\n", >>> >> > Regards, Bin
Hi Hannes, On 08.09.2015 12:13, Hannes Schmelzer wrote: > what about: > https://patchwork.ozlabs.org/patch/510476/ I think the current plan is to first revert this patch for this release. To not introduce a risk of a new setup as done in the patch you are referencing above. This patch will most likely then be picked up after the release. So that it has more time to get tested before a release. BTW: I think that Simon has queued the revert in his repo (Simon, please correct me if I'm wrong). The revert should definitely hit mainline soon. I've been hit by this problem now multiple times already. Thanks, Stefan
Hi, okay ... just wanted to get sure that the patch has been noticed. best regards, Hannes On 08.09.2015 12:36, Stefan Roese wrote: > Hi Hannes, > > On 08.09.2015 12:13, Hannes Schmelzer wrote: >> what about: >> https://patchwork.ozlabs.org/patch/510476/ > > I think the current plan is to first revert this patch for this > release. To not introduce a risk of a new setup as done in the patch > you are referencing above. This patch will most likely then be picked > up after the release. So that it has more time to get tested before a > release. > > BTW: I think that Simon has queued the revert in his repo (Simon, > please correct me if I'm wrong). The revert should definitely hit > mainline soon. I've been hit by this problem now multiple times already. > > Thanks, > Stefan > >
diff --git a/net/tftp.c b/net/tftp.c index 18ce84c..89be32a 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -19,10 +19,10 @@ /* Well known TFTP port # */ #define WELL_KNOWN_PORT 69 /* Millisecs to timeout for lost pkt */ -#define TIMEOUT 100UL +#define TIMEOUT 5000UL #ifndef CONFIG_NET_RETRY_COUNT /* # of timeouts before giving up */ -# define TIMEOUT_COUNT 1000 +# define TIMEOUT_COUNT 10 #else # define TIMEOUT_COUNT (CONFIG_NET_RETRY_COUNT * 2) #endif @@ -711,10 +711,10 @@ void tftp_start(enum proto_t protocol) if (ep != NULL) timeout_ms = simple_strtol(ep, NULL, 10); - if (timeout_ms < 10) { - printf("TFTP timeout (%ld ms) too low, set min = 10 ms\n", + if (timeout_ms < 1000) { + printf("TFTP timeout (%ld ms) too low, set min = 1000 ms\n", timeout_ms); - timeout_ms = 10; + timeout_ms = 1000; } debug("TFTP blocksize = %i, timeout = %ld ms\n",
Commit 620776d "tftp: adjust settings to be suitable for 100Mbit ethernet" causes the following error message when trying to load a file using 'tftp' command via a tftp server. TFTP error: 'Unsupported option(s) requested' (8) This is due to with commit 620776d changes, the tftp option 'timeout' value is now set to zero which is an invalid value as per RFC2349 [1]. Valid values range between "1" and "255" seconds, inclusive. With some tftp servers that strictly implement the RFC requirement, it reports such an error message. Revert commit 620776d for RFC compliance. [1] https://www.ietf.org/rfc/rfc2349.txt Signed-off-by: Bin Meng <bmeng.cn@gmail.com> Acked-by: Joe Hershberger <joe.hershberger@ni.com> --- Changes in v3: - Drop e1000 build warning patch which is already applied Changes in v2: - Rewrite the commit message to mention RFC2349 net/tftp.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)