Re: [PATCH] device: restart the DHCP transaction when the carrier goes up during the defer period

[Thomas Haller <thaller redhat com>]

On Mon, 2015-07-20 at 14:24 +0200, Lubomir Rintel wrote:
> This makes NetworkManager recover on brief carrier toggle following a 
> DHCP outage.
>  src/devices/nm-device.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c
> index 913c488..53ddb66 100644
> --- a/src/devices/nm-device.c
> +++ b/src/devices/nm-device.c
> @@ -1295,6 +1295,10 @@ link_disconnect_action_cancel (NMDevice *self)
>               g_source_remove (priv->carrier_defer_id);
>               _LOGD (LOGD_DEVICE, "link disconnected (canceling 
> deferred action) (id=%u)", priv->carrier_defer_id);
>               priv->carrier_defer_id = 0;
> +
> +             /* Don't let the DHCP requests that were lost while 
> the carrier was down
> +              * contribute to the DHCP timeout -- restart the 
> transaction. */
> +             update_dynamic_ip_setup (self);
>       }
>  }
>  

link_disconnect_action_cancel() doesn't seem the right place to do
this, because then the name of the function is misleading, and it gets
called from dispose() -- at which you don't want to renew the DHCP
lease (well, technically, you wouldn't because the dhcp client is
already cleared).
Can you not move the call to nm_device_set_carrier()?



Also, in commit 348452f1e06e9bde9f84b90db4f5620ee672389a, it seems
wrong that update_dynamic_ip_setup() clears ip6_saved_properties.
How about restoring update_for_ip_ifname_change() which does:

update_for_ip_ifname_change()
{
   g_hash_table_remove_all (priv->ip6_saved_properties);
   update_dynamic_ip_setup (self);
}




Thomas

Attachment: signature.asc
Description: This is a digitally signed message part