Bug 1367261

Summary: bond's IP changes after reboot RHVH which is active-backup mode
Product: Red Hat Enterprise Linux 7 Reporter: Huijuan Zhao <huzhao>
Component: cockpitAssignee: Marius Vollmer <mvollmer>
Status: CLOSED CURRENTRELEASE QA Contact: qe-baseos-daemons
Severity: urgent Docs Contact:
Priority: urgent    
Version: 7.2CC: aloughla, atragler, bgalvani, bmcclain, bugs, cshao, danken, dguo, edwardh, fdeutsch, fgiudici, huzhao, leiwang, lrintel, mpitt, mvollmer, rbarry, rkhan, snagar, stefw, sukulkar, thaller, weiwang, yaniwang, ycui, ylavi, yzhao
Target Milestone: pre-dev-freezeKeywords: Extras
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2018-03-02 09:23:46 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: Network RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Embargoed:
Bug Depends On: 1420708    
Bug Blocks: 1326798, 1329957    
Attachments:
Description Flags
screenshot of incorrect display of cockpit
none
screenshot of bond settings
none
All logs
none
All files in /etc/sysconfig/network-scripts none

Description Huijuan Zhao 2016-08-16 05:24:36 UTC
Created attachment 1191065 [details]
screenshot of incorrect display of cockpit

Description of problem:
Setup bond0 over two NICs with active-backup mode, reboot RHVH, the bond0's IP is changed(change to backup NIC's IP), and cockpit can not display bond0's member NICs Sending/Receiving packages.
The bond0's IP should not change after reboot RHVH with active-backup mode.


Version-Release number of selected component (if applicable):
redhat-virtualization-host-4.0-20160812.0
imgbased-0.8.4-1.el7ev.noarch
cockpit-ws-0.114-2.el7.x86_64
cockpit-ovirt-dashboard-0.10.6-1.3.6.el7ev.noarch


How reproducible:
100%


Steps to Reproduce:
1. Install RHVH rhvh-4.0-0.20160812.0
2. Login cockpit via "ip:9090"
3. Enter to Networking page, setup bond0 with active-backup mode over two NICs(enp2s0 and enp1s7).
   And enp2s0 is primary NIC, both NICs are "Connect automatically".
4. Reboot RHVH, focus on bond0's IP
5. Login cockpit -> Networking page, enter to bond0 page, check member NICs' Sending/Receiving packages


