Hi Robert,
On 04/19/2018 12:21 PM, Robert Tiemann wrote:
Hi,
we have discovered a problem with NTP servers not being used under
certain conditions. Here is how to reproduce our problem:
1. Device is using Ethernet, cable is plugged into a switch. No other
technologies are involved.
2. Make sure there is no DHCP server on the network.
3. Start ConnMan and let it try to connect via Ethernet using DHCP.
Autoconfiguration kicks in after some time, and the IP address is
set to 169.254.x.y.
4. Start DHCP server.
5. After some time, ConnMan obtains a lease and configures the wired
service according to data received from the DHCP server.
6. All settings, including timeserver, are correctly configured now,
but the timeserver is not started.
7. Replugging the Ethernet cable triggers new DHCP configuration, but
this time the timeserver is started.
I have tracked down the problem to function timeserver_start() in
src/timeserver.c. At autoconfiguration time (step 3), there are no
nameservers configured and timeserver_start() returns early, leaving
the resolv pointer NULL. Consequently, __connman_timeserver_sync()
fails each time it is called.
At the time ConnMan obtains a proper DHCP lease, function
default_changed() in timeserver.c is *not* called, but
__connman_timeserver_sync() is. The latter fails because the resolv
pointer is still NULL. At this time you have a working IP connection,
but the timeserver is not running at all.
Now, I think the timeserver should work even without any nameservers
configured because the timeserver(s) could be provided as raw IP
address(es). This patch changes timeserver_start() such that the
nameservers are optional and resolv is always allocated.
Thanks for tracking this done. Very helpful analysis. I've added the
above paragraphs to the commit message. It contains quite a lot of
valuable information, would be sad to lose it.
I suspect, however, that the timeserver_notifier should be notified
as
well after having obtained the DHCP lease, right? I don't know,
however, where and how this should be done, or even if it would be > correct in the
first place.
Indeed, we need should kick timeserver to checkup on changes. I suspect,
this could be added at the dhcp_callback/success() code path in network.c.
Patch applied.
Thanks,
Daniel