[PATCH v2 0/5] Rust abstractions for network device drivers

[PATCH v2 0/5] Rust abstractions for network device drivers

[FUJITA Tomonori]

This patchset adds minimum Rust abstractions for network device
drivers and an example of a Rust network device driver, a simpler
version of drivers/net/dummy.c.

The major change is a way to drop an skb (1/5 patch); a driver needs
to explicitly call a function to drop a skb. The code to let a skb
go out of scope can't be compiled.

I dropped get_stats64 support patch that the current sample driver
doesn't use. Instead I added a patch to update the NETWORKING DRIVERS
entry in MAINTAINERS.

Changes since v1 [1]:
- a driver must explicitly call a function to drop a skb.
- simplify the code (thanks to Benno Lossin).
- update MAINTAINERS file.

[1] https://lwn.net/ml/netdev/20230613045326.3938283-1-fujita.tomonori@gmail.com/

FUJITA Tomonori (5):
  rust: core abstractions for network device drivers
  rust: add support for ethernet operations
  rust: add methods for configure net_device
  samples: rust: add dummy network driver
  MAINTAINERS: add Rust network abstractions files to the NETWORKING
    DRIVERS entry

 MAINTAINERS                     |   2 +
 rust/bindings/bindings_helper.h |   3 +
 rust/helpers.c                  |  23 ++
 rust/kernel/lib.rs              |   3 +
 rust/kernel/net.rs              |   5 +
 rust/kernel/net/dev.rs          | 598 ++++++++++++++++++++++++++++++++
 samples/rust/Kconfig            |  13 +
 samples/rust/Makefile           |   1 +
 samples/rust/rust_net_dummy.rs  |  75 ++++
 scripts/Makefile.build          |   2 +-
 10 files changed, 724 insertions(+), 1 deletion(-)
 create mode 100644 rust/kernel/net.rs
 create mode 100644 rust/kernel/net/dev.rs
 create mode 100644 samples/rust/rust_net_dummy.rs


base-commit: d2e3115d717197cb2bc020dd1f06b06538474ac3
-- 
2.34.1

[PATCH v2 1/5] rust: core abstractions for network device drivers

[FUJITA Tomonori]

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@gmail.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          | 330 ++++++++++++++++++++++++++++++++
 5 files changed, 356 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..fe20616668a9
--- /dev/null
+++ b/rust/kernel/net/dev.rs
@@ -0,0 +1,330 @@
+// 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, build_error, error::*, prelude::vtable, types::ForeignOwnable};
+use {core::ffi::c_void, core::marker::PhantomData};
+
+/// Corresponds to the kernel's `struct net_device`.
+///
+/// # Invariants
+///
+/// The `ptr` points to the contiguous memory for `struct net_device` and a pointer,
+/// which stores an address returned by `ForeignOwnable::into_foreign()`.
+pub struct Device<D: ForeignOwnable + Send + Sync> {
+    ptr: *mut bindings::net_device,
+    _p: PhantomData<D>,
+}
+
+impl<D: ForeignOwnable + Send + Sync> Device<D> {
+    /// Creates a new [`Device`] instance.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` must point to the contiguous memory
+    /// for `struct net_device` and a pointer, which stores an address returned
+    /// by `ForeignOwnable::into_foreign()`.
+    unsafe fn from_ptr(ptr: *mut bindings::net_device) -> Self {
+        // INVARIANT: The safety requirements ensure the invariant.
+        Self {
+            ptr,
+            _p: PhantomData,
+        }
+    }
+
+    /// Gets the private data of a device driver.
+    pub fn drv_priv_data(&self) -> D::Borrowed<'_> {
+        // SAFETY: The type invariants guarantee that self.ptr is valid and
+        // bindings::netdev_priv(self.ptr) returns a pointer that stores an address
+        // returned by `ForeignOwnable::into_foreign()`.
+        unsafe {
+            D::borrow(core::ptr::read(
+                bindings::netdev_priv(self.ptr) 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 an object, which is `Sync`
+// so it's safe to sharing its pointer.
+unsafe impl<D: ForeignOwnable + Send + Sync> Send for Device<D> {}
+// 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 an object, which is `Sync`,
+// can be used from any thread too.
+unsafe impl<D: ForeignOwnable + Send + Sync> Sync for Device<D> {}
+
+/// 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> {
+    dev: Device<T>,
+    is_registered: bool,
+    _p: PhantomData<T>,
+}
+
+impl<T: DeviceOperations> Drop for Registration<T> {
+    fn drop(&mut self) {
+        // SAFETY: The type invariants of `Device` guarantee that `self.dev.ptr` is valid and
+        // bindings::netdev_priv(self.ptr) returns a pointer that stores an address
+        // returned by `ForeignOwnable::into_foreign()`.
+        unsafe {
+            let _ = T::from_foreign(core::ptr::read(
+                bindings::netdev_priv(self.dev.ptr) as *const *const c_void
+            ));
+        }
+        // SAFETY: The type invariants of `Device` guarantee that `self.dev.ptr` is valid.
+        unsafe {
+            if self.is_registered {
+                bindings::unregister_netdev(self.dev.ptr);
+            }
+            bindings::free_netdev(self.dev.ptr);
+        }
+    }
+}
+
+impl<T: DeviceOperations> Registration<T> {
+    /// 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: T) -> Result<Self> {
+        // SAFETY: Just an FFI call with no additional safety requirements.
+        let ptr = unsafe {
+            bindings::alloc_etherdev_mqs(
+                core::mem::size_of::<*const c_void>() as i32,
+                tx_queue_size,
+                rx_queue_size,
+            )
+        };
+        if ptr.is_null() {
+            return Err(code::ENOMEM);
+        }
+
+        // SAFETY: It's safe to write an address 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());
+        }
+
+        // SAFETY: `ptr` points to contiguous memory for `struct net_device` and a pointer,
+        // which stores an address returned by `ForeignOwnable::into_foreign()`.
+        let dev = unsafe { Device::from_ptr(ptr) };
+        Ok(Registration {
+            dev,
+            is_registered: false,
+            _p: PhantomData,
+        })
+    }
+
+    /// Returns a network device.
+    ///
+    /// A device driver normally configures the device before registration.
+    pub fn dev_get(&mut self) -> &mut Device<T> {
+        &mut self.dev
+    }
+
+    /// Registers a network device.
+    pub fn register(&mut self) -> Result {
+        if self.is_registered {
+            return Err(code::EINVAL);
+        }
+        // SAFETY: The type invariants guarantee that `self.dev.ptr` is valid.
+        let ret = unsafe {
+            (*self.dev.ptr).netdev_ops = &Self::DEVICE_OPS;
+            bindings::register_netdev(self.dev.ptr)
+        };
+        if ret != 0 {
+            Err(Error::from_errno(ret))
+        } else {
+            self.is_registered = true;
+            Ok(())
+        }
+    }
+
+    const DEVICE_OPS: bindings::net_device_ops = bindings::net_device_ops {
+        ndo_init: if <T>::HAS_INIT {
+            Some(Self::init_callback)
+        } else {
+            None
+        },
+        ndo_uninit: if <T>::HAS_UNINIT {
+            Some(Self::uninit_callback)
+        } else {
+            None
+        },
+        ndo_open: if <T>::HAS_OPEN {
+            Some(Self::open_callback)
+        } else {
+            None
+        },
+        ndo_stop: if <T>::HAS_STOP {
+            Some(Self::stop_callback)
+        } else {
+            None
+        },
+        ndo_start_xmit: if <T>::HAS_START_XMIT {
+            Some(Self::start_xmit_callback)
+        } else {
+            None
+        },
+        // SAFETY: The rest is zeroed out to initialize `struct net_device_ops`,
+        // set `Option<&F>` to be `None`.
+        ..unsafe { core::mem::MaybeUninit::<bindings::net_device_ops>::zeroed().assume_init() }
+    };
+
+    unsafe extern "C" fn init_callback(netdev: *mut bindings::net_device) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
+            let dev = unsafe { Device::from_ptr(netdev) };
+            T::init(dev)?;
+            Ok(0)
+        })
+    }
+
+    unsafe extern "C" fn uninit_callback(netdev: *mut bindings::net_device) {
+        // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
+        let dev = unsafe { Device::from_ptr(netdev) };
+        T::uninit(dev);
+    }
+
+    unsafe extern "C" fn open_callback(netdev: *mut bindings::net_device) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
+            let dev = unsafe { Device::from_ptr(netdev) };
+            T::open(dev)?;
+            Ok(0)
+        })
+    }
+
+    unsafe extern "C" fn stop_callback(netdev: *mut bindings::net_device) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
+            let dev = unsafe { Device::from_ptr(netdev) };
+            T::stop(dev)?;
+            Ok(0)
+        })
+    }
+
+    unsafe extern "C" fn start_xmit_callback(
+        skb: *mut bindings::sk_buff,
+        netdev: *mut bindings::net_device,
+    ) -> bindings::netdev_tx_t {
+        // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
+        let dev = unsafe { Device::from_ptr(netdev) };
+        // SAFETY: The C API guarantees that `skb` is valid until a driver releases the skb.
+        let skb = unsafe { SkBuff::from_ptr(skb) };
+        T::start_xmit(dev, skb) as bindings::netdev_tx_t
+    }
+}
+
+// SAFETY: `Registration` exposes only `Device` object which can be used from any thread.
+unsafe impl<T: DeviceOperations> Send for Registration<T> {}
+// SAFETY: `Registration` exposes only `Device` object which can be used from any thread.
+unsafe impl<T: DeviceOperations> Sync for Registration<T> {}
+
+/// Corresponds to the kernel's `enum netdev_tx`.
+#[repr(i32)]
+pub enum TxCode {
+    /// Driver took care of packet.
+    Ok = bindings::netdev_tx_NETDEV_TX_OK,
+    /// Driver tx path was busy.
+    Busy = bindings::netdev_tx_NETDEV_TX_BUSY,
+}
+
+/// Corresponds to the kernel's `struct net_device_ops`.
+///
+/// A device driver must implement this. Only very basic operations are supported for now.
+#[vtable]
+pub trait DeviceOperations: ForeignOwnable + Send + Sync {
+    /// Corresponds to `ndo_init` in `struct net_device_ops`.
+    fn init(_dev: Device<Self>) -> Result {
+        Ok(())
+    }
+
+    /// Corresponds to `ndo_uninit` in `struct net_device_ops`.
+    fn uninit(_dev: Device<Self>) {}
+
+    /// Corresponds to `ndo_open` in `struct net_device_ops`.
+    fn open(_dev: Device<Self>) -> Result {
+        Ok(())
+    }
+
+    /// Corresponds to `ndo_stop` in `struct net_device_ops`.
+    fn stop(_dev: Device<Self>) -> Result {
+        Ok(())
+    }
+
+    /// Corresponds to `ndo_start_xmit` in `struct net_device_ops`.
+    fn start_xmit(_dev: Device<Self>, _skb: SkBuff) -> TxCode {
+        TxCode::Busy
+    }
+}
+
+/// Corresponds to the kernel's `struct sk_buff`.
+///
+/// A driver manages `struct sk_buff` in two ways. In both ways, the ownership is transferred
+/// between C and Rust. The allocation and release are done asymmetrically.
+///
+/// On the tx side (`ndo_start_xmit` operation in `struct net_device_ops`), the kernel allocates
+/// a `sk_buff' object and passes it to the driver. The driver is responsible for the release
+/// after transmission.
+/// On the rx side, the driver allocates a `sk_buff` object then passes it to the kernel
+/// after receiving data.
+///
+/// A driver must explicitly call a function to drop a `sk_buff` object.
+/// The code to let a `SkBuff` object go out of scope can't be compiled.
+///
+/// # Invariants
+///
+/// The pointer is valid.
+pub struct SkBuff(*mut bindings::sk_buff);
+
+impl SkBuff {
+    /// Creates a new [`SkBuff`] instance.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` must be valid.
+    unsafe fn from_ptr(ptr: *mut bindings::sk_buff) -> Self {
+        // INVARIANT: The safety requirements ensure the invariant.
+        Self(ptr)
+    }
+
+    /// Provides a time stamp.
+    pub fn tx_timestamp(&mut self) {
+        // SAFETY: The type invariants guarantee that `self.0` is valid.
+        unsafe {
+            bindings::skb_tx_timestamp(self.0);
+        }
+    }
+
+    /// Consumes a [`sk_buff`] object.
+    pub fn consume(self) {
+        // SAFETY: The type invariants guarantee that `self.0` is valid.
+        unsafe {
+            bindings::kfree_skb_reason(self.0, bindings::skb_drop_reason_SKB_CONSUMED);
+        }
+        core::mem::forget(self);
+    }
+}
+
+impl Drop for SkBuff {
+    #[inline(always)]
+    fn drop(&mut self) {
+        build_error!("skb must be released explicitly");
+    }
+}
-- 
2.34.1

