network: don't block icmp proxies on ping awaits

network/src/backend.rs

@@ -8,7 +8,7 @@ use crate::vbridge::{BridgeJoinHandle, VirtualBridge};
 use anyhow::{anyhow, Result};
 use etherparse::SlicedPacket;
 use futures::TryStreamExt;
-use log::{debug, info, warn};
+use log::{debug, info, trace, warn};
 use smoltcp::iface::{Config, Interface, SocketSet};
 use smoltcp::phy::Medium;
 use smoltcp::time::Instant;
@@ -26,7 +26,6 @@ pub struct NetworkBackend {
 
 enum NetworkStackSelect<'a> {
     Receive(&'a [u8]),
-    BridgeSend(Option<Vec<u8>>),
     Send(Option<Vec<u8>>),
     Reclaim,
 }
@@ -46,8 +45,8 @@ impl NetworkStack<'_> {
     async fn poll(&mut self, buffer: &mut [u8]) -> Result<()> {
         let what = select! {
             x = self.kdev.read(buffer) => NetworkStackSelect::Receive(&buffer[0..x?]),
-            x = self.bridge.bridge_rx_receiver.recv() => NetworkStackSelect::BridgeSend(x),
-            x = self.bridge.broadcast_rx_receiver.recv() => NetworkStackSelect::BridgeSend(x.ok()),
+            x = self.bridge.bridge_rx_receiver.recv() => NetworkStackSelect::Send(x),
+            x = self.bridge.broadcast_rx_receiver.recv() => NetworkStackSelect::Send(x.ok()),
             x = self.tx.recv() => NetworkStackSelect::Send(x),
             _ = self.router.process_reclaim() => NetworkStackSelect::Reclaim,
         };
@@ -55,7 +54,7 @@ impl NetworkStack<'_> {
         match what {
             NetworkStackSelect::Receive(packet) => {
                 if let Err(error) = self.bridge.bridge_tx_sender.try_send(packet.to_vec()) {
-                    warn!("failed to send guest packet to bridge: {}", error);
+                    trace!("failed to send guest packet to bridge: {}", error);
                 }
 
                 let slice = SlicedPacket::from_ethernet(packet)?;
@@ -69,19 +68,9 @@ impl NetworkStack<'_> {
                     .poll(Instant::now(), &mut self.udev, &mut self.sockets);
             }
 
-            NetworkStackSelect::BridgeSend(Some(packet)) => {
-                if let Err(error) = self.udev.tx.try_send(packet) {
-                    warn!("failed to send bridge packet to guest: {}", error);
-                }
-            }
+            NetworkStackSelect::Send(Some(packet)) => self.kdev.write_all(&packet).await?,
 
-            NetworkStackSelect::BridgeSend(None) => {}
-
-            NetworkStackSelect::Send(packet) => {
-                if let Some(packet) = packet {
-                    self.kdev.write_all(&packet).await?
-                }
-            }
+            NetworkStackSelect::Send(None) => {}
 
             NetworkStackSelect::Reclaim => {}
         }

network/src/icmp.rs

@@ -39,13 +39,13 @@ struct IcmpHandlerToken(IpAddr, Option<u16>, u16);
 
 #[derive(Debug)]
 pub enum IcmpReply {
-    Icmp4 {
+    Icmpv4 {
         header: Icmpv4Header,
         echo: IcmpEchoHeader,
         payload: Vec<u8>,
     },
 
-    Icmp6 {
+    Icmpv6 {
         header: Icmpv6Header,
         echo: IcmpEchoHeader,
         payload: Vec<u8>,
@@ -53,6 +53,8 @@ pub enum IcmpReply {
 }
 
 type IcmpHandlerMap = Arc<Mutex<HashMap<IcmpHandlerToken, oneshot::Sender<IcmpReply>>>>;
+
+#[derive(Clone)]
 pub struct IcmpClient {
     socket: Arc<UdpSocket>,
     handlers: IcmpHandlerMap,
@@ -118,7 +120,7 @@ impl IcmpClient {
                         Some(echo.id),
                         echo.seq,
                     );
-                    let reply = IcmpReply::Icmp4 {
+                    let reply = IcmpReply::Icmpv4 {
                         header: icmpv4.header(),
                         echo,
                         payload: icmpv4.payload().to_vec(),
@@ -141,7 +143,7 @@ impl IcmpClient {
 
                     let token = IcmpHandlerToken(IpAddr::V6(*addr.ip()), Some(echo.id), echo.seq);
 
-                    let reply = IcmpReply::Icmp6 {
+                    let reply = IcmpReply::Icmpv6 {
                         header: icmpv6.header(),
                         echo,
                         payload: icmpv6.payload().to_vec(),

network/src/nat.rs

@@ -50,7 +50,7 @@ impl Display for NatKey {
     }
 }
 
-#[derive(Debug)]
+#[derive(Debug, Clone)]
 pub struct NatHandlerContext {
     pub mtu: usize,
     pub key: NatKey,

network/src/proxynat/icmp.rs

@@ -1,12 +1,15 @@
-use std::{net::IpAddr, time::Duration};
+use std::{
+    net::{IpAddr, Ipv4Addr, Ipv6Addr},
+    time::Duration,
+};
 
 use anyhow::{anyhow, Result};
 use async_trait::async_trait;
 use etherparse::{
-    Icmpv4Header, Icmpv4Type, Icmpv6Header, Icmpv6Type, IpNumber, Ipv4Slice, Ipv6Slice, NetSlice,
-    PacketBuilder, SlicedPacket,
+    IcmpEchoHeader, Icmpv4Header, Icmpv4Type, Icmpv6Header, Icmpv6Type, IpNumber, Ipv4Slice,
+    Ipv6Slice, NetSlice, PacketBuilder, SlicedPacket,
 };
-use log::{debug, warn};
+use log::{debug, trace, warn};
 use smoltcp::wire::IpAddress;
 use tokio::{
     select,
@@ -104,6 +107,8 @@ impl ProxyIcmpHandler {
             }
         }
 
+        context.reclaim().await?;
+
         Ok(())
     }
 
@@ -122,39 +127,22 @@ impl ProxyIcmpHandler {
                 return Ok(());
             };
 
-            let Some(IcmpReply::Icmp4 {
-                header: _,
-                echo,
-                payload,
-            }) = client
-                .ping4(
+            let context = context.clone();
+            let client = client.clone();
+            let payload = payload.to_vec();
+            tokio::task::spawn(async move {
+                if let Err(error) = ProxyIcmpHandler::process_echo_ipv4(
+                    context,
+                    client,
                     external_ipv4,
-                    echo.id,
-                    echo.seq,
+                    echo,
                     payload,
-                    Duration::from_secs(ICMP_PING_TIMEOUT_SECS),
                 )
-                .await?
-            else {
-                return Ok(());
-            };
-
-            let packet =
-                PacketBuilder::ethernet2(context.key.local_mac.0, context.key.client_mac.0);
-            let packet = match (context.key.external_ip.addr, context.key.client_ip.addr) {
-                (IpAddress::Ipv4(external_addr), IpAddress::Ipv4(client_addr)) => {
-                    packet.ipv4(external_addr.0, client_addr.0, 20)
+                .await
+                {
+                    trace!("icmp4 echo failed: {}", error);
                 }
-                _ => {
-                    return Err(anyhow!("IP endpoint mismatch"));
-                }
-            };
-            let packet = packet.icmpv4_echo_reply(echo.id, echo.seq);
-            let mut buffer: Vec<u8> = Vec::new();
-            packet.write(&mut buffer, &payload)?;
-            if let Err(error) = context.try_send(buffer) {
-                debug!("failed to transmit icmp packet: {}", error);
-            }
+            });
         }
         Ok(())
     }
@@ -174,43 +162,112 @@ impl ProxyIcmpHandler {
                 return Ok(());
             };
 
-            let Some(IcmpReply::Icmp6 {
-                header: _,
-                echo,
-                payload,
-            }) = client
-                .ping6(
+            let context = context.clone();
+            let client = client.clone();
+            let payload = payload.to_vec();
+            tokio::task::spawn(async move {
+                if let Err(error) = ProxyIcmpHandler::process_echo_ipv6(
+                    context,
+                    client,
                     external_ipv6,
-                    echo.id,
-                    echo.seq,
+                    echo,
                     payload,
-                    Duration::from_secs(ICMP_PING_TIMEOUT_SECS),
                 )
-                .await?
-            else {
-                return Ok(());
-            };
-
-            let packet =
-                PacketBuilder::ethernet2(context.key.local_mac.0, context.key.client_mac.0);
-            let packet = match (context.key.external_ip.addr, context.key.client_ip.addr) {
-                (IpAddress::Ipv6(external_addr), IpAddress::Ipv6(client_addr)) => {
-                    packet.ipv6(external_addr.0, client_addr.0, 20)
+                .await
+                {
+                    trace!("icmp6 echo failed: {}", error);
                 }
-                _ => {
-                    return Err(anyhow!("IP endpoint mismatch"));
-                }
-            };
-            let packet = packet.icmpv6_echo_reply(echo.id, echo.seq);
-            let mut buffer: Vec<u8> = Vec::new();
-            packet.write(&mut buffer, &payload)?;
-            if let Err(error) = context.try_send(buffer) {
-                debug!("failed to transmit icmp packet: {}", error);
-            }
+            });
         }
 
         context.reclaim().await?;
 
         Ok(())
     }
+
+    async fn process_echo_ipv4(
+        context: NatHandlerContext,
+        client: IcmpClient,
+        external_ipv4: Ipv4Addr,
+        echo: IcmpEchoHeader,
+        payload: Vec<u8>,
+    ) -> Result<()> {
+        let reply = client
+            .ping4(
+                external_ipv4,
+                echo.id,
+                echo.seq,
+                &payload,
+                Duration::from_secs(ICMP_PING_TIMEOUT_SECS),
+            )
+            .await?;
+        let Some(IcmpReply::Icmpv4 {
+            header: _,
+            echo,
+            payload,
+        }) = reply
+        else {
+            return Ok(());
+        };
+
+        let packet = PacketBuilder::ethernet2(context.key.local_mac.0, context.key.client_mac.0);
+        let packet = match (context.key.external_ip.addr, context.key.client_ip.addr) {
+            (IpAddress::Ipv4(external_addr), IpAddress::Ipv4(client_addr)) => {
+                packet.ipv4(external_addr.0, client_addr.0, 20)
+            }
+            _ => {
+                return Err(anyhow!("IP endpoint mismatch"));
+            }
+        };
+        let packet = packet.icmpv4_echo_reply(echo.id, echo.seq);
+        let mut buffer: Vec<u8> = Vec::new();
+        packet.write(&mut buffer, &payload)?;
+        if let Err(error) = context.try_send(buffer) {
+            debug!("failed to transmit icmp packet: {}", error);
+        }
+        Ok(())
+    }
+
+    async fn process_echo_ipv6(
+        context: NatHandlerContext,
+        client: IcmpClient,
+        external_ipv6: Ipv6Addr,
+        echo: IcmpEchoHeader,
+        payload: Vec<u8>,
+    ) -> Result<()> {
+        let reply = client
+            .ping6(
+                external_ipv6,
+                echo.id,
+                echo.seq,
+                &payload,
+                Duration::from_secs(ICMP_PING_TIMEOUT_SECS),
+            )
+            .await?;
+        let Some(IcmpReply::Icmpv6 {
+            header: _,
+            echo,
+            payload,
+        }) = reply
+        else {
+            return Ok(());
+        };
+
+        let packet = PacketBuilder::ethernet2(context.key.local_mac.0, context.key.client_mac.0);
+        let packet = match (context.key.external_ip.addr, context.key.client_ip.addr) {
+            (IpAddress::Ipv6(external_addr), IpAddress::Ipv6(client_addr)) => {
+                packet.ipv6(external_addr.0, client_addr.0, 20)
+            }
+            _ => {
+                return Err(anyhow!("IP endpoint mismatch"));
+            }
+        };
+        let packet = packet.icmpv6_echo_reply(echo.id, echo.seq);
+        let mut buffer: Vec<u8> = Vec::new();
+        packet.write(&mut buffer, &payload)?;
+        if let Err(error) = context.try_send(buffer) {
+            debug!("failed to transmit icmp packet: {}", error);
+        }
+        Ok(())
+    }
 }