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]
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)

Powered by blists - more mailing lists