Re: Regression in bonding driver - devices without set_mac

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

Toby Corkindale <tjc@wintrmute.net> wrote:

>I originally reported this in Dec 2014, as:
>https://bugzilla.kernel.org/show_bug.cgi?id=89161
>
>The bug is still present in the 4.2 Linux kernel.

	I had some time to look into this today, and there is a related
code path that panics the kernel, so I'm working on a patch to resolve
things, and will be posting that shortly.

>If you look at the code, here:
>https://github.com/torvalds/linux/commit/f54424412b6b2f64cae4d7c39d981ca14ce0052c
>
>Then you see that the intention is for devices without set_mac support
>to be supported. Although in older code they DID work, and that
>ability died sometime near this commit. And has never returned since.

	Unfortunately, I believe the referenced commit is in error.  The
change log states:

	[...] It's wrong because we
    only require ndo_set_mac_address in case bonding is in active-backup mode
    and FOM isn't FOM_ACTIVE.

	This assertion is incorrect; ndo_set_mac_address is necessary
for all modes with the only exception being active-backup with
fail_over_mac enabled.

	All of the load balance modes (everything except active-backup)
will change the MAC of the slave devices at some point, and thus require
ndo_set_mac_address support.

	PPP devices should function in active-backup mode, and should
enable fail_over_mac automatically (if the PPP device is the first
slave) if f_o_m is not already enabled.

	The prior discussion on this patch can be found here:

http://www.spinics.net/lists/netdev/msg289311.html

>The code path in current kernels does allow devices through that block
>of code, but it fails somewhere else -- the devices are not
>successfully added as slaves, but the only error printed is the
>warning about not supporting MAC address setting.

	What is happening in the current code is that, for load balance
modes, e.g., balance-rr, the test for ndo_set_mac_address support
passes, but, later, bond_enslave attempts to actually set the MAC of the
new slave, and this fails (because there is no ndo_set_mac).

>Is there anything I can do to try and sort this regression out, with you?

	As I said, I do not believe this is a regression.

	-J

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