This is the mail archive of the ecos-discuss@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: RedBoot TFTP last block ack oddness


Andrew Dyer wrote:
> On 9/8/05, David Vrabel <dvrabel@arcom.com> wrote:
>>
>>RedBoot's TFTP client has two oddities when ack'ing the last data block.
>>
>>1. It fails to ack the last data block when loading ELF images if
>>there's trailing data after the last section it loads (more correctly,
>>it fails to ack the first block it doesn't want).  For example, if an
>>image had a trailing .comment section.
> 
> Perhaps this:  http://sourceware.org/ml/ecos-discuss/2004-03/msg00148.html
> patch would help.

That did the trick. Thanks, Andrew.

Gary, a reworked patch against recent CVS is attached.

David Vrabel
-- 
David Vrabel, Design Engineer

Arcom, Clifton Road           Tel: +44 (0)1223 411200 ext. 3233
Cambridge CB1 7EA, UK         Web: http://www.arcom.com/
Index: ecos-working/packages/redboot/current/ChangeLog
===================================================================
--- ecos-working.orig/packages/redboot/current/ChangeLog	2005-09-05 16:24:00.000000000 +0100
+++ ecos-working/packages/redboot/current/ChangeLog	2005-09-09 10:57:58.000000000 +0100
@@ -1,3 +1,16 @@
+2005-09-09  Andrew Dyer  <adyer@righthandtech.com>
+
+	* src/load.c: add calls to redboot_getc_terminate before exiting
+	load_elf_image() in various error scenarios, change the final call
+	to redboot_getc_terminate to have the error flag set.  This will
+	cause a tftp nak and close down the connection since for ELF files
+	we don't read the whole content but end the connection when the
+	runnable parts are in.
+
+	* src/net/tftp_client.c: add tftp_error() to send an error back to
+	the server. define tftp_stream_terminate() and pass it into the
+	redboot interface.
+
 2005-09-03  Andrew Lunn  <andrew.lunn@ascom.ch>
 
 	* cdl/redboot.cdl: White space changes to aid readability. 
Index: ecos-working/packages/redboot/current/include/net/tftp_support.h
===================================================================
--- ecos-working.orig/packages/redboot/current/include/net/tftp_support.h	2002-07-01 21:55:28.000000000 +0100
+++ ecos-working/packages/redboot/current/include/net/tftp_support.h	2005-09-09 10:53:21.000000000 +0100
@@ -87,6 +87,7 @@
 extern int   tftp_stream_open(connection_info_t *info, int *err);
 extern int   tftp_stream_read(char *buf, int len, int *err);
 extern void  tftp_stream_close(int *err);
+extern void  tftp_stream_terminate(bool abort, int (*getc)(void));
 extern char *tftp_error(int err);
 
 #define TFTP_TIMEOUT_PERIOD 5
Index: ecos-working/packages/redboot/current/src/load.c
===================================================================
--- ecos-working.orig/packages/redboot/current/src/load.c	2004-09-19 14:01:54.000000000 +0100
+++ ecos-working/packages/redboot/current/src/load.c	2005-09-09 10:56:10.000000000 +0100
@@ -307,6 +307,7 @@
     // Read the header
     if (_read(getc, (unsigned char *)&ehdr, sizeof(ehdr)) != sizeof(ehdr)) {
         diag_printf("Can't read ELF header\n");
+        redboot_getc_terminate(true);
         return 0;
     }
     offset += sizeof(ehdr);    
