netdev - Re: [PATCH net-next 0/2] GCPS Spec Compliance Patch Set

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: <Z/V2pCKe8N6Uxa0O@home.paul.comp>
Date: Tue, 8 Apr 2025 22:19:00 +0300
From: Paul Fertser <fercerpav@...il.com>
To: Hari Kalavakunta <kalavakunta.hari.prasad@...il.com>
Cc: Sam Mendoza-Jonas <sam@...dozajonas.com>, davem@...emloft.net,
        edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
        horms@...nel.org, netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        npeacock@...a.com, akozlov@...a.com
Subject: Re: [PATCH net-next 0/2] GCPS Spec Compliance Patch Set

On Tue, Apr 08, 2025 at 11:53:47AM -0700, Hari Kalavakunta wrote:
> On 4/8/2025 11:26 AM, Paul Fertser wrote:
> > Can you please add the checks so that we are sure that hardware,
> > software and the specification all match after your fixes?
>
> Sure, I can give a try. Could you please provide an example or guideline
> that I can use as a reference for proper alignment?

Well, there's ncsi_validate_rsp_pkt() and also some handlers use
netdev_warn() or netdev_err() as appropriate and in any case they do
not try to use the returned data if it didn't pass validation and
return an error instead.

> > Also, please do provide the example counter values read from real
> > hardware (even if they're not yet exposed properly you can still
> > obtain them with some hack; don't forget to mention what network card
> > they were read from).
>
> Our verification process is centered on confirming that the counter values
> are accurately populated within the ncsi_channel_stats struct, specifically
> in the ncsi_rsp_handler_gcps function. This verification is conducted using
> synthesized statistics, rather than actual data from a network card. Sure, I
> will provide NCSI packet capture showing the synthesized data used for
> testing by end of the day.

In other words, you're testing your code only with simulated data so
there's no way to guarantee it's going to work on any real life
hardware (as we know hardware doesn't always exactly match the specs)?
That's unsettling. Please do mention it in the commit log, it's an
essential point. Better yet, consider going a bit off-centre after the
regular verification and do a control run on real hardware.

After all, that's what the code is for so if it all possible it's
better to know if it does the actual job before merging (to avoid
noise from follow-up patches like yours which fix something that never
worked because it was never tested).