[wlan-ffi-transport][wlan-mlme] FrameSender,FrameProcessor -> EthernetRx/Tx WlanRx/Tx

This change splits FrameSender into EthernetRx and WlanTx, and
FrameProcessor into EthernetTx and WlanRx and moves each to the
wlan-ffi-transport crate. By splitting the protocols, this change
positions wlansoftmac to implement a NetworkBridge that handles
EthernetImpl and EthernetIfc messages similar to how SoftmacBridge
handles WlanSoftmac and WlanSoftmacIfc messages.

This is last of the most recent series of changes to the FFI surface
between the Rust and C++ portions of the wlansoftmac driver. The
remaining portions of the FFI are more or less permanent in the sense
that fuchsia.wlan.softmac API changes will not in general requires
changes in the FFI.

Thank you to kiettran@, seanolson@, swiggett@, and johntan@ for your
numerous reviews, input, and changes to conclude the migration of
wlansoftmac off Banjo bindings and onto this more stable FFI surface.

Test: Successful iface creation, scan, connect, disconnect with iwlwifi on NUC
Multiply: fuchsia-pkg://fuchsia.com/wlan-mlme-tests
Multiply: fuchsia-pkg://fuchsia.com/wlansoftmac-rust-tests
Multiply: fuchsia-pkg://fuchsia.com/wlan-hw-sim-contemporary-privacy-tests
Multiply: fuchsia-pkg://fuchsia.com/wlan-hw-sim-legacy-privacy-tests
Multiply: fuchsia-pkg://fuchsia.com/wlan-hw-sim-rate-selection-tests
Change-Id: I43055757f5deb7c884820d07eeabddbfb40e5aec
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1039994
Reviewed-by: Dylan Swiggett <swiggett@google.com>
Commit-Queue: Auto-Submit <auto-submit@fuchsia-infra.iam.gserviceaccount.com>
Reviewed-by: Rebecca Silberstein <silberst@google.com>
Fuchsia-Auto-Submit: Charles Celerier <chcl@google.com>
API-Review: Rebecca Silberstein <silberst@google.com>

Author: c6c7

sdk/fidl/fuchsia.wlan.softmac/fuchsia.wlan.softmac.api

@@ -1,3 +1,3 @@
 {
-  "fidl/fuchsia.wlan.softmac": "ea2fbf40da8b4ae8adbe4b09eb857544"
+  "fidl/fuchsia.wlan.softmac": "ebf4cf8da7cb019a6e9870094d28abe6"
 }

sdk/fidl/fuchsia.wlan.softmac/softmac.fidl

@@ -356,62 +356,73 @@ closed protocol WlanSoftmacIfcBridge {
     strict StopBridgedDriver() -> ();
 };
 
+/// Protocol for sending an Ethernet frame from the bridged wlansoftmac
+/// driver to the wlansoftmac driver.
+///
+/// # Experimental
+///
+/// This protocol is implemented as a foreign function interface (FFI)
+/// between the wlansoftmac driver and the bridged driver solely to improve
+/// the performance of processing data frames through the wlan-mlme library.
+@available(added=HEAD)
+closed protocol EthernetRx {
+    strict Transfer(table {
+        1: packet_address uint64;
+        2: packet_size uint64;
+    }) -> () error zx.Status;
+};
 
-/// Protocol for sending Ethernet and WLAN frames from the wlansoftmac driver to the bridged
+/// Protocol for sending an Ethernet frame from the wlansoftmac driver to the bridged
 /// wlansoftmac driver.
 ///
 /// # Experimental
 ///
 /// This protocol is implemented as a foreign function interface (FFI)
 /// between the wlansoftmac driver and the bridged driver solely to improve
 /// the performance of processing data frames through the wlan-mlme library.
+@available(added=HEAD)
+closed protocol EthernetTx {
+    strict Transfer(table {
+        1: packet_address uint64;
+        2: packet_size uint64;
+        3: async_id uint64;
+    }) -> () error zx.Status;
+};
+
+/// Protocol for sending a WLAN frame from the wlansoftmac driver to the bridged
+/// wlansoftmac driver.
+///
+/// # Experimental
 ///
-/// By using an FFI, messages through this protocol never enter a FIDL
-/// channel and thus do not incur the cost of the associated system calls
-/// Empirically, we observed implementing this protocol as an FFI saves on
-/// the order of ~500µs per frame processed when compared to implementing
-/// this protocol with a FIDL channel.
+/// This protocol is implemented as a foreign function interface (FFI)
+/// between the wlansoftmac driver and the bridged driver solely to improve
+/// the performance of processing data frames through the wlan-mlme library.
 @available(added=HEAD)
