netdev - Re: [ethtool PATCH v1 2/2] ethtool:QSFP Plus/QSFP28 Diagnostics Information Support

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: <1466978549.2551.27.camel@decadent.org.uk>
Date:	Mon, 27 Jun 2016 00:02:29 +0200
From:	Ben Hutchings <ben@...adent.org.uk>
To:	Vidya Sagar Ravipati <vidya@...ulusnetworks.com>
Cc:	netdev@...r.kernel.org, bwh@...nel.org,
	David Miller <davem@...emloft.net>, bkenward@...arflare.com,
	daniel@...earbox.net, Gal Pressman <galp@...lanox.com>,
	Roopa Prabhu <roopa@...ulusnetworks.com>,
	Andy Gospodarek <gospo@...ulusnetworks.com>,
	Dustin Byford <dustin@...ulusnetworks.com>
Subject: Re: [ethtool PATCH v1 2/2] ethtool:QSFP Plus/QSFP28 Diagnostics
 Information Support

On Sun, 2016-06-26 at 09:40 -0700, Vidya Sagar Ravipati wrote:
> On Sun, Jun 26, 2016 at 2:33 AM, Ben Hutchings <ben@...adent.org.uk> wrote:
[...]
> > This looks very similar to sff8472_diags, only with the actual values
> > separated from the arrays of thresholds.
> > 
> > Can the structure and code be combined with sfpdiag.c, with the
> > additional per-channel diagnostics being optional?
> 
> Diagnostic dom information in QSFP  has lot more information compared
> to SFPs and as part of this checkin , basic dom information in qsfp which is
> equivalent to sfp dom  is getting exposed as part of this checkin.
> 
> Here are list of fields (not complete)  which  are used for  debugging QSFP
> issues, will be added for this structure in next patch sets
> a) TX/RX output amplitude conttrol
> b)  TX_DISABLE
> b) TX_FAULT
> c) TX CDR
> d) RX CDR
> e) RX output disable
> f) Rate select option
> 
> Please let me know if it make sense to maintain the different structure
> with above explanation  or whether it is required to be combined.
[...]

I think there's enough information in common that it does make sense to
use common reporting functions, and that in turn suggests that it would
make sense to use a common structure.  You could alternately have the
callers in sfpdiag.c and qsfp.c extract the relevant fields and pass
them into the reporting functions.

The substantial duplication of reporting code from sfpid.c in your
latest submission is not OK.

Ben.

-- 

Ben Hutchings
compatible: Gracefully accepts erroneous data from any source

Download attachment "signature.asc" of type "application/pgp-signature" (820 bytes)