Re: [PATCH v2] rust: support raw DPDK API

["Van Haaren, Harry" <harry.van.haaren@intel.com>]

> From: Gregory Etelson <getelson@nvidia.com>
> Sent: Saturday, March 8, 2025 6:50 PM
> To: Van Haaren, Harry <harry.van.haaren@intel.com>; Richardson, Bruce <bruce.richardson@intel.com>; dev@dpdk.org <dev@dpdk.org>
> Cc: getelson@nvidia.com <getelson@nvidia.com>; mkashani@nvidia.com <mkashani@nvidia.com>; thomas@monjalon.net <thomas@monjalon.net>
> Subject: [PATCH v2] rust: support raw DPDK API
>  
> The patch converts include files with DPDK API to RUST and binds new
> RUST API files into raw module under dpdk crate.
> 
> The RUST files and DPDK libraries build from C sources
> allow creation of DPDK application in RUST.
> 
> RUST DPDK application must specify the `dpdk` crate as
> dependency in Cargo.toml file.
> 
> RUST `dpdk` crate is installed into
> $MESON_INSTALL_DESTDIR_PREFIX/$libdir/rust directory.
> 
> Software requirements:
> - clang
> - RUST installation
> - bindgen-cli crate
> 
> RUST dpdk installation instructions:
> 1. Configure DPDK with `-Deanble_rust=true`
> 2. Build and install DPDK. The installation procedure will create
>    $MESON_INSTALL_DESTDIR_PREFIX/$libdir/rust crate.
> 3. Update PKG_CONFIG_PATH to point to DPDK installation.
> 
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>

Please include a "revision change" summary, this helps reviewers understand
what changes you have addressed, and what changes are not present. This usually
is formatted below a --- line, so Git doesn't include it in commit notes.

Also add to CC any participants of the wider email discussion.
(+CC Igor and Stephen here now)

--- 
v2:
- update rust crate name to "dpdk" from "dpdklib" (Bruce)
- update binding generation to use dpdk::raw::* namespace (Bruce)
- <were other comments/notes fixed?>
--- e.g. "rte_ethdev" -> "ethdev" namespace change?
--- e.g. "cargo fmt"?
--- e.g. "build.rs" simplification & fix?

Pulling comments from "detailed review" email thread (see v1 on patchwork)
- Gregory: "The idea was to provide generic DPDK API and let application to resolve unsafe interfaces."

For this patch, OK. But we (DPDK project) should take the next steps, and provide a "Safe Rust" API. The main reasons are that:
1) DPDK APIs have many rules around multithreading; (lcore concepts, rx_burst() threading requirements, etc).
2) Cross-language bindings are hard, FFI is hard (even though C/Rust is one of the "easier" ones)

Overall, I'd like to set the "goal" of Rust in DPDK to be "if it compiles, it is correct".
For example, if compiler allows "rxq.rx_burst()" to be called, it must ensure that a &mut reference is valid at that point.
That gives a Rust bindings *actual value*, as opposed to "allowing DPDK usage from Rust (with all threading/unsafety just like in C)".

The bindings are the required base for that: making it available. But it is important state
that these raw bindings are NOT ready for Rust folks to go build applications in "safe Rust".


- Igor "Indentation/styling is a bit off or inconsistent. I'd suggest to run `cargo fmt` to format the code."

Big +1. Rust has a default formatting engine, and using it means everybody wins (no checkpatch, no manual style rework).
By reworking (and auto-saving..) my updates of the first patch, it reformatted a bunch of code, losing my own changes.
Small things like this really hinder productivity. (Off topic: I'd like to see a DPDK auto-formatted for C code too!)

- Igor "I'd make `init_port_config` and `start_port` a method on `Port`, rather than a free function, but that's just my preference."

+1 here too. There is a logical "ownership" of functions to structs. For value added Rust APIs, we must
not copy the existing DPDK namespacing (it does not capture "ownership" in the way that Rust API should).
Instead, logically understanding the thread-requirements and boundaries of concepts like an "Rxq" struct instance,
and ensuring appropriate Rust "Send" and "Sync" bounds are applied is key to a good Rust API (as per https://youtu.be/lb6xn2xQ-NQ?t=702).

<snip>


> diff --git a/examples/rust/helloworld/src/main.rs b/examples/rust/helloworld/src/main.rs
> new file mode 100644

<snip>

> +pub type DpdkPort = u16;
> +pub struct Port {
> +    pub port_id:DpdkPort,
> +    pub dev_info:rte_eth_dev_info,
> +    pub dev_conf:rte_eth_conf,
> +    pub rxq_num:u16,
> +    pub txq_num:u16,
> +}
> +
> +impl Port {
> +    unsafe fn new(id:DpdkPort) -> Self {
> +        Port {
> +            port_id:id,
> +            dev_info: unsafe {
> +                let uninit: ::std::mem::MaybeUninit<rte_eth_dev_info> = ::std::mem::MaybeUninit::zeroed().assume_init();
> +                *uninit.as_ptr()
> +            },
> +            dev_conf: unsafe {
> +                let uninit: ::std::mem::MaybeUninit<rte_eth_conf> = ::std::mem::MaybeUninit::zeroed().assume_init();
> +                *uninit.as_ptr()
> +            },
> +            rxq_num:1,
> +            txq_num:1,
> +        }
> +    }
> +}
> +
> +pub unsafe fn iter_rte_eth_dev_owned_by(owner_id:u64) -> impl Iterator<Item=DpdkPort> {

Looking at the above parts of "Port abstraction in Rust", I'm not sure it really adds anything.
It feels a bit "middle of the road" (aka, adding some "abstraction", but not going far enough).

What if we took an even smaller step: remove the Rust struct concepts, and just call the "dpdk::raw::*" unsafe
functions directly, as if they were C code. No "struct Port, impl Port", and no "Iterator<DpdkPort" concepts at all.
That would show "correct" usage of the raw generated bindings, and provide raw API unit-tests.

In a future patch, we can start to build "Safe Rust" essential-basics of EAL, and Ethdev. This is to abstract over the
"dpdk::raw::*" APIs, "compiles == correct", end-user documentation, etc all as is normal in Rust ecosystem. Example:

Today: V2 patch (note the "Port" struct in Rust):
  pub unsafe fn init_port_config(port: &mut Port) { ... }

Future Raw API (note the raw u16 like C APIs):
  unsafe fn init_port_config(port: u16) { ... }

Future Safe API (mockup, this needs thought):
  let ports = eal.get_eth_ports();
  for p in &mut ports {
     p.initialize(rxqs, txqs, ...)?;
  }

Thoughts?

Regards, -Harry