[U-Boot] [PATCH] NetLoop initialization bug
[U-Boot] [PATCH] NetLoop initialization bug
Upon u-boot's start up the first ping command causes a failure of the consequent TFTP command. It happens in the recently added mechanism of the NetLoop initialization where initialization of global network parameters is separated in the NetInitLoop routine which is called per env_id change. Thus, ping request will initialize the network parameters necessary for ping operation only, afterwards the env_changed_id will be set to the env_id that will prevent all following initialization requests from other protocols. The problem is that the initialized by ping subset of network parameters is not sufficient for other protocols and particularly for TFTP which requires the NetServerIp also.
Signed-off-by: michael.zaidman@kodak.com
diff --git a/net/net.c b/net/net.c index a89f6a0..dc98d0f 100644 --- a/net/net.c +++ b/net/net.c @@ -288,64 +288,13 @@ NetInitLoop(proto_t protocol) if (env_changed_id == env_id) return 0;
- switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif -#if defined(CONFIG_CMD_PING) - case PING: -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: -#endif - case NETCONS: - case TFTP: - NetCopyIP(&NetOurIP, &bd->bi_ip_addr); - NetOurGatewayIP = getenv_IPaddr ("gatewayip"); - NetOurSubnetMask= getenv_IPaddr ("netmask"); - NetOurVLAN = getenv_VLAN("vlan"); - NetOurNativeVLAN = getenv_VLAN("nvlan"); + NetCopyIP(&NetOurIP, &bd->bi_ip_addr); + NetOurGatewayIP = getenv_IPaddr ("gatewayip"); + NetOurSubnetMask= getenv_IPaddr ("netmask"); + NetServerIP = getenv_IPaddr ("serverip"); + NetOurNativeVLAN = getenv_VLAN("nvlan"); + NetOurVLAN = getenv_VLAN("vlan");
- switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif - case NETCONS: - case TFTP: - NetServerIP = getenv_IPaddr ("serverip"); - break; -#if defined(CONFIG_CMD_PING) - case PING: - /* nothing */ - break; -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: - /* nothing */ - break; -#endif - default: - break; - } - - break; - case BOOTP: - case RARP: - /* - * initialize our IP addr to 0 in order to accept ANY - * IP addr assigned to us by the BOOTP / RARP server - */ - NetOurIP = 0; - NetServerIP = getenv_IPaddr ("serverip"); - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ - NetOurNativeVLAN = getenv_VLAN("nvlan"); - case CDP: - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ - NetOurNativeVLAN = getenv_VLAN("nvlan"); - break; - default: - break; - } env_changed_id = env_id; return 0; } @@ -443,18 +392,19 @@ restart: /* Start with a clean slate... */ BootpTry = 0; NetOurIP = 0; - NetServerIP = getenv_IPaddr ("serverip"); DhcpRequest(); /* Basically same as BOOTP */ break; #endif
case BOOTP: BootpTry = 0; + NetOurIP = 0; BootpRequest (); break;
case RARP: RarpTry = 0; + NetOurIP = 0; RarpRequest (); break; #if defined(CONFIG_CMD_PING)
Regards, Michael Zaidman, michael.zaidman@kodak.com
Hi Heiko,
The patch "netloop: speed up NetLoop" you delivered into the u-boot-2009.03 introduced bug I have described below a few days ago. Could you please take a look at the proposed fix?
Thanks, Michael
---------- Forwarded message ---------- From: Michael Zaidman michael.zaidman@gmail.com Date: Sun, Mar 22, 2009 at 8:35 PM Subject: [U-Boot] [PATCH] NetLoop initialization bug To: u-boot@lists.denx.de
[U-Boot] [PATCH] NetLoop initialization bug
Upon u-boot's start up the first ping command causes a failure of the consequent TFTP command. It happens in the recently added mechanism of the NetLoop initialization where initialization of global network parameters is separated in the NetInitLoop routine which is called per env_id change. Thus, ping request will initialize the network parameters necessary for ping operation only, afterwards the env_changed_id will be set to the env_id that will prevent all following initialization requests from other protocols. The problem is that the initialized by ping subset of network parameters is not sufficient for other protocols and particularly for TFTP which requires the NetServerIp also.
Signed-off-by: michael.zaidman@gmail.com
diff --git a/net/net.c b/net/net.c index a89f6a0..dc98d0f 100644 --- a/net/net.c +++ b/net/net.c @@ -288,64 +288,13 @@ NetInitLoop(proto_t protocol) if (env_changed_id == env_id) return 0;
- switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif -#if defined(CONFIG_CMD_PING) - case PING: -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: -#endif - case NETCONS: - case TFTP: - NetCopyIP(&NetOurIP, &bd->bi_ip_addr); - NetOurGatewayIP = getenv_IPaddr ("gatewayip"); - NetOurSubnetMask= getenv_IPaddr ("netmask"); - NetOurVLAN = getenv_VLAN("vlan"); - NetOurNativeVLAN = getenv_VLAN("nvlan"); + NetCopyIP(&NetOurIP, &bd->bi_ip_addr); + NetOurGatewayIP = getenv_IPaddr ("gatewayip"); + NetOurSubnetMask= getenv_IPaddr ("netmask"); + NetServerIP = getenv_IPaddr ("serverip"); + NetOurNativeVLAN = getenv_VLAN("nvlan"); + NetOurVLAN = getenv_VLAN("vlan");
- switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif - case NETCONS: - case TFTP: - NetServerIP = getenv_IPaddr ("serverip"); - break; -#if defined(CONFIG_CMD_PING) - case PING: - /* nothing */ - break; -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: - /* nothing */ - break; -#endif - default: - break; - } - - break; - case BOOTP: - case RARP: - /* - * initialize our IP addr to 0 in order to accept ANY - * IP addr assigned to us by the BOOTP / RARP server - */ - NetOurIP = 0; - NetServerIP = getenv_IPaddr ("serverip"); - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ - NetOurNativeVLAN = getenv_VLAN("nvlan"); - case CDP: - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ - NetOurNativeVLAN = getenv_VLAN("nvlan"); - break; - default: - break; - } env_changed_id = env_id; return 0; } @@ -443,18 +392,19 @@ restart: /* Start with a clean slate... */ BootpTry = 0; NetOurIP = 0; - NetServerIP = getenv_IPaddr ("serverip"); DhcpRequest(); /* Basically same as BOOTP */ break; #endif
case BOOTP: BootpTry = 0; + NetOurIP = 0; BootpRequest (); break;
case RARP: RarpTry = 0; + NetOurIP = 0; RarpRequest (); break; #if defined(CONFIG_CMD_PING)
Regards, Michael
Hello Michael,
Michael Zaidman wrote:
Hi Heiko,
The patch "netloop: speed up NetLoop" you delivered into the u-boot-2009.03 introduced bug I have described below a few days ago. Could you please take a look at the proposed fix?
Sorry, seems I missed it.
Thanks, Michael
---------- Forwarded message ---------- From: Michael Zaidman michael.zaidman@gmail.com Date: Sun, Mar 22, 2009 at 8:35 PM Subject: [U-Boot] [PATCH] NetLoop initialization bug To: u-boot@lists.denx.de
[U-Boot] [PATCH] NetLoop initialization bug
Thanks for catching this. Your patch doesn;t apply, I get "malformed patch". Before sending a new one, please have a look at my comments:
Upon u-boot's start up the first ping command causes a failure of the consequent TFTP command. It happens in the recently added mechanism of the NetLoop initialization where initialization of global network parameters is separated in the NetInitLoop routine which is called per env_id change. Thus, ping request will initialize the network parameters necessary for ping operation only, afterwards the env_changed_id will be set to the env_id that will prevent all following initialization requests from other protocols. The problem is that the initialized by ping subset of network parameters is not sufficient for other protocols and particularly for TFTP which requires the NetServerIp also.
Signed-off-by: michael.zaidman@gmail.com
diff --git a/net/net.c b/net/net.c index a89f6a0..dc98d0f 100644 --- a/net/net.c +++ b/net/net.c @@ -288,64 +288,13 @@ NetInitLoop(proto_t protocol) if (env_changed_id == env_id) return 0;
switch (protocol) {-#if defined(CONFIG_CMD_NFS)
case NFS:-#endif -#if defined(CONFIG_CMD_PING)
case PING:-#endif -#if defined(CONFIG_CMD_SNTP)
case SNTP:-#endif
case NETCONS:case TFTP:NetCopyIP(&NetOurIP, &bd->bi_ip_addr);NetOurGatewayIP = getenv_IPaddr ("gatewayip");NetOurSubnetMask= getenv_IPaddr ("netmask");NetOurVLAN = getenv_VLAN("vlan");NetOurNativeVLAN = getenv_VLAN("nvlan");
NetCopyIP(&NetOurIP, &bd->bi_ip_addr);NetOurGatewayIP = getenv_IPaddr ("gatewayip");NetOurSubnetMask= getenv_IPaddr ("netmask");NetServerIP = getenv_IPaddr ("serverip");
The following 2 vars are just used, if CONFIG_CMD_CDP is used, can we do a "#if defined" around it?
NetOurNativeVLAN = getenv_VLAN("nvlan");NetOurVLAN = getenv_VLAN("vlan");
[...]
@@ -443,18 +392,19 @@ restart: /* Start with a clean slate... */ BootpTry = 0; NetOurIP = 0;
NetServerIP = getenv_IPaddr ("serverip"); DhcpRequest(); /* Basically same as BOOTP */ break;#endif
case BOOTP: BootpTry = 0;
NetOurIP = 0;
why we need this here?
BootpRequest (); break; case RARP: RarpTry = 0;
NetOurIP = 0;
also here? Has this to do something with the bugfix?
bye Heiko
Dear Heiko,
In message 49D054BA.1060904@denx.de you wrote:
The following 2 vars are just used, if CONFIG_CMD_CDP is used, can we do a "#if defined" around it?
NetOurNativeVLAN = getenv_VLAN("nvlan");NetOurVLAN = getenv_VLAN("vlan");
We probably could, but we then had to do this in several places, and the code would become less readable, I think.
Best regards,
Wolfgang Denk
Hello Heiko,
Please see my comments and updated patch below.
On Mon, Mar 30, 2009 at 7:12 AM, Heiko Schocher hs@denx.de wrote:
Hello Michael,
Michael Zaidman wrote:
Hi Heiko,
The patch "netloop: speed up NetLoop" you delivered into the u-boot-2009.03 introduced bug I have described below a few days ago. Could you please take a look at the proposed fix?
Sorry, seems I missed it.
Thanks, Michael
---------- Forwarded message ---------- From: Michael Zaidman michael.zaidman@gmail.com Date: Sun, Mar 22, 2009 at 8:35 PM Subject: [U-Boot] [PATCH] NetLoop initialization bug To: u-boot@lists.denx.de
[U-Boot] [PATCH] NetLoop initialization bug
Thanks for catching this. Your patch doesn;t apply, I get "malformed patch". Before sending a new one, please have a look at my comments:
Upon u-boot's start up the first ping command causes a failure of the consequent TFTP command. It happens in the recently added mechanism of the NetLoop initialization where initialization of global network parameters is separated in the NetInitLoop routine which is called per env_id change. Thus, ping request will initialize the network parameters necessary for ping operation only, afterwards the env_changed_id will be set to the env_id that will prevent all following initialization requests from other protocols. The problem is that the initialized by ping subset of network parameters is not sufficient for other protocols and particularly for TFTP which requires the NetServerIp also.
Signed-off-by: michael.zaidman@gmail.com
diff --git a/net/net.c b/net/net.c index a89f6a0..dc98d0f 100644 --- a/net/net.c +++ b/net/net.c @@ -288,64 +288,13 @@ NetInitLoop(proto_t protocol) if (env_changed_id == env_id) return 0;
- switch (protocol) {
-#if defined(CONFIG_CMD_NFS)
- case NFS:
-#endif -#if defined(CONFIG_CMD_PING)
- case PING:
-#endif -#if defined(CONFIG_CMD_SNTP)
- case SNTP:
-#endif
- case NETCONS:
- case TFTP:
- NetCopyIP(&NetOurIP, &bd->bi_ip_addr);
- NetOurGatewayIP = getenv_IPaddr ("gatewayip");
- NetOurSubnetMask= getenv_IPaddr ("netmask");
- NetOurVLAN = getenv_VLAN("vlan");
- NetOurNativeVLAN = getenv_VLAN("nvlan");
- NetCopyIP(&NetOurIP, &bd->bi_ip_addr);
- NetOurGatewayIP = getenv_IPaddr ("gatewayip");
- NetOurSubnetMask= getenv_IPaddr ("netmask");
- NetServerIP = getenv_IPaddr ("serverip");
The following 2 vars are just used, if CONFIG_CMD_CDP is used, can we do a "#if defined" around it?
- NetOurNativeVLAN = getenv_VLAN("nvlan");
- NetOurVLAN = getenv_VLAN("vlan");
These two variables have been initialized in original code for all protocols supported by u-boot. As I can see, at least the NetOurVLAN is used by most of them. This was the reason for keeping them in place.
[...]
@@ -443,18 +392,19 @@ restart: /* Start with a clean slate... */ BootpTry = 0; NetOurIP = 0;
- NetServerIP = getenv_IPaddr ("serverip");
DhcpRequest(); /* Basically same as BOOTP */ break; #endif
case BOOTP: BootpTry = 0;
- NetOurIP = 0;
why we need this here?
Generally, for the same reason the DHCP does - "Start with a clean state..." On the other hand you are right - we do not need to clear the NetOurIP address for BOOTP, DHCP and RARP because the current implementation does not check it. So it has been removed in my new patch below.
BootpRequest (); break;
case RARP: RarpTry = 0;
- NetOurIP = 0;
also here? Has this to do something with the bugfix?
The same as before.
bye Heiko
-- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Here is the updated patch:
Subject: [U-Boot] [PATCH] NetLoop initialization bug
--- net/net.c | 66 +++++------------------------------------------------------- 1 files changed, 6 insertions(+), 60 deletions(-)
diff --git a/net/net.c b/net/net.c index a89f6a0..91ed0d7 100644 --- a/net/net.c +++ b/net/net.c @@ -288,64 +288,13 @@ NetInitLoop(proto_t protocol) if (env_changed_id == env_id) return 0;
- switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif -#if defined(CONFIG_CMD_PING) - case PING: -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: -#endif - case NETCONS: - case TFTP: - NetCopyIP(&NetOurIP, &bd->bi_ip_addr); - NetOurGatewayIP = getenv_IPaddr ("gatewayip"); - NetOurSubnetMask= getenv_IPaddr ("netmask"); - NetOurVLAN = getenv_VLAN("vlan"); - NetOurNativeVLAN = getenv_VLAN("nvlan"); + NetCopyIP(&NetOurIP, &bd->bi_ip_addr); + NetOurGatewayIP = getenv_IPaddr ("gatewayip"); + NetOurSubnetMask= getenv_IPaddr ("netmask"); + NetServerIP = getenv_IPaddr ("serverip"); + NetOurNativeVLAN = getenv_VLAN("nvlan"); + NetOurVLAN = getenv_VLAN("vlan");
- switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif - case NETCONS: - case TFTP: - NetServerIP = getenv_IPaddr ("serverip"); - break; -#if defined(CONFIG_CMD_PING) - case PING: - /* nothing */ - break; -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: - /* nothing */ - break; -#endif - default: - break; - } - - break; - case BOOTP: - case RARP: - /* - * initialize our IP addr to 0 in order to accept ANY - * IP addr assigned to us by the BOOTP / RARP server - */ - NetOurIP = 0; - NetServerIP = getenv_IPaddr ("serverip"); - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ - NetOurNativeVLAN = getenv_VLAN("nvlan"); - case CDP: - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ - NetOurNativeVLAN = getenv_VLAN("nvlan"); - break; - default: - break; - } env_changed_id = env_id; return 0; } @@ -440,10 +389,7 @@ restart:
#if defined(CONFIG_CMD_DHCP) case DHCP: - /* Start with a clean slate... */ BootpTry = 0; - NetOurIP = 0; - NetServerIP = getenv_IPaddr ("serverip"); DhcpRequest(); /* Basically same as BOOTP */ break; #endif
Hello Michael,
Michael Zaidman wrote:
Please see my comments and updated patch below.
On Mon, Mar 30, 2009 at 7:12 AM, Heiko Schocher hs@denx.de wrote:
Hello Michael,
Michael Zaidman wrote:
[...]
The following 2 vars are just used, if CONFIG_CMD_CDP is used, can we do a "#if defined" around it?
NetOurNativeVLAN = getenv_VLAN("nvlan");NetOurVLAN = getenv_VLAN("vlan");These two variables have been initialized in original code for all protocols supported by u-boot. As I can see, at least the NetOurVLAN is used by most of them. This was the reason for keeping them in place.
Ok.
[...]
@@ -443,18 +392,19 @@ restart: /* Start with a clean slate... */ BootpTry = 0; NetOurIP = 0;
NetServerIP = getenv_IPaddr ("serverip"); DhcpRequest(); /* Basically same as BOOTP */ break;#endif
case BOOTP: BootpTry = 0;
NetOurIP = 0;why we need this here?
Generally, for the same reason the DHCP does - "Start with a clean state..." On the other hand you are right - we do not need to clear the NetOurIP address for BOOTP, DHCP and RARP because the current implementation does not check it. So it has been removed in my new patch below.
thanks.
Here is the updated patch:
Subject: [U-Boot] [PATCH] NetLoop initialization bug
Hmm.. can you please add your commit message and your Signed-off-by
thanks Heiko
Hi Michael,
Please see my comments and updated patch below.
As a side note, please send your patch as an inline attachment also adding your signed-off-by line. It's probably easiest to actually use git to apply your changes to a branch and use "git-format-patch" to create the patch.
Please also provide more dsecriptive commit text explaining what the patch does.
The patch in your mail does still not apply. Neither git-am (after fixing the patch with a valid e-mail) nor patch can do anything with it.
Thanks Detlev
On Tue, Mar 31, 2009 at 12:30 PM, Detlev Zundel dzu@denx.de wrote:
Hi Michael,
Please see my comments and updated patch below.
As a side note, please send your patch as an inline attachment also adding your signed-off-by line. It's probably easiest to actually use git to apply your changes to a branch and use "git-format-patch" to create the patch.
Please also provide more dsecriptive commit text explaining what the patch does.
The patch in your mail does still not apply. Neither git-am (after fixing the patch with a valid e-mail) nor patch can do anything with it.
Thanks Detlev
-- ;; Self-replicator in ELisp ((lambda (l) (prin1-to-string (list l (list (quote quote) l)))) (quote (lambda (l) (prin1-to-string (list l (list (quote quote) l)))))) -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu@denx.de
Hi Detlev,
Please try this one. I used the git-format-patch for its creation.
Thanks, Michael
From 8cc38fd564c0671b34afdc36df4c224d51d98334 Mon Sep 17 00:00:00 2001
From: Michael Zaidman michael.zaidman@gmail.com Date: Tue, 31 Mar 2009 14:21:35 +0300 Subject: [PATCH] The patch fixes the bug of partial intialization of global network parameters.
Upon u-boot's start up the first ping command causes a failure of the consequent TFTP transfers. It happens in the recently added mechanism of NetLoop initialization where initialization of global network parameters is separated in the NetInitLoop routine which is called per env_id change. Thus, ping request will initialize the network parameters necessary for ping operation only, afterwards the env_changed_id will be set to the env_id that will prevent all following initialization requests from other protocols. The problem is that the initialized by ping subset of network parameters is not sufficient for other protocols and particularly for TFTP which requires the NetServerIp also.
Signed-off-by: Michael Zaidman michael.zaidman@gmail.com --- net/net.c | 66 +++++------------------------------------------------------- 1 files changed, 6 insertions(+), 60 deletions(-)
diff --git a/net/net.c b/net/net.c index a89f6a0..91ed0d7 100644 --- a/net/net.c +++ b/net/net.c @@ -288,64 +288,13 @@ NetInitLoop(proto_t protocol) if (env_changed_id == env_id) return 0;
- switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif -#if defined(CONFIG_CMD_PING) - case PING: -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: -#endif - case NETCONS: - case TFTP: - NetCopyIP(&NetOurIP, &bd->bi_ip_addr); - NetOurGatewayIP = getenv_IPaddr ("gatewayip"); - NetOurSubnetMask= getenv_IPaddr ("netmask"); - NetOurVLAN = getenv_VLAN("vlan"); - NetOurNativeVLAN = getenv_VLAN("nvlan"); + NetCopyIP(&NetOurIP, &bd->bi_ip_addr); + NetOurGatewayIP = getenv_IPaddr ("gatewayip"); + NetOurSubnetMask= getenv_IPaddr ("netmask"); + NetServerIP = getenv_IPaddr ("serverip"); + NetOurNativeVLAN = getenv_VLAN("nvlan"); + NetOurVLAN = getenv_VLAN("vlan");
- switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif - case NETCONS: - case TFTP: - NetServerIP = getenv_IPaddr ("serverip"); - break; -#if defined(CONFIG_CMD_PING) - case PING: - /* nothing */ - break; -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: - /* nothing */ - break; -#endif - default: - break; - } - - break; - case BOOTP: - case RARP: - /* - * initialize our IP addr to 0 in order to accept ANY - * IP addr assigned to us by the BOOTP / RARP server - */ - NetOurIP = 0; - NetServerIP = getenv_IPaddr ("serverip"); - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ - NetOurNativeVLAN = getenv_VLAN("nvlan"); - case CDP: - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ - NetOurNativeVLAN = getenv_VLAN("nvlan"); - break; - default: - break; - } env_changed_id = env_id; return 0; } @@ -440,10 +389,7 @@ restart:
#if defined(CONFIG_CMD_DHCP) case DHCP: - /* Start with a clean slate... */ BootpTry = 0; - NetOurIP = 0; - NetServerIP = getenv_IPaddr ("serverip"); DhcpRequest(); /* Basically same as BOOTP */ break; #endif
On Tue, Mar 31, 2009 at 12:30 PM, Detlev Zundel dzu@denx.de wrote:
Hi Michael,
[...]
The patch in your mail does still not apply. Neither git-am (after fixing the patch with a valid e-mail) nor patch can do anything with it.
Thanks Detlev
Sorry for the previous mail, I have to enable the git operation via gmail on my workstation.
Meanwhile I created the patch again by diff.
Thanks, Michael
The patch fixes the bug of partial initialization of global network parameters.
Upon u-boot's start up the first ping command causes a failure of the consequent TFTP transfers. It happens in the recently added mechanism of NetLoop initialization where initialization of global network parameters is separated in the NetInitLoop routine which is called per env_id change. Thus, ping request will initialize the network parameters necessary for ping operation only, afterwards the env_changed_id will be set to the env_id that will prevent all following initialization requests from other protocols. The problem is that the initialized by ping subset of network parameters is not sufficient for other protocols and particularly for TFTP which requires the NetServerIp also.
Signed-off-by: Michael Zaidman <michael.zaidman at gmail.com>
michael@michaelz:~/develop$ diff -u u-boot-git/net/net.c u-boot-git-test1/net/net.c --- u-boot-git/net/net.c 2009-03-31 13:56:18.000000000 +0300 +++ u-boot-git-test1/net/net.c 2009-03-30 15:57:14.000000000 +0300 @@ -288,64 +288,13 @@ if (env_changed_id == env_id) return 0;
- switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif -#if defined(CONFIG_CMD_PING) - case PING: -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: -#endif - case NETCONS: - case TFTP: - NetCopyIP(&NetOurIP, &bd->bi_ip_addr); - NetOurGatewayIP = getenv_IPaddr ("gatewayip"); - NetOurSubnetMask= getenv_IPaddr ("netmask"); - NetOurVLAN = getenv_VLAN("vlan"); - NetOurNativeVLAN = getenv_VLAN("nvlan"); + NetCopyIP(&NetOurIP, &bd->bi_ip_addr); + NetOurGatewayIP = getenv_IPaddr ("gatewayip"); + NetOurSubnetMask= getenv_IPaddr ("netmask"); + NetServerIP = getenv_IPaddr ("serverip"); + NetOurNativeVLAN = getenv_VLAN("nvlan"); + NetOurVLAN = getenv_VLAN("vlan");
- switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif - case NETCONS: - case TFTP: - NetServerIP = getenv_IPaddr ("serverip"); - break; -#if defined(CONFIG_CMD_PING) - case PING: - /* nothing */ - break; -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: - /* nothing */ - break; -#endif - default: - break; - } - - break; - case BOOTP: - case RARP: - /* - * initialize our IP addr to 0 in order to accept ANY - * IP addr assigned to us by the BOOTP / RARP server - */ - NetOurIP = 0; - NetServerIP = getenv_IPaddr ("serverip"); - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ - NetOurNativeVLAN = getenv_VLAN("nvlan"); - case CDP: - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ - NetOurNativeVLAN = getenv_VLAN("nvlan"); - break; - default: - break; - } env_changed_id = env_id; return 0; } @@ -440,10 +389,7 @@
#if defined(CONFIG_CMD_DHCP) case DHCP: - /* Start with a clean slate... */ BootpTry = 0; - NetOurIP = 0; - NetServerIP = getenv_IPaddr ("serverip"); DhcpRequest(); /* Basically same as BOOTP */ break; #endif
Hi Michael,
Sorry for the previous mail, I have to enable the git operation via gmail on my workstation.
What do you mean by this? Usually you only need a mailer which is capable to attach things inline. Even better - if you have a direct access to an mta or an smtp server, use git-send-email after git-format-patch.
Meanwhile I created the patch again by diff.
Well this is really more work for the people importing the patches, so if at all possible, get tghe setup working with the automatically generated patches.
The patch fixes the bug of partial initialization of global network parameters.
Upon u-boot's start up the first ping command causes a failure of the consequent TFTP transfers. It happens in the recently added mechanism of NetLoop initialization where initialization of global network parameters is separated in the NetInitLoop routine which is called per env_id change. Thus, ping request will initialize the network parameters necessary for ping operation only, afterwards the env_changed_id will be set to the env_id that will prevent all following initialization requests from other protocols. The problem is that the initialized by ping subset of network parameters is not sufficient for other protocols and particularly for TFTP which requires the NetServerIp also.
Signed-off-by: Michael Zaidman <michael.zaidman at gmail.com>
Please don't elide your e-mail address here. The consent is that we use real e-mail addresses in the signed-off-by lines (as you also did in your previous post). One negative impact for example is that our statistic scripts will fail for such entries.
Apart from that you have to nudge Ben or Heiko as to the real content. I only wanted to help get the formalities straight ;)
Cheers Detlev
On Thu, Apr 2, 2009 at 3:47 PM, Detlev Zundel dzu@denx.de wrote:
I have to enable the git operation via gmail on my workstation.
What do you mean by this? Usually you only need a mailer which is capable to attach things inline. Even better - if you have a direct access to an mta or an smtp server, use git-send-email after git-format-patch.
Ok, I tried the msmtp to access my www.gmail.com account with no success nether directly (is prohibited by corporate policy) nor via corporate email relay (the gmail smtp server uses non standard port which requires reflection in the email relay configuration and no one is willing to do it for me :-).
Apart from that you have to nudge Ben or Heiko as to the real content. I only wanted to help get the formalities straight ;)
Cheers Detlev
Dear Heiko,
Could you please do an additional try to apply the patch produced with git-format-patch. I hope it is OK now.
Thanks, Michael
From 774a3d83c4363a0bbeb0616786f522877882c88b Mon Sep 17 00:00:00 2001
From: Michael Zaidman michael.zaidman@gmail.com Date: Thu, 2 Apr 2009 18:25:03 +0300 Subject: [U-Boot] [PATCH] NetLoop initialization bug
The patch fixes the bug of partial initialization of global network parameters.
Upon u-boot's start up the first ping command causes a failure of the consequent TFTP transfers. It happens in the recently added mechanism of NetLoop initialization where initialization of global network parameters is separated in the NetInitLoop routine which is called per env_id change. Thus, ping request will initialize the network parameters necessary for ping operation only, afterwards the env_changed_id will be set to the env_id that will prevent all following initialization requests from other protocols. The problem is that the initialized by ping subset of network parameters is not sufficient for other protocols and particularly for TFTP which requires the NetServerIp also.
Signed-off-by: Michael Zaidman michael.zaidman@gmail.com --- net/net.c | 62 ++---------------------------------------------------------- 1 files changed, 3 insertions(+), 59 deletions(-)
diff --git a/net/net.c b/net/net.c index a89f6a0..edb4477 100644 --- a/net/net.c +++ b/net/net.c @@ -285,68 +285,15 @@ NetInitLoop(proto_t protocol) int env_id = get_env_id ();
/* update only when the environment has changed */ - if (env_changed_id == env_id) - return 0; - - switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif -#if defined(CONFIG_CMD_PING) - case PING: -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: -#endif - case NETCONS: - case TFTP: + if (env_changed_id != env_id) { NetCopyIP(&NetOurIP, &bd->bi_ip_addr); NetOurGatewayIP = getenv_IPaddr ("gatewayip"); NetOurSubnetMask= getenv_IPaddr ("netmask"); - NetOurVLAN = getenv_VLAN("vlan"); - NetOurNativeVLAN = getenv_VLAN("nvlan"); - - switch (protocol) { -#if defined(CONFIG_CMD_NFS) - case NFS: -#endif - case NETCONS: - case TFTP: - NetServerIP = getenv_IPaddr ("serverip"); - break; -#if defined(CONFIG_CMD_PING) - case PING: - /* nothing */ - break; -#endif -#if defined(CONFIG_CMD_SNTP) - case SNTP: - /* nothing */ - break; -#endif - default: - break; - } - - break; - case BOOTP: - case RARP: - /* - * initialize our IP addr to 0 in order to accept ANY - * IP addr assigned to us by the BOOTP / RARP server - */ - NetOurIP = 0; NetServerIP = getenv_IPaddr ("serverip"); - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ NetOurNativeVLAN = getenv_VLAN("nvlan"); - case CDP: - NetOurVLAN = getenv_VLAN("vlan"); /* VLANs must be read */ - NetOurNativeVLAN = getenv_VLAN("nvlan"); - break; - default: - break; + NetOurVLAN = getenv_VLAN("vlan"); + env_changed_id = env_id; } - env_changed_id = env_id; return 0; }
@@ -440,10 +387,7 @@ restart:
#if defined(CONFIG_CMD_DHCP) case DHCP: - /* Start with a clean slate... */ BootpTry = 0; - NetOurIP = 0; - NetServerIP = getenv_IPaddr ("serverip"); DhcpRequest(); /* Basically same as BOOTP */ break; #endif
Hi Michael,
Michael Zaidman wrote:
On Thu, Apr 2, 2009 at 3:47 PM, Detlev Zundel dzu@denx.de wrote:
I have to enable the git operation via gmail on my workstation.
What do you mean by this? Usually you only need a mailer which is capable to attach things inline. Even better - if you have a direct access to an mta or an smtp server, use git-send-email after git-format-patch.
Ok, I tried the msmtp to access my www.gmail.com account with no success nether directly (is prohibited by corporate policy) nor via corporate email relay (the gmail smtp server uses non standard port which requires reflection in the email relay configuration and no one is willing to do it for me :-).
Apart from that you have to nudge Ben or Heiko as to the real content. I only wanted to help get the formalities straight ;)
Cheers Detlev
Dear Heiko,
Could you please do an additional try to apply the patch produced with git-format-patch. I hope it is OK now.
I tried to apply it but it didn't work, but this isn't my usual computer. I'll try on my other machine tonight. BTW - what mailer are you using?
regards, Ben
Dear Ben Warren,
In message 49D5338C.1050004@gmail.com you wrote:
Could you please do an additional try to apply the patch produced with git-format-patch. I hope it is OK now.
I tried to apply it but it didn't work, but this isn't my usual computer. I'll try on my other machine tonight. BTW - what mailer are you using?
It fails for me, too.
Best regards,
Wolfgang Denk
Hi Ben,
On Fri, Apr 3, 2009 at 12:52 AM, Ben Warren biggerbadderben@gmail.com wrote:
I tried to apply it but it didn't work, but this isn't my usual computer. I'll try on my other machine tonight. BTW - what mailer are you using?
I used Firefox as front-end web interface to communicate with gmail server - the only option I have at my work place. It obviously caused the problem. The http://kerneltrap.org/Linux/Email_Clients_and_Patches perfectly describes this. Today I created the patch on my home's Linux with git-format-patch and sent it by git-send-email via msmtp. I would like to apologize for inconvenience and ask you to try patch again.
BTW - what mailer do you use to communicate to the gmail server?
Thanks, Michael
Dear Michael & Ben,
In message 660c0f820904031320s392b010bx8796823194bcd77a@mail.gmail.com you wrote:
I used Firefox as front-end web interface to communicate with gmail server - the only option I have at my work place. It obviously caused the problem. The http://kerneltrap.org/Linux/Email_Clients_and_Patches perfectly describes this. Today I created the patch on my home's Linux with git-format-patch and sent it by git-send-email via msmtp. I would like to apologize for inconvenience and ask you to try patch again.
I confirm that this patch applies cleanly.
[I cannot comment on the content, though.]
Best regards,
Wolfgang Denk
Wolfgang Denk wrote:
Dear Michael & Ben,
In message 660c0f820904031320s392b010bx8796823194bcd77a@mail.gmail.com you wrote:
I used Firefox as front-end web interface to communicate with gmail server - the only option I have at my work place. It obviously caused the problem. The http://kerneltrap.org/Linux/Email_Clients_and_Patches perfectly describes this. Today I created the patch on my home's Linux with git-format-patch and sent it by git-send-email via msmtp. I would like to apologize for inconvenience and ask you to try patch again.
I confirm that this patch applies cleanly.
[I cannot comment on the content, though.]
Best regards,
Wolfgang Denk
Cool - I'll try it out soon.
regards, Ben
Hi Michael,
Michael Zaidman wrote:
Hi Ben,
On Fri, Apr 3, 2009 at 12:52 AM, Ben Warren biggerbadderben@gmail.com wrote:
I tried to apply it but it didn't work, but this isn't my usual computer. I'll try on my other machine tonight. BTW - what mailer are you using?
I used Firefox as front-end web interface to communicate with gmail server - the only option I have at my work place. It obviously caused the problem. The http://kerneltrap.org/Linux/Email_Clients_and_Patches perfectly describes this. Today I created the patch on my home's Linux with git-format-patch and sent it by git-send-email via msmtp. I would like to apologize for inconvenience and ask you to try patch again.
BTW - what mailer do you use to communicate to the gmail server?
I use Thunderbird via IMAP. IMAP is nice because the messages all stay on the server so if you have a few different machines and OSs you don't have to worry about keeping things coherent. Sending patches through TBird usually works, as long as you cut&paste into a 'preformat' block (available from a pull-down menu). But it's not bulletproof. The only sure way to send a patch without worrying about line wrapping etc. is to use git-send-email. You can find instructions online on how to do this with gmail, IIRC you need to have some sendmail-like relay running on your Linux box. I don't remember the details and am not at that computer right now.
cheers, Ben
Hi Ben,
Ben Warren wrote:
Hi Michael,
Michael Zaidman wrote:
Hi Ben,
On Fri, Apr 3, 2009 at 12:52 AM, Ben Warren biggerbadderben@gmail.com wrote:
I tried to apply it but it didn't work, but this isn't my usual computer. I'll try on my other machine tonight. BTW - what mailer are you using?
I used Firefox as front-end web interface to communicate with gmail server - the only option I have at my work place. It obviously caused the problem. The http://kerneltrap.org/Linux/Email_Clients_and_Patches perfectly describes this. Today I created the patch on my home's Linux with git-format-patch and sent it by git-send-email via msmtp. I would like to apologize for inconvenience and ask you to try patch again.
BTW - what mailer do you use to communicate to the gmail server?
I use Thunderbird via IMAP. IMAP is nice because the messages all stay on the server so if you have a few different machines and OSs you don't have to worry about keeping things coherent. Sending patches through TBird usually works, as long as you cut&paste into a 'preformat' block (available from a pull-down menu). But it's not bulletproof. The only sure way to send a patch without worrying about line wrapping etc. is to use git-send-email. You can find instructions online on how to do this with gmail, IIRC you need to have some sendmail-like relay running on your Linux box. I don't remember the details and am not at that computer right now.
cheers, Ben
Thanks for the info. I played today with the Evolution which also keeps mails on the server and has 'preformated' option trying to compose correct inline patch attachment but with no success.
Good night, Michael
participants (5)
-
Ben Warren -
Detlev Zundel -
Heiko Schocher -
Michael Zaidman -
Wolfgang Denk