Hi Samuel, Thanks for the reply.
On Thu, Jul 08, 2010 at 07:35:38PM +0200, Samuel Ortiz wrote: > Hi Forest, > > On Wed, Jul 07, 2010 at 10:51:58PM -0400, Forest Bond wrote: > > Hi Samuel, > > > > On Thu, Jul 08, 2010 at 01:44:59AM +0200, Samuel Ortiz wrote: > > > On Tue, Jul 06, 2010 at 11:13:42PM -0400, Forest Bond wrote: > > > > You'll notice that this patch does not take into consideration your > > > > recommendation that nameserver host routes be added in > > > > __connman_service_append_nameserver. The reason for this is that the > > > > gateway > > > > often not known when that function is called. > > > In which case is the gateway not known at that point ? > > > In the DHCP case, ipv4_probe is called by dhcp_bound, i.e. after we > > > actually > > > received a complete DHCP offer. If you don't have a gateway at that point > > > you > > > won't get it I suppose. > > > In the manual case, if you don't have a gateway, that means the user > > > haven't > > > set it and there's not much you can do about it. > > > > > > Could you please describe how you're not getting a gateway there, I > > > may be missing a point ? > > > > I can explain as well as I understand it. My conclusion is based on what I > > am > > seeing in testing, which is not necessarily what is supposed to be > > happening by > > design. > > > > Here are the relevant call sequences as I understand them: > > > > The gateway is added to the routing table like this: > > > > connman_dhcp_bound > > `-> ipv4_probe > > `-> __connman_service_append_nameserver <- You are here. > > `-> connection_probe > > `-> __connman_service_indicate_state (READY) > > `-> update_nameservers > > `-> connman_resolver_append <- Nameservers go > > into > > effect here. > > `-> set_default_gateway > > `-> connman_inet_set_gateway_address <- Gateway is added > > to > > the routing table > > here. > > > > The gateway is stored in service->ipconfig like this: > > > > rtnl_newroute > > `-> process_newroute > > `-> __connman_ipconfig_newroute <- Gateway is > > stored in > > > > service->ipconfig. > > > > So, to answer your question, yes the gateway is known when > > __connman_service_append_nameserver is called. But it is not yet stored in > > service->ipconfig, so in most cases this expression returns NULL: > > > > > > __connman_ipconfig_get_gateway(connman_network_get_index(service->network)) > True, I was thinking that you would go through the element stuff to retrieve > the gateway. But your remark below about us not catching the DNS manual > setting path makes the above solution invalid. > > > As I see it, either the connman's late storage of the gateway in > > service->ipconfig is wrong, or my assumption that I should be getting the > > gateway using __connman_ipconfig_get_gateway is wrong. > I think the ipdevice->gateway setting is correct, I'd rather not change this > part. > > > If connman is handling the gateway incorrectly, it could be changed in a few > > different ways: > > > > * Perhaps the service should not have state READY until after the gateway > > has > > been confirmed set via rtnl. > > * Maybe the gateway should be stored in the ipconfig structure in a > > different > > place (in connection_probe, for instance). > > > > Or, we could leave connman's gateway handling logic alone and I could just > > get > > the gateway by searching the element tree for the connection element and > > getting > > it using connman_element_get_value like connection_probe does now. > > > > However, there is another problem with adding the route in > > __connman_service_append_nameserver. User-specified nameservers > > (service->nameservers) are not set that way, they are set in set_property > > (service.c) directly. Shouldn't we also install host routes for these > > nameservers? > We certainly should, yes. > > So, I am attaching 2 patches to this email: > > 0001-Factorize-host-route-setting-routine.patch that cleans the inet.c host > route setting stuff and that should go upstream anyway. > > And > 0002-Set-DNS-host-routes-before-toggling-the-service-READ.patch that basically > sets the DNS host routes before we set the service to READY, from > connection_probe(). Once the routes are properly set, bumping the service to > READY will trigger an update_nameservers() call, which should allow our DNS > proxy to succesfully connect to the nameservers. These look right to me. Doing this in connection.c makes things a lot cleaner. > Would you mind giving those 2 patches a try, please ? They should apply > cleanly on top of the latest ConnMan git. Yes, I will try them in the morning as I don't currently have access to the test machine. Thanks, Forest -- Forest Bond http://www.alittletooquiet.net http://www.pytagsfs.org
signature.asc
Description: Digital signature
_______________________________________________ connman mailing list [email protected] http://lists.connman.net/listinfo/connman