[PATCH v2 2/5] rust: add support for ethernet operations

[FUJITA Tomonori]

This improves abstractions for network device drivers to implement
struct ethtool_ops, the majority of ethernet device drivers need to
do.

struct ethtool_ops also needs to access to device private data like
struct net_device_ops.

Currently, only get_ts_info operation is supported. The following
patch adds the Rust version of the dummy network driver, which uses
the operation.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/net/dev.rs          | 87 ++++++++++++++++++++++++++++++++-
 2 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 468bf606f174..6446ff764980 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -8,6 +8,7 @@
 
 #include <linux/errname.h>
 #include <linux/etherdevice.h>
+#include <linux/ethtool.h>
 #include <linux/netdevice.h>
 #include <linux/slab.h>
 #include <linux/refcount.h>
diff --git a/rust/kernel/net/dev.rs b/rust/kernel/net/dev.rs
index fe20616668a9..ff00616e4fef 100644
--- a/rust/kernel/net/dev.rs
+++ b/rust/kernel/net/dev.rs
@@ -142,7 +142,7 @@ pub fn register(&mut self) -> Result {
         if self.is_registered {
             return Err(code::EINVAL);
         }
-        // SAFETY: The type invariants guarantee that `self.dev.ptr` is valid.
+        // SAFETY: The type invariants of `Device` guarantee that `self.dev.ptr` is valid.
         let ret = unsafe {
             (*self.dev.ptr).netdev_ops = &Self::DEVICE_OPS;
             bindings::register_netdev(self.dev.ptr)
@@ -155,6 +155,18 @@ pub fn register(&mut self) -> Result {
         }
     }
 
+    /// Sets `ethtool_ops` of `net_device`.
+    pub fn set_ether_operations<E: EtherOperations>(&mut self) -> Result {
+        if self.is_registered {
+            return Err(code::EINVAL);
+        }
+        // SAFETY: The type invariants of `Device` guarantee that `self.dev.ptr` is valid.
+        unsafe {
+            (*(self.dev.ptr)).ethtool_ops = &EtherOperationsAdapter::<E>::ETHER_OPS;
+        }
+        Ok(())
+    }
+
     const DEVICE_OPS: bindings::net_device_ops = bindings::net_device_ops {
         ndo_init: if <T>::HAS_INIT {
             Some(Self::init_callback)
@@ -328,3 +340,76 @@ fn drop(&mut self) {
         build_error!("skb must be released explicitly");
     }
 }
+
+/// Builds the kernel's `struct ethtool_ops`.
+struct EtherOperationsAdapter<E: EtherOperations> {
+    _p: PhantomData<E>,
+}
+
+impl<E: EtherOperations> EtherOperationsAdapter<E> {
+    unsafe extern "C" fn get_ts_info_callback(
+        netdev: *mut bindings::net_device,
+        info: *mut bindings::ethtool_ts_info,
+    ) -> core::ffi::c_int {
+        from_result(|| {
+            // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
+            let dev = unsafe { Device::from_ptr(netdev) };
+            // SAFETY: The C API guarantees that `info` is valid while this function is running.
+            let info = unsafe { EthtoolTsInfo::from_ptr(info) };
+            E::get_ts_info(dev, info)?;
+            Ok(0)
+        })
+    }
+
+    const ETHER_OPS: bindings::ethtool_ops = bindings::ethtool_ops {
+        get_ts_info: if <E>::HAS_GET_TS_INFO {
+            Some(Self::get_ts_info_callback)
+        } else {
+            None
+        },
+        // SAFETY: The rest is zeroed out to initialize `struct ethtool_ops`,
+        // set `Option<&F>` to be `None`.
+        ..unsafe { core::mem::MaybeUninit::<bindings::ethtool_ops>::zeroed().assume_init() }
+    };
+}
+
+/// Corresponds to the kernel's `struct ethtool_ops`.
+#[vtable]
+pub trait EtherOperations: ForeignOwnable + Send + Sync {
+    /// Corresponds to `get_ts_info` in `struct ethtool_ops`.
+    fn get_ts_info(_dev: Device<Self>, _info: EthtoolTsInfo) -> Result {
+        Err(Error::from_errno(bindings::EOPNOTSUPP as i32))
+    }
+}
+
+/// Corresponds to the kernel's `struct ethtool_ts_info`.
+///
+/// # Invariants
+///
+/// The pointer is valid.
+pub struct EthtoolTsInfo(*mut bindings::ethtool_ts_info);
+
+impl EthtoolTsInfo {
+    /// Creates a new `EthtoolTsInfo' instance.
+    ///
+    /// # Safety
+    ///
+    /// Callers must ensure that `ptr` must be valid.
+    unsafe fn from_ptr(ptr: *mut bindings::ethtool_ts_info) -> Self {
+        // INVARIANT: The safety requirements ensure the invariant.
+        Self(ptr)
+    }
+
+    /// Sets up the info for software timestamping.
+    pub fn ethtool_op_get_ts_info<D: ForeignOwnable + Send + Sync>(
+        dev: &Device<D>,
+        info: &mut EthtoolTsInfo,
+    ) -> Result {
+        // SAFETY: The type invariants of `Device` guarantee that `dev.ptr` are valid.
+        // The type invariants guarantee that `info.0` are valid.
+        unsafe {
+            bindings::ethtool_op_get_ts_info(dev.ptr, info.0);
+        }
+        Ok(())
+    }
+}
-- 
2.34.1

[PATCH v2 3/5] rust: add methods for configure net_device

[FUJITA Tomonori]

adds methods to net::Device for the basic configurations of
net_device.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/helpers.c         |   7 ++
 rust/kernel/net/dev.rs | 185 ++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 191 insertions(+), 1 deletion(-)

diff --git a/rust/helpers.c b/rust/helpers.c
index 70d50767ff4e..6c51deb18dc1 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -22,6 +22,7 @@
 #include <linux/build_bug.h>
 #include <linux/err.h>
 #include <linux/errname.h>
+#include <linux/etherdevice.h>
 #include <linux/refcount.h>
 #include <linux/mutex.h>
 #include <linux/netdevice.h>
@@ -31,6 +32,12 @@
 #include <linux/wait.h>
 
 #ifdef CONFIG_NET
+void rust_helper_eth_hw_addr_random(struct net_device *dev)
+{
+	eth_hw_addr_random(dev);
+}
+EXPORT_SYMBOL_GPL(rust_helper_eth_hw_addr_random);
+
 void *rust_helper_netdev_priv(const struct net_device *dev)
 {
 	return netdev_priv(dev);
diff --git a/rust/kernel/net/dev.rs b/rust/kernel/net/dev.rs
index ff00616e4fef..e4d8d8260c10 100644
--- a/rust/kernel/net/dev.rs
+++ b/rust/kernel/net/dev.rs
@@ -8,9 +8,116 @@
 //! [`include/linux/skbuff.h`](../../../../include/linux/skbuff.h),
 //! [`include/uapi/linux/if_link.h`](../../../../include/uapi/linux/if_link.h).
 
-use crate::{bindings, build_error, error::*, prelude::vtable, types::ForeignOwnable};
+use crate::{bindings, build_error, error::*, prelude::vtable, str::CStr, types::ForeignOwnable};
 use {core::ffi::c_void, core::marker::PhantomData};
 
+/// Flags associated with a [`Device`].
+pub mod flags {
+    /// Interface is up.
+    pub const IFF_UP: u32 = bindings::net_device_flags_IFF_UP;
+    /// Broadcast address valid.
+    pub const IFF_BROADCAST: u32 = bindings::net_device_flags_IFF_BROADCAST;
+    /// Device on debugging.
+    pub const IFF_DEBUG: u32 = bindings::net_device_flags_IFF_DEBUG;
+    /// Loopback device.
+    pub const IFF_LOOPBACK: u32 = bindings::net_device_flags_IFF_LOOPBACK;
+    /// Has p-p link.
+    pub const IFF_POINTOPOINT: u32 = bindings::net_device_flags_IFF_POINTOPOINT;
+    /// Avoids use of trailers.
+    pub const IFF_NOTRAILERS: u32 = bindings::net_device_flags_IFF_NOTRAILERS;
+    /// Interface RFC2863 OPER_UP.
+    pub const IFF_RUNNING: u32 = bindings::net_device_flags_IFF_RUNNING;
+    /// No ARP protocol.
+    pub const IFF_NOARP: u32 = bindings::net_device_flags_IFF_NOARP;
+    /// Receives all packets.
+    pub const IFF_PROMISC: u32 = bindings::net_device_flags_IFF_PROMISC;
+    /// Receive all multicast packets.
+    pub const IFF_ALLMULTI: u32 = bindings::net_device_flags_IFF_ALLMULTI;
+    /// Master of a load balancer.
+    pub const IFF_MASTER: u32 = bindings::net_device_flags_IFF_MASTER;
+    /// Slave of a load balancer.
+    pub const IFF_SLAVE: u32 = bindings::net_device_flags_IFF_SLAVE;
+    /// Supports multicast.
+    pub const IFF_MULTICAST: u32 = bindings::net_device_flags_IFF_MULTICAST;
+    /// Capable of setting media type.
+    pub const IFF_PORTSEL: u32 = bindings::net_device_flags_IFF_PORTSEL;
+    /// Auto media select active.
+    pub const IFF_AUTOMEDIA: u32 = bindings::net_device_flags_IFF_AUTOMEDIA;
+    /// Dialup device with changing addresses.
+    pub const IFF_DYNAMIC: u32 = bindings::net_device_flags_IFF_DYNAMIC;
+}
+
+/// Private flags associated with a [`Device`].
+pub mod priv_flags {
+    /// 802.1Q VLAN device.
+    pub const IFF_802_1Q_VLAN: u64 = bindings::netdev_priv_flags_IFF_802_1Q_VLAN;
+    /// Ethernet bridging device.
+    pub const IFF_EBRIDGE: u64 = bindings::netdev_priv_flags_IFF_EBRIDGE;
+    /// Bonding master or slave device.
+    pub const IFF_BONDING: u64 = bindings::netdev_priv_flags_IFF_BONDING;
+    /// ISATAP interface (RFC4214).
+    pub const IFF_ISATAP: u64 = bindings::netdev_priv_flags_IFF_ISATAP;
+    /// WAN HDLC device.
+    pub const IFF_WAN_HDLC: u64 = bindings::netdev_priv_flags_IFF_WAN_HDLC;
+    /// dev_hard_start_xmit() is allowed to release skb->dst.
+    pub const IFF_XMIT_DST_RELEASE: u64 = bindings::netdev_priv_flags_IFF_XMIT_DST_RELEASE;
+    /// Disallows bridging this ether device.
+    pub const IFF_DONT_BRIDGE: u64 = bindings::netdev_priv_flags_IFF_DONT_BRIDGE;
+    /// Disables netpoll at run-time.
+    pub const IFF_DISABLE_NETPOLL: u64 = bindings::netdev_priv_flags_IFF_DISABLE_NETPOLL;
+    /// Device used as macvlan port.
+    pub const IFF_MACVLAN_PORT: u64 = bindings::netdev_priv_flags_IFF_MACVLAN_PORT;
+    /// Device used as bridge port.
+    pub const IFF_BRIDGE_PORT: u64 = bindings::netdev_priv_flags_IFF_BRIDGE_PORT;
+    /// Device used as Open vSwitch datapath port.
+    pub const IFF_OVS_DATAPATH: u64 = bindings::netdev_priv_flags_IFF_OVS_DATAPATH;
+    /// The interface supports sharing skbs on transmit.
+    pub const IFF_TX_SKB_SHARING: u64 = bindings::netdev_priv_flags_IFF_TX_SKB_SHARING;
+    /// Supports unicast filtering.
+    pub const IFF_UNICAST_FLT: u64 = bindings::netdev_priv_flags_IFF_UNICAST_FLT;
+    /// Device used as team port.
+    pub const IFF_TEAM_PORT: u64 = bindings::netdev_priv_flags_IFF_TEAM_PORT;
+    /// Device supports sending custom FCS.
+    pub const IFF_SUPP_NOFCS: u64 = bindings::netdev_priv_flags_IFF_SUPP_NOFCS;
+    /// Device supports hardware address change when it's running.
+    pub const IFF_LIVE_ADDR_CHANGE: u64 = bindings::netdev_priv_flags_IFF_LIVE_ADDR_CHANGE;
+    /// Macvlan device.
+    pub const IFF_MACVLAN: u64 = bindings::netdev_priv_flags_IFF_MACVLAN;
+    /// IFF_XMIT_DST_RELEASE not taking into account underlying stacked devices.
+    pub const IFF_XMIT_DST_RELEASE_PERM: u64 =
+        bindings::netdev_priv_flags_IFF_XMIT_DST_RELEASE_PERM;
+    /// L3 master device.
+    pub const IFF_L3MDEV_MASTER: u64 = bindings::netdev_priv_flags_IFF_L3MDEV_MASTER;
+    /// Device can run without qdisc attached.
+    pub const IFF_NO_QUEUE: u64 = bindings::netdev_priv_flags_IFF_NO_QUEUE;
+    /// Device is a Open vSwitch master.
+    pub const IFF_OPENVSWITCH: u64 = bindings::netdev_priv_flags_IFF_OPENVSWITCH;
+    /// Device is enslaved to an L3 master.
+    pub const IFF_L3MDEV_SLAVE: u64 = bindings::netdev_priv_flags_IFF_L3MDEV_SLAVE;
+    /// Team device.
+    pub const IFF_TEAM: u64 = bindings::netdev_priv_flags_IFF_TEAM;
+    /// Device has had Rx Flow indirection table configured.
+    pub const IFF_RXFH_CONFIGURED: u64 = bindings::netdev_priv_flags_IFF_RXFH_CONFIGURED;
+    /// The headroom value is controlled by an external entity.
+    pub const IFF_PHONY_HEADROOM: u64 = bindings::netdev_priv_flags_IFF_PHONY_HEADROOM;
+    /// MACsec device.
+    pub const IFF_MACSEC: u64 = bindings::netdev_priv_flags_IFF_MACSEC;
+    /// Device doesn't support the rx_handler hook.
+    pub const IFF_NO_RX_HANDLER: u64 = bindings::netdev_priv_flags_IFF_NO_RX_HANDLER;
+    /// Failover master device.
+    pub const IFF_FAILOVER: u64 = bindings::netdev_priv_flags_IFF_FAILOVER;
+    /// Lower device of a failover master device.
+    pub const IFF_FAILOVER_SLAVE: u64 = bindings::netdev_priv_flags_IFF_FAILOVER_SLAVE;
+    /// Only invokes the rx handler of L3 master device.
+    pub const IFF_L3MDEV_RX_HANDLER: u64 = bindings::netdev_priv_flags_IFF_L3MDEV_RX_HANDLER;
+    /// Prevents ipv6 addrconf.
+    pub const IFF_NO_ADDRCONF: u64 = bindings::netdev_priv_flags_IFF_NO_ADDRCONF;
+    /// Capable of xmitting frames with skb_headlen(skb) == 0.
+    pub const IFF_TX_SKB_NO_LINEAR: u64 = bindings::netdev_priv_flags_IFF_TX_SKB_NO_LINEAR;
+    /// Supports setting carrier via IFLA_PROTO_DOWN.
+    pub const IFF_CHANGE_PROTO_DOWN: u64 = bindings::netdev_priv_flags_IFF_CHANGE_PROTO_DOWN;
+}
+
 /// Corresponds to the kernel's `struct net_device`.
 ///
 /// # Invariants
@@ -49,6 +156,82 @@ pub fn drv_priv_data(&self) -> D::Borrowed<'_> {
             ))
         }
     }
