mbox series

[ethtool,v2,0/4] Extend module EEPROM API

Message ID 1621865940-287332-1-git-send-email-moshe@nvidia.com
Headers show
Series Extend module EEPROM API | expand

Message

Moshe Shemesh May 24, 2021, 2:18 p.m. UTC 
Ethtool supports module EEPROM dumps via the `ethtool -m <dev>` command.
But in current state its functionality is limited - offset and length
parameters, which are used to specify a linear desired region of EEPROM
data to dump, is not enough, considering emergence of complex module
EEPROM layouts such as CMIS 4.0.

Moreover, CMIS 4.0 extends the amount of pages that may be accessible by
introducing another parameter for page addressing - banks. Besides,
currently module EEPROM is represented as a chunk of concatenated pages,
where lower 128 bytes of all pages, except page 00h, are omitted. Offset
and length are used to address parts of this fake linear memory. But in
practice drivers, which implement get_module_info() and
get_module_eeprom() ethtool ops still calculate page number and set I2C
address on their own.

This series adds support in `ethtool -m` of dumping an arbitrary page
specified by page number, bank number and I2C address. Implement netlink
handler for `ethtool -m` in order to make such requests to the kernel
and extend CLI by adding corresponding parameters.
New command line format:
 ethtool -m <dev> [hex on|off] [raw on|off] [offset N] [length N] [page N] [bank N] [i2c N]

Netlink infrastructure works on per-page basis and allows dumps of a
single page at once. But in case user requests human-readable output,
which currently may require more than one page, userspace can make such
additional calls to kernel on demand and place pages in a linked list.
It allows to get pages from cache on demand and pass them to refactored
SFF decoders.

Change Log:
v1 -> v2:
- Changed offset defines to specification values.
- Added default offset value (128) if page number is specified.
- Fixed return values.
- Removed page_available()

Vladyslav Tarasiuk (4):
  ethtool: Add netlink handler for getmodule (-m)
  ethtool: Refactor human-readable module EEPROM output for new API
  ethtool: Rename QSFP-DD identifiers to use CMIS 4.0
  ethtool: Update manpages to reflect changes to getmodule (-m) command

 Makefile.am             |   3 +-
 qsfp-dd.c => cmis4.c    | 220 +++++++++++----------
 cmis4.h                 | 128 +++++++++++++
 ethtool.8.in            |  14 ++
 ethtool.c               |   4 +
 internal.h              |  12 ++
 list.h                  |  34 ++++
 netlink/desc-ethtool.c  |  13 ++
 netlink/extapi.h        |   2 +
 netlink/module-eeprom.c | 416 ++++++++++++++++++++++++++++++++++++++++
 qsfp-dd.h               |  29 +--
 qsfp.c                  | 130 +++++++------
 qsfp.h                  |  51 ++---
 sff-common.c            |   3 +
 sff-common.h            |   3 +-
 15 files changed, 870 insertions(+), 192 deletions(-)
 rename qsfp-dd.c => cmis4.c (55%)
 create mode 100644 cmis4.h
 create mode 100644 list.h
 create mode 100644 netlink/module-eeprom.c

Comments

Ido Schimmel May 31, 2021, 2:23 p.m. UTC | #1 
On Mon, May 24, 2021 at 05:18:59PM +0300, Moshe Shemesh wrote:
> From: Vladyslav Tarasiuk <vladyslavt@nvidia.com>

> 

> QSFP-DD and DSFP EEPROM layout complies to CMIS 4.0 specification. As

> DSFP support is added, there are currently two standards, which share

> the same infrastructure. Rename QSFP_DD and qsfp_dd occurrences to use

> CMIS4 or cmis4 respectively to make function names generic for any

> module compliant to CMIS 4.0.

> 

> Signed-off-by: Vladyslav Tarasiuk <vladyslavt@nvidia.com>

> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>

> ---

>  Makefile.am             |   2 +-

>  qsfp-dd.c => cmis4.c    | 210 ++++++++++++++++++++--------------------

>  cmis4.h                 | 128 ++++++++++++++++++++++++

>  netlink/module-eeprom.c |   2 +-

>  qsfp.c                  |   2 +-

>  5 files changed, 236 insertions(+), 108 deletions(-)

>  rename qsfp-dd.c => cmis4.c (56%)

>  create mode 100644 cmis4.h


Is there a reason to call this "cmis4" instead of just "cmis"? Revision
5.0 was published earlier this month [1] and I assume more revisions
will follow.

Other standards (e.g., SFF-8024) also have multiple revisions and the
revision number is only mentioned in the "revision compliance" field.

[1] http://www.qsfp-dd.com/wp-content/uploads/2021/05/CMIS5p0.pdf
Moshe Shemesh June 3, 2021, 7:17 a.m. UTC | #2 
On 5/31/2021 5:23 PM, Ido Schimmel wrote:
> On Mon, May 24, 2021 at 05:18:59PM +0300, Moshe Shemesh wrote:

>> From: Vladyslav Tarasiuk <vladyslavt@nvidia.com>

>>

>> QSFP-DD and DSFP EEPROM layout complies to CMIS 4.0 specification. As

>> DSFP support is added, there are currently two standards, which share

>> the same infrastructure. Rename QSFP_DD and qsfp_dd occurrences to use

>> CMIS4 or cmis4 respectively to make function names generic for any

>> module compliant to CMIS 4.0.

>>

>> Signed-off-by: Vladyslav Tarasiuk <vladyslavt@nvidia.com>

>> Reviewed-by: Moshe Shemesh <moshe@nvidia.com>

>> ---

>>   Makefile.am             |   2 +-

>>   qsfp-dd.c => cmis4.c    | 210 ++++++++++++++++++++--------------------

>>   cmis4.h                 | 128 ++++++++++++++++++++++++

>>   netlink/module-eeprom.c |   2 +-

>>   qsfp.c                  |   2 +-

>>   5 files changed, 236 insertions(+), 108 deletions(-)

>>   rename qsfp-dd.c => cmis4.c (56%)

>>   create mode 100644 cmis4.h

> Is there a reason to call this "cmis4" instead of just "cmis"? Revision

> 5.0 was published earlier this month [1] and I assume more revisions

> will follow.



We called it cmis4 as we comply here with CMIS version 4.

However, I understand your point that other eeprom module specifications 
only spec number is mentioned and probably we can do the same here.

Andrew, Michal, WDYT ?


> Other standards (e.g., SFF-8024) also have multiple revisions and the

> revision number is only mentioned in the "revision compliance" field.

>

> [1] http://www.qsfp-dd.com/wp-content/uploads/2021/05/CMIS5p0.pdf