Skip to content

core/dhcp: Lease time support#148

Open
rowanG077 wants to merge 3 commits intoenjoy-digital:masterfrom
rowanG077:dhcp-opts
Open

core/dhcp: Lease time support#148
rowanG077 wants to merge 3 commits intoenjoy-digital:masterfrom
rowanG077:dhcp-opts

Conversation

@rowanG077
Copy link
Copy Markdown
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

Comment thread liteeth/core/dhcp.py
last_byte = Signal()

self.comb += [
# @Florent: Is this necessary? Can we assume last_be is 0b1000 For non-last words?
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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