+
+    /// Sets the name of a device.
+    pub fn set_name(&mut self, name: &CStr) -> Result {
+        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
+        unsafe {
+            if name.len() > (*self.ptr).name.len() {
+                return Err(code::EINVAL);
+            }
+            (*self.ptr)
+                .name
+                .as_mut_ptr()
+                .copy_from_nonoverlapping(name.as_char_ptr(), name.len());
+        }
+        Ok(())
+    }
+
+    /// Sets carrier.
+    pub fn netif_carrier_on(&mut self) {
+        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
+        unsafe { bindings::netif_carrier_on(self.ptr) }
+    }
+
+    /// Clears carrier.
+    pub fn netif_carrier_off(&mut self) {
+        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
+        unsafe { bindings::netif_carrier_off(self.ptr) }
+    }
+
+    /// Sets the max mtu of the device.
+    pub fn set_max_mtu(&mut self, max_mtu: u32) {
+        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
+        unsafe {
+            (*self.ptr).max_mtu = max_mtu;
+        }
+    }
+
+    /// Sets the minimum mtu of the device.
+    pub fn set_min_mtu(&mut self, min_mtu: u32) {
+        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
+        unsafe {
+            (*self.ptr).min_mtu = min_mtu;
+        }
+    }
+
+    /// Returns the flags of the device.
+    pub fn get_flags(&self) -> u32 {
+        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
+        unsafe { (*self.ptr).flags }
+    }
+
+    /// Sets the flags of the device.
+    pub fn set_flags(&mut self, flags: u32) {
+        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
+        unsafe {
+            (*self.ptr).flags = flags;
+        }
+    }
+
+    /// Returns the priv_flags of the device.
+    pub fn get_priv_flags(&self) -> u64 {
+        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
+        unsafe { (*self.ptr).priv_flags }
+    }
+
+    /// Sets the priv_flags of the device.
+    pub fn set_priv_flags(&mut self, flags: u64) {
+        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
+        unsafe { (*self.ptr).priv_flags = flags }
+    }
+
+    /// Generate a random Ethernet address (MAC) to be used by a net device
+    /// and set addr_assign_type.
+    pub fn set_random_eth_hw_addr(&mut self) {
+        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
+        unsafe { bindings::eth_hw_addr_random(self.ptr) }
+    }
 }
 
 // SAFETY: `Device` is just a wrapper for the kernel`s `struct net_device`, which can be used
