[U-Boot-Users] [PATCH] TFTP: add host ip addr support
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index 21682c0..2fb0e7c 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( tftpboot, 3, 1, do_tftpb, "tftpboot- boot image via network using TFTP protocol\n", - "[loadAddress] [bootfilename]\n" + "[loadAddress] [host ip addr:bootfilename]\n" );
int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) diff --git a/net/tftp.c b/net/tftp.c index 8b95bcf..d5d6e65 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -34,7 +34,7 @@ #define TFTP_ERROR 5 #define TFTP_OACK 6
- +static IPaddr_t TftpServerIP; static int TftpServerPort; /* The UDP port at their end */ static int TftpOurPort; /* The UDP port at our end */ static int TftpTimeoutCount; @@ -231,7 +231,7 @@ TftpSend (void) break; }
- NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len); + NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len); }
@@ -464,19 +464,27 @@ TftpStart (void) printf ("*** Warning: no boot file name; using '%s'\n", tftp_filename); } else { - tftp_filename = BootFile; + char *p=BootFile; + p = strchr (p, ':'); + if (p != NULL) { + TftpServerIP = string_to_ip (BootFile); + ++p; + strcpy (tftp_filename, p); + } else { + strcpy (tftp_filename, BootFile); + } }
#if defined(CONFIG_NET_MULTI) printf ("Using %s device\n", eth_get_name()); #endif - puts ("TFTP from server "); print_IPaddr (NetServerIP); + puts ("TFTP from server "); print_IPaddr (TftpServerIP); puts ("; our IP address is "); print_IPaddr (NetOurIP);
/* Check if we need to send across this subnet */ if (NetOurGatewayIP && NetOurSubnetMask) { IPaddr_t OurNet = NetOurIP & NetOurSubnetMask; - IPaddr_t ServerNet = NetServerIP & NetOurSubnetMask; + IPaddr_t ServerNet = TftpServerIP & NetOurSubnetMask;
if (OurNet != ServerNet) { puts ("; sending through gateway ");
Jean-Christophe PLAGNIOL-VILLARD wrote:
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index 21682c0..2fb0e7c 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( tftpboot, 3, 1, do_tftpb, "tftpboot- boot image via network using TFTP protocol\n",
- "[loadAddress] [bootfilename]\n"
- "[loadAddress] [host ip addr:bootfilename]\n"
Hi Jean-Christophe,
Interesting. I can see how that would be useful rather than having to modify the dhcp server configuration when you want to use an alternate server.
Critique: the help is misleading: "host ip addr:" is optional so it needs another set of [] around it. Having spaces in "host ip addr" is more readable, but makes it look like it is three things or perhaps a keyword.
I would suggest the following, but there may be a better way... "[loadAddress] [[hostIPaddr:]bootfilename]\n"
Best regards, gvb
allow to use a different server as set in serverip
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index 21682c0..e03ffbf 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( tftpboot, 3, 1, do_tftpb, "tftpboot- boot image via network using TFTP protocol\n", - "[loadAddress] [bootfilename]\n" + "[loadAddress] [[host ip addr]:bootfilename]\n" );
int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) diff --git a/net/tftp.c b/net/tftp.c index 8b95bcf..d5d6e65 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -34,7 +34,7 @@ #define TFTP_ERROR 5 #define TFTP_OACK 6
- +static IPaddr_t TftpServerIP; static int TftpServerPort; /* The UDP port at their end */ static int TftpOurPort; /* The UDP port at our end */ static int TftpTimeoutCount; @@ -231,7 +231,7 @@ TftpSend (void) break; }
- NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len); + NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len); }
@@ -464,19 +464,27 @@ TftpStart (void) printf ("*** Warning: no boot file name; using '%s'\n", tftp_filename); } else { - tftp_filename = BootFile; + char *p=BootFile; + p = strchr (p, ':'); + if (p != NULL) { + TftpServerIP = string_to_ip (BootFile); + ++p; + strcpy (tftp_filename, p); + } else { + strcpy (tftp_filename, BootFile); + } }
#if defined(CONFIG_NET_MULTI) printf ("Using %s device\n", eth_get_name()); #endif - puts ("TFTP from server "); print_IPaddr (NetServerIP); + puts ("TFTP from server "); print_IPaddr (TftpServerIP); puts ("; our IP address is "); print_IPaddr (NetOurIP);
/* Check if we need to send across this subnet */ if (NetOurGatewayIP && NetOurSubnetMask) { IPaddr_t OurNet = NetOurIP & NetOurSubnetMask; - IPaddr_t ServerNet = NetServerIP & NetOurSubnetMask; + IPaddr_t ServerNet = TftpServerIP & NetOurSubnetMask;
if (OurNet != ServerNet) { puts ("; sending through gateway ");
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index e03ffbf..894d863 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( nfs, 3, 1, do_nfs, "nfs\t- boot image via network using NFS protocol\n", - "[loadAddress] [host ip addr:bootfilename]\n" + "[loadAddress] [[host ip addr]:bootfilename]\n" ); #endif
Jean-Christophe PLAGNIOL-VILLARD wrote:
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index e03ffbf..894d863 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( nfs, 3, 1, do_nfs, "nfs\t- boot image via network using NFS protocol\n",
- "[loadAddress] [host ip addr:bootfilename]\n"
- "[loadAddress] [[host ip addr]:bootfilename]\n"
); #endif
Sorry for being a PITA, but since we're changing documentation shouldn't this be:
- "[loadAddress] [host ip addr:bootfilename]\n" + "[loadAddress] [[host ip addr:]bootfilename]\n"
Can you spot the difference?
regards, Ben
In message 47854A1E.5060704@gmail.com you wrote:
- "[loadAddress] [host ip addr:bootfilename]\n"
- "[loadAddress] [[host ip addr]:bootfilename]\n"
); #endif
Sorry for being a PITA, but since we're changing documentation shouldn't this be:
You're not a PITA, but a careful reader.
- "[loadAddress] [host ip addr:bootfilename]\n"
- "[loadAddress] [[host ip addr:]bootfilename]\n"
Can you spot the difference?
Indeed - it's the difference between right and wrong, between working and failing.
Also, I ask to change this (as gvb previously recommended) into
"[loadAddress] [[HostIpAddr:]bootfilename]\n"
Best regards,
Wolfgang Denk
allow to use a different server as set in serverip
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index 21682c0..b86ca86 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( tftpboot, 3, 1, do_tftpb, "tftpboot- boot image via network using TFTP protocol\n", - "[loadAddress] [bootfilename]\n" + "[loadAddress] [[hostIPaddr:]bootfilename]\n" );
int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) diff --git a/net/tftp.c b/net/tftp.c index 8b95bcf..d5d6e65 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -34,7 +34,7 @@ #define TFTP_ERROR 5 #define TFTP_OACK 6
- +static IPaddr_t TftpServerIP; static int TftpServerPort; /* The UDP port at their end */ static int TftpOurPort; /* The UDP port at our end */ static int TftpTimeoutCount; @@ -231,7 +231,7 @@ TftpSend (void) break; }
- NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len); + NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len); }
@@ -464,19 +464,27 @@ TftpStart (void) printf ("*** Warning: no boot file name; using '%s'\n", tftp_filename); } else { - tftp_filename = BootFile; + char *p=BootFile; + p = strchr (p, ':'); + if (p != NULL) { + TftpServerIP = string_to_ip (BootFile); + ++p; + strcpy (tftp_filename, p); + } else { + strcpy (tftp_filename, BootFile); + } }
#if defined(CONFIG_NET_MULTI) printf ("Using %s device\n", eth_get_name()); #endif - puts ("TFTP from server "); print_IPaddr (NetServerIP); + puts ("TFTP from server "); print_IPaddr (TftpServerIP); puts ("; our IP address is "); print_IPaddr (NetOurIP);
/* Check if we need to send across this subnet */ if (NetOurGatewayIP && NetOurSubnetMask) { IPaddr_t OurNet = NetOurIP & NetOurSubnetMask; - IPaddr_t ServerNet = NetServerIP & NetOurSubnetMask; + IPaddr_t ServerNet = TftpServerIP & NetOurSubnetMask;
if (OurNet != ServerNet) { puts ("; sending through gateway ");
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index b86ca86..dbf6b86 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( nfs, 3, 1, do_nfs, "nfs\t- boot image via network using NFS protocol\n", - "[loadAddress] [host ip addr:bootfilename]\n" + "[loadAddress] [[hostIPaddr:]bootfilename]\n" ); #endif
Jean-Christophe PLAGNIOL-VILLARD wrote:
@@ -464,19 +464,27 @@ TftpStart (void) printf ("*** Warning: no boot file name; using '%s'\n", tftp_filename); } else {
tftp_filename = BootFile;
char *p=BootFile;p = strchr (p, ':');if (p != NULL) {TftpServerIP = string_to_ip (BootFile);++p;strcpy (tftp_filename, p);} else {strcpy (tftp_filename, BootFile);}
Doesn't this mean that TftpServerIP will be undefined if no server is specified. Should it not be:
@@ -464,19 +464,27 @@ TftpStart (void) printf ("*** Warning: no boot file name; using '%s'\n", tftp_filename); } else { - tftp_filename = BootFile; + char *p=BootFile; + p = strchr (p, ':'); + if (p != NULL) { + TftpServerIP = string_to_ip (BootFile); + ++p; + strcpy (tftp_filename, p); + } else { + TftpServerIP = NetServerIP; + strcpy (tftp_filename, BootFile); + }
Andre
On 12:33 Fri 11 Jan , Andre Renaud wrote:
Jean-Christophe PLAGNIOL-VILLARD wrote:
Doesn't this mean that TftpServerIP will be undefined if no server is specified. Should it not be:
@@ -464,19 +464,27 @@ TftpStart (void) printf ("*** Warning: no boot file name; using '%s'\n", tftp_filename); } else {
tftp_filename = BootFile;
char *p=BootFile;p = strchr (p, ':');if (p != NULL) {TftpServerIP = string_to_ip (BootFile);++p;strcpy (tftp_filename, p);} else {TftpServerIP = NetServerIP;strcpy (tftp_filename, BootFile);}
I will prefer + TftpServerIP = NetServerIP; if (BootFile[0] == '\0') { sprintf(default_filename, "%02lX%02lX%02lX%02lX.img", NetOurIP & 0xFF, @@ -464,19 +465,26 @@ TftpStart (void) printf ("*** Warning: no boot file name; using '%s'\n", tftp_filename); } else { - tftp_filename = BootFile; + char *p=BootFile; + p = strchr (p, ':'); + if (p != NULL) { + TftpServerIP = string_to_ip (BootFile); + ++p; + strcpy (tftp_filename, p); + } else + strcpy (tftp_filename, BootFile);
Andre
-- Bluewater Systems Ltd - ARM Technology Solutions Centre
Andre Renaud Bluewater Systems LtdPhone: +64 3 3779127 (Aus 1 800 148 751) Level 17, 119 Armagh St Fax: +64 3 3779135 PO Box 13889 Email: arenaud@bluewatersys.com Christchurch Web: http://www.bluewatersys.com New Zealand
In message 1199914268-29217-2-git-send-email-plagnioj@jcrosoft.com you wrote:
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index e03ffbf..894d863 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( nfs, 3, 1, do_nfs, "nfs\t- boot image via network using NFS protocol\n",
- "[loadAddress] [host ip addr:bootfilename]\n"
- "[loadAddress] [[host ip addr]:bootfilename]\n"
); #endif
Rejected, because this is not correct:
=> print serverip serverip=192.168.1.1 => nfs 200000 :/opt/eldk/ppc_4xx/images/uImage Using ppc_4xx_eth0 device File transfer via NFS from server 0.0.0.0; our IP address is 192.168.80.2 Filename '/opt/eldk/ppc_4xx/images/uImage'. Load address: 0x200000 Loading: *** ERROR: Cannot mount ...
Best regards,
Wolfgang Denk
In message 1199914268-29217-1-git-send-email-plagnioj@jcrosoft.com you wrote:
allow to use a different server as set in serverip
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index 21682c0..e03ffbf 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( tftpboot, 3, 1, do_tftpb, "tftpboot- boot image via network using TFTP protocol\n",
- "[loadAddress] [bootfilename]\n"
- "[loadAddress] [[host ip addr]:bootfilename]\n"
NAK.
Ben, please don't apply. I reject this patch.
Jean-Christophe - as already discussed on IRC I really ask you to pay attemtion to gvb's comments on your patch:
| Critique: the help is misleading: "host ip addr:" is optional so it | needs another set of [] around it. Having spaces in "host ip addr" is | more readable, but makes it look like it is three things or perhaps a | keyword. | | I would suggest the following, but there may be a better way... | "[loadAddress] [[hostIPaddr:]bootfilename]\n"
Also make sure that you place the ';' in the right pair of brackets.
Best regards,
Wolfgang Denk
allow to use a different server as set in serverip
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index 21682c0..4e3d489 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( tftpboot, 3, 1, do_tftpb, "tftpboot- boot image via network using TFTP protocol\n", - "[loadAddress] [bootfilename]\n" + "[loadAddress] [[host ip addr:]bootfilename]\n" );
int do_rarpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) diff --git a/net/tftp.c b/net/tftp.c index 8b95bcf..d5d6e65 100644 --- a/net/tftp.c +++ b/net/tftp.c @@ -34,7 +34,7 @@ #define TFTP_ERROR 5 #define TFTP_OACK 6
- +static IPaddr_t TftpServerIP; static int TftpServerPort; /* The UDP port at their end */ static int TftpOurPort; /* The UDP port at our end */ static int TftpTimeoutCount; @@ -231,7 +231,7 @@ TftpSend (void) break; }
- NetSendUDPPacket(NetServerEther, NetServerIP, TftpServerPort, TftpOurPort, len); + NetSendUDPPacket(NetServerEther, TftpServerIP, TftpServerPort, TftpOurPort, len); }
@@ -464,19 +464,27 @@ TftpStart (void) printf ("*** Warning: no boot file name; using '%s'\n", tftp_filename); } else { - tftp_filename = BootFile; + char *p=BootFile; + p = strchr (p, ':'); + if (p != NULL) { + TftpServerIP = string_to_ip (BootFile); + ++p; + strcpy (tftp_filename, p); + } else { + strcpy (tftp_filename, BootFile); + } }
#if defined(CONFIG_NET_MULTI) printf ("Using %s device\n", eth_get_name()); #endif - puts ("TFTP from server "); print_IPaddr (NetServerIP); + puts ("TFTP from server "); print_IPaddr (TftpServerIP); puts ("; our IP address is "); print_IPaddr (NetOurIP);
/* Check if we need to send across this subnet */ if (NetOurGatewayIP && NetOurSubnetMask) { IPaddr_t OurNet = NetOurIP & NetOurSubnetMask; - IPaddr_t ServerNet = NetServerIP & NetOurSubnetMask; + IPaddr_t ServerNet = TftpServerIP & NetOurSubnetMask;
if (OurNet != ServerNet) { puts ("; sending through gateway ");
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index 4e3d489..fec87b6 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( nfs, 3, 1, do_nfs, "nfs\t- boot image via network using NFS protocol\n", - "[loadAddress] [host ip addr:bootfilename]\n" + "[loadAddress] [[host ip addr:]bootfilename]\n" ); #endif
In message 1199958365-18516-2-git-send-email-plagnioj@jcrosoft.com you wrote:
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index 4e3d489..fec87b6 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -87,7 +87,7 @@ int do_nfs (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( nfs, 3, 1, do_nfs, "nfs\t- boot image via network using NFS protocol\n",
- "[loadAddress] [host ip addr:bootfilename]\n"
- "[loadAddress] [[host ip addr:]bootfilename]\n"
Rejected. Please see previous reject message for the reason.
Best regards,
Wolfgang Denk
In message 1199958365-18516-1-git-send-email-plagnioj@jcrosoft.com you wrote:
allow to use a different server as set in serverip
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
diff --git a/common/cmd_net.c b/common/cmd_net.c index 21682c0..4e3d489 100644 --- a/common/cmd_net.c +++ b/common/cmd_net.c @@ -51,7 +51,7 @@ int do_tftpb (cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) U_BOOT_CMD( tftpboot, 3, 1, do_tftpb, "tftpboot- boot image via network using TFTP protocol\n",
- "[loadAddress] [bootfilename]\n"
- "[loadAddress] [[host ip addr:]bootfilename]\n"
Rejected. Please READ what I wrote before. Heed gvb's suggestion.
Best regards,
Wolfgang Denk
Jean-Christophe PLAGNIOL-VILLARD wrote:
<snip>
This doesn't work. Baseline: (before patch applied):
=> tftp 100000 mpc8349emds/u-boot.bin.newnfs Speed: 100, full duplex Using TSEC0 device TFTP from server 10.69.69.21; our IP address is 10.69.69.201 Filename 'mpc8349emds/u-boot.bin.newnfs'. Load address: 0x100000 Loading: ####################################### done Bytes transferred = 198116 (305e4 hex)
Problem #1. First time through, with patch. Doesn't use serverip, so no default server:
=> tftp 100000 mpc8349emds/u-boot.bin Speed: 100, full duplex Using TSEC0 device TFTP from server 0.0.0.0; our IP address is 10.69.69.201; sending through gatew1 Filename '<NULL>'. Load address: 0x100000 Loading: * TFTP error: 'File not found' (1) Starting again
Problem #2. Doesn't parse file name:
=> tftp 100000 10.69.69.21:mpc8349emds/u-boot.bin Speed: 100, full duplex Using TSEC0 device TFTP from server 10.69.69.21; our IP address is 10.69.69.201 Filename '<NULL>'. Load address: 0x100000 Loading: ################################## done Bytes transferred = 170996 (29bf4 hex)
Please fix these if you want the code included.
regards, Ben
On Mon, 24 Dec 2007 16:55:38 +0100 Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com wrote:
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
Jean-Christophe, I love your work, but your changelogs totally suck. Could you please add a bit of text explaining what kind of problem this patch solves?
Haavard
In message 20071226214227.499a2109@siona you wrote:
On Mon, 24 Dec 2007 16:55:38 +0100 Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com wrote:
Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD plagnioj@jcrosoft.com
Jean-Christophe, I love your work, but your changelogs totally suck. Could you please add a bit of text explaining what kind of problem this patch solves?
I support this request.
Best regards,
Wolfgang Denk
participants (6)
-
Andre Renaud -
Ben Warren -
Haavard Skinnemoen -
Jean-Christophe PLAGNIOL-VILLARD -
Jerry Van Baren -
Wolfgang Denk