netdev - Re: [PATCH net-next 4/8] net/funeth: ethtool operations

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: <Yc4x97vqj2fU9Zg/@lunn.ch>
Date:   Thu, 30 Dec 2021 23:25:59 +0100
From:   Andrew Lunn <andrew@...n.ch>
To:     Dimitris Michailidis <d.michailidis@...gible.com>
Cc:     davem@...emloft.net, kuba@...nel.org, netdev@...r.kernel.org
Subject: Re: [PATCH net-next 4/8] net/funeth: ethtool operations

> > I _think_ this is wrong. pause->autoneg means we are autoneg'ing
> > pause, not that we are using auto-neg in general. The user can have
> > autoneg turned on, but force pause by setting pause->autoneg to False.
> > In that case, the pause->rx_pause and pause->tx_pause are given direct
> > to the MAC, not auto negotiated.
> 
> Having this mixed mode needs device FW support, which isn't there today.

So if you are asked to set pause with pause->autoneg False, return
-EOPNOTSUPP. And pause get should always return True. It is O.K, to
support a subset of a feature, and say you don't support the
rest. That is much better than wrongly implementing it until your
firmware gets the needed support.

> If you force pause, then the link flaps and negotiates FW will apply the new
> negotiated settings. For your scenario you'd want it to support only partial
> application. Meanwhile the partner doesn't know we don't obey the negotiated
> settings so I am suspicious that all of this would work.

What happen when Linux is controlling the hardware, not firmware, is
that phylib makes a callback into the MAC driver telling it the
results of the autoneg. Part of those results are what pause has been
negotiated, if pause is part of the negotiation. The MAC driver then
needs to program the MAC hardware with that information. If pause
autoneg is not being used, you directly program the hardware. When you
have hidden the hardware from Linux, you need a similar API to the
firmware to tell it how to program the hardware.

Forcing pause is mostly there to work around broken link peers who get
pause wrong. And unfortunately, lots of drivers get pause wrong, and
it is not helped by the API being poorly defined, and people
re-inventing the wheel by using firmware, not Linux to control the
hardware. But it also means you don't care too much if the link peer
is confused, it was probably doing the wrong thing anyway.

      Andrew