-- 
2.34.1

[PATCH v2 4/5] samples: rust: add dummy network driver

[FUJITA Tomonori]

This is a simpler version of drivers/net/dummy.c.

This demonstrates the usage of abstractions for network device drivers.

Allows allocator_api feature for Box::try_new();

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 samples/rust/Kconfig           | 13 ++++++
 samples/rust/Makefile          |  1 +
 samples/rust/rust_net_dummy.rs | 75 ++++++++++++++++++++++++++++++++++
 scripts/Makefile.build         |  2 +-
 4 files changed, 90 insertions(+), 1 deletion(-)
 create mode 100644 samples/rust/rust_net_dummy.rs

diff --git a/samples/rust/Kconfig b/samples/rust/Kconfig
index b0f74a81c8f9..02bda7bdf722 100644
--- a/samples/rust/Kconfig
+++ b/samples/rust/Kconfig
@@ -30,6 +30,19 @@ config SAMPLE_RUST_PRINT
 
 	  If unsure, say N.
 
+config SAMPLE_RUST_NET_DUMMY
+	tristate "Dummy network driver"
+	depends on NET
+	help
+	  This is the simpler version of drivers/net/dummy.c.
+	  No intention to replace it. This provides educational information
+	  for Rust abstractions for network device drivers.
+
+	  To compile this as a module, choose M here:
+	  the module will be called rust_net_dummy.
+
+	  If unsure, say N.
+
 config SAMPLE_RUST_HOSTPROGS
 	bool "Host programs"
 	help
diff --git a/samples/rust/Makefile b/samples/rust/Makefile
index 03086dabbea4..440dee2971ba 100644
--- a/samples/rust/Makefile
+++ b/samples/rust/Makefile
@@ -2,5 +2,6 @@
 
 obj-$(CONFIG_SAMPLE_RUST_MINIMAL)		+= rust_minimal.o
 obj-$(CONFIG_SAMPLE_RUST_PRINT)			+= rust_print.o
+obj-$(CONFIG_SAMPLE_RUST_NET_DUMMY)		+= rust_net_dummy.o
 
 subdir-$(CONFIG_SAMPLE_RUST_HOSTPROGS)		+= hostprogs
diff --git a/samples/rust/rust_net_dummy.rs b/samples/rust/rust_net_dummy.rs
new file mode 100644
index 000000000000..78f8f3321fd2
--- /dev/null
+++ b/samples/rust/rust_net_dummy.rs
@@ -0,0 +1,75 @@
+// SPDX-License-Identifier: GPL-2.0
+//
+//! Rust dummy netdev.
+
+//use kernel::types::ForeignOwnable;
+use kernel::{
+    c_str,
+    net::dev::{
+        flags, priv_flags, Device, DeviceOperations, EtherOperations, EthtoolTsInfo, Registration,
+        SkBuff, TxCode,
+    },
+    prelude::*,
+};
+
+module! {
+    type: DummyNetdev,
+    name: "rust_net_dummy",
+    author: "Rust for Linux Contributors",
+    description: "Rust dummy netdev",
+    license: "GPL v2",
+}
+
+#[vtable]
+impl DeviceOperations for Box<DriverData> {
+    fn init(dev: Device<Box<DriverData>>) -> Result {
+        // how to access to the driver private data.
+        let _ = dev.drv_priv_data();
+        Ok(())
+    }
+
+    fn start_xmit(_dev: Device<Box<DriverData>>, mut skb: SkBuff) -> TxCode {
+        skb.tx_timestamp();
+        skb.consume();
+        TxCode::Ok
+    }
+}
+
+/// For device driver specific information.
+struct DriverData {}
+
+struct DummyNetdev {
+    _r: Registration<Box<DriverData>>,
+}
+
+#[vtable]
+impl EtherOperations for Box<DriverData> {
+    fn get_ts_info(dev: Device<Box<DriverData>>, mut info: EthtoolTsInfo) -> Result {
+        EthtoolTsInfo::ethtool_op_get_ts_info(&dev, &mut info)
+    }
+}
+
+impl kernel::Module for DummyNetdev {
+    fn init(_module: &'static ThisModule) -> Result<Self> {
+        let data = Box::try_new(DriverData {})?;
+        let mut r = Registration::<Box<DriverData>>::try_new_ether(1, 1, data)?;
+        r.set_ether_operations::<Box<DriverData>>()?;
+
+        let netdev = r.dev_get();
+        netdev.set_name(c_str!("dummy%d"))?;
+
+        netdev.set_flags(netdev.get_flags() | flags::IFF_NOARP & !flags::IFF_MULTICAST);
+        netdev.set_priv_flags(
+            netdev.get_priv_flags() | priv_flags::IFF_LIVE_ADDR_CHANGE | priv_flags::IFF_NO_QUEUE,
+        );
+        netdev.set_random_eth_hw_addr();
+        netdev.set_min_mtu(0);
+        netdev.set_max_mtu(0);
+
+        r.register()?;
+
+        // TODO: Replaces pr_info with the wrapper of netdev_info().
+        pr_info!("Hello Rust dummy netdev!");
+        Ok(DummyNetdev { _r: r })
+    }
+}
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 78175231c969..1404967e908e 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -277,7 +277,7 @@ $(obj)/%.lst: $(src)/%.c FORCE
 # Compile Rust sources (.rs)
 # ---------------------------------------------------------------------------
 
-rust_allowed_features := new_uninit
+rust_allowed_features := allocator_api,new_uninit
 
 rust_common_cmd = \
 	RUST_MODFILE=$(modfile) $(RUSTC_OR_CLIPPY) $(rust_flags) \
-- 
2.34.1

[PATCH v2 5/5] MAINTAINERS: add Rust network abstractions files to the NETWORKING DRIVERS entry

[FUJITA Tomonori]

The files are placed at rust/kernel/ directory for now but the files
are likely to be moved to net/ directory if things go well.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 250518fc70ff..66b8e43b05a2 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -14581,6 +14581,8 @@ F:	include/linux/inetdevice.h
 F:	include/linux/netdevice.h
 F:	include/uapi/linux/if_*
 F:	include/uapi/linux/netdevice.h
+F:	rust/kernel/net.rs
+F:	rust/kernel/net/
 
 NETWORKING DRIVERS (WIRELESS)
 M:	Kalle Valo <kvalo@kernel.org>
-- 
2.34.1

Re: [PATCH v2 0/5] Rust abstractions for network device drivers

[Jakub Kicinski]

