Opened 4 years ago

Closed 2 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:
Has a Patch: yes 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)

debugger.png (85.0 KB) - added by kallisti5 4 years ago.
Attaching debugger to Network preflet always shows the same stack.
interfaces.png (29.0 KB) - added by kallisti5 4 years ago.
One last interesting observation. I haven't touched this file... that config doesn't look sane.
0001-Don-t-query-DNS-when-statically-configuring-an-inter.patch (1.2 KB) - added by ambroff 2 years ago.
Don't query DNS when statically configuring an interface

Download all attachments as: .zip

Change History (16)

Changed 4 years ago by kallisti5

Attachment: debugger.png added

Attaching debugger to Network preflet always shows the same stack.

comment:1 Changed 4 years ago by kallisti5

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)

Changed 4 years ago by kallisti5

Attachment: interfaces.png added

One last interesting observation. I haven't touched this file... that config doesn't look sane.

comment:2 Changed 2 years ago by ambroff

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 Changed 2 years ago by ambroff

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 Changed 2 years ago by waddlesplash

Cc: PulkoMandy markh added
Component: Network & Internet/StackKits/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 Changed 2 years ago by pulkomandy

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.

Last edited 2 years ago by pulkomandy (previous) (diff)

comment:6 Changed 2 years ago by markh

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 Changed 2 years ago by ambroff

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

  1. Trunk has this issue
  2. 12388 has nothing to do with it. Reverting doesn't solve the problem.
  3. 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 Changed 2 years ago by ambroff

Has a Patch: set

comment:9 Changed 2 years ago by waddlesplash

Looks OK to me, except for the static_cast()s -- are those really needed?

comment:10 Changed 2 years ago by ambroff

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 Changed 2 years ago by pulkomandy

I think the second cast is not needed, only the port/service parameter is ambiguous?

Changed 2 years ago by ambroff

Don't query DNS when statically configuring an interface

comment:12 Changed 2 years ago by ambroff

Oh shoot. You're right. Fixed.

comment:13 Changed 2 years ago by pulkomandy

Resolution: fixed
Status: newclosed

Applied in hrev50988. Thanks!

Note: See TracTickets for help on using tickets.