fix: tighten shared virtual nic lifecycle

Key shared virtual NIC registry entries by both netns and device
name. Instances in different namespaces can no longer reuse the same
underlying device by accident.

Store each member's configured MTU separately and publish it as
the initial MTU claim after device creation. Later members no longer
inherit only the first member's MTU.

Shut down the dispatcher when the last current member leaves, and
invalidate the shared NIC if member cleanup fails. The registry entry
remains available for a later clean recreate.

Keep Magic DNS fake-IP routes owned by shared TUN members through
the shared backend. Stopping one member no longer removes the route
while another shared member still owns it, and Linux retained routes
are re-applied after address deletion.

Extend the shared TUN Magic DNS integration test to drop one shared
member first, verify DNS still works through the remaining member,
and then verify final cleanup after the last member exits.
This commit is contained in:
sijie.sun
2026-06-15 00:24:08 +08:00
parent d87624b353
commit ca17e856a6
6 changed files with 617 additions and 37 deletions
+117 -3
View File
@@ -1,6 +1,8 @@
use cidr::Ipv4Inet;
use tokio_util::sync::CancellationToken;
#[cfg(feature = "tun")]
use crate::instance::virtual_nic::NicBackend;
use crate::peers::peer_manager::PeerManager;
use std::{net::Ipv4Addr, sync::Arc, time::Duration};
@@ -16,6 +18,47 @@ pub struct DnsRunner {
netns: Option<String>,
tun_inet: Ipv4Inet,
fake_ip: Ipv4Addr,
#[cfg(feature = "tun")]
route_backend: Option<NicBackend>,
}
#[cfg(feature = "tun")]
#[derive(Clone)]
struct MagicDnsFakeIpRouteClaim {
tun_dev_name: String,
fake_ip: Ipv4Addr,
netns: Option<String>,
route_backend: NicBackend,
}
#[cfg(feature = "tun")]
impl MagicDnsFakeIpRouteClaim {
async fn add(self) -> anyhow::Result<()> {
let cost = if cfg!(target_os = "windows") {
Some(4)
} else {
None
};
MagicDnsServerInstance::add_fake_ip_route(
&self.tun_dev_name,
self.fake_ip,
self.netns,
cost,
Some(&self.route_backend),
)
.await
}
async fn remove(self) {
MagicDnsServerInstance::remove_fake_ip_route(
&self.tun_dev_name,
self.fake_ip,
self.netns,
Some(&self.route_backend),
)
.await;
}
}
impl DnsRunner {
@@ -33,6 +76,8 @@ impl DnsRunner {
netns: None,
tun_inet,
fake_ip,
#[cfg(feature = "tun")]
route_backend: None,
}
}
@@ -48,6 +93,12 @@ impl DnsRunner {
runner
}
#[cfg(feature = "tun")]
pub fn with_route_backend(mut self, route_backend: NicBackend) -> Self {
self.route_backend = Some(route_backend);
self
}
async fn clean_env(&mut self) {
if let Some(server) = self.server.take() {
server.clean_env().await;
@@ -55,17 +106,73 @@ impl DnsRunner {
self.client.take();
}
#[cfg(feature = "tun")]
fn should_manage_fake_ip_route(&self) -> bool {
self.route_backend.is_some()
&& self.tun_dev.is_some()
&& !self.tun_inet.contains(&self.fake_ip)
}
#[cfg(feature = "tun")]
fn fake_ip_route_claim(&self) -> Option<MagicDnsFakeIpRouteClaim> {
if !self.should_manage_fake_ip_route() {
return None;
}
let Some(tun_dev_name) = &self.tun_dev else {
return None;
};
let route_backend = self.route_backend.clone()?;
Some(MagicDnsFakeIpRouteClaim {
tun_dev_name: tun_dev_name.clone(),
fake_ip: self.fake_ip,
netns: self.netns.clone(),
route_backend,
})
}
async fn run_once(&mut self) -> anyhow::Result<()> {
#[cfg(feature = "tun")]
if let Some(claim) = self.fake_ip_route_claim() {
claim
.add()
.await
.map_err(|err| anyhow::anyhow!("failed to add magic dns fake-ip route: {err}"))?;
}
// try server first
match MagicDnsServerInstance::new_with_netns(
#[cfg(feature = "tun")]
let server_result = if self.should_manage_fake_ip_route() {
MagicDnsServerInstance::new_with_external_fake_ip_route(
self.peer_mgr.clone(),
self.tun_dev.clone(),
self.tun_inet,
self.fake_ip,
self.netns.clone(),
)
.await
} else {
MagicDnsServerInstance::new_with_route_backend(
self.peer_mgr.clone(),
self.tun_dev.clone(),
self.tun_inet,
self.fake_ip,
self.netns.clone(),
None,
)
.await
};
#[cfg(not(feature = "tun"))]
let server_result = MagicDnsServerInstance::new_with_netns(
self.peer_mgr.clone(),
self.tun_dev.clone(),
self.tun_inet,
self.fake_ip,
self.netns.clone(),
)
.await
{
.await;
match server_result {
Ok(server) => {
self.server = Some(server);
tracing::info!("DnsRunner::run_once: server started");
@@ -84,11 +191,18 @@ impl DnsRunner {
}
pub async fn run(&mut self, canel_token: CancellationToken) {
#[cfg(feature = "tun")]
let fake_ip_route_claim = self.fake_ip_route_claim();
loop {
tracing::info!("DnsRunner::run: start");
tokio::select! {
_ = canel_token.cancelled() => {
self.clean_env().await;
#[cfg(feature = "tun")]
if let Some(claim) = fake_ip_route_claim.clone() {
claim.remove().await;
}
tracing::info!("DnsRunner::run: cancelled");
return;
}
@@ -12,6 +12,8 @@ use super::{
server::Server,
system_config::{OSConfig, SystemConfig},
};
#[cfg(feature = "tun")]
use crate::instance::virtual_nic::NicBackend;
use crate::{
common::{
PeerId,
@@ -66,6 +68,9 @@ pub(super) struct MagicDnsServerInstanceData {
netns: Option<String>,
tun_ip: Ipv4Addr,
fake_ip: Ipv4Addr,
#[cfg(feature = "tun")]
route_backend: Option<NicBackend>,
manage_fake_ip_route: bool,
my_peer_id: PeerId,
// zone -> (tunnel remote addr -> route)
@@ -513,12 +518,27 @@ fn get_system_config(
}
impl MagicDnsServerInstance {
async fn add_fake_ip_route(
pub(super) async fn add_fake_ip_route(
tun_dev_name: &str,
fake_ip: Ipv4Addr,
netns: Option<String>,
cost: Option<i32>,
#[cfg(feature = "tun")] route_backend: Option<&NicBackend>,
) -> Result<(), anyhow::Error> {
#[cfg(feature = "tun")]
if let Some(route_backend) = route_backend {
match route_backend.add_route_with_cost(fake_ip, 32, cost).await {
Err(EtError::IOError(err)) if err.kind() == io::ErrorKind::AlreadyExists => {
let ifcfg = IfConfiger::default();
let _guard = NetNS::new(netns).guard();
ifcfg.remove_ipv4_route(tun_dev_name, fake_ip, 32).await?;
route_backend.add_route_with_cost(fake_ip, 32, cost).await?;
return Ok(());
}
ret => return ret.map_err(Into::into),
}
}
let ifcfg = IfConfiger::default();
let _guard = NetNS::new(netns).guard();
match ifcfg.add_ipv4_route(tun_dev_name, fake_ip, 32, cost).await {
@@ -533,7 +553,25 @@ impl MagicDnsServerInstance {
}
}
async fn remove_fake_ip_route(tun_dev_name: &str, fake_ip: Ipv4Addr, netns: Option<String>) {
pub(super) async fn remove_fake_ip_route(
tun_dev_name: &str,
fake_ip: Ipv4Addr,
netns: Option<String>,
#[cfg(feature = "tun")] route_backend: Option<&NicBackend>,
) {
#[cfg(feature = "tun")]
if let Some(route_backend) = route_backend {
if let Err(err) = route_backend.remove_route(fake_ip, 32).await {
tracing::warn!(
?err,
?tun_dev_name,
?fake_ip,
"remove magic dns route failed"
);
}
return;
}
let ifcfg = IfConfiger::default();
let _guard = NetNS::new(netns).guard();
if let Err(err) = ifcfg.remove_ipv4_route(tun_dev_name, fake_ip, 32).await {
@@ -561,6 +599,60 @@ impl MagicDnsServerInstance {
tun_inet: Ipv4Inet,
fake_ip: Ipv4Addr,
netns: Option<String>,
) -> Result<Self, anyhow::Error> {
Self::new_inner(
peer_mgr,
tun_dev,
tun_inet,
fake_ip,
netns,
#[cfg(feature = "tun")]
None,
true,
)
.await
}
#[cfg(feature = "tun")]
pub async fn new_with_route_backend(
peer_mgr: Arc<PeerManager>,
tun_dev: Option<String>,
tun_inet: Ipv4Inet,
fake_ip: Ipv4Addr,
netns: Option<String>,
route_backend: Option<NicBackend>,
) -> Result<Self, anyhow::Error> {
Self::new_inner(
peer_mgr,
tun_dev,
tun_inet,
fake_ip,
netns,
route_backend,
true,
)
.await
}
#[cfg(feature = "tun")]
pub async fn new_with_external_fake_ip_route(
peer_mgr: Arc<PeerManager>,
tun_dev: Option<String>,
tun_inet: Ipv4Inet,
fake_ip: Ipv4Addr,
netns: Option<String>,
) -> Result<Self, anyhow::Error> {
Self::new_inner(peer_mgr, tun_dev, tun_inet, fake_ip, netns, None, false).await
}
async fn new_inner(
peer_mgr: Arc<PeerManager>,
tun_dev: Option<String>,
tun_inet: Ipv4Inet,
fake_ip: Ipv4Addr,
netns: Option<String>,
#[cfg(feature = "tun")] route_backend: Option<NicBackend>,
manage_fake_ip_route: bool,
) -> Result<Self, anyhow::Error> {
let tcp_listener = TcpTunnelListener::new(MAGIC_DNS_INSTANCE_ADDR.parse()?);
let mut rpc_server = StandAloneServer::new(tcp_listener);
@@ -573,7 +665,8 @@ impl MagicDnsServerInstance {
let mut dns_server = Server::new(dns_config);
dns_server.run().await?;
if !tun_inet.contains(&fake_ip)
if manage_fake_ip_route
&& !tun_inet.contains(&fake_ip)
&& let Some(tun_dev_name) = &tun_dev
{
let cost = if cfg!(target_os = "windows") {
@@ -581,7 +674,15 @@ impl MagicDnsServerInstance {
} else {
None
};
Self::add_fake_ip_route(tun_dev_name, fake_ip, netns.clone(), cost).await?;
Self::add_fake_ip_route(
tun_dev_name,
fake_ip,
netns.clone(),
cost,
#[cfg(feature = "tun")]
route_backend.as_ref(),
)
.await?;
}
let data = Arc::new(MagicDnsServerInstanceData {
@@ -590,6 +691,9 @@ impl MagicDnsServerInstance {
netns,
tun_ip: tun_inet.address(),
fake_ip,
#[cfg(feature = "tun")]
route_backend,
manage_fake_ip_route,
my_peer_id: peer_mgr.my_peer_id(),
route_infos: DashMap::new(),
system_config: get_system_config(tun_dev.as_deref())?,
@@ -632,11 +736,18 @@ impl MagicDnsServerInstance {
}
}
if !self.tun_inet.contains(&self.data.fake_ip)
if self.data.manage_fake_ip_route
&& !self.tun_inet.contains(&self.data.fake_ip)
&& let Some(tun_dev_name) = &self.data.tun_dev
{
Self::remove_fake_ip_route(tun_dev_name, self.data.fake_ip, self.data.netns.clone())
.await;
Self::remove_fake_ip_route(
tun_dev_name,
self.data.fake_ip,
self.data.netns.clone(),
#[cfg(feature = "tun")]
self.data.route_backend.as_ref(),
)
.await;
}
let _ = self
+24 -4
View File
@@ -864,6 +864,7 @@ impl Instance {
peer_mgr: Arc<PeerManager>,
tun_dev: Option<String>,
tun_ip: Ipv4Inet,
#[cfg(feature = "tun")] route_backend: Option<super::virtual_nic::NicBackend>,
) -> Option<DnsRunner> {
let ctx = peer_mgr.get_global_ctx();
if !ctx.config.get_flags().accept_dns {
@@ -877,6 +878,12 @@ impl Instance {
MAGIC_DNS_FAKE_IP.parse().unwrap(),
ctx.net_ns.name(),
);
#[cfg(feature = "tun")]
let runner = if let Some(route_backend) = route_backend {
runner.with_route_backend(route_backend)
} else {
runner
};
Some(runner)
}
@@ -1042,12 +1049,19 @@ impl Instance {
continue;
}
#[cfg(feature = "magic-dns")]
let route_backend = new_nic_ctx.shared_route_backend_for_dns();
#[cfg(feature = "magic-dns")]
let ifname = new_nic_ctx.ifname().await;
Self::use_new_nic_ctx(
nic_ctx.clone(),
new_nic_ctx,
#[cfg(feature = "magic-dns")]
Self::create_magic_dns_runner(peer_manager_c.clone(), ifname, ip),
Self::create_magic_dns_runner(
peer_manager_c.clone(),
ifname,
ip,
route_backend,
),
)
.await;
}
@@ -1128,9 +1142,10 @@ impl Instance {
// Create Magic DNS runner only if we have IPv4
#[cfg(feature = "magic-dns")]
{
let route_backend = new_nic_ctx.shared_route_backend_for_dns();
let ifname = new_nic_ctx.ifname().await;
let dns_runner = if let Some(ipv4) = ipv4_addr {
Self::create_magic_dns_runner(peer_mgr, ifname, ipv4)
Self::create_magic_dns_runner(peer_mgr, ifname, ipv4, route_backend)
} else {
None
};
@@ -1738,8 +1753,9 @@ impl Instance {
#[cfg(feature = "magic-dns")]
{
let route_backend = new_nic_ctx.shared_route_backend_for_dns();
let magic_dns_runner = if let Some(ipv4) = global_ctx.get_ipv4() {
Self::create_magic_dns_runner(peer_manager.clone(), None, ipv4)
Self::create_magic_dns_runner(peer_manager.clone(), None, ipv4, route_backend)
} else {
None
};
@@ -1876,7 +1892,11 @@ mod tests {
let second_shared_nic = second.shared_nic_for_test().unwrap();
assert!(Arc::ptr_eq(&first_shared_nic, &second_shared_nic));
let registered_nic = registry.lock().await.get("et-shared").unwrap();
let registered_nic = registry
.lock()
.await
.get_by_dev_name_for_test("et-shared")
.unwrap();
assert!(Arc::ptr_eq(&registered_nic, &first_shared_nic));
}
+293 -21
View File
@@ -312,13 +312,20 @@ impl SharedVirtualNic {
) -> Result<(), Error> {
self.ensure_valid()?;
match self.member_registrations.insert(member_id, registration_id) {
Some(old_registration_id) if old_registration_id != registration_id => {
self.remove_member_claims(member_id).await?;
match self.member_registrations.get(&member_id).copied() {
Some(old_registration_id) if old_registration_id == registration_id => {
return Ok(());
}
_ => {}
Some(_) => {
if let Err(err) = self.remove_member_claims(member_id).await {
self.invalidate_and_shutdown_dispatcher().await;
return Err(err);
}
}
None => {}
}
self.member_registrations.insert(member_id, registration_id);
Ok(())
}
@@ -366,8 +373,15 @@ impl SharedVirtualNic {
return Ok(());
}
if let Err(err) = self.remove_member_claims(member_id).await {
self.member_registrations.remove(&member_id);
self.invalidate_and_shutdown_dispatcher().await;
return Err(err);
}
self.member_registrations.remove(&member_id);
self.remove_member_claims(member_id).await
self.shutdown_dispatcher_if_idle().await;
Ok(())
}
async fn apply_member_claims(
@@ -384,7 +398,7 @@ impl SharedVirtualNic {
self.sync_dispatcher_sources_for_ifcfg_update(member_id, &old_claims, &next_claims)
.await?;
if let Err(err) = self.apply_ifcfg_delta(&delta).await {
if let Err(err) = self.apply_ifcfg_delta(&delta, &next_ifcfg).await {
let _ = self
.sync_dispatcher_sources_for_member(member_id, &old_claims)
.await;
@@ -417,6 +431,37 @@ impl SharedVirtualNic {
Ok(())
}
async fn apply_member_mtu_for_registration(
&mut self,
member_id: SharedVirtualNicMemberId,
registration_id: SharedVirtualNicMemberRegistrationId,
mtu: u32,
) -> Result<(), Error> {
if !self.is_current_member_registration(member_id, registration_id) {
return Ok(());
}
let mut claims = self.ifcfg.claims_of(member_id);
claims.mtu = Some(mtu);
self.apply_member_claims(member_id, claims).await
}
#[cfg(mobile)]
async fn apply_member_mtu_for_mobile_registration(
&mut self,
member_id: SharedVirtualNicMemberId,
registration_id: SharedVirtualNicMemberRegistrationId,
mtu: u32,
) -> Result<(), Error> {
if !self.is_current_member_registration(member_id, registration_id) {
return Ok(());
}
let mut claims = self.ifcfg.claims_of(member_id);
claims.mtu = Some(mtu);
self.apply_member_claims_for_mobile(member_id, claims).await
}
async fn remove_member_claims(
&mut self,
member_id: SharedVirtualNicMemberId,
@@ -428,7 +473,7 @@ impl SharedVirtualNic {
return Ok(());
};
#[cfg(not(mobile))]
self.apply_ifcfg_delta(&delta).await?;
self.apply_ifcfg_delta(&delta, &next_ifcfg).await?;
#[cfg(mobile)]
drop(delta);
self.remove_dispatcher_sources_for_member(member_id).await?;
@@ -437,7 +482,30 @@ impl SharedVirtualNic {
Ok(())
}
async fn apply_ifcfg_delta(&self, delta: &SharedIfConfigDelta) -> Result<(), Error> {
async fn shutdown_dispatcher_if_idle(&mut self) {
if !self.member_registrations.is_empty() {
return;
}
self.shutdown_dispatcher().await;
}
async fn invalidate_and_shutdown_dispatcher(&mut self) {
self.mark_invalid();
self.shutdown_dispatcher().await;
}
async fn shutdown_dispatcher(&mut self) {
if let Some(dispatcher) = self.dispatcher.take() {
dispatcher.shutdown_without_invalidation().await;
}
}
async fn apply_ifcfg_delta(
&self,
delta: &SharedIfConfigDelta,
_next_ifcfg: &SharedIfConfig,
) -> Result<(), Error> {
let nic = self.nic.lock().await;
for route in &delta.ipv4_routes.removed {
@@ -476,6 +544,26 @@ impl SharedVirtualNic {
.await?;
}
#[cfg(target_os = "linux")]
if !delta.ipv4_addresses.removed.is_empty() {
for route in _next_ifcfg.ipv4_route_owners.keys() {
ignore_added_ifcfg_already_exists(
nic.add_route_with_cost(route.address, route.prefix, route.cost)
.await,
)?;
}
}
#[cfg(target_os = "linux")]
if !delta.ipv6_addresses.removed.is_empty() {
for route in _next_ifcfg.ipv6_route_owners.keys() {
ignore_added_ifcfg_already_exists(
nic.add_ipv6_route_with_cost(route.address, route.prefix, route.cost)
.await,
)?;
}
}
Ok(())
}
@@ -614,6 +702,14 @@ fn ignore_removed_ifcfg_not_found(result: Result<(), Error>) -> Result<(), Error
}
}
#[cfg(target_os = "linux")]
fn ignore_added_ifcfg_already_exists(result: Result<(), Error>) -> Result<(), Error> {
match result {
Err(Error::IOError(err)) if err.kind() == std::io::ErrorKind::AlreadyExists => Ok(()),
other => other,
}
}
struct SharedVirtualNicMemberRegistration {
member_id: SharedVirtualNicMemberId,
registration_id: SharedVirtualNicMemberRegistrationId,
@@ -671,6 +767,7 @@ impl Drop for SharedVirtualNicMemberRegistration {
#[derive(Clone)]
pub struct SharedVirtualNicMember {
member_id: SharedVirtualNicMemberId,
configured_mtu: u32,
shared_nic: Arc<Mutex<SharedVirtualNic>>,
close_notifier: Arc<Notify>,
registration: Arc<SharedVirtualNicMemberRegistration>,
@@ -679,6 +776,7 @@ pub struct SharedVirtualNicMember {
impl SharedVirtualNicMember {
fn new(
member_id: SharedVirtualNicMemberId,
configured_mtu: u32,
shared_nic: Arc<Mutex<SharedVirtualNic>>,
close_notifier: Arc<Notify>,
member_tunnel_table: SharedVirtualNicMemberTunnelTable,
@@ -686,6 +784,7 @@ impl SharedVirtualNicMember {
let registration_id = uuid::Uuid::new_v4();
Self {
member_id,
configured_mtu,
shared_nic: shared_nic.clone(),
close_notifier,
registration: Arc::new(SharedVirtualNicMemberRegistration {
@@ -709,6 +808,11 @@ impl SharedVirtualNicMember {
self.close_notifier.clone()
}
#[cfg(test)]
fn configured_mtu_for_test(&self) -> u32 {
self.configured_mtu
}
pub async fn create_dev(&self) -> Result<Box<dyn Tunnel>, Error> {
let (member_tunnel, shared_tunnel) = create_ring_tunnel_pair();
{
@@ -717,6 +821,13 @@ impl SharedVirtualNicMember {
.attach_member_registration(self.member_id, self.registration.registration_id)
.await?;
shared_nic.ensure_dispatcher().await?;
shared_nic
.apply_member_mtu_for_registration(
self.member_id,
self.registration.registration_id,
self.configured_mtu,
)
.await?;
}
self.registration
.register_tunnel(shared_tunnel, self.close_notifier.clone())?;
@@ -735,6 +846,13 @@ impl SharedVirtualNicMember {
.attach_member_registration(self.member_id, self.registration.registration_id)
.await?;
shared_nic.ensure_dispatcher_for_mobile(tun_fd).await?;
shared_nic
.apply_member_mtu_for_mobile_registration(
self.member_id,
self.registration.registration_id,
self.configured_mtu,
)
.await?;
}
self.registration
.register_tunnel(shared_tunnel, self.close_notifier.clone())?;
@@ -898,7 +1016,22 @@ impl SharedVirtualNicMember {
#[derive(Default)]
pub struct SharedVirtualNicRegistry {
nics: BTreeMap<String, SharedVirtualNicRegistryEntry>,
nics: BTreeMap<SharedVirtualNicRegistryKey, SharedVirtualNicRegistryEntry>,
}
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
struct SharedVirtualNicRegistryKey {
net_ns: Option<String>,
dev_name: String,
}
impl SharedVirtualNicRegistryKey {
fn new(dev_name: String, config: &VirtualNicConfig) -> Self {
Self {
net_ns: config.net_ns_name(),
dev_name,
}
}
}
struct SharedVirtualNicRegistryEntry {
@@ -934,13 +1067,32 @@ impl SharedVirtualNicRegistry {
Self::default()
}
pub fn get(&self, dev_name: &str) -> Option<Arc<Mutex<SharedVirtualNic>>> {
pub fn get(
&self,
dev_name: &str,
config: &VirtualNicConfig,
) -> Option<Arc<Mutex<SharedVirtualNic>>> {
let key = SharedVirtualNicRegistryKey::new(dev_name.to_owned(), config);
self.nics
.get(dev_name)
.get(&key)
.filter(|entry| entry.is_valid())
.map(|entry| entry.nic())
}
#[cfg(test)]
pub fn get_by_dev_name_for_test(&self, dev_name: &str) -> Option<Arc<Mutex<SharedVirtualNic>>> {
let mut matches = self
.nics
.iter()
.filter(|(key, entry)| key.dev_name == dev_name && entry.is_valid())
.map(|(_, entry)| entry.nic());
let first = matches.next()?;
if matches.next().is_some() {
return None;
}
Some(first)
}
pub fn get_or_create(
&mut self,
dev_name: String,
@@ -954,17 +1106,15 @@ impl SharedVirtualNicRegistry {
dev_name: String,
config: VirtualNicConfig,
) -> &SharedVirtualNicRegistryEntry {
let needs_new_entry = self
.nics
.get(&dev_name)
.is_none_or(|entry| !entry.is_valid());
let key = SharedVirtualNicRegistryKey::new(dev_name, &config);
let needs_new_entry = self.nics.get(&key).is_none_or(|entry| !entry.is_valid());
if needs_new_entry {
let entry = SharedVirtualNicRegistryEntry::new(SharedVirtualNic::new(config));
self.nics.insert(dev_name.clone(), entry);
self.nics.insert(key.clone(), entry);
}
self.nics
.get(&dev_name)
.get(&key)
.expect("shared virtual nic registry entry should exist")
}
@@ -975,9 +1125,11 @@ impl SharedVirtualNicRegistry {
member_id: SharedVirtualNicMemberId,
close_notifier: Arc<Notify>,
) -> SharedVirtualNicMember {
let configured_mtu = config.mtu();
let entry = self.get_or_create_entry(dev_name, config);
SharedVirtualNicMember::new(
member_id,
configured_mtu,
entry.nic(),
close_notifier,
entry.member_tunnel_table(),
@@ -1114,11 +1266,20 @@ fn ipv6_inet(address: Ipv6Addr, prefix: i32) -> Result<Ipv6Inet, Error> {
mod tests {
use std::str::FromStr as _;
use crate::common::netns::NetNS;
use crate::common::{ifcfg::IfConfiguerTrait, netns::NetNS};
use tokio::sync::Notify;
use super::*;
struct FailingRemoveIpIfConfiger;
#[async_trait::async_trait]
impl IfConfiguerTrait for FailingRemoveIpIfConfiger {
async fn remove_ip(&self, _name: &str, _ip: Option<Ipv4Inet>) -> Result<(), Error> {
Err(anyhow::anyhow!("forced remove_ip failure").into())
}
}
fn member_id(n: u128) -> SharedVirtualNicMemberId {
uuid::Uuid::from_u128(n)
}
@@ -1135,6 +1296,14 @@ mod tests {
VirtualNicConfig::new(String::new(), 1500, NetNS::new(None))
}
fn virtual_nic_config_with_mtu(mtu: u32) -> VirtualNicConfig {
VirtualNicConfig::new(String::new(), mtu, NetNS::new(None))
}
fn virtual_nic_config_in_netns(net_ns: &str) -> VirtualNicConfig {
VirtualNicConfig::new(String::new(), 1500, NetNS::new(Some(net_ns.to_owned())))
}
#[test]
fn duplicate_routes_keep_owner_sets_and_single_os_delta() {
let route = SharedIpv4Route::new(Ipv4Addr::new(10, 10, 0, 0), 24, None);
@@ -1267,8 +1436,78 @@ mod tests {
);
}
#[tokio::test]
async fn failed_member_registration_cleanup_invalidates_shared_nic() {
let mut shared_nic = SharedVirtualNic::new(virtual_nic_config());
let member = member_id(1);
let registration = uuid::Uuid::from_u128(10);
let ip = Ipv4Inet::from_str("10.60.0.2/24").unwrap();
shared_nic.member_registrations.insert(member, registration);
shared_nic.ifcfg_mut().apply_member_claims(
member,
SharedIfConfigClaims {
ipv4_addresses: BTreeSet::from([ip]),
..Default::default()
},
);
let nic = shared_nic.nic();
let mut nic = nic.lock().await;
nic.set_ifname_for_test("et0".to_string());
nic.set_ifcfg_for_test(Box::new(FailingRemoveIpIfConfiger));
drop(nic);
let result = shared_nic
.remove_member_registration_claims(member, registration)
.await;
assert!(result.is_err());
assert!(!shared_nic.is_valid());
assert!(!shared_nic.member_registrations.contains_key(&member));
assert_eq!(
shared_nic.ifcfg().owners_of_ipv4_address(&ip),
BTreeSet::from([member])
);
}
#[tokio::test]
async fn failed_registration_replacement_keeps_old_registration_and_invalidates() {
let mut shared_nic = SharedVirtualNic::new(virtual_nic_config());
let member = member_id(1);
let old_registration = uuid::Uuid::from_u128(10);
let next_registration = uuid::Uuid::from_u128(11);
let ip = Ipv4Inet::from_str("10.70.0.2/24").unwrap();
shared_nic
.member_registrations
.insert(member, old_registration);
shared_nic.ifcfg_mut().apply_member_claims(
member,
SharedIfConfigClaims {
ipv4_addresses: BTreeSet::from([ip]),
..Default::default()
},
);
let nic = shared_nic.nic();
let mut nic = nic.lock().await;
nic.set_ifname_for_test("et0".to_string());
nic.set_ifcfg_for_test(Box::new(FailingRemoveIpIfConfiger));
drop(nic);
let result = shared_nic
.attach_member_registration(member, next_registration)
.await;
assert!(result.is_err());
assert!(!shared_nic.is_valid());
assert_eq!(
shared_nic.member_registrations.get(&member),
Some(&old_registration)
);
}
#[test]
fn registry_reuses_shared_virtual_nic_for_same_dev_name() {
fn registry_reuses_shared_virtual_nic_for_same_dev_name_and_netns() {
let mut registry = SharedVirtualNicRegistry::new();
let first = registry.get_or_create("et0".to_string(), virtual_nic_config());
@@ -1277,6 +1516,17 @@ mod tests {
assert!(Arc::ptr_eq(&first, &second));
}
#[test]
fn registry_keeps_same_dev_name_in_different_netns_separate() {
let mut registry = SharedVirtualNicRegistry::new();
let first = registry.get_or_create("et0".to_string(), virtual_nic_config_in_netns("net-a"));
let second =
registry.get_or_create("et0".to_string(), virtual_nic_config_in_netns("net-b"));
assert!(!Arc::ptr_eq(&first, &second));
}
#[test]
fn registry_keeps_different_dev_names_separate() {
let mut registry = SharedVirtualNicRegistry::new();
@@ -1298,7 +1548,7 @@ mod tests {
assert!(!Arc::ptr_eq(&first, &second));
assert!(
registry
.get("et0")
.get("et0", &virtual_nic_config())
.is_some_and(|nic| Arc::ptr_eq(&nic, &second))
);
}
@@ -1314,9 +1564,31 @@ mod tests {
member_id,
Arc::new(Notify::new()),
);
let shared_nic = registry.get("et0").unwrap();
let shared_nic = registry.get("et0", &virtual_nic_config()).unwrap();
assert_eq!(member.member_id(), member_id);
assert!(Arc::ptr_eq(&member.shared_nic(), &shared_nic));
}
#[test]
fn registry_create_member_keeps_member_configured_mtu() {
let mut registry = SharedVirtualNicRegistry::new();
let first = registry.create_member(
"et0".to_string(),
virtual_nic_config_with_mtu(1400),
member_id(1),
Arc::new(Notify::new()),
);
let second = registry.create_member(
"et0".to_string(),
virtual_nic_config_with_mtu(1300),
member_id(2),
Arc::new(Notify::new()),
);
assert_eq!(first.configured_mtu_for_test(), 1400);
assert_eq!(second.configured_mtu_for_test(), 1300);
assert!(Arc::ptr_eq(&first.shared_nic(), &second.shared_nic()));
}
}
+46
View File
@@ -260,6 +260,14 @@ impl VirtualNicConfig {
net_ns,
}
}
pub fn mtu(&self) -> u32 {
self.mtu
}
pub fn net_ns_name(&self) -> Option<String> {
self.net_ns.name()
}
}
pub struct VirtualNic {
@@ -836,6 +844,19 @@ impl VirtualNic {
self.config.mtu
}
#[cfg(test)]
pub(crate) fn set_ifname_for_test(&mut self, ifname: String) {
self.ifname = Some(ifname);
}
#[cfg(test)]
pub(crate) fn set_ifcfg_for_test(
&mut self,
ifcfg: Box<dyn IfConfiguerTrait + Send + Sync + 'static>,
) {
self.ifcfg = ifcfg;
}
pub fn get_ifcfg(&self) -> IfConfiger {
IfConfiger::default()
}
@@ -926,6 +947,23 @@ impl NicBackend {
}
}
pub async fn add_route_with_cost(
&self,
address: Ipv4Addr,
cidr: u8,
cost: Option<i32>,
) -> Result<(), Error> {
match self {
Self::Dedicated(nic) => {
nic.lock()
.await
.add_route_with_cost(address, cidr, cost)
.await
}
Self::Shared(member) => member.add_route_with_cost(address, cidr, cost).await,
}
}
pub async fn remove_route(&self, address: Ipv4Addr, cidr: u8) -> Result<(), Error> {
match self {
Self::Dedicated(nic) => nic.lock().await.remove_route(address, cidr).await,
@@ -1068,6 +1106,14 @@ impl NicCtx {
)
}
#[cfg(feature = "magic-dns")]
pub(crate) fn shared_route_backend_for_dns(&self) -> Option<NicBackend> {
match self.backend {
NicBackend::Dedicated(_) => None,
NicBackend::Shared(_) => Some(self.backend.clone()),
}
}
fn dedicated_backend(global_ctx: &ArcGlobalCtx) -> NicBackend {
let nic_config = Self::virtual_nic_config(global_ctx);
NicBackend::dedicated(Arc::new(Mutex::new(VirtualNic::new(nic_config))))
+19 -2
View File
@@ -1343,10 +1343,27 @@ pub async fn shared_tun_magic_dns_same_namespace_real_tun() {
)
.await;
drop_insts(vec![center, shared_1, shared_2, remote]).await;
drop_insts(vec![shared_1]).await;
let routes_after_first_drop = run_ip_in_ns_output("net_b", &["route", "show"]);
assert!(
ipv4_route_exists_in_ns("net_b", &format!("{MAGIC_DNS_FAKE_IP} dev {dev_name}")),
"magic dns fake-ip route should remain while another shared tun member owns it: {routes_after_first_drop}"
);
wait_for_condition(
|| async { magic_dns_record_matches("net_b", "shared-dns-2", "10.144.255.3").await },
Duration::from_secs(8),
)
.await;
wait_for_condition(
|| async { magic_dns_record_matches("net_b", "shared-dns-remote", "10.144.255.4").await },
Duration::from_secs(8),
)
.await;
drop_insts(vec![center, shared_2, remote]).await;
assert!(
!ipv4_route_exists_in_ns("net_b", &format!("{MAGIC_DNS_FAKE_IP} dev {dev_name}")),
"magic dns fake-ip route should be removed with the shared tun member"
"magic dns fake-ip route should be removed with the last shared tun member"
);
}