On Mon, 10 Jul 2023 16:36:58 +0900 FUJITA Tomonori wrote:
> This patchset adds minimum Rust abstractions for network device
> drivers and an example of a Rust network device driver, a simpler
> version of drivers/net/dummy.c.
> 
> The major change is a way to drop an skb (1/5 patch); a driver needs
> to explicitly call a function to drop a skb. The code to let a skb
> go out of scope can't be compiled.
> 
> I dropped get_stats64 support patch that the current sample driver
> doesn't use. Instead I added a patch to update the NETWORKING DRIVERS
> entry in MAINTAINERS.

I'd like to double down on my suggestion to try to implement a real
PHY driver. Most of the bindings in patch 3 will never be used by
drivers. (Re)implementing a real driver will guide you towards useful
stuff and real problems.

Re: [PATCH v2 0/5] Rust abstractions for network device drivers

[Greg KH]

On Mon, Jul 10, 2023 at 11:29:52AM -0700, Jakub Kicinski wrote:
> On Mon, 10 Jul 2023 16:36:58 +0900 FUJITA Tomonori wrote:
> > This patchset adds minimum Rust abstractions for network device
> > drivers and an example of a Rust network device driver, a simpler
> > version of drivers/net/dummy.c.
> > 
> > The major change is a way to drop an skb (1/5 patch); a driver needs
> > to explicitly call a function to drop a skb. The code to let a skb
> > go out of scope can't be compiled.
> > 
> > I dropped get_stats64 support patch that the current sample driver
> > doesn't use. Instead I added a patch to update the NETWORKING DRIVERS
> > entry in MAINTAINERS.
> 
> I'd like to double down on my suggestion to try to implement a real
> PHY driver. Most of the bindings in patch 3 will never be used by
> drivers. (Re)implementing a real driver will guide you towards useful
> stuff and real problems.

And I'd recommend that we not take any more bindings without real users,
as there seems to be just a collection of these and it's hard to
actually review them to see how they are used...

thanks,

greg k-h

Re: [PATCH v2 0/5] Rust abstractions for network device drivers

[FUJITA Tomonori]

Hi,

On Mon, 10 Jul 2023 11:29:52 -0700
Jakub Kicinski <kuba@kernel.org> wrote:

> On Mon, 10 Jul 2023 16:36:58 +0900 FUJITA Tomonori wrote:
>> This patchset adds minimum Rust abstractions for network device
>> drivers and an example of a Rust network device driver, a simpler
>> version of drivers/net/dummy.c.
>> 
>> The major change is a way to drop an skb (1/5 patch); a driver needs
>> to explicitly call a function to drop a skb. The code to let a skb
>> go out of scope can't be compiled.
>> 
>> I dropped get_stats64 support patch that the current sample driver
>> doesn't use. Instead I added a patch to update the NETWORKING DRIVERS
>> entry in MAINTAINERS.
> 
> I'd like to double down on my suggestion to try to implement a real
> PHY driver. Most of the bindings in patch 3 will never be used by
> drivers. (Re)implementing a real driver will guide you towards useful
> stuff and real problems.

You meant reimplementing one of drivers in drivers/net/phy in Rust
(with Rust abstractions for PHY drivers)?

Even the approach, we would get hit the same problem? Replacing an
existing working driver in C doesn't make sense much thus the
abstractions cannot be merged until someone want to implement a driver
in Rust for new PHY hardware.

Or you think that PHY drivers (and probably the abstractions) are
relatively simple so merging the abstractions for them is acceptable
without a real driver (we could put a real drivers under
samples/rust/)?

thanks,

Re: [PATCH v2 0/5] Rust abstractions for network device drivers

[Andrew Lunn]

> Or you think that PHY drivers (and probably the abstractions) are
> relatively simple so merging the abstractions for them is acceptable
> without a real driver (we could put a real drivers under
> samples/rust/)?

PHY drivers are much simpler than Ethernet drivers. But more
importantly, the API to the rest of network stack is much much
smaller. So a binding for a sample driver is going to cover a large
part of that API, unlike your sample Ethernet driver binding which
covers a tiny part of the API. The PHY binding is then actually
useful, unlike the binding for Ethernet.

As for reimplementing an existing driver, vs a new driver for hardware
which is currently unsupported, that would depend on you. You could
reach out to some vendors and see if they have devices which are
missing mainline drivers. See if they will donate an RDK and the
datasheet in return for a free driver written in Rust. Whole new
drivers do come along reasonably frequently, so there is probably
scope for a new driver. Automotive is a big source of new code and
devices at the moment.

	Andrew

Re: [PATCH v2 0/5] Rust abstractions for network device drivers

[FUJITA Tomonori]

Hi,

On Tue, 11 Jul 2023 15:17:33 +0200
Andrew Lunn <andrew@lunn.ch> wrote:

>> Or you think that PHY drivers (and probably the abstractions) are
>> relatively simple so merging the abstractions for them is acceptable
>> without a real driver (we could put a real drivers under
>> samples/rust/)?
> 
> PHY drivers are much simpler than Ethernet drivers. But more
> importantly, the API to the rest of network stack is much much
> smaller. So a binding for a sample driver is going to cover a large
> part of that API, unlike your sample Ethernet driver binding which
> covers a tiny part of the API. The PHY binding is then actually
> useful, unlike the binding for Ethernet.

Point taken, I work on PHY drivers first.

> As for reimplementing an existing driver, vs a new driver for hardware
> which is currently unsupported, that would depend on you. You could
> reach out to some vendors and see if they have devices which are
> missing mainline drivers. See if they will donate an RDK and the
> datasheet in return for a free driver written in Rust. Whole new
> drivers do come along reasonably frequently, so there is probably
> scope for a new driver. Automotive is a big source of new code and
> devices at the moment.

Understood. Let me reseach the existing drivers to think about that.

Thanks,

Re: [PATCH v2 1/5] rust: core abstractions for network device drivers

[Benno Lossin]

