From: Yann Dirson Date: Thu, 30 Nov 2023 16:45:00 +0000 (+0100) Subject: net: keep a cache of network interfaces X-Git-Tag: 0.3.0~11^2~7 X-Git-Url: http://xenbits.xensource.com/gitweb?a=commitdiff_plain;h=8e46e3ff5dbf3507910feac08974055445a3c0ca;p=xen-guest-agent.git net: keep a cache of network interfaces Collects in a central place a map of iface indices, their names, and their toolstack interface if any. Finally replaces add_vif_info() calls with get_toolstack_interface() called once to populate the cache. Finally the RmIp events can be applied when an interface disappears, which essentially fixes #13. Though this makes a new problem apparent: recreating the VIF sometimes does not get caught on Linux, when the iface is created as eth0 and then renamed by udev as enX0 in a subsequent Newlink message: since we are now only attempting to identify the toolkit-interface once, if when we try to access the /sys node for eth0 the renaming already occured the toolkit-interface is frozen as None. Signed-off-by: Yann Dirson --- diff --git a/src/collector_net.rs b/src/collector_net.rs index f84e50a..a998dc2 100644 --- a/src/collector_net.rs +++ b/src/collector_net.rs @@ -1,4 +1,4 @@ -use crate::datastructs::NetEvent; +use crate::datastructs::{NetEvent, NetInterfaceCache}; use futures::stream::Stream; use std::error::Error; use std::io; @@ -7,7 +7,7 @@ pub struct NetworkSource { } impl NetworkSource { - pub fn new() -> io::Result { + pub fn new(_cache: &'static mut NetInterfaceCache) -> io::Result { Ok(NetworkSource {}) } diff --git a/src/collector_net_netlink.rs b/src/collector_net_netlink.rs index c585c22..a052acb 100644 --- a/src/collector_net_netlink.rs +++ b/src/collector_net_netlink.rs @@ -1,6 +1,5 @@ use async_stream::try_stream; -use crate::datastructs::{NetEvent, NetEventOp, NetInterface, ToolstackNetInterface}; -use crate::unix_helpers::interface_name; +use crate::datastructs::{NetEvent, NetEventOp, NetInterface, NetInterfaceCache}; use futures::channel::mpsc::UnboundedReceiver; use futures::stream::{Stream, StreamExt}; use netlink_packet_core::{ @@ -24,19 +23,22 @@ use rtnetlink::constants::{ RTMGRP_IPV6_IFADDR, RTMGRP_LINK, }; +use std::collections::hash_map; use std::convert::TryInto; use std::error::Error; use std::io; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr}; +use std::rc::Rc; use std::vec::Vec; pub struct NetworkSource { handle: netlink_proto::ConnectionHandle, messages: UnboundedReceiver<(NetlinkMessage, SocketAddr)>, + iface_cache: &'static mut NetInterfaceCache, } impl NetworkSource { - pub fn new() -> io::Result { + pub fn new(iface_cache: &'static mut NetInterfaceCache) -> io::Result { let (mut connection, handle, messages) = new_connection(NETLINK_ROUTE)?; // What kinds of broadcast messages we want to listen for. let nl_mgroup_flags = RTMGRP_LINK | RTMGRP_IPV4_IFADDR | RTMGRP_IPV6_IFADDR; @@ -47,7 +49,7 @@ impl NetworkSource { .bind(&nl_addr) .expect("failed to bind"); tokio::spawn(connection); - Ok(NetworkSource { handle, messages }) + Ok(NetworkSource { handle, messages, iface_cache }) } pub async fn collect_current(&mut self) -> Result, Box> { @@ -129,7 +131,7 @@ impl NetworkSource { Ok(event) } - fn nl_linkmessage_decode(&mut self, msg: &LinkMessage) -> io::Result<(NetInterface, String)> { + fn nl_linkmessage_decode(&mut self, msg: &LinkMessage) -> io::Result<(Rc, String)> { let LinkMessage{header, nlas, ..} = msg; // extract fields of interest @@ -148,18 +150,17 @@ impl NetworkSource { .map(|b| format!("{b:02x}")) .collect::>().join(":")); - let iface = NetInterface { index: header.index, - name: iface_name.unwrap_or(String::from("")), - toolstack_iface: ToolstackNetInterface::None, - }; + let iface = self.iface_cache + .entry(header.index) + .or_insert_with_key(|index| NetInterface::new(*index, iface_name).into()); match mac_address { - Some(mac_address) => Ok((iface, mac_address)), - None => Ok((iface, "".to_string())), // FIXME ad-hoc ugly, use Option instead + Some(mac_address) => Ok((iface.clone(), mac_address)), + None => Ok((iface.clone(), "".to_string())), // FIXME ad-hoc ugly, use Option instead } } - fn nl_addressmessage_decode(&mut self, msg: &AddressMessage) -> io::Result<(NetInterface, IpAddr)> { + fn nl_addressmessage_decode(&mut self, msg: &AddressMessage) -> io::Result<(Rc, IpAddr)> { let AddressMessage{header, nlas, ..} = msg; // extract fields of interest @@ -193,15 +194,18 @@ impl NetworkSource { _ => None, }; - let iface = NetInterface { index: header.index, - name: interface_name(header.index), - toolstack_iface: ToolstackNetInterface::None, + let iface = match self.iface_cache.entry(header.index) { + hash_map::Entry::Occupied(entry) => { entry.get().clone() }, + hash_map::Entry::Vacant(_) => { + return Err(io::Error::new(io::ErrorKind::InvalidData, + format!("unknown interface for index {}", header.index))); + }, }; match address { - Some(address) => Ok((iface, address)), + Some(address) => Ok((iface.clone(), address)), None => Err(io::Error::new(io::ErrorKind::InvalidData, "unknown address")), } } - } + diff --git a/src/collector_net_pnet.rs b/src/collector_net_pnet.rs index cfe117a..ddbe3c1 100644 --- a/src/collector_net_pnet.rs +++ b/src/collector_net_pnet.rs @@ -1,9 +1,9 @@ use async_stream::try_stream; -use crate::datastructs::{NetEvent, NetEventOp, NetInterface, ToolstackNetInterface}; +use crate::datastructs::{NetEvent, NetEventOp, NetInterface, NetInterfaceCache}; use futures::stream::Stream; use ipnetwork::IpNetwork; use pnet_base::MacAddr; -use std::collections::{HashMap, HashSet}; +use std::collections::{HashMap, HashSet, hash_map}; use std::error::Error; use std::io; use std::time::Duration; @@ -29,11 +29,12 @@ impl InterfaceInfo { type AddressesState = HashMap; pub struct NetworkSource { addresses_cache: AddressesState, + iface_cache: &'static mut NetInterfaceCache, } impl NetworkSource { - pub fn new() -> io::Result { - Ok(NetworkSource {addresses_cache: AddressesState::new()}) + pub fn new(iface_cache: &'static mut NetInterfaceCache) -> io::Result { + Ok(NetworkSource {addresses_cache: AddressesState::new(), iface_cache}) } pub async fn collect_current(&mut self) -> Result, Box> { @@ -81,9 +82,15 @@ impl NetworkSource { // disappearing addresses for (cached_iface_index, cached_info) in self.addresses_cache.iter() { - let iface = NetInterface { index: *cached_iface_index, - name: cached_info.name.to_string(), - toolstack_iface: ToolstackNetInterface::None, + // iface object from iface_cache + let iface = match self.iface_cache.entry(*cached_iface_index) { + hash_map::Entry::Occupied(entry) => { entry.get().clone() }, + hash_map::Entry::Vacant(_) => { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + format!("disappearing interface with index {} not in iface_cache", + cached_iface_index))); + }, }; let iface_adresses = if let Some(iface_info) = current_addresses.get(cached_iface_index) { @@ -102,10 +109,12 @@ impl NetworkSource { } // appearing addresses for (iface_index, iface_info) in current_addresses.iter() { - let iface = NetInterface { index: *iface_index, - name: iface_info.name.to_string(), - toolstack_iface: ToolstackNetInterface::None, - }; + let iface = self.iface_cache + .entry(*iface_index) + .or_insert_with_key(|index| { + NetInterface::new(*index, Some(iface_info.name.clone())).into() + }) + .clone(); let cache_adresses = if let Some(cache_info) = self.addresses_cache.get(iface_index) { &cache_info.addresses diff --git a/src/datastructs.rs b/src/datastructs.rs index 381e069..25e3655 100644 --- a/src/datastructs.rs +++ b/src/datastructs.rs @@ -1,4 +1,6 @@ +use std::collections::HashMap; use std::net::IpAddr; +use std::rc::Rc; pub struct KernelInfo { pub release: String, @@ -30,6 +32,30 @@ pub struct NetInterface { pub toolstack_iface: ToolstackNetInterface, } +impl NetInterface { + pub fn new(index: u32, name: Option) -> NetInterface { + let name = match name { + Some(string) => { string }, + None => { + log::error!("new interface with index {index} has no name"); + String::from("") // this is not valid, but user will now be aware + }, + }; + NetInterface { index, + name: name.clone(), + toolstack_iface: crate::vif_detect::get_toolstack_interface(&name), + } + } +} + +// The cache of currently-known network interfaces. We have to use +// reference counting on the cached items, as we want on one hand to +// use references to those items from NetEvent, and OTOH we want to +// remove interfaces from here once unplugged. And Rust won't let us +// use `&'static NetInterface` because we can do the latter, which is +// good in the end. +pub type NetInterfaceCache = HashMap>; + #[derive(Debug)] pub enum NetEventOp { AddMac(String), @@ -40,6 +66,6 @@ pub enum NetEventOp { #[derive(Debug)] pub struct NetEvent { - pub iface: NetInterface, + pub iface: Rc, pub op: NetEventOp, } diff --git a/src/main.rs b/src/main.rs index 7c3dd13..5adf922 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,7 +23,7 @@ mod vif_detect; use clap::Parser; -use crate::datastructs::KernelInfo; +use crate::datastructs::{KernelInfo, NetInterfaceCache}; use crate::publisher::Publisher; use crate::collector_net::NetworkSource; use crate::collector_memory::MemorySource; @@ -64,9 +64,9 @@ async fn main() -> Result<(), Box> { let mut timer_stream = tokio::time::interval(Duration::from_secs(MEM_PERIOD_SECONDS)); // network events - let mut collector_net = NetworkSource::new()?; - for mut event in collector_net.collect_current().await? { - vif_detect::add_vif_info(&mut event); + let network_cache = Box::leak(Box::new(NetInterfaceCache::new())); + let mut collector_net = NetworkSource::new(network_cache)?; + for event in collector_net.collect_current().await? { if REPORT_INTERNAL_NICS || ! event.iface.toolstack_iface.is_none() { publisher.publish_netevent(&event)?; } @@ -79,8 +79,7 @@ async fn main() -> Result<(), Box> { select! { event = netevent_stream.try_next().fuse() => { match event? { - Some(mut event) => { - vif_detect::add_vif_info(&mut event); + Some(event) => { if REPORT_INTERNAL_NICS || ! event.iface.toolstack_iface.is_none() { publisher.publish_netevent(&event)?; } else { diff --git a/src/vif_detect.rs b/src/vif_detect.rs index 4b17587..1590719 100644 --- a/src/vif_detect.rs +++ b/src/vif_detect.rs @@ -3,6 +3,3 @@ use crate::datastructs::{NetEvent, ToolstackNetInterface}; pub fn get_toolstack_interface(iface_name: &str) -> ToolstackNetInterface { return ToolstackNetInterface::None; } - -pub fn add_vif_info(_event: &mut NetEvent) -> () { -} diff --git a/src/vif_detect_freebsd.rs b/src/vif_detect_freebsd.rs index 04dd334..f1c90c9 100644 --- a/src/vif_detect_freebsd.rs +++ b/src/vif_detect_freebsd.rs @@ -1,4 +1,4 @@ -use crate::datastructs::{NetEvent, ToolstackNetInterface}; +use crate::datastructs::ToolstackNetInterface; // identifies a VIF as named "xn%ID" @@ -16,7 +16,3 @@ pub fn get_toolstack_interface(iface_name: &str) -> ToolstackNetInterface { }, } } - -pub fn add_vif_info(event: &mut NetEvent) -> () { - event.iface.toolstack_iface = get_toolstack_interface(&event.iface.name); -} diff --git a/src/vif_detect_linux.rs b/src/vif_detect_linux.rs index 73629fa..d909a40 100644 --- a/src/vif_detect_linux.rs +++ b/src/vif_detect_linux.rs @@ -1,4 +1,4 @@ -use crate::datastructs::{NetEvent, ToolstackNetInterface}; +use crate::datastructs::ToolstackNetInterface; use std::fs; // identifies a VIF from sysfs as devtype="vif", and take the VIF id @@ -39,7 +39,3 @@ pub fn get_toolstack_interface(iface_name: &str) -> ToolstackNetInterface { }, } } - -pub fn add_vif_info(event: &mut NetEvent) { - event.iface.toolstack_iface = get_toolstack_interface(&event.iface.name); -}