netdev - Re: iopoll abstraction

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: <20241002.095636.680321517586867502.fujita.tomonori@gmail.com>
Date: Wed, 02 Oct 2024 09:56:36 +0000 (UTC)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: dirk.behme@...bosch.com
Cc: andrew@...n.ch, aliceryhl@...gle.com, fujita.tomonori@...il.com,
 netdev@...r.kernel.org, rust-for-linux@...r.kernel.org,
 hkallweit1@...il.com, tmgross@...ch.edu, ojeda@...nel.org,
 alex.gaynor@...il.com, gary@...yguo.net, bjorn3_gh@...tonmail.com,
 benno.lossin@...ton.me, a.hindborg@...sung.com
Subject: Re: iopoll abstraction

On Wed, 2 Oct 2024 06:39:48 +0200
Dirk Behme <dirk.behme@...bosch.com> wrote:

>> I generally point developers at iopoll.h, because developers nearly
>> always get this sort of polling for something to happen wrong.
>> The kernel sleep functions guarantee the minimum sleep time. They say
>> nothing about the maximum sleep time. You can ask it to sleep for 1ms,
>> and in reality, due to something stealing the CPU and not being RT
>> friendly, it actually sleeps for 10ms. This extra long sleep time
>> blows straight past your timeout, if you have a time based timeout.
>> What most developers do is after the sleep() returns they check to see
>> if the timeout has been reached and then exit with -ETIMEDOUT. They
>> don't check the state of the hardware, which given its had a long time
>> to do its thing, probably is now in a good state. But the function
>> returns -ETIMEDOUT.
>> There should always be a check of the hardware state after the sleep,
>> in order to determine ETIMEDOUT vs 0.
>> As i said, most C developers get this wrong. So i don't really see why
>> Rust developers also will not get this wrong. So i like to discourage
>> this sort of code, and have Rust implementations of iopoll.h.
> 
> 
> Do we talk about some simple Rust wrappers for the macros in iopoll.h?
> E.g. something like [1]?
> 
> Or are we talking about some more complex (safety) dependencies which
> need some more complex abstraction handling?

(snip)

> int rust_helper_readb_poll_timeout(const volatile void * addr,
>                                   u64 val, u64 cond, u64 delay_us,
>                                   u64 timeout_us)
> {
>        return readb_poll_timeout(addr, val, cond, delay_us, timeout_us);
> }

I'm not sure a simple wrapper for iopoll.h works. We need to pass a
function. I'm testing a macro like the following (not added ktime
timeout yet):

macro_rules! read_poll_timeout {
    ($op:expr, $val:expr, $cond:expr, $sleep:expr, $timeout:expr, $($args:expr),*) => {{
        let _ = $val;
        loop {
            $val = $op($($args),*);
            if $cond {
                break;
            }
            kernel::delay::sleep($sleep);
        }
        if $cond {
            Ok(())
        } else {
            Err(kernel::error::code::ETIMEDOUT)
        }
    }};
}