summaryrefslogtreecommitdiff
path: root/python
diff options
context:
space:
mode:
authorChristian Poessinger <christian@poessinger.com>2020-07-24 17:20:50 +0200
committerChristian Poessinger <christian@poessinger.com>2020-07-25 17:30:12 +0200
commitf81b0443cf09c34cb1f2060094e3eb294b8fa192 (patch)
treeda5c3b85a8fc3cd113ceae3a2f2a8b8c770954e9 /python
parentadd7eaebe7b8ebd4e143eb939d3ba7871ead0502 (diff)
downloadvyos-1x-f81b0443cf09c34cb1f2060094e3eb294b8fa192.tar.gz
vyos-1x-f81b0443cf09c34cb1f2060094e3eb294b8fa192.zip
bonding: ifconfig: T2653: move to get_config_dict()
The current VyOS CLI parser code written in Python contains a ton of duplicates which I can also hold myself accountable for - or maybe mainly me - depends on the angle of judge.
Diffstat (limited to 'python')
-rw-r--r--python/vyos/ifconfig/bond.py118
-rw-r--r--python/vyos/ifconfig/interface.py16
-rw-r--r--python/vyos/validate.py5
3 files changed, 126 insertions, 13 deletions
diff --git a/python/vyos/ifconfig/bond.py b/python/vyos/ifconfig/bond.py
index 47dd4ff34..5a48ac632 100644
--- a/python/vyos/ifconfig/bond.py
+++ b/python/vyos/ifconfig/bond.py
@@ -14,14 +14,15 @@
# License along with this library. If not, see <http://www.gnu.org/licenses/>.
import os
+import jmespath
from vyos.ifconfig.interface import Interface
from vyos.ifconfig.vlan import VLAN
+from vyos.util import cmd
from vyos.validate import assert_list
from vyos.validate import assert_positive
-
@Interface.register
@VLAN.enable
class BondIf(Interface):
@@ -179,7 +180,13 @@ class BondIf(Interface):
>>> BondIf('bond0').get_arp_ip_target()
'192.0.2.1'
"""
- return self.get_interface('bond_arp_ip_target')
+ # As this function might also be called from update() of a VLAN interface
+ # we must check if the bond_arp_ip_target retrieval worked or not - as this
+ # can not be set for a bond vif interface
+ try:
+ return self.get_interface('bond_arp_ip_target')
+ except FileNotFoundError:
+ return ''
def set_arp_ip_target(self, target):
"""
@@ -209,11 +216,31 @@ class BondIf(Interface):
>>> BondIf('bond0').add_port('eth0')
>>> BondIf('bond0').add_port('eth1')
"""
- # An interface can only be added to a bond if it is in 'down' state. If
- # interface is in 'up' state, the following Kernel error will be thrown:
- # bond0: eth1 is up - this may be due to an out of date ifenslave.
- Interface(interface).set_admin_state('down')
- return self.set_interface('bond_add_port', f'+{interface}')
+
+ # From drivers/net/bonding/bond_main.c:
+ # ...
+ # bond_set_slave_link_state(new_slave,
+ # BOND_LINK_UP,
+ # BOND_SLAVE_NOTIFY_NOW);
+ # ...
+ #
+ # The kernel will ALWAYS place new bond members in "up" state regardless
+ # what the CLI will tell us!
+
+ # Physical interface must be in admin down state before they can be
+ # enslaved. If this is not the case an error will be shown:
+ # bond0: eth0 is up - this may be due to an out of date ifenslave
+ slave = Interface(interface)
+ slave_state = slave.get_admin_state()
+ if slave_state == 'up':
+ slave.set_admin_state('down')
+
+ ret = self.set_interface('bond_add_port', f'+{interface}')
+ # The kernel will ALWAYS place new bond members in "up" state regardless
+ # what the LI is configured for - thus we place the interface in its
+ # desired state
+ slave.set_admin_state(slave_state)
+ return ret
def del_port(self, interface):
"""
@@ -277,3 +304,80 @@ class BondIf(Interface):
>>> BondIf('bond0').set_mode('802.3ad')
"""
return self.set_interface('bond_mode', mode)
+
+ def update(self, config):
+ """ General helper function which works on a dictionary retrived by
+ get_config_dict(). It's main intention is to consolidate the scattered
+ interface setup code and provide a single point of entry when workin
+ on any interface. """
+
+ # use ref-counting function to place an interface into admin down state.
+ # set_admin_state_up() must be called the same amount of times else the
+ # interface won't come up. This can/should be used to prevent link flapping
+ # when changing interface parameters require the interface to be down.
+ # We will disable it once before reconfiguration and enable it afterwards.
+ if 'shutdown_required' in config:
+ self.set_admin_state('down')
+
+ # call base class first
+ super().update(config)
+
+ # ARP monitor targets need to be synchronized between sysfs and CLI.
+ # Unfortunately an address can't be send twice to sysfs as this will
+ # result in the following exception: OSError: [Errno 22] Invalid argument.
+ #
+ # We remove ALL addresses prior to adding new ones, this will remove
+ # addresses manually added by the user too - but as we are limited to 16 adresses
+ # from the kernel side this looks valid to me. We won't run into an error
+ # when a user added manual adresses which would result in having more
+ # then 16 adresses in total.
+ arp_tgt_addr = list(map(str, self.get_arp_ip_target().split()))
+ for addr in arp_tgt_addr:
+ self.set_arp_ip_target('-' + addr)
+
+ # Add configured ARP target addresses
+ value = jmespath.search('arp_monitor.target', config)
+ if isinstance(value, str):
+ value = [value]
+ if value:
+ for addr in value:
+ self.set_arp_ip_target('+' + addr)
+
+ # Bonding transmit hash policy
+ value = config.get('hash_policy')
+ if value: self.set_hash_policy(value)
+
+ # Some interface options can only be changed if the interface is
+ # administratively down
+ if self.get_admin_state() == 'down':
+ # Delete bond member port(s)
+ for interface in self.get_slaves():
+ self.del_port(interface)
+
+ # Bonding policy/mode
+ value = config.get('mode')
+ if value: self.set_mode(value)
+
+ # Add (enslave) interfaces to bond
+ value = jmespath.search('member.interface', config)
+ if value:
+ for interface in value:
+ # if we've come here we already verified the interface does
+ # not have an addresses configured so just flush any
+ # remaining ones
+ cmd(f'ip addr flush dev "{interface}"')
+ self.add_port(interface)
+
+ # Primary device interface - must be set after 'mode'
+ value = config.get('primary')
+ if value: self.set_primary(value)
+
+ # Enable/Disable of an interface must always be done at the end of the
+ # derived class to make use of the ref-counting set_admin_state()
+ # function. We will only enable the interface if 'up' was called as
+ # often as 'down'. This is required by some interface implementations
+ # as certain parameters can only be changed when the interface is
+ # in admin-down state. This ensures the link does not flap during
+ # reconfiguration.
+ state = 'down' if 'disable' in config else 'up'
+ self.set_admin_state(state)
diff --git a/python/vyos/ifconfig/interface.py b/python/vyos/ifconfig/interface.py
index 1fe4f74f2..7e887db1b 100644
--- a/python/vyos/ifconfig/interface.py
+++ b/python/vyos/ifconfig/interface.py
@@ -205,6 +205,7 @@ class Interface(Control):
# make sure the ifname is the first argument and not from the dict
self.config['ifname'] = ifname
+ self._admin_state_down_cnt = 0
# we must have updated config before initialising the Interface
super().__init__(**kargs)
@@ -594,7 +595,13 @@ class Interface(Control):
if not int(flags, 16) & 1:
return None
- return self.set_interface('admin_state', state)
+ if state == 'up':
+ self._admin_state_down_cnt -= 1
+ if self._admin_state_down_cnt < 1:
+ return self.set_interface('admin_state', state)
+ else:
+ self._admin_state_down_cnt += 1
+ return self.set_interface('admin_state', state)
def set_proxy_arp(self, enable):
"""
@@ -829,8 +836,11 @@ class Interface(Control):
# There are some items in the configuration which can only be applied
# if this instance is not bound to a bridge. This should be checked
# by the caller but better save then sorry!
- if not config.get('is_bridge_member', False):
- # Bind interface instance into VRF
+ if not any(k in ['is_bond_member', 'is_bridge_member'] for k in config):
+ # Bind interface to given VRF or unbind it if vrf node is not set.
+ # unbinding will call 'ip link set dev eth0 nomaster' which will
+ # also drop the interface out of a bridge or bond - thus this is
+ # checked before
self.set_vrf(config.get('vrf', ''))
# DHCP options
diff --git a/python/vyos/validate.py b/python/vyos/validate.py
index a0620e4dd..ceeb6888a 100644
--- a/python/vyos/validate.py
+++ b/python/vyos/validate.py
@@ -279,7 +279,6 @@ def is_member(conf, interface, intftype=None):
False -> interface type cannot have members
"""
ret_val = None
-
if intftype not in ['bonding', 'bridge', None]:
raise ValueError((
f'unknown interface type "{intftype}" or it cannot '
@@ -292,9 +291,9 @@ def is_member(conf, interface, intftype=None):
conf.set_level([])
for it in intftype:
- base = 'interfaces ' + it
+ base = ['interfaces', it]
for intf in conf.list_nodes(base):
- memberintf = [base, intf, 'member', 'interface']
+ memberintf = base + [intf, 'member', 'interface']
if xml.is_tag(memberintf):
if interface in conf.list_nodes(memberintf):
ret_val = intf