Skip to content

Adding support for DHCP option 121#15

Open
tpjg wants to merge 5 commits into
nanovms:STABLE-2_1_xfrom
tpjg:STABLE-2_1_x
Open

Adding support for DHCP option 121#15
tpjg wants to merge 5 commits into
nanovms:STABLE-2_1_xfrom
tpjg:STABLE-2_1_x

Conversation

@tpjg

@tpjg tpjg commented Mar 20, 2026

Copy link
Copy Markdown

Timo Gatsonides added 3 commits January 4, 2026 12:10
Move route table and option parser from nanos into lwip, gated by
LWIP_DHCP_CLASSLESS_STATIC_ROUTES. Pass netif to dhcp_parse_reply()
to eliminate the nanos_dhcp_current_netif workaround.

@francescolavra francescolavra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @tpjg. There is a missing part in this implementation: the router addresses received from the DHCP server via option 121 are being parsed and added to the static routes, but are not actually being used when routing outgoing packets.

Comment thread src/core/ipv4/dhcp.c Outdated
routes

Also removes some references to nanos in comments.
@tpjg

tpjg commented Mar 31, 2026

Copy link
Copy Markdown
Author

Hi @tpjg. There is a missing part in this implementation: the router addresses received from the DHCP server via option 121 are being parsed and added to the static routes, but are not actually being used when routing outgoing packets.

Argh. Absolutely correct. Very sorry, I was focussed (too much) on the cloud instance metadata at 169.254.169.254 that is 'on net' caught by the ip4_addr_islinklocal that I completely forgot about this. See commit 35f60a0.

@tpjg tpjg requested a review from francescolavra April 3, 2026 21:48

@francescolavra francescolavra left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @tpjg
Most of my comments are about simplifying the code and minimizing the computational load required to walk the routing table. Since this code is potentially run for every outgoing network packet whose destination doesn't fit in the subnet of the interface, it should be as efficient as possible.

Comment thread src/core/ipv4/ip4_route_table.c Outdated
Comment thread src/core/ipv4/ip4_route_table.c Outdated
Comment thread src/core/ipv4/dhcp_classless_route.c Outdated
Comment thread src/core/ipv4/dhcp_classless_route.c Outdated
Comment thread src/core/ipv4/dhcp_classless_route.c Outdated
Comment thread src/core/ipv4/ip4_route_table.c Outdated
Comment thread src/core/ipv4/ip4_route_table.c Outdated
Comment thread src/core/ipv4/dhcp_classless_route.c Outdated
Comment thread src/core/ipv4/ip4_route_table.c Outdated
Comment thread src/include/lwip/opt.h Outdated
Remove dhcp_classless_route.c; move parser into dhcp.c as static
function.
Replace option121_received array with ip4_route_exists(). Use
LWIP_ERROR, lwip_htonl where appropriate.
@francescolavra

Copy link
Copy Markdown
Member

@tpjg I reviewed your latest changes and they look good to me. There are still a few unresolved issues, do you have time to address them? If not, I could do it myself in a new PR (of course preserving your authorship info).

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