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: <e5109b0f-5fd5-4ae7-91e2-3975e3371ebb@lunn.ch>
Date: Fri, 20 Oct 2023 21:59:02 +0200
From: Andrew Lunn <andrew@...n.ch>
To: "Andreas Hindborg (Samsung)" <nmi@...aspace.dk>
Cc: FUJITA Tomonori <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, Oct 20, 2023 at 07:26:50PM +0200, Andreas Hindborg (Samsung) wrote:
> 
> Hi,
> 
> FUJITA Tomonori <fujita.tomonori@...il.com> writes:
> 
> <cut>
> 
> > +
> > +    /// Returns true if the link is up.
> > +    pub fn get_link(&self) -> bool {
> > +        const LINK_IS_UP: u32 = 1;
> > +        // SAFETY: `phydev` is pointing to a valid object by the type invariant of `Self`.
> > +        let phydev = unsafe { *self.0.get() };
> > +        phydev.link() == LINK_IS_UP
> > +    }
> 
> I would prefer `is_link_up` or `link_is_up`.

Hi Andreas

Have you read the comment on the previous versions of this patch. It
seems like everybody wants a different name for this, and we are going
round and round and round. Please could you spend the time to read all
the previous comments and then see if you still want this name, and
explain why. Otherwise i doubt we are going to reach consensus.

> 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?

> > +    /// 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?

      Andrew

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