[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZJN9WmRCJU8nN9jE@boqun-archlinux>
Date: Wed, 21 Jun 2023 15:44:42 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: FUJITA Tomonori <fujita.tomonori@...il.com>
Cc: netdev@...r.kernel.org, rust-for-linux@...r.kernel.org,
aliceryhl@...gle.com, andrew@...n.ch,
miguel.ojeda.sandonis@...il.com
Subject: Re: [PATCH 1/5] rust: core abstractions for network device drivers
On Tue, Jun 13, 2023 at 01:53:22PM +0900, FUJITA Tomonori wrote:
> This patch adds very basic abstractions to implement network device
> drivers, corresponds to the kernel's net_device and net_device_ops
> structs with support for register_netdev/unregister_netdev functions.
>
> allows the const_maybe_uninit_zeroed feature for
> core::mem::MaybeUinit::<T>::zeroed() in const function.
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@...il.com>
> ---
> rust/bindings/bindings_helper.h | 2 +
> rust/helpers.c | 16 ++
> rust/kernel/lib.rs | 3 +
> rust/kernel/net.rs | 5 +
> rust/kernel/net/dev.rs | 344 ++++++++++++++++++++++++++++++++
> 5 files changed, 370 insertions(+)
> create mode 100644 rust/kernel/net.rs
> create mode 100644 rust/kernel/net/dev.rs
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 3e601ce2548d..468bf606f174 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -7,6 +7,8 @@
> */
>
> #include <linux/errname.h>
> +#include <linux/etherdevice.h>
> +#include <linux/netdevice.h>
> #include <linux/slab.h>
> #include <linux/refcount.h>
> #include <linux/wait.h>
> diff --git a/rust/helpers.c b/rust/helpers.c
> index bb594da56137..70d50767ff4e 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -24,10 +24,26 @@
> #include <linux/errname.h>
> #include <linux/refcount.h>
> #include <linux/mutex.h>
> +#include <linux/netdevice.h>
> +#include <linux/skbuff.h>
> #include <linux/spinlock.h>
> #include <linux/sched/signal.h>
> #include <linux/wait.h>
>
> +#ifdef CONFIG_NET
> +void *rust_helper_netdev_priv(const struct net_device *dev)
> +{
> + return netdev_priv(dev);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_netdev_priv);
> +
> +void rust_helper_skb_tx_timestamp(struct sk_buff *skb)
> +{
> + skb_tx_timestamp(skb);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_skb_tx_timestamp);
> +#endif
> +
> __noreturn void rust_helper_BUG(void)
> {
> BUG();
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 85b261209977..fc7d048d359d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -13,6 +13,7 @@
>
> #![no_std]
> #![feature(allocator_api)]
> +#![feature(const_maybe_uninit_zeroed)]
> #![feature(coerce_unsized)]
> #![feature(dispatch_from_dyn)]
> #![feature(new_uninit)]
> @@ -34,6 +35,8 @@
> pub mod error;
> pub mod init;
> pub mod ioctl;
> +#[cfg(CONFIG_NET)]
> +pub mod net;
> pub mod prelude;
> pub mod print;
> mod static_assert;
> diff --git a/rust/kernel/net.rs b/rust/kernel/net.rs
> new file mode 100644
> index 000000000000..28fe8f398463
> --- /dev/null
> +++ b/rust/kernel/net.rs
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Networking core.
> +
> +pub mod dev;
> diff --git a/rust/kernel/net/dev.rs b/rust/kernel/net/dev.rs
> new file mode 100644
> index 000000000000..d072c81f99ce
> --- /dev/null
> +++ b/rust/kernel/net/dev.rs
> @@ -0,0 +1,344 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Network device.
> +//!
> +//! C headers: [`include/linux/etherdevice.h`](../../../../include/linux/etherdevice.h),
> +//! [`include/linux/ethtool.h`](../../../../include/linux/ethtool.h),
> +//! [`include/linux/netdevice.h`](../../../../include/linux/netdevice.h),
> +//! [`include/linux/skbuff.h`](../../../../include/linux/skbuff.h),
> +//! [`include/uapi/linux/if_link.h`](../../../../include/uapi/linux/if_link.h).
> +
> +use crate::{bindings, error::*, prelude::vtable, types::ForeignOwnable};
> +use {core::ffi::c_void, core::marker::PhantomData};
> +
> +/// Corresponds to the kernel's `struct net_device`.
> +///
> +/// # Invariants
> +///
> +/// The pointer is valid.
> +pub struct Device(*mut bindings::net_device);
> +
> +impl Device {
> + /// Creates a new [`Device`] instance.
> + ///
> + /// # Safety
> + ///
> + /// Callers must ensure that `ptr` must be valid.
> + unsafe fn from_ptr(ptr: *mut bindings::net_device) -> Self {
> + // INVARIANT: The safety requirements ensure the invariant.
> + Self(ptr)
> + }
> +
> + /// Gets a pointer to network device private data.
> + fn priv_data_ptr(&self) -> *const c_void {
> + // SAFETY: The type invariants guarantee that `self.0` is valid.
> + // During the initialization of `Registration` instance, the kernel allocates
> + // contiguous memory for `struct net_device` and a pointer to its private data.
> + // So it's safe to read an address from the returned address from `netdev_priv()`.
> + unsafe { core::ptr::read(bindings::netdev_priv(self.0) as *const *const c_void) }
> + }
> +}
> +
> +// SAFETY: `Device` is just a wrapper for the kernel`s `struct net_device`, which can be used
> +// from any thread. `struct net_device` stores a pointer to `DriverData::Data`, which is `Sync`
> +// so it's safe to sharing its pointer.
> +unsafe impl Send for Device {}
> +// SAFETY: `Device` is just a wrapper for the kernel`s `struct net_device`, which can be used
> +// from any thread. `struct net_device` stores a pointer to `DriverData::Data`, which is `Sync`,
> +// can be used from any thread too.
> +unsafe impl Sync for Device {}
> +
> +/// Trait for device driver specific information.
> +///
> +/// This data structure is passed to a driver with the operations for `struct net_device`
> +/// like `struct net_device_ops`, `struct ethtool_ops`, `struct rtnl_link_ops`, etc.
> +pub trait DriverData {
> + /// The object are stored in C object, `struct net_device`.
> + type Data: ForeignOwnable + Send + Sync;
> +}
> +
> +/// Registration structure for a network device driver.
> +///
> +/// This allocates and owns a `struct net_device` object.
> +/// Once the `net_device` object is registered via `register_netdev` function,
> +/// the kernel calls various functions such as `struct net_device_ops` operations with
> +/// the `net_device` object.
> +///
> +/// A driver must implement `struct net_device_ops` so the trait for it is tied.
> +/// Other operations like `struct ethtool_ops` are optional.
> +pub struct Registration<T: DeviceOperations<D>, D: DriverData> {
> + dev: Device,
> + is_registered: bool,
> + _p: PhantomData<(D, T)>,
> +}
> +
> +impl<D: DriverData, T: DeviceOperations<D>> Drop for Registration<T, D> {
> + fn drop(&mut self) {
> + // SAFETY: The type invariants guarantee that `self.dev.0` is valid.
> + unsafe {
> + let _ = D::Data::from_foreign(self.dev.priv_data_ptr());
> + if self.is_registered {
> + bindings::unregister_netdev(self.dev.0);
> + }
> + bindings::free_netdev(self.dev.0);
> + }
> + }
> +}
> +
> +impl<D: DriverData, T: DeviceOperations<D>> Registration<T, D> {
> + /// Creates a new [`Registration`] instance for ethernet device.
> + ///
> + /// A device driver can pass private data.
> + pub fn try_new_ether(tx_queue_size: u32, rx_queue_size: u32, data: D::Data) -> Result<Self> {
> + // SAFETY: FFI call.
> + let ptr = from_err_ptr(unsafe {
> + bindings::alloc_etherdev_mqs(
> + core::mem::size_of::<*const c_void>() as i32,
> + tx_queue_size,
> + rx_queue_size,
> + )
> + })?;
> +
> + // SAFETY: `ptr` is valid and non-null since `alloc_etherdev_mqs()`
> + // returned a valid pointer which was null-checked.
Hmm.. neither alloc_etherdev_mqs() nor `from_err_ptr` would do the
null-check IIUC, so you may get a `ptr` whose values is null here.
Regards,
Boqun
> + let dev = unsafe { Device::from_ptr(ptr) };
> + // SAFETY: It's safe to write an address to the returned pointer
> + // from `netdev_priv()` because `alloc_etherdev_mqs()` allocates
> + // contiguous memory for `struct net_device` and a pointer.
> + unsafe {
> + let priv_ptr = bindings::netdev_priv(ptr) as *mut *const c_void;
> + core::ptr::write(priv_ptr, data.into_foreign());
> + }
> + Ok(Registration {
> + dev,
> + is_registered: false,
> + _p: PhantomData,
> + })
> + }
> +
[...]
Powered by blists - more mailing lists