netdev - Re: Adding IEEE802.3cg Clause 148 PLCA support to Linux

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: <Y2gFj9wZzJO6z2v8@lunn.ch>
Date:   Sun, 6 Nov 2022 20:05:51 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     piergiorgio.beruto@...il.com
Cc:     netdev@...r.kernel.org
Subject: Re: Adding IEEE802.3cg Clause 148 PLCA support to Linux

On Sun, Nov 06, 2022 at 07:11:32PM +0100, piergiorgio.beruto@...il.com wrote:
> > I suggest you define new ethtool netlink messages. I don't think PHY tunables would make a good interface, since you have > multiple values which need configuring, and you also have some status information.

Your email client is messing up emails. I follow the netique rules, my
lines are wrapped at around 75 characters. This is recommended
practice for all Linux kernel mailing lists. Your mailer has destroyed
this. Please also wrap your own text at about 75 characters.

> That sounds fair to me, thanks for your advice.
> 
> > So you probably want a message to set the configuration, and another to get the current configuration. For the set, you 
> > probably want an attribute per configuration value, and allow a subset of attributes to be included in the message. The get 
> > configuration should by default return all the attributes, but not enforce this, since some vendor will implement it wrong 
> > and miss something out.
> 
> Yes, that sounds about right. If you have any hint on where in the code to start looking at, I'll start from there.

ethtool --cable-test packs a number of optional attributes into a
netlink message. It then gets passed to phylib. You could use that as
an example. The way cable tests results are passed back later is
pretty unusual, so don't copy that code!

> > What I don't see in the Open Alliance spec is anything about interrupts. It would be interesting to see if any vendor triggers 
> > an interrupt when PST changes. A PHY which has this should probably send a linkstate message to userspace reporting the 
> > state change. For PHYs without interrupts, phylib will poll the read_status method once per second. You probably want to 
> > check the PST bit during that poll. If EN is true, but PST is false, is the link considered down?
> 

> This is actually an interesting point. First of all, yes, vendors do have IRQs for the PST. At least, the products I'm working on do, including the already released NCN26010.

Each PHY driver is going to need its own code for enabling the
interrupt, handling etc, since none of this is standardized. This is
one reason why you provide helpers, but don't force there use.

> My thinking is that the PST should be taken into account to evaluate the status of the link. On a multi-drop network with no autoneg and no link training the link status would not make much sense anyway, just like the connected status of an UDP socket wouldn't make sense.

So the read_status() call should evaluate the PST bit, along with
EN. Again, a helper to do that would be useful.

The user API is the most important bit of this work. Linux considers
the uAPI an stable ABI. Once you have defined it, it cannot change in
ways which break backwards compatibility. So the initial reviews of
code you present will concentrate on the uAPI. Once that is good,
reviews will then swap to all the implementation details in phylib and
the drivers.

    Andrew