netdev - Re: [PATCH net-next v5 1/5] rust: core abstractions for network PHY drivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20231020.093446.482864708938996774.fujita.tomonori@gmail.com>
Date: Fri, 20 Oct 2023 09:34:46 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: benno.lossin@...ton.me
Cc: fujita.tomonori@...il.com, netdev@...r.kernel.org,
 rust-for-linux@...r.kernel.org, andrew@...n.ch,
 miguel.ojeda.sandonis@...il.com, tmgross@...ch.edu, boqun.feng@...il.com,
 wedsonaf@...il.com, greg@...ah.com
Subject: Re: [PATCH net-next v5 1/5] rust: core abstractions for network
 PHY drivers

On Thu, 19 Oct 2023 15:20:51 +0000
Benno Lossin <benno.lossin@...ton.me> wrote:

> I would like to remove the mutable static variable and simplify
> the macro.

How about adding DriverVTable array to Registration?

/// Registration structure for a PHY driver.
///
/// # Invariants
///
/// The `drivers` slice are currently registered to the kernel via `phy_drivers_register`.
pub struct Registration<const N: usize> {
    drivers: [DriverVTable; N],
}

impl<const N: usize> Registration<{ N }> {
    /// Registers a PHY driver.
    pub fn register(
        module: &'static crate::ThisModule,
        drivers: [DriverVTable; N],
    ) -> Result<Self> {
        let mut reg = Registration { drivers };
        let ptr = reg.drivers.as_mut_ptr().cast::<bindings::phy_driver>();
        // SAFETY: The type invariants of [`DriverVTable`] ensure that all elements of the `drivers` slice
        // are initialized properly. So an FFI call with a valid pointer.
        to_result(unsafe {
            bindings::phy_drivers_register(ptr, reg.drivers.len().try_into()?, module.0)
        })?;
        // INVARIANT: The `drivers` slice is successfully registered to the kernel via `phy_drivers_register`.
        Ok(reg)
    }
}

Then the macro becomes simpler. No need to add any public
functions. Also I think that this approach supports the manual
registration.


    (drivers: [$($driver:ident),+], device_table: [$($dev:expr),+], $($f:tt)*) => {
        const N: usize = $crate::module_phy_driver!(@count_devices $($driver),+);
        struct Module {
            _reg: ::kernel::net::phy::Registration<N>,
        }

        $crate::prelude::module! {
            type: Module,
            $($f)*
        }

        impl ::kernel::Module for Module {
            fn init(module: &'static ThisModule) -> Result<Self> {
                let drivers = [$(::kernel::net::phy::create_phy_driver::<$driver>()),+];
                let reg = ::kernel::net::phy::Registration::register(module, drivers)?;
                Ok(Module { _reg: reg })
            }
        }

        $crate::module_phy_driver!(@device_table [$($dev),+]);
    }