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