[U-Boot] [PATCH] NetLoop initialization bug

[U-Boot] [PATCH] NetLoop initialization bug

[Michael Zaidman]

[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 at 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 at kodak.com

[U-Boot] [PATCH] NetLoop initialization bug

[Michael Zaidman]

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

[U-Boot] [PATCH] NetLoop initialization bug

[Heiko Schocher]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany 

[U-Boot] [PATCH] NetLoop initialization bug

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Q:  How many DEC repairman does it take to fix a flat ?
A:  Five; four to hold the car up and one to swap tires.

[U-Boot] [PATCH] NetLoop initialization bug

[Michael Zaidman]

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 at 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 at 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
-- 
1.5.6.3

Thanks,
Michael

[U-Boot] [PATCH] NetLoop initialization bug

[Heiko Schocher]

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
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

[U-Boot] [PATCH] NetLoop initialization bug

[Detlev Zundel]

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 at denx.de

[U-Boot] [PATCH] NetLoop initialization bug

[Michael Zaidman]

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 at denx.de
>

Hi Detlev,

Please try this one. I used the git-format-patch for its creation.

Thanks,
Michael

[U-Boot] [PATCH] NetLoop initialization bug

[Michael Zaidman]

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@gmail.com>


michael at 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

[U-Boot] [PATCH] NetLoop initialization bug

[Detlev Zundel]

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

-- 
Number theorists do it perfectly and rationally.
--
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 at denx.de

[U-Boot] [PATCH] NetLoop initialization bug

[Michael Zaidman]

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

[U-Boot] [PATCH] NetLoop initialization bug

[Ben Warren]

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

[U-Boot] [PATCH] NetLoop initialization bug

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
"Ja, mach' nur einen Plan,    sei nur ein grosses Licht
und mach' dann noch 'nen zweiten Plan,    geh'n tun sie beide nicht."
                                     -- Bert Brecht, Dreigroschenoper

[U-Boot] [PATCH] NetLoop initialization bug

[Michael Zaidman]

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

[U-Boot] [PATCH] NetLoop initialization bug

[Wolfgang Denk]

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

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Each kiss is as the first.
	-- Miramanee, Kirk's wife, "The Paradise Syndrome",
	   stardate 4842.6

[U-Boot] [PATCH] NetLoop initialization bug

[Ben Warren]

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

[U-Boot] [PATCH] NetLoop initialization bug

[Ben Warren]

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

[U-Boot] [PATCH] NetLoop initialization bug

[Michael Zaidman]

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

[U-Boot] [PATCH] NetLoop initialization bug

[Michael Zaidman]

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 =3D get_env_id ();

 	/* update only when the environment has changed */
-	if (env_changed_id =3D=3D 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 !=3D env_id) {
 		NetCopyIP(&NetOurIP, &bd->bi_ip_addr);
 		NetOurGatewayIP =3D getenv_IPaddr ("gatewayip");
 		NetOurSubnetMask=3D getenv_IPaddr ("netmask");
-		NetOurVLAN =3D getenv_VLAN("vlan");
-		NetOurNativeVLAN =3D getenv_VLAN("nvlan");
-
-		switch (protocol) {
-#if defined(CONFIG_CMD_NFS)
-		case NFS:
-#endif
-		case NETCONS:
-		case TFTP:
-			NetServerIP =3D 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 =3D 0;
 		NetServerIP =3D getenv_IPaddr ("serverip");
-		NetOurVLAN =3D getenv_VLAN("vlan");	/* VLANs must be read */
 		NetOurNativeVLAN =3D getenv_VLAN("nvlan");
-	case CDP:
-		NetOurVLAN =3D getenv_VLAN("vlan");	/* VLANs must be read */
-		NetOurNativeVLAN =3D getenv_VLAN("nvlan");
-		break;
-	default:
-		break;
+		NetOurVLAN =3D getenv_VLAN("vlan");
+		env_changed_id =3D env_id;
 	}
-	env_changed_id =3D env_id;
 	return 0;
 }

@@ -440,10 +387,7 @@ restart:

 #if defined(CONFIG_CMD_DHCP)
 		case DHCP:
-			/* Start with a clean slate... */
 			BootpTry =3D 0;
-			NetOurIP =3D 0;
-			NetServerIP =3D getenv_IPaddr ("serverip");
 			DhcpRequest();		/* Basically same as BOOTP */
 			break;
 #endif
--=20
1.5.6.3

[U-Boot] [PATCH] NetLoop initialization bug

[Michael Zaidman]

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 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 <michael.zaidman@gmail.com>
---
 net/net.c |   61 +++----------------------------------------------------------
 1 files changed, 3 insertions(+), 58 deletions(-)

diff --git a/net/net.c b/net/net.c
index a89f6a0..97883df 100644
--- a/net/net.c
+++ b/net/net.c
@@ -285,67 +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;
 	return 0;
 }
@@ -440,10 +388,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
-- 
1.5.6.3

[U-Boot] [PATCH] NetLoop initialization bug

[Michael Zaidman]

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 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 <michael.zaidman@gmail.com>
---
 net/net.c |   63 +++---------------------------------------------------------
 1 files changed, 4 insertions(+), 59 deletions(-)

diff --git a/net/net.c b/net/net.c
index a89f6a0..b8648bd 100644
--- a/net/net.c
+++ b/net/net.c
@@ -285,68 +285,16 @@ 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 +388,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
-- 
1.5.6.3