vxlan-group/vxlan-mcastgrp and vxlan-physdev changes are not supported on
vxlan devices in UP state. We need to down, change the vxlan attributes
and up the vxlan again.
Ticket: CM-24987
Reviewed-By: Roopa - CCR-8693
Signed-off-by: Julien Fortin <[email protected]>
Following commit: 865cf3fcbc (see commit message/explanation
bellow). We thought about a better way to make sure the cache stays synced without dropping
notifications.
The fix is explained in the code with the following comment:
After sending a no master request (IFLA_MASTER=0) the kernels send
2 or 3 notifications (with IFLA_MASTER) before sending the final
notification where IFLA_MASTER is removed. For performance purposes
we don't wait for those notifications, we simply update the cache
to reflect the change (if we got an ACK on the nomaster request).
Those extra notification re-add the former slave to it's master
(in our internal data-structures at least). ifupdown2 relies on
the cache to get accurate information, this puts the cache in an
unreliable state. We can detected this bad state and avoid it. Afer
a nomaster request we "register" the device as "nomaster", meaning
that we will manually remove the IFLA_MASTER attribute from any
subsequent packet, until the final packet arrives - then unregister
the device from the nomasterq.
Ticket: CM-24705
Reviewed By: Roopa
commit 865cf3fcbc
Author: Julien Fortin <[email protected]>
Date: Thu May 9 14:51:59 2019 +0800
addons: bond: query cache for bond slaves before running ops
When un-slaving a device (ip link set dev X nomaster) the kernel
sends various notifications before sending the final notification
where the IFLA_MASTER flag is removed.
For optimization purposes once the nomaster operation is confirmed
by the kernel (ACK packet) we manually update our cache to reflect
the change without having to wait for the kernel notification. For
some reason the kernel sends additional notifications (with IFLA_
MASTER flag still set) before sending the final notification packet
which doesn't have this flag.
Our internal cache is processing those notifications and populates
its internal data-structure, such as the master-slaves dictionary.
The cache will eventually reflect the correct kernel state after
receiving and processing all the notifications.
The issue here was that we were querying the cache for the bond
slaves almost immediately after the nomaster requests. Most of the
time everything worked fine, but it is possible to receive one or
more "in between notifications" (with IFLA_MASTER flag) thus re-
adding those slaves to our master-slaves dictionary and returning
an invalid slave list to the user.
In my opinion the ideal fix here would be to simply ignore those
"in between notifications" until we receive the final notifications
without the IFLA_MASTER flag. Today we are using a similar mechanism
for link_del RTM_DELLINK and "in between" RTM_NEWLINK notifications
Dropping netlink packet notifications might introduce side-effects
and potential errors in the future. Those failure would be very hard
to track and debug, so today's fix is a workaround.
Signed-off-by: Julien Fortin <[email protected]>
When un-slaving a device (ip link set dev X nomaster) the kernel
sends various notifications before sending the final notification
where the IFLA_MASTER flag is removed.
For optimization purposes once the nomaster operation is confirmed
by the kernel (ACK packet) we manually update our cache to reflect
the change without having to wait for the kernel notification. For
some reason the kernel sends additional notifications (with IFLA_
MASTER flag still set) before sending the final notification packet
which doesn't have this flag.
Our internal cache is processing those notifications and populates
its internal data-structure, such as the master-slaves dictionary.
The cache will eventually reflect the correct kernel state after
receiving and processing all the notifications.
The issue here was that we were querying the cache for the bond
slaves almost immediately after the nomaster requests. Most of the
time everything worked fine, but it is possible to receive one or
more "in between notifications" (with IFLA_MASTER flag) thus re-
adding those slaves to our master-slaves dictionary and returning
an invalid slave list to the user.
In my opinion the ideal fix here would be to simply ignore those
"in between notifications" until we receive the final notifications
without the IFLA_MASTER flag. Today we are using a similar mechanism
for link_del RTM_DELLINK and "in between" RTM_NEWLINK notifications
Dropping netlink packet notifications might introduce side-effects
and potential errors in the future. Those failure would be very hard
to track and debug, so today's fix is a workaround.
Ticket: CM-24705
Reviewed By: Roopa
Testing Done:
To reproduce the issue, run the following script (bond_loop.sh) on
a switch/VX. On local VX running in VirtualBox the failure would
show after about ~200 loop iterations. The script stores ifreload
logs to make debugging easier. Please read-through this script
before running it.
$ cat bond_loop.sh
function pause(){
read -p "$*"
}
for index in `seq 1 10000`
do
echo "Run #$index"
echo """auto lo
iface lo inet loopback
auto eth0
iface eth0 inet dhcp
auto swp1
iface swp1
auto bond0
iface bond0
bond-slaves swp1
bond-mode balance-xor""" > /etc/network/interfaces
ifreload -av &> /tmp/ifreload-$index-A.log
if ! ifquery -ac &> /tmp/ifquery-$index-A.log; then
echo "[A:$index] === FAIL: ifquery returns: $?"
cat /tmp/ifquery-$index-A.log
pause 'Press [Enter] key to continue...'
fi
############################################
sed -i '$d' /etc/network/interfaces ;
ifreload -av &> /tmp/ifreload-$index-B.log
if ! ifquery -ac &> /tmp/ifquery-$index-B.log; then
echo "[B:$index] === FAIL: ifquery returns: $?"
cat /tmp/ifquery-$index-B.log
pause 'Press [Enter] key to continue...'
fi
done
Signed-off-by: Julien Fortin <[email protected]>
We disabled DRY-RUN mode because it was introducing huge memory leaks. The
cause of this issue was that we kept strong references on objects/modules
and methods in our internal data-structure.
To solve this problem we need to move all the existing dry-run handling code
to use weakrefs. Then another problem emerged ; you can't use the weakref
module on bound methods. To solve this last issue we had to get some inspira
tion from ActiveState recipe 81253, implementing customs class to hold
weakrefs on bound methods but using a ingenious trick to call the dry-run
method when needed.
Signed-off-by: Julien Fortin <[email protected]>
* dev:
addons: protodown vrrp macvlans only when created
addons: address: rolling back address configuration to use string instead of IPNetwork objects
Not when we are reloading and applying config ot existing macvlans -
somebody may be using them, we don't want to protodown them in that
case.
Signed-off-by: Quentin Young <[email protected]>
Not when we are reloading and applying config ot existing macvlans -
somebody may be using them, we don't want to protodown them in that
case.
Signed-off-by: Quentin Young <[email protected]>
$ ifquery vlan100
auto vlan100
iface vlan100
address 2001:aa::2/64
address 2001:aa::150/64
vlan-id 100
vlan-raw-device bridge
$
when commenting the second ipv6 address and doing ifreload -a, the commented out ipv6 wasn't
removed by ifupdown2. The address module compares IPNetwork objects tries to detect changes.
Unfortunately this change wasn't caught because the two object are matching.
IPNetwork("2001:aa::2/64") == IPNetwork("2001:aa::150/64"): True
To avoid this we need to go back to string comparaison. In the future we should do int checks
like we do today for mac addresses.
Signed-off-by: Julien Fortin <[email protected]>
$ ifquery vlan100
auto vlan100
iface vlan100
address 2001:aa::2/64
address 2001:aa::150/64
vlan-id 100
vlan-raw-device bridge
$
when commenting the second ipv6 address and doing ifreload -a, the commented out ipv6 wasn't
removed by ifupdown2. The address module compares IPNetwork objects tries to detect changes.
Unfortunately this change wasn't caught because the two object are matching.
IPNetwork("2001:aa::2/64") == IPNetwork("2001:aa::150/64"): True
To avoid this we need to go back to string comparaison. In the future we should do int checks
like we do today for mac addresses.
Signed-off-by: Julien Fortin <[email protected]>
The internal addresses data structure "self._addr_cache" was updated to a new format:
{
ifname: {
4: [ipv4, ...],
6: [ipv6, ...],
}
}
Unfortunately force_remove_addr wasn't updated to support this new format hence it
was constantly and silently failing.
Signed-off-by: Julien Fortin <[email protected]>
The following config is valid but `ifquery -a` outputs an invalid config.
This patch prevents ifquery -a to dump an empty attribute. We are just fixing
the dump code here because the address module allow the following attributes
to have empty values: broadcast, scope, preferred-lifetime and pointopoint
$ cat /etc/network/interfaces
auto lo
iface lo inet loopback
auto eth0
iface eth0 inet dhcp
auto swp1
iface swp1
address 71.12.21.48
address 2001:620:5ca1:160::57
preferred-lifetime 30
$
$
$ ifquery -a
auto lo
iface lo inet loopback
auto eth0
iface eth0 inet dhcp
auto swp1
iface swp1
address 71.12.21.48
address 2001:620:5ca1:160::57
preferred-lifetime
preferred-lifetime 30
$
$
Signed-off-by: Julien Fortin <[email protected]>
The vxlan module is due for a big refactoring. The current code is
very messy and hard to get around. Now each attribute has its own
subfunction to get the user config and/or policy value then translate
it to a netlink value saved in a dictionary that we inject in our
VXLAN add/set request.
Ticket: CM-24735
Reviewed-by: CCR-8593
Signed-off-by: Julien Fortin <[email protected]>
* upstream/dev: (21 commits)
addons: address: fix same subnet address ignored issue
debian: ifupdown2.postinst temporarily disabling ifupdown2d
debian: changelog: update 2.0.0-cl4u0 entry
addons: bridge: fix 'NoneType' object has no attribute '__getitem__' error
debian: ifupdown2.service: start ifupdown2d after syslog.service
debian: changelog: update 2.0.0-cl4u0 entry
debian: changelog: new "1.2.6-cl3u25" entry
nlcache: add new internatl cache data-strucutre: _slaves_master dictionary
Recommit "ifupdown2: Update FEC check for new values in ethtool 4.19"
Revert "ifupdown2: Update FEC check for new values in ethtool 4.19"
ifupdown2: Update FEC check for new values in ethtool 4.19
ifupdown2d: run: add signal handler for SIGTERM, INT, QUIT
nlmanager: nlpacket: AttributeIPAddress: decode IPAddress without prefixlen attribute
addons: bridge: change bridge-vxlan-arp-nd-suppress policy default to off
addons: bridge: brport vxlan learning controlled by new policy: bridge_vxlan_port_learning
addons: bridge: set bridge-arp-nd-suppress on by default on vxlan interfaces
addons: bridge: set bridge-arp-nd-suppress on by default on vxlan interfaces
addons: dhcp|address: on dhcp release flush internal address cache
addons: address: if hwaddress changed delete old fdb entry
addons: vxlan: convert anycastip to IPAddress object
...
Julien Fortin
Quentin Young
Roopa Prabhu