External email: Use caution opening links or attachments


> From: Etelson, Gregory
> Sent: Thursday, April 17, 2025 7:58 PM
> To: Van Haaren, Harry
> Cc: dev@dpdk.org; getelson@nvidia.com; Richardson, Bruce; owen.hilyard@unh.edu
> Subject: Re: [PATCH] rust: RFC/demo of safe API for Dpdk Eal, Eth and Rxq
>
> Hello Harry,
>
> Thank you for sharing the API.
> Please check out my comments below.

Thanks for reading & discussion!

<snip>

> > +
> > +            pub fn start(&mut self) -> (Vec<RxqHandle>, Vec<TxqHandle>) {
> > +                // call rte_eth_dev_start() here, then give ownership of Rxq/Txq to app
>
> After a call to Port::start, Rx and Tx queues are detached from it's port.
> With that model how rte_eth_dev_stop() and subsequent rte_eth_dev_start()
> DPDK calls can be implemented ?

Correct, the RxqHandle and TxqHandle don't have a "back reference" to the port.
There are a number of ways to ensure eth_dev_stop() cannot be called without the
Rxq/Txqs being "returned" to the Port instance first.

Eg: Use an Arc<T>. The port instance "owns" the Arc<T>, which means it is going to keep
   the Arc alive. Now give each Rxq/Txq a clone of this Arc. When the Drop impl of the
   Rxq/Txq runs, it will decrement the Arc. So just letting the Rxq/Txq go out of scope
   will be enough to have the Port understand that handle is now gone.

   The port itself can use Arc::into_inner function[1], which returns Option<T>. If the
   Some(T) is returned, then all instances of RxqHandle/TxqHandle have been dropped,
   meaning it is safe to eth_dev_stop(), as it is impossible to poll RXQs if there's no Rxq :)
   [1] https://doc.rust-lang.org/std/sync/struct.Arc.html#method.into_inner

// Pseudo-code here:
Dpdk::Eth::Port::stop(&mut self) -> Result<(), Error> {
    let handles_dropped = self.handle_arc.into_inner(); // returns "T" if its the only reference to the Arc
    if handles_dropped.is_none() {
        return Err("an Rxq or Txq handle remains alive, cannot safely stop this port");
    }
}

There's probably a few others, but that's "idiomatic Rust" solution.
We'd have to pass the Arc from the RxqHandle into the Rxq instance itself too,
but that's fine.

<snip>

> > +fn main() {
> > +    let mut dpdk = dpdk::Eal::init().expect("dpdk must init ok");
> > +    let rx_mempool = dpdk::Mempool::new(4096);
> > +
> > +    let mut ports = dpdk.take_eth_ports().expect("take eth ports ok");
>
> Eal::take_eth_ports() resets EAL ports.

I don't think it "resets" here. The "take eth ports" removes the Port instances from
the dpdk::Eal struct, but there's no "reset" behaviour.

> A call to rte_dev_probe() will ether fail, because Eal::eth_ports is None
> or create another port-0, depending on implementation.

I don't see how or why rte_dev_probe() would be called. The idea is not to allow Rust
apps call DPDK C APIs "when they want". The safe Rust API provides the required abstraction.
So its not possible to have another call to rte_dev_probe(), after the 1st time under eal_init().

Similar topic: Hotplug. I have experience with designing C APIs around hotplug
use-cases (Music/DJ software, from before my DPDK/networking days!). I think DPDK has
an interesting "push hotplug" approach (aka, App makes a function call to "request" the device).
Then on successful return, we can call rte_eth_dev_get_port_by_name() to get the u16 port_id,
and build the Port instance from that. Outline API:

enum EalHotplugDev {
    EthDev(Dpdk::Eth::Port), // enums can have contents in Rust :)
    CryptoDev(Dpdk::Crypto),
    // Etc
}

Eal::hotplug_add(bus: String, dev: String, args: String) -> Result<EalHotplugDev, Error> {
    // TODO: call rte_eal_hotplug_add()
    // TODO: identify how to know if its an Eth, Crypto, Dma, or other dev type?
    match (dev_type) {
        "eth" => {
            let port_id = rte_eth_dev_get_port_by_name(dev);
            EalHotplugDev::EthDev( Dpdk::Eth::Port::new(port_id) )
        }
    }
}

Applications could then do:
  let Ok(dev) = eal.hotplug_add("pci", "02:00.0", "dev_option=true") else {
      // failed to hotplug, log error?
      return;
  }
  match (dev) {
      EthDev => {
          // handle the dev here, e.g. configure & spawn thread to poll Rxq like before.
      }
  }

I like having an outline of difficult to "bolt on" features (hotplug is typically hard to add later..)
but I recommend we focus on getting core APIs and such running before more detail/time/implementation here.


Regards, -Harry