Re: Getting NM to re-try DHCP

[Dan Williams <dcbw redhat com>]

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