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: <20231021.124907.1866440680448148505.fujita.tomonori@gmail.com>
Date: Sat, 21 Oct 2023 12:49:07 +0900 (JST)
From: FUJITA Tomonori <fujita.tomonori@...il.com>
To: nmi@...aspace.dk
Cc: andrew@...n.ch, fujita.tomonori@...il.com, netdev@...r.kernel.org,
 rust-for-linux@...r.kernel.org, miguel.ojeda.sandonis@...il.com,
 tmgross@...ch.edu, boqun.feng@...il.com, wedsonaf@...il.com,
 benno.lossin@...ton.me, greg@...ah.com
Subject: Re: [PATCH net-next v5 1/5] rust: core abstractions for network
 PHY drivers

On Fri, 20 Oct 2023 22:30:49 +0200
"Andreas Hindborg (Samsung)" <nmi@...aspace.dk> wrote:

>>> If this function is called with `u32::MAX` `(*phydev).speed` will become -1. Is that OK?
>>
>> Have you ever seen a Copper Ethernet interface which can do u32:MAX
>> Mbps? Do you think such a thing will ever be possible?
> 
> Probably not. Maybe a dummy device for testing? I don't know if it would
> be OK with a negative value, hence the question. If things break with a
> negative value, it makes sense to force the value into the valid range.
> Make it impossible to break it, instead of relying on nobody ever doing
> things you did not expect.

You can find discussion on this in the previous comments too.


>>> > +    /// Callback for notification of link change.
>>> > +    fn link_change_notify(_dev: &mut Device) {}
>>> 
>>> It is probably an error if these functions are called, and so BUG() would be
>>> appropriate? See the discussion in [1].
>>
>> Do you really want to kill the machine dead, no syncing of the disk,
>> loose everything not yet written to the disk, maybe corrupt the disk
>> etc?
> 
> A WARN then? Benno suggests a compile time error, that is probably a
> better approach. The code should not be called, so I would really want
> to know if that was ever the case.

These functions are never called so I don't think that WARN or
whatever doesn't matter. The api of vtable macro is misleading so I'm
not sure it's worth trying something with the api. I would prefer to
leave this alone until Benno's work.