DHCP lease handling

[GoogleCodeExporter]

I am using the 1.0 version of the IDE, and I am glad to see that DHCP has been 
included in the official Ethernet library; however, I was wondering how the 
leases are handled by the system.

In particular, one popular DHCP library for pre-1.0 versions had a function 
that you were required to call from time to time so that the lease could be 
renewed; is this handled automatically by the Ethernet library now? And if not, 
I think it should be added.

Original issue reported on code.google.com by alessand...@gmail.com on 17 Nov 2011 at 12:42

[GoogleCodeExporter]

Hi Alessandro,

The DHCP code in Arduino 1.0 doesn't automatically handle renewing the lease, 
and you're right that it should be added.

As you suggest, having a function that should be called from time to time, 
which will renew the lease if needed, would seem the most sensible solution.  
If anyone wants to submit a patch or pull request that would be cool, otherwise 
it will get added in a while when I get chance.

Cheers,

Adrian.

Original comment by adrian.m...@gmail.com on 9 Dec 2011 at 12:52

[GoogleCodeExporter]

I have made a shot at the lease renew/rebind functionality.
Either pull request https://github.com/arduino/Arduino/pull/54
or the attached patch.

Original comment by pe...@birchroad.net on 13 Dec 2011 at 8:13

Attachments:

• fix_issue_716_r1.patch

[GoogleCodeExporter]

I updated the pull request with code so that the IP address used will be 
updated in case of changes when renewing.

Original comment by pe...@birchroad.net on 16 Dec 2011 at 6:59

[GoogleCodeExporter]

Is it necessary to periodically call a method for renewing the lease or is this 
operation automatically done? Thanks

Original comment by Scato...@gmail.com on 20 Feb 2012 at 3:23

[GoogleCodeExporter]

You should periodically call .maintain() on the Ethernet class.
You could do it once every loop since it keeps its internal renew/rebind timer 
and it adds very little overhead if none of the timers are reached.
I'm pretty sure .maintain should be called at least every 1/3 of the lease time 
set in the server but it depends on the server implementation.
I certanly recommend whats in the pull request. I'll see if I can make a patch 
for that as well.

I also recommend the fix in issue #669 since that helps out with some UDP 
protocol issues.

Original comment by pe...@birchroad.net on 20 Feb 2012 at 5:02

[GoogleCodeExporter]

Updated patch with everything thats in the pull request.
Improvements from _r1 is that it actually changes the ip if so required after 
renew/rebind.

Original comment by pe...@birchroad.net on 21 Feb 2012 at 7:15

Attachments:

• fix_issue_716_r2.patch

[GoogleCodeExporter]

thank you very much for your answer and for the patch

Original comment by paolo.ca...@gmail.com on 21 Feb 2012 at 8:45

[GoogleCodeExporter]

Yet an update. This time I added a return status on Ethernet.maintain() as 
discussed on the now closed pull request. (it's not included yet, I just closed 
the request)

Attached is a patch with all previous changes and a modified 
DhcpAddressPrinter.ino I used for testing described below.
The conclusion of the tests... it works.

The test setup:
A Windows 7 machine was running a DHCP server from 
http://www.dhcpserver.de/dhcpsrv.htm set up on ip 192.168.85.1 and giving out 
addresses in the range from 192.168.85.10 to .50.
The server was set to have a 120 seconds lease time.
Wireshark, the protocol analyzer, was also running on the same machine.

The modified DhcpAddressPrinter sketch was compiled using the changes in _r3 
and uploaded to a Arduino UNO with a Arduino ETHShield SD.
The Arduino was connected to the windows machine using a cross-over ethernet 
cable.

Test procedure:
* Open a the Serial Monitor in Arduino to log the messages from 
DhcpAddressPrinter
* Let the Arduino device get an IP.
* Ping from the windows machine to the assigned IP
* Let the Arduino do 2 renewals 
* Change the MAC address for the previously assigned IP in the dhcp database on 
the server.
* Let the Arduino device renew and get a new IP.
* ping the old IP from the windows machine.
* ping the new IP from the windows machine.

Test result:
--output from Serial Monitor
0 setup()
8858 My IP address: 192.168.85.10.
69106 A renew or rebind event happened while maintaining the dhcp lease
2 was the returned status
IP address after renew/rebind:192.168.85.10.
129105 A renew or rebind event happened while maintaining the dhcp lease
2 was the returned status
IP address after renew/rebind:192.168.85.10.
189600 A renew or rebind event happened while maintaining the dhcp lease
2 was the returned status
IP address after renew/rebind:192.168.85.11.
249106 A renew or rebind event happened while maintaining the dhcp lease
2 was the returned status
IP address after renew/rebind:192.168.85.11.

--result of pings
Pinging to the assigned IP worked in all cases.
After the reassignment to a new IP the old IP did not return any replies.

