* [ECOS] Improving TFTP performance
@ 2007-12-19 12:41 Øyvind Harboe
2007-12-19 13:53 ` [ECOS] " Øyvind Harboe
0 siblings, 1 reply; 10+ messages in thread
From: Øyvind Harboe @ 2007-12-19 12:41 UTC (permalink / raw)
To: eCos Disuss
[-- Attachment #1: Type: text/plain, Size: 392 bytes --]
Has anyone done any work in improving TFTP get performance?
Block size is currently set to 512 whereas TFTP RFC 248 allows for
negotiating block size...
http://www.faqs.org/rfcs/rfc2348.html
Attached is some work in progress. It doesn't work, but including it
in case anyone
is interested in this.
--
Øyvind Harboe
http://www.zylin.com - eCos ARM & FPGA developer kit
[-- Attachment #2: tftpfaster.txt --]
[-- Type: text/plain, Size: 4234 bytes --]
### Eclipse Workspace Patch 1.0
#P ecos
Index: net/common/current/cdl/net.cdl
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/cdl/net.cdl,v
retrieving revision 1.17
diff -u -r1.17 net.cdl
--- net/common/current/cdl/net.cdl 7 Jan 2007 14:46:55 -0000 1.17
+++ net/common/current/cdl/net.cdl 19 Dec 2007 09:04:00 -0000
@@ -167,6 +167,16 @@
threads can have precedence over TFTP server processing."
}
+ cdl_option CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE {
+ display "Pervert TFTP protocol to allow bigger packets."
+ flavor data
+ default_value 512
+ legal_values 512 to 65464
+ description "
+ Some TFTP servers pragmatically allow increasing the
+ packet size, contrary to the TFTP standard. Use with caution!"
+ }
+
cdl_option CYGPKG_NET_TFTPD_THREAD_STACK_SIZE {
display "Stack size for TFTP threads."
flavor data
Index: net/common/current/src/tftp_client.c
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/src/tftp_client.c,v
retrieving revision 1.10
diff -u -r1.10 tftp_client.c
--- net/common/current/src/tftp_client.c 16 Sep 2005 14:56:26 -0000 1.10
+++ net/common/current/src/tftp_client.c 19 Dec 2007 09:04:01 -0000
@@ -57,6 +57,7 @@
#include <network.h>
#include <arpa/tftp.h>
#include <tftp_support.h>
+#include <stdlib.h>
#define min(x,y) (x<y ? x : y)
@@ -66,7 +67,8 @@
// On error, *err will hold the reason.
// This version uses the server name. This can be a name for DNS lookup
// or a dotty or colony number format for IPv4 or IPv6.
-int tftp_client_get(const char * const filename,
+static int tftp_client_get_inner(char *data,
+ const char * const filename,
const char * const server,
const int port,
char *buf,
@@ -85,7 +87,6 @@
int error;
struct sockaddr local_addr, from_addr;
- char data[SEGSIZE+sizeof(struct tftphdr)];
struct tftphdr *hdr = (struct tftphdr *)data;
const char *fp;
char *cp, *bp;
@@ -214,9 +215,9 @@
}
}
} else {
- recv_len = sizeof(data);
+ recv_len = CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE;
from_len = sizeof(from_addr);
- if ((data_len = recvfrom(s, &data, recv_len, 0,
+ if ((data_len = recvfrom(s, data, recv_len, 0,
&from_addr, &from_len)) < 0) {
// What happened?
*err = TFTP_NETERR;
@@ -244,7 +245,7 @@
// To prevent an out-of-sequence packet from
// terminating transmission prematurely, set
// actual_len to a full size packet.
- actual_len = SEGSIZE;
+ actual_len = CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE;
}
// Send out the ACK
hdr->th_opcode = htons(ACK);
@@ -256,7 +257,7 @@
goto out;
}
// A short packet marks the end of the file.
- if ((actual_len >= 0) && (actual_len < SEGSIZE)) {
+ if ((actual_len >= 0) && (actual_len < CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE)) {
// End of data
close(s);
freeaddrinfo(res);
@@ -290,6 +291,35 @@
freeaddrinfo(res);
return -1;
}
+
+
+int tftp_client_get(const char * const filename,
+ const char * const server,
+ const int port,
+ char *buf,
+ int len,
+ const int mode,
+ int * const err) {
+ int result;
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ char *data = malloc(CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE+sizeof(struct tftphdr));
+ if (data==NULL)
+ {
+ *err=TFTP_ENOSPACE;
+ return -1;
+ }
+#else
+ char data[SEGSIZE+sizeof(struct tftphdr)];
+#endif
+ result=tftp_client_get_inner(data, filename, server, port, buf, len, mode, err);
+
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ free(data);
+#endif
+
+ return result;
+}
+
//
// Read a file from a host into a local buffer. Returns the
// number of bytes actually read, or (-1) if an error occurs.
[-- Attachment #3: Type: text/plain, Size: 148 bytes --]
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 10+ messages in thread
* [ECOS] Re: Improving TFTP performance
2007-12-19 12:41 [ECOS] Improving TFTP performance Øyvind Harboe
@ 2007-12-19 13:53 ` Øyvind Harboe
2007-12-19 14:17 ` Andrew Lunn
2007-12-19 14:28 ` Sergei Gavrikov
0 siblings, 2 replies; 10+ messages in thread
From: Øyvind Harboe @ 2007-12-19 13:53 UTC (permalink / raw)
To: eCos Disuss
[-- Attachment #1: Type: text/plain, Size: 253 bytes --]
Take #2. Wrote up negotiation of segsize, passes first smoketest.
Anyone interesting helping to test/comment on the patch?
Tested w/this server: http://tftpd32.jounin.net
--
Øyvind Harboe
http://www.zylin.com - eCos ARM & FPGA developer kit
[-- Attachment #2: tftpfaster.txt --]
[-- Type: text/plain, Size: 6891 bytes --]
### Eclipse Workspace Patch 1.0
#P ecos
Index: net/common/current/cdl/net.cdl
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/cdl/net.cdl,v
retrieving revision 1.17
diff -u -r1.17 net.cdl
--- net/common/current/cdl/net.cdl 7 Jan 2007 14:46:55 -0000 1.17
+++ net/common/current/cdl/net.cdl 19 Dec 2007 10:36:38 -0000
@@ -167,6 +167,16 @@
threads can have precedence over TFTP server processing."
}
+ cdl_option CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE {
+ display "TFTP protocol allows negotiation of bigger packets."
+ flavor data
+ default_value 512
+ legal_values 512 to 65464
+ description "
+ tftp blksize egotiation support. >512 byte block sizes improves
+ tftp GET performance"
+ }
+
cdl_option CYGPKG_NET_TFTPD_THREAD_STACK_SIZE {
display "Stack size for TFTP threads."
flavor data
Index: net/common/current/include/arpa/tftp.h
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/include/arpa/tftp.h,v
retrieving revision 1.2
diff -u -r1.2 tftp.h
--- net/common/current/include/arpa/tftp.h 7 Aug 2002 14:42:35 -0000 1.2
+++ net/common/current/include/arpa/tftp.h 19 Dec 2007 10:36:38 -0000
@@ -70,6 +70,7 @@
#define DATA 03 /* data packet */
#define ACK 04 /* acknowledgement */
#define ERROR 05 /* error code */
+#define OACK 06 /* option acknowledge */
struct tftphdr {
short th_opcode; /* packet type */
Index: net/common/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/ChangeLog,v
retrieving revision 1.81
diff -u -r1.81 ChangeLog
--- net/common/current/ChangeLog 15 Jan 2007 18:37:52 -0000 1.81
+++ net/common/current/ChangeLog 19 Dec 2007 10:36:38 -0000
@@ -1,3 +1,8 @@
+2007-12-19 Oyvind Harboe <oyvind.harboe@zylin.com>
+
+ * src/tftp_client.c, include/arpa/tftp.h, cdl/net.cdl: tftp blksize
+ negotiation support. >512 byte block sizes improves tftp GET performance
+
2007-01-15 Gary Thomas <gary@mlbassoc.com>
* src/dhcp_support.c (dhcp_mgt_entry): Better handling when restarting
Index: net/common/current/src/tftp_client.c
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/src/tftp_client.c,v
retrieving revision 1.10
diff -u -r1.10 tftp_client.c
--- net/common/current/src/tftp_client.c 16 Sep 2005 14:56:26 -0000 1.10
+++ net/common/current/src/tftp_client.c 19 Dec 2007 10:36:38 -0000
@@ -57,6 +57,8 @@
#include <network.h>
#include <arpa/tftp.h>
#include <tftp_support.h>
+#include <stdlib.h>
+#include <stdio.h>
#define min(x,y) (x<y ? x : y)
@@ -66,14 +68,16 @@
// On error, *err will hold the reason.
// This version uses the server name. This can be a name for DNS lookup
// or a dotty or colony number format for IPv4 or IPv6.
-int tftp_client_get(const char * const filename,
+static int tftp_client_get_inner(char *data,
+ const char * const filename,
const char * const server,
const int port,
char *buf,
int len,
const int mode,
int * const err) {
-
+
+ int blksize=512;
int result = 0;
int s=-1;
int actual_len, data_len;
@@ -85,7 +89,6 @@
int error;
struct sockaddr local_addr, from_addr;
- char data[SEGSIZE+sizeof(struct tftphdr)];
struct tftphdr *hdr = (struct tftphdr *)data;
const char *fp;
char *cp, *bp;
@@ -112,6 +115,13 @@
}
while (*fp) *cp++ = *fp++;
*cp++ = '\0';
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ fp="blksize";
+ while (*fp) *cp++ = *fp++;
+ *cp++ = '\0';
+ cp+=sprintf(cp, "%d", CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE);
+ *cp++ = '\0';
+#endif
memset(&hints,0,sizeof(hints));
hints.ai_family = PF_UNSPEC;
@@ -214,15 +224,34 @@
}
}
} else {
- recv_len = sizeof(data);
+ recv_len = blksize;
from_len = sizeof(from_addr);
- if ((data_len = recvfrom(s, &data, recv_len, 0,
+ if ((data_len = recvfrom(s, data, recv_len, 0,
&from_addr, &from_len)) < 0) {
// What happened?
*err = TFTP_NETERR;
goto out;
}
- if (ntohs(hdr->th_opcode) == DATA) {
+ if (ntohs(hdr->th_opcode) == OACK) {
+ // We can have only *one* option, the one we sent..
+ if (strncmp(data+2, "blksize", from_len)==0)
+ {
+ blksize=atol(data+2+strlen("blksize")+1);
+ } else
+ {
+ // option ignored, use default.
+ }
+ // Send out the ACK
+ hdr->th_opcode = htons(ACK);
+ hdr->th_block = htons(last_good_block);
+ if (sendto(s, data, 4 /* FIXME */, 0,
+ &from_addr, from_len) < 0) {
+ // Problem sending request
+ *err = TFTP_NETERR;
+ goto out;
+ }
+
+ } else if (ntohs(hdr->th_opcode) == DATA) {
actual_len = 0;
if (ntohs(hdr->th_block) == (last_good_block+1)) {
// Consume this data
@@ -244,7 +273,7 @@
// To prevent an out-of-sequence packet from
// terminating transmission prematurely, set
// actual_len to a full size packet.
- actual_len = SEGSIZE;
+ actual_len = blksize;
}
// Send out the ACK
hdr->th_opcode = htons(ACK);
@@ -256,7 +285,8 @@
goto out;
}
// A short packet marks the end of the file.
- if ((actual_len >= 0) && (actual_len < SEGSIZE)) {
+ /* 4 = Sizeof TFTP header */
+ if ((actual_len >= 0) && (actual_len < (blksize-4))) {
// End of data
close(s);
freeaddrinfo(res);
@@ -290,6 +320,35 @@
freeaddrinfo(res);
return -1;
}
+
+
+int tftp_client_get(const char * const filename,
+ const char * const server,
+ const int port,
+ char *buf,
+ int len,
+ const int mode,
+ int * const err) {
+ int result;
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ char *data = malloc(CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE+sizeof(struct tftphdr));
+ if (data==NULL)
+ {
+ *err=TFTP_ENOSPACE;
+ return -1;
+ }
+#else
+ char data[SEGSIZE+sizeof(struct tftphdr)];
+#endif
+ result=tftp_client_get_inner(data, filename, server, port, buf, len, mode, err);
+
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ free(data);
+#endif
+
+ return result;
+}
+
//
// Read a file from a host into a local buffer. Returns the
// number of bytes actually read, or (-1) if an error occurs.
[-- Attachment #3: Type: text/plain, Size: 148 bytes --]
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ECOS] Re: Improving TFTP performance
2007-12-19 13:53 ` [ECOS] " Øyvind Harboe
@ 2007-12-19 14:17 ` Andrew Lunn
2007-12-19 14:47 ` Øyvind Harboe
2007-12-19 14:28 ` Sergei Gavrikov
1 sibling, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2007-12-19 14:17 UTC (permalink / raw)
To: ?yvind Harboe; +Cc: eCos Disuss
On Wed, Dec 19, 2007 at 11:38:22AM +0100, ?yvind Harboe wrote:
> Take #2. Wrote up negotiation of segsize, passes first smoketest.
> Anyone interesting helping to test/comment on the patch?
>
> Tested w/this server: http://tftpd32.jounin.net
>
> --
> ?yvind Harboe
> http://www.zylin.com - eCos ARM & FPGA developer kit
> ### Eclipse Workspace Patch 1.0
> #P ecos
> Index: net/common/current/cdl/net.cdl
> ===================================================================
> RCS file: /cvs/ecos/ecos-opt/net/net/common/current/cdl/net.cdl,v
> retrieving revision 1.17
> diff -u -r1.17 net.cdl
> --- net/common/current/cdl/net.cdl 7 Jan 2007 14:46:55 -0000 1.17
> +++ net/common/current/cdl/net.cdl 19 Dec 2007 10:36:38 -0000
> @@ -167,6 +167,16 @@
> threads can have precedence over TFTP server processing."
> }
>
> + cdl_option CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE {
> + display "TFTP protocol allows negotiation of bigger packets."
> + flavor data
> + default_value 512
> + legal_values 512 to 65464
> + description "
> + tftp blksize egotiation support. >512 byte block sizes improves
> + tftp GET performance"
> + }
You should comment that this requires a server which implements the
optional RFC 2348.
> + if (ntohs(hdr->th_opcode) == OACK) {
> + // We can have only *one* option, the one we sent..
> + if (strncmp(data+2, "blksize", from_len)==0)
> + {
> + blksize=atol(data+2+strlen("blksize")+1);
> + } else
> + {
> + // option ignored, use default.
> + }
> + // Send out the ACK
> + hdr->th_opcode = htons(ACK);
> + hdr->th_block = htons(last_good_block);
> + if (sendto(s, data, 4 /* FIXME */, 0,
> + &from_addr, from_len) < 0) {
> + // Problem sending request
> + *err = TFTP_NETERR;
> + goto out;
> + }
> +
> + }
This should be conditionally compiled.
> + int * const err) {
> + int result;
> +#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
> + char *data = malloc(CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE+sizeof(struct tftphdr));
No lines longer than 80 characters please.
You probably need to look at the case of the server returning ERROR
when you pass it the additional option during a get. You probably
should try the GET again without the option and see if it works.
Andrew
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ECOS] Re: Improving TFTP performance
2007-12-19 13:53 ` [ECOS] " Øyvind Harboe
2007-12-19 14:17 ` Andrew Lunn
@ 2007-12-19 14:28 ` Sergei Gavrikov
1 sibling, 0 replies; 10+ messages in thread
From: Sergei Gavrikov @ 2007-12-19 14:28 UTC (permalink / raw)
To: ?yvind Harboe; +Cc: eCos Disuss
On Wed, Dec 19, 2007 at 11:38:22AM +0100, ?yvind Harboe wrote:
> Take #2. Wrote up negotiation of segsize, passes first smoketest.
> Anyone interesting helping to test/comment on the patch?
>
> Tested w/this server: http://tftpd32.jounin.net
>
> --
> ?yvind Harboe
> http://www.zylin.com - eCos ARM & FPGA developer kit
What can you say about the getting performance? Do you have some results
to show it?
AFAIK, most of *nix guys cannot test this directly. Default TFTP server
for the most Linux distributions was a netkit-tftp. The NetKit tftpd
only supports RFC783. So, they have to install alternate TFTP server
(aka `atftpd'). That is multi-threaded TFTP server which implements all
extensions and multicast: RFC1350, RFC2090, RFC2347, RFC2348, RFC2349.
[RFC]
I would prefer to see in eCos net bag something like `atftp_client.c'
(advanced/alternate) and I would prefer to do not waste those K.I.S.S.
and RFC783 compliant tftp_client.c/tftp_server.c.
Sergei
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ECOS] Re: Improving TFTP performance
2007-12-19 14:17 ` Andrew Lunn
@ 2007-12-19 14:47 ` Øyvind Harboe
2007-12-19 15:11 ` Andrew Lunn
0 siblings, 1 reply; 10+ messages in thread
From: Øyvind Harboe @ 2007-12-19 14:47 UTC (permalink / raw)
To: eCos Disuss
[-- Attachment #1: Type: text/plain, Size: 286 bytes --]
I see a performance improvement from 150 to 400k/s on our system
w/packetsize 32768.
Note that the code should now be identical unless the maximum packet
size is changed
from 512 to something bigger.
--
Øyvind Harboe
http://www.zylin.com - eCos ARM & FPGA developer kit
[-- Attachment #2: tftpfaster2.txt --]
[-- Type: text/plain, Size: 7654 bytes --]
### Eclipse Workspace Patch 1.0
#P ecos
Index: net/common/current/cdl/net.cdl
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/cdl/net.cdl,v
retrieving revision 1.17
diff -u -r1.17 net.cdl
--- net/common/current/cdl/net.cdl 7 Jan 2007 14:46:55 -0000 1.17
+++ net/common/current/cdl/net.cdl 19 Dec 2007 13:51:55 -0000
@@ -167,6 +167,17 @@
threads can have precedence over TFTP server processing."
}
+ cdl_option CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE {
+ display "TFTP protocol allows negotiation of bigger packets.
+ Requires server which supports RFC 2348 blksize negotiation."
+ flavor data
+ default_value 512
+ legal_values 512 to 65464
+ description "
+ tftp blksize egotiation support. >512 byte block sizes improves
+ tftp GET performance"
+ }
+
cdl_option CYGPKG_NET_TFTPD_THREAD_STACK_SIZE {
display "Stack size for TFTP threads."
flavor data
Index: net/common/current/include/arpa/tftp.h
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/include/arpa/tftp.h,v
retrieving revision 1.2
diff -u -r1.2 tftp.h
--- net/common/current/include/arpa/tftp.h 7 Aug 2002 14:42:35 -0000 1.2
+++ net/common/current/include/arpa/tftp.h 19 Dec 2007 13:51:55 -0000
@@ -70,6 +70,7 @@
#define DATA 03 /* data packet */
#define ACK 04 /* acknowledgement */
#define ERROR 05 /* error code */
+#define OACK 06 /* option acknowledge */
struct tftphdr {
short th_opcode; /* packet type */
Index: net/common/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/ChangeLog,v
retrieving revision 1.81
diff -u -r1.81 ChangeLog
--- net/common/current/ChangeLog 15 Jan 2007 18:37:52 -0000 1.81
+++ net/common/current/ChangeLog 19 Dec 2007 13:51:55 -0000
@@ -1,3 +1,8 @@
+2007-12-19 Oyvind Harboe <oyvind.harboe@zylin.com>
+
+ * src/tftp_client.c, include/arpa/tftp.h, cdl/net.cdl: tftp blksize
+ negotiation support. >512 byte block sizes improves tftp GET performance
+
2007-01-15 Gary Thomas <gary@mlbassoc.com>
* src/dhcp_support.c (dhcp_mgt_entry): Better handling when restarting
Index: net/common/current/src/tftp_client.c
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/src/tftp_client.c,v
retrieving revision 1.10
diff -u -r1.10 tftp_client.c
--- net/common/current/src/tftp_client.c 16 Sep 2005 14:56:26 -0000 1.10
+++ net/common/current/src/tftp_client.c 19 Dec 2007 13:51:56 -0000
@@ -57,6 +57,8 @@
#include <network.h>
#include <arpa/tftp.h>
#include <tftp_support.h>
+#include <stdlib.h>
+#include <stdio.h>
#define min(x,y) (x<y ? x : y)
@@ -66,14 +68,17 @@
// On error, *err will hold the reason.
// This version uses the server name. This can be a name for DNS lookup
// or a dotty or colony number format for IPv4 or IPv6.
-int tftp_client_get(const char * const filename,
+static int tftp_client_get_inner(char *data,
+ const char * const filename,
const char * const server,
const int port,
char *buf,
int len,
const int mode,
- int * const err) {
-
+ int * const err,
+ int negotiate) {
+
+ int blksize=512;
int result = 0;
int s=-1;
int actual_len, data_len;
@@ -85,7 +90,6 @@
int error;
struct sockaddr local_addr, from_addr;
- char data[SEGSIZE+sizeof(struct tftphdr)];
struct tftphdr *hdr = (struct tftphdr *)data;
const char *fp;
char *cp, *bp;
@@ -112,6 +116,16 @@
}
while (*fp) *cp++ = *fp++;
*cp++ = '\0';
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ if (negotiate)
+ {
+ fp="blksize";
+ while (*fp) *cp++ = *fp++;
+ *cp++ = '\0';
+ cp+=sprintf(cp, "%d", CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE);
+ *cp++ = '\0';
+ }
+#endif
memset(&hints,0,sizeof(hints));
hints.ai_family = PF_UNSPEC;
@@ -214,15 +228,37 @@
}
}
} else {
- recv_len = sizeof(data);
+ recv_len = blksize+sizeof(struct tftphdr);
from_len = sizeof(from_addr);
- if ((data_len = recvfrom(s, &data, recv_len, 0,
+ if ((data_len = recvfrom(s, data, recv_len, 0,
&from_addr, &from_len)) < 0) {
// What happened?
*err = TFTP_NETERR;
goto out;
}
- if (ntohs(hdr->th_opcode) == DATA) {
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ if (ntohs(hdr->th_opcode) == OACK) {
+ // We can have only *one* option, the one we sent..
+ if (strncmp(data+2, "blksize", data_len)==0)
+ {
+ blksize=atol(data+2+strlen("blksize")+1);
+ } else
+ {
+ // option ignored, use default.
+ }
+ // Send out the ACK
+ hdr->th_opcode = htons(ACK);
+ hdr->th_block = htons(last_good_block);
+ if (sendto(s, data, 4 /* FIXME */, 0,
+ &from_addr, from_len) < 0) {
+ // Problem sending request
+ *err = TFTP_NETERR;
+ goto out;
+ }
+
+ } else
+#endif
+ if (ntohs(hdr->th_opcode) == DATA) {
actual_len = 0;
if (ntohs(hdr->th_block) == (last_good_block+1)) {
// Consume this data
@@ -244,7 +280,7 @@
// To prevent an out-of-sequence packet from
// terminating transmission prematurely, set
// actual_len to a full size packet.
- actual_len = SEGSIZE;
+ actual_len = blksize;
}
// Send out the ACK
hdr->th_opcode = htons(ACK);
@@ -256,7 +292,8 @@
goto out;
}
// A short packet marks the end of the file.
- if ((actual_len >= 0) && (actual_len < SEGSIZE)) {
+ /* 4 = Sizeof TFTP header */
+ if ((actual_len >= 0) && (actual_len < blksize)) {
// End of data
close(s);
freeaddrinfo(res);
@@ -290,6 +327,50 @@
freeaddrinfo(res);
return -1;
}
+
+
+int tftp_client_get(const char * const filename,
+ const char * const server,
+ const int port,
+ char *buf,
+ int len,
+ const int mode,
+ int * const err) {
+ int result;
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ char *data = malloc(CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE+
+ sizeof(struct tftphdr));
+ if (data==NULL)
+ {
+ *err=TFTP_ENOSPACE;
+ return -1;
+ }
+#else
+ char data[SEGSIZE+sizeof(struct tftphdr)];
+#endif
+ result=tftp_client_get_inner(data, filename, server,
+ port, buf, len, mode, err,
+ CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512);
+ if (result<0)
+ {
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ // try without negotiating packet size. The serves that do
+ // not support options negotiation would normally just ignore
+ // the options and thus this code path will probably never be
+ // executed
+ result=tftp_client_get_inner(data, filename, server,
+ port, buf, len, mode, err,
+ 0);
+#endif
+ }
+
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ free(data);
+#endif
+
+ return result;
+}
+
//
// Read a file from a host into a local buffer. Returns the
// number of bytes actually read, or (-1) if an error occurs.
[-- Attachment #3: Type: text/plain, Size: 148 bytes --]
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ECOS] Re: Improving TFTP performance
2007-12-19 14:47 ` Øyvind Harboe
@ 2007-12-19 15:11 ` Andrew Lunn
2007-12-19 15:34 ` Øyvind Harboe
0 siblings, 1 reply; 10+ messages in thread
From: Andrew Lunn @ 2007-12-19 15:11 UTC (permalink / raw)
To: ?yvind Harboe; +Cc: eCos Disuss
> + cdl_option CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE {
> + display "TFTP protocol allows negotiation of bigger packets.
> + Requires server which supports RFC 2348 blksize negotiation."
> + flavor data
> + default_value 512
> + legal_values 512 to 65464
> + description "
> + tftp blksize egotiation support. >512 byte block sizes improves
> + tftp GET performance"
> + }
The display should be kept to one line maximum. Put all the rest on
the description lines. You should also say that the default value of
512 causes this to be disabled. It might even be better to change the
flavor of this to booldata, so it can be enabled/disabled and the
value set. It then makes your code cleaner.
> + // try without negotiating packet size. The serves that do
> + // not support options negotiation would normally just ignore
> + // the options and thus this code path will probably never be
> + // executed
I took a very quick look at RFC 1350. I don't see it defining what to
do when the RRQ has extra parameters. It does not say they should be
ignored. So sending back an error is acceptable. That means i don't
like this comment.
Andrew
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ECOS] Re: Improving TFTP performance
2007-12-19 15:11 ` Andrew Lunn
@ 2007-12-19 15:34 ` Øyvind Harboe
2007-12-19 22:42 ` Andrew Lunn
0 siblings, 1 reply; 10+ messages in thread
From: Øyvind Harboe @ 2007-12-19 15:34 UTC (permalink / raw)
To: eCos Disuss
[-- Attachment #1: Type: text/plain, Size: 1733 bytes --]
On Dec 19, 2007 3:17 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > + cdl_option CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE {
> > + display "TFTP protocol allows negotiation of bigger packets.
> > + Requires server which supports RFC 2348 blksize negotiation."
> > + flavor data
> > + default_value 512
> > + legal_values 512 to 65464
> > + description "
> > + tftp blksize egotiation support. >512 byte block sizes improves
> > + tftp GET performance"
> > + }
>
> The display should be kept to one line maximum. Put all the rest on
> the description lines. You should also say that the default value of
> 512 causes this to be disabled. It might even be better to change the
> flavor of this to booldata, so it can be enabled/disabled and the
> value set. It then makes your code cleaner.
I fought w/booldata for a bit, but couldn't make sense of it.
> > + // try without negotiating packet size. The serves that do
> > + // not support options negotiation would normally just ignore
> > + // the options and thus this code path will probably never be
> > + // executed
>
> I took a very quick look at RFC 1350. I don't see it defining what to
> do when the RRQ has extra parameters. It does not say they should be
> ignored. So sending back an error is acceptable. That means i don't
> like this comment.
Comment fixed.
I tried w/two different servers. The one without options negotiation
just ignored
the options, so I haven't exercised the error code path.
> Andrew
>
>
Better?
--
Øyvind Harboe
http://www.zylin.com - eCos ARM & FPGA developer kit
[-- Attachment #2: tftpfaster4.txt --]
[-- Type: text/plain, Size: 7735 bytes --]
### Eclipse Workspace Patch 1.0
#P ecos
Index: net/common/current/cdl/net.cdl
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/cdl/net.cdl,v
retrieving revision 1.17
diff -u -r1.17 net.cdl
--- net/common/current/cdl/net.cdl 7 Jan 2007 14:46:55 -0000 1.17
+++ net/common/current/cdl/net.cdl 19 Dec 2007 14:33:26 -0000
@@ -167,6 +167,16 @@
threads can have precedence over TFTP server processing."
}
+ cdl_option CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE {
+ display "TFTP protocol allows negotiation of bigger packets."
+ flavor data
+ default_value 512
+ legal_values 512 to 65464
+ description "Requires server which supports RFC 2348 blksize negotiation.
+ >512 byte block sizes improves tftp GET performance. When maximum
+ packetsize is 512, the blksize negotiation option does not take place."
+ }
+
cdl_option CYGPKG_NET_TFTPD_THREAD_STACK_SIZE {
display "Stack size for TFTP threads."
flavor data
Index: net/common/current/include/arpa/tftp.h
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/include/arpa/tftp.h,v
retrieving revision 1.2
diff -u -r1.2 tftp.h
--- net/common/current/include/arpa/tftp.h 7 Aug 2002 14:42:35 -0000 1.2
+++ net/common/current/include/arpa/tftp.h 19 Dec 2007 14:33:26 -0000
@@ -70,6 +70,7 @@
#define DATA 03 /* data packet */
#define ACK 04 /* acknowledgement */
#define ERROR 05 /* error code */
+#define OACK 06 /* option acknowledge */
struct tftphdr {
short th_opcode; /* packet type */
Index: net/common/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/ChangeLog,v
retrieving revision 1.81
diff -u -r1.81 ChangeLog
--- net/common/current/ChangeLog 15 Jan 2007 18:37:52 -0000 1.81
+++ net/common/current/ChangeLog 19 Dec 2007 14:33:26 -0000
@@ -1,3 +1,8 @@
+2007-12-19 Oyvind Harboe <oyvind.harboe@zylin.com>
+
+ * src/tftp_client.c, include/arpa/tftp.h, cdl/net.cdl: tftp blksize
+ negotiation support. >512 byte block sizes improves tftp GET performance
+
2007-01-15 Gary Thomas <gary@mlbassoc.com>
* src/dhcp_support.c (dhcp_mgt_entry): Better handling when restarting
Index: net/common/current/src/tftp_client.c
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/src/tftp_client.c,v
retrieving revision 1.10
diff -u -r1.10 tftp_client.c
--- net/common/current/src/tftp_client.c 16 Sep 2005 14:56:26 -0000 1.10
+++ net/common/current/src/tftp_client.c 19 Dec 2007 14:33:26 -0000
@@ -57,6 +57,8 @@
#include <network.h>
#include <arpa/tftp.h>
#include <tftp_support.h>
+#include <stdlib.h>
+#include <stdio.h>
#define min(x,y) (x<y ? x : y)
@@ -66,14 +68,17 @@
// On error, *err will hold the reason.
// This version uses the server name. This can be a name for DNS lookup
// or a dotty or colony number format for IPv4 or IPv6.
-int tftp_client_get(const char * const filename,
+static int tftp_client_get_inner(char *data,
+ const char * const filename,
const char * const server,
const int port,
char *buf,
int len,
const int mode,
- int * const err) {
-
+ int * const err,
+ int negotiate) {
+
+ int blksize=512;
int result = 0;
int s=-1;
int actual_len, data_len;
@@ -85,7 +90,6 @@
int error;
struct sockaddr local_addr, from_addr;
- char data[SEGSIZE+sizeof(struct tftphdr)];
struct tftphdr *hdr = (struct tftphdr *)data;
const char *fp;
char *cp, *bp;
@@ -112,6 +116,16 @@
}
while (*fp) *cp++ = *fp++;
*cp++ = '\0';
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ if (negotiate)
+ {
+ fp="blksize";
+ while (*fp) *cp++ = *fp++;
+ *cp++ = '\0';
+ cp+=sprintf(cp, "%d", CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE);
+ *cp++ = '\0';
+ }
+#endif
memset(&hints,0,sizeof(hints));
hints.ai_family = PF_UNSPEC;
@@ -214,15 +228,37 @@
}
}
} else {
- recv_len = sizeof(data);
+ recv_len = blksize+sizeof(struct tftphdr);
from_len = sizeof(from_addr);
- if ((data_len = recvfrom(s, &data, recv_len, 0,
+ if ((data_len = recvfrom(s, data, recv_len, 0,
&from_addr, &from_len)) < 0) {
// What happened?
*err = TFTP_NETERR;
goto out;
}
- if (ntohs(hdr->th_opcode) == DATA) {
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ if (ntohs(hdr->th_opcode) == OACK) {
+ // We can have only *one* option, the one we sent..
+ if (strncmp(data+2, "blksize", data_len)==0)
+ {
+ blksize=atol(data+2+strlen("blksize")+1);
+ } else
+ {
+ // option ignored, use default.
+ }
+ // Send out the ACK
+ hdr->th_opcode = htons(ACK);
+ hdr->th_block = htons(last_good_block);
+ if (sendto(s, data, 4 /* FIXME */, 0,
+ &from_addr, from_len) < 0) {
+ // Problem sending request
+ *err = TFTP_NETERR;
+ goto out;
+ }
+
+ } else
+#endif
+ if (ntohs(hdr->th_opcode) == DATA) {
actual_len = 0;
if (ntohs(hdr->th_block) == (last_good_block+1)) {
// Consume this data
@@ -244,7 +280,7 @@
// To prevent an out-of-sequence packet from
// terminating transmission prematurely, set
// actual_len to a full size packet.
- actual_len = SEGSIZE;
+ actual_len = blksize;
}
// Send out the ACK
hdr->th_opcode = htons(ACK);
@@ -256,7 +292,8 @@
goto out;
}
// A short packet marks the end of the file.
- if ((actual_len >= 0) && (actual_len < SEGSIZE)) {
+ /* 4 = Sizeof TFTP header */
+ if ((actual_len >= 0) && (actual_len < blksize)) {
// End of data
close(s);
freeaddrinfo(res);
@@ -290,6 +327,50 @@
freeaddrinfo(res);
return -1;
}
+
+
+int tftp_client_get(const char * const filename,
+ const char * const server,
+ const int port,
+ char *buf,
+ int len,
+ const int mode,
+ int * const err) {
+ int result;
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ char *data = malloc(CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE+
+ sizeof(struct tftphdr));
+ if (data==NULL)
+ {
+ *err=TFTP_ENOSPACE;
+ return -1;
+ }
+#else
+ char data[SEGSIZE+sizeof(struct tftphdr)];
+#endif
+ result=tftp_client_get_inner(data, filename, server,
+ port, buf, len, mode, err,
+ CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512);
+ if (result<0)
+ {
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ // try without negotiating packet size. The serves that do
+ // not support options negotiation may or may not ignore the
+ // options. If they return an error in the case of options
+ // this code path will try without packet size negotiation.
+ result=tftp_client_get_inner(data, filename, server,
+ port, buf, len, mode, err,
+ 0);
+#endif
+ }
+
+#if CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE>512
+ free(data);
+#endif
+
+ return result;
+}
+
//
// Read a file from a host into a local buffer. Returns the
// number of bytes actually read, or (-1) if an error occurs.
[-- Attachment #3: Type: text/plain, Size: 148 bytes --]
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ECOS] Re: Improving TFTP performance
2007-12-19 15:34 ` Øyvind Harboe
@ 2007-12-19 22:42 ` Andrew Lunn
2007-12-19 23:39 ` Gary Thomas
2007-12-19 23:52 ` Øyvind Harboe
0 siblings, 2 replies; 10+ messages in thread
From: Andrew Lunn @ 2007-12-19 22:42 UTC (permalink / raw)
To: ?yvind Harboe; +Cc: eCos Disuss
On Wed, Dec 19, 2007 at 03:47:32PM +0100, ?yvind Harboe wrote:
> On Dec 19, 2007 3:17 PM, Andrew Lunn <andrew@lunn.ch> wrote:
> > > + cdl_option CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE {
> > > + display "TFTP protocol allows negotiation of bigger packets.
> > > + Requires server which supports RFC 2348 blksize negotiation."
> > > + flavor data
> > > + default_value 512
> > > + legal_values 512 to 65464
> > > + description "
> > > + tftp blksize egotiation support. >512 byte block sizes improves
> > > + tftp GET performance"
> > > + }
> >
> > The display should be kept to one line maximum. Put all the rest on
> > the description lines. You should also say that the default value of
> > 512 causes this to be disabled. It might even be better to change the
> > flavor of this to booldata, so it can be enabled/disabled and the
> > value set. It then makes your code cleaner.
>
> I fought w/booldata for a bit, but couldn't make sense of it.
Something like this is O.K.
cdl_component CYGOPY_NET_TFTPD_CLIENT_BIG_PACKET {
display "Extension to allow negotiation of big packets"
flavor bool
default_value 0
description "Implements RFC XXXX, an optional extension to the TFTP
protocol to allow the client and server to negotiate
to use bigger packets. This can make upload/download
faster"
cdl_option CYGOPY_NET_TFTPD_CLIENT_BIG_PACKET {
display "Packet size to negotiate"
flavor data
default_value 32768
legal_values 8 to 65535
description "
Size of the packets to negotiate. In an error free
environment, bigger packets will result in faster transfers."
}
}
This will make your code cleaner. You no longer need the hard coded > 512
etc.
Andrew
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ECOS] Re: Improving TFTP performance
2007-12-19 22:42 ` Andrew Lunn
@ 2007-12-19 23:39 ` Gary Thomas
2007-12-19 23:52 ` Øyvind Harboe
1 sibling, 0 replies; 10+ messages in thread
From: Gary Thomas @ 2007-12-19 23:39 UTC (permalink / raw)
To: eCos Disuss
Andrew Lunn wrote:
> On Wed, Dec 19, 2007 at 03:47:32PM +0100, ?yvind Harboe wrote:
>> On Dec 19, 2007 3:17 PM, Andrew Lunn <andrew@lunn.ch> wrote:
>>>> + cdl_option CYGPKG_NET_TFTPD_CLIENT_GET_PACKETSIZE {
>>>> + display "TFTP protocol allows negotiation of bigger packets.
>>>> + Requires server which supports RFC 2348 blksize negotiation."
>>>> + flavor data
>>>> + default_value 512
>>>> + legal_values 512 to 65464
>>>> + description "
>>>> + tftp blksize egotiation support. >512 byte block sizes improves
>>>> + tftp GET performance"
>>>> + }
>>> The display should be kept to one line maximum. Put all the rest on
>>> the description lines. You should also say that the default value of
>>> 512 causes this to be disabled. It might even be better to change the
>>> flavor of this to booldata, so it can be enabled/disabled and the
>>> value set. It then makes your code cleaner.
>> I fought w/booldata for a bit, but couldn't make sense of it.
>
> Something like this is O.K.
>
> cdl_component CYGOPY_NET_TFTPD_CLIENT_BIG_PACKET {
> display "Extension to allow negotiation of big packets"
> flavor bool
> default_value 0
>
> description "Implements RFC XXXX, an optional extension to the TFTP
> protocol to allow the client and server to negotiate
> to use bigger packets. This can make upload/download
> faster"
>
> cdl_option CYGOPY_NET_TFTPD_CLIENT_BIG_PACKET {
> display "Packet size to negotiate"
> flavor data
> default_value 32768
> legal_values 8 to 65535
> description "
> Size of the packets to negotiate. In an error free
> environment, bigger packets will result in faster transfers."
> }
> }
>
>
> This will make your code cleaner. You no longer need the hard coded > 512
> etc.
CYGOPY_??? AFAIK, this isn't one of our normal prefixes (and for namespace
pollution issues, we _try_ to remain consistent). Maybe you want CYGOPT_???
--
------------------------------------------------------------
Gary Thomas | Consulting for the
MLB Associates | Embedded world
------------------------------------------------------------
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [ECOS] Re: Improving TFTP performance
2007-12-19 22:42 ` Andrew Lunn
2007-12-19 23:39 ` Gary Thomas
@ 2007-12-19 23:52 ` Øyvind Harboe
1 sibling, 0 replies; 10+ messages in thread
From: Øyvind Harboe @ 2007-12-19 23:52 UTC (permalink / raw)
To: eCos Disuss
[-- Attachment #1: Type: text/plain, Size: 217 bytes --]
> This will make your code cleaner. You no longer need the hard coded > 512
> etc.
Updated. Times up for this time, duty calls....
--
Øyvind Harboe
http://www.zylin.com - eCos ARM & FPGA developer kit
[-- Attachment #2: tftpfaster6.txt --]
[-- Type: text/plain, Size: 8147 bytes --]
### Eclipse Workspace Patch 1.0
#P ecos
Index: net/common/current/cdl/net.cdl
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/cdl/net.cdl,v
retrieving revision 1.17
diff -u -r1.17 net.cdl
--- net/common/current/cdl/net.cdl 7 Jan 2007 14:46:55 -0000 1.17
+++ net/common/current/cdl/net.cdl 19 Dec 2007 15:32:47 -0000
@@ -167,6 +167,28 @@
threads can have precedence over TFTP server processing."
}
+ cdl_component CYGPKG_NET_TFTPD_CLIENT_BIG_PACKET {
+ display "Extension to allow negotiation of big packets"
+ flavor bool
+ default_value 0
+
+ description "Implements RFC 2348, an optional extension to the TFTP
+ protocol to allow the client and server to negotiate
+ to use bigger packets. This can make upload/download
+ faster"
+
+ cdl_option CYGPKG_NET_TFTPD_CLIENT_BIG_PACKET_SIZE {
+ display "Packet size to negotiate"
+ flavor data
+ default_value 512
+ legal_values 512 to 65464
+ description "
+ Size of the packets to negotiate. In an error free
+ environment, bigger packets will result in faster transfers."
+ }
+ }
+
+
cdl_option CYGPKG_NET_TFTPD_THREAD_STACK_SIZE {
display "Stack size for TFTP threads."
flavor data
Index: net/common/current/include/arpa/tftp.h
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/include/arpa/tftp.h,v
retrieving revision 1.2
diff -u -r1.2 tftp.h
--- net/common/current/include/arpa/tftp.h 7 Aug 2002 14:42:35 -0000 1.2
+++ net/common/current/include/arpa/tftp.h 19 Dec 2007 15:32:47 -0000
@@ -70,6 +70,7 @@
#define DATA 03 /* data packet */
#define ACK 04 /* acknowledgement */
#define ERROR 05 /* error code */
+#define OACK 06 /* option acknowledge */
struct tftphdr {
short th_opcode; /* packet type */
Index: net/common/current/ChangeLog
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/ChangeLog,v
retrieving revision 1.81
diff -u -r1.81 ChangeLog
--- net/common/current/ChangeLog 15 Jan 2007 18:37:52 -0000 1.81
+++ net/common/current/ChangeLog 19 Dec 2007 15:32:47 -0000
@@ -1,3 +1,8 @@
+2007-12-19 Oyvind Harboe <oyvind.harboe@zylin.com>
+
+ * src/tftp_client.c, include/arpa/tftp.h, cdl/net.cdl: tftp blksize
+ negotiation support. >512 byte block sizes improves tftp GET performance
+
2007-01-15 Gary Thomas <gary@mlbassoc.com>
* src/dhcp_support.c (dhcp_mgt_entry): Better handling when restarting
Index: net/common/current/src/tftp_client.c
===================================================================
RCS file: /cvs/ecos/ecos-opt/net/net/common/current/src/tftp_client.c,v
retrieving revision 1.10
diff -u -r1.10 tftp_client.c
--- net/common/current/src/tftp_client.c 16 Sep 2005 14:56:26 -0000 1.10
+++ net/common/current/src/tftp_client.c 19 Dec 2007 15:32:47 -0000
@@ -57,6 +57,8 @@
#include <network.h>
#include <arpa/tftp.h>
#include <tftp_support.h>
+#include <stdlib.h>
+#include <stdio.h>
#define min(x,y) (x<y ? x : y)
@@ -66,14 +68,20 @@
// On error, *err will hold the reason.
// This version uses the server name. This can be a name for DNS lookup
// or a dotty or colony number format for IPv4 or IPv6.
-int tftp_client_get(const char * const filename,
+static int tftp_client_get_inner(char *data,
+ const char * const filename,
const char * const server,
const int port,
char *buf,
int len,
const int mode,
- int * const err) {
-
+ int * const err
+#ifdef CYGPKG_NET_TFTPD_CLIENT_BIG_PACKET
+ ,int negotiate
+#endif
+ ) {
+
+ int blksize=SEGSIZE;
int result = 0;
int s=-1;
int actual_len, data_len;
@@ -85,7 +93,6 @@
int error;
struct sockaddr local_addr, from_addr;
- char data[SEGSIZE+sizeof(struct tftphdr)];
struct tftphdr *hdr = (struct tftphdr *)data;
const char *fp;
char *cp, *bp;
@@ -112,6 +119,16 @@
}
while (*fp) *cp++ = *fp++;
*cp++ = '\0';
+#ifdef CYGPKG_NET_TFTPD_CLIENT_BIG_PACKET
+ if (negotiate)
+ {
+ fp="blksize";
+ while (*fp) *cp++ = *fp++;
+ *cp++ = '\0';
+ cp+=sprintf(cp, "%d", CYGPKG_NET_TFTPD_CLIENT_BIG_PACKET_SIZE);
+ *cp++ = '\0';
+ }
+#endif
memset(&hints,0,sizeof(hints));
hints.ai_family = PF_UNSPEC;
@@ -214,15 +231,37 @@
}
}
} else {
- recv_len = sizeof(data);
+ recv_len = blksize+sizeof(struct tftphdr);
from_len = sizeof(from_addr);
- if ((data_len = recvfrom(s, &data, recv_len, 0,
+ if ((data_len = recvfrom(s, data, recv_len, 0,
&from_addr, &from_len)) < 0) {
// What happened?
*err = TFTP_NETERR;
goto out;
}
- if (ntohs(hdr->th_opcode) == DATA) {
+#ifdef CYGPKG_NET_TFTPD_CLIENT_BIG_PACKET
+ if (ntohs(hdr->th_opcode) == OACK) {
+ // We can have only *one* option, the one we sent..
+ if (strncmp(data+2, "blksize", data_len)==0)
+ {
+ blksize=atol(data+2+strlen("blksize")+1);
+ } else
+ {
+ // option ignored, use default.
+ }
+ // Send out the ACK
+ hdr->th_opcode = htons(ACK);
+ hdr->th_block = htons(last_good_block);
+ if (sendto(s, data, 4 /* FIXME */, 0,
+ &from_addr, from_len) < 0) {
+ // Problem sending request
+ *err = TFTP_NETERR;
+ goto out;
+ }
+
+ } else
+#endif
+ if (ntohs(hdr->th_opcode) == DATA) {
actual_len = 0;
if (ntohs(hdr->th_block) == (last_good_block+1)) {
// Consume this data
@@ -244,7 +283,7 @@
// To prevent an out-of-sequence packet from
// terminating transmission prematurely, set
// actual_len to a full size packet.
- actual_len = SEGSIZE;
+ actual_len = blksize;
}
// Send out the ACK
hdr->th_opcode = htons(ACK);
@@ -256,7 +295,8 @@
goto out;
}
// A short packet marks the end of the file.
- if ((actual_len >= 0) && (actual_len < SEGSIZE)) {
+ /* 4 = Sizeof TFTP header */
+ if ((actual_len >= 0) && (actual_len < blksize)) {
// End of data
close(s);
freeaddrinfo(res);
@@ -290,6 +330,53 @@
freeaddrinfo(res);
return -1;
}
+
+
+int tftp_client_get(const char * const filename,
+ const char * const server,
+ const int port,
+ char *buf,
+ int len,
+ const int mode,
+ int * const err) {
+ int result;
+#ifdef CYGPKG_NET_TFTPD_CLIENT_BIG_PACKET
+ char *data = malloc(CYGPKG_NET_TFTPD_CLIENT_BIG_PACKET_SIZE+
+ sizeof(struct tftphdr));
+ if (data==NULL)
+ {
+ *err=TFTP_ENOSPACE;
+ return -1;
+ }
+#else
+ char data[SEGSIZE+sizeof(struct tftphdr)];
+#endif
+ result=tftp_client_get_inner(data, filename, server,
+ port, buf, len, mode, err
+#ifdef CYGPKG_NET_TFTPD_CLIENT_BIG_PACKET
+ ,1
+#endif
+ );
+ if (result<0)
+ {
+#ifdef CYGPKG_NET_TFTPD_CLIENT_BIG_PACKET
+ // try without negotiating packet size. The serves that do
+ // not support options negotiation may or may not ignore the
+ // options. If they return an error in the case of options
+ // this code path will try without packet size negotiation.
+ result=tftp_client_get_inner(data, filename, server,
+ port, buf, len, mode, err,
+ 0);
+#endif
+ }
+
+#ifdef CYGPKG_NET_TFTPD_CLIENT_BIG_PACKET
+ free(data);
+#endif
+
+ return result;
+}
+
//
// Read a file from a host into a local buffer. Returns the
// number of bytes actually read, or (-1) if an error occurs.
[-- Attachment #3: Type: text/plain, Size: 148 bytes --]
--
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2007-12-19 15:34 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-12-19 12:41 [ECOS] Improving TFTP performance Øyvind Harboe
2007-12-19 13:53 ` [ECOS] " Øyvind Harboe
2007-12-19 14:17 ` Andrew Lunn
2007-12-19 14:47 ` Øyvind Harboe
2007-12-19 15:11 ` Andrew Lunn
2007-12-19 15:34 ` Øyvind Harboe
2007-12-19 22:42 ` Andrew Lunn
2007-12-19 23:39 ` Gary Thomas
2007-12-19 23:52 ` Øyvind Harboe
2007-12-19 14:28 ` Sergei Gavrikov
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 read-only IMAP folder(s) and NNTP newsgroup(s).