search

genodelabs / genode

Genode OS Framework
https://genode.org/
Other
1.08k stars 254 forks source link

nic_router: accept requested DHCP REQUEST replies always #3716

Closed alex-ab closed 4 years ago

alex-ab commented 4 years ago

The nic_router currently denies to accept DHCP REQUEST replies, which it had before accepted from a DHCP offer, if some option fields are missing. This leads to the situation, that the assigned IP address by the DHCP server is ignored by the nic_router.

m-stein commented 4 years ago

Thanks for investigating this! Commit https://github.com/alex-ab/genode/commit/bfe0a4e7465e74ddf55a47caa94bc0ee2615a6fa looks good to me.

m-stein commented 4 years ago

Of course, we could think about getting rid of the exception handling here at all. But as this pattern is spread over the whole NIC router, it may be better to fix it in the context of a dedicated issue.

chelmuth commented 4 years ago

Of course, we could think about getting rid of the exception handling here at all. But as this pattern is spread over the whole NIC router, it may be better to fix it in the context of a dedicated issue.

Thanks @m-stein for this comment as I have the feeling this trial-error scheme does not fit the option parsing well. I'd appreciate you opening an issue for this.

That said, I'm not sure what this patch tries to fix. A mis-configured DHCP server? A symptom of #3717? Or, a valid real-world scenario?

alex-ab commented 4 years ago

It is a bug, since the nic_router tells the dhcp server to accept the offer and later on does not do so.

chelmuth commented 4 years ago

I see, the IP config is done on ACK. I mixed this up with a DHCP re-request. Thanks, I'll merge this to staging.

m-stein commented 4 years ago

@alex-ab This is correct. At the other hand a DHCP server that doesn't provide a subnet mask and a router IP seems rare. However, according to https://tools.ietf.org/html/rfc2131 and https://tools.ietf.org/html/rfc2132, it seems, that the options "Router Address" and "Subnet Mask" are not considered necessary for common DHCP.

chelmuth commented 4 years ago

May one you please rebase the commit on current staging/master?

alex-ab commented 4 years ago

Done by c43f28a654f01a099327786b319c69a306b91668.