Skip to content

DHCP renew / rebind #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
xorbit opened this issue Apr 16, 2021 · 5 comments
Closed

DHCP renew / rebind #35

xorbit opened this issue Apr 16, 2021 · 5 comments

Comments

@xorbit
Copy link
Contributor

xorbit commented Apr 16, 2021

Currently the DHCP client gets called once, and that's it. The device gets an IP, the lease expires, but the device happily keeps on using the IP, even if the DHCP server may assign the IP to another device in the mean time. Works for demos but not a good thing to have devices that do this on your network.

I was thinking of rewriting the DHCP client to maintain the lease properly, but this would likely require an eth.maintain_dhcp_lease() or similar call in the main loop to handle what needs to be done. So I would like to know:

  • Is someone already working on fixing this, and I would be duplicating effort?
  • Is this a desired improvement? (I would think so.)
  • Any implementation guidance to make sure a PR would be accepted?
  • I didn't see this in other drivers, it looks like the ESP32SPI must be maintaining the DHCP lease internally so it doesn't seem to need it. Is there some established form I should follow?
  • Any suggestion for the name of the maintenance function?
@brentru
Copy link
Member

brentru commented Apr 16, 2021

Is someone already working on fixing this, and I would be duplicating effort?

Not that I know of.

Is this a desired improvement? (I would think so.)

Yes, much appreciated and desired 👍

Any implementation guidance to make sure a PR would be accepted?

Not sure, I'll be glad to test/comment/accept as requested. The DHCP client is ported from the Arduino Ethernet Library (https://github.com/arduino-libraries/Ethernet/blob/master/src/Dhcp.cpp).

, it looks like the ESP32SPI must be maintaining the DHCP lease internally

I believe that is happening within the WiFi client implemented in nina-fw (https://github.com/adafruit/nina-fw/tree/master/arduino/libraries/WiFi/src).

Any suggestion for the name of the maintenance function?

Arduino uses ethernet.maintain() (https://www.arduino.cc/en/Reference/EthernetMaintain), I think your suggestion of maintain_dhcp_lease() would be OK within the main loop.

@xorbit
Copy link
Contributor Author

xorbit commented Apr 28, 2021

I have been working on this and have code that mostly works, but has a really weird issue. I have it running together with the WSGI web server, and I keep getting an error like this:

* DHCP: Time to renew lease
* DHCP: Send request to (192, 168, 1, 1)
Traceback (most recent call last):
  File "code.py", line 104, in <module>
  File "/lib/adafruit_wiznet5k/adafruit_wiznet5k_wsgiserver.py", line 90, in update_poll
  File "/lib/adafruit_wiznet5k/adafruit_wiznet5k_wsgiserver.py", line 147, in _get_environ
  File "/lib/adafruit_wiznet5k/adafruit_wiznet5k_socket.py", line 382, in readline
RuntimeError: Didn't receive response, failing out...

Code done running.

What should happen is that the DHCP component opens a socket, sends out a request, and then receives a response on that socket. What instead seems to be happening is that another socket, one of the ones listening on port 80 for the web server, receives the response or at least is told it has data available, it starts to interpret this but doesn't get a "\r\n" and times out, taking down the server.

Of course it's not great that the whole web server can be brought down by a malformed request, this shouldn't really happen either, but that's a problem for another day. What I don't get here is why the wrong socket, one that is listening on a different port, would be told it has data available. Has anyone else observed this? Any ideas?

@xorbit
Copy link
Contributor Author

xorbit commented Apr 29, 2021

I may have fixed the above, which happened when using a valid socket.
Now I saw one triggered by this:

* socket_available called on socket 3, protocol 33
* socket_available called on socket 4, protocol 33
* socket_available called on socket 5, protocol 33
* socket_available called on socket 2, protocol 33
* socket_available called on socket 1, protocol 33
* socket_available called on socket 0, protocol 33
* DHCP: Time to renew lease
* socket_available called on socket 3, protocol 33
* socket_available called on socket 4, protocol 33
* socket_available called on socket 5, protocol 33
* socket_available called on socket 2, protocol 33
* socket_available called on socket 1, protocol 33
* socket_available called on socket 0, protocol 33
*** Get socket
Allocated socket #2
* w5k socket connect, protocol=2, port=67, ip=192.168.1.1
*** Opening socket 2
* Opening W5k Socket, protocol=2
* DHCP: Send request to (192, 168, 1, 1)
* socket_available called on socket 3, protocol 33
* socket_available called on socket 4, protocol 33
* socket_available called on socket 5, protocol 33
* socket_available called on socket 2, protocol 33
* socket_available called on socket 1, protocol 33
* socket_available called on socket 0, protocol 33
* socket_available called on socket 2, protocol 2
* socket_available called on socket 3, protocol 33

It seems there's an issue in the socket allocation code: it allocated socket 2 for DHCP, but this socket was already allocated to the web server. 😬
Which caused the web server to crash with RuntimeError: Didn't receive response, failing out... like above.

@xorbit
Copy link
Contributor Author

xorbit commented Apr 29, 2021

I seem to have the wrinkles ironed out. See pull request #36.

@brentru
Copy link
Member

brentru commented May 6, 2021

Closing via #36

@brentru brentru closed this as completed May 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants