[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 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 --- 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 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
Hi Ben, Ben Warren wrote: > Hi Michael, > > Michael Zaidman wrote: >> >> Hi Ben, >> >> On Fri, Apr 3, 2009 at 12:52 AM, Ben Warren >> 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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[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 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 --- 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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
Wolfgang Denk wrote: > Dear Michael & Ben, > > In message <660c0f820904031320s392b010bx8796823194bcd...@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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
Hi Michael, Michael Zaidman wrote: > Hi Ben, > > On Fri, Apr 3, 2009 at 12:52 AM, Ben Warren 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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
Dear Michael & Ben, In message <660c0f820904031320s392b010bx8796823194bcd...@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: w...@denx.de Each kiss is as the first. -- Miramanee, Kirk's wife, "The Paradise Syndrome", stardate 4842.6 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
Hi Ben, On Fri, Apr 3, 2009 at 12:52 AM, Ben Warren 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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
Dear Ben Warren, In message <49d5338c.1050...@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: w...@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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
Hi Michael, Michael Zaidman wrote: > On Thu, Apr 2, 2009 at 3:47 PM, Detlev Zundel 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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
On Thu, Apr 2, 2009 at 3:47 PM, Detlev Zundel 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 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 --- 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 -- 1.5.6.3 ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
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 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: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
On Tue, Mar 31, 2009 at 12:30 PM, Detlev Zundel 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 mich...@michaelz:~/develop$ diff -u u-boot-git/net/net.c u-boot-git-test1/net/net.c --- u-boot-git/net/net.c2009-03-31 13:56:18.0 +0300 +++ u-boot-git-test1/net/net.c 2009-03-30 15:57:14.0 +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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
On Tue, Mar 31, 2009 at 12:30 PM, Detlev Zundel 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: d...@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 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 --- 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
Re: [U-Boot] [PATCH] NetLoop initialization bug
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: d...@denx.de ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
Hello Michael, Michael Zaidman wrote: > Please see my comments and updated patch below. > > On Mon, Mar 30, 2009 at 7:12 AM, Heiko Schocher 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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
Hello Heiko, Please see my comments and updated patch below. On Mon, Mar 30, 2009 at 7:12 AM, Heiko Schocher 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 >> 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.zaid...@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,
Re: [U-Boot] [PATCH] NetLoop initialization bug
Dear Heiko, In message <49d054ba.1060...@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: w...@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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH] NetLoop initialization bug
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 > 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.zaid...@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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[U-Boot] [PATCH] NetLoop initialization bug
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 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.zaid...@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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
[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.zaid...@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.zaid...@kodak.com ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot