SNP Integration Test: Improve Clarity

uefi-test-runner/src/proto/network/mod.rs

@@ -4,10 +4,18 @@ pub fn test() {
     info!("Testing Network protocols");
 
     http::test();
-    pxe::test();
-    snp::test();
+    #[cfg(feature = "pxe")]
+    {
+        pxe::test();
+        // Currently, we are in the unfortunate situation that the SNP test
+        // depends on the PXE test, as it assigns an IPv4 address to the
+        // interface.
+        snp::test();
+    }
 }
 
 mod http;
+#[cfg(feature = "pxe")]
 mod pxe;
+#[cfg(feature = "pxe")]
 mod snp;

uefi-test-runner/src/proto/network/pxe.rs

@@ -5,11 +5,6 @@ use uefi::proto::network::pxe::{BaseCode, DhcpV4Packet, IpFilter, IpFilters, Udp
 use uefi::{CStr8, boot};
 
 pub fn test() {
-    // Skip the test if the `pxe` feature is not enabled.
-    if cfg!(not(feature = "pxe")) {
-        return;
-    }
-
     info!("Testing The PXE base code protocol");
 
     let handles = boot::find_handles::<BaseCode>().expect("failed to get PXE base code handles");

uefi-test-runner/src/proto/network/snp.rs

@@ -1,40 +1,82 @@
 // SPDX-License-Identifier: MIT OR Apache-2.0
 
-use core::time::Duration;
-
+use core::ops::DerefMut;
+use uefi::proto::device_path::DevicePath;
+use uefi::proto::device_path::text::{AllowShortcuts, DisplayOnly};
 use uefi::proto::network::MacAddress;
-use uefi::proto::network::snp::{InterruptStatus, ReceiveFlags, SimpleNetwork};
+use uefi::proto::network::snp::{InterruptStatus, NetworkState, ReceiveFlags, SimpleNetwork};
 use uefi::{Status, boot};
 
+const ETHERNET_PROTOCOL_IPV4: u16 = 0x0800;
+
+/// Receives the next IPv4 packet and prints corresponding metadata.
+fn receive(simple_network: &mut SimpleNetwork, buffer: &mut [u8]) -> uefi::Result<usize> {
+    let mut recv_src_mac = MacAddress([0; 32]);
+    let mut recv_dst_mac = MacAddress([0; 32]);
+    let mut recv_ethernet_protocol = 0;
+
+    let res = simple_network.receive(
+        buffer,
+        None,
+        Some(&mut recv_src_mac),
+        Some(&mut recv_dst_mac),
+        Some(&mut recv_ethernet_protocol),
+    );
+
+    res.inspect(|_| {
+        debug!("Received:");
+        debug!("  src_mac       =  {:x?}", recv_src_mac);
+        debug!("  dst_mac       =  {:x?}", recv_dst_mac);
+        debug!("  ethernet_proto=0x{:x?}", recv_ethernet_protocol);
+
+        // Ensure that we do not accidentally get an ARP packet, which we
+        // do not expect in this test.
+        assert_eq!(recv_ethernet_protocol, ETHERNET_PROTOCOL_IPV4);
+    })
+}
+
+/// This test sends a simple UDP/IP packet to the `EchoService` (created by
+/// `cargo xtask run`) and receives its message.
 pub fn test() {
     info!("Testing the simple network protocol");
 
     let handles = boot::find_handles::<SimpleNetwork>().unwrap_or_default();
-
     for handle in handles {
-        let simple_network = boot::open_protocol_exclusive::<SimpleNetwork>(handle);
-        if simple_network.is_err() {
+        let Ok(mut simple_network) = boot::open_protocol_exclusive::<SimpleNetwork>(handle) else {
             continue;
+        };
+        // Print device path
+        {
+            let simple_network_dvp = boot::open_protocol_exclusive::<DevicePath>(handle)
+                .expect("Should have device path");
+            log::info!(
+                "Network interface: {}",
+                simple_network_dvp
+                    .to_string(DisplayOnly(true), AllowShortcuts(true))
+                    .unwrap()
+            );
         }
-        let simple_network = simple_network.unwrap();
 
-        // Check shutdown
-        let res = simple_network.shutdown();
-        assert!(res == Ok(()) || res == Err(Status::NOT_STARTED.into()));
+        assert_eq!(
+            simple_network.mode().state,
+            NetworkState::STOPPED,
+            "Should be in stopped state"
+        );
 
-        // Check stop
-        let res = simple_network.stop();
-        assert!(res == Ok(()) || res == Err(Status::NOT_STARTED.into()));
+        // Check media
+        if !bool::from(simple_network.mode().media_present_supported)
+            || !bool::from(simple_network.mode().media_present)
+        {
+            continue;
+        }
 
-        // Check start
         simple_network
             .start()
-            .expect("Failed to start Simple Network");
+            .expect("Network should not be started yet");
 
-        // Check initialize
         simple_network
             .initialize(0, 0)
-            .expect("Failed to initialize Simple Network");
+            .expect("Network should not be initialized yet");
 
         // edk2 virtio-net driver does not support statistics, so
         // allow UNSUPPORTED (same for collect_statistics below).
@@ -54,13 +96,12 @@ pub fn test() {
             )
             .expect("Failed to set receive filters");
 
-        // Check media
-        if !bool::from(simple_network.mode().media_present_supported)
-            || !bool::from(simple_network.mode().media_present)
-        {
-            continue;
-        }
-
+        // EthernetFrame(IPv4Packet(UDPPacket(Payload))).
+        // The ethernet frame header will be filled by `transmit()`.
+        // The UDP packet contains the byte sequence `4, 4, 3, 2, 1`.
+        //
+        // The packet is sent to the `EchoService` created by
+        // `cargo xtask run`. It runs on UDP port 21572.
         let payload = b"\0\0\0\0\0\0\0\0\0\0\0\0\0\0\
             \x45\x00\
             \x00\x21\
@@ -77,7 +118,6 @@ pub fn test() {
             \xa9\xe4\
             \x04\x01\x02\x03\x04";
 
-        let dest_addr = MacAddress([0xffu8; 32]);
         assert!(
             !simple_network
                 .get_interrupt_status()
@@ -91,8 +131,8 @@ pub fn test() {
                 simple_network.mode().media_header_size as usize,
                 payload,
                 None,
-                Some(dest_addr),
-                Some(0x0800),
+                Some(simple_network.mode().broadcast_address),
+                Some(ETHERNET_PROTOCOL_IPV4),
             )
             .expect("Failed to transmit frame");
 
@@ -107,16 +147,10 @@ pub fn test() {
         let mut buffer = [0u8; 1500];
 
         info!("Waiting for the reception");
-        if simple_network.receive(&mut buffer, None, None, None, None)
-            == Err(Status::NOT_READY.into())
-        {
-            boot::stall(Duration::from_secs(1));
-
-            simple_network
-                .receive(&mut buffer, None, None, None, None)
-                .unwrap();
-        }
+        let n = receive(simple_network.deref_mut(), &mut buffer).unwrap();
+        debug!("Reply has {n} bytes");
 
+        // Check payload in UDP packet that was reversed by our EchoService.
         assert_eq!(buffer[42..47], [4, 4, 3, 2, 1]);
 
         // Get stats
@@ -137,5 +171,7 @@ pub fn test() {
                 }
             }
         }
+
+        simple_network.shutdown().unwrap();
     }
 }

uefi/src/proto/network/snp.rs

@@ -269,7 +269,7 @@ impl SimpleNetwork {
         unsafe { &*(ptr::from_ref(&self.0.wait_for_packet).cast::<Event>()) }
     }
 
-    /// Returns a reference to the Simple Network mode.
+    /// Returns a reference to the [`NetworkMode`].
     #[must_use]
     pub fn mode(&self) -> &NetworkMode {
         unsafe { &*self.0.mode }

xtask/src/qemu.rs

@@ -499,7 +499,7 @@ pub fn run_qemu(arch: UefiArch, opt: &QemuOpt) -> Result<()> {
             "-netdev",
             "user,id=net0,net=192.168.17.0/24,tftp=uefi-test-runner/tftp/,bootfile=fake-boot-file",
             "-device",
-            "virtio-net-pci,netdev=net0",
+            "virtio-net-pci,netdev=net0,mac=52:54:00:00:00:01",
         ]);
         Some(net::EchoService::start())
     } else {