From f3a31e6b9c7bd1ee2b1e181a32657574da28b9a4 Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 8 Aug 2025 19:14:52 +0200 Subject: [PATCH 1/4] feat(vpc-manager): rename config -> vrfconfig Signed-off-by: Fredi Raspall --- mgmt/src/vpc_manager/mod.rs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/mgmt/src/vpc_manager/mod.rs b/mgmt/src/vpc_manager/mod.rs index 4d949a470..95e13c33a 100644 --- a/mgmt/src/vpc_manager/mod.rs +++ b/mgmt/src/vpc_manager/mod.rs @@ -312,8 +312,8 @@ impl TryFrom<&InternalConfig> for RequiredInformationBase { let mut vrfs = MultiIndexVrfPropertiesSpecMap::default(); let mut vteps = MultiIndexVtepPropertiesSpecMap::default(); let mut associations = MultiIndexInterfaceAssociationSpecMap::default(); - for config in internal.vrfs.iter_by_tableid() { - for iface in config.interfaces.values() { + for vrfconfig in internal.vrfs.iter_by_tableid() { + for iface in vrfconfig.interfaces.values() { match &iface.iftype { InterfaceType::Ethernet(eth) => { let mut tap = InterfaceSpecBuilder::default(); @@ -375,17 +375,17 @@ impl TryFrom<&InternalConfig> for RequiredInformationBase { let mut bridge = InterfaceSpecBuilder::default(); vrf.controller(None); vrf.admin_state(AdminState::Up); - match config.tableid { + match vrfconfig.tableid { None => {} Some(route_table_id) => { - debug!("route_table set for config: {config:?}"); + debug!("route_table set for config: {vrfconfig:?}"); vrf.properties(InterfacePropertiesSpec::Vrf(VrfPropertiesSpec { route_table_id, })); - match &config.vpc_id { + match &vrfconfig.vpc_id { None => { - error!("no vpc_id set for config: {config:?}"); - panic!("no vpc_id set for config: {config:?}"); + error!("no vpc_id set for config: {vrfconfig:?}"); + panic!("no vpc_id set for config: {vrfconfig:?}"); } Some(id) => { vrf.name(id.vrf_name()); @@ -400,7 +400,7 @@ impl TryFrom<&InternalConfig> for RequiredInformationBase { vlan_protocol: EthType::VLAN, })); vtep.properties(InterfacePropertiesSpec::Vtep(VtepPropertiesSpec { - vni: config.vni.expect("vni not set"), + vni: vrfconfig.vni.expect("vni not set"), local: vtep_ip, ttl: VtepConfig::TTL, port: Vxlan::PORT, From 60c1ad17a5835369e7525ba926135ce255cc271a Mon Sep 17 00:00:00 2001 From: Fredi Raspall Date: Fri, 8 Aug 2025 19:17:16 +0200 Subject: [PATCH 2/4] fix(vpc-manager): Avoid hitting unreachable! The dataplane may receive configurations that do not include any VPC. In that case (i.e. underlay-only configs) we can't assume that the vtep configuration is there as it is not needed. So, the vpc manager should only assume that a vtep config is present if there are VPCs (vrfs aside from the default). The original code worked fine because the default vrf was excepted by the vpc manager: if there were no vrfs other than the default, we were not checking for the existence of a vtep, and if there were, the validation of the configuration ensured that a vtep was present. A guard for this was removed in 3301da3ec51f71ceb98339dea97d2d4237ab8c6b when the vpc manager started to create interfaces for the default vrf, which broke the original assumption. Restoring the guard that excepted the default vrf is no longer an option since we need to configure interfaces for the default vrf. So, split the method to iterate over non-default vrfs and then deal with the interfaces for the default vrf. Signed-off-by: Fredi Raspall --- mgmt/src/vpc_manager/mod.rs | 117 ++++++++++++++++++++---------------- 1 file changed, 66 insertions(+), 51 deletions(-) diff --git a/mgmt/src/vpc_manager/mod.rs b/mgmt/src/vpc_manager/mod.rs index 95e13c33a..b2e9715cc 100644 --- a/mgmt/src/vpc_manager/mod.rs +++ b/mgmt/src/vpc_manager/mod.rs @@ -4,7 +4,7 @@ use crate::processor::confbuild::namegen::VpcInterfacesNames; use config::InternalConfig; -use config::internal::interfaces::interface::InterfaceType; +use config::internal::interfaces::interface::{InterfaceConfigTable, InterfaceType}; use config::internal::routing::evpn::VtepConfig; use derive_builder::Builder; use futures::TryStreamExt; @@ -301,6 +301,58 @@ impl Vpc { } } +/// Create an InterfaceSpec for an InterfaceConfig +fn add_interface_specs(interfaces: &mut MultiIndexInterfaceSpecMap, ifaces: &InterfaceConfigTable) { + for iface in ifaces.values() { + match &iface.iftype { + InterfaceType::Ethernet(eth) => { + let mut tap = InterfaceSpecBuilder::default(); + match InterfaceName::try_from(format!("{}-tap", iface.name.as_str())) { + Ok(name) => { + tap.name(name); + } + Err(e) => { + error!("{e}"); + continue; + } + }; + match eth.mac.map(SourceMac::try_from) { + Some(Ok(mac)) => { + tap.mac(Some(mac)); + } + None => { + tap.mac(None); + } + Some(Err(e)) => { + error!("{e}"); + continue; + } + }; + tap.properties(InterfacePropertiesSpec::Tap); + tap.mtu(iface.mtu); + tap.admin_state(AdminState::Up); + match tap.build() { + Ok(iface) => match interfaces.try_insert(iface) { + Ok(added) => { + debug!("added proxy tap interface to spec: {added:?}"); + } + Err(e) => { + error!("{e}"); + } + }, + Err(e) => { + error!("{e}"); + continue; + } + } + } + _ => { + continue; + } + } + } +} + // TODO: break up this method into smaller components impl TryFrom<&InternalConfig> for RequiredInformationBase { type Error = RequiredInformationBaseBuilderError; @@ -312,55 +364,10 @@ impl TryFrom<&InternalConfig> for RequiredInformationBase { let mut vrfs = MultiIndexVrfPropertiesSpecMap::default(); let mut vteps = MultiIndexVtepPropertiesSpecMap::default(); let mut associations = MultiIndexInterfaceAssociationSpecMap::default(); - for vrfconfig in internal.vrfs.iter_by_tableid() { - for iface in vrfconfig.interfaces.values() { - match &iface.iftype { - InterfaceType::Ethernet(eth) => { - let mut tap = InterfaceSpecBuilder::default(); - match InterfaceName::try_from(format!("{}-tap", iface.name.as_str())) { - Ok(name) => { - tap.name(name); - } - Err(e) => { - error!("{e}"); - continue; - } - }; - match eth.mac.map(SourceMac::try_from) { - Some(Ok(mac)) => { - tap.mac(Some(mac)); - } - None => { - tap.mac(None); - } - Some(Err(e)) => { - error!("{e}"); - continue; - } - }; - tap.properties(InterfacePropertiesSpec::Tap); - tap.mtu(iface.mtu); - tap.admin_state(AdminState::Up); - match tap.build() { - Ok(iface) => match interfaces.try_insert(iface) { - Ok(added) => { - debug!("added proxy tap interface to spec: {added:?}"); - } - Err(e) => { - error!("{e}"); - } - }, - Err(e) => { - error!("{e}"); - continue; - } - } - } - _ => { - continue; - } - } - } + + // non-default VRFs + for vrfconfig in internal.vrfs.iter_by_tableid().filter(|cfg| !cfg.default) { + add_interface_specs(&mut interfaces, &vrfconfig.interfaces); let main_vtep = internal.vtep.as_ref().unwrap_or_else(|| unreachable!()); let vtep_ip = match main_vtep.address { UnicastIpAddr::V4(vtep_ip) => vtep_ip, @@ -376,7 +383,7 @@ impl TryFrom<&InternalConfig> for RequiredInformationBase { vrf.controller(None); vrf.admin_state(AdminState::Up); match vrfconfig.tableid { - None => {} + None => unreachable!(), Some(route_table_id) => { debug!("route_table set for config: {vrfconfig:?}"); vrf.properties(InterfacePropertiesSpec::Vrf(VrfPropertiesSpec { @@ -498,6 +505,14 @@ impl TryFrom<&InternalConfig> for RequiredInformationBase { } } } + + // default VRF + let vrfconfig = internal + .vrfs + .default_vrf_config() + .unwrap_or_else(|| unreachable!()); + add_interface_specs(&mut interfaces, &vrfconfig.interfaces); + rb_builder.interfaces(interfaces); rb_builder.vteps(vteps); rb_builder.vrfs(vrfs); From dfa207384f7bad639b935239c3c618fc5fad0ac6 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 11 Aug 2025 12:07:29 -0600 Subject: [PATCH 3/4] fix(interface-manager): revert "feat(interface-manager)!: dummy support" This is not workable until we move the vpc manager to its own network namespace. Till then it just breaks the CI. This reverts commit dafe5faf59e4fcd67bc5020ed9704cb832bb8fdc. Signed-off-by: Daniel Noland --- interface-manager/src/interface/mod.rs | 4 ---- interface-manager/src/interface/properties.rs | 3 --- mgmt/src/vpc_manager/mod.rs | 1 - mgmt/tests/reconcile.rs | 2 -- net/src/interface/display.rs | 3 +-- net/src/interface/mod.rs | 2 -- 6 files changed, 1 insertion(+), 14 deletions(-) diff --git a/interface-manager/src/interface/mod.rs b/interface-manager/src/interface/mod.rs index 96e7fad25..779ad21e5 100644 --- a/interface-manager/src/interface/mod.rs +++ b/interface-manager/src/interface/mod.rs @@ -150,7 +150,6 @@ impl Create for Manager { ])) .build() } - InterfacePropertiesSpec::Dummy => LinkDummy::new(requirement.name.as_ref()).build(), InterfacePropertiesSpec::Vtep(properties) => { LinkVxlan::new(requirement.name.as_ref(), properties.vni.as_u32()) .set_info_data(InfoData::Vxlan(vec![ @@ -815,9 +814,6 @@ impl TryFromLinkMessage for Interface { match (kind, pci_netdev_builder.build()) { (Some(kind), Err(_)) => match kind { - InfoKind::Dummy => { - builder.properties(InterfaceProperties::Dummy); - } InfoKind::Tun => { builder.properties(InterfaceProperties::Tap); } diff --git a/interface-manager/src/interface/properties.rs b/interface-manager/src/interface/properties.rs index 49282702f..bde4f7ade 100644 --- a/interface-manager/src/interface/properties.rs +++ b/interface-manager/src/interface/properties.rs @@ -13,8 +13,6 @@ use serde::{Deserialize, Serialize}; pub enum InterfacePropertiesSpec { /// The planned properties of a bridge. Bridge(BridgePropertiesSpec), - /// The planned properties of a dummy interface - Dummy, /// The planned properties of a tap device Tap, /// The expected properties of a pci netdev. @@ -36,7 +34,6 @@ impl AsRequirement for InterfaceProperties { InterfaceProperties::Bridge(props) => { InterfacePropertiesSpec::Bridge(props.as_requirement()) } - InterfaceProperties::Dummy => InterfacePropertiesSpec::Dummy, InterfaceProperties::Vtep(props) => { InterfacePropertiesSpec::Vtep(props.as_requirement()?) } diff --git a/mgmt/src/vpc_manager/mod.rs b/mgmt/src/vpc_manager/mod.rs index b2e9715cc..13438a8a0 100644 --- a/mgmt/src/vpc_manager/mod.rs +++ b/mgmt/src/vpc_manager/mod.rs @@ -602,7 +602,6 @@ mod contract { } } InterfacePropertiesSpec::Tap - | InterfacePropertiesSpec::Dummy | InterfacePropertiesSpec::Pci(_) => {} } } diff --git a/mgmt/tests/reconcile.rs b/mgmt/tests/reconcile.rs index 8c72ac018..24c5f1d72 100644 --- a/mgmt/tests/reconcile.rs +++ b/mgmt/tests/reconcile.rs @@ -170,7 +170,6 @@ async fn reconcile_demo() { InterfacePropertiesSpec::Pci(prop) => { pci_props.try_insert(prop.clone()).unwrap(); } - InterfacePropertiesSpec::Dummy => {} InterfacePropertiesSpec::Tap => {} } } @@ -260,7 +259,6 @@ async fn reconcile_demo() { for interface in interfaces { match &interface.properties { InterfacePropertiesSpec::Bridge(_) - | InterfacePropertiesSpec::Dummy | InterfacePropertiesSpec::Pci(_) | InterfacePropertiesSpec::Tap => {} InterfacePropertiesSpec::Vtep(props) => { diff --git a/net/src/interface/display.rs b/net/src/interface/display.rs index 4728ba1c7..ad83266ab 100644 --- a/net/src/interface/display.rs +++ b/net/src/interface/display.rs @@ -106,7 +106,7 @@ impl Display for InterfaceProperties { InterfaceProperties::Vrf(vrf) => vrf.fmt(f), InterfaceProperties::Vtep(vtep) => vtep.fmt(f), InterfaceProperties::Pci(rep) => rep.fmt(f), - InterfaceProperties::Dummy | InterfaceProperties::Tap => "".fmt(f), + InterfaceProperties::Tap => "".fmt(f), InterfaceProperties::Other => write!(f, "other"), } } @@ -115,7 +115,6 @@ impl Display for InterfaceProperties { fn ifproperty_to_str(properties: &InterfaceProperties) -> &'static str { match properties { InterfaceProperties::Bridge(_) => "bridge", - InterfaceProperties::Dummy => "dummy", InterfaceProperties::Vrf(_) => "vrf", InterfaceProperties::Vtep(_) => "vtep", InterfaceProperties::Pci(_) => "pci", diff --git a/net/src/interface/mod.rs b/net/src/interface/mod.rs index 019011054..728791eb4 100644 --- a/net/src/interface/mod.rs +++ b/net/src/interface/mod.rs @@ -307,8 +307,6 @@ impl Interface { pub enum InterfaceProperties { /// Properties of bridges Bridge(BridgeProperties), - /// Dummy interface properties - Dummy, /// Properties of VTEPs (vxlan devices) Vtep(VtepProperties), /// Properties of VRFs From 37a817de81709e0e58daeac13e5d9db5cb3c9700 Mon Sep 17 00:00:00 2001 From: Daniel Noland Date: Mon, 11 Aug 2025 12:10:23 -0600 Subject: [PATCH 4/4] fix(mgmt): remove stray mention of dummy The dummy interface revert missed this. Signed-off-by: Daniel Noland --- interface-manager/src/interface/mod.rs | 2 +- mgmt/src/vpc_manager/mod.rs | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/interface-manager/src/interface/mod.rs b/interface-manager/src/interface/mod.rs index 779ad21e5..b4c3c6013 100644 --- a/interface-manager/src/interface/mod.rs +++ b/interface-manager/src/interface/mod.rs @@ -53,7 +53,7 @@ use rtnetlink::packet_route::link::{ InfoBridge, InfoData, InfoKind, InfoVrf, InfoVxlan, LinkAttribute, LinkFlags, LinkInfo, LinkMessage, State, }; -use rtnetlink::{LinkBridge, LinkDummy, LinkUnspec, LinkVrf, LinkVxlan}; +use rtnetlink::{LinkBridge, LinkUnspec, LinkVrf, LinkVxlan}; use serde::{Deserialize, Serialize}; use std::num::NonZero; use tracing::{debug, error, warn}; diff --git a/mgmt/src/vpc_manager/mod.rs b/mgmt/src/vpc_manager/mod.rs index 13438a8a0..48cbb2fce 100644 --- a/mgmt/src/vpc_manager/mod.rs +++ b/mgmt/src/vpc_manager/mod.rs @@ -152,8 +152,7 @@ impl Observe for VpcManager { InterfaceProperties::Other | InterfaceProperties::Tap | InterfaceProperties::Pci(_) - | InterfaceProperties::Bridge(_) - | InterfaceProperties::Dummy => { /* nothing to index */ } + | InterfaceProperties::Bridge(_) => { /* nothing to index */ } } } for sliced in indexes_to_remove { @@ -601,8 +600,7 @@ mod contract { .unwrap(); } } - InterfacePropertiesSpec::Tap - | InterfacePropertiesSpec::Pci(_) => {} + InterfacePropertiesSpec::Tap | InterfacePropertiesSpec::Pci(_) => {} } } if !bridges.is_empty() {