> 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@gmail.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          | 330 ++++++++++++++++++++++++++++++++
>  5 files changed, 356 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..fe20616668a9
> --- /dev/null
> +++ b/rust/kernel/net/dev.rs
> @@ -0,0 +1,330 @@
> +// 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, build_error, error::*, prelude::vtable, types::ForeignOwnable};
> +use {core::ffi::c_void, core::marker::PhantomData};
> +
> +/// Corresponds to the kernel's `struct net_device`.
> +///
> +/// # Invariants
> +///
> +/// The `ptr` points to the contiguous memory for `struct net_device` and a pointer,
> +/// which stores an address returned by `ForeignOwnable::into_foreign()`.
> +pub struct Device<D: ForeignOwnable + Send + Sync> {
> +    ptr: *mut bindings::net_device,
> +    _p: PhantomData<D>,
> +}
> +
> +impl<D: ForeignOwnable + Send + Sync> Device<D> {
> +    /// Creates a new [`Device`] instance.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure that `ptr` must point to the contiguous memory
> +    /// for `struct net_device` and a pointer, which stores an address returned
> +    /// by `ForeignOwnable::into_foreign()`.
> +    unsafe fn from_ptr(ptr: *mut bindings::net_device) -> Self {
> +        // INVARIANT: The safety requirements ensure the invariant.
> +        Self {
> +            ptr,
> +            _p: PhantomData,
> +        }
> +    }
> +
> +    /// Gets the private data of a device driver.
> +    pub fn drv_priv_data(&self) -> D::Borrowed<'_> {
> +        // SAFETY: The type invariants guarantee that self.ptr is valid and
> +        // bindings::netdev_priv(self.ptr) returns a pointer that stores an address
> +        // returned by `ForeignOwnable::into_foreign()`.
> +        unsafe {
> +            D::borrow(core::ptr::read(
> +                bindings::netdev_priv(self.ptr) 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 an object, which is `Sync`
> +// so it's safe to sharing its pointer.
> +unsafe impl<D: ForeignOwnable + Send + Sync> Send for Device<D> {}
> +// 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 an object, which is `Sync`,
> +// can be used from any thread too.
> +unsafe impl<D: ForeignOwnable + Send + Sync> Sync for Device<D> {}
> +
> +/// 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> {
> +    dev: Device<T>,
> +    is_registered: bool,
> +    _p: PhantomData<T>,
> +}
> +
> +impl<T: DeviceOperations> Drop for Registration<T> {
> +    fn drop(&mut self) {
> +        // SAFETY: The type invariants of `Device` guarantee that `self.dev.ptr` is valid and
> +        // bindings::netdev_priv(self.ptr) returns a pointer that stores an address
> +        // returned by `ForeignOwnable::into_foreign()`.
> +        unsafe {
> +            let _ = T::from_foreign(core::ptr::read(
> +                bindings::netdev_priv(self.dev.ptr) as *const *const c_void
> +            ));
> +        }
> +        // SAFETY: The type invariants of `Device` guarantee that `self.dev.ptr` is valid.
> +        unsafe {
> +            if self.is_registered {
> +                bindings::unregister_netdev(self.dev.ptr);
> +            }
> +            bindings::free_netdev(self.dev.ptr);
> +        }
> +    }
> +}
> +
> +impl<T: DeviceOperations> Registration<T> {
> +    /// 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: T) -> Result<Self> {
> +        // SAFETY: Just an FFI call with no additional safety requirements.
> +        let ptr = unsafe {
> +            bindings::alloc_etherdev_mqs(
> +                core::mem::size_of::<*const c_void>() as i32,
> +                tx_queue_size,
> +                rx_queue_size,
> +            )
> +        };
> +        if ptr.is_null() {
> +            return Err(code::ENOMEM);
> +        }
> +
> +        // SAFETY: It's safe to write an address 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());
> +        }
> +
> +        // SAFETY: `ptr` points to contiguous memory for `struct net_device` and a pointer,
> +        // which stores an address returned by `ForeignOwnable::into_foreign()`.
> +        let dev = unsafe { Device::from_ptr(ptr) };
> +        Ok(Registration {
> +            dev,
> +            is_registered: false,
> +            _p: PhantomData,
> +        })
> +    }
> +
> +    /// Returns a network device.
> +    ///
> +    /// A device driver normally configures the device before registration.
> +    pub fn dev_get(&mut self) -> &mut Device<T> {
> +        &mut self.dev
> +    }

I think you could instead implement `AsMut`.

> +
> +    /// Registers a network device.
> +    pub fn register(&mut self) -> Result {
> +        if self.is_registered {
> +            return Err(code::EINVAL);
> +        }
> +        // SAFETY: The type invariants guarantee that `self.dev.ptr` is valid.
> +        let ret = unsafe {
> +            (*self.dev.ptr).netdev_ops = &Self::DEVICE_OPS;
> +            bindings::register_netdev(self.dev.ptr)
> +        };
> +        if ret != 0 {
> +            Err(Error::from_errno(ret))
> +        } else {
> +            self.is_registered = true;
> +            Ok(())
> +        }
> +    }
> +
> +    const DEVICE_OPS: bindings::net_device_ops = bindings::net_device_ops {
> +        ndo_init: if <T>::HAS_INIT {
> +            Some(Self::init_callback)
> +        } else {
> +            None
> +        },
> +        ndo_uninit: if <T>::HAS_UNINIT {
> +            Some(Self::uninit_callback)
> +        } else {
> +            None
> +        },
> +        ndo_open: if <T>::HAS_OPEN {
> +            Some(Self::open_callback)
> +        } else {
> +            None
> +        },
> +        ndo_stop: if <T>::HAS_STOP {
> +            Some(Self::stop_callback)
> +        } else {
> +            None
> +        },
> +        ndo_start_xmit: if <T>::HAS_START_XMIT {
> +            Some(Self::start_xmit_callback)
> +        } else {
> +            None
> +        },
> +        // SAFETY: The rest is zeroed out to initialize `struct net_device_ops`,
> +        // set `Option<&F>` to be `None`.
> +        ..unsafe { core::mem::MaybeUninit::<bindings::net_device_ops>::zeroed().assume_init() }
> +    };
> +
> +    unsafe extern "C" fn init_callback(netdev: *mut bindings::net_device) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
> +            let dev = unsafe { Device::from_ptr(netdev) };
> +            T::init(dev)?;
> +            Ok(0)
> +        })
> +    }
> +
> +    unsafe extern "C" fn uninit_callback(netdev: *mut bindings::net_device) {
> +        // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
> +        let dev = unsafe { Device::from_ptr(netdev) };
> +        T::uninit(dev);
> +    }
> +
> +    unsafe extern "C" fn open_callback(netdev: *mut bindings::net_device) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
> +            let dev = unsafe { Device::from_ptr(netdev) };
> +            T::open(dev)?;
> +            Ok(0)
> +        })
> +    }
> +
> +    unsafe extern "C" fn stop_callback(netdev: *mut bindings::net_device) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
> +            let dev = unsafe { Device::from_ptr(netdev) };
> +            T::stop(dev)?;
> +            Ok(0)
> +        })
> +    }
> +
> +    unsafe extern "C" fn start_xmit_callback(
> +        skb: *mut bindings::sk_buff,
> +        netdev: *mut bindings::net_device,
> +    ) -> bindings::netdev_tx_t {
> +        // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
> +        let dev = unsafe { Device::from_ptr(netdev) };
> +        // SAFETY: The C API guarantees that `skb` is valid until a driver releases the skb.
> +        let skb = unsafe { SkBuff::from_ptr(skb) };
> +        T::start_xmit(dev, skb) as bindings::netdev_tx_t
> +    }
> +}
> +
> +// SAFETY: `Registration` exposes only `Device` object which can be used from any thread.
> +unsafe impl<T: DeviceOperations> Send for Registration<T> {}
> +// SAFETY: `Registration` exposes only `Device` object which can be used from any thread.
> +unsafe impl<T: DeviceOperations> Sync for Registration<T> {}
> +
> +/// Corresponds to the kernel's `enum netdev_tx`.
> +#[repr(i32)]
> +pub enum TxCode {
> +    /// Driver took care of packet.
> +    Ok = bindings::netdev_tx_NETDEV_TX_OK,
> +    /// Driver tx path was busy.
> +    Busy = bindings::netdev_tx_NETDEV_TX_BUSY,
> +}

Is it really necessary that this has the same representation as the
C constants? Would a function that converts be sufficient? I think we
should let the compiler decide the layout when we have no concrete
requirements.

> +
> +/// Corresponds to the kernel's `struct net_device_ops`.
> +///
> +/// A device driver must implement this. Only very basic operations are supported for now.
> +#[vtable]
> +pub trait DeviceOperations: ForeignOwnable + Send + Sync {
> +    /// Corresponds to `ndo_init` in `struct net_device_ops`.
> +    fn init(_dev: Device<Self>) -> Result {
> +        Ok(())
> +    }
> +
> +    /// Corresponds to `ndo_uninit` in `struct net_device_ops`.
> +    fn uninit(_dev: Device<Self>) {}
> +
> +    /// Corresponds to `ndo_open` in `struct net_device_ops`.
> +    fn open(_dev: Device<Self>) -> Result {
> +        Ok(())
> +    }
> +
> +    /// Corresponds to `ndo_stop` in `struct net_device_ops`.
> +    fn stop(_dev: Device<Self>) -> Result {
> +        Ok(())
> +    }
> +
> +    /// Corresponds to `ndo_start_xmit` in `struct net_device_ops`.
> +    fn start_xmit(_dev: Device<Self>, _skb: SkBuff) -> TxCode {
> +        TxCode::Busy
> +    }
> +}
> +
> +/// Corresponds to the kernel's `struct sk_buff`.
> +///
> +/// A driver manages `struct sk_buff` in two ways. In both ways, the ownership is transferred
> +/// between C and Rust. The allocation and release are done asymmetrically.
> +///
> +/// On the tx side (`ndo_start_xmit` operation in `struct net_device_ops`), the kernel allocates
> +/// a `sk_buff' object and passes it to the driver. The driver is responsible for the release
> +/// after transmission.
> +/// On the rx side, the driver allocates a `sk_buff` object then passes it to the kernel
> +/// after receiving data.
> +///
> +/// A driver must explicitly call a function to drop a `sk_buff` object.
> +/// The code to let a `SkBuff` object go out of scope can't be compiled.
> +///
> +/// # Invariants
> +///
> +/// The pointer is valid.
> +pub struct SkBuff(*mut bindings::sk_buff);
> +
> +impl SkBuff {
> +    /// Creates a new [`SkBuff`] instance.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure that `ptr` must be valid.
> +    unsafe fn from_ptr(ptr: *mut bindings::sk_buff) -> Self {
> +        // INVARIANT: The safety requirements ensure the invariant.
> +        Self(ptr)
> +    }
> +
> +    /// Provides a time stamp.
> +    pub fn tx_timestamp(&mut self) {
> +        // SAFETY: The type invariants guarantee that `self.0` is valid.
> +        unsafe {
> +            bindings::skb_tx_timestamp(self.0);
> +        }
> +    }
> +
> +    /// Consumes a [`sk_buff`] object.
> +    pub fn consume(self) {
> +        // SAFETY: The type invariants guarantee that `self.0` is valid.
> +        unsafe {
> +            bindings::kfree_skb_reason(self.0, bindings::skb_drop_reason_SKB_CONSUMED);
> +        }
> +        core::mem::forget(self);

I read the prior discussion and I just wanted to make one thing sure:
dropping the `sk_buff` is not required for the safety of a program (of course
it still is a leak and that is not good), so it does not mean UB can occur
at some later point.

If leaking a `sk_buff` is fine, then I have no complaints, but we need to
keep this in mind when reviewing code that uses `sk_buff`, since there
using `forget` or other ways of leaking objects should not happen.

--
Cheers,
Benno

> +    }
> +}
> +
> +impl Drop for SkBuff {
> +    #[inline(always)]
> +    fn drop(&mut self) {
> +        build_error!("skb must be released explicitly");
> +    }
> +}
> --
> 2.34.1
> 

Re: [PATCH v2 2/5] rust: add support for ethernet operations

[Benno Lossin]

