Re: [PATCH net] bonding: Prevent IPv6 link local address on enslaved devices

[Jay Vosburgh <jay.vosburgh@canonical.com>]

Karl Heiss <kheiss@gmail.com> wrote:

>On Fri, Jan 8, 2016 at 2:56 PM, Jay Vosburgh <jay.vosburgh@canonical.com> wrote:
>> Karl Heiss <kheiss@gmail.com> wrote:
[...]
>>>@@ -1216,7 +1215,6 @@ static void bond_upper_dev_unlink(struct net_device *bond_dev,
>>>                                 struct net_device *slave_dev)
>>> {
>>>       netdev_upper_dev_unlink(slave_dev, bond_dev);
>>>-      slave_dev->flags &= ~IFF_SLAVE;
>>>       rtmsg_ifinfo(RTM_NEWLINK, slave_dev, IFF_SLAVE, GFP_KERNEL);
>>> }
>>
>>         Will this change cause issues for user space monitoring of the
>> RTM_NEWLINKs, as now the message will have IFF_SLAVE in the flags for
>> both the "link" and "unlink" cases?  How would link be distinguished
>> from unlink?
>>
>>         Since the unlink happens only in __bond_release_one or in the
>> case of a failure within bond_enslave, does clearing the flag in
>> bond_upper_dev_unlink cause any actual issues?
>>
>>         -J
>>
>
>Oops.  You are correct that the RTM_NEWLINK would appear to be identical to
>the link case.  I had originally done this to prevent any NETDEV_CHANGE events
>from causing the link local address and subsequent neighbor advertisements just
>as the device is unlinked.  However, the bond_upper_dev_unlink() changes were a
>result of speculation, not actual observation.
>
>If we feel that we are safe from any NETDEV_CHANGE events and/or the
>consequences during unlink, I am fine with leaving the bond_upper_dev_unlink()
>code as-is.

	I looked briefly, and I don't see a source of NETDEV_CHANGE
notifiers between the bond_upper_dev_unlink and dev_close calls in
__bond_release_one.  Note that dev_set_promiscuity / allmulti do end up
in __dev_notify_flags, but it excludes NETDEV_CHANGE for PROMISC and
ALLMULTI, so I think that's not an issue.

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com