Hi,
On Mon, 2015-10-05 at 14:46 -0700, Abtin Keshavarzian wrote:
> We tracked the source of this as follows: (all methods/functions are in
> ipconfig.c):
>
> - When the interface is brought down, we get a DELLINK message in Connman
> - This triggers a call to '__connman_ipconfig_dellink()' in ipconfig.c
> - In this function at the very end we remove the ipdevice from
> 'ipdevice_hash' using
> 'g_hash_table_remove(ipdevice_hash, GINT_TO_POINTER(index))'
> - This indirectly causes a call to `free_ipdevice()' to free the associated
> ipdevice
> element in
> the hash table
> - In 'free_ipdevice()' we get the interface name using
> 'connman_inet_ifname(index)' from the
> index which is stored in 'ipdevice->index'
> - 'connman_inet_ifname(index)' uses ioctl to get the ifname, but since the
> interface is (being)
> removed, there is no name and we return NULL as ifname
> - Back in 'free_ipdevice()' we call 'set_ipv6_state(ifname,
> ipdevice->ipv6_enabled)'
> - In 'set_ipv6_state()' if the passed in ifname is null, then the change is
> applied to all interfaces
> - So we disable ipv6 on all interfaces (instead of the one being removed).
Nice work, thanks for the detailed report!
> We have the Connman logs showing this chain of events which I'd be happy to
> share.
> The quick fix we are using now is in 'free_ipdevice()' as follows:
>
> +++ connman/src/ipconfig.c
> @@ -422,8 +402,10 @@ static void free_ipdevice(gpointer data)
>
> g_free(ipdevice->address);
>
> - set_ipv6_state(ifname, ipdevice->ipv6_enabled);
> - set_ipv6_privacy(ifname, ipdevice->ipv6_privacy);
> + if (ifname) {
> + set_ipv6_state(ifname, ipdevice->ipv6_enabled);
> + set_ipv6_privacy(ifname, ipdevice->ipv6_privacy);
> + }
>
> g_free(ifname);
> g_free(ipdevice);
>
> Basically checking the 'ifname' and only invoking 'set_ipv6_state()' with a
> non-null 'ifname'
Yes, that looks like the quick fix right now. Can you send a patch for
this?
The long term solution is to use the ipdevice data structure instead of
the ioctl in connman_inet_if{name,index} as ipdevice properly follows
the change in interface names anyway. I'll look into that if I have
time...
> Also about the variable 'bool ipv6_enabled' in 'struct connman_ipdevice':
>
> - This value is set initially in '__connman_ipconfig_newlink()' when a new
> ipdevice is created.
> - And it is used only when the ipdevice is freed in 'free_ipdevice()'
> - This variable is not changed when 'enable_ipv6()' or 'disable_ipv6()' is
> called.
>
> Is this the intended use for this variable? i.e., remembering the initial
> state and setting it when the
> ipdevice is deleted.
The commit where this was introduced is
d4e1dfc943644199849eb3de2463eb0ca0344408 and the change says "Restore
original IPv6 interface status when connman quits". So yes, that's the
inteded behavior.
Thanks,
Patrik
_______________________________________________
connman mailing list
[email protected]
https://lists.connman.net/mailman/listinfo/connman
