Re: [ECOS] Re: Improving TFTP performance

["Øyvind Harboe" <oyvind.harboe@zylin.com>]

[-- 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