@@ -318,15 +319,18 @@
 #endif
     if (ehdr.e_type != ET_EXEC) {
         diag_printf("Only absolute ELF images supported\n");
+        redboot_getc_terminate(true);
         return 0;
     }
     if (ehdr.e_phnum > MAX_PHDR) {
         diag_printf("Too many program headers\n");
+        redboot_getc_terminate(true);
         return 0;
     }
     while (offset < ehdr.e_phoff) {
         if ((*getc)() < 0) {
             diag_printf(SHORT_DATA);
+            redboot_getc_terminate(true);
             return 0;
         }
         offset++;
@@ -334,6 +338,7 @@
     for (phx = 0;  phx < ehdr.e_phnum;  phx++) {
         if (_read(getc, (unsigned char *)&phdr[phx], sizeof(phdr[0])) != sizeof(phdr[0])) {
             diag_printf("Can't read ELF program header\n");
+            redboot_getc_terminate(true);
             return 0;
         }
 #if 0 // DEBUG
@@ -376,6 +381,7 @@
             if (offset > phdr[phx].p_offset) {
                 if ((phdr[phx].p_offset + len) < offset) {
                     diag_printf("Can't load ELF file - program headers out of order\n");
+                    redboot_getc_terminate(true);
                     return 0;
                 }
                 addr += offset - phdr[phx].p_offset;
@@ -383,6 +389,7 @@
                 while (offset < phdr[phx].p_offset) {
                     if ((*getc)() < 0) {
                         diag_printf(SHORT_DATA);
+                        redboot_getc_terminate(true);
                         return 0;
                     }
                     offset++;
@@ -400,6 +407,7 @@
 #endif
                 if ((ch = (*getc)()) < 0) {
                     diag_printf(SHORT_DATA);
+                    redboot_getc_terminate(true);
                     return 0;
                 }
                 *addr++ = ch;
@@ -422,7 +430,10 @@
         entry_address = ehdr.e_entry;
     }
 
-    redboot_getc_terminate(false);
+    // nak everything to stop the transfer, since redboot
+    // usually doesn't read all the way to the end of the
+    // elf files.
+    redboot_getc_terminate(true);
     if (addr_offset) diag_printf("Address offset = %p\n", (void *)addr_offset);
     diag_printf("Entry point: %p, address range: %p-%p\n", 
                 (void*)entry_address, (void *)load_address, (void *)load_address_end);
Index: ecos-working/packages/redboot/current/src/net/tftp_client.c
===================================================================
--- ecos-working.orig/packages/redboot/current/src/net/tftp_client.c	2004-04-23 21:38:18.000000000 +0100
+++ ecos-working/packages/redboot/current/src/net/tftp_client.c	2005-09-09 11:11:34.000000000 +0100
@@ -158,10 +158,49 @@
     return 0;
 }
 
+static int
+tftp_error_ack(int *err, short code, char *msg)
+{
+    struct tftphdr *hdr = (struct tftphdr *)tftp_stream.data;
+
+    if (strlen(msg) > (SEGSIZE-1)) {
+      *(msg + SEGSIZE) = NULL;
+    }
+
+    if (tftp_stream.packets_received > 0) {
+        hdr->th_opcode = htons(ERROR);
+        hdr->th_code = code;
+        strcpy(&hdr->th_data, msg);
+        if (__udp_sendto(tftp_stream.data, (5 + strlen(msg)), 
+                         &tftp_stream.from_addr, &tftp_stream.local_addr) < 0) {
+            // Problem sending ACK
+            *err = TFTP_NETERR;
+            return -1;
+        }
+    }
+    return 0;
+}
+
 void
 tftp_stream_close(int *err)
 {
-    tftp_ack(err);
+    if (tftp_stream.open == true) {
+        tftp_ack(err);
+        tftp_stream.open = false;
+    }
+}
+
+void
+tftp_stream_terminate(bool abort,
+                      int (*getc)(void))
+{
+    int err;
+
+    if (abort)
+        tftp_error_ack(&err, EUNDEF, "redboot tftp_stream_terminate");
+    else
+        tftp_ack(&err);
+
     tftp_stream.open = false;
 }
 
@@ -274,6 +313,6 @@
 // RedBoot interface
 //
 GETC_IO_FUNCS(tftp_io, tftp_stream_open, tftp_stream_close,
-              0, tftp_stream_read, tftp_error);
+              tftp_stream_terminate, tftp_stream_read, tftp_error);
 RedBoot_load(tftp, tftp_io, true, true, 0);
 

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]