WIP: net: DHCP use the DNS servers option we request #1369
Conversation
subsys/net/lib/dns/resolve.c
Outdated
There was a problem hiding this comment.
very mad...
was this for debugging?
There was a problem hiding this comment.
@nashif you move faster than I can Type WIP :)
johnandersen777
changed the title
|
Note to self I might need to rebase onto #1341. |
samples/net/dhcpv4_client/prj.conf
Outdated
There was a problem hiding this comment.
Why do we need to enable IPv6 in DHCPv4 client sample?
samples/net/dhcpv4_client/prj.conf
Outdated
There was a problem hiding this comment.
Please do not enable the debugs in default prj.conf file. If you need debugging prints, create a conf file just for this purpose and use that in your testing.
subsys/net/ip/dhcpv4.c
Outdated
There was a problem hiding this comment.
Use NET_IPV4_ADDR_LEN here, then you can also remove max_ipv4_addr_len variable.
subsys/net/ip/dhcpv4.c
Outdated
There was a problem hiding this comment.
use sizeof(dns_string) as a length, this way we can get rid of extra variable
subsys/net/ip/dhcpv4.c
Outdated
There was a problem hiding this comment.
here length should be min(strlen(net_sprint_ipv4_addr(&dns), sizeof(dns_string))
subsys/net/ip/dhcpv4.c
Outdated
|
As @jukkar found I had lot's of random debugging in there to help me feel out the codebase. I'm still working on this I want it to be perfect before I remove the WIP. I just like to leave any WIP stuff in a PR so others can see it's being worked on so they don't duplicate effort 👍 |
+1 for that (in reasonable bounds), but naming PRs is still important (a common problem in this project actually). For example, "net: dhcp: dns: Bound is now a vaild state" doesn't tell me much. |
johnandersen777
changed the title
johnandersen777
force-pushed
the
net_dhcp_state_bound
branch
from
2c28cb3 to
ea9271b
Compare
Signed-off-by: John Andersen <[email protected]>
johnandersen777
force-pushed
the
net_dhcp_state_bound
branch
from
ea9271b to
f5f8274
Compare
|
Hi @pdxjohnny any progress on this one, are you able to re-submit the PR? |
…oject-rtos#1424) The callstack of 2k is causing segfault in BLE stack, for example, when it starts physical web advertising, or the Websocket over 6lowpan, increasing it to 4k fixes the segfault. Fixes zephyrproject-rtos#1369 zephyrproject-rtos#1421 Signed-off-by: Jimmy Huang <[email protected]>
|
@pdxjohnny : Ping, hit this again in real life. Let me know if you'd like me to pick this up and polish for submission. |
|
|
||
| NET_DBG("options_dns: %s", dns_servers[0]); | ||
|
|
||
| // TODO Change this to its own context |
There was a problem hiding this comment.
Re: this TODO. I guess, for initial version of this feature it's not required.
| } | ||
|
|
||
| memset(dns_string, 0, sizeof(dns_string)); | ||
| memcpy(dns_string, net_sprint_ipv4_addr(&dns), |
There was a problem hiding this comment.
But this is what I don't like. Convert binary address to string, just for it to be converted back? Nope, we should allow to init resolver with struct in_addr.
There was a problem hiding this comment.
This definitely won't do, because net_sprint_ipv4_addr() is debugging func, available only when logging is enabled.
@pdxjohnny : I'm in the process of refactoring this patch already.
|
@pdxjohnny : So, I took your patch as a base, added missing parts, refactored and submitted as #5711. I kept your git authorship (but the patch is of course updated) and added my sign-off. Let me know if you have any concerns. Otherwise, I'm closing this patch as superseded. Thanks. |
4 participants
DHCP requests are asking for DNS servers but then doing nothing with them. This patch updates DNS to accept BOUND as a valid state. It also modifies the DHCP client to create a new DNS context when the DHCP client gets the DNS server option back in a packet.