Actual results:
1. After step3, bond0 can get IP1(enp2s0's IP). Enter to bond0 page, the cockpit can display member NICs' Sending/Receiving packages.
2. After step4, bond0 gets another IP2(enp1s7's IP)
3. After step5, the cockpit can not display member NICs' Sending/Receiving packages(Please refer to the attachment for detailed info)

Expected results:
2. After step4, bond0 should not change IP, should also get IP1
3. After step5, the cockpit can display member NICs' Sending/Receiving packages

Additional info:

Comment 1 Huijuan Zhao 2016-08-16 05:25:49 UTC
Created attachment 1191067 [details]
screenshot of bond settings

Comment 2 Huijuan Zhao 2016-08-16 05:26:35 UTC
Created attachment 1191069 [details]
All logs

Comment 3 Huijuan Zhao 2016-08-16 05:32:09 UTC
Created attachment 1191074 [details]
All files in /etc/sysconfig/network-scripts

Comment 4 Huijuan Zhao 2016-08-16 05:36:20 UTC
Additional info:
This issue will cause many failures, such as RHVH can not connect RHVM once reboot

Comment 5 Fabian Deutsch 2016-08-16 08:43:24 UTC
I don't see any relevant node impact here, moving to network.

Comment 6 Yaniv Kaul 2016-08-17 06:41:48 UTC
Does this has anything to do with oVirt?
1. Does it happen in RHEL as well?
2. Does it happen when configuring the bond via nmcli directly, not via Cockpit?

Comment 7 Huijuan Zhao 2016-08-18 11:15:06 UTC
(In reply to Yaniv Kaul from comment #6)
> Does this has anything to do with oVirt?
> 1. Does it happen in RHEL as well?
> 2. Does it happen when configuring the bond via nmcli directly, not via
> Cockpit?

Hi Yaniv,

Yes, this is cockpit and network related issue.
Due to already hide Network in cockpit, this issue is not critical for RHVH 4.0 GA. And I have to focus on network and upgrade feature testing now on RHVH 20160817 build. So I will test scenario 1 and 2 later.

Thanks!

Comment 8 Yaniv Kaul 2016-08-18 12:25:38 UTC
(In reply to Huijuan Zhao from comment #7)
> (In reply to Yaniv Kaul from comment #6)
> > Does this has anything to do with oVirt?
> > 1. Does it happen in RHEL as well?
> > 2. Does it happen when configuring the bond via nmcli directly, not via
> > Cockpit?
> 
> Hi Yaniv,
> 
> Yes, this is cockpit and network related issue.

So why is the bug on VDSM?

> Due to already hide Network in cockpit, this issue is not critical for RHVH
> 4.0 GA. And I have to focus on network and upgrade feature testing now on
> RHVH 20160817 build. So I will test scenario 1 and 2 later.
> 
> Thanks!

Comment 9 Huijuan Zhao 2016-08-18 12:43:54 UTC
Hi Fabian,

Could you help to move this bug to right component? 
Thanks!

Comment 10 Ryan Barry 2016-08-18 14:09:31 UTC
I'll test this today, but since cockpit interacts with networkmanager directly, I suspect that this is probably reproducible when using nmcli directly, and probably also with RHEL.

The display in cockpit is definitely cockpit, however:

Aug 16 02:38:48 dell-pet105-02 kernel: bond0: link status definitely up for interface enp2s0, 1000 Mbps full duplex
Aug 16 02:38:48 dell-pet105-02 kernel: bond0: making interface enp2s0 the new active one
Aug 16 02:38:48 dell-pet105-02 kernel: bond0: first active interface up!
Aug 16 02:38:48 dell-pet105-02 NetworkManager[920]: <info>  Activation (bond0) Beginning DHCPv4 transaction (timeout in 45 seconds)
Aug 16 02:38:48 dell-pet105-02 NetworkManager[920]: <info>  dhclient started with pid 1323
Aug 16 02:38:48 dell-pet105-02 dhclient[1323]: DHCPREQUEST on bond0 to 255.255.255.255 port 67 (xid=0x201f293e)
Aug 16 02:38:49 dell-pet105-02 kernel: e1000: enp1s7 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: RX/TX
Aug 16 02:38:49 dell-pet105-02 NetworkManager[920]: <info>  (enp1s7): link connected
Aug 16 02:38:49 dell-pet105-02 kernel: bond0: link status definitely up for interface enp1s7, 1000 Mbps full duplex
Aug 16 02:38:52 dell-pet105-02 dhclient[1323]: DHCPREQUEST on bond0 to 255.255.255.255 port 67 (xid=0x201f293e)
Aug 16 02:38:54 dell-pet105-02 NetworkManager[920]: <info>  (bond0): device state change: ip-config -> ip-check (reason 'none') [70 80 0]
Aug 16 02:38:54 dell-pet105-02 NetworkManager[920]: <info>  (bond0): device state change: ip-check -> secondaries (reason 'none') [80 90 0]
Aug 16 02:38:54 dell-pet105-02 NetworkManager[920]: <info>  (bond0): device state change: secondaries -> activated (reason 'none') [90 100 0]
Aug 16 02:38:54 dell-pet105-02 NetworkManager[920]: <info>  NetworkManager state is now CONNECTED_LOCAL
Aug 16 02:38:54 dell-pet105-02 NetworkManager[920]: <info>  NetworkManager state is now CONNECTED_GLOBAL
Aug 16 02:38:54 dell-pet105-02 NetworkManager[920]: <info>  Policy set '1d49153e-a5d9-4d79-9e19-74aabd126868' (bond0) as default for IPv6 routing and DNS.
Aug 16 02:38:54 dell-pet105-02 NetworkManager[920]: <info>  (bond0): Activation: successful, device activated.
Aug 16 02:38:54 dell-pet105-02 nm-dispatcher: Dispatching action 'up' for bond0
Aug 16 02:39:02 dell-pet105-02 dhclient[1323]: DHCPDISCOVER on bond0 to 255.255.255.255 port 67 interval 8 (xid=0x7a7e6f31)
Aug 16 02:39:03 dell-pet105-02 dhclient[1323]: DHCPREQUEST on bond0 to 255.255.255.255 port 67 (xid=0x7a7e6f31)
Aug 16 02:39:03 dell-pet105-02 dhclient[1323]: DHCPOFFER from 10.66.151.254
Aug 16 02:39:03 dell-pet105-02 dhclient[1323]: DHCPACK from 10.66.151.254 (xid=0x7a7e6f31)
...
Aug 16 02:39:03 dell-pet105-02 dhclient[1323]: bound to 10.66.149.131 -- renewal in 6775 seconds.

It appears (to me) that this is handled correctly --

The correct master is brought up
The DHCP request is sent out through bond0 (from that master)

But the "wrong" IP address is returned (since both slave interfaces are set to "DHCP", I can't know what DHCP reservations they have).

Without seeing the logs from dhcpd (or packet dumps), it's unknown what MAC was sent out.

This does not appear to be a bug in vdsm or cockpit, though. Maybe not even networkmanager.

Can you please provide the dhcpd logs for this request (if you can get them)? Or stop the interface, start tcpdump, and grab the traffic?

Comment 11 Huijuan Zhao 2016-08-20 09:27:34 UTC
(a)
“2. Does it happen when configuring the bond via nmcli directly, not via Cockpit?“

(a.1) Still encounter this issue via nmtui on redhat-virtualization-host-4.0-20160817.0. Not just cockpit related issue. So I think this is urgent, will change Severity to Urgent.
(a.2) But no such issue when write ifcfg-bond0 (and slaves ifcfg-enp2s0, ifcfg-enp1s7) manually on redhat-virtualization-host-4.0-20160817.0.

(b)
"Can you please provide the dhcpd logs for this request (if you can get them)? Or stop the interface, start tcpdump, and grab the traffic?"

I need eng-ops@ help to get the log, so I will  offer it later when eng-ops@ is available. 
But no such issue on vintage builds(RHEV-H-7.2 for 3.6) with the same machine and NICs, so I think maybe this is just RHVH4.0 issue.

Comment 12 Huijuan Zhao 2016-08-22 07:48:39 UTC
"1. Does it happen in RHEL as well?"

Yes, it happens in RHEL as well (rhel-atomic-installer-7.2-10.x86_64.iso)

Comment 13 Dan Kenigsberg 2016-08-22 07:59:05 UTC
So please reassign the bug to NetworkManager. I'm not 100% it is their bug, but it certainly has nothing to do with ovirt.

Comment 14 Huijuan Zhao 2016-08-22 08:37:24 UTC
(In reply to Huijuan Zhao from comment #12)
> "1. Does it happen in RHEL as well?"
> 
> Yes, it happens in RHEL as well (rhel-atomic-installer-7.2-10.x86_64.iso)

Update:

It also happens in RHEL 7.2 release build.

Test version:
Red Hat Enterprise Linux Server 7.2 (Maipo)
kernel-3.10.0-327.el7.x86_64
NetworkManager-1.0.6-27.el7.x86_64
NetworkManager-tui-1.0.6-27.el7.x86_64

Comment 15 Ying Cui 2016-08-22 08:42:06 UTC
Dan, see bug comment 0, comment 12, and comment 14, we need to move this bug to platform, not related vdsm part.

Comment 18 Francesco Giudici 2016-12-19 15:47:55 UTC
This issue seems to have the same root cause of https://bugzilla.redhat.com/show_bug.cgi?id=1387506

As explained in https://bugzilla.redhat.com/show_bug.cgi?id=1387506#c4, The MAC address of the bond device is the one of the first enslaved device. In NM 1.0.x, devices are added at boot as soon as they are available to NM, so the order in which the devices are enslaved (and so the bond MAC) is somehow random.
In NM-1-4 (RHEL-7.3), devices start sorted by their ifindex.

So, in order to have a persistant MAC address at boot you have to rely on nm-1-4 or adopt the WA suggested on the same above link, i.e., set the ethernet.cloned-mac-address of both slaves to the same value.

There is still an issue with nm-1-4: when you configure the bond, the MAC will be the one from the first device you enslave. But after the first reboot, the MAC will be the one from the slave device with the lower ifindex.
The WA instead should avoid this too.

Having NM enforcing the MAC on some policy during setup (e.g., taking the MAC with "lower value" like happens in bridges) seems not a good thing to do: when you add another slave to your already running bond, you don't want to disrupt connectivity (which is something will happen for sure if you change your MAC).

So, summing up current situation: 
1) the issue is due to the fact that bond MAC is borrowed from the first enslaved device
2) nm-1-4 provides consistent MAC at boot, but cannot ensure the MAC will be preserved when rebooting after initial configuration
3) a WA is available
4) Changing the MAC when a new slave is added would disrupt connectivity

The solution would be to allow to specify the bond MAC address explicitly. This is not yet allowed, a bug (rfe) tracking this is here:
https://bugzilla.redhat.com/show_bug.cgi?id=1386872

Would be resorting to the WA viable, at least till the new feature is in place?

Comment 19 Francesco Giudici 2017-01-05 13:54:35 UTC
the rfe that allows setting bond MAC is in modified state, i.e., we have a patch ready upstream. It will be ported in RHEL-7.4-
nm-1-4, as already said, provides consistent enslaving order to bond interface at boot (sorted by ifindex), preserving MAC.
I think nothing more should be done in NM for this.

Still one point is open: when the initial configuration is done in cockpit, the order in which slave connections are enabled matters (MAC will be the one from the first enslaved interface): I guess this is under cockpit control.
After the first reboot, the MAC will be the one from the interface with lower ifindex instead (NM policy). So, we have to sync cockpit activation order policy with the one from NM.
Or maybe we can just force bond to have a fixed MAC, leveraging:
https://bugzilla.redhat.com/show_bug.cgi?id=1386872
setting bond MAC during initial configuration.
Or, still, change all the slave MAC to the desired one (which should work also for already shipped NetworkManager versions).
But this would require cockpit to allow to set the bond MAC I guess.
So, moving this to cockpit component.

Comment 20 Marius Vollmer 2017-01-11 08:40:50 UTC
(In reply to Francesco Giudici from comment #19)

> Still one point is open: when the initial configuration is done in cockpit,
> the order in which slave connections are enabled matters (MAC will be the
> one from the first enslaved interface): I guess this is under cockpit
> control.

It is.

Would it help to first configure the relevant interfaces as slaves, and then create the bond?

E.g., Cockpit could do the equivalent of:

 # nmcli con add type ethernet slave-type bond ifname eth0 master bond0
 # nmcli con add type ethernet slave-type bond ifname eth1 master bond0
 # nmcli can add type bond ifname bond0

Would that lead to a consistent MAC?

[ The only issue with this is "type ethernet".  This might not be the
  appropriate type for the interface, and we would need to add code to
  Cockpit to guess the right type for the interface.  I guess this could
  be done but feels wrong and would make me shake my little fist at NM a bit.
]

Failing this, how can Cockpit add interfaces in the right order?  Is ifindex exposed on D-Bus?

Comment 21 Marius Vollmer 2017-01-11 11:13:26 UTC
(In reply to Francesco Giudici from comment #19)

> Or maybe we can just force bond to have a fixed MAC, leveraging:
> https://bugzilla.redhat.com/show_bug.cgi?id=1386872
> setting bond MAC during initial configuration.

I'll work on this.  I'll add UI to change the MAC in general, and make it so that a new bond gets an appropriate MAC.

Comment 22 Francesco Giudici 2017-01-12 11:46:07 UTC
(In reply to Marius Vollmer from comment #20)
> (In reply to Francesco Giudici from comment #19)
> 
> > Still one point is open: when the initial configuration is done in cockpit,
> > the order in which slave connections are enabled matters (MAC will be the
> > one from the first enslaved interface): I guess this is under cockpit
> > control.
> 
> It is.
> 
> Would it help to first configure the relevant interfaces as slaves, and then
> create the bond?
> 
> E.g., Cockpit could do the equivalent of:
> 
>  # nmcli con add type ethernet slave-type bond ifname eth0 master bond0
>  # nmcli con add type ethernet slave-type bond ifname eth1 master bond0
>  # nmcli can add type bond ifname bond0
> 
> Would that lead to a consistent MAC?
> 

Well, that should work, also if in an indirect way: at NM boot the device will be added to the device list sorted by their ifindex.
When adding ifname bond0 it will end up triggering the "schedule_activate_all_cb" which will check and try to activate all devices in the device list, checking the list from the beginning.
Still this does not seem to me very robust, as when adding/removing devices from the list, the ifindex sorting is not preserved (so this would not work for pluggable devices).

> [ The only issue with this is "type ethernet".  This might not be the
>   appropriate type for the interface, and we would need to add code to
>   Cockpit to guess the right type for the interface.  I guess this could
>   be done but feels wrong and would make me shake my little fist at NM a bit.
> ]

Well, just a note... I don't think at ifindex sorting as the definitive answer to (boot) activation order of device/connections... this is much better than nothing and is already in place in NM, but maybe we could find a better policy to ensure activation order. 

> 
> Failing this, how can Cockpit add interfaces in the right order?  Is ifindex
> exposed on D-Bus?

No, NM does not expose device ifindex on dbus. But I guess you can retrieve it easily from the interface name (e.g., in sysfs: /sys/class/net/<ifname>/ifindex)

Comment 23 Francesco Giudici 2017-01-12 11:48:57 UTC
(In reply to Marius Vollmer from comment #21)
> (In reply to Francesco Giudici from comment #19)
> 
> > Or maybe we can just force bond to have a fixed MAC, leveraging:
> > https://bugzilla.redhat.com/show_bug.cgi?id=1386872
> > setting bond MAC during initial configuration.
> 
> I'll work on this.  I'll add UI to change the MAC in general, and make it so
> that a new bond gets an appropriate MAC.

Thanks Marius, I think that would be the safest solution.

Comment 24 Yaniv Lavi 2017-01-23 08:20:59 UTC
Can you please provide a update on this fix?

Comment 25 Marius Vollmer 2017-01-23 08:29:03 UTC
(In reply to Yaniv Dary from comment #24)
> Can you please provide a update on this fix?

The work for this is lined up for the next sprint, which will start sometime after devconf.cz, so beginning of February.

https://trello.com/c/Do2N49tA/451-networking-allow-changing-mac-and-set-for-bonds-etc

Comment 26 Edward Haas 2017-02-02 09:28:30 UTC
Per my understanding, the only consistent solution to preserve the bond mac address from definition through reboots is to set cloned-mac-address to stable.

The problem I see with this from the client side (in my case oVirt host) is that we will not be able to acquire the device (take the device from NM control to client control) and preserve the address.
This will be incompatible with initscripts and perhaps other options, as the logic is internal to NM only.

I would have preferred to specify which is the slave from which the bond address should be taken.

Comment 27 Yaniv Lavi 2017-02-02 11:32:33 UTC
What is the status? When should we expect a fix for this issue?

Comment 28 Yaniv Lavi 2017-02-02 14:21:34 UTC
The fix proposed is not a good option for us, please consider the option outlined in comment #26.

Comment 29 Marius Vollmer 2017-02-06 09:48:06 UTC
(In reply to Edward Haas from comment #26)

> I would have preferred to specify which is the slave from which the bond
> address should be taken.

For a active-backup bond, the mac address is taken from the primary interface, just as the IP config.  Is that appropriate?

In any case, you can edit the mac address after creating the bond, and we might provide a drop-down that lists all mac addresses of all slaves.  Would that be enough?

Comment 30 Marius Vollmer 2017-02-06 09:50:43 UTC
> When should we expect a fix for this issue?

We are still discussing what the fix should be, right?

Comment 31 Marius Vollmer 2017-02-06 09:58:21 UTC
(This bug has been "urgent" for 6 month now, so I guess I have another month or so, right? :-)

Comment 32 Edward Haas 2017-02-06 11:19:48 UTC
(In reply to Marius Vollmer from comment #29)
> (In reply to Edward Haas from comment #26)
> 
> > I would have preferred to specify which is the slave from which the bond
> > address should be taken.
> 
> For a active-backup bond, the mac address is taken from the primary
> interface, just as the IP config.  Is that appropriate?

Yes, that is consistent on bond definition, but per this BZ, it is not consistent over reboots.
If NM can wait a limited time for all the slaves to be detected and only then attach them to the bond based on the same rule, we should be fine for this mode.

> 
> In any case, you can edit the mac address after creating the bond, and we
> might provide a drop-down that lists all mac addresses of all slaves.  Would
> that be enough?

Editing the mac address by the client requires it to manage it, something that we hoped to avoid.
This will mean that the client should determine the current mac of the bond, persist it and when acquiring apply the mac on the refreshed bond.

We would have preferred not to manage the bond mac, just let the bond logic handle it. But if there will be no other solution, we will have no choice.

Comment 33 Marius Vollmer 2017-02-06 12:20:29 UTC
Note that explicitly setting a bond mac address requires the work from bug 1386872 as well.

Comment 34 Marius Vollmer 2017-02-06 12:29:58 UTC
(In reply to Edward Haas from comment #32)
> (In reply to Marius Vollmer from comment #29)
> > (In reply to Edward Haas from comment #26)
> > 
> > > I would have preferred to specify which is the slave from which the bond
> > > address should be taken.
> > 
> > For a active-backup bond, the mac address is taken from the primary
> > interface, just as the IP config.  Is that appropriate?
> 
> Yes, that is consistent on bond definition, but per this BZ, it is not
> consistent over reboots.

Yes, I didn't express myself very well.  Cockpit will set ethernet.cloned-mac-address during bond creation to the mac of the primary slave.  It will (initially) not automatically change ethernet.cloned-mac-address when the primarly slave changes later on.

In addition to that, Cockpit will also allow people to set ethernet.cloned-mac-address explicitly.

> This will mean that the client should determine the current mac of the bond,
> persist it and when acquiring apply the mac on the refreshed bond.

I don't understand this.

> We would have preferred not to manage the bond mac, just let the bond logic
> handle it. But if there will be no other solution, we will have no choice.

I too am suprised that it seems to be necessary to fine-tune the MAC of a bond for sane operation.  Shouldn't the default already be sane?

Comment 35 Edward Haas 2017-02-06 15:01:01 UTC
(In reply to Marius Vollmer from comment #34)
> 
> Yes, I didn't express myself very well.  Cockpit will set
> ethernet.cloned-mac-address during bond creation to the mac of the primary
> slave.  It will (initially) not automatically change
> ethernet.cloned-mac-address when the primarly slave changes later on.
> 
> In addition to that, Cockpit will also allow people to set
> ethernet.cloned-mac-address explicitly.
> 
> > This will mean that the client should determine the current mac of the bond,
> > persist it and when acquiring apply the mac on the refreshed bond.
> 
> I don't understand this.

I refer to NM clients: cockpit, ovirt vdsm.
Flow/Steps:
- cockpit sets the initial configuration, NM applies it and controls it.
- vdsm comes and acquires NM interfaces, asking NM to unmanage the interface and defining it using initscripts (or other options).
- vdsm controls the interface from now on.

The only way to be in sync and have a common behaviour, is to depend on NM logic to choose the mac, otherwise each client needs to interfere and keep some logic.
Current VDSM logic is depending on the initscripts, and it seemed to have worked so far well. We would prefer cockpit and NM to keep initscript logic and be consistent from definition through reboots.
comment 20 seems to follow initscripts logic (not 100% sure).


> 
> > We would have preferred not to manage the bond mac, just let the bond logic
> > handle it. But if there will be no other solution, we will have no choice.
> 
> I too am suprised that it seems to be necessary to fine-tune the MAC of a
> bond for sane operation.  Shouldn't the default already be sane?

initscripts behaviour seemed to be sane so far, I hope we can keep it.

Comment 36 Stef Walter 2017-02-06 15:26:52 UTC
@mariusvollmer says: one thing that I forgot until now: we need NetworkManager 1.6 to set MACs on bonds

Comment 37 Marius Vollmer 2017-02-07 11:55:51 UTC
https://github.com/cockpit-project/cockpit/pull/5851

Comment 38 Yaniv Lavi 2017-02-07 23:13:03 UTC
Please review the patch.

Comment 39 Edward Haas 2017-02-08 07:09:20 UTC
(In reply to Yaniv Dary from comment #38)
> Please review the patch.

There are several options to stabilize the bond mac address from definition through reboots, the provided patch solves it by exposing the option to set the mac address (which by the way is only relevant for NM 1.6, RHEL7.3 has NM 1.4).
It is a valid solution for cockpit-networkmanager  sync, but if we add here initscripts and oVirt, plus simplicity from the user side, IMO it is not enough.

We are currently under discussions on this subject to see how we can solve it such that it will satisfy all parties.

Comment 40 Marius Vollmer 2017-02-08 09:02:42 UTC
> initscripts behaviour seemed to be sane so far, I hope we can keep it.

What is the initscripts behavior exactly?

Does it all come down to in what order interfaces are enslaved when activating a bond?  This fundamentally introduces unreliability, no?  Can we get rid of this behavior in the bonding driver?

But assuming that order does matter:

Right now, Cockpit first creates the bond and then enslaves its interfaces in essentially random order (probably alphabetical, have to check).

When NetworkManager activates a fully configured bridge, it tries to add the slaves in a stable order, using ifindex.  Since order matters, Cockpit should let NetworkManager take care of it.  Cockpit needs to first configure the interfaces as slaves of the not-yet-existing bond, and then create the bond.

Then the question is: Do the initscripts use the same order as NetworkManager?  If not, how do we fix that?

Comment 41 Marius Vollmer 2017-02-08 09:11:37 UTC
(In reply to Edward Haas from comment #39)

> There are several options to stabilize the bond mac address from definition
> through reboots, the provided patch solves it by exposing the option to set
> the mac address (which by the way is only relevant for NM 1.6, RHEL7.3 has
> NM 1.4).

The essential change that is supposed to fix the bug here is

    https://github.com/cockpit-project/cockpit/pull/5851/commits/01acde56453f67e29b3121e54dc05cc62db3b02c

This will essentially add a line

    MACADDR=<mac>

to the ifcfg-* file for the bond master.

The <mac> in that line comes from the primary interface of a active-backup mode bond, or if a single slave was active before creating the bond, from that single active slave.

(The fact that the user can now also change this line explicitly in the UI is not essential for the fix, but I feel we need to allow this since Cockpit writes these line automatically in certain cases.)

> It is a valid solution for cockpit-networkmanager  sync, but if we add here
> initscripts and oVirt, plus simplicity from the user side, IMO it is not
> enough.

Initscripts also support MACADDR (and it's a bit disappointing that NM doesn't yet for bonds and bridges, IMO), so what is missing for initscripts?

Comment 42 Marius Vollmer 2017-02-08 09:20:05 UTC
(In reply to Edward Haas from comment #35)

> The only way to be in sync and have a common behaviour, is to depend on NM
> logic to choose the mac, otherwise each client needs to interfere and keep
> some logic.

The proposed solution would configure an explicit MAC address for the bond master via a MACADDR=<mac> entry in the ifcfg-* file.  This should remove all instability of the bond MAC address, and is supported by the initscripts, no?

(Unfortunately, it's not yet supported by NetworkManager for bonds and bridges, so we have to wait for that.)

Comment 43 Francesco Giudici 2017-02-08 09:41:43 UTC
(In reply to Marius Vollmer from comment #40)
> > initscripts behaviour seemed to be sane so far, I hope we can keep it.
> 
> What is the initscripts behavior exactly?
> 
> Does it all come down to in what order interfaces are enslaved when
> activating a bond?  This fundamentally introduces unreliability, no?  Can we
> get rid of this behavior in the bonding driver?

I think the point for the bonding driver is that it allows to enslave new interfaces without affecting connectivity. (But... why then don't let the default MAC created when the bond interface has no slaves be kept there?)

> 
> But assuming that order does matter:
> 
> Right now, Cockpit first creates the bond and then enslaves its interfaces
> in essentially random order (probably alphabetical, have to check).
> 
> When NetworkManager activates a fully configured bridge, it tries to add the
> slaves in a stable order, using ifindex.  Since order matters, Cockpit
> should let NetworkManager take care of it.  Cockpit needs to first configure
> the interfaces as slaves of the not-yet-existing bond, and then create the
> bond.

IMHO this could be fine too. At least this would allow NM to add some logic in the selection of the MAC among the slaves when bringing up the bond.

> 
> Then the question is: Do the initscripts use the same order as
> NetworkManager?  If not, how do we fix that?

They don't. I think they are consistent because the same process is used when starting bond on creation and on boot: first, all the ifcfg files are created. Then network service is (re)started. It will process the ifcfg files in some order (I think alphabetical order but maybe is the file creation order... anyway, does not really matter).
On boot, network service will do the same, processing the files in the same order.

Not a very stable sorting though. A change in one file can trigger the change in the MAC.
Also NM approach, the ifindex number, is not 100% reliable in stability I guess.

Regarding syncing the two... would be nice to have this.
Easiest way is to fix the MAC of the bond I guess... otherwise we should add a common logic both in NM and in initscripts.

So, summarizing, I think we have only two options to achieve stable MAC picking:
1) set bond MAC
2) (a) cockpit creates first slaves and then bond connection; (b) NM and initscripts should implement a (new?) common logic to pick bond MAC.

Comment 44 Edward Haas 2017-02-08 10:46:02 UTC
The initscripts logic for bonds goes something like this:
On ifup <bond name>, the bond is created and all its slaves are collected and attached in an alphabetical order: for device in $(LANG=C grep -l "^[[:space:]]*MASTER=\"\?${DEVICE}\"\?\([[:space:]#]\|$\)" /etc/sysconfig/network-scripts/ifcfg-*) ; do

Therefore, the order of attaching slaves is by their ifcfg file name.
If NM will attach its slaves based on the slave (device) name in an alphabetic order, we should be in sync.
Cockpit or other clients that uses NM, should create first the slaves and then the bond, so NM logic will kick in.

The alternative, setting the bond mac is something I hope we can avoid, but if we have no other choice, it should go something like this:
- Create the bond with the slaves attached.
- Let NM logic decide which mac should be used.
- Fix the given MAC so it will not change on reboots (cloned-mac-address).
oVirt as a client, will have to acquire bonds from NM, by preserving the given mac address, setting it in the bond ifcfg file.

IMO Cockpit or other clients should not keep any logic on what mac to choose, so it will not collide with other clients logic. Allowing the client to set the mac address manually is fine by me, but that means the user knows what he is doing.

Comment 45 Marius Vollmer 2017-02-08 11:28:06 UTC
(In reply to Francesco Giudici from comment #43)

> So, summarizing, I think we have only two options to achieve stable MAC
> picking:
> 1) set bond MAC
> 2) (a) cockpit creates first slaves and then bond connection; (b) NM and
> initscripts should implement a (new?) common logic to pick bond MAC.

And both options need changes from NM, right?

Why do we want to switch from NM to initscripts mid-way anyway?  Because Linux is about choice? :-)


Option 1) is in https://github.com/cockpit-project/cockpit/pull/5851.

It has the disadvantage that the bond will keep the same MAC address even if the interface that provided that MAC initially is eventually removed from the bond.  At that point the bond and the newly-liberated interface have the same MAC, right?  This is the main argument against opton 1), since it requires the user to understand what's going on on a deeper level than what he needed to create the bond in the first place.

Comment 47 Francesco Giudici 2017-02-08 13:23:43 UTC
(In reply to Edward Haas from comment #44)
> The initscripts logic for bonds goes something like this:
> On ifup <bond name>, the bond is created and all its slaves are collected
> and attached in an alphabetical order: for device in $(LANG=C grep -l
> "^[[:space:]]*MASTER=\"\?${DEVICE}\"\?\([[:space:]#]\|$\)"
> /etc/sysconfig/network-scripts/ifcfg-*) ; do
> 
> Therefore, the order of attaching slaves is by their ifcfg file name.
> If NM will attach its slaves based on the slave (device) name in an
> alphabetic order, we should be in sync.

Well, here you rely on the ifcfg file name beeing ifcfg-<device_name>.
If we want to be in sync, we want to be in sync in a more robust way I would say.
Anyway, device name could for sure a metric to fix the enslave order.

> Cockpit or other clients that uses NM, should create first the slaves and
> then the bond, so NM logic will kick in.

Ok, this is option 2).

> 
> The alternative, setting the bond mac is something I hope we can avoid, but
> if we have no other choice, it should go something like this:
> - Create the bond with the slaves attached.
> - Let NM logic decide which mac should be used.
> - Fix the given MAC so it will not change on reboots (cloned-mac-address).
> oVirt as a client, will have to acquire bonds from NM, by preserving the
> given mac address, setting it in the bond ifcfg file.
> 
> IMO Cockpit or other clients should not keep any logic on what mac to
> choose, so it will not collide with other clients logic. Allowing the client
> to set the mac address manually is fine by me, but that means the user knows
> what he is doing.

Both option 1) and 2) seem fine to me.
Sure, option 1) means someone fixes the MAC on the bond (maybe the MAC spawned when bond interface is created and no slaves are still attached could be used)
but that seems no that bad. There is a static configuration that is valid and everyone will honor.
Option 2) would be fine too, let NM pick the enslaving order by creating slaves first.

Would syncing with initscripts be mandatory at this stage?

Comment 48 Marius Vollmer 2017-02-08 13:47:28 UTC
(In reply to Francesco Giudici from comment #47)

> Sure, option 1) means someone fixes the MAC on the bond (maybe the MAC
> spawned when bond interface is created and no slaves are still attached
> could be used)
> but that seems no that bad. There is a static configuration that is valid
> and everyone will honor.

Yes, if the bond gets a completely new MAC address that is not he same as any other MAC address on the network, then this should work and be stable.

But I think there is one extra complication: People want to put a active interface into a new bond, and the new bond should seamlessly take over for that that interface.  In this case, the bond needs to get the MAC of its slave.

Comment 49 Francesco Giudici 2017-02-08 14:17:38 UTC
(In reply to Marius Vollmer from comment #48)
> (In reply to Francesco Giudici from comment #47)
> 
> > Sure, option 1) means someone fixes the MAC on the bond (maybe the MAC
> > spawned when bond interface is created and no slaves are still attached
> > could be used)
> > but that seems no that bad. There is a static configuration that is valid
> > and everyone will honor.
> 
> Yes, if the bond gets a completely new MAC address that is not he same as
> any other MAC address on the network, then this should work and be stable.
> 
> But I think there is one extra complication: People want to put a active
> interface into a new bond, and the new bond should seamlessly take over for
> that that interface.  In this case, the bond needs to get the MAC of its
> slave.

So, this mean you want to enslave that interface first: but in this case you create the bond with just that interface, otherwise you don't have control on the MAC address picked up, right?
But than I guess you will end up adding other slaves (otherwise why a bond?) and you will have no assurance that the MAC will be kept on reboot. This is also the current behavior with initscripts I guess.

This anyway let me think that what to do with the bond MAC is strictly scenario dependent.
I would then say that if we want to be able to deal with all of these, we have to *allow* option 1 to the user but have option 2 as default (maybe relaxing the initscript sync at least for now).

Comment 50 Marius Vollmer 2017-02-09 07:35:12 UTC
(In reply to Francesco Giudici from comment #49)

> So, this mean you want to enslave that interface first: but in this case you
> create the bond with just that interface, otherwise you don't have control
> on the MAC address picked up, right?
>
> But than I guess you will end up adding other slaves (otherwise why a bond?)
> and you will have no assurance that the MAC will be kept on reboot. This is
> also the current behavior with initscripts I guess.

Yes, good summary.

> This anyway let me think that what to do with the bond MAC is strictly
> scenario dependent.
> I would then say that if we want to be able to deal with all of these, we
> have to *allow* option 1 to the user but have option 2 as default (maybe
> relaxing the initscript sync at least for now).

I agree.

I'll modify https://github.com/cockpit-project/cockpit/pull/5851 to _not_ set ethernet.cloned-mac-address of a new bond, but I'll keep the UI for manually setting it.  This removes the magic but adds knobs for people who know what they want.

I'll also make a second PR that changes how Cockpit creates bonds, bridges, and teams.  It'll first configure the slaves, then configure the bond, and then activate the bond.

Comment 51 Marius Vollmer 2017-02-09 13:37:10 UTC
(In reply to Marius Vollmer from comment #50)

> I'll also make a second PR that changes how Cockpit creates bonds, bridges,
> and teams.  It'll first configure the slaves, then configure the bond, and
> then activate the bond.

https://github.com/cockpit-project/cockpit/pull/5870

Comment 52 Marius Vollmer 2017-02-10 12:03:06 UTC
(In reply to Marius Vollmer from comment #50)

> I'll modify https://github.com/cockpit-project/cockpit/pull/5851 to _not_
> set ethernet.cloned-mac-address of a new bond, but I'll keep the UI for
> manually setting it.  This removes the magic but adds knobs for people who
> know what they want.

Done.

Comment 53 Marius Vollmer 2017-02-10 12:07:07 UTC
NetworkManager people, please checj my understanding:

With the two pull requests (#5851, #5870), Cockpit is doing the right thing by leaving the order of slave activation entirely to NetworkManager.  However, NetworkManager itself needs changes to enforce a consistent order across reboots.

This means that the erroneous behavior reported in the bug still happens, and a bond can still spontaneously change its MAC across a reboot, right?

Comment 54 Yaniv Lavi 2017-02-15 09:49:21 UTC
(In reply to Marius Vollmer from comment #53)
> NetworkManager people, please checj my understanding:
> 
> With the two pull requests (#5851, #5870), Cockpit is doing the right thing
> by leaving the order of slave activation entirely to NetworkManager. 
> However, NetworkManager itself needs changes to enforce a consistent order
> across reboots.
> 
> This means that the erroneous behavior reported in the bug still happens,
> and a bond can still spontaneously change its MAC across a reboot, right?

Yes, blocking on the correct bug.

Comment 55 Martin Pitt 2018-03-02 09:23:46 UTC
The two referenced cockpit PRs landed in Cockpit 132 and 134, which have been in RHEL 7.4 for a long time (it has 138 in Base and 160 in Extras).

As bug 1420708 got fixed in RHEL 7 half a year ago, I think this can be closed now.