--analyze of the wireshark log
All packets sent looks reasonable (disclaimer:I'm not a DHCP guru)
All renewal request include the current IP as it should
The timing seems within limits. Renew should happen at about 1/2 the lease time.

Original comment by pe...@birchroad.net on 21 Feb 2012 at 9:08

Attachments:

• fix_issue_716_r3.patch
• DhcpAddressPrinter.ino

[GoogleCodeExporter]

Thanks for the update.  Have you done any testing with longer lease times?  If 
I remember correctly, I'm getting a 3 day lease time from the DHCP server in my 
studio, and something longer than millis will take to wrap round (which is 
something like 12 hours, I think?)

Original comment by adrian.m...@gmail.com on 21 Feb 2012 at 10:57

[GoogleCodeExporter]

In many cases in the main program it is important to know if the ip address is 
obtained or not. For doing this we can change the code in this way:

***Ethernet.cpp
//Return: 0 nothing changed, 1 ip available, 2 no ip

int EthernetClass::maintain(){
  if(_dhcp != NULL){
    //we have a pointer to dhcp, use it
    switch (_dhcp->checkLease() ){
      case DHCP_CHECK_NONE:
          return(0);
        //nothing done
        break;
      case DHCP_CHECK_RENEW_OK: //codice eseguito fino al break!
      case DHCP_CHECK_REBIND_OK:
        //we might have got a new IP.
        W5100.setIPAddress(_dhcp->getLocalIp().raw_address());
        W5100.setGatewayIp(_dhcp->getGatewayIp().raw_address());
        W5100.setSubnetMask(_dhcp->getSubnetMask().raw_address());
        _dnsServerAddress = _dhcp->getDnsServerIp();
            return(1);
        break;
      default:
        //this is actually a error, it will retry though
          return(3);
        break;
    }
  }
}

***and in our .ino file we can check in this way:

boolean chekDhcp()
{
    //0 nothing changed, 1 ip available, 2 no ip
    switch( Ethernet.maintain() ){
    case 0:
      return(ipObtained);
    break;
    case 1:
      return(true);
    break;
    case 2:
      return(false);
    break;  
    }
}

where ipObtained is a global boolean so we have kept the last state.

Original comment by paolo.ca...@gmail.com on 21 Feb 2012 at 11:23

[GoogleCodeExporter]

"Yet an update. This time I added a return status on Ethernet.maintain() as 
discussed on the now closed pull request. (it's not included yet, I just closed 
the request)" unfortunatly i didn't saw it... we worked on the same subject...
Thanks for your work.

Original comment by paolo.ca...@gmail.com on 21 Feb 2012 at 11:27

[GoogleCodeExporter]

1) Sorry @paolo :-) that is one of the reasons I closed the pull request. To 
move all discussions here.

2) @adrian, I haven't done any testing, but... millis is only used to compare 
one second to the next. If >1000ms then 1 is subtracted from the renew counter 
which is triggered when it reaches 0. And I'm doing the signed/unsigned trick 
on millis to take care of the wrap. See Dhcp.cpp - DhcpClass::checkLease, 
around line 394
So things should work as long as the 7/8 of the lease in seconds fits in a 
signed long. (Dhcp.cpp, Line 94-107)

Original comment by pe...@birchroad.net on 21 Feb 2012 at 1:00

[GoogleCodeExporter]

Ok. one thing that struck me when I read through comment 12 again.
As it is implemented you would like to call .maintain() at least once every 
second as it only decreases the counter if more that 1 sec has passed, it 
doesn't check how many seconds has passed. If more than a second between a beat 
might be missed.

Does that have to be altered and if so, any suggestions?
If you are that low on spare clock cycles that you can't check once a second, 
how likely is it that you will be using the DHCP library anyway since it's 
locking?

Original comment by pe...@birchroad.net on 21 Feb 2012 at 1:20

[GoogleCodeExporter]

Adrian, does the patch in comment 8 look good to you?

Original comment by dmel...@gmail.com on 22 Feb 2012 at 12:36

• Changed state: Accepted
• Added labels: Component-Ethernet, Milestone-1.0.1

[GoogleCodeExporter]

I have same problems using the patch file. Can you upload the .h and the .cpp 
files please? Thanks in advance

Original comment by paolo.ca...@gmail.com on 22 Feb 2012 at 10:51

[GoogleCodeExporter]

Ah, I definitely wasn't calling .maintain() at least once a second.  That will 
explain (a) why it didn't seem to work, and (b) why the code that was dealing 
with millis() looked a bit strange (i.e. looked like I needed to read through 
it properly :-)

I don't think that having to check every second is going to work, as surely 
there'll be many occasions where your code might block for more than a second?  
Setting up a TCP connection for example, could quite easily block for a number 
of seconds, as would a delay(1000) :-)

I think the ideal would be where you can just check once per run through 
loop().  As long as each pass through loop() happens less than half the time 
taken for millis() to wrap round, I /think/ it should be possible to do it 
without having any problems (and surely we can choose the "maximum amount of 
time has passed" for any times when it's unclear, so we err on the side of 
renewing a bit more regularly than necessary)

