Re: Getting NM to re-try DHCP

[Jirka Klimes <jklimes redhat com>]

On Tuesday 21 of June 2011 07:00:00 Dan Williams wrote:
> On Fri, 2011-06-17 at 12:58 +0200, Jirka Klimes wrote:
> > On Thursday 26 of May 2011 20:22:26 Dan Williams wrote:
> > > > A patch doing that is in the attachment.
> > > 
> > > A few concerns with the patch; it might get more complicated because of
> > > them.  First, if the connection is deleted before the idle handler
> > > runs, we'll be left with garbage.  So we'd need to do
> > > g_object_weak_ref() on the connection object, cache the idle ID, and
> > > run g_source_remove() on that idle ID from the weak ref callback. 
> > > Next, in
> > > reset_connection_retries() we'd want to remove that weak ref when
> > > freeing the ResetRetriesData structure.
> > > 
> > > Second, if the connection gets activated again manually by the user
> > > while we're waiting the 5 minutes then we should g_source_remove() the
> > > idle ID.  That gets more complicated.
> > > 
> > > Perhaps a simpler approach would be to have a single, global idle
> > > handler in the NMPolicy that runs over the connection list and resets
> > > the retries as appropriate?  First, in device_state_changed() we could
> > > attach the current seconds-since-epoch time to the object using
> > > g_object_set_data() when its # retries reaches 0, and then if no reset
> > > idle handler was scheduled, schedule one for 300 seconds later.
> > > (otherwise allow the existing idle handler to run earlier since
> > > presumably it was scheduled by an earlier failed connection we want to
> > > reset).
> > > 
> > > Then, when the reset idle handler does run, iterate over each
> > > connection, save the earliest reset timestamp, and reschedule the idle
> > > handler for that timestamp + 300 seconds.  During this iteration of
> > > course we reset the retries count for every connection that has a reset
> > > timestamp earlier than now.
> > > 
> > > If the connection gets activated, nm-policy.c's signal handlers can
> > > listen for that and clear out the invalid timestamp data too.
> > > 
> > > If that would all work, that would allow us to avoid doing all the
> > > alloc/dealloc of a custom data structure, plus we only have to manage
> > > one idle handler.  Plus we don't have to care too  much about stuff
> > > like connection deletion and activation happening before our idle
> > > handler runs since those will be easier to deal with.
> > > 
> > > Thoughts?
> > > 
> > > Dan
> > 
> > Please find the reworked patch is the attachment.
> 
> Looks good, though for completeness we'd want to g_source_remove() the
> timeout's ID in nm_policy_destroy() too.  It won't really have an effect
> since nm_policy_destroy() is only called when NM is exiting though.
> 
> Thanks!
> Dan

Pushed to master: fdfbe00aac3f17b19bb8d84cba1c8f210d90e8a0

Jirka