> This improves abstractions for network device drivers to implement
> struct ethtool_ops, the majority of ethernet device drivers need to
> do.
> 
> struct ethtool_ops also needs to access to device private data like
> struct net_device_ops.
> 
> Currently, only get_ts_info operation is supported. The following
> patch adds the Rust version of the dummy network driver, which uses
> the operation.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/bindings/bindings_helper.h |  1 +
>  rust/kernel/net/dev.rs          | 87 ++++++++++++++++++++++++++++++++-
>  2 files changed, 87 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 468bf606f174..6446ff764980 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -8,6 +8,7 @@
> 
>  #include <linux/errname.h>
>  #include <linux/etherdevice.h>
> +#include <linux/ethtool.h>
>  #include <linux/netdevice.h>
>  #include <linux/slab.h>
>  #include <linux/refcount.h>
> diff --git a/rust/kernel/net/dev.rs b/rust/kernel/net/dev.rs
> index fe20616668a9..ff00616e4fef 100644
> --- a/rust/kernel/net/dev.rs
> +++ b/rust/kernel/net/dev.rs
> @@ -142,7 +142,7 @@ pub fn register(&mut self) -> Result {
>          if self.is_registered {
>              return Err(code::EINVAL);
>          }
> -        // SAFETY: The type invariants guarantee that `self.dev.ptr` is valid.
> +        // SAFETY: The type invariants of `Device` guarantee that `self.dev.ptr` is valid.

Shouldn't this be squashed into patch 1?

>          let ret = unsafe {
>              (*self.dev.ptr).netdev_ops = &Self::DEVICE_OPS;
>              bindings::register_netdev(self.dev.ptr)
> @@ -155,6 +155,18 @@ pub fn register(&mut self) -> Result {
>          }
>      }
> 
> +    /// Sets `ethtool_ops` of `net_device`.
> +    pub fn set_ether_operations<E: EtherOperations>(&mut self) -> Result {
> +        if self.is_registered {
> +            return Err(code::EINVAL);
> +        }
> +        // SAFETY: The type invariants of `Device` guarantee that `self.dev.ptr` is valid.
> +        unsafe {
> +            (*(self.dev.ptr)).ethtool_ops = &EtherOperationsAdapter::<E>::ETHER_OPS;
> +        }
> +        Ok(())
> +    }
> +

I think it would make sense to also add

```
pub fn setup_ether_operations(&mut self) -> Result
where
    T: EtherOperations
{
    self.set_ether_operations::<T>();
}
```

Since normally you would implement `EtherOperations` for the same type
that also implement `DeviceOperations`, right?

>      const DEVICE_OPS: bindings::net_device_ops = bindings::net_device_ops {
>          ndo_init: if <T>::HAS_INIT {
>              Some(Self::init_callback)
> @@ -328,3 +340,76 @@ fn drop(&mut self) {
>          build_error!("skb must be released explicitly");
>      }
>  }
> +
> +/// Builds the kernel's `struct ethtool_ops`.
> +struct EtherOperationsAdapter<E: EtherOperations> {
> +    _p: PhantomData<E>,
> +}
> +
> +impl<E: EtherOperations> EtherOperationsAdapter<E> {
> +    unsafe extern "C" fn get_ts_info_callback(
> +        netdev: *mut bindings::net_device,
> +        info: *mut bindings::ethtool_ts_info,
> +    ) -> core::ffi::c_int {
> +        from_result(|| {
> +            // SAFETY: The C API guarantees that `netdev` is valid while this function is running.
> +            let dev = unsafe { Device::from_ptr(netdev) };
> +            // SAFETY: The C API guarantees that `info` is valid while this function is running.
> +            let info = unsafe { EthtoolTsInfo::from_ptr(info) };
> +            E::get_ts_info(dev, info)?;
> +            Ok(0)
> +        })
> +    }
> +
> +    const ETHER_OPS: bindings::ethtool_ops = bindings::ethtool_ops {
> +        get_ts_info: if <E>::HAS_GET_TS_INFO {
> +            Some(Self::get_ts_info_callback)
> +        } else {
> +            None
> +        },
> +        // SAFETY: The rest is zeroed out to initialize `struct ethtool_ops`,
> +        // set `Option<&F>` to be `None`.
> +        ..unsafe { core::mem::MaybeUninit::<bindings::ethtool_ops>::zeroed().assume_init() }
> +    };
> +}
> +
> +/// Corresponds to the kernel's `struct ethtool_ops`.
> +#[vtable]
> +pub trait EtherOperations: ForeignOwnable + Send + Sync {
> +    /// Corresponds to `get_ts_info` in `struct ethtool_ops`.
> +    fn get_ts_info(_dev: Device<Self>, _info: EthtoolTsInfo) -> Result {
> +        Err(Error::from_errno(bindings::EOPNOTSUPP as i32))

Note that you have to use `-(EOPNOTSUPP as i32)`. Maybe just add this
in `error.rs` via the `declare_err` macro.

--
Cheers,
Benno

> +    }
> +}
> +
> +/// Corresponds to the kernel's `struct ethtool_ts_info`.
> +///
> +/// # Invariants
> +///
> +/// The pointer is valid.
> +pub struct EthtoolTsInfo(*mut bindings::ethtool_ts_info);
> +
> +impl EthtoolTsInfo {
> +    /// Creates a new `EthtoolTsInfo' instance.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must ensure that `ptr` must be valid.
> +    unsafe fn from_ptr(ptr: *mut bindings::ethtool_ts_info) -> Self {
> +        // INVARIANT: The safety requirements ensure the invariant.
> +        Self(ptr)
> +    }
> +
> +    /// Sets up the info for software timestamping.
> +    pub fn ethtool_op_get_ts_info<D: ForeignOwnable + Send + Sync>(
> +        dev: &Device<D>,
> +        info: &mut EthtoolTsInfo,
> +    ) -> Result {
> +        // SAFETY: The type invariants of `Device` guarantee that `dev.ptr` are valid.
> +        // The type invariants guarantee that `info.0` are valid.
> +        unsafe {
> +            bindings::ethtool_op_get_ts_info(dev.ptr, info.0);
> +        }
> +        Ok(())
> +    }
> +}
> --
> 2.34.1
> 

Re: [PATCH v2 3/5] rust: add methods for configure net_device

[Benno Lossin]

> adds methods to net::Device for the basic configurations of
> net_device.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/helpers.c         |   7 ++
>  rust/kernel/net/dev.rs | 185 ++++++++++++++++++++++++++++++++++++++++-
>  2 files changed, 191 insertions(+), 1 deletion(-)
> 
> diff --git a/rust/helpers.c b/rust/helpers.c
> index 70d50767ff4e..6c51deb18dc1 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -22,6 +22,7 @@
>  #include <linux/build_bug.h>
>  #include <linux/err.h>
>  #include <linux/errname.h>
> +#include <linux/etherdevice.h>
>  #include <linux/refcount.h>
>  #include <linux/mutex.h>
>  #include <linux/netdevice.h>
> @@ -31,6 +32,12 @@
>  #include <linux/wait.h>
> 
>  #ifdef CONFIG_NET
> +void rust_helper_eth_hw_addr_random(struct net_device *dev)
> +{
> +	eth_hw_addr_random(dev);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_eth_hw_addr_random);
> +
>  void *rust_helper_netdev_priv(const struct net_device *dev)
>  {
>  	return netdev_priv(dev);
> diff --git a/rust/kernel/net/dev.rs b/rust/kernel/net/dev.rs
> index ff00616e4fef..e4d8d8260c10 100644
> --- a/rust/kernel/net/dev.rs
> +++ b/rust/kernel/net/dev.rs
> @@ -8,9 +8,116 @@
>  //! [`include/linux/skbuff.h`](../../../../include/linux/skbuff.h),
>  //! [`include/uapi/linux/if_link.h`](../../../../include/uapi/linux/if_link.h).
> 
> -use crate::{bindings, build_error, error::*, prelude::vtable, types::ForeignOwnable};
> +use crate::{bindings, build_error, error::*, prelude::vtable, str::CStr, types::ForeignOwnable};
>  use {core::ffi::c_void, core::marker::PhantomData};
> 
> +/// Flags associated with a [`Device`].
> +pub mod flags {
> +    /// Interface is up.
> +    pub const IFF_UP: u32 = bindings::net_device_flags_IFF_UP;
> +    /// Broadcast address valid.
> +    pub const IFF_BROADCAST: u32 = bindings::net_device_flags_IFF_BROADCAST;
> +    /// Device on debugging.
> +    pub const IFF_DEBUG: u32 = bindings::net_device_flags_IFF_DEBUG;
> +    /// Loopback device.
> +    pub const IFF_LOOPBACK: u32 = bindings::net_device_flags_IFF_LOOPBACK;
> +    /// Has p-p link.
> +    pub const IFF_POINTOPOINT: u32 = bindings::net_device_flags_IFF_POINTOPOINT;
> +    /// Avoids use of trailers.
> +    pub const IFF_NOTRAILERS: u32 = bindings::net_device_flags_IFF_NOTRAILERS;
> +    /// Interface RFC2863 OPER_UP.
> +    pub const IFF_RUNNING: u32 = bindings::net_device_flags_IFF_RUNNING;
> +    /// No ARP protocol.
> +    pub const IFF_NOARP: u32 = bindings::net_device_flags_IFF_NOARP;
> +    /// Receives all packets.
> +    pub const IFF_PROMISC: u32 = bindings::net_device_flags_IFF_PROMISC;
> +    /// Receive all multicast packets.
> +    pub const IFF_ALLMULTI: u32 = bindings::net_device_flags_IFF_ALLMULTI;
> +    /// Master of a load balancer.
> +    pub const IFF_MASTER: u32 = bindings::net_device_flags_IFF_MASTER;
> +    /// Slave of a load balancer.
> +    pub const IFF_SLAVE: u32 = bindings::net_device_flags_IFF_SLAVE;
> +    /// Supports multicast.
> +    pub const IFF_MULTICAST: u32 = bindings::net_device_flags_IFF_MULTICAST;
> +    /// Capable of setting media type.
> +    pub const IFF_PORTSEL: u32 = bindings::net_device_flags_IFF_PORTSEL;
> +    /// Auto media select active.
> +    pub const IFF_AUTOMEDIA: u32 = bindings::net_device_flags_IFF_AUTOMEDIA;
> +    /// Dialup device with changing addresses.
> +    pub const IFF_DYNAMIC: u32 = bindings::net_device_flags_IFF_DYNAMIC;
> +}
> +
> +/// Private flags associated with a [`Device`].
> +pub mod priv_flags {
> +    /// 802.1Q VLAN device.
> +    pub const IFF_802_1Q_VLAN: u64 = bindings::netdev_priv_flags_IFF_802_1Q_VLAN;
> +    /// Ethernet bridging device.
> +    pub const IFF_EBRIDGE: u64 = bindings::netdev_priv_flags_IFF_EBRIDGE;
> +    /// Bonding master or slave device.
> +    pub const IFF_BONDING: u64 = bindings::netdev_priv_flags_IFF_BONDING;
> +    /// ISATAP interface (RFC4214).
> +    pub const IFF_ISATAP: u64 = bindings::netdev_priv_flags_IFF_ISATAP;
> +    /// WAN HDLC device.
> +    pub const IFF_WAN_HDLC: u64 = bindings::netdev_priv_flags_IFF_WAN_HDLC;
> +    /// dev_hard_start_xmit() is allowed to release skb->dst.
> +    pub const IFF_XMIT_DST_RELEASE: u64 = bindings::netdev_priv_flags_IFF_XMIT_DST_RELEASE;
> +    /// Disallows bridging this ether device.
> +    pub const IFF_DONT_BRIDGE: u64 = bindings::netdev_priv_flags_IFF_DONT_BRIDGE;
> +    /// Disables netpoll at run-time.
> +    pub const IFF_DISABLE_NETPOLL: u64 = bindings::netdev_priv_flags_IFF_DISABLE_NETPOLL;
> +    /// Device used as macvlan port.
> +    pub const IFF_MACVLAN_PORT: u64 = bindings::netdev_priv_flags_IFF_MACVLAN_PORT;
> +    /// Device used as bridge port.
> +    pub const IFF_BRIDGE_PORT: u64 = bindings::netdev_priv_flags_IFF_BRIDGE_PORT;
> +    /// Device used as Open vSwitch datapath port.
> +    pub const IFF_OVS_DATAPATH: u64 = bindings::netdev_priv_flags_IFF_OVS_DATAPATH;
> +    /// The interface supports sharing skbs on transmit.
> +    pub const IFF_TX_SKB_SHARING: u64 = bindings::netdev_priv_flags_IFF_TX_SKB_SHARING;
> +    /// Supports unicast filtering.
> +    pub const IFF_UNICAST_FLT: u64 = bindings::netdev_priv_flags_IFF_UNICAST_FLT;
> +    /// Device used as team port.
> +    pub const IFF_TEAM_PORT: u64 = bindings::netdev_priv_flags_IFF_TEAM_PORT;
> +    /// Device supports sending custom FCS.
> +    pub const IFF_SUPP_NOFCS: u64 = bindings::netdev_priv_flags_IFF_SUPP_NOFCS;
> +    /// Device supports hardware address change when it's running.
> +    pub const IFF_LIVE_ADDR_CHANGE: u64 = bindings::netdev_priv_flags_IFF_LIVE_ADDR_CHANGE;
> +    /// Macvlan device.
> +    pub const IFF_MACVLAN: u64 = bindings::netdev_priv_flags_IFF_MACVLAN;
> +    /// IFF_XMIT_DST_RELEASE not taking into account underlying stacked devices.
> +    pub const IFF_XMIT_DST_RELEASE_PERM: u64 =
> +        bindings::netdev_priv_flags_IFF_XMIT_DST_RELEASE_PERM;
> +    /// L3 master device.
> +    pub const IFF_L3MDEV_MASTER: u64 = bindings::netdev_priv_flags_IFF_L3MDEV_MASTER;
> +    /// Device can run without qdisc attached.
> +    pub const IFF_NO_QUEUE: u64 = bindings::netdev_priv_flags_IFF_NO_QUEUE;
> +    /// Device is a Open vSwitch master.
> +    pub const IFF_OPENVSWITCH: u64 = bindings::netdev_priv_flags_IFF_OPENVSWITCH;
> +    /// Device is enslaved to an L3 master.
> +    pub const IFF_L3MDEV_SLAVE: u64 = bindings::netdev_priv_flags_IFF_L3MDEV_SLAVE;
> +    /// Team device.
> +    pub const IFF_TEAM: u64 = bindings::netdev_priv_flags_IFF_TEAM;
> +    /// Device has had Rx Flow indirection table configured.
> +    pub const IFF_RXFH_CONFIGURED: u64 = bindings::netdev_priv_flags_IFF_RXFH_CONFIGURED;
> +    /// The headroom value is controlled by an external entity.
> +    pub const IFF_PHONY_HEADROOM: u64 = bindings::netdev_priv_flags_IFF_PHONY_HEADROOM;
> +    /// MACsec device.
> +    pub const IFF_MACSEC: u64 = bindings::netdev_priv_flags_IFF_MACSEC;
> +    /// Device doesn't support the rx_handler hook.
> +    pub const IFF_NO_RX_HANDLER: u64 = bindings::netdev_priv_flags_IFF_NO_RX_HANDLER;
> +    /// Failover master device.
> +    pub const IFF_FAILOVER: u64 = bindings::netdev_priv_flags_IFF_FAILOVER;
> +    /// Lower device of a failover master device.
> +    pub const IFF_FAILOVER_SLAVE: u64 = bindings::netdev_priv_flags_IFF_FAILOVER_SLAVE;
> +    /// Only invokes the rx handler of L3 master device.
> +    pub const IFF_L3MDEV_RX_HANDLER: u64 = bindings::netdev_priv_flags_IFF_L3MDEV_RX_HANDLER;
> +    /// Prevents ipv6 addrconf.
> +    pub const IFF_NO_ADDRCONF: u64 = bindings::netdev_priv_flags_IFF_NO_ADDRCONF;
> +    /// Capable of xmitting frames with skb_headlen(skb) == 0.
> +    pub const IFF_TX_SKB_NO_LINEAR: u64 = bindings::netdev_priv_flags_IFF_TX_SKB_NO_LINEAR;
> +    /// Supports setting carrier via IFLA_PROTO_DOWN.
> +    pub const IFF_CHANGE_PROTO_DOWN: u64 = bindings::netdev_priv_flags_IFF_CHANGE_PROTO_DOWN;
> +}
> +
>  /// Corresponds to the kernel's `struct net_device`.
>  ///
>  /// # Invariants
> @@ -49,6 +156,82 @@ pub fn drv_priv_data(&self) -> D::Borrowed<'_> {
>              ))
>          }
>      }
> +
> +    /// Sets the name of a device.
> +    pub fn set_name(&mut self, name: &CStr) -> Result {
> +        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
> +        unsafe {
> +            if name.len() > (*self.ptr).name.len() {
> +                return Err(code::EINVAL);
> +            }
> +            (*self.ptr)
> +                .name
> +                .as_mut_ptr()
> +                .copy_from_nonoverlapping(name.as_char_ptr(), name.len());
> +        }

Just to make sure, the `name` field in `net_device` does not need to
be nul-terminated, right?

--
Cheers,
Benno

> +        Ok(())
> +    }
> +
> +    /// Sets carrier.
> +    pub fn netif_carrier_on(&mut self) {
> +        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
> +        unsafe { bindings::netif_carrier_on(self.ptr) }
> +    }
> +
> +    /// Clears carrier.
> +    pub fn netif_carrier_off(&mut self) {
> +        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
> +        unsafe { bindings::netif_carrier_off(self.ptr) }
> +    }
> +
> +    /// Sets the max mtu of the device.
> +    pub fn set_max_mtu(&mut self, max_mtu: u32) {
> +        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
> +        unsafe {
> +            (*self.ptr).max_mtu = max_mtu;
> +        }
> +    }
> +
> +    /// Sets the minimum mtu of the device.
> +    pub fn set_min_mtu(&mut self, min_mtu: u32) {
> +        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
> +        unsafe {
> +            (*self.ptr).min_mtu = min_mtu;
> +        }
> +    }
> +
> +    /// Returns the flags of the device.
> +    pub fn get_flags(&self) -> u32 {
> +        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
> +        unsafe { (*self.ptr).flags }
> +    }
> +
> +    /// Sets the flags of the device.
> +    pub fn set_flags(&mut self, flags: u32) {
> +        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
> +        unsafe {
> +            (*self.ptr).flags = flags;
> +        }
> +    }
> +
> +    /// Returns the priv_flags of the device.
> +    pub fn get_priv_flags(&self) -> u64 {
> +        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
> +        unsafe { (*self.ptr).priv_flags }
> +    }
> +
> +    /// Sets the priv_flags of the device.
> +    pub fn set_priv_flags(&mut self, flags: u64) {
> +        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
> +        unsafe { (*self.ptr).priv_flags = flags }
> +    }
> +
> +    /// Generate a random Ethernet address (MAC) to be used by a net device
> +    /// and set addr_assign_type.
> +    pub fn set_random_eth_hw_addr(&mut self) {
> +        // SAFETY: The type invariants guarantee that `self.ptr` is valid.
> +        unsafe { bindings::eth_hw_addr_random(self.ptr) }
> +    }
>  }
> 
>  // SAFETY: `Device` is just a wrapper for the kernel`s `struct net_device`, which can be used
> --
> 2.34.1
>