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

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Mar 10, 2025 at 04:13:21PM +0000, Van Haaren, Harry wrote:
> > 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.
> 

Yes, definite +1 for this. Let's not try and have a half-C, half-Rust set
of APIs, but instead let's have a clean low-level mapping directly to C
functions, and then build a proper, safe API on top of that.

/Bruce