Skip to content

Commit

Permalink
vyos.ifconfig: T7018: drop 'iftype' class attribute (#4280)
Browse files Browse the repository at this point in the history
Under very rare cases we can run into a race condition where interfaces are
still in creation phase but are already referenced..

This can trigger:

  File "/usr/libexec/vyos/conf_mode/system_conntrack.py", line 270, in <module>
    apply(c)
  File "/usr/libexec/vyos/conf_mode/system_conntrack.py", line 249, in apply
    call_dependents()
  File "/usr/lib/python3/dist-packages/vyos/configdep.py", line 147, in call_dependents
    f()
  File "/usr/lib/python3/dist-packages/vyos/configdep.py", line 118, in func_impl
    run_config_mode_script(script, config)
  File "/usr/lib/python3/dist-packages/vyos/configdep.py", line 106, in run_config_mode_script
    mod.verify(c)
  File "/usr/libexec/vyos//conf_mode/service_conntrack-sync.py", line 72, in verify
    if len(get_ipv4(interface)) < 1:
           ^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/vyos/template.py", line 458, in get_ipv4
    return Interface(interface).get_addr_v4()
           ^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib/python3/dist-packages/vyos/ifconfig/interface.py", line 334, in __init__
    if not self.iftype:
    ^^^^^^^^^^^

  AttributeError: 'Interface' object has no attribute 'iftype'

This commit removes the code path in question and the class attribute check.
The reason for the iftype attribute in the past was a common _create() method
serving for all interface types. As we already have a lot of derived
implementations and not all honor the classes iftype/type member - or even
worse honor it only in 50% of the occurrences it's time to drop it.
  • Loading branch information
c-po authored Jan 7, 2025
1 parent fb651c0 commit 08c0054
Show file tree
Hide file tree
Showing 22 changed files with 39 additions and 60 deletions.
4 changes: 3 additions & 1 deletion python/vyos/ifconfig/bond.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ class BondIf(Interface):
monitoring may be performed.
"""

iftype = 'bond'
definition = {
**Interface.definition,
** {
Expand Down Expand Up @@ -109,6 +108,9 @@ def get_inherit_bond_options() -> list:
]
return options

def _create(self):
super()._create('bond')

def remove(self):
"""
Remove interface from operating system. Removing the interface
Expand Down
4 changes: 3 additions & 1 deletion python/vyos/ifconfig/bridge.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ class BridgeIf(Interface):
The Linux bridge code implements a subset of the ANSI/IEEE 802.1d standard.
"""
iftype = 'bridge'
definition = {
**Interface.definition,
**{
Expand Down Expand Up @@ -107,6 +106,9 @@ class BridgeIf(Interface):
},
}}

def _create(self):
super()._create('bridge')

def get_vlan_filter(self):
"""
Get the status of the bridge VLAN filter
Expand Down
5 changes: 3 additions & 2 deletions python/vyos/ifconfig/dummy.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,13 @@ class DummyIf(Interface):
interface. The purpose of a dummy interface is to provide a device to route
packets through without actually transmitting them.
"""

iftype = 'dummy'
definition = {
**Interface.definition,
**{
'section': 'dummy',
'prefixes': ['dum', ],
},
}

def _create(self):
super()._create('dummy')
6 changes: 4 additions & 2 deletions python/vyos/ifconfig/ethernet.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ class EthernetIf(Interface):
Abstraction of a Linux Ethernet Interface
"""

iftype = 'ethernet'
definition = {
**Interface.definition,
**{
Expand Down Expand Up @@ -119,6 +118,9 @@ def __init__(self, ifname, **kargs):
super().__init__(ifname, **kargs)
self.ethtool = Ethtool(ifname)

def _create(self):
pass

def remove(self):
"""
Remove interface from config. Removing the interface deconfigures all
Expand All @@ -137,7 +139,7 @@ def remove(self):
# Remove all VLAN subinterfaces - filter with the VLAN dot
for vlan in [
x
for x in Section.interfaces(self.iftype)
for x in Section.interfaces('ethernet')
if x.startswith(f'{self.ifname}.')
]:
Interface(vlan).remove()
Expand Down
3 changes: 1 addition & 2 deletions python/vyos/ifconfig/geneve.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class GeneveIf(Interface):
https://developers.redhat.com/blog/2019/05/17/an-introduction-to-linux-virtual-interfaces-tunnels/#geneve
https://lwn.net/Articles/644938/
"""
iftype = 'geneve'
definition = {
**Interface.definition,
**{
Expand All @@ -49,7 +48,7 @@ def _create(self):
'parameters.ipv6.flowlabel' : 'flowlabel',
}

cmd = 'ip link add name {ifname} type {type} id {vni} remote {remote}'
cmd = 'ip link add name {ifname} type geneve id {vni} remote {remote}'
for vyos_key, iproute2_key in mapping.items():
# dict_search will return an empty dict "{}" for valueless nodes like
# "parameters.nolearning" - thus we need to test the nodes existence
Expand Down
5 changes: 3 additions & 2 deletions python/vyos/ifconfig/input.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,13 @@ class InputIf(Interface):
a single stack of qdiscs, classes and filters can be shared between
multiple interfaces.
"""

iftype = 'ifb'
definition = {
**Interface.definition,
**{
'section': 'input',
'prefixes': ['ifb', ],
},
}

def _create(self):
super()._create('ifb')
23 changes: 4 additions & 19 deletions python/vyos/ifconfig/interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
from netaddr import EUI
from netaddr import mac_unix_expanded

from vyos.base import ConfigError
from vyos.configdict import list_diff
from vyos.configdict import dict_merge
from vyos.configdict import get_vlan_ids
Expand Down Expand Up @@ -74,7 +73,6 @@ class Interface(Control):
OperationalClass = Operational

options = ['debug', 'create']
required = []
default = {
'debug': True,
'create': True,
Expand Down Expand Up @@ -336,22 +334,10 @@ def __init__(self, ifname, **kargs):
super().__init__(**kargs)

if not self.exists(ifname):
# Any instance of Interface, such as Interface('eth0') can be used
# safely to access the generic function in this class as 'type' is
# unset, the class can not be created
if not hasattr(self, 'iftype'):
raise ConfigError(f'Interface "{ifname}" has no "iftype" attribute defined!')
self.config['type'] = self.iftype

# Should an Instance of a child class (EthernetIf, DummyIf, ..)
# be required, then create should be set to False to not accidentally create it.
# In case a subclass does not define it, we use get to set the default to True
if self.config.get('create',True):
for k in self.required:
if k not in kargs:
name = self.default['type']
raise ConfigError(f'missing required option {k} for {name} {ifname} creation')

if self.config.get('create', True):
self._create()
# If we can not connect to the interface then let the caller know
# as the class could not be correctly initialised
Expand All @@ -364,13 +350,14 @@ def __init__(self, ifname, **kargs):
self.operational = self.OperationalClass(ifname)
self.vrrp = VRRP(ifname)

def _create(self):
def _create(self, type: str=''):
# Do not create interface that already exist or exists in netns
netns = self.config.get('netns', None)
if self.exists(f'{self.ifname}', netns=netns):
return

cmd = 'ip link add dev {ifname} type {type}'.format(**self.config)
cmd = f'ip link add dev {self.ifname}'
if type: cmd += f' type {type}'
if 'netns' in self.config: cmd = f'ip netns exec {netns} {cmd}'
self._cmd(cmd)

Expand Down Expand Up @@ -1954,8 +1941,6 @@ def update(self, config):

class VLANIf(Interface):
""" Specific class which abstracts 802.1q and 802.1ad (Q-in-Q) VLAN interfaces """
iftype = 'vlan'

def _create(self):
# bail out early if interface already exists
if self.exists(f'{self.ifname}'):
Expand Down
1 change: 0 additions & 1 deletion python/vyos/ifconfig/l2tpv3.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ class L2TPv3If(Interface):
either hot standby or load balancing services. Additionally, link integrity
monitoring may be performed.
"""
iftype = 'l2tp'
definition = {
**Interface.definition,
**{
Expand Down
6 changes: 5 additions & 1 deletion python/vyos/ifconfig/loopback.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,20 @@ class LoopbackIf(Interface):
uses to communicate with itself.
"""
_persistent_addresses = ['127.0.0.1/8', '::1/128']
iftype = 'loopback'
definition = {
**Interface.definition,
**{
'section': 'loopback',
'prefixes': ['lo', ],
'bridgeable': True,
'eternal': 'lo$',
}
}

def _create(self):
# we can not create this interface as it is managed by the Kernel
pass

def remove(self):
"""
Loopback interface can not be deleted from operating system. We can
Expand Down
3 changes: 1 addition & 2 deletions python/vyos/ifconfig/macsec.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class MACsecIf(Interface):
other security solutions such as IPsec (layer 3) or TLS (layer 4), as all
those solutions are used for their own specific use cases.
"""
iftype = 'macsec'
definition = {
**Interface.definition,
**{
Expand All @@ -43,7 +42,7 @@ def _create(self):
"""

# create tunnel interface
cmd = 'ip link add link {source_interface} {ifname} type {type}'.format(**self.config)
cmd = 'ip link add link {source_interface} {ifname} type macsec'.format(**self.config)
cmd += f' cipher {self.config["security"]["cipher"]}'

if 'encrypt' in self.config["security"]:
Expand Down
5 changes: 2 additions & 3 deletions python/vyos/ifconfig/macvlan.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ class MACVLANIf(Interface):
"""
Abstraction of a Linux MACvlan interface
"""
iftype = 'macvlan'
definition = {
**Interface.definition,
**{
Expand All @@ -35,12 +34,12 @@ def _create(self):
down by default.
"""
# please do not change the order when assembling the command
cmd = 'ip link add {ifname} link {source_interface} type {type} mode {mode}'
cmd = 'ip link add {ifname} link {source_interface} type macvlan mode {mode}'
self._cmd(cmd.format(**self.config))

# interface is always A/D down. It needs to be enabled explicitly
self.set_admin_state('down')

def set_mode(self, mode):
cmd = f'ip link set dev {self.ifname} type {self.iftype} mode {mode}'
cmd = f'ip link set dev {self.ifname} type macvlan mode {mode}'
return self._cmd(cmd)
1 change: 0 additions & 1 deletion python/vyos/ifconfig/pppoe.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

@Interface.register
class PPPoEIf(Interface):
iftype = 'pppoe'
definition = {
**Interface.definition,
**{
Expand Down
1 change: 0 additions & 1 deletion python/vyos/ifconfig/sstpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

@Interface.register
class SSTPCIf(Interface):
iftype = 'sstpc'
definition = {
**Interface.definition,
**{
Expand Down
9 changes: 4 additions & 5 deletions python/vyos/ifconfig/tunnel.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,9 +90,8 @@ def __init__(self, ifname, **kargs):
# T3357: we do not have the 'encapsulation' in kargs when calling this
# class from op-mode like "show interfaces tunnel"
if 'encapsulation' in kargs:
self.iftype = kargs['encapsulation']
# The gretap interface has the possibility to act as L2 bridge
if self.iftype in ['gretap', 'ip6gretap']:
if kargs['encapsulation'] in ['gretap', 'ip6gretap']:
# no multicast, ttl or tos for gretap
self.definition = {
**TunnelIf.definition,
Expand All @@ -110,10 +109,10 @@ def _create(self):
mapping = { **self.mapping, **self.mapping_ipv4 }

cmd = 'ip tunnel add {ifname} mode {encapsulation}'
if self.iftype in ['gretap', 'ip6gretap', 'erspan', 'ip6erspan']:
if self.config['encapsulation'] in ['gretap', 'ip6gretap', 'erspan', 'ip6erspan']:
cmd = 'ip link add name {ifname} type {encapsulation}'
# ERSPAN requires the serialisation of packets
if self.iftype in ['erspan', 'ip6erspan']:
if self.config['encapsulation'] in ['erspan', 'ip6erspan']:
cmd += ' seq'

for vyos_key, iproute2_key in mapping.items():
Expand All @@ -132,7 +131,7 @@ def _create(self):

def _change_options(self):
# gretap interfaces do not support changing any parameter
if self.iftype in ['gretap', 'ip6gretap', 'erspan', 'ip6erspan']:
if self.config['encapsulation'] in ['gretap', 'ip6gretap', 'erspan', 'ip6erspan']:
return

if self.config['encapsulation'] in ['ipip6', 'ip6ip6', 'ip6gre']:
Expand Down
3 changes: 1 addition & 2 deletions python/vyos/ifconfig/veth.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ class VethIf(Interface):
"""
Abstraction of a Linux veth interface
"""
iftype = 'veth'
definition = {
**Interface.definition,
**{
Expand All @@ -46,7 +45,7 @@ def _create(self):
return

# create virtual-ethernet interface
cmd = 'ip link add {ifname} type {type}'.format(**self.config)
cmd = f'ip link add {self.ifname} type veth'
cmd += f' peer name {self.config["peer_name"]}'
self._cmd(cmd)

Expand Down
3 changes: 0 additions & 3 deletions python/vyos/ifconfig/vrrp.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,15 +26,12 @@
from vyos.utils.file import wait_for_file_write_complete
from vyos.utils.process import process_running


class VRRPError(Exception):
pass


class VRRPNoData(VRRPError):
pass


class VRRP(object):
_vrrp_prefix = '00:00:5E:00:01:'
location = {
Expand Down
1 change: 0 additions & 1 deletion python/vyos/ifconfig/vti.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@

@Interface.register
class VTIIf(Interface):
iftype = 'vti'
definition = {
**Interface.definition,
**{
Expand Down
1 change: 0 additions & 1 deletion python/vyos/ifconfig/vtun.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

@Interface.register
class VTunIf(Interface):
iftype = 'vtun'
definition = {
**Interface.definition,
**{
Expand Down
4 changes: 1 addition & 3 deletions python/vyos/ifconfig/vxlan.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,6 @@ class VXLANIf(Interface):
For more information please refer to:
https://www.kernel.org/doc/Documentation/networking/vxlan.txt
"""

iftype = 'vxlan'
definition = {
**Interface.definition,
**{
Expand Down Expand Up @@ -94,7 +92,7 @@ def _create(self):
remote_list = self.config['remote'][1:]
self.config['remote'] = self.config['remote'][0]

cmd = 'ip link add {ifname} type {type} dstport {port}'
cmd = 'ip link add {ifname} type vxlan dstport {port}'
for vyos_key, iproute2_key in mapping.items():
# dict_search will return an empty dict "{}" for valueless nodes like
# "parameters.nolearning" - thus we need to test the nodes existence
Expand Down
9 changes: 4 additions & 5 deletions python/vyos/ifconfig/wireguard.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
from vyos.ifconfig import Operational
from vyos.template import is_ipv6


class WireGuardOperational(Operational):
def _dump(self):
"""Dump wireguard data in a python friendly way."""
Expand Down Expand Up @@ -160,18 +159,18 @@ def show_interface(self):
@Interface.register
class WireGuardIf(Interface):
OperationalClass = WireGuardOperational
iftype = 'wireguard'
definition = {
**Interface.definition,
**{
'section': 'wireguard',
'prefixes': [
'wg',
],
'prefixes': ['wg', ],
'bridgeable': False,
},
}

def _create(self):
super()._create('wireguard')

def get_mac(self):
"""Get a synthetic MAC address."""
return self.get_mac_synthetic()
Expand Down
Loading

0 comments on commit 08c0054

Please sign in to comment.