Original comment by adrian.m...@gmail.com on 22 Feb 2012 at 5:12

[GoogleCodeExporter]

@adrian, a very valid case indeed, point taken.

Here is another try then. This is a complete diff against the source again as 
well as separate files for Dhcp.* for those who want it.
The logic changed is in about line 197 in the _r3.patch.

With a delay(18400) in loop i got the following result when I redid the test 
above with a somewhat more modified sketch (also attached).
I haven't had time to make a >50 days (overflow time according to the 
reference) test with it.
-- 

0 setup()
9858 My IP address: 
192.168.85.10.
255.255.255.0.
192.168.85.1.
8.8.8.8.
65318 a maintain event occured after (sec):55
Status:2
IP address after renew/rebind:
192.168.85.10.
255.255.255.0.
192.168.85.1.
8.8.8.8.
120850 a maintain event occured after (sec):55
Status:2
IP address after renew/rebind:
192.168.85.10.
255.255.255.0.
192.168.85.1.
8.8.8.8.
176381 a maintain event occured after (sec):55
Status:2
IP address after renew/rebind:
192.168.85.10.
255.255.255.0.
192.168.85.1.
8.8.8.8.
232408 a maintain event occured after (sec):56
Status:2
IP address after renew/rebind:
192.168.85.11.
255.255.255.0.
192.168.85.1.
8.8.8.8.
287940 a maintain event occured after (sec):55
Status:2
IP address after renew/rebind:
192.168.85.11.
255.255.255.0.
192.168.85.1.
8.8.8.8.

Original comment by pe...@birchroad.net on 22 Feb 2012 at 8:42

Attachments:

• fix_issue_716_r3.patch
• Dhcp.h
• Dhcp.cpp
• DhcpAddressPrinter.ino

[GoogleCodeExporter]

Thanks for posting the .h and .cpp!

Original comment by paolo.ca...@gmail.com on 23 Feb 2012 at 1:36

[GoogleCodeExporter]

I applied the latest patch.  Let me know if this works for you.  I'm not a huge 
fan of the API (i.e. returning the status from the maintain() function seems a 
bit awkward) but it seems okay for now.  It might be something that changes 
later, though.  Adrian, any thoughts?

https://github.com/arduino/Arduino/commit/9f3438c1896c5e4bcd2dad14d9fa53527af2f8
b9

Original comment by dmel...@gmail.com on 2 Mar 2012 at 11:53

• Changed state: Fixed

[GoogleCodeExporter]

I applied the patch and added the call to maintain(), but if I power cycle my 
router, all my other devices show up immediately on my router page except for 
Wiznet. Is this expected even with this fix? I can still access the webserver 
running in the arduino.

Original comment by j3rr...@gmail.com on 10 Mar 2012 at 7:28

[GoogleCodeExporter]

As I don't have the wiznet ethernet shield I can't test it.. but can I
speculate that if you connect the wiznet into router A serving subnet
192.168.1.0/24 and unplug the network cable (without power cycling the
arduino) and connect it to another router B serving 10.0.0.0/24 that the
ethernet lib's IP address will not be updated into the new range ?

Arduino's ethernet lib should detect the physical link going down and up;
it should try to obtain a new lease from the DHCP server at such physical
events.

Original comment by jbrazio on 11 Mar 2012 at 12:57

[GoogleCodeExporter]

The latest patch (as applied in github) seems to be working well for me.  
Renewed the lease on the first testing I did, it'll be another couple of days 
before the lease has expired on the second DHCP server I'm testing it with, but 
seems good.

I expect jbrazio is right that it won't currently notice if the physical link 
goes down, but we can move that to being a new issue (and a future extension of 
the Ethernet.maintain() method maybe?), so I'll create a new issue for that.

Original comment by adrian.m...@gmail.com on 11 Mar 2012 at 2:13

[GoogleCodeExporter]

I've created a new issue (issue 855) for the physical layer problem.

Original comment by adrian.m...@gmail.com on 11 Mar 2012 at 2:19

[GoogleCodeExporter]

First someone with access to the official wiznet ethernet shield should
test my speculation and confirm it.. only after that the bug report should
have been open. :-)

Original comment by jbrazio on 11 Mar 2012 at 2:58

[GoogleCodeExporter]

Well, given that the DHCP code all happens in the Arduino, rather than on the 
WizNet, I know that it definitely won't automatically renew the lease :-)

It might not be possible to notice, that would need someone to check the WizNet 
datasheet, but if it turns out not to be possible then the issue can be closed 
as "can't fix" and will be there if anyone else is searching for the problem in 
future :-)

Original comment by adrian.m...@gmail.com on 11 Mar 2012 at 3:27

[GoogleCodeExporter]

i hate to be the retard but how do you install the patch?

Original comment by dethsch...@gmail.com on 30 Mar 2012 at 4:10