| Message ID | 7D46E86EC0A8354091174257B2FED10138DD6205@DLEE12.ent.ti.com |
|---|---|
| State | Rejected |
| Headers | show |
Dear "Maupin, Chase", In message <7D46E86EC0A8354091174257B2FED10138DD6205@DLEE12.ent.ti.com> you wrote: > > I recently was trying out a Linksys WRT54G2 V1 router and Thi sis out-of-tree code, so we cannot help much - we hav eno idea which modifications they made to the code. > So the net effect was that the dhcp server in this router was > providing information in its response packet that was overriding the > setting I had programmed. I dug into the u-boot code a little and This is normal, and intended. If you run DHCP, the information returned from the server is put in the respective environment variables. That's how it works. Note however, that of course this will not change the persistent copy of the environment - to do that, you would have to run "saveenv". > The first was to set CONFIG_BOOTP_SERVERIP in my board config file. > I discarded this because this would seem to make me always ignore the > serverip setting from the DHCP server, and it seemed likely that > there were cases where you would want this. I don;t understand your description here, but indeed, putting CONFIG_BOOTP_SERVERIP in the config file almost never makes sense, and it is definitly not acceptable for mainline. > The second was to check if the serverip was already set in the > environment and if so then not override that value with the one in > the response from the DHCP server. The below patch snippet does this This is not what is supposed to happen. You are breaking regular functionality this way. > Is this the proper way to fix this issue and the right location to > make the change? The overall goal was to make sure that if I > specified a particular serverip that I wanted to use, then the DHCP > server should not be changing that. No, this is not an acceptable patch. What exactly _are_ you trying to fix? What is wrong with using the DHCP server supplied address? If it is incorrect, you should fix your DHCP server configuration. Best regards, Wolfgang Denk
On 04/18/2012 11:28 AM, Wolfgang Denk wrote: > >> Is this the proper way to fix this issue and the right location to >> make the change? The overall goal was to make sure that if I >> specified a particular serverip that I wanted to use, then the DHCP >> server should not be changing that. > No, this is not an acceptable patch. > > What exactly _are_ you trying to fix? What is wrong with using the > DHCP server supplied address? If it is incorrect, you should fix your > DHCP server configuration. I have the same type of problem. In my case our company controls the DHCP server that hands out addresses for the network, but I want to use a *local* TFTP server (the one on my development machine). I can't modify the DHCP server so if I use "dhcp" to get and address u-boot also overwrites serverip. At home my FiOS router has a DHCP server (that I can't modify since its closed and is needed to provide addresses to my TV set top box) that provides addresses for my network, and has the same problem... Would an option to dhcp (i.e. one that tells it to *only* modify ipaddr) suffice? Or would it be better to provide an entirely a new command that optionally updates the variables in the environment be better? Then users have the option of using "dhcp" as is, or for environments like mine (and apparently Chase) can just get an IP address and go from there...
> -----Original Message----- > From: Wolfgang Denk [mailto:wd@denx.de] > Sent: Wednesday, April 18, 2012 10:28 AM > To: Maupin, Chase > Cc: u-boot@lists.denx.de; Rini, Tom > Subject: Re: [U-Boot] [RFC] Preventing overriding of serverip when > set in environment by user > > Dear "Maupin, Chase", > > In message > <7D46E86EC0A8354091174257B2FED10138DD6205@DLEE12.ent.ti.com> you > wrote: > > > > I recently was trying out a Linksys WRT54G2 V1 router and > > Thi sis out-of-tree code, so we cannot help much - we hav eno idea > which modifications they made to the code. > > > So the net effect was that the dhcp server in this router was > > providing information in its response packet that was overriding > the > > setting I had programmed. I dug into the u-boot code a little and > > This is normal, and intended. If you run DHCP, the information > returned from the server is put in the respective environment > variables. That's how it works. > > Note however, that of course this will not change the persistent > copy > of the environment - to do that, you would have to run "saveenv". > > > The first was to set CONFIG_BOOTP_SERVERIP in my board config > file. > > I discarded this because this would seem to make me always ignore > the > > serverip setting from the DHCP server, and it seemed likely that > > there were cases where you would want this. > > I don;t understand your description here, but indeed, putting > CONFIG_BOOTP_SERVERIP in the config file almost never makes sense, > and > it is definitly not acceptable for mainline. > > > The second was to check if the serverip was already set in the > > environment and if so then not override that value with the one > in > > the response from the DHCP server. The below patch snippet does > this > > This is not what is supposed to happen. You are breaking regular > functionality this way. > > > Is this the proper way to fix this issue and the right location > to > > make the change? The overall goal was to make sure that if I > > specified a particular serverip that I wanted to use, then the > DHCP > > server should not be changing that. > > No, this is not an acceptable patch. > > What exactly _are_ you trying to fix? What is wrong with using the > DHCP server supplied address? If it is incorrect, you should fix > your > DHCP server configuration. > I can see your point. Why treat serverip any different than the ipaddr returned from the DHCP server? I guess this can be worked around by resetting serverip between the dhcp command and the tftp command (using autoload off). Or I can take the approach I am currently using which is to put the linksys router away and buy a cheap trendnet router that doesn't do this :) > 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@denx.de > You may call me by my name, Wirth, or by my value, Worth. > - Nicklaus Wirth
Dear Peter Barada, In message <4F8EE1E1.1060201@logicpd.com> you wrote: > > > What exactly _are_ you trying to fix? What is wrong with using the > > DHCP server supplied address? If it is incorrect, you should fix your > > DHCP server configuration. > I have the same type of problem. > > In my case our company controls the DHCP server that hands out addresses > for the network, but I want to use a *local* TFTP server (the one on my > development machine). I can't modify the DHCP server so if I use "dhcp" > to get and address u-boot also overwrites serverip. At home my FiOS If you know the DHCP server provides an address you don;t want to use, and if you know the address that you want to use, then why don't you just use it? Something like: => setenv tftpserverip 192.168.12.34 => setenv autoload no => dhcp => setenv ipaddr ${tftpserverip} => tftp ... ? > Would an option to dhcp (i.e. one that tells it to *only* modify ipaddr) > suffice? Or would it be better to provide an entirely a new command that > optionally updates the variables in the environment be better? Then > users have the option of using "dhcp" as is, or for environments like > mine (and apparently Chase) can just get an IP address and go from there... Why make it complicated when a few simple commands are sufficient? Best regards, Wolfgang Denk
Dear Chase, In message <7D46E86EC0A8354091174257B2FED10138DD6FB8@DLEE12.ent.ti.com> you wrote: > > Sorry, I didn't mean I was talking about the code on the router. I meant I was using the router as my DHCP server for my Linux host and my development board. Cab you please restrict your line length to some 70 characters or so? Thanks. Well, when it's your box, then why don't you configure the DHCP server correctly? > First, thanks for your response. What I am trying to fix is that I > have a development board that is connected to my Linux host using a > Linksys WRT54G2 router. That particular router is returning a setting > in DHCP that points to the Linksys router itself as the server. In > this case I want my serverip to point to my Linux host which is > running a TFTP server. As mentioned, all you have to do is to configure your DHCP server correctly. It is perfectly normal to use different servers for DHCP and TFTP. All you have to do is to configure the "next-server" parameter in DHCP to point to the TFTP server > Other routers such as a Trendnet WRLS-N300 do not exhibit this > problem so this seems to be an issue with the DHCP response on the > Linksys router. ... This is a configuration option for the DHCP server. > ... However, thinking about this my reasoning was that if > a user were to take the time to set a value for serverip in their > u-boot environment then the network router should not override that Who says it was the user who set this variable? It could as well be the result from a previous call to DHCP which returned different results (for example, because there is a pool of DHCP servers). > setting. I think of this as who should be the authority in the > system. If the user has specified a setting shouldn't that have more > authority than the default setup of the DHCP server, since the user > should be more knowledgeable about their particular configuration? There is a pretty clear definition here: trhe documentation says that the DHCP command may set a certain number of (documented!) environment variables. If you think you want to ignore any of these settings, then just make sure to set these to the values you like after running the DHCP command, thus ignoring the DHCP settings. This can be done with 2 lines of trivial scripting and does not require any code changes (see my previous message). > As for fixing the DHCP server configuration I see two cases where > this can be an issue: > > 1. In the case of this Linksys router I found no option allowing me > to change the DHCP server configuration other than to change what > range of addresses it provides. So this would lead to needing to Are you absolutely sure that the Linksys router does not support the "next-server" parameter in it's configuration? > modify the router firmware. 2. What about an environment with a > shared DHCP server, where often times you may want the default value > of the DHCP server for 95% of your use cases, but a particular user > may want to override this setting. Then just ognore the resuylts returnd by the DHCP server. It is all at your hands. Nobody forces you to use the address returned by the DHCP call for your TFTP downloads. > The goal of this patch was to enable user's to override the DHCP > server setting by taking a pro-active action of defining serverip. In But this is wrong, as you cannot know how the nvironment variable got set, and if it actually contains a usable value. DHCP is _supposed_ to overwrite this variable with a valid setting. Ofcourse, it is up to you if you use these results, or if you ignore them. > server could provide a value if it was configured this way. Again, it > is all back to who should have ultimate authority in the system. But, It is the user who has this authority. Which is exactly why I will not accept this patch - it would implement some policy which you think fits for your purposes - but it does not fit in general. > I can also understand your position in that if I asked the DHCP > server for information I should honor the information it gives me, > and if the DHCP server is giving bad information then fix the server > (which may not always be possible/practical). It would always be > possible to work around this by turning autoload off and then > changing the bootcmd to do: > > dhcp; setenv serverip xxx.xxx.xxx.xxx; tftp > > Would that be our recommendation? Yes, indeed. If you want to ignore the server reply, then please do this explicitly. Best regards, Wolfgang Denk
On 18/04/2012 17:46, Peter Barada wrote: > On 04/18/2012 11:28 AM, Wolfgang Denk wrote: >> >>> Is this the proper way to fix this issue and the right location to >>> make the change? The overall goal was to make sure that if I >>> specified a particular serverip that I wanted to use, then the DHCP >>> server should not be changing that. >> No, this is not an acceptable patch. >> >> What exactly _are_ you trying to fix? What is wrong with using the >> DHCP server supplied address? If it is incorrect, you should fix your >> DHCP server configuration. > I have the same type of problem. > > In my case our company controls the DHCP server that hands out addresses > for the network, but I want to use a *local* TFTP server (the one on my > development machine). I can't modify the DHCP server so if I use "dhcp" > to get and address u-boot also overwrites serverip. At home my FiOS > router has a DHCP server (that I can't modify since its closed and is > needed to provide addresses to my TV set top box) that provides > addresses for my network, and has the same problem... > > Would an option to dhcp (i.e. one that tells it to *only* modify ipaddr) > suffice? Or would it be better to provide an entirely a new command that > optionally updates the variables in the environment be better? Then > users have the option of using "dhcp" as is, or for environments like > mine (and apparently Chase) can just get an IP address and go from there... There is already such as option. You can pass the tftp server in the "tftp"command, see code: tftp [loadAddress] [[hostIPaddr:]bootfilename] for example, something like tftp 0x80008000 192.168.1.1:uImage Is it not enough ? Best regards, Stefano Babic
> -----Original Message----- > From: Stefano Babic [mailto:sbabic@denx.de] > Sent: Thursday, April 19, 2012 4:14 AM > To: Peter Barada > Cc: u-boot@lists.denx.de; Maupin, Chase > Subject: Re: [U-Boot] [RFC] Preventing overriding of serverip > when set in environment by user > > On 18/04/2012 17:46, Peter Barada wrote: > > On 04/18/2012 11:28 AM, Wolfgang Denk wrote: > >> > >>> Is this the proper way to fix this issue and the right > location to > >>> make the change? The overall goal was to make sure that if I > >>> specified a particular serverip that I wanted to use, then > the DHCP > >>> server should not be changing that. > >> No, this is not an acceptable patch. > >> > >> What exactly _are_ you trying to fix? What is wrong with > using the > >> DHCP server supplied address? If it is incorrect, you should > fix your > >> DHCP server configuration. > > I have the same type of problem. > > > > In my case our company controls the DHCP server that hands out > addresses > > for the network, but I want to use a *local* TFTP server (the > one on my > > development machine). I can't modify the DHCP server so if I > use "dhcp" > > to get and address u-boot also overwrites serverip. At home my > FiOS > > router has a DHCP server (that I can't modify since its closed > and is > > needed to provide addresses to my TV set top box) that provides > > addresses for my network, and has the same problem... > > > > Would an option to dhcp (i.e. one that tells it to *only* > modify ipaddr) > > suffice? Or would it be better to provide an entirely a new > command that > > optionally updates the variables in the environment be better? > Then > > users have the option of using "dhcp" as is, or for > environments like > > mine (and apparently Chase) can just get an IP address and go > from there... > > There is already such as option. You can pass the tftp server in > the > "tftp"command, see code: > > tftp [loadAddress] [[hostIPaddr:]bootfilename] > > for example, something like > tftp 0x80008000 192.168.1.1:uImage > > Is it not enough ? I think it will be. This can be worked around and I had a misunderstanding of how the DHCP response was being used. Thanks. > > Best regards, > Stefano Babic > > -- > ================================================================= > ==== > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev > Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, > Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: > sbabic@denx.de > ================================================================= > ====
diff --git a/net/bootp.c b/net/bootp.c index a003c42..186a7ac 100644 --- a/net/bootp.c +++ b/net/bootp.c @@ -106,11 +106,19 @@ static int BootpCheckPkt(uchar *pkt, unsigned dest, unsigned src, unsigned len) static void BootpCopyNetParams(Bootp_t *bp) { IPaddr_t tmp_ip; + IPaddr_t tmp_serverip; NetCopyIP(&NetOurIP, &bp->bp_yiaddr); #if !defined(CONFIG_BOOTP_SERVERIP) NetCopyIP(&tmp_ip, &bp->bp_siaddr); - if (tmp_ip != 0) + + /* Check if the serverip variable is set. If so then do not set + NetServerIP which will result in overwriting the serverip variable + in the environment. + */ + tmp_serverip = getenv_IPaddr("serverip"); + + if (!tmp_serverip && (tmp_ip != 0)) NetCopyIP(&NetServerIP, &bp->bp_siaddr); memcpy (NetServerEther, ((Ethernet_t *)NetRxPacket)->et_src, 6); #endif