On Fri, 7 Mar 2025, Van Haaren, Harry wrote:
External email: Use caution opening links or attachments
From: Gregory Etelson <getel...@nvidia.com>
Sent: Thursday, March 6, 2025 1:37 PM
To: dev@dpdk.org <dev@dpdk.org>
Cc: getel...@nvidia.com <getel...@nvidia.com>; tho...@monjalon.net <tho...@monjalon.net>;
mkash...@nvidia.com <mkash...@nvidia.com>; Richardson, Bruce <bruce.richard...@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 <getel...@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:
