Opened 9 years ago
Closed 8 years ago
#12399 closed bug (fixed)
Static network address configuration fails
Reported by: | kallisti5 | Owned by: | axeld |
---|---|---|---|
Priority: | high | Milestone: | R1/beta1 |
Component: | Kits/Network Kit | Version: | R1/Development |
Keywords: | Cc: | PulkoMandy, markh | |
Blocked By: | Blocking: | ||
Platform: | All |
Description
Static network address configuration seems broken as of hrev49658 (x86_64)
Reproduction steps:
- No DHCP available
- Launch network preferences
- Select static
- Enter a valid network config
- Choose apply
- Dialog locks up. It will eventually resume, without applying the changes.
Attachments (3)
Change History (16)
by , 9 years ago
Attachment: | debugger.png added |
---|
comment:1 by , 9 years ago
Interestingly, configuring the network with ifconfig works.. it then shows up in the network preflet.
You trigger the same Network preflet lockup by typing anything into the DNS resolver box as well. (same stack trace)
by , 9 years ago
Attachment: | interfaces.png added |
---|
One last interesting observation. I haven't touched this file... that config doesn't look sane.
comment:2 by , 8 years ago
I spent some time tracing through this in the debugger today after running into this bug. The "w>Network" thread is the interesting one. It's blocking here in send_dg(). BNetworkAddressResolver::SetTo() leads to a getaddrinfo() call, and this is done for the address, netmask and gateway.
After it has exhausted all of its retries it moves on. I need to read some more of this code to understand what the right behavior should be. Either this situation should be detected a lot earlier or the timeouts in that loop need to be tighter.
comment:3 by , 8 years ago
Looks like this behavior was introduced in a change to BNetworkAddress for #12388. Looks like there was some disagreement about whether or not this was a desirable change, and I'm inclined to agree.
For the Network app in particular, fixing this either requires short circuiting the hostname lookup in BNetworkAddress, or, the option that i prefer, eliminate it entirely.
I'm new to Haiku as of like two days ago so I'm still reading through the source code, but I don't think it would be unreasonable to add an additional abstraction that can be used by whatever currently requires BNetworkAddress::fHostName. I have a lot of code to read before I really understand what's going on.
comment:4 by , 8 years ago
Cc: | added |
---|---|
Component: | Network & Internet/Stack → Kits/Network Kit |
(cc'ing PulkoMandy as he's been in charge of the network kit lately, and markh who wrote the apparently offending patch.)
I suppose I'm the one to blame here, for having merged that patch. I'm still a bit confused about what exactly is going on here -- so, BNetworkAddress tries to get the hostname of the static IP, which fails because there is no internet connection, and so static setup fails?
Also, welcome to Haiku! What brings you to these parts? :)
comment:5 by , 8 years ago
getaddrinfo on a static IP should work without needing to query anything on the network. For this to work, the AI_NUMERICHOST flag should be set and the IP address should be in numeric form (no DNS query needed in that case).
In the case of BNetworkAddressResolver, this maps to the B_NO_ADDRESS_RESOLUTION flag.
This flags seems to be missing in InterfaceAddressView.cpp:347: address.SetTo(string.String());
. It is set properly in {{{IPAddressControl.cpp:81: bool success = address.SetTo(fFamily, Text(), (char*)NULL,
}}}. I don't see other calls to BNetworkAddress::SetTo from a quick glance at the network preferences.
Just adding the flag in InterfaceAddressView would fix the problem, then.
comment:6 by , 8 years ago
I looked over my patch for #12388, but I don't see anything there that would cause this issue.
Without looking at the code, I would say pulkomandy is right. There should not be any address resolution in this case.
comment:7 by , 8 years ago
Also, welcome to Haiku! What brings you to these parts? :)
Thanks! I was a user of BeOS R5, and lately I've been looking for fun projects to work on during the occasional lazy Saturday.
You're right pulkomandy and markh, #12388 has nothing to do with this. I should have actually tested my theory that it was related before making that claim. Sorry for the goose chase. I just saw that the timing was close, and the patch for that ticket had a hunk that added an additional call to BNetworkAddressResolver::GetNextAddress(), which made me jump to conclusions.
I verified that
- Trunk has this issue
- 12388 has nothing to do with it. Reverting doesn't solve the problem.
- Adding the B_NO_ADDRESS_RESOLUTION flag as sugested by pulkomandy does indeed solve the problem.
I'm attaching a patch that adds that flag.
comment:8 by , 8 years ago
patch: | 0 → 1 |
---|
comment:9 by , 8 years ago
Looks OK to me, except for the static_cast()s -- are those really needed?
comment:10 by , 8 years ago
Unfortunately yes with GCC 5.
src/preferences/network/InterfaceAddressView.cpp:347:59: error: call of overloaded 'SetTo(const char*, int, <anonymous enum>)' is ambiguous address.SetTo(string.String(), 0, B_NO_ADDRESS_RESOLUTION); ^ In file included from /home/kambroff/code/haiku/haiku/headers/os/net/NetworkInterface.h:13:0, from src/preferences/network/InterfaceAddressView.h:14, from src/preferences/network/InterfaceAddressView.cpp:11: /home/kambroff/code/haiku/haiku/headers/os/net/NetworkAddress.h:47:15: note: candidate: status_t BNetworkAddress::SetTo(const char*, uint16, uint32) status_t SetTo(const char* address, uint16 port = 0, ^ /home/kambroff/code/haiku/haiku/headers/os/net/NetworkAddress.h:49:15: note: candidate: status_t BNetworkAddress::SetTo(const char*, const char*, uint32) status_t SetTo(const char* address, const char* service, ^
comment:11 by , 8 years ago
I think the second cast is not needed, only the port/service parameter is ambiguous?
by , 8 years ago
Attachment: | 0001-Don-t-query-DNS-when-statically-configuring-an-inter.patch added |
---|
Don't query DNS when statically configuring an interface
Attaching debugger to Network preflet always shows the same stack.