refactor: indirect interface_names to interfaces

Before, interface_names was a literal copy of the VPPMessage() from
sw_interface_details, so interfaces and interface_names kept the
messages twice. This change makes interface_names a pointer to the index
on interfaces.

- Update the cache creation to make the indirection from interface_names
  to interfaces
- Introduce get_interface_by_name()
- Update/fix all the call sites

Tested:
- All unit tests and yamltests pass before and after this change
- The hippo integration test passes before and after this change
This commit is contained in:
Pim van Pelt
2022-12-03 12:14:07 +00:00
parent 806f4f4fc8
commit a622b1d54e
2 changed files with 87 additions and 60 deletions

View File

@ -126,7 +126,14 @@ class Reconciler:
"""Remove all addresses from interface ifname, except those in address_list, """Remove all addresses from interface ifname, except those in address_list,
which may be an empty list, in which case all addresses are removed. which may be an empty list, in which case all addresses are removed.
""" """
idx = self.vpp.cache["interface_names"][ifname].sw_if_index _iface = self.vpp.get_interface_by_name(ifname)
if not _iface:
self.logger.error(
f"Trying to prune addresses from non-existent interface {ifname}"
)
return
idx = _iface.sw_if_index
removed_addresses = [] removed_addresses = []
for addr in self.vpp.cache["interface_addresses"][idx]: for addr in self.vpp.cache["interface_addresses"][idx]:
if not addr in address_list: if not addr in address_list:
@ -280,11 +287,11 @@ class Reconciler:
Returns False if they are identical.""" Returns False if they are identical."""
if not ifname in self.vpp.cache["interface_names"]: vpp_iface = self.vpp.get_interface_by_name(ifname)
return True if (
vpp_iface = self.vpp.cache["interface_names"][ifname] not vpp_iface
or vpp_iface.sw_if_index not in self.vpp.cache["vxlan_tunnels"]
if vpp_iface.sw_if_index not in self.vpp.cache["vxlan_tunnels"]: ):
return True return True
vpp_vxlan = self.vpp.cache["vxlan_tunnels"][vpp_iface.sw_if_index] vpp_vxlan = self.vpp.cache["vxlan_tunnels"][vpp_iface.sw_if_index]
@ -306,10 +313,11 @@ class Reconciler:
Returns False if the TAP is a Linux Control Plane LIP. Returns False if the TAP is a Linux Control Plane LIP.
Returns False if they are identical.""" Returns False if they are identical."""
if not ifname in self.vpp.cache["interface_names"]:
return True vpp_iface = self.vpp.get_interface_by_name(ifname)
vpp_iface = self.vpp.cache["interface_names"][ifname]
vpp_tap = self.vpp.cache["taps"][vpp_iface.sw_if_index] vpp_tap = self.vpp.cache["taps"][vpp_iface.sw_if_index]
if not vpp_iface:
return True
_config_ifname, config_iface = tap.get_by_name(self.cfg, ifname) _config_ifname, config_iface = tap.get_by_name(self.cfg, ifname)
if not config_iface: if not config_iface:
@ -351,11 +359,12 @@ class Reconciler:
Returns False if they are identical. Returns False if they are identical.
""" """
if not ifname in self.vpp.cache["interface_names"]:
return True
vpp_iface = self.vpp.cache["interface_names"][ifname] vpp_iface = self.vpp.get_interface_by_name(ifname)
if not vpp_iface.sw_if_index in self.vpp.cache["bondethernets"]: if (
not vpp_iface
or not vpp_iface.sw_if_index in self.vpp.cache["bondethernets"]
):
return True return True
config_ifname, config_iface = bondethernet.get_by_name(self.cfg, ifname) config_ifname, config_iface = bondethernet.get_by_name(self.cfg, ifname)
@ -387,7 +396,6 @@ class Reconciler:
if self.__tap_has_diff(vpp_ifname): if self.__tap_has_diff(vpp_ifname):
removed_taps.append(vpp_ifname) removed_taps.append(vpp_ifname)
continue continue
self.logger.debug(f"TAP OK: {vpp_ifname}")
for ifname in removed_taps: for ifname in removed_taps:
cli = f"delete tap {ifname}" cli = f"delete tap {ifname}"
@ -478,8 +486,8 @@ class Reconciler:
removed_interfaces = [] removed_interfaces = []
for numtags in [2, 1]: for numtags in [2, 1]:
for vpp_ifname in self.vpp.get_sub_interfaces(): for vpp_ifname in self.vpp.get_sub_interfaces():
vpp_iface = self.vpp.cache["interface_names"][vpp_ifname] vpp_iface = self.vpp.get_interface_by_name(vpp_ifname)
if vpp_iface.sub_number_of_tags != numtags: if not vpp_iface or vpp_iface.sub_number_of_tags != numtags:
continue continue
if self.vpp.tap_is_lcp(vpp_ifname): if self.vpp.tap_is_lcp(vpp_ifname):
@ -528,7 +536,10 @@ class Reconciler:
def __prune_phys(self): def __prune_phys(self):
"""Set default MTU and remove IPs for PHYs that are not in the config.""" """Set default MTU and remove IPs for PHYs that are not in the config."""
for vpp_ifname in self.vpp.get_phys(): for vpp_ifname in self.vpp.get_phys():
vpp_iface = self.vpp.cache["interface_names"][vpp_ifname] vpp_iface = self.vpp.get_interface_by_name(vpp_ifname)
if not vpp_iface:
continue
_config_ifname, config_iface = interface.get_by_name(self.cfg, vpp_ifname) _config_ifname, config_iface = interface.get_by_name(self.cfg, vpp_ifname)
if not config_iface: if not config_iface:
## Interfaces were sent DOWN in the __prune_admin_state() step previously ## Interfaces were sent DOWN in the __prune_admin_state() step previously
@ -711,7 +722,9 @@ class Reconciler:
if not ifname in interface.get_interfaces( if not ifname in interface.get_interfaces(
self.cfg self.cfg
) + loopback.get_loopbacks(self.cfg): ) + loopback.get_loopbacks(self.cfg):
vpp_iface = self.vpp.cache["interface_names"][ifname] vpp_iface = self.vpp.get_interface_by_name(ifname)
if not vpp_iface:
continue
if self.vpp.tap_is_lcp(ifname): if self.vpp.tap_is_lcp(ifname):
continue continue
@ -944,7 +957,10 @@ class Reconciler:
if not ifname in self.vpp.cache["interface_names"]: if not ifname in self.vpp.cache["interface_names"]:
## New loopback ## New loopback
continue continue
vpp_iface = self.vpp.cache["interface_names"][ifname] vpp_iface = self.vpp.get_interface_by_name(ifname)
if not vpp_iface:
continue
config_ifname, config_iface = loopback.get_by_name(self.cfg, ifname) config_ifname, config_iface = loopback.get_by_name(self.cfg, ifname)
if "mac" in config_iface and config_iface["mac"] != str( if "mac" in config_iface and config_iface["mac"] != str(
vpp_iface.l2_address vpp_iface.l2_address
@ -959,7 +975,10 @@ class Reconciler:
if not ifname in self.vpp.cache["interface_names"]: if not ifname in self.vpp.cache["interface_names"]:
## New interface ## New interface
continue continue
vpp_iface = self.vpp.cache["interface_names"][ifname] vpp_iface = self.vpp.get_interface_by_name(ifname)
if not vpp_iface:
continue
config_ifname, config_iface = interface.get_by_name(self.cfg, ifname) config_ifname, config_iface = interface.get_by_name(self.cfg, ifname)
if "mac" in config_iface and config_iface["mac"] != str( if "mac" in config_iface and config_iface["mac"] != str(
vpp_iface.l2_address vpp_iface.l2_address
@ -971,8 +990,8 @@ class Reconciler:
def __sync_bondethernets(self): def __sync_bondethernets(self):
"""Synchronize the VPP Dataplane configuration for bondethernets""" """Synchronize the VPP Dataplane configuration for bondethernets"""
for ifname in bondethernet.get_bondethernets(self.cfg): for ifname in bondethernet.get_bondethernets(self.cfg):
if ifname in self.vpp.cache["interface_names"]: vpp_iface = self.vpp.get_interface_by_name(ifname)
vpp_iface = self.vpp.cache["interface_names"][ifname] if vpp_iface:
vpp_members = [ vpp_members = [
self.vpp.cache["interfaces"][x].interface_name self.vpp.cache["interfaces"][x].interface_name
for x in self.vpp.cache["bondethernet_members"][ for x in self.vpp.cache["bondethernet_members"][
@ -981,7 +1000,6 @@ class Reconciler:
] ]
else: else:
## New BondEthernet ## New BondEthernet
vpp_iface = None
vpp_members = [] vpp_members = []
config_bond_ifname, config_bond_iface = bondethernet.get_by_name( config_bond_ifname, config_bond_iface = bondethernet.get_by_name(
@ -995,8 +1013,8 @@ class Reconciler:
member_ifname, member_iface = interface.get_by_name( member_ifname, member_iface = interface.get_by_name(
self.cfg, member_ifname self.cfg, member_ifname
) )
member_iface = self.vpp.cache["interface_names"][member_ifname] member_iface = self.vpp.get_interface_by_name(member_ifname)
if not member_ifname in vpp_members: if not member_iface or member_ifname not in vpp_members:
if len(vpp_members) == 0: if len(vpp_members) == 0:
bondmac = member_iface.l2_address bondmac = member_iface.l2_address
cli = f"bond add {config_bond_ifname} {member_iface.interface_name}" cli = f"bond add {config_bond_ifname} {member_iface.interface_name}"
@ -1084,11 +1102,8 @@ class Reconciler:
if "bvi" in config_bridge_iface: if "bvi" in config_bridge_iface:
bviname = config_bridge_iface["bvi"] bviname = config_bridge_iface["bvi"]
if not ( bvi_iface = self.vpp.get_interface_by_name(bviname)
bviname in self.vpp.cache["interface_names"] if not bvi_iface or bvi_iface.sw_if_index != bvi_sw_if_index:
and self.vpp.cache["interface_names"][bviname].sw_if_index
== bvi_sw_if_index
):
cli = f"set interface l2 bridge {bviname} {int(instance)} bvi" cli = f"set interface l2 bridge {bviname} {int(instance)} bvi"
self.cli["sync"].append(cli) self.cli["sync"].append(cli)
@ -1118,12 +1133,8 @@ class Reconciler:
config_tx_ifname, _config_tx_iface = interface.get_by_name( config_tx_ifname, _config_tx_iface = interface.get_by_name(
self.cfg, config_rx_iface["l2xc"] self.cfg, config_rx_iface["l2xc"]
) )
vpp_rx_iface = None vpp_rx_iface = self.vpp.get_interface_by_name(config_rx_ifname)
vpp_tx_iface = None vpp_tx_iface = self.vpp.get_interface_by_name(config_tx_ifname)
if config_rx_ifname in self.vpp.cache["interface_names"]:
vpp_rx_iface = self.vpp.cache["interface_names"][config_rx_ifname]
if config_tx_ifname in self.vpp.cache["interface_names"]:
vpp_tx_iface = self.vpp.cache["interface_names"][config_tx_ifname]
l2xc_changed = False l2xc_changed = False
if not vpp_rx_iface or not vpp_tx_iface: if not vpp_rx_iface or not vpp_tx_iface:
@ -1173,16 +1184,18 @@ class Reconciler:
config_mtu = 1500 config_mtu = 1500
vpp_mtu = 9000 vpp_mtu = 9000
if ifname.startswith("loop"): if ifname.startswith("loop"):
if ifname in self.vpp.cache["interface_names"]: _iface = self.vpp.get_interface_by_name(ifname)
vpp_mtu = self.vpp.cache["interface_names"][ifname].mtu[0] if _iface:
vpp_mtu = _iface.mtu[0]
vpp_ifname, config_iface = loopback.get_by_name(self.cfg, ifname) vpp_ifname, config_iface = loopback.get_by_name(self.cfg, ifname)
if "mtu" in config_iface: if "mtu" in config_iface:
config_mtu = config_iface["mtu"] config_mtu = config_iface["mtu"]
else: else:
if numtags > 0: if numtags > 0:
vpp_mtu = 0 vpp_mtu = 0
if ifname in self.vpp.cache["interface_names"]: _iface = self.vpp.get_interface_by_name(ifname)
vpp_mtu = self.vpp.cache["interface_names"][ifname].mtu[0] if _iface:
vpp_mtu = _iface.mtu[0]
vpp_ifname, config_iface = interface.get_by_name(self.cfg, ifname) vpp_ifname, config_iface = interface.get_by_name(self.cfg, ifname)
config_mtu = interface.get_mtu(self.cfg, ifname) config_mtu = interface.get_mtu(self.cfg, ifname)
@ -1289,11 +1302,13 @@ class Reconciler:
if "addresses" in config_iface: if "addresses" in config_iface:
config_addresses = config_iface["addresses"] config_addresses = config_iface["addresses"]
if vpp_ifname in self.vpp.cache["interface_names"]: if vpp_ifname in self.vpp.cache["interface_names"]:
sw_if_index = self.vpp.cache["interface_names"][vpp_ifname].sw_if_index _iface = self.vpp.get_interface_by_name(vpp_ifname)
if sw_if_index in self.vpp.cache["interface_addresses"]: if _iface.sw_if_index in self.vpp.cache["interface_addresses"]:
vpp_addresses = [ vpp_addresses = [
str(x) str(x)
for x in self.vpp.cache["interface_addresses"][sw_if_index] for x in self.vpp.cache["interface_addresses"][
_iface.sw_if_index
]
] ]
for addr in config_addresses: for addr in config_addresses:
if addr in vpp_addresses: if addr in vpp_addresses:
@ -1315,10 +1330,10 @@ class Reconciler:
config_admin_state = interface.get_admin_state(self.cfg, ifname) config_admin_state = interface.get_admin_state(self.cfg, ifname)
vpp_admin_state = 0 vpp_admin_state = 0
if vpp_ifname in self.vpp.cache["interface_names"]: _iface = self.vpp.get_interface_by_name(vpp_ifname)
vpp_admin_state = ( if _iface:
self.vpp.cache["interface_names"][vpp_ifname].flags & 1 vpp_admin_state = _iface.flags & 1 # IF_STATUS_API_FLAG_ADMIN_UP
) # IF_STATUS_API_FLAG_ADMIN_UP
if config_admin_state == vpp_admin_state: if config_admin_state == vpp_admin_state:
continue continue
state = "up" state = "up"

View File

@ -128,13 +128,14 @@ class VPPApi:
def cache_remove_bondethernet_member(self, ifname): def cache_remove_bondethernet_member(self, ifname):
"""Removes the bonderthernet member interface, identified by name, from the VPP config cache""" """Removes the bonderthernet member interface, identified by name, from the VPP config cache"""
if not ifname in self.cache["interface_names"]:
iface = self.get_interface_by_name(ifname)
if not iface:
self.logger.warning( self.logger.warning(
f"Trying to remove a bondethernet member interface which is not in the config: {ifname}" f"Trying to remove a bondethernet member interface which is not in the config: {ifname}"
) )
return False return False
iface = self.cache["interface_names"][ifname]
for bond_idx, members in self.cache["bondethernet_members"].items(): for bond_idx, members in self.cache["bondethernet_members"].items():
if iface.sw_if_index in members: if iface.sw_if_index in members:
self.cache["bondethernet_members"][bond_idx].remove(iface.sw_if_index) self.cache["bondethernet_members"][bond_idx].remove(iface.sw_if_index)
@ -143,36 +144,40 @@ class VPPApi:
def cache_remove_l2xc(self, ifname): def cache_remove_l2xc(self, ifname):
"""Remvoes the l2xc from the VPP config cache""" """Remvoes the l2xc from the VPP config cache"""
if not ifname in self.cache["interface_names"]:
iface = self.get_interface_by_name(ifname)
if not iface:
self.logger.warning( self.logger.warning(
f"Trying to remove an L2XC which is not in the config: {ifname}" f"Trying to remove an L2XC which is not in the config: {ifname}"
) )
return False return False
iface = self.cache["interface_names"][ifname]
self.cache["l2xcs"].pop(iface.sw_if_index, None) self.cache["l2xcs"].pop(iface.sw_if_index, None)
return True return True
def cache_remove_vxlan_tunnel(self, ifname): def cache_remove_vxlan_tunnel(self, ifname):
"""Removes a vxlan_tunnel from the VPP config cache""" """Removes a vxlan_tunnel from the VPP config cache"""
if not ifname in self.cache["interface_names"]:
iface = self.get_interface_by_name(ifname)
if not iface:
self.logger.warning( self.logger.warning(
f"Trying to remove a VXLAN Tunnel which is not in the config: {ifname}" f"Trying to remove a VXLAN Tunnel which is not in the config: {ifname}"
) )
return False return False
iface = self.cache["interface_names"][ifname]
self.cache["vxlan_tunnels"].pop(iface.sw_if_index, None) self.cache["vxlan_tunnels"].pop(iface.sw_if_index, None)
return True return True
def cache_remove_interface(self, ifname): def cache_remove_interface(self, ifname):
"""Removes the interface, identified by name, from the VPP config cache""" """Removes the interface, identified by name, from the VPP config cache"""
if not ifname in self.cache["interface_names"]:
iface = self.get_interface_by_name(ifname)
if not iface:
self.logger.warning( self.logger.warning(
f"Trying to remove an interface which is not in the config: {ifname}" f"Trying to remove an interface which is not in the config: {ifname}"
) )
return False return False
iface = self.cache["interface_names"][ifname]
del self.cache["interfaces"][iface.sw_if_index] del self.cache["interfaces"][iface.sw_if_index]
if len(self.cache["interface_addresses"][iface.sw_if_index]) > 0: if len(self.cache["interface_addresses"][iface.sw_if_index]) > 0:
self.logger.warning(f"Not all addresses were removed on {ifname}") self.logger.warning(f"Not all addresses were removed on {ifname}")
@ -219,7 +224,7 @@ class VPPApi:
api_response = self.vpp.api.sw_interface_dump() api_response = self.vpp.api.sw_interface_dump()
for iface in api_response: for iface in api_response:
self.cache["interfaces"][iface.sw_if_index] = iface self.cache["interfaces"][iface.sw_if_index] = iface
self.cache["interface_names"][iface.interface_name] = iface self.cache["interface_names"][iface.interface_name] = iface.sw_if_index
self.cache["interface_addresses"][iface.sw_if_index] = [] self.cache["interface_addresses"][iface.sw_if_index] = []
self.logger.debug(f"Retrieving IPv4 addresses for {iface.interface_name}") self.logger.debug(f"Retrieving IPv4 addresses for {iface.interface_name}")
ipr = self.vpp.api.ip_address_dump( ipr = self.vpp.api.ip_address_dump(
@ -283,6 +288,15 @@ class VPPApi:
ret = False ret = False
return ret return ret
def get_interface_by_name(self, name):
"""Return the VPP interface specified by name, or None if it cannot be found"""
try:
idx = self.cache["interface_names"][name]
return self.cache["interfaces"][idx]
except KeyError:
pass
return None
def get_sub_interfaces(self): def get_sub_interfaces(self):
"""Return all interfaces which have a sub-id and one or more tags""" """Return all interfaces which have a sub-id and one or more tags"""
subints = [ subints = [
@ -362,11 +376,9 @@ class VPPApi:
def tap_is_lcp(self, tap_ifname): def tap_is_lcp(self, tap_ifname):
"""Returns True if the given tap_ifname is a TAP interface belonging to an LCP, """Returns True if the given tap_ifname is a TAP interface belonging to an LCP,
or False otherwise.""" or False otherwise."""
if not tap_ifname in self.cache["interface_names"]:
return False
vpp_iface = self.cache["interface_names"][tap_ifname] vpp_iface = self.get_interface_by_name(tap_ifname)
if not vpp_iface.interface_dev_type == "virtio": if not vpp_iface or not vpp_iface.interface_dev_type == "virtio":
return False return False
for _idx, lcp in self.cache["lcps"].items(): for _idx, lcp in self.cache["lcps"].items():