* Re: [PATCH] rust: support DPDK API
2025-03-06 13:37 [PATCH] rust: support DPDK API Gregory Etelson
@ 2025-03-06 19:26 ` Van Haaren, Harry
2025-03-07 16:56 ` Etelson, Gregory
2025-03-07 15:54 ` Van Haaren, Harry
` (2 subsequent siblings)
3 siblings, 1 reply; 10+ messages in thread
From: Van Haaren, Harry @ 2025-03-06 19:26 UTC (permalink / raw)
To: Gregory Etelson, dev; +Cc: thomas, mkashani, Richardson, Bruce
> From: Gregory Etelson <getelson@nvidia.com>
> Sent: Thursday, March 6, 2025 1:37 PM
> To: dev@dpdk.org <dev@dpdk.org>
> Cc: getelson@nvidia.com <getelson@nvidia.com>; thomas@monjalon.net <thomas@monjalon.net>; mkashani@nvidia.com <mkashani@nvidia.com>; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [PATCH] rust: support DPDK API
Cool, I like this subject, great!
> The patch converts include files with DPDK API to RUST and binds new
> RUST API files info dpdklib package.
>
> The RUST dpdklib files and DPDK libraries build from C sources
> allow creation of DPDK application in RUST.
>
> RUST DPDK application must specify the `dpdklib` package as
> dependency in Cargo.toml file.
>
> RUST `dpdklib` package is installed into
> MESON_INSTALL_DESTDIR_PREFIX/rust directory.
>
> Software requirements:
> - clang
> - RUST installation
> - bindgen-cli crate
>
> RUST dpdklib installation instructions:
> 1. Configure DPDK with `-Deanble_rust=true`
> 2. Build and install DPDK. The installation procedure will create
> MESON_INSTALL_DESTDIR_PREFIX/rust directory.
> 3. Update PKG_CONFIG_PATH to point to DPDK installation.
Interesting approach to automate it; are there specific reasons that this approach was taken,
or did this just seem the easiest/best way to include "Rust support" into upstream DPDK?
Alternatives could be (*not* suggesting to rework the patch!) a dpdk-sys
crate where the bindgen etc is done externally to DPDK itself. Many existing
approaches (some examples: https://youtu.be/lb6xn2xQ-NQ?t=130) use that method.
I kind of like (this approach) of having the binding generation upstream with DPDK itself;
it makes the Rust support upstream and keeps it close to DPDK... however it
means that it is "the one" official/DPDK-community-approved library/crate.
There's some nice tidy-ups to cleanup the namespaces possible if this is "the crate".
Perhaps (sorry, borderline bikeshed-topics..) renames for clarity & readability, e.g.:
dpdklib::rte_eal::rte_eal_init()
to
dpdk::eal::init()
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
I'll get to a testing & review in the next days, however I'd like to ask some bigger picture
questions, to understand/provide input on approach, and how big an effor to expect here!
See above linked youtube video - I had sketched out some API concepts for exposing a
"safe Rust API" wrapper around the DPDK C API, which also encodes threading requirements.
Some questions:
- Is there an overaching "we're trying to achieve X with Rust", or specifically "safe Rust"?
- Is this patch the "get DPDK + Rust working", with follow up patches for "safe wrappers" the intention?
- Or is this patch all to expect for now?
Again, thanks for the patch, I think Rust is important for DPDK & infrastructure software,
and will try make time to help test/review/discuss this patch, and the wider Rust effort!
Regards, -Harry
<snip code itself>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: support DPDK API
2025-03-06 19:26 ` Van Haaren, Harry
@ 2025-03-07 16:56 ` Etelson, Gregory
0 siblings, 0 replies; 10+ messages in thread
From: Etelson, Gregory @ 2025-03-07 16:56 UTC (permalink / raw)
To: Van Haaren, Harry
Cc: Gregory Etelson, dev, thomas, mkashani, Richardson, Bruce
Hello Harry,
>
>> From: Gregory Etelson <getelson@nvidia.com>
>> Sent: Thursday, March 6, 2025 1:37 PM
>> To: dev@dpdk.org <dev@dpdk.org>
>> Cc: getelson@nvidia.com <getelson@nvidia.com>; thomas@monjalon.net <thomas@monjalon.net>; mkashani@nvidia.com <mkashani@nvidia.com>; Richardson, Bruce <bruce.richardson@intel.com>
>> Subject: [PATCH] rust: support DPDK API
>
> Cool, I like this subject, great!
>
>> The patch converts include files with DPDK API to RUST and binds new
>> RUST API files info dpdklib package.
>>
>> The RUST dpdklib files and DPDK libraries build from C sources
>> allow creation of DPDK application in RUST.
>>
>> RUST DPDK application must specify the `dpdklib` package as
>> dependency in Cargo.toml file.
>>
>> RUST `dpdklib` package is installed into
>> MESON_INSTALL_DESTDIR_PREFIX/rust directory.
>>
>> Software requirements:
>> - clang
>> - RUST installation
>> - bindgen-cli crate
>>
>> RUST dpdklib installation instructions:
>> 1. Configure DPDK with `-Deanble_rust=true`
>> 2. Build and install DPDK. The installation procedure will create
>> MESON_INSTALL_DESTDIR_PREFIX/rust directory.
>> 3. Update PKG_CONFIG_PATH to point to DPDK installation.
>
> Interesting approach to automate it; are there specific reasons that this approach was taken,
> or did this just seem the easiest/best way to include "Rust support" into upstream DPDK?
> Alternatives could be (*not* suggesting to rework the patch!) a dpdk-sys
> crate where the bindgen etc is done externally to DPDK itself. Many existing
> approaches (some examples: https://youtu.be/lb6xn2xQ-NQ?t=130) use that method.
>
> I kind of like (this approach) of having the binding generation upstream with DPDK itself;
> it makes the Rust support upstream and keeps it close to DPDK... however it
> means that it is "the one" official/DPDK-community-approved library/crate.
>
My reason for adding the crate directly into upstream DPDK branch was to produce
a single centralized RUST version of DPDK API.
If DPDK API files are converted externally, there is no control over the result.
Internal crate can help smoothing bindgen translation - DPDK files can be
adjusted to reduce manual translation corrections.
The crate content will be automatically updated after DPDK API change.
Also, internal crate will increate overall RUST awareness in DPDK comunity.
> There's some nice tidy-ups to cleanup the namespaces possible if this is "the crate".
> Perhaps (sorry, borderline bikeshed-topics..) renames for clarity & readability, e.g.:
> dpdklib::rte_eal::rte_eal_init()
The crate exports existing DPDK API to RUST application as it is.
This way RUST application receives stable DPDK API - the same one C applications
have.
> to
> dpdk::eal::init()
>
That is not current DPDK API.
>> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
>
> I'll get to a testing & review in the next days, however I'd like to ask some bigger picture
> questions, to understand/provide input on approach, and how big an effor to expect here!
> See above linked youtube video - I had sketched out some API concepts for exposing a
> "safe Rust API" wrapper around the DPDK C API, which also encodes threading requirements.
>
> Some questions:
> - Is there an overaching "we're trying to achieve X with Rust", or specifically "safe Rust"?
> - Is this patch the "get DPDK + Rust working", with follow up patches for "safe wrappers" the intention?
> - Or is this patch all to expect for now?
The goal was to create RUST application over existing DPDK API.
The patch converts DPDK API files required for the helloworld example.
Additional API files will be added on demand.
>
> Again, thanks for the patch, I think Rust is important for DPDK & infrastructure software,
> and will try make time to help test/review/discuss this patch, and the wider Rust effort!
>
> Regards, -Harry
>
> <snip code itself>
>
Regards,
Gregory
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: support DPDK API
2025-03-06 13:37 [PATCH] rust: support DPDK API Gregory Etelson
2025-03-06 19:26 ` Van Haaren, Harry
@ 2025-03-07 15:54 ` Van Haaren, Harry
2025-03-07 16:20 ` Bruce Richardson
2025-03-07 18:00 ` Etelson, Gregory
2025-03-08 14:28 ` Igor Gutorov
2025-03-08 18:50 ` [PATCH v2] rust: support raw " Gregory Etelson
3 siblings, 2 replies; 10+ messages in thread
From: Van Haaren, Harry @ 2025-03-07 15:54 UTC (permalink / raw)
To: Gregory Etelson, dev; +Cc: thomas, mkashani, Richardson, Bruce
> From: Gregory Etelson <getelson@nvidia.com>
> Sent: Thursday, March 6, 2025 1:37 PM
> To: dev@dpdk.org <dev@dpdk.org>
> Cc: getelson@nvidia.com <getelson@nvidia.com>; thomas@monjalon.net <thomas@monjalon.net>; mkashani@nvidia.com <mkashani@nvidia.com>; Richardson, Bruce <bruce.richardson@intel.com>
> Subject: [PATCH] rust: support DPDK API
More in-depth review in this reply..
> The patch converts include files with DPDK API to RUST and binds new
> RUST API files info dpdklib package.
>
> The RUST dpdklib files and DPDK libraries build from C sources
> allow creation of DPDK application in RUST.
>
> RUST DPDK application must specify the `dpdklib` package as
> dependency in Cargo.toml file.
>
> RUST `dpdklib` package is installed into
> MESON_INSTALL_DESTDIR_PREFIX/rust directory.
>
> Software requirements:
> - clang
> - RUST installation
> - bindgen-cli crate
>
> RUST dpdklib installation instructions:
> 1. Configure DPDK with `-Deanble_rust=true`
> 2. Build and install DPDK. The installation procedure will create
> MESON_INSTALL_DESTDIR_PREFIX/rust directory.
> 3. Update PKG_CONFIG_PATH to point to DPDK installation.
I've tested these instructions, using the default prefix = "/usr/local"
This puts the rust files in "/usr/local/rust", so some tidy-up required.
Debugging (or "searching for the files" was difficult - perhaps a log of
the bindgen output from "rust-env.sh" would be helpful for 1st time setup?)
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> ---
> buildtools/meson.build | 4 +
> buildtools/rust-env.sh | 78 +++++++++++
> examples/rust/helloworld/Cargo.lock | 14 ++
> examples/rust/helloworld/Cargo.toml | 7 +
> examples/rust/helloworld/build.rs | 21 +++
> examples/rust/helloworld/src/main.rs | 197 +++++++++++++++++++++++++++
> meson_options.txt | 2 +
> 7 files changed, 323 insertions(+)
> create mode 100755 buildtools/rust-env.sh
> create mode 100644 examples/rust/helloworld/Cargo.lock
> create mode 100644 examples/rust/helloworld/Cargo.toml
> create mode 100644 examples/rust/helloworld/build.rs
> create mode 100644 examples/rust/helloworld/src/main.rs
>
> diff --git a/buildtools/meson.build b/buildtools/meson.build
> index 4e2c1217a2..dd16567f51 100644
> --- a/buildtools/meson.build
> +++ b/buildtools/meson.build
> @@ -50,3 +50,7 @@ else
> pmdinfo += 'ar'
> pmdinfogen += 'elf'
> endif
> +
> +if get_option('enable_rust')
> + meson.add_install_script('rust-env.sh')
> +endif
> diff --git a/buildtools/rust-env.sh b/buildtools/rust-env.sh
> new file mode 100755
> index 0000000000..705bc0d95b
> --- /dev/null
> +++ b/buildtools/rust-env.sh
> @@ -0,0 +1,78 @@
> +#! /bin/sh
> +
> +# Convert DPDK API files into RUST.
> +# DPDK files selection is on demand.
> +#
> +# The coversion is done in 4 stages:
> +# 1. Preparation [Optional]
> +# Due to the bindgen conversion utility limitations source file may need
> +# manual adjustment.
> +# 2. Preprocessing [Mandatory]
> +# Run preprocessor on a source file before conversion.
> +# 3. Conversion [Mandatory]
> +# Convert preprocessed C file into RUST file
> +# 4. Post translation [Optional]
> +# Manually fix translation.
> +
> +# DPDK files list
> +files='
> +rte_build_config.h
> +rte_eal.h
> +rte_ethdev.h
> +rte_mbuf.h
> +rte_mbuf_core.h
> +rte_mempool.h
> +'
> +
> +rust_dir="${MESON_INSTALL_DESTDIR_PREFIX}/rust"
Here "rust_dir"="/usr/local/rust" if the default prefix is used.
At least, a "dpdk" must be in there somewhere (we're not Rust itself after all!)
but also different systems put files in e.g: "/usr/local/lib/x86_64-linux-gnu/dpdk"
Appending a "/rust" to the end of that is probably what was intended?
> +include_dir="${MESON_INSTALL_DESTDIR_PREFIX}/include"
> +
> +if test -d "$rust_dir"; then
> + rm -rf "$rust_dir"
> +fi
> +
> +mkdir -p "$rust_dir/src"
> +if ! test -d "$rust_dir"; then
> + echo "failed to create Rust library $rust_dir"
> + exit 255
> +fi
> +touch "$rust_dir/src/lib.rs"
> +
> +bindgen_opt='--no-layout-tests --no-derive-debug'
> +bindgen_clang_opt='-Wno-unused-command-line-argument'
> +
> +create_rust_lib ()
> +{
> + base=$1
> +
> + cp $include_dir/${base}.h /tmp/${base}.h
> +
> +# bindgen cannot process complex macro definitions
> +# manually simplify macros before conversion
> + sed -i -e 's/RTE_BIT64(\([0-9]*\))/(1UL << \1)/g' /tmp/${base}.h
> + sed -i -e 's/RTE_BIT32(\([0-9]*\))/(1U << \1)/g' /tmp/${base}.h
> + sed -i -e 's/UINT64_C(\([0-9]*\))/\1/g' /tmp/${base}.h
> +
> + # clang output has better integration with bindgen than GCC
> + clang -E -dD -I$include_dir /tmp/${base}.h > /tmp/$base.i
# Adding this helps debug the header conversion & output file locations
echo "writing Rust converted header to ${rust_dir}/src/${base}.rs"
> + bindgen $bindgen_opt --output $rust_dir/src/$base.rs /tmp/$base.i -- $bindgen_clang_opt
> + rm -f /tmp/$base.i /tmp/$base.h
> +}
> +
> +for file in $files; do
> + base=$(basename $file | cut -d. -f 1)
> + create_rust_lib $base
> + echo "pub mod $base;" >> "$rust_dir/src/lib.rs"
> +done
> +
> +
> +cat > "$rust_dir/Cargo.toml" <<EOF
> +[package]
> +name = "dpdklib"
> +version = "$(cat $MESON_SOURCE_ROOT/VERSION | sed 's/\.0\([1-9]\)/\.\1/')"
Missing {} brackets around MESON_SOURCE_ROOT perhaps? Got an error on "/VERSION" not found.
> +EOF
> +
> +# post conversion updates
> +# RUST does not accept aligned structures into packed structure.
> +# TODO: fix DPDK definitions.
> +sed -i 's/repr(align(2))/repr(packed(2))/g' "$rust_dir/src/rte_ethdev.rs"
Hmm, interesting, lets keep a TODO here, but as this changes the "allowable" or valid binary layouts,
it is potentially going to cause differences between C and Rust's interpretation of the memory backing the structs.
TODO: investigate each struct individually, and determine if/what is really required/correct.
> diff --git a/examples/rust/helloworld/Cargo.lock b/examples/rust/helloworld/Cargo.lock
> new file mode 100644
> index 0000000000..d18af80c66
> --- /dev/null
> +++ b/examples/rust/helloworld/Cargo.lock
> @@ -0,0 +1,14 @@
> +# This file is automatically @generated by Cargo.
> +# It is not intended for manual editing.
> +version = 4
> +
> +[[package]]
> +name = "dpdklib"
> +version = "25.3.0-rc1"
> +
> +[[package]]
> +name = "helloworld"
> +version = "0.1.0"
> +dependencies = [
> + "dpdklib",
> +]
> diff --git a/examples/rust/helloworld/Cargo.toml b/examples/rust/helloworld/Cargo.toml
> new file mode 100644
> index 0000000000..7f9a77968a
> --- /dev/null
> +++ b/examples/rust/helloworld/Cargo.toml
> @@ -0,0 +1,7 @@
> +[package]
> +name = "helloworld"
> +version = "0.1.0"
> +edition = "2024"
> +
> +[dependencies]
> +dpdklib = {path = "MESON_INSTALL_DESTDIR_PREFIX/rust"}
> \ No newline at end of file
Above Cargo.toml looks normal. Perhaps we could suggest/provide a method where
DPDK does not need to be "ninja install"-ed, but instead process a local directory?
That would likely make it easier to locally build/test Rust bindings.
> diff --git a/examples/rust/helloworld/build.rs b/examples/rust/helloworld/build.rs
> new file mode 100644
> index 0000000000..5c76188e18
> --- /dev/null
> +++ b/examples/rust/helloworld/build.rs
> @@ -0,0 +1,21 @@
> +use std::process::Command;
> +
> +pub fn main() {
> + let mut pkgconfig = Command::new("pkg-config");
> +
> + match pkgconfig.args(["--libs", "libdpdk"]).output() {
> + Ok (output) => {
> + let stdout = String::from_utf8_lossy(&output.stdout).trim_end().to_string();
> + for token in stdout.split_ascii_whitespace().filter(|s| !s.is_empty()) {
> + if token.starts_with("-L") {
> + println!("cargo::rustc-link-search=native={}", &token[2..]);
> + } else if token.starts_with("-l") {
> + println!("cargo::rustc-link-lib={}", &token[2..]);
> + }
> + }
> + }
> + Err(error) => {
> + panic!("failed to read libdpdk package: {:?}", error);
> + }
> + }
> +}
Could a simple "probe()" like below replace the more complex logic? I've linked DPDK in the
past using this more boring method.. or is there a reason that the link-search-native and
link-lib parts have to be handled manually? Doing a Command::new() is hacky, nice to remove it if possible!
if let Err(e) = pkg_config::Config::new().probe("libdpdk") {
panic!("libdpdk probe failed: {e:?}");
}
The build.rs file itself also needs a "rebuild if self changed" line:
println!("cargo:rerun-if-changed=build.rs");
> diff --git a/examples/rust/helloworld/src/main.rs b/examples/rust/helloworld/src/main.rs
> new file mode 100644
> index 0000000000..cf666aece6
> --- /dev/null
> +++ b/examples/rust/helloworld/src/main.rs
> @@ -0,0 +1,197 @@
> +/// Usage: helloworld -a <port 1 params> -a <port 2 params> ...
> +
> +use std::env;
> +use std::ffi::{CString};
> +use std::os::raw::{c_int, c_char};
> +use std::ffi::CStr;
> +
> +use dpdklib::rte_eal::{
> + // Functions
> + rte_eal_init,
> + rte_eal_cleanup,
> +};
> +
> +use dpdklib::rte_ethdev::{
As per first reply, I'd like to see the end-result safe-Rust namespacing be shortened.
This can be easily done by renaming the name= in Cargo.toml, and
removing the "rte_" prefix at bindgen output stage for each file.
dpdk::ethdev:: { RTE_ETH_NAME_MAX_LEN, ... };
Perhaps its a good method to put the generated (unsafe, raw-C-bindings) in this
namespace, and build a safe crate (with the dpdk::ethdev::* namespacing) over it.
Thoughts?
> + RTE_ETH_NAME_MAX_LEN,
> + RTE_ETH_DEV_NO_OWNER,
> + RTE_ETH_RSS_IP,
> + rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_RSS,
> + rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_VMDQ_DCB_RSS,
> +
> + // Structures
> + rte_eth_dev_info,
> + rte_eth_conf,
> + rte_eth_txconf,
> + rte_eth_rxconf,
> +
> + // Functions
> + rte_eth_dev_get_name_by_port,
> + rte_eth_find_next_owned_by,
> + rte_eth_dev_info_get,
> + rte_eth_dev_configure,
> + rte_eth_tx_queue_setup,
> + rte_eth_rx_queue_setup,
> + rte_eth_dev_start,
> +};
> +
> +use dpdklib::rte_build_config::{
> + RTE_MAX_ETHPORTS,
> +};
> +
> +use dpdklib::rte_mbuf::{
> + rte_pktmbuf_pool_create,
> +};
> +
> +use dpdklib::rte_mbuf_core::{
> + RTE_MBUF_DEFAULT_BUF_SIZE
> +};
> +
> +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 {
This is where the first manual code is wrapping C concepts into Rust.
And the approach taken here is quite "C thinking", where it is possible
to misuse an API, and hence "unsafe" is used for many functions (more below)
and also for the C FFI function calls.
I believe this part can be improved, and that there is big value in those improvements:
Today the code is "C but compiled by the Rust compiler", the programmer has to use the APIs correctly.
I'll send a patch in reply to this one for review, suggesting ways to improve.
Future code goal: "Rust checks code: invalid code will not compile, or provide a clean error at runtime."
> + 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> {
> + let mut port_id:DpdkPort = 0 as DpdkPort;
> + std::iter::from_fn(move || {
> + let cur = port_id;
> + port_id = unsafe {
> + rte_eth_find_next_owned_by(cur, owner_id) as DpdkPort
> + };
> + if port_id == RTE_MAX_ETHPORTS as DpdkPort {
> + return None
> + }
> + if cur == port_id { port_id += 1 }
> + Some(cur)
> + })
> +}
> +
> +pub unsafe fn iter_rte_eth_dev() -> impl Iterator<Item=DpdkPort> {
> + unsafe {
> + iter_rte_eth_dev_owned_by(RTE_ETH_DEV_NO_OWNER as u64)
> + }
> +}
Cool - I like the egonomics idea of providing Iterator<DpdkPort>.
The implementation here is unsafe, which is unfortunate, we'd like to
ensure that all "end-user visible" Rust code is "safe Rust".
> +pub unsafe fn init_port_config(port: &mut Port) {
> + let ret = unsafe {
> + rte_eth_dev_info_get(port.port_id, &mut port.dev_info as *mut rte_eth_dev_info)
> + };
> + if ret != 0 {
> + panic!("port-{}: failed to get dev info {ret}", port.port_id);
> + }
> +
> + port.dev_conf.rx_adv_conf.rss_conf.rss_key = std::ptr::null_mut();
> + port.dev_conf.rx_adv_conf.rss_conf.rss_hf = if port.rxq_num > 1 {
> + RTE_ETH_RSS_IP as u64 & port.dev_info.flow_type_rss_offloads
> + } else {0};
> +
> + if port.dev_conf.rx_adv_conf.rss_conf.rss_hf != 0 {
> + port.dev_conf.rxmode.mq_mode =
> + rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_VMDQ_DCB_RSS & rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_RSS;
> + }
> +}
> +
> +pub unsafe fn show_ports_summary(ports: &Vec<Port>) {
> + let mut name_buf:[c_char;RTE_ETH_NAME_MAX_LEN as usize]= [0 as c_char;RTE_ETH_NAME_MAX_LEN as usize];
> + let title = format!("{:<4} {:<32} {:<14}", "Port", "Name", "Driver");
> + println!("{title}");
> + ports.iter().for_each(|p| unsafe {
> + let _rc = rte_eth_dev_get_name_by_port(p.port_id, name_buf.as_mut_ptr());
> + let name = CStr::from_ptr(name_buf.as_ptr());
> + let drv = CStr::from_ptr(p.dev_info.driver_name);
> + let summary = format!("{:<4} {:<32} {:<14}",
> + p.port_id, name.to_str().unwrap(), drv.to_str().unwrap());
> + println!("{summary}");
> + });
> +
> +}
There are some interesting changes of thoughts in Rust vs C, in how applications actually
allocate or "own" data. The above show_ports_summary() function requires a &Vec<Port>.
That demands that the backing storage for Port is a Vec<Port>, and a (immutable) reference
is passed into this function. Hence the type is:
summary(ports: &Vec<Port>)
There is no reason to "demand" that the application stores Ports in a Vec. While common to store things
in Vec<T>, we have now demanded "more than required" from the application. In some situations
(perhaps not 100% relevant here, but..) like embedded systems, where dynamic allocation isn't available
there IS no Vec<T> available. In other application scenarios, perhaps a HashMap is better.
The best way to do this in Rust is to use the Iterator trait, which doesn't require any specific
"backing storage" (e.g. linear-data for 'slice' or Vec, or HashMap per-item allocated data, or ...).
Instead the Iterator trait allows the function itself to iterate of the "Item". Example:
pub fn show_ports_summary_iterator<'a>(ports: impl Iterator<Item = &'a Port>) {
ports.for_each(|p| { /* p is a &Port */);
}
There should be no unsafe required for the above debug print, and no statically sized C-string allocations.
Instead of fetching the port name via C function at print-time, the Port::new() function can cache the value
and convert it to a Rust String type - avoids unsafe {}, reduces scope of unsafe variables in the program.
> +unsafe fn start_port(port:&mut Port) {
> + let mut rc = unsafe {
> + rte_eth_dev_configure(port.port_id, port.rxq_num, port.txq_num,
> + &port.dev_conf as *const rte_eth_conf)
> + };
> + if rc != 0 { panic!("failed to configure port-{}: {rc}", port.port_id)}
> + println!("port-{} configured", port.port_id);
> +
> + rc = unsafe {
> + rte_eth_tx_queue_setup(port.port_id, 0, 64, 0, 0 as *const rte_eth_txconf)
> + };
> + if rc != 0 { panic!("port-{}: failed to configure TX queue 0 {rc}", port.port_id)}
> + println!("port-{} configured TX queue 0", port.port_id);
> +
> + let mbuf_pool_name = CString::new(format!("mbuf pool port-{}", port.port_id)).unwrap();
> + let mbuf_pool : *mut dpdklib::rte_mbuf::rte_mempool = unsafe {
> + rte_pktmbuf_pool_create(mbuf_pool_name.as_ptr(), 1024, 0, 0,
> + RTE_MBUF_DEFAULT_BUF_SIZE as u16, 0)
> + };
> + if mbuf_pool == 0 as *mut dpdklib::rte_mbuf::rte_mempool {
> + panic!("port-{}: failed to allocate mempool {rc}", port.port_id)
> + }
> + println!("port-{} mempool ready", port.port_id);
> +
> + let mut rxq_conf:rte_eth_rxconf = port.dev_info.default_rxconf.clone();
> + rxq_conf.offloads = 0;
> + rc = unsafe {
> + rte_eth_rx_queue_setup(port.port_id, 0, 64, 0,
> + &mut rxq_conf as *mut rte_eth_rxconf,
> + mbuf_pool as *mut dpdklib::rte_ethdev::rte_mempool)
> + };
> + if rc != 0 { panic!("port-{}: failed to configure RX queue 0 {rc}", port.port_id)}
> + println!("port-{} configured RX queue 0", port.port_id);
> + rc = unsafe {
> + rte_eth_dev_start(port.port_id)
> + };
> + if rc != 0 { panic!("failed to start port-{}: {rc}", port.port_id)}
> + println!("port-{} started", port.port_id);
> +}
Similar concepts around reworking "unsafe" away as above, future discussion.
> +fn main() {
> +
> + let mut argv: Vec<*mut c_char> = env::args()
> + .map(|arg| CString::new(arg).unwrap().into_raw()).collect();
> +
> + let rc = unsafe {
> + rte_eal_init(env::args().len() as c_int, argv.as_mut_ptr())
> + };
> + if rc == -1 {
> + unsafe { rte_eal_cleanup(); }
> + }
> +
> + let mut ports:Vec<Port> = vec![];
> + unsafe {
> + for port_id in iter_rte_eth_dev()
> + .take(dpdklib::rte_build_config::RTE_MAX_ETHPORTS as usize) {
> + let mut port = Port::new(port_id);
> + init_port_config(&mut port);
> + println!("init port {port_id}");
> + start_port(&mut port);
> + ports.push(port);
> + }
> + }
> +
> + unsafe { show_ports_summary(&ports); }
> +
> + println!("Hello, world!");
> +}
Overall, I'm very happy to see this patch, I want to ensure folks on list hear that!
The unsafe {} around each block of "do something" is not idiomatic Rust - we can improve that.
The summary_iterator() function above can be called like a normal function:
show_ports_summary_iterator(ports.iter());
> diff --git a/meson_options.txt b/meson_options.txt
> index e49b2fc089..d37b9ba1dc 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -60,3 +60,5 @@ option('tests', type: 'boolean', value: true, description:
> 'build unit tests')
> option('use_hpet', type: 'boolean', value: false, description:
> 'use HPET timer in EAL')
> +option('enable_rust', type: 'boolean', value: false, description:
> + 'enable RUST')
> --
> 2.45.2
Great work, and looking forward to hearing about next-steps, and discussion on the above feedback.
Regards! (& thanks for reading till the end.. this turned into a long response :) -Harry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: support DPDK API
2025-03-07 15:54 ` Van Haaren, Harry
@ 2025-03-07 16:20 ` Bruce Richardson
2025-03-07 18:15 ` Etelson, Gregory
2025-03-07 18:00 ` Etelson, Gregory
1 sibling, 1 reply; 10+ messages in thread
From: Bruce Richardson @ 2025-03-07 16:20 UTC (permalink / raw)
To: Van Haaren, Harry; +Cc: Gregory Etelson, dev, thomas, mkashani
On Fri, Mar 07, 2025 at 03:54:33PM +0000, Van Haaren, Harry wrote:
> > From: Gregory Etelson <getelson@nvidia.com>
> > Sent: Thursday, March 6, 2025 1:37 PM
> > To: dev@dpdk.org <dev@dpdk.org>
> > Cc: getelson@nvidia.com <getelson@nvidia.com>; thomas@monjalon.net <thomas@monjalon.net>; mkashani@nvidia.com <mkashani@nvidia.com>; Richardson, Bruce <bruce.richardson@intel.com>
> > Subject: [PATCH] rust: support DPDK API
>
<snip>
> +
> > +use dpdklib::rte_ethdev::{
>
> As per first reply, I'd like to see the end-result safe-Rust namespacing be shortened.
>
> This can be easily done by renaming the name= in Cargo.toml, and
> removing the "rte_" prefix at bindgen output stage for each file.
> dpdk::ethdev:: { RTE_ETH_NAME_MAX_LEN, ... };
>
> Perhaps its a good method to put the generated (unsafe, raw-C-bindings) in this
> namespace, and build a safe crate (with the dpdk::ethdev::* namespacing) over it.
> Thoughts?
>
I love naming discussions :-)
+1 to separate namespaces for low-level DPDK wraps, and more rustic
higher-level APIs. However, that doesn't mean that the names get to be
ugly! Let's definitely remove the "lib" in dpdklib (or libdpdk), and the
"rte_" prefix from libs.
How about having "dpdk::raw::*" e.g. "dpdk::raw::ethdev" for low-level
wraps and "dpdk::*" e.g. "dpdk::ethdev" for higher level apis?
/Bruce
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: support DPDK API
2025-03-07 16:20 ` Bruce Richardson
@ 2025-03-07 18:15 ` Etelson, Gregory
0 siblings, 0 replies; 10+ messages in thread
From: Etelson, Gregory @ 2025-03-07 18:15 UTC (permalink / raw)
To: Bruce Richardson
Cc: Van Haaren, Harry, Gregory Etelson, dev, thomas, mkashani
On Fri, 7 Mar 2025, Bruce Richardson wrote:
> External email: Use caution opening links or attachments
>
>
> On Fri, Mar 07, 2025 at 03:54:33PM +0000, Van Haaren, Harry wrote:
>>> From: Gregory Etelson <getelson@nvidia.com>
>>> Sent: Thursday, March 6, 2025 1:37 PM
>>> To: dev@dpdk.org <dev@dpdk.org>
>>> Cc: getelson@nvidia.com <getelson@nvidia.com>; thomas@monjalon.net <thomas@monjalon.net>; mkashani@nvidia.com <mkashani@nvidia.com>; Richardson, Bruce <bruce.richardson@intel.com>
>>> Subject: [PATCH] rust: support DPDK API
>>
> <snip>
>
>> +
>>> +use dpdklib::rte_ethdev::{
>>
>> As per first reply, I'd like to see the end-result safe-Rust namespacing be shortened.
>>
>> This can be easily done by renaming the name= in Cargo.toml, and
>> removing the "rte_" prefix at bindgen output stage for each file.
>> dpdk::ethdev:: { RTE_ETH_NAME_MAX_LEN, ... };
>>
>> Perhaps its a good method to put the generated (unsafe, raw-C-bindings) in this
>> namespace, and build a safe crate (with the dpdk::ethdev::* namespacing) over it.
>> Thoughts?
>>
>
> I love naming discussions :-)
>
> +1 to separate namespaces for low-level DPDK wraps, and more rustic
> higher-level APIs. However, that doesn't mean that the names get to be
> ugly! Let's definitely remove the "lib" in dpdklib (or libdpdk), and the
> "rte_" prefix from libs.
>
> How about having "dpdk::raw::*" e.g. "dpdk::raw::ethdev" for low-level
> wraps and "dpdk::*" e.g. "dpdk::ethdev" for higher level apis?
>
I think that is good idea.
Application can use ether dpdk::raw::* or dpdk::* API depending on internal
logic.
> /Bruce
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: support DPDK API
2025-03-07 15:54 ` Van Haaren, Harry
2025-03-07 16:20 ` Bruce Richardson
@ 2025-03-07 18:00 ` Etelson, Gregory
1 sibling, 0 replies; 10+ messages in thread
From: Etelson, Gregory @ 2025-03-07 18:00 UTC (permalink / raw)
To: Van Haaren, Harry
Cc: Gregory Etelson, dev, thomas, mkashani, Richardson, Bruce
On Fri, 7 Mar 2025, Van Haaren, Harry wrote:
> External email: Use caution opening links or attachments
>
>
>> From: Gregory Etelson <getelson@nvidia.com>
>> Sent: Thursday, March 6, 2025 1:37 PM
>> To: dev@dpdk.org <dev@dpdk.org>
>> Cc: getelson@nvidia.com <getelson@nvidia.com>; thomas@monjalon.net <thomas@monjalon.net>; mkashani@nvidia.com <mkashani@nvidia.com>; Richardson, Bruce <bruce.richardson@intel.com>
>> Subject: [PATCH] rust: support DPDK API
>
> More in-depth review in this reply..
>
>> The patch converts include files with DPDK API to RUST and binds new
>> RUST API files info dpdklib package.
>>
>> The RUST dpdklib files and DPDK libraries build from C sources
>> allow creation of DPDK application in RUST.
>>
>> RUST DPDK application must specify the `dpdklib` package as
>> dependency in Cargo.toml file.
>>
>> RUST `dpdklib` package is installed into
>> MESON_INSTALL_DESTDIR_PREFIX/rust directory.
>>
>> Software requirements:
>> - clang
>> - RUST installation
>> - bindgen-cli crate
>>
>> RUST dpdklib installation instructions:
>> 1. Configure DPDK with `-Deanble_rust=true`
>> 2. Build and install DPDK. The installation procedure will create
>> MESON_INSTALL_DESTDIR_PREFIX/rust directory.
>> 3. Update PKG_CONFIG_PATH to point to DPDK installation.
>
> I've tested these instructions, using the default prefix = "/usr/local"
> This puts the rust files in "/usr/local/rust", so some tidy-up required.
> Debugging (or "searching for the files" was difficult - perhaps a log of
> the bindgen output from "rust-env.sh" would be helpful for 1st time setup?)
RUST installation directory is kind of open issue.
The current patch installs RUST crate during DPDK installation.
RUST environment can be undefined at that time.
Maybe it's better to install the crate with a dedicated meson command.
>
>
>> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
>> ---
>> buildtools/meson.build | 4 +
>> buildtools/rust-env.sh | 78 +++++++++++
>> examples/rust/helloworld/Cargo.lock | 14 ++
>> examples/rust/helloworld/Cargo.toml | 7 +
>> examples/rust/helloworld/build.rs | 21 +++
>> examples/rust/helloworld/src/main.rs | 197 +++++++++++++++++++++++++++
>> meson_options.txt | 2 +
>> 7 files changed, 323 insertions(+)
>> create mode 100755 buildtools/rust-env.sh
>> create mode 100644 examples/rust/helloworld/Cargo.lock
>> create mode 100644 examples/rust/helloworld/Cargo.toml
>> create mode 100644 examples/rust/helloworld/build.rs
>> create mode 100644 examples/rust/helloworld/src/main.rs
>>
>> diff --git a/buildtools/meson.build b/buildtools/meson.build
>> index 4e2c1217a2..dd16567f51 100644
>> --- a/buildtools/meson.build
>> +++ b/buildtools/meson.build
>> @@ -50,3 +50,7 @@ else
>> pmdinfo += 'ar'
>> pmdinfogen += 'elf'
>> endif
>> +
>> +if get_option('enable_rust')
>> + meson.add_install_script('rust-env.sh')
>> +endif
>> diff --git a/buildtools/rust-env.sh b/buildtools/rust-env.sh
>> new file mode 100755
>> index 0000000000..705bc0d95b
>> --- /dev/null
>> +++ b/buildtools/rust-env.sh
>> @@ -0,0 +1,78 @@
>> +#! /bin/sh
>> +
>> +# Convert DPDK API files into RUST.
>> +# DPDK files selection is on demand.
>> +#
>> +# The coversion is done in 4 stages:
>> +# 1. Preparation [Optional]
>> +# Due to the bindgen conversion utility limitations source file may need
>> +# manual adjustment.
>> +# 2. Preprocessing [Mandatory]
>> +# Run preprocessor on a source file before conversion.
>> +# 3. Conversion [Mandatory]
>> +# Convert preprocessed C file into RUST file
>> +# 4. Post translation [Optional]
>> +# Manually fix translation.
>> +
>> +# DPDK files list
>> +files='
>> +rte_build_config.h
>> +rte_eal.h
>> +rte_ethdev.h
>> +rte_mbuf.h
>> +rte_mbuf_core.h
>> +rte_mempool.h
>> +'
>> +
>> +rust_dir="${MESON_INSTALL_DESTDIR_PREFIX}/rust"
>
> Here "rust_dir"="/usr/local/rust" if the default prefix is used.
> At least, a "dpdk" must be in there somewhere (we're not Rust itself after all!)
> but also different systems put files in e.g: "/usr/local/lib/x86_64-linux-gnu/dpdk"
>
> Appending a "/rust" to the end of that is probably what was intended?
Same as above.
>
>
>> +include_dir="${MESON_INSTALL_DESTDIR_PREFIX}/include"
>> +
>> +if test -d "$rust_dir"; then
>> + rm -rf "$rust_dir"
>> +fi
>> +
>> +mkdir -p "$rust_dir/src"
>> +if ! test -d "$rust_dir"; then
>> + echo "failed to create Rust library $rust_dir"
>> + exit 255
>> +fi
>> +touch "$rust_dir/src/lib.rs"
>> +
>> +bindgen_opt='--no-layout-tests --no-derive-debug'
>> +bindgen_clang_opt='-Wno-unused-command-line-argument'
>> +
>> +create_rust_lib ()
>> +{
>> + base=$1
>> +
>> + cp $include_dir/${base}.h /tmp/${base}.h
>> +
>> +# bindgen cannot process complex macro definitions
>> +# manually simplify macros before conversion
>> + sed -i -e 's/RTE_BIT64(\([0-9]*\))/(1UL << \1)/g' /tmp/${base}.h
>> + sed -i -e 's/RTE_BIT32(\([0-9]*\))/(1U << \1)/g' /tmp/${base}.h
>> + sed -i -e 's/UINT64_C(\([0-9]*\))/\1/g' /tmp/${base}.h
>> +
>> + # clang output has better integration with bindgen than GCC
>> + clang -E -dD -I$include_dir /tmp/${base}.h > /tmp/$base.i
>
> # Adding this helps debug the header conversion & output file locations
> echo "writing Rust converted header to ${rust_dir}/src/${base}.rs"
Meson add_install_script() dumps shell output.
>
>> + bindgen $bindgen_opt --output $rust_dir/src/$base.rs /tmp/$base.i -- $bindgen_clang_opt
>> + rm -f /tmp/$base.i /tmp/$base.h
>> +}
>> +
>> +for file in $files; do
>> + base=$(basename $file | cut -d. -f 1)
>> + create_rust_lib $base
>> + echo "pub mod $base;" >> "$rust_dir/src/lib.rs"
>> +done
>> +
>> +
>> +cat > "$rust_dir/Cargo.toml" <<EOF
>> +[package]
>> +name = "dpdklib"
>> +version = "$(cat $MESON_SOURCE_ROOT/VERSION | sed 's/\.0\([1-9]\)/\.\1/')"
>
> Missing {} brackets around MESON_SOURCE_ROOT perhaps? Got an error on "/VERSION" not found.
>
I'll fix that.
>> +EOF
>> +
>> +# post conversion updates
>> +# RUST does not accept aligned structures into packed structure.
>> +# TODO: fix DPDK definitions.
>> +sed -i 's/repr(align(2))/repr(packed(2))/g' "$rust_dir/src/rte_ethdev.rs"
>
> Hmm, interesting, lets keep a TODO here, but as this changes the "allowable" or valid binary layouts,
> it is potentially going to cause differences between C and Rust's interpretation of the memory backing the structs.
> TODO: investigate each struct individually, and determine if/what is really required/correct.
>
There was no actual violation in this case because network headers definitions
were properly aligned.
C compiler ether ignored it or managed to figure it out.
RUST does not allow to mix align() and packed() in general.
Adjusting C file will not change structures layout and reduce post-bindgen
corrections. Hence the TODO.
>
>> diff --git a/examples/rust/helloworld/Cargo.lock b/examples/rust/helloworld/Cargo.lock
>> new file mode 100644
>> index 0000000000..d18af80c66
>> --- /dev/null
>> +++ b/examples/rust/helloworld/Cargo.lock
>> @@ -0,0 +1,14 @@
>> +# This file is automatically @generated by Cargo.
>> +# It is not intended for manual editing.
>> +version = 4
>> +
>> +[[package]]
>> +name = "dpdklib"
>> +version = "25.3.0-rc1"
>> +
>> +[[package]]
>> +name = "helloworld"
>> +version = "0.1.0"
>> +dependencies = [
>> + "dpdklib",
>> +]
>> diff --git a/examples/rust/helloworld/Cargo.toml b/examples/rust/helloworld/Cargo.toml
>> new file mode 100644
>> index 0000000000..7f9a77968a
>> --- /dev/null
>> +++ b/examples/rust/helloworld/Cargo.toml
>> @@ -0,0 +1,7 @@
>> +[package]
>> +name = "helloworld"
>> +version = "0.1.0"
>> +edition = "2024"
>> +
>> +[dependencies]
>> +dpdklib = {path = "MESON_INSTALL_DESTDIR_PREFIX/rust"}
>> \ No newline at end of file
>
> Above Cargo.toml looks normal. Perhaps we could suggest/provide a method where
> DPDK does not need to be "ninja install"-ed, but instead process a local directory?
> That would likely make it easier to locally build/test Rust bindings.
>
Agree. RUST crate should have a dedicated installation command.
>> diff --git a/examples/rust/helloworld/build.rs b/examples/rust/helloworld/build.rs
>> new file mode 100644
>> index 0000000000..5c76188e18
>> --- /dev/null
>> +++ b/examples/rust/helloworld/build.rs
>> @@ -0,0 +1,21 @@
>> +use std::process::Command;
>> +
>> +pub fn main() {
>> + let mut pkgconfig = Command::new("pkg-config");
>> +
>> + match pkgconfig.args(["--libs", "libdpdk"]).output() {
>> + Ok (output) => {
>> + let stdout = String::from_utf8_lossy(&output.stdout).trim_end().to_string();
>> + for token in stdout.split_ascii_whitespace().filter(|s| !s.is_empty()) {
>> + if token.starts_with("-L") {
>> + println!("cargo::rustc-link-search=native={}", &token[2..]);
>> + } else if token.starts_with("-l") {
>> + println!("cargo::rustc-link-lib={}", &token[2..]);
>> + }
>> + }
>> + }
>> + Err(error) => {
>> + panic!("failed to read libdpdk package: {:?}", error);
>> + }
>> + }
>> +}
>
> Could a simple "probe()" like below replace the more complex logic? I've linked DPDK in the
> past using this more boring method.. or is there a reason that the link-search-native and
> link-lib parts have to be handled manually? Doing a Command::new() is hacky, nice to remove it if possible!
>
> if let Err(e) = pkg_config::Config::new().probe("libdpdk") {
> panic!("libdpdk probe failed: {e:?}");
> }
>
> The build.rs file itself also needs a "rebuild if self changed" line:
> println!("cargo:rerun-if-changed=build.rs");
>
I'll fix that.
>> diff --git a/examples/rust/helloworld/src/main.rs b/examples/rust/helloworld/src/main.rs
>> new file mode 100644
>> index 0000000000..cf666aece6
>> --- /dev/null
>> +++ b/examples/rust/helloworld/src/main.rs
>> @@ -0,0 +1,197 @@
>> +/// Usage: helloworld -a <port 1 params> -a <port 2 params> ...
>> +
>> +use std::env;
>> +use std::ffi::{CString};
>> +use std::os::raw::{c_int, c_char};
>> +use std::ffi::CStr;
>> +
>> +use dpdklib::rte_eal::{
>> + // Functions
>> + rte_eal_init,
>> + rte_eal_cleanup,
>> +};
>> +
>> +use dpdklib::rte_ethdev::{
>
> As per first reply, I'd like to see the end-result safe-Rust namespacing be shortened.
>
> This can be easily done by renaming the name= in Cargo.toml, and
> removing the "rte_" prefix at bindgen output stage for each file.
> dpdk::ethdev:: { RTE_ETH_NAME_MAX_LEN, ... };
>
> Perhaps its a good method to put the generated (unsafe, raw-C-bindings) in this
> namespace, and build a safe crate (with the dpdk::ethdev::* namespacing) over it.
> Thoughts?
>
The idea was to provide generic DPDK API and let application to resolve unsafe
interfaces.
>
>> + RTE_ETH_NAME_MAX_LEN,
>> + RTE_ETH_DEV_NO_OWNER,
>> + RTE_ETH_RSS_IP,
>> + rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_RSS,
>> + rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_VMDQ_DCB_RSS,
>> +
>> + // Structures
>> + rte_eth_dev_info,
>> + rte_eth_conf,
>> + rte_eth_txconf,
>> + rte_eth_rxconf,
>> +
>> + // Functions
>> + rte_eth_dev_get_name_by_port,
>> + rte_eth_find_next_owned_by,
>> + rte_eth_dev_info_get,
>> + rte_eth_dev_configure,
>> + rte_eth_tx_queue_setup,
>> + rte_eth_rx_queue_setup,
>> + rte_eth_dev_start,
>> +};
>> +
>> +use dpdklib::rte_build_config::{
>> + RTE_MAX_ETHPORTS,
>> +};
>> +
>> +use dpdklib::rte_mbuf::{
>> + rte_pktmbuf_pool_create,
>> +};
>> +
>> +use dpdklib::rte_mbuf_core::{
>> + RTE_MBUF_DEFAULT_BUF_SIZE
>> +};
>> +
>> +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 {
>
> This is where the first manual code is wrapping C concepts into Rust.
> And the approach taken here is quite "C thinking", where it is possible
> to misuse an API, and hence "unsafe" is used for many functions (more below)
> and also for the C FFI function calls.
>
> I believe this part can be improved, and that there is big value in those improvements:
> Today the code is "C but compiled by the Rust compiler", the programmer has to use the APIs correctly.
>
> I'll send a patch in reply to this one for review, suggesting ways to improve.
> Future code goal: "Rust checks code: invalid code will not compile, or provide a clean error at runtime."
>
>
Agree. The RUST part is pretty dull here.
The idea was to show how DPDK API can be used from RUST application.
>
>> + 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> {
>> + let mut port_id:DpdkPort = 0 as DpdkPort;
>> + std::iter::from_fn(move || {
>> + let cur = port_id;
>> + port_id = unsafe {
>> + rte_eth_find_next_owned_by(cur, owner_id) as DpdkPort
>> + };
>> + if port_id == RTE_MAX_ETHPORTS as DpdkPort {
>> + return None
>> + }
>> + if cur == port_id { port_id += 1 }
>> + Some(cur)
>> + })
>> +}
>> +
>> +pub unsafe fn iter_rte_eth_dev() -> impl Iterator<Item=DpdkPort> {
>> + unsafe {
>> + iter_rte_eth_dev_owned_by(RTE_ETH_DEV_NO_OWNER as u64)
>> + }
>> +}
>
> Cool - I like the egonomics idea of providing Iterator<DpdkPort>.
> The implementation here is unsafe, which is unfortunate, we'd like to
> ensure that all "end-user visible" Rust code is "safe Rust".
>
>
>> +pub unsafe fn init_port_config(port: &mut Port) {
>> + let ret = unsafe {
>> + rte_eth_dev_info_get(port.port_id, &mut port.dev_info as *mut rte_eth_dev_info)
>> + };
>> + if ret != 0 {
>> + panic!("port-{}: failed to get dev info {ret}", port.port_id);
>> + }
>> +
>> + port.dev_conf.rx_adv_conf.rss_conf.rss_key = std::ptr::null_mut();
>> + port.dev_conf.rx_adv_conf.rss_conf.rss_hf = if port.rxq_num > 1 {
>> + RTE_ETH_RSS_IP as u64 & port.dev_info.flow_type_rss_offloads
>> + } else {0};
>> +
>> + if port.dev_conf.rx_adv_conf.rss_conf.rss_hf != 0 {
>> + port.dev_conf.rxmode.mq_mode =
>> + rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_VMDQ_DCB_RSS & rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_RSS;
>> + }
>> +}
>> +
>> +pub unsafe fn show_ports_summary(ports: &Vec<Port>) {
>> + let mut name_buf:[c_char;RTE_ETH_NAME_MAX_LEN as usize]= [0 as c_char;RTE_ETH_NAME_MAX_LEN as usize];
>> + let title = format!("{:<4} {:<32} {:<14}", "Port", "Name", "Driver");
>> + println!("{title}");
>> + ports.iter().for_each(|p| unsafe {
>> + let _rc = rte_eth_dev_get_name_by_port(p.port_id, name_buf.as_mut_ptr());
>> + let name = CStr::from_ptr(name_buf.as_ptr());
>> + let drv = CStr::from_ptr(p.dev_info.driver_name);
>> + let summary = format!("{:<4} {:<32} {:<14}",
>> + p.port_id, name.to_str().unwrap(), drv.to_str().unwrap());
>> + println!("{summary}");
>> + });
>> +
>> +}
>
> There are some interesting changes of thoughts in Rust vs C, in how applications actually
> allocate or "own" data. The above show_ports_summary() function requires a &Vec<Port>.
> That demands that the backing storage for Port is a Vec<Port>, and a (immutable) reference
> is passed into this function. Hence the type is:
> summary(ports: &Vec<Port>)
>
> There is no reason to "demand" that the application stores Ports in a Vec. While common to store things
> in Vec<T>, we have now demanded "more than required" from the application. In some situations
> (perhaps not 100% relevant here, but..) like embedded systems, where dynamic allocation isn't available
> there IS no Vec<T> available. In other application scenarios, perhaps a HashMap is better.
>
> The best way to do this in Rust is to use the Iterator trait, which doesn't require any specific
> "backing storage" (e.g. linear-data for 'slice' or Vec, or HashMap per-item allocated data, or ...).
> Instead the Iterator trait allows the function itself to iterate of the "Item". Example:
>
> pub fn show_ports_summary_iterator<'a>(ports: impl Iterator<Item = &'a Port>) {
> ports.for_each(|p| { /* p is a &Port */);
> }
>
> There should be no unsafe required for the above debug print, and no statically sized C-string allocations.
> Instead of fetching the port name via C function at print-time, the Port::new() function can cache the value
> and convert it to a Rust String type - avoids unsafe {}, reduces scope of unsafe variables in the program.
>
>
>> +unsafe fn start_port(port:&mut Port) {
>> + let mut rc = unsafe {
>> + rte_eth_dev_configure(port.port_id, port.rxq_num, port.txq_num,
>> + &port.dev_conf as *const rte_eth_conf)
>> + };
>> + if rc != 0 { panic!("failed to configure port-{}: {rc}", port.port_id)}
>> + println!("port-{} configured", port.port_id);
>> +
>> + rc = unsafe {
>> + rte_eth_tx_queue_setup(port.port_id, 0, 64, 0, 0 as *const rte_eth_txconf)
>> + };
>> + if rc != 0 { panic!("port-{}: failed to configure TX queue 0 {rc}", port.port_id)}
>> + println!("port-{} configured TX queue 0", port.port_id);
>> +
>> + let mbuf_pool_name = CString::new(format!("mbuf pool port-{}", port.port_id)).unwrap();
>> + let mbuf_pool : *mut dpdklib::rte_mbuf::rte_mempool = unsafe {
>> + rte_pktmbuf_pool_create(mbuf_pool_name.as_ptr(), 1024, 0, 0,
>> + RTE_MBUF_DEFAULT_BUF_SIZE as u16, 0)
>> + };
>> + if mbuf_pool == 0 as *mut dpdklib::rte_mbuf::rte_mempool {
>> + panic!("port-{}: failed to allocate mempool {rc}", port.port_id)
>> + }
>> + println!("port-{} mempool ready", port.port_id);
>> +
>> + let mut rxq_conf:rte_eth_rxconf = port.dev_info.default_rxconf.clone();
>> + rxq_conf.offloads = 0;
>> + rc = unsafe {
>> + rte_eth_rx_queue_setup(port.port_id, 0, 64, 0,
>> + &mut rxq_conf as *mut rte_eth_rxconf,
>> + mbuf_pool as *mut dpdklib::rte_ethdev::rte_mempool)
>> + };
>> + if rc != 0 { panic!("port-{}: failed to configure RX queue 0 {rc}", port.port_id)}
>> + println!("port-{} configured RX queue 0", port.port_id);
>> + rc = unsafe {
>> + rte_eth_dev_start(port.port_id)
>> + };
>> + if rc != 0 { panic!("failed to start port-{}: {rc}", port.port_id)}
>> + println!("port-{} started", port.port_id);
>> +}
>
> Similar concepts around reworking "unsafe" away as above, future discussion.
>
>> +fn main() {
>> +
>> + let mut argv: Vec<*mut c_char> = env::args()
>> + .map(|arg| CString::new(arg).unwrap().into_raw()).collect();
>> +
>> + let rc = unsafe {
>> + rte_eal_init(env::args().len() as c_int, argv.as_mut_ptr())
>> + };
>> + if rc == -1 {
>> + unsafe { rte_eal_cleanup(); }
>> + }
>> +
>> + let mut ports:Vec<Port> = vec![];
>> + unsafe {
>> + for port_id in iter_rte_eth_dev()
>> + .take(dpdklib::rte_build_config::RTE_MAX_ETHPORTS as usize) {
>> + let mut port = Port::new(port_id);
>> + init_port_config(&mut port);
>> + println!("init port {port_id}");
>> + start_port(&mut port);
>> + ports.push(port);
>> + }
>> + }
>> +
>> + unsafe { show_ports_summary(&ports); }
>> +
>> + println!("Hello, world!");
>> +}
>
> Overall, I'm very happy to see this patch, I want to ensure folks on list hear that!
> The unsafe {} around each block of "do something" is not idiomatic Rust - we can improve that.
> The summary_iterator() function above can be called like a normal function:
>
> show_ports_summary_iterator(ports.iter());
>
>
>> diff --git a/meson_options.txt b/meson_options.txt
>> index e49b2fc089..d37b9ba1dc 100644
>> --- a/meson_options.txt
>> +++ b/meson_options.txt
>> @@ -60,3 +60,5 @@ option('tests', type: 'boolean', value: true, description:
>> 'build unit tests')
>> option('use_hpet', type: 'boolean', value: false, description:
>> 'use HPET timer in EAL')
>> +option('enable_rust', type: 'boolean', value: false, description:
>> + 'enable RUST')
>> --
>> 2.45.2
>
> Great work, and looking forward to hearing about next-steps, and discussion on the above feedback.
>
> Regards! (& thanks for reading till the end.. this turned into a long response :) -Harry
:thumbs up:
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: support DPDK API
2025-03-06 13:37 [PATCH] rust: support DPDK API Gregory Etelson
2025-03-06 19:26 ` Van Haaren, Harry
2025-03-07 15:54 ` Van Haaren, Harry
@ 2025-03-08 14:28 ` Igor Gutorov
2025-03-08 19:14 ` Etelson, Gregory
2025-03-08 18:50 ` [PATCH v2] rust: support raw " Gregory Etelson
3 siblings, 1 reply; 10+ messages in thread
From: Igor Gutorov @ 2025-03-08 14:28 UTC (permalink / raw)
To: Gregory Etelson; +Cc: dev, thomas, mkashani, Bruce Richardson
Hi Gregory!
As a DPDK as well as a Rust user, I'm quite excited about this patch.
I'm wondering though, is DPDK getting an official Rust API? I'm
subscribed to dev and user mailing lists, and haven't seen any
announcements - did I miss something?
Bellow are some stylistic suggestions
On Thu, Mar 6, 2025 at 4:38 PM Gregory Etelson <getelson@nvidia.com> wrote:
>
> The patch converts include files with DPDK API to RUST and binds new
> RUST API files info dpdklib package.
>
> The RUST dpdklib files and DPDK libraries build from C sources
> allow creation of DPDK application in RUST.
>
> RUST DPDK application must specify the `dpdklib` package as
> dependency in Cargo.toml file.
>
> RUST `dpdklib` package is installed into
> MESON_INSTALL_DESTDIR_PREFIX/rust directory.
>
> Software requirements:
> - clang
> - RUST installation
> - bindgen-cli crate
>
> RUST dpdklib installation instructions:
> 1. Configure DPDK with `-Deanble_rust=true`
> 2. Build and install DPDK. The installation procedure will create
> MESON_INSTALL_DESTDIR_PREFIX/rust directory.
> 3. Update PKG_CONFIG_PATH to point to DPDK installation.
>
> Signed-off-by: Gregory Etelson <getelson@nvidia.com>
> ---
> buildtools/meson.build | 4 +
> buildtools/rust-env.sh | 78 +++++++++++
> examples/rust/helloworld/Cargo.lock | 14 ++
> examples/rust/helloworld/Cargo.toml | 7 +
> examples/rust/helloworld/build.rs | 21 +++
> examples/rust/helloworld/src/main.rs | 197 +++++++++++++++++++++++++++
> meson_options.txt | 2 +
> 7 files changed, 323 insertions(+)
> create mode 100755 buildtools/rust-env.sh
> create mode 100644 examples/rust/helloworld/Cargo.lock
> create mode 100644 examples/rust/helloworld/Cargo.toml
> create mode 100644 examples/rust/helloworld/build.rs
> create mode 100644 examples/rust/helloworld/src/main.rs
>
> diff --git a/buildtools/meson.build b/buildtools/meson.build
> index 4e2c1217a2..dd16567f51 100644
> --- a/buildtools/meson.build
> +++ b/buildtools/meson.build
> @@ -50,3 +50,7 @@ else
> pmdinfo += 'ar'
> pmdinfogen += 'elf'
> endif
> +
> +if get_option('enable_rust')
> + meson.add_install_script('rust-env.sh')
> +endif
> diff --git a/buildtools/rust-env.sh b/buildtools/rust-env.sh
> new file mode 100755
> index 0000000000..705bc0d95b
> --- /dev/null
> +++ b/buildtools/rust-env.sh
> @@ -0,0 +1,78 @@
> +#! /bin/sh
> +
> +# Convert DPDK API files into RUST.
> +# DPDK files selection is on demand.
> +#
> +# The coversion is done in 4 stages:
nit: typo (coversion -> conversion)
> +# 1. Preparation [Optional]
> +# Due to the bindgen conversion utility limitations source file may need
> +# manual adjustment.
> +# 2. Preprocessing [Mandatory]
> +# Run preprocessor on a source file before conversion.
> +# 3. Conversion [Mandatory]
> +# Convert preprocessed C file into RUST file
> +# 4. Post translation [Optional]
> +# Manually fix translation.
> +
> +# DPDK files list
> +files='
> +rte_build_config.h
> +rte_eal.h
> +rte_ethdev.h
> +rte_mbuf.h
> +rte_mbuf_core.h
> +rte_mempool.h
> +'
> +
> +rust_dir="${MESON_INSTALL_DESTDIR_PREFIX}/rust"
> +include_dir="${MESON_INSTALL_DESTDIR_PREFIX}/include"
> +
> +if test -d "$rust_dir"; then
> + rm -rf "$rust_dir"
> +fi
> +
> +mkdir -p "$rust_dir/src"
> +if ! test -d "$rust_dir"; then
> + echo "failed to create Rust library $rust_dir"
> + exit 255
> +fi
> +touch "$rust_dir/src/lib.rs"
> +
> +bindgen_opt='--no-layout-tests --no-derive-debug'
> +bindgen_clang_opt='-Wno-unused-command-line-argument'
> +
> +create_rust_lib ()
> +{
> + base=$1
> +
> + cp $include_dir/${base}.h /tmp/${base}.h
> +
> +# bindgen cannot process complex macro definitions
> +# manually simplify macros before conversion
> + sed -i -e 's/RTE_BIT64(\([0-9]*\))/(1UL << \1)/g' /tmp/${base}.h
> + sed -i -e 's/RTE_BIT32(\([0-9]*\))/(1U << \1)/g' /tmp/${base}.h
> + sed -i -e 's/UINT64_C(\([0-9]*\))/\1/g' /tmp/${base}.h
> +
> + # clang output has better integration with bindgen than GCC
> + clang -E -dD -I$include_dir /tmp/${base}.h > /tmp/$base.i
> + bindgen $bindgen_opt --output $rust_dir/src/$base.rs /tmp/$base.i -- $bindgen_clang_opt
> + rm -f /tmp/$base.i /tmp/$base.h
> +}
> +
> +for file in $files; do
> + base=$(basename $file | cut -d. -f 1)
> + create_rust_lib $base
> + echo "pub mod $base;" >> "$rust_dir/src/lib.rs"
> +done
> +
> +
> +cat > "$rust_dir/Cargo.toml" <<EOF
> +[package]
> +name = "dpdklib"
> +version = "$(cat $MESON_SOURCE_ROOT/VERSION | sed 's/\.0\([1-9]\)/\.\1/')"
> +EOF
> +
> +# post conversion updates
> +# RUST does not accept aligned structures into packed structure.
> +# TODO: fix DPDK definitions.
> +sed -i 's/repr(align(2))/repr(packed(2))/g' "$rust_dir/src/rte_ethdev.rs"
> diff --git a/examples/rust/helloworld/Cargo.lock b/examples/rust/helloworld/Cargo.lock
> new file mode 100644
> index 0000000000..d18af80c66
> --- /dev/null
> +++ b/examples/rust/helloworld/Cargo.lock
> @@ -0,0 +1,14 @@
> +# This file is automatically @generated by Cargo.
> +# It is not intended for manual editing.
> +version = 4
> +
> +[[package]]
> +name = "dpdklib"
> +version = "25.3.0-rc1"
> +
> +[[package]]
> +name = "helloworld"
> +version = "0.1.0"
> +dependencies = [
> + "dpdklib",
> +]
> diff --git a/examples/rust/helloworld/Cargo.toml b/examples/rust/helloworld/Cargo.toml
> new file mode 100644
> index 0000000000..7f9a77968a
> --- /dev/null
> +++ b/examples/rust/helloworld/Cargo.toml
> @@ -0,0 +1,7 @@
> +[package]
> +name = "helloworld"
> +version = "0.1.0"
> +edition = "2024"
> +
> +[dependencies]
> +dpdklib = {path = "MESON_INSTALL_DESTDIR_PREFIX/rust"}
> \ No newline at end of file
> diff --git a/examples/rust/helloworld/build.rs b/examples/rust/helloworld/build.rs
> new file mode 100644
> index 0000000000..5c76188e18
> --- /dev/null
> +++ b/examples/rust/helloworld/build.rs
> @@ -0,0 +1,21 @@
> +use std::process::Command;
> +
> +pub fn main() {
> + let mut pkgconfig = Command::new("pkg-config");
> +
> + match pkgconfig.args(["--libs", "libdpdk"]).output() {
> + Ok (output) => {
> + let stdout = String::from_utf8_lossy(&output.stdout).trim_end().to_string();
> + for token in stdout.split_ascii_whitespace().filter(|s| !s.is_empty()) {
> + if token.starts_with("-L") {
> + println!("cargo::rustc-link-search=native={}", &token[2..]);
> + } else if token.starts_with("-l") {
> + println!("cargo::rustc-link-lib={}", &token[2..]);
> + }
> + }
> + }
> + Err(error) => {
> + panic!("failed to read libdpdk package: {:?}", error);
> + }
> + }
> +}
> diff --git a/examples/rust/helloworld/src/main.rs b/examples/rust/helloworld/src/main.rs
> new file mode 100644
> index 0000000000..cf666aece6
> --- /dev/null
> +++ b/examples/rust/helloworld/src/main.rs
> @@ -0,0 +1,197 @@
> +/// Usage: helloworld -a <port 1 params> -a <port 2 params> ...
> +
> +use std::env;
> +use std::ffi::{CString};
> +use std::os::raw::{c_int, c_char};
> +use std::ffi::CStr;
> +
> +use dpdklib::rte_eal::{
> + // Functions
> + rte_eal_init,
> + rte_eal_cleanup,
> +};
> +
> +use dpdklib::rte_ethdev::{
> + RTE_ETH_NAME_MAX_LEN,
> + RTE_ETH_DEV_NO_OWNER,
> + RTE_ETH_RSS_IP,
> + rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_RSS,
> + rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_VMDQ_DCB_RSS,
> +
> + // Structures
> + rte_eth_dev_info,
> + rte_eth_conf,
> + rte_eth_txconf,
> + rte_eth_rxconf,
> +
> + // Functions
> + rte_eth_dev_get_name_by_port,
> + rte_eth_find_next_owned_by,
> + rte_eth_dev_info_get,
> + rte_eth_dev_configure,
> + rte_eth_tx_queue_setup,
> + rte_eth_rx_queue_setup,
> + rte_eth_dev_start,
> +};
> +
> +use dpdklib::rte_build_config::{
> + RTE_MAX_ETHPORTS,
> +};
> +
> +use dpdklib::rte_mbuf::{
> + rte_pktmbuf_pool_create,
> +};
> +
> +use dpdklib::rte_mbuf_core::{
> + RTE_MBUF_DEFAULT_BUF_SIZE
> +};
> +
> +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> {
> + let mut port_id:DpdkPort = 0 as DpdkPort;
> + std::iter::from_fn(move || {
> + let cur = port_id;
> + port_id = unsafe {
> + rte_eth_find_next_owned_by(cur, owner_id) as DpdkPort
> + };
> + if port_id == RTE_MAX_ETHPORTS as DpdkPort {
> + return None
> + }
> + if cur == port_id { port_id += 1 }
> + Some(cur)
> + })
> +}
> +
> +pub unsafe fn iter_rte_eth_dev() -> impl Iterator<Item=DpdkPort> {
> + unsafe {
> + iter_rte_eth_dev_owned_by(RTE_ETH_DEV_NO_OWNER as u64)
> + }
> +}
It seems to me that it is expected to avoid iterating more than
`RTE_MAX_ETHPORTS` times.
I'd suggest to add a `.take()` call here:
pub unsafe fn iter_rte_eth_dev() -> impl Iterator<Item=DpdkPort> {
unsafe {
iter_rte_eth_dev_owned_by(RTE_ETH_DEV_NO_OWNER as u64)
}
.take(dpdklib::rte_build_config::RTE_MAX_ETHPORTS as usize)
}
Then the users of this function don't need to know about this detail.
Also, would this make `iter_rte_eth_dev()` safe?
> +
> +pub unsafe fn init_port_config(port: &mut Port) {
> + let ret = unsafe {
> + rte_eth_dev_info_get(port.port_id, &mut port.dev_info as *mut rte_eth_dev_info)
> + };
> + if ret != 0 {
> + panic!("port-{}: failed to get dev info {ret}", port.port_id);
> + }
> +
> + port.dev_conf.rx_adv_conf.rss_conf.rss_key = std::ptr::null_mut();
> + port.dev_conf.rx_adv_conf.rss_conf.rss_hf = if port.rxq_num > 1 {
> + RTE_ETH_RSS_IP as u64 & port.dev_info.flow_type_rss_offloads
> + } else {0};
> +
> + if port.dev_conf.rx_adv_conf.rss_conf.rss_hf != 0 {
> + port.dev_conf.rxmode.mq_mode =
> + rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_VMDQ_DCB_RSS & rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_RSS;
> + }
> +}
> +
> +pub unsafe fn show_ports_summary(ports: &Vec<Port>) {
Is it safe to remove `unsafe` here? Since `Port` is already
initialized, all of the calls inside this function should be valid,
unless I'm missing something.
> + let mut name_buf:[c_char;RTE_ETH_NAME_MAX_LEN as usize]= [0 as c_char;RTE_ETH_NAME_MAX_LEN as usize];
> + let title = format!("{:<4} {:<32} {:<14}", "Port", "Name", "Driver");
> + println!("{title}");
> + ports.iter().for_each(|p| unsafe {
> + let _rc = rte_eth_dev_get_name_by_port(p.port_id, name_buf.as_mut_ptr());
> + let name = CStr::from_ptr(name_buf.as_ptr());
> + let drv = CStr::from_ptr(p.dev_info.driver_name);
> + let summary = format!("{:<4} {:<32} {:<14}",
> + p.port_id, name.to_str().unwrap(), drv.to_str().unwrap());
> + println!("{summary}");
> + });
> +
> +}
> +unsafe fn start_port(port:&mut Port) {
Same here with `unsafe` as with `show_ports_summary`. It seems all
error conditions are checked, and the application terminates if
something goes wrong.
> + let mut rc = unsafe {
> + rte_eth_dev_configure(port.port_id, port.rxq_num, port.txq_num,
> + &port.dev_conf as *const rte_eth_conf)
> + };
> + if rc != 0 { panic!("failed to configure port-{}: {rc}", port.port_id)}
> + println!("port-{} configured", port.port_id);
> +
> + rc = unsafe {
> + rte_eth_tx_queue_setup(port.port_id, 0, 64, 0, 0 as *const rte_eth_txconf)
> + };
> + if rc != 0 { panic!("port-{}: failed to configure TX queue 0 {rc}", port.port_id)}
> + println!("port-{} configured TX queue 0", port.port_id);
> +
> + let mbuf_pool_name = CString::new(format!("mbuf pool port-{}", port.port_id)).unwrap();
> + let mbuf_pool : *mut dpdklib::rte_mbuf::rte_mempool = unsafe {
> + rte_pktmbuf_pool_create(mbuf_pool_name.as_ptr(), 1024, 0, 0,
> + RTE_MBUF_DEFAULT_BUF_SIZE as u16, 0)
> + };
> + if mbuf_pool == 0 as *mut dpdklib::rte_mbuf::rte_mempool {
nit: I'd suggest
if mbuf_pool.is_null() {
...
}
> + panic!("port-{}: failed to allocate mempool {rc}", port.port_id)
> + }
> + println!("port-{} mempool ready", port.port_id);
> +
> + let mut rxq_conf:rte_eth_rxconf = port.dev_info.default_rxconf.clone();
> + rxq_conf.offloads = 0;
> + rc = unsafe {
> + rte_eth_rx_queue_setup(port.port_id, 0, 64, 0,
> + &mut rxq_conf as *mut rte_eth_rxconf,
> + mbuf_pool as *mut dpdklib::rte_ethdev::rte_mempool)
This is quite pedantic, but it is often recommended to use `.cast()`
in raw pointer casting:
`mbuf_pool.cast::<dpdklib::rte_ethdev::rte_mempool>()` or even simply
`mbuf_pool.cast()`. [1].
> + };
> + if rc != 0 { panic!("port-{}: failed to configure RX queue 0 {rc}", port.port_id)}
> + println!("port-{} configured RX queue 0", port.port_id);
> + rc = unsafe {
> + rte_eth_dev_start(port.port_id)
> + };
> + if rc != 0 { panic!("failed to start port-{}: {rc}", port.port_id)}
> + println!("port-{} started", port.port_id);
> +}
> +
> +fn main() {
> +
> + let mut argv: Vec<*mut c_char> = env::args()
> + .map(|arg| CString::new(arg).unwrap().into_raw()).collect();
> +
> + let rc = unsafe {
> + rte_eal_init(env::args().len() as c_int, argv.as_mut_ptr())
> + };
> + if rc == -1 {
> + unsafe { rte_eal_cleanup(); }
> + }
> +
> + let mut ports:Vec<Port> = vec![];
> + unsafe {
> + for port_id in iter_rte_eth_dev()
> + .take(dpdklib::rte_build_config::RTE_MAX_ETHPORTS as usize) {
> + let mut port = Port::new(port_id);
> + init_port_config(&mut port);
> + println!("init port {port_id}");
> + start_port(&mut port);
> + ports.push(port);
> + }
> + }
If you prefer, this can be written like so:
let ports: Vec<Port> = unsafe {
iter_rte_eth_dev()
.take(dpdklib::rte_build_config::RTE_MAX_ETHPORTS as usize)
.map(|port_id| {
let mut port = Port::new(port_id);
init_port_config(&mut port);
println!("init port {port_id}");
start_port(&mut port);
port
})
.collect()
}
> +
> + unsafe { show_ports_summary(&ports); }
> +
> + println!("Hello, world!");
> +}
> diff --git a/meson_options.txt b/meson_options.txt
> index e49b2fc089..d37b9ba1dc 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -60,3 +60,5 @@ option('tests', type: 'boolean', value: true, description:
> 'build unit tests')
> option('use_hpet', type: 'boolean', value: false, description:
> 'use HPET timer in EAL')
> +option('enable_rust', type: 'boolean', value: false, description:
> + 'enable RUST')
> --
> 2.45.2
>
Some other thoughts/suggestions:
- Indentation/styling is a bit off or inconsistent. I'd suggest to run
`cargo fmt` to format the code. It is possible to configure the output
to match DPDK's needs [2]. Also, `cargo fmt --check` can check if
`rustfmt` would make any formatting changes. This could be a part of
the checks in `devtools` in the future.
- I haven't checked, but I think some of my suggestions would pop up
by `cargo clippy` analysis. It may be worth having this tool be a part
of the `devtools` checks as well in the future.
- 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] https://github.com/rust-lang/rust-clippy/issues/8017
[2] https://rust-lang.github.io/rustfmt/?version=master&search=
--
Regards,
Igor Gutorov
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] rust: support DPDK API
2025-03-08 14:28 ` Igor Gutorov
@ 2025-03-08 19:14 ` Etelson, Gregory
0 siblings, 0 replies; 10+ messages in thread
From: Etelson, Gregory @ 2025-03-08 19:14 UTC (permalink / raw)
To: Igor Gutorov; +Cc: Gregory Etelson, dev, thomas, mkashani, Bruce Richardson
Hello Igor,
> Hi Gregory!
>
> As a DPDK as well as a Rust user, I'm quite excited about this patch.
> I'm wondering though, is DPDK getting an official Rust API? I'm
> subscribed to dev and user mailing lists, and haven't seen any
> announcements - did I miss something?
>
> Bellow are some stylistic suggestions
>
There is no DPDK API for RUST yet.
My patch allows RUST application to use native DPDK API.
I'll adopt you coding style suggestions in the next patch release.
Regards,
Gregory
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2] rust: support raw DPDK API
2025-03-06 13:37 [PATCH] rust: support DPDK API Gregory Etelson
` (2 preceding siblings ...)
2025-03-08 14:28 ` Igor Gutorov
@ 2025-03-08 18:50 ` Gregory Etelson
3 siblings, 0 replies; 10+ messages in thread
From: Gregory Etelson @ 2025-03-08 18:50 UTC (permalink / raw)
To: harry.van.haaren, bruce.richardson, dev; +Cc: getelson, mkashani, thomas
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>
---
buildtools/meson.build | 4 +
buildtools/rust-env.sh | 81 +++++++++++
examples/rust/helloworld/Cargo.lock | 14 ++
examples/rust/helloworld/Cargo.toml | 7 +
examples/rust/helloworld/build.rs | 22 +++
examples/rust/helloworld/src/main.rs | 197 +++++++++++++++++++++++++++
meson_options.txt | 2 +
7 files changed, 327 insertions(+)
create mode 100755 buildtools/rust-env.sh
create mode 100644 examples/rust/helloworld/Cargo.lock
create mode 100644 examples/rust/helloworld/Cargo.toml
create mode 100644 examples/rust/helloworld/build.rs
create mode 100644 examples/rust/helloworld/src/main.rs
diff --git a/buildtools/meson.build b/buildtools/meson.build
index 4e2c1217a2..b9d0092f07 100644
--- a/buildtools/meson.build
+++ b/buildtools/meson.build
@@ -50,3 +50,7 @@ else
pmdinfo += 'ar'
pmdinfogen += 'elf'
endif
+
+if get_option('enable_rust')
+ meson.add_install_script(['rust-env.sh', get_option('libdir')])
+endif
diff --git a/buildtools/rust-env.sh b/buildtools/rust-env.sh
new file mode 100755
index 0000000000..fe0877643b
--- /dev/null
+++ b/buildtools/rust-env.sh
@@ -0,0 +1,81 @@
+#! /bin/sh
+
+# Convert DPDK API files into RUST.
+# DPDK files selection is on demand.
+#
+# The coversion is done in 4 stages:
+# 1. Preparation [Optional]
+# Due to the bindgen conversion utility limitations source file may need
+# manual adjustment.
+# 2. Preprocessing [Mandatory]
+# Run preprocessor on a source file before conversion.
+# 3. Conversion [Mandatory]
+# Convert preprocessed C file into RUST file
+# 4. Post translation [Optional]
+# Manually fix translation.
+
+# DPDK files list
+files='
+rte_build_config.h
+rte_eal.h
+rte_ethdev.h
+rte_mbuf.h
+rte_mbuf_core.h
+rte_mempool.h
+'
+libdir="$1"
+rust_dir="${MESON_INSTALL_DESTDIR_PREFIX}/$libdir/rust"
+include_dir="${MESON_INSTALL_DESTDIR_PREFIX}/include"
+
+if test -d "$rust_dir"; then
+ rm -rf "$rust_dir"
+fi
+
+mkdir -p "$rust_dir/src/raw"
+if ! test -d "$rust_dir"; then
+ echo "failed to create Rust library $rust_dir"
+ exit 255
+fi
+
+bindgen_opt='--no-layout-tests --no-derive-debug'
+bindgen_clang_opt='-Wno-unused-command-line-argument'
+
+create_rust_lib ()
+{
+ base=$1
+
+ cp $include_dir/${base}.h /tmp/${base}.h
+
+# bindgen cannot process complex macro definitions
+# manually simplify macros before conversion
+ sed -i -e 's/RTE_BIT64(\([0-9]*\))/(1UL << \1)/g' /tmp/${base}.h
+ sed -i -e 's/RTE_BIT32(\([0-9]*\))/(1U << \1)/g' /tmp/${base}.h
+ sed -i -e 's/UINT64_C(\([0-9]*\))/\1/g' /tmp/${base}.h
+
+ # clang output has better integration with bindgen than GCC
+ clang -E -dD -I$include_dir /tmp/${base}.h > /tmp/$base.i
+ bindgen $bindgen_opt --output $rust_dir/src/raw/$base.rs /tmp/$base.i -- $bindgen_clang_opt
+ rm -f /tmp/$base.i /tmp/$base.h
+}
+
+echo 'pub mod raw;' > "$rust_dir/src/lib.rs"
+
+touch "$rust_dir/src/raw/mod.rs"
+for file in $files; do
+ base=$(basename $file | cut -d. -f 1)
+ create_rust_lib $base
+ echo "pub mod $base;" >> "$rust_dir/src/raw/mod.rs"
+done
+
+cat > "$rust_dir/Cargo.toml" <<EOF
+[package]
+name = "dpdk"
+version = "$(cat ${MESON_SOURCE_ROOT}/VERSION | sed 's/\.0\([1-9]\)/\.\1/')"
+EOF
+
+# post conversion updates
+# RUST does not accept aligned structures into packed structure.
+# TODO: fix DPDK definitions.
+sed -i 's/repr(align(2))/repr(packed(2))/g' "$rust_dir/src/raw/rte_ethdev.rs"
+
+echo "Install RUST DPDK crate in $rust_dir"
diff --git a/examples/rust/helloworld/Cargo.lock b/examples/rust/helloworld/Cargo.lock
new file mode 100644
index 0000000000..d18af80c66
--- /dev/null
+++ b/examples/rust/helloworld/Cargo.lock
@@ -0,0 +1,14 @@
+# This file is automatically @generated by Cargo.
+# It is not intended for manual editing.
+version = 4
+
+[[package]]
+name = "dpdklib"
+version = "25.3.0-rc1"
+
+[[package]]
+name = "helloworld"
+version = "0.1.0"
+dependencies = [
+ "dpdklib",
+]
diff --git a/examples/rust/helloworld/Cargo.toml b/examples/rust/helloworld/Cargo.toml
new file mode 100644
index 0000000000..ceeecf958d
--- /dev/null
+++ b/examples/rust/helloworld/Cargo.toml
@@ -0,0 +1,7 @@
+[package]
+name = "helloworld"
+version = "0.1.0"
+edition = "2024"
+
+[dependencies]
+dpdk = {path = $MESON_INSTALL_DESTDIR_PREFIX/$libdir/rust }
diff --git a/examples/rust/helloworld/build.rs b/examples/rust/helloworld/build.rs
new file mode 100644
index 0000000000..f4cef36499
--- /dev/null
+++ b/examples/rust/helloworld/build.rs
@@ -0,0 +1,22 @@
+use std::process::Command;
+
+pub fn main() {
+ let mut pkgconfig = Command::new("pkg-config");
+
+ match pkgconfig.args(["--libs", "libdpdk"]).output() {
+ Ok (output) => {
+ let stdout = String::from_utf8_lossy(&output.stdout).trim_end().to_string();
+ for token in stdout.split_ascii_whitespace().filter(|s| !s.is_empty()) {
+ if token.starts_with("-L") {
+ println!("cargo::rustc-link-search=native={}", &token[2..]);
+ } else if token.starts_with("-l") {
+ println!("cargo::rustc-link-lib={}", &token[2..]);
+ }
+ }
+ println!("cargo:rerun-if-changed=build.rs");
+ }
+ Err(error) => {
+ panic!("failed to read libdpdk package: {:?}", error);
+ }
+ }
+}
diff --git a/examples/rust/helloworld/src/main.rs b/examples/rust/helloworld/src/main.rs
new file mode 100644
index 0000000000..7f8d5c58c7
--- /dev/null
+++ b/examples/rust/helloworld/src/main.rs
@@ -0,0 +1,197 @@
+/// Usage: helloworld -a <port 1 params> -a <port 2 params> ...
+
+use std::env;
+use std::ffi::{CString};
+use std::os::raw::{c_int, c_char};
+use std::ffi::CStr;
+
+use dpdk::raw::rte_eal::{
+ // Functions
+ rte_eal_init,
+ rte_eal_cleanup,
+};
+
+use dpdk::raw::rte_ethdev::{
+ RTE_ETH_NAME_MAX_LEN,
+ RTE_ETH_DEV_NO_OWNER,
+ RTE_ETH_RSS_IP,
+ rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_RSS,
+ rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_VMDQ_DCB_RSS,
+
+ // Structures
+ rte_eth_dev_info,
+ rte_eth_conf,
+ rte_eth_txconf,
+ rte_eth_rxconf,
+
+ // Functions
+ rte_eth_dev_get_name_by_port,
+ rte_eth_find_next_owned_by,
+ rte_eth_dev_info_get,
+ rte_eth_dev_configure,
+ rte_eth_tx_queue_setup,
+ rte_eth_rx_queue_setup,
+ rte_eth_dev_start,
+};
+
+use dpdk::raw::rte_build_config::{
+ RTE_MAX_ETHPORTS,
+};
+
+use dpdk::raw::rte_mbuf::{
+ rte_pktmbuf_pool_create,
+};
+
+use dpdk::raw::rte_mbuf_core::{
+ RTE_MBUF_DEFAULT_BUF_SIZE
+};
+
+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> {
+ let mut port_id:DpdkPort = 0 as DpdkPort;
+ std::iter::from_fn(move || {
+ let cur = port_id;
+ port_id = unsafe {
+ rte_eth_find_next_owned_by(cur, owner_id) as DpdkPort
+ };
+ if port_id == RTE_MAX_ETHPORTS as DpdkPort {
+ return None
+ }
+ if cur == port_id { port_id += 1 }
+ Some(cur)
+ })
+}
+
+pub unsafe fn iter_rte_eth_dev() -> impl Iterator<Item=DpdkPort> {
+ unsafe {
+ iter_rte_eth_dev_owned_by(RTE_ETH_DEV_NO_OWNER as u64)
+ }
+}
+
+pub unsafe fn init_port_config(port: &mut Port) {
+ let ret = unsafe {
+ rte_eth_dev_info_get(port.port_id, &mut port.dev_info as *mut rte_eth_dev_info)
+ };
+ if ret != 0 {
+ panic!("port-{}: failed to get dev info {ret}", port.port_id);
+ }
+
+ port.dev_conf.rx_adv_conf.rss_conf.rss_key = std::ptr::null_mut();
+ port.dev_conf.rx_adv_conf.rss_conf.rss_hf = if port.rxq_num > 1 {
+ RTE_ETH_RSS_IP as u64 & port.dev_info.flow_type_rss_offloads
+ } else {0};
+
+ if port.dev_conf.rx_adv_conf.rss_conf.rss_hf != 0 {
+ port.dev_conf.rxmode.mq_mode =
+ rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_VMDQ_DCB_RSS & rte_eth_rx_mq_mode_RTE_ETH_MQ_RX_RSS;
+ }
+}
+
+pub unsafe fn show_ports_summary(ports: &Vec<Port>) {
+ let mut name_buf:[c_char;RTE_ETH_NAME_MAX_LEN as usize]= [0 as c_char;RTE_ETH_NAME_MAX_LEN as usize];
+ let title = format!("{:<4} {:<32} {:<14}", "Port", "Name", "Driver");
+ println!("{title}");
+ ports.iter().for_each(|p| unsafe {
+ let _rc = rte_eth_dev_get_name_by_port(p.port_id, name_buf.as_mut_ptr());
+ let name = CStr::from_ptr(name_buf.as_ptr());
+ let drv = CStr::from_ptr(p.dev_info.driver_name);
+ let summary = format!("{:<4} {:<32} {:<14}",
+ p.port_id, name.to_str().unwrap(), drv.to_str().unwrap());
+ println!("{summary}");
+ });
+
+}
+unsafe fn start_port(port:&mut Port) {
+ let mut rc = unsafe {
+ rte_eth_dev_configure(port.port_id, port.rxq_num, port.txq_num,
+ &port.dev_conf as *const rte_eth_conf)
+ };
+ if rc != 0 { panic!("failed to configure port-{}: {rc}", port.port_id)}
+ println!("port-{} configured", port.port_id);
+
+ rc = unsafe {
+ rte_eth_tx_queue_setup(port.port_id, 0, 64, 0, 0 as *const rte_eth_txconf)
+ };
+ if rc != 0 { panic!("port-{}: failed to configure TX queue 0 {rc}", port.port_id)}
+ println!("port-{} configured TX queue 0", port.port_id);
+
+ let mbuf_pool_name = CString::new(format!("mbuf pool port-{}", port.port_id)).unwrap();
+ let mbuf_pool : *mut dpdk::raw::rte_mbuf::rte_mempool = unsafe {
+ rte_pktmbuf_pool_create(mbuf_pool_name.as_ptr(), 1024, 0, 0,
+ RTE_MBUF_DEFAULT_BUF_SIZE as u16, 0)
+ };
+ if mbuf_pool == 0 as *mut dpdk::raw::rte_mbuf::rte_mempool {
+ panic!("port-{}: failed to allocate mempool {rc}", port.port_id)
+ }
+ println!("port-{} mempool ready", port.port_id);
+
+ let mut rxq_conf:rte_eth_rxconf = port.dev_info.default_rxconf.clone();
+ rxq_conf.offloads = 0;
+ rc = unsafe {
+ rte_eth_rx_queue_setup(port.port_id, 0, 64, 0,
+ &mut rxq_conf as *mut rte_eth_rxconf,
+ mbuf_pool as *mut dpdk::raw::rte_ethdev::rte_mempool)
+ };
+ if rc != 0 { panic!("port-{}: failed to configure RX queue 0 {rc}", port.port_id)}
+ println!("port-{} configured RX queue 0", port.port_id);
+ rc = unsafe {
+ rte_eth_dev_start(port.port_id)
+ };
+ if rc != 0 { panic!("failed to start port-{}: {rc}", port.port_id)}
+ println!("port-{} started", port.port_id);
+}
+
+fn main() {
+
+ let mut argv: Vec<*mut c_char> = env::args()
+ .map(|arg| CString::new(arg).unwrap().into_raw()).collect();
+
+ let rc = unsafe {
+ rte_eal_init(env::args().len() as c_int, argv.as_mut_ptr())
+ };
+ if rc == -1 {
+ unsafe { rte_eal_cleanup(); }
+ }
+
+ let mut ports:Vec<Port> = vec![];
+ unsafe {
+ for port_id in iter_rte_eth_dev()
+ .take(dpdk::raw::rte_build_config::RTE_MAX_ETHPORTS as usize) {
+ let mut port = Port::new(port_id);
+ init_port_config(&mut port);
+ println!("init port {port_id}");
+ start_port(&mut port);
+ ports.push(port);
+ }
+ }
+
+ unsafe { show_ports_summary(&ports); }
+
+ println!("Hello, world!");
+}
diff --git a/meson_options.txt b/meson_options.txt
index e49b2fc089..d37b9ba1dc 100644
--- a/meson_options.txt
+++ b/meson_options.txt
@@ -60,3 +60,5 @@ option('tests', type: 'boolean', value: true, description:
'build unit tests')
option('use_hpet', type: 'boolean', value: false, description:
'use HPET timer in EAL')
+option('enable_rust', type: 'boolean', value: false, description:
+ 'enable RUST')
--
2.45.2
^ permalink raw reply [flat|nested] 10+ messages in thread