On 17/12/2024 8:22 am, Vladimir Oltean wrote:
On Mon, Dec 16, 2024 at 01:47:17AM -0500, Faizal Rahim wrote:The i226 hardware doesn't implement the process of verification internally, this is left to the driver. Add a simple implementation of the state machine defined in IEEE 802.3-2018, Section 99.4.7. The state machine is started manually by user after "verify-enabled" command is enabled. Implementation includes: 1. Send and receive verify frame 2. Verification state handling 3. Send and receive response frame Tested by triggering verification handshake: $ sudo ethtool --set-mm enp1s0 pmac-enabled on $ sudo ethtool --set-mm enp1s0 tx-enabled on $ sudo ethtool --set-mm enp1s0 verify-enabled on Note that Ethtool API requires enabling "pmac-enabled on" and "tx-enabled on" before "verify-enabled on" can be issued. After the upcoming patch ("igc: Add support to get MAC Merge data via ethtool") is implemented, verification status can be checked using: $ ethtool --show-mm enp1s0 MAC Merge layer state for enp1s0: pMAC enabled: on TX enabled: on TX active: on TX minimum fragment size: 252 RX minimum fragment size: 252 Verify enabled: on Verify time: 128 Max verify time: 128 Verification status: SUCCEEDED Co-developed-by: Vinicius Costa Gomes <vinicius.gomes@xxxxxxxxx> Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@xxxxxxxxx> Signed-off-by: Faizal Rahim <faizal.abdul.rahim@xxxxxxxxxxxxxxx> ---Am I missing something, or this does not handle link state changes, where the verification should restart on each link up? (maybe the old link partner didn't support FPE and the new one does, or vice versa) Either I don't follow the link between igc_watchdog_task() and any verification related task, or it doesn't exist.
The latter. I missed this "link state changes" interaction, will rework, thanks.
Anyway, while browsing through this software implementation of a verification process, I cannot help but think we'd be making a huge mistake to allow each driver to reimplement it on its own. We just recently got stmmac to do something fairly clean, with the help and great perseverence of Furong Xu (now copied). I spent a bit of time extracting stmmac's core logic and putting it in ethtool. If Furong had such good will so as to regression-test the attached patch, do you think you could use this as a starting place instead, and implement some ops and call some library methods, instead of writing the entire logic yourself?
Totally agree with moving it to a layer reusable by any driver. Thank you so much for the skeleton patch implementing it in ethtool — I’ll expand on it from here.
