Skip to content
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

core/dhcp: Lease time support #148

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rowanG077
Copy link
Contributor

@rowanG077 rowanG077 commented Sep 5, 2023

This PR adds DHCP lease time support. To do that a DHCP OPTIONS parser is implemented.

At first I tried to do it fully in the 32-bit datapath. But it's simply too much. Header can be at any position, Multiple options per 32-bit packet are possible, padding which increases variety even more... If LiteX had some kind of function support It would be possible but I simply don't know how to express that.

So in the end I decided to just bite the bullet and do options parsing with 8-bit width.

Another problem that is fixed now is that MESSAGETYPE option no longer is assumed to be first option. The spec does not say it must be the first option so this could be very problematic for some Liteeth DHCP server combinations.

I did already add gateway and netmask parsing but they aren't wired up.

TODOs:

  • Make it function correctly
  • Use received lease time to automatically restart DHCP process before the lease has expired
  • Make the DHCP module self contained. No longer have external start, timeout and done signals

last_byte = Signal()

self.comb += [
# @Florent: Is this necessary? Can we assume last_be is 0b1000 For non-last words?
Copy link
Contributor Author

@rowanG077 rowanG077 Sep 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@enjoy-digital See comment

@rowanG077 rowanG077 changed the title core/dhcp: Add proper options parser core/dhcp: Add DHCP options parser Sep 5, 2023
@rowanG077 rowanG077 marked this pull request as ready for review September 13, 2023 12:07
@rowanG077
Copy link
Contributor Author

@enjoy-digital This works now and is ready for review.

Since the DHCP module can now be self-contained I removed the exposed signals in the core generator.

@rowanG077 rowanG077 changed the title core/dhcp: Add DHCP options parser core/dhcp: Improve DHCP core Sep 13, 2023
@rowanG077 rowanG077 changed the title core/dhcp: Improve DHCP core core/dhcp: Lease time support Sep 13, 2023
@rowanG077
Copy link
Contributor Author

@enjoy-digital Just pinging you so it doesn't fly under the radar. I would like it if this can get merged since I really prefer to use the upstream liteeth instead of a mass of custom PR :).

@enjoy-digital
Copy link
Owner

@rowanG077: Thanks for the PR and sorry for the delay. I'll try to plan some time to look at this very soon.

@rowanG077
Copy link
Contributor Author

@enjoy-digital I hate to bother you with this but this and other PRs have been waiting for a while. This means we currently diverge quite a bit from upstream liteeth and I don't really want that. If you want me to put this PR in a different form or walk you through the changes I can do so as well.

@enjoy-digital
Copy link
Owner

Hi @rowanG077,

sorry for the delay, lots of different priorities to handle but I'll try to do a closer review soon to avoid blocking things.

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

Successfully merging this pull request may close these issues.

2 participants