Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752928AbbFFN3e (ORCPT ); Sat, 6 Jun 2015 09:29:34 -0400 Received: from mail-ie0-f181.google.com ([209.85.223.181]:36554 "EHLO mail-ie0-f181.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751728AbbFFN3X (ORCPT ); Sat, 6 Jun 2015 09:29:23 -0400 MIME-Version: 1.0 In-Reply-To: <1433543047-34803-1-git-send-email-jarod@redhat.com> References: <1433543047-34803-1-git-send-email-jarod@redhat.com> Date: Sat, 6 Jun 2015 15:29:22 +0200 Message-ID: Subject: Re: [PATCH] net/bonding: fix propagation of user-specified bond MAC From: Nikolay Aleksandrov To: Jarod Wilson Cc: linux-kernel@vger.kernel.org, Jay Vosburgh , Veaceslav Falico , Andy Gospodarek , "open list:BONDING DRIVER" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 3364 Lines: 78 On Sat, Jun 6, 2015 at 12:24 AM, Jarod Wilson wrote: > Its possible for users to specify their own MAC address for a bonded link, > and this used to work, until sometime in 2013... > > First, commit 409cc1f8a changed a condition to set the bond's mac to a > slave device's, dropping the is_zero_ether_addr() check in favor of using > bond->dev_addr_from_first. > > Next, commit 6c8c4e4c2 added a bond->slave_cnt == 0 condition. > > Then, commit 97a1e6396 removed dev_addr_from_first and keyed off of > bond->dev->addr_assign_type. > > The other contitional in the check to call bond_set_dev_addr() has gone > through some permutations, finally landing at the following check: > > if (!bond_has_slaves(bond) && > bond->dev->addr_assign_type == NET_ADDR_RANDOM) > bond_set_dev_addr(bond->dev, slave_dev); > > When the bond is initially brought up, with no active slaves, it gets > assigned a random address, and addr_assign_type is set to NET_ADDR_RANDOM. > Next up though, the user can provide their own MAC, which ultimately calls > bond_set_mac_address(). However, because addr_assign_type isn't touched, > the above conditions are still met, and the slave's MAC overwrites the > user-provided MAC. > > The simple fix is to set addr_assign_type = NET_ADDR_SET at the tail end > of bond_set_mac_address() doing its thing, and user-specified MAC > addresses no longer get overwritten. > > Nb: this is slightly tricky to test on current Fedora, as nmcli seems to > be braindead when it comes to setting a MAC address for a bond. I can do a > "nmcli con edit bond0", set ethernet.mac-address "xx:yy:zz:aa:bb:cc", but > it doesn't ever seem to do anything, and it doesn't persist to the next > boot. Manual tinkering was required to verify the issue and the fix using > ip link set commands. > > CC: Jay Vosburgh > CC: Veaceslav Falico > CC: Andy Gospodarek > CC: netdev@vger.kernel.org (open list:BONDING DRIVER) > Signed-off-by: Jarod Wilson > --- Hi Jarod, When I did 97a1e6396, I tested all of these cases successfully and they still work. in net/core/dev.c, dev_set_mac_address() we have: dev->addr_assign_type = NET_ADDR_SET; So it's actually changed when the user sets the mac and you don't have to do it in bond_set_mac_address(). Just to confirm, I tried this just now: # modprobe bonding # ip l sh bond0 9: bond0: mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default link/ether d2:62:c7:90:93:b9 brd ff:ff:ff:ff:ff:ff # ip l set dev bond0 address 00:11:22:33:44:55 # ip l sh bond0 9: bond0: mtu 1500 qdisc noqueue state DOWN mode DEFAULT group default link/ether 00:11:22:33:44:55 brd ff:ff:ff:ff:ff:ff # ifenslave bond0 enp6s0 # ip l sh bond0 9: bond0: mtu 1500 qdisc noqueue state UP mode DEFAULT group default link/ether 00:11:22:33:44:55 brd ff:ff:ff:ff:ff:ff The user-specified mac address is kept. Cheers, Nik -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/