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
| ||
|
Message-ID: <CAKoUArk=MsPCXFgXkkdACQwpk+X1ionXO_yJm_=8kpHjrLXOZw@mail.gmail.com> Date: Sun, 26 Jun 2016 23:41:16 +0300 From: Rami Rosen <roszenrami@...il.com> To: Vidya Sagar Ravipati <vidya@...ulusnetworks.com> Cc: ben@...adent.org.uk, Netdev <netdev@...r.kernel.org>, bwh@...nel.org, David Miller <davem@...emloft.net>, bkenward@...arflare.com, daniel@...earbox.net, galp@...lanox.com, Roopa Prabhu <roopa@...ulusnetworks.com>, gospo@...ulusnetworks.com, dustin@...ulusnetworks.com Subject: Re: [ethtool PATCH v1 2/2] ethtool:QSFP Plus/QSFP28 Diagnostics Information Support Hi Vidya, Thanks a lot for your your work! Here are my 2 cents: > + /* SFP voltage in 0.1mV units */ > + __u16 sfp_voltage; > + /* SFP Temp in 16-bit signed 1/256 Celcius */ > + __s16 sfp_temp; > + /* [4] tables are low/high warn, low/high alarm */ You already had just 5 lines earlier: /* SFP voltage in 0.1mV units */ Shouldn't it be: /* SFP threshold voltage in 0.1mV units */ ? > + /* SFP voltage in 0.1mV units */ > + __u16 thresh_sfp_voltage[4]; pagging should be: paging, support_alarms should be:supports_alarms > + * If pagging support exists, then support_alarms is marked as 1 > + */ > + + if (eeprom_len == ETH_MODULE_SFF_8636_MAX_LEN) { + if (!(id[SFF8636_STATUS_2_OFFSET] & + SFF8636_STATUS_PAGE_3_PRESENT)) { + sd.supports_alarms = 1; + } + } Shouldn't it be power/ TX bias current fields? ("current" only once) > + /* > + * SFF-8636/8436 spec is not clear whether RX power/ TX bias current > + * current fields are supported or not. A valid temperature reading > + * is used as existence for TX/RX power. Should it be: SFF-8472/8079 (without "$")? > + * > + * Common utilities across SFF-8436/8636 and SFF-8472/8079$ > + * are defined in this file > + * Regards, Rami Rosen
Powered by blists - more mailing lists