-closed protocol FrameProcessor {
-    strict WlanRx(table {
+closed protocol WlanRx {
+    strict Transfer(table {
         1: packet_address uint64;
         2: packet_size uint64;
         3: packet_info WlanRxInfo;
         4: async_id uint64;
     }) -> ();
-    strict EthernetTx(table {
-        1: packet_address uint64;
-        2: packet_size uint64;
-        3: async_id uint64;
-    }) -> () error zx.Status;
 };
 
-/// Protocol for sending Ethernet and WLAN frames from the bridged wlansoftmac
+/// Protocol for sending a WLAN frame from the bridged wlansoftmac
 /// driver to the wlansoftmac driver.
 ///
 /// # Experimental
 ///
 /// This protocol is implemented as a foreign function interface (FFI)
 /// between the wlansoftmac driver and the bridged driver solely to improve
 /// the performance of processing data frames through the wlan-mlme library.
-///
-/// By using an FFI, messages through this protocol never enter a FIDL
-/// channel and thus do not incur the cost of the associated system calls
-/// Empirically, we observed implementing this protocol as an FFI saves on
-/// the order of ~500µs per frame processed when compared to implementing
-/// this protocol with a FIDL channel.
 @available(added=HEAD)
-closed protocol FrameSender {
-    strict WlanTx(table {
+closed protocol WlanTx {
+    strict Transfer(table {
         1: packet_address uint64;
         2: packet_size uint64;
         3: packet_info WlanTxInfo;
         4: async_id uint64;
     }) -> () error zx.Status;
-    strict EthernetRx(table {
-        1: packet_address uint64;
-        2: packet_size uint64;
-    }) -> () error zx.Status;
 };
 
 @discoverable
@@ -816,13 +827,16 @@ closed protocol WlanSoftmacBridge {
     ///   - `ifc_bridge`: The client end of a `WlanSoftmacIfcBridge` server which the
     ///     `wlansoftmac` driver will use to forward `WlanSoftmacIfc` events to
     ///     the bridged driver.
-    ///   - `frame_processor`: A `frame_processor_t*` casted to a `uint64`. The
-    ///     `frame_processor_t` is defined in
+    ///   - `ethernet_tx`: A `ethernet_tx_t*` casted to a `uint64`. The
+    ///     `ethernet_tx_t` is defined in
+    ///     `//src/connectivity/wlan/drivers/wlansoftmac/rust_driver/c-binding/bindings.h`.
+    ///   - `wlan_rx`: A `wlan_rx_t*` casted to a `uint64`. The
+    ///     `wlan_rx_t` is defined in
     ///     `//src/connectivity/wlan/drivers/wlansoftmac/rust_driver/c-binding/bindings.h`.
     ///
-    /// The server must copy the contents of the `frame_processor_t` before
-    /// returning from this method. The lifetime of the `frame_processor_t*` is only as
-    /// long as this method call, but the contents of the `frame_processor_t` will
+    /// The server must copy the contents of `ethernet_tx_t` and `wlan_rx_t` before
+    /// returning from this method. The lifetimes of `ethernet_tx_t*` and `wlan_rx_t*` are only as
+    /// long as this method call, but the contents of `ethernet_tx_t` and `wlan_rx_t` will
     /// live until the server stops the MLME.
     ///
     /// The server returns a server end of a `fuchsia.wlan.mlme/MLME` protocol. The SME
@@ -834,8 +848,8 @@ closed protocol WlanSoftmacBridge {
     /// protocols, so `WlanSoftmacIfcBridge` protocol must be Zircon transported to be
     /// usable by the bridged driver. Second, the Zircon transport adds
     /// significant latency compared to the Driver transport. As a result, the
-    /// `frame_processor` argument provides an FFI for the wlansoftmac driver
-    /// to send Ethernet and WLAN packets to the bridged driver with
+    /// `ethernet_tx` and `wlan_rx` arguments provide an FFI for the wlansoftmac driver
+    /// to send Ethernet and receive WLAN packets to the bridged driver with
     /// latency comparable or better than a Driver transported protocol.
     ///
     /// Except where noted, `WlanSoftmacBridge` methods must only be called after
@@ -847,7 +861,8 @@ closed protocol WlanSoftmacBridge {
     @available(added=HEAD)
     strict Start(resource struct {
         ifc_bridge client_end:WlanSoftmacIfcBridge;
-        frame_processor uint64;
+        ethernet_tx uint64;
+        wlan_rx uint64;
     }) -> (resource struct {
         sme_channel zx.Handle:CHANNEL;
     }) error zx.Status;

src/connectivity/wlan/drivers/wlansoftmac/rust_driver/c-binding/bindings.h

@@ -18,24 +18,28 @@
 typedef struct {
   void *ctx;
   /**
-   * Sends a WLAN MAC frame to the C++ portion of wlansoftmac.
+   * Sends an Ethernet frame to the C++ portion of wlansoftmac.
    *
    * # Safety
    *
-   * Behavior is undefined unless `payload` contains a persisted `FrameSender.WlanTx` request
+   * Behavior is undefined unless `payload` contains a persisted `EthernetRx.Transfer` request
    * and `payload_len` is the length of the persisted byte array.
    */
-  zx_status_t (*wlan_tx)(void *ctx, const uint8_t *payload, uintptr_t payload_len);
+  zx_status_t (*transfer)(void *ctx, const uint8_t *payload, uintptr_t payload_len);
+} ethernet_rx_t;
+
+typedef struct {
+  void *ctx;
   /**
-   * Sends an Ethernet frame to the C++ portion of wlansoftmac.
+   * Sends a WLAN MAC frame to the C++ portion of wlansoftmac.
    *
    * # Safety
    *
-   * Behavior is undefined unless `payload` contains a persisted `FrameSender.EthernetRx` request
+   * Behavior is undefined unless `payload` contains a persisted `WlanTx.Transfer` request
    * and `payload_len` is the length of the persisted byte array.
    */
-  zx_status_t (*ethernet_rx)(void *ctx, const uint8_t *payload, uintptr_t payload_len);
-} frame_sender_t;
+  zx_status_t (*transfer)(void *ctx, const uint8_t *payload, uintptr_t payload_len);
+} wlan_tx_t;
 
 /**
  * Type that wraps a pointer to a buffer allocated in the C++ portion of wlansoftmac.
@@ -87,9 +91,13 @@ typedef struct {
 
 typedef struct {
   const void *ctx;
-  void (*wlan_rx)(const void *ctx, const uint8_t *request, uintptr_t request_size);
-  zx_status_t (*ethernet_tx)(const void *ctx, const uint8_t *request, uintptr_t request_size);
-} frame_processor_t;
+  zx_status_t (*transfer)(const void *ctx, const uint8_t *request, uintptr_t request_size);
+} ethernet_tx_t;
+
+typedef struct {
+  const void *ctx;
+  void (*transfer)(const void *ctx, const uint8_t *request, uintptr_t request_size);
+} wlan_rx_t;
 
 /**
  * Start and run a bridged wlansoftmac driver hosting an MLME server and an SME server.
@@ -132,7 +140,7 @@ typedef struct {
  */
 extern "C" zx_status_t start_and_run_bridged_wlansoftmac(
     void *init_completer, void (*run_init_completer)(void *init_completer, zx_status_t status),
-    frame_sender_t frame_sender, wlansoftmac_buffer_provider_ops_t buffer_provider,
+    ethernet_rx_t ethernet_rx, wlan_tx_t wlan_tx, wlansoftmac_buffer_provider_ops_t buffer_provider,
     zx_handle_t wlan_softmac_bridge_client_handle);
 
 #endif  // SRC_CONNECTIVITY_WLAN_DRIVERS_WLANSOFTMAC_RUST_DRIVER_C_BINDING_BINDINGS_H_

src/connectivity/wlan/drivers/wlansoftmac/rust_driver/c-binding/cbindgen.toml

@@ -17,36 +17,31 @@ sys_includes = [ "zircon/types.h" ]
 
 [parse]
 parse_deps = true
-include = ["wlan_mlme", "wlan_ffi_transport"]
+include = ["wlan_ffi_transport"]
 
 [export]
-include = [
-    "FfiFrameProcessor",
-    "FfiFrameProcessorOps",
-]
+include = [ "FfiEthernetTx", "FfiWlanRx" ]
 exclude = [
-    # Unused types
-    "LinkStatus",
-
     # Voided types.
     "FfiBufferCtx",
-    "FfiFrameProcessorCtx",
-    "FfiFrameSenderCtx",
+    "FfiEthernetRxCtx",
+    "FfiEthernetTxCtx",
+    "FfiWlanRxCtx",
+    "FfiWlanTxCtx",
 ]
 
 [export.rename]
-# wlan-ffi-transport types
 "FfiBuffer" = "wlansoftmac_buffer_t"
-"FfiBufferProvider" = "wlansoftmac_buffer_provider_ops_t"
-
-# wlan-mlme types
-"FfiFrameProcessor" = "frame_processor_t"
-"FfiFrameSender" = "frame_sender_t"
-
-# This struct is used for void* context fields for data path operations.
 "FfiBufferCtx" = "void"
-"FfiFrameProcessorCtx" = "void"
-"FfiFrameSenderCtx" = "void"
+"FfiBufferProvider" = "wlansoftmac_buffer_provider_ops_t"
+"FfiEthernetRx" = "ethernet_rx_t"
+"FfiEthernetRxCtx" = "void"
+"FfiEthernetTx" = "ethernet_tx_t"
+"FfiEthernetTxCtx" = "void"
+"FfiWlanRx" = "wlan_rx_t"
+"FfiWlanRxCtx" = "void"
+"FfiWlanTx" = "wlan_tx_t"
+"FfiWlanTxCtx" = "void"
 
 [fn]
 prefix = 'extern "C"'

src/connectivity/wlan/drivers/wlansoftmac/rust_driver/c-binding/src/lib.rs

@@ -10,8 +10,10 @@ use {
     fuchsia_async::LocalExecutor,
     fuchsia_zircon as zx,
     std::{ffi::c_void, sync::Once},
-    wlan_ffi_transport::{BufferProvider, FfiBufferProvider},
-    wlan_mlme::device::{Device, FfiFrameSender},
+    wlan_ffi_transport::{
+        BufferProvider, EthernetRx, FfiBufferProvider, FfiEthernetRx, FfiWlanTx, WlanTx,
+    },
+    wlan_mlme::device::Device,
 };
 
 static LOGGER_ONCE: Once = Once::new();
@@ -57,7 +59,8 @@ static LOGGER_ONCE: Once = Once::new();
 pub unsafe extern "C" fn start_and_run_bridged_wlansoftmac(
     init_completer: *mut c_void,
     run_init_completer: unsafe extern "C" fn(init_completer: *mut c_void, status: zx::zx_status_t),
-    frame_sender: FfiFrameSender,
+    ethernet_rx: FfiEthernetRx,
+    wlan_tx: FfiWlanTx,
     buffer_provider: FfiBufferProvider,
     wlan_softmac_bridge_client_handle: zx::sys::zx_handle_t,
 ) -> zx::sys::zx_status_t {
@@ -83,7 +86,8 @@ pub unsafe extern "C" fn start_and_run_bridged_wlansoftmac(
         let channel = fidl::AsyncChannel::from_channel(channel);
         fidl_softmac::WlanSoftmacBridgeProxy::new(channel)
     };
-    let device = Device::new(wlan_softmac_bridge_proxy, frame_sender.into());
+    let device =
+        Device::new(wlan_softmac_bridge_proxy, EthernetRx::new(ethernet_rx), WlanTx::new(wlan_tx));
 
     let result = executor.run_singlethreaded(wlansoftmac_rust::start_and_serve(
         move |result: Result<(), zx::Status>| match result {

src/connectivity/wlan/drivers/wlansoftmac/rust_driver/src/lib.rs

@@ -20,10 +20,10 @@ use {
     },
     std::pin::Pin,
     tracing::{error, info},
-    wlan_ffi_transport::BufferProvider,
+    wlan_ffi_transport::{BufferProvider, EthernetTx, WlanRx},
     wlan_fidl_ext::{ResponderExt, SendResultExt, WithName},
     wlan_mlme::{
-        device::{completers::InitCompleter, DeviceOps, FrameProcessor},
+        device::{completers::InitCompleter, DeviceOps},
         DriverEvent, DriverEventSink,
     },
     wlan_sme::serve::create_sme,
@@ -444,14 +444,20 @@ async fn bootstrap_generic_sme<D: DeviceOps>(
     // Calling WlanSoftmac.Start() indicates to the vendor driver that this driver (wlansoftmac) is
     // ready to receive WlanSoftmacIfc messages. wlansoftmac will buffer all WlanSoftmacIfc messages
     // in an mpsc::UnboundedReceiver<DriverEvent> sink until the MLME server drains them.
-    let usme_bootstrap_channel_via_iface_creation =
-        match device.start(ifc_bridge, FrameProcessor::new(driver_event_sink)).await {
-            Ok(channel) => channel,
-            Err(status) => {
-                error!("Failed to receive a UsmeBootstrap handle: {}", status);
-                return Err(status);
-            }
-        };
+    let usme_bootstrap_channel_via_iface_creation = match device
+        .start(
+            ifc_bridge,
+            EthernetTx::new(Box::new(driver_event_sink.clone())),
+            WlanRx::new(Box::new(driver_event_sink)),
+        )
+        .await
+    {
+        Ok(channel) => channel,
+        Err(status) => {
+            error!("Failed to receive a UsmeBootstrap handle: {}", status);
+            return Err(status);
+        }
+    };
     let server = fidl::endpoints::ServerEnd::<fidl_sme::UsmeBootstrapMarker>::new(
         usme_bootstrap_channel_via_iface_creation,
     );