Date:   Tue, 12 Oct 2021 16:25:11 +0300
From:   Ido Schimmel <idosch@...sch.org>
To:     netdev@...r.kernel.org
Cc:     mkubecek@...e.cz, popadrian1996@...il.com, andrew@...n.ch,
        mlxsw@...dia.com, moshe@...dia.com,
        Ido Schimmel <idosch@...dia.com>
Subject: [PATCH ethtool-next 00/14] ethtool: Use memory maps for EEPROM parsing

From: Ido Schimmel <idosch@...dia.com>

This patchset prepares ethtool(8) for retrieval and parsing of optional
and banked module EEPROM pages, such as the ones present in CMIS. This
is done by better integration of the recent 'MODULE_EEPROM_GET' netlink
interface into ethtool(8).

Background
==========

ethtool(8) contains parsers for various module EEPROM memory maps such
as SFF-8079, SFF-8636 and CMIS. Using the legacy IOCTL interface,
ethtool(8) can ask the kernel to provide a buffer with the EEPROM
contents. The buffer is then passed to the parsers that parse and print
the EEPROM contents.

The major disadvantage of this method is that in addition to ethtool(8),
the kernel also needs to be familiar with the layout of the various
memory maps, as it should not report to user space optional pages that
do not exist.

In addition, with the emergence of more complex layouts (e.g., CMIS)
that include both optional and banked pages, the layout of the linear
buffer provided by the kernel is unclear.

For these reasons, kernel 5.13 was extended with the 'MODULE_EEPROM_GET'
netlink message that allows user space to request specific EEPROM pages.

Motivation
==========

Unfortunately, the current integration of 'MODULE_EEPROM_GET' into
ethtool(8) is not ideal. In the IOCTL path, a single large buffer is
passed to the parsers, whereas in the netlink path, individual pages are
passed. This is problematic for several reasons.

First, this approach is not very scalable as standards such as CMIS
support a lot of optional and banked pages. Passing them as separate
arguments is not going to work.

Second, the knowledge of which optional and banked pages are available
should be encapsulated in the individual parsers, not in the common
netlink code (i.e., netlink/module-eeprom.c). Currently, the common code
is blindly requesting from the kernel optional pages that might not
exist.

Third, the difference in the way information is passed to the parsers
propagates all the way to the individual parsing functions. For example,
cmis_show_link_len() vs. cmis_show_link_len_from_page().

Implementation
==============

In order to solve above mentioned problems and make it easier to
integrate retrieval and parsing of optional and banked pages, this
patchset reworks the EEPROM parsing code to use memory maps.

For each parser, a structure describing the layout of the memory map is
initialized with pointers to individual pages.

In the IOCTL path, this structure contains pointers to sections of the
linear buffer that was retrieved from the kernel.

In the netlink path, this structure contains pointers to individual
pages requested from the kernel. Care is taken to ensure that pages that
do not exist are not requested from the kernel.

After the structure is initialized, it is passed to the parsing code
that parses and prints the information.

This approach can be easily extended to support more optional and banked
pages and allows us to keep the parsing code common to both the IOCTL
and netlink paths. The only difference lies in how the memory map is
initialized when the parser is invoked.

Testing
=======

Build tested each patch with the following configuration options:

netlink | pretty-dump
--------|------------
v       | v
x       | x
v       | x
x       | v

No differences in output before and after the patchset (*). Tested with
QSFP (PC/AOC), QSFP-DD (PC/AOC), SFP (PC) and both IOCTL and netlink.

No reports from AddressSanitizer / valgrind.

(*) The only difference is in a few registers in CMIS that were not
parsed correctly to begin with.

Patchset overview
=================

Patches #1-#4 move CMIS to use a memory map and consolidate the code
paths between the IOCTL and netlink paths.

Patches #5-#8 do the same for SFF-8636.

Patch #9 does the same for SFF-8079.

Patch #10 exports a function to allow parsers to request a specific
EEPROM page.

Patches #11-#13 change parsers to request only specific and valid EEPROM
pages instead of getting potentially invalid pages from the common
netlink code (i.e., netlink/module-eeprom.c).

Patch #14 converts the common netlink code to simply call into
individual parsers based on their SFF-8024 Identifier Value. The command
context is passed to these parsers instead of potentially invalid pages.

Ido Schimmel (14):
  cmis: Rename CMIS parsing functions
  cmis: Initialize CMIS memory map
  cmis: Use memory map during parsing
  cmis: Consolidate code between IOCTL and netlink paths
  sff-8636: Rename SFF-8636 parsing functions
  sff-8636: Initialize SFF-8636 memory map
  sff-8636: Use memory map during parsing
  sff-8636: Consolidate code between IOCTL and netlink paths
  sff-8079: Split SFF-8079 parsing function
  netlink: eeprom: Export a function to request an EEPROM page
  cmis: Request specific pages for parsing in netlink path
  sff-8636: Request specific pages for parsing in netlink path
  sff-8079: Request specific pages for parsing in netlink path
  netlink: eeprom: Defer page requests to individual parsers

 cmis.c                  | 268 ++++++++++++++--------
 cmis.h                  |   8 +-
 ethtool.c               |   8 +-
 internal.h              |   8 +-
 netlink/extapi.h        |  11 +
 netlink/module-eeprom.c | 318 ++++++++------------------
 qsfp.c                  | 484 +++++++++++++++++++++++++---------------
 sfpid.c                 |  28 ++-
 8 files changed, 635 insertions(+), 498 deletions(-)

-- 
2.31.1

Powered by blists - more mailing lists