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] [day] [month] [year] [list]

Date:	Mon, 27 Jun 2016 00:06:18 +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 Mon, 2016-06-27 at 00:02 +0200, Ben Hutchings wrote:
> 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.

I mean sfpdiag.c here.  I didn't check for duplication from sfpid.c,
but I think you've avoided that.

Ben.

-- 

Ben Hutchings
Humour is the best antidote to reality.

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