[PATCH ethtool-next 00/14] ethtool: Use memory maps for EEPROM parsing

[PATCH ethtool-next 00/14] ethtool: Use memory maps for EEPROM parsing

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.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

[PATCH ethtool-next 01/14] cmis: Rename CMIS parsing functions

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Currently, there are two CMIS parsing functions. qsfp_dd_show_all() and
cmis_show_all(). The former is called from the IOCTL path with a buffer
containing EEPROM contents and the latter is called from the netlink
path with pointer to individual EEPROM pages.

Rename them with '_ioctl' and '_nl' suffixes to make the distinction
clear.

In subsequent patches, these two functions will only differ in the way
they initialize the CMIS memory map for parsing, while the parsing code
itself will be shared between the two.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 cmis.c                  | 6 +++---
 cmis.h                  | 6 +++---
 netlink/module-eeprom.c | 2 +-
 qsfp.c                  | 2 +-
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/cmis.c b/cmis.c
index 591cc72953b7..68c5b2d3277b 100644
--- a/cmis.c
+++ b/cmis.c
@@ -326,7 +326,7 @@ static void cmis_show_vendor_info(const __u8 *id)
 			       "CLEI code");
 }
 
-void qsfp_dd_show_all(const __u8 *id)
+void cmis_show_all_ioctl(const __u8 *id)
 {
 	cmis_show_identifier(id);
 	cmis_show_power_info(id);
@@ -340,8 +340,8 @@ void qsfp_dd_show_all(const __u8 *id)
 	cmis_show_rev_compliance(id);
 }
 
-void cmis_show_all(const struct ethtool_module_eeprom *page_zero,
-		   const struct ethtool_module_eeprom *page_one)
+void cmis_show_all_nl(const struct ethtool_module_eeprom *page_zero,
+		      const struct ethtool_module_eeprom *page_one)
 {
 	const __u8 *page_zero_data = page_zero->data;
 
diff --git a/cmis.h b/cmis.h
index e3012ccfdd79..734b90f4ddb4 100644
--- a/cmis.h
+++ b/cmis.h
@@ -120,9 +120,9 @@
 #define YESNO(x) (((x) != 0) ? "Yes" : "No")
 #define ONOFF(x) (((x) != 0) ? "On" : "Off")
 
-void qsfp_dd_show_all(const __u8 *id);
+void cmis_show_all_ioctl(const __u8 *id);
 
-void cmis_show_all(const struct ethtool_module_eeprom *page_zero,
-		   const struct ethtool_module_eeprom *page_one);
+void cmis_show_all_nl(const struct ethtool_module_eeprom *page_zero,
+		      const struct ethtool_module_eeprom *page_one);
 
 #endif /* CMIS_H__ */
diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
index 48cd2cc55bee..fc4ef1a53aff 100644
--- a/netlink/module-eeprom.c
+++ b/netlink/module-eeprom.c
@@ -332,7 +332,7 @@ static void decoder_print(void)
 		break;
 	case SFF8024_ID_QSFP_DD:
 	case SFF8024_ID_DSFP:
-		cmis_show_all(page_zero, page_one);
+		cmis_show_all_nl(page_zero, page_one);
 		break;
 	default:
 		dump_hex(stdout, page_zero->data, page_zero->length, page_zero->offset);
diff --git a/qsfp.c b/qsfp.c
index 3f37f1036e96..27fdd3bd1771 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -856,7 +856,7 @@ static void sff8636_show_page_zero(const __u8 *id)
 void sff8636_show_all(const __u8 *id, __u32 eeprom_len)
 {
 	if (id[SFF8636_ID_OFFSET] == SFF8024_ID_QSFP_DD) {
-		qsfp_dd_show_all(id);
+		cmis_show_all_ioctl(id);
 		return;
 	}
 
-- 
2.31.1

[PATCH ethtool-next 02/14] cmis: Initialize CMIS memory map

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

The CMIS memory map [1] consists of Lower Memory and Upper Memory.

The content of the Lower Memory is fixed and can be addressed using an
offset between 0 and 127 (inclusive).

The Upper Memory is variable and optional and can be addressed by
specifying a bank number, a page number and an offset between 128 and
255 (inclusive).

Create a structure describing this memory map and initialize it with
pointers to available pages.

In the IOCTL path, the structure holds pointers to regions of the
continuous buffer passed to user space via the 'ETHTOOL_GMODULEEEPROM'
command.

In the netlink path, the structure holds pointers to individual pages
passed to user space via the 'MODULE_EEPROM_GET' message.

This structure will later allow us to consolidate the IOCTL and netlink
parsing code paths and also easily support additional EEPROM pages.

[1] CMIS Rev. 5, pag. 97, section 8.1.1, Figure 8-1

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 cmis.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 cmis.h |  2 ++
 2 files changed, 65 insertions(+)

diff --git a/cmis.c b/cmis.c
index 68c5b2d3277b..8a6788416a00 100644
--- a/cmis.c
+++ b/cmis.c
@@ -13,6 +13,15 @@
 #include "sff-common.h"
 #include "cmis.h"
 
+struct cmis_memory_map {
+	const __u8 *lower_memory;
+	const __u8 *upper_memory[1][2];	/* Bank, Page */
+#define page_00h upper_memory[0x0][0x0]
+#define page_01h upper_memory[0x0][0x1]
+};
+
+#define CMIS_PAGE_SIZE		0x80
+
 static void cmis_show_identifier(const __u8 *id)
 {
 	sff8024_show_identifier(id, CMIS_ID_OFFSET);
@@ -326,8 +335,34 @@ static void cmis_show_vendor_info(const __u8 *id)
 			       "CLEI code");
 }
 
+static void cmis_memory_map_init_buf(struct cmis_memory_map *map,
+				     const __u8 *id)
+{
+	/* Lower Memory and Page 00h are always present.
+	 *
+	 * Offset into Upper Memory is between page size and twice the page
+	 * size. Therefore, set the base address of each page to base address
+	 * plus page size multiplied by the page number.
+	 */
+	map->lower_memory = id;
+	map->page_00h = id;
+
+	/* Page 01h is only present when the module memory model is paged and
+	 * not flat.
+	 */
+	if (map->lower_memory[CMIS_MEMORY_MODEL_OFFSET] &
+	    CMIS_MEMORY_MODEL_MASK)
+		return;
+
+	map->page_01h = id + CMIS_PAGE_SIZE;
+}
+
 void cmis_show_all_ioctl(const __u8 *id)
 {
+	struct cmis_memory_map map = {};
+
+	cmis_memory_map_init_buf(&map, id);
+
 	cmis_show_identifier(id);
 	cmis_show_power_info(id);
 	cmis_show_connector(id);
@@ -340,10 +375,38 @@ void cmis_show_all_ioctl(const __u8 *id)
 	cmis_show_rev_compliance(id);
 }
 
+static void
+cmis_memory_map_init_pages(struct cmis_memory_map *map,
+			   const struct ethtool_module_eeprom *page_zero,
+			   const struct ethtool_module_eeprom *page_one)
+{
+	/* Lower Memory and Page 00h are always present.
+	 *
+	 * Offset into Upper Memory is between page size and twice the page
+	 * size. Therefore, set the base address of each page to its base
+	 * address minus page size. For Page 00h, this is the address of the
+	 * Lower Memory.
+	 */
+	map->lower_memory = page_zero->data;
+	map->page_00h = page_zero->data;
+
+	/* Page 01h is only present when the module memory model is paged and
+	 * not flat.
+	 */
+	if (map->lower_memory[CMIS_MEMORY_MODEL_OFFSET] &
+	    CMIS_MEMORY_MODEL_MASK)
+		return;
+
+	map->page_01h = page_one->data - CMIS_PAGE_SIZE;
+}
+
 void cmis_show_all_nl(const struct ethtool_module_eeprom *page_zero,
 		      const struct ethtool_module_eeprom *page_one)
 {
 	const __u8 *page_zero_data = page_zero->data;
+	struct cmis_memory_map map = {};
+
+	cmis_memory_map_init_pages(&map, page_zero, page_one);
 
 	cmis_show_identifier(page_zero_data);
 	cmis_show_power_info(page_zero_data);
diff --git a/cmis.h b/cmis.h
index 734b90f4ddb4..53cbb5f57127 100644
--- a/cmis.h
+++ b/cmis.h
@@ -4,6 +4,8 @@
 /* Identifier and revision compliance (Page 0) */
 #define CMIS_ID_OFFSET				0x00
 #define CMIS_REV_COMPLIANCE_OFFSET		0x01
+#define CMIS_MEMORY_MODEL_OFFSET		0x02
+#define CMIS_MEMORY_MODEL_MASK			0x80
 
 #define CMIS_MODULE_TYPE_OFFSET			0x55
 #define CMIS_MT_MMF				0x01
-- 
2.31.1

[PATCH ethtool-next 03/14] cmis: Use memory map during parsing

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Instead of passing one large buffer to the individual parsing functions,
use the memory map structure from the previous patch.

This has the added benefit of checking which optional pages are actually
available and it will also allow us to consolidate the IOCTL and netlink
parsing code paths.

Tested by making sure that the only differences in output in both the
IOCTL and netlink paths before and after the patch are in a few
registers in Page 01h that were previously parsed from Page 00h.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 cmis.c | 175 +++++++++++++++++++++++++++++----------------------------
 cmis.h |   1 -
 2 files changed, 88 insertions(+), 88 deletions(-)

diff --git a/cmis.c b/cmis.c
index 8a6788416a00..2e01446b2315 100644
--- a/cmis.c
+++ b/cmis.c
@@ -22,19 +22,19 @@ struct cmis_memory_map {
 
 #define CMIS_PAGE_SIZE		0x80
 
-static void cmis_show_identifier(const __u8 *id)
+static void cmis_show_identifier(const struct cmis_memory_map *map)
 {
-	sff8024_show_identifier(id, CMIS_ID_OFFSET);
+	sff8024_show_identifier(map->lower_memory, CMIS_ID_OFFSET);
 }
 
-static void cmis_show_connector(const __u8 *id)
+static void cmis_show_connector(const struct cmis_memory_map *map)
 {
-	sff8024_show_connector(id, CMIS_CTOR_OFFSET);
+	sff8024_show_connector(map->page_00h, CMIS_CTOR_OFFSET);
 }
 
-static void cmis_show_oui(const __u8 *id)
+static void cmis_show_oui(const struct cmis_memory_map *map)
 {
-	sff8024_show_oui(id, CMIS_VENDOR_OUI_OFFSET);
+	sff8024_show_oui(map->page_00h, CMIS_VENDOR_OUI_OFFSET);
 }
 
 /**
@@ -42,9 +42,9 @@ static void cmis_show_oui(const __u8 *id)
  * [1] CMIS Rev. 3, pag. 45, section 1.7.2.1, Table 18
  * [2] CMIS Rev. 4, pag. 81, section 8.2.1, Table 8-2
  */
-static void cmis_show_rev_compliance(const __u8 *id)
+static void cmis_show_rev_compliance(const struct cmis_memory_map *map)
 {
-	__u8 rev = id[CMIS_REV_COMPLIANCE_OFFSET];
+	__u8 rev = map->lower_memory[CMIS_REV_COMPLIANCE_OFFSET];
 	int major = (rev >> 4) & 0x0F;
 	int minor = rev & 0x0F;
 
@@ -58,17 +58,17 @@ static void cmis_show_rev_compliance(const __u8 *id)
  * [2] CMIS Rev. 4, pag. 94, section 8.3.9, Table 8-18
  * [3] QSFP-DD Hardware Rev 5.0, pag. 22, section 4.2.1
  */
-static void cmis_show_power_info(const __u8 *id)
+static void cmis_show_power_info(const struct cmis_memory_map *map)
 {
 	float max_power = 0.0f;
 	__u8 base_power = 0;
 	__u8 power_class;
 
 	/* Get the power class (first 3 most significat bytes) */
-	power_class = (id[CMIS_PWR_CLASS_OFFSET] >> 5) & 0x07;
+	power_class = (map->page_00h[CMIS_PWR_CLASS_OFFSET] >> 5) & 0x07;
 
 	/* Get the base power in multiples of 0.25W */
-	base_power = id[CMIS_PWR_MAX_POWER_OFFSET];
+	base_power = map->page_00h[CMIS_PWR_MAX_POWER_OFFSET];
 	max_power = base_power * 0.25f;
 
 	printf("\t%-41s : %d\n", "Power class", power_class + 1);
@@ -83,20 +83,20 @@ static void cmis_show_power_info(const __u8 *id)
  * [1] CMIS Rev. 3, pag. 59, section 1.7.3.10, Table 31
  * [2] CMIS Rev. 4, pag. 94, section 8.3.10, Table 8-19
  */
-static void cmis_show_cbl_asm_len(const __u8 *id)
+static void cmis_show_cbl_asm_len(const struct cmis_memory_map *map)
 {
 	static const char *fn = "Cable assembly length";
 	float mul = 1.0f;
 	float val = 0.0f;
 
 	/* Check if max length */
-	if (id[CMIS_CBL_ASM_LEN_OFFSET] == CMIS_6300M_MAX_LEN) {
+	if (map->page_00h[CMIS_CBL_ASM_LEN_OFFSET] == CMIS_6300M_MAX_LEN) {
 		printf("\t%-41s : > 6.3km\n", fn);
 		return;
 	}
 
 	/* Get the multiplier from the first two bits */
-	switch (id[CMIS_CBL_ASM_LEN_OFFSET] & CMIS_LEN_MUL_MASK) {
+	switch (map->page_00h[CMIS_CBL_ASM_LEN_OFFSET] & CMIS_LEN_MUL_MASK) {
 	case CMIS_MULTIPLIER_00:
 		mul = 0.1f;
 		break;
@@ -114,7 +114,7 @@ static void cmis_show_cbl_asm_len(const __u8 *id)
 	}
 
 	/* Get base value from first 6 bits and multiply by mul */
-	val = (id[CMIS_CBL_ASM_LEN_OFFSET] & CMIS_LEN_VAL_MASK);
+	val = (map->page_00h[CMIS_CBL_ASM_LEN_OFFSET] & CMIS_LEN_VAL_MASK);
 	val = (float)val * mul;
 	printf("\t%-41s : %0.2fm\n", fn, val);
 }
@@ -126,14 +126,17 @@ static void cmis_show_cbl_asm_len(const __u8 *id)
  * [1] CMIS Rev. 3, pag. 63, section 1.7.4.2, Table 39
  * [2] CMIS Rev. 4, pag. 99, section 8.4.2, Table 8-27
  */
-static void cmis_print_smf_cbl_len(const __u8 *id)
+static void cmis_print_smf_cbl_len(const struct cmis_memory_map *map)
 {
 	static const char *fn = "Length (SMF)";
 	float mul = 1.0f;
 	float val = 0.0f;
 
+	if (!map->page_01h)
+		return;
+
 	/* Get the multiplier from the first two bits */
-	switch (id[CMIS_SMF_LEN_OFFSET] & CMIS_LEN_MUL_MASK) {
+	switch (map->page_01h[CMIS_SMF_LEN_OFFSET] & CMIS_LEN_MUL_MASK) {
 	case CMIS_MULTIPLIER_00:
 		mul = 0.1f;
 		break;
@@ -145,7 +148,7 @@ static void cmis_print_smf_cbl_len(const __u8 *id)
 	}
 
 	/* Get base value from first 6 bits and multiply by mul */
-	val = (id[CMIS_SMF_LEN_OFFSET] & CMIS_LEN_VAL_MASK);
+	val = (map->page_01h[CMIS_SMF_LEN_OFFSET] & CMIS_LEN_VAL_MASK);
 	val = (float)val * mul;
 	printf("\t%-41s : %0.2fkm\n", fn, val);
 }
@@ -155,21 +158,24 @@ static void cmis_print_smf_cbl_len(const __u8 *id)
  * [1] CMIS Rev. 3, pag. 71, section 1.7.4.10, Table 46
  * [2] CMIS Rev. 4, pag. 105, section 8.4.10, Table 8-34
  */
-static void cmis_show_sig_integrity(const __u8 *id)
+static void cmis_show_sig_integrity(const struct cmis_memory_map *map)
 {
+	if (!map->page_01h)
+		return;
+
 	/* CDR Bypass control: 2nd bit from each byte */
 	printf("\t%-41s : ", "Tx CDR bypass control");
-	printf("%s\n", YESNO(id[CMIS_SIG_INTEG_TX_OFFSET] & 0x02));
+	printf("%s\n", YESNO(map->page_01h[CMIS_SIG_INTEG_TX_OFFSET] & 0x02));
 
 	printf("\t%-41s : ", "Rx CDR bypass control");
-	printf("%s\n", YESNO(id[CMIS_SIG_INTEG_RX_OFFSET] & 0x02));
+	printf("%s\n", YESNO(map->page_01h[CMIS_SIG_INTEG_RX_OFFSET] & 0x02));
 
 	/* CDR Implementation: 1st bit from each byte */
 	printf("\t%-41s : ", "Tx CDR");
-	printf("%s\n", YESNO(id[CMIS_SIG_INTEG_TX_OFFSET] & 0x01));
+	printf("%s\n", YESNO(map->page_01h[CMIS_SIG_INTEG_TX_OFFSET] & 0x01));
 
 	printf("\t%-41s : ", "Rx CDR");
-	printf("%s\n", YESNO(id[CMIS_SIG_INTEG_RX_OFFSET] & 0x01));
+	printf("%s\n", YESNO(map->page_01h[CMIS_SIG_INTEG_RX_OFFSET] & 0x01));
 }
 
 /**
@@ -182,14 +188,14 @@ static void cmis_show_sig_integrity(const __u8 *id)
  * --> pag. 98, section 8.4, Table 8-25
  * --> page 100, section 8.4.3, 8.4.4
  */
-static void cmis_show_mit_compliance(const __u8 *id)
+static void cmis_show_mit_compliance(const struct cmis_memory_map *map)
 {
 	static const char *cc = " (Copper cable,";
 
 	printf("\t%-41s : 0x%02x", "Transmitter technology",
-	       id[CMIS_MEDIA_INTF_TECH_OFFSET]);
+	       map->page_00h[CMIS_MEDIA_INTF_TECH_OFFSET]);
 
-	switch (id[CMIS_MEDIA_INTF_TECH_OFFSET]) {
+	switch (map->page_00h[CMIS_MEDIA_INTF_TECH_OFFSET]) {
 	case CMIS_850_VCSEL:
 		printf(" (850 nm VCSEL)\n");
 		break;
@@ -240,22 +246,22 @@ static void cmis_show_mit_compliance(const __u8 *id)
 		break;
 	}
 
-	if (id[CMIS_MEDIA_INTF_TECH_OFFSET] >= CMIS_COPPER_UNEQUAL) {
+	if (map->page_00h[CMIS_MEDIA_INTF_TECH_OFFSET] >= CMIS_COPPER_UNEQUAL) {
 		printf("\t%-41s : %udb\n", "Attenuation at 5GHz",
-		       id[CMIS_COPPER_ATT_5GHZ]);
+		       map->page_00h[CMIS_COPPER_ATT_5GHZ]);
 		printf("\t%-41s : %udb\n", "Attenuation at 7GHz",
-		       id[CMIS_COPPER_ATT_7GHZ]);
+		       map->page_00h[CMIS_COPPER_ATT_7GHZ]);
 		printf("\t%-41s : %udb\n", "Attenuation at 12.9GHz",
-		       id[CMIS_COPPER_ATT_12P9GHZ]);
+		       map->page_00h[CMIS_COPPER_ATT_12P9GHZ]);
 		printf("\t%-41s : %udb\n", "Attenuation at 25.8GHz",
-		       id[CMIS_COPPER_ATT_25P8GHZ]);
-	} else {
+		       map->page_00h[CMIS_COPPER_ATT_25P8GHZ]);
+	} else if (map->page_01h) {
 		printf("\t%-41s : %.3lfnm\n", "Laser wavelength",
-		       (((id[CMIS_NOM_WAVELENGTH_MSB] << 8) |
-				id[CMIS_NOM_WAVELENGTH_LSB]) * 0.05));
+		       (((map->page_01h[CMIS_NOM_WAVELENGTH_MSB] << 8) |
+			  map->page_01h[CMIS_NOM_WAVELENGTH_LSB]) * 0.05));
 		printf("\t%-41s : %.3lfnm\n", "Laser wavelength tolerance",
-		       (((id[CMIS_WAVELENGTH_TOL_MSB] << 8) |
-		       id[CMIS_WAVELENGTH_TOL_LSB]) * 0.005));
+		       (((map->page_01h[CMIS_WAVELENGTH_TOL_MSB] << 8) |
+			  map->page_01h[CMIS_WAVELENGTH_TOL_LSB]) * 0.005));
 	}
 }
 
@@ -275,28 +281,16 @@ static void cmis_show_mit_compliance(const __u8 *id)
  * [2] CMIS Rev. 4:
  * --> pag. 84, section 8.2.4, Table 8-6
  */
-static void cmis_show_mod_lvl_monitors(const __u8 *id)
+static void cmis_show_mod_lvl_monitors(const struct cmis_memory_map *map)
 {
+	const __u8 *id = map->lower_memory;
+
 	PRINT_TEMP("Module temperature",
 		   OFFSET_TO_TEMP(CMIS_CURR_TEMP_OFFSET));
 	PRINT_VCC("Module voltage",
 		  OFFSET_TO_U16(CMIS_CURR_VCC_OFFSET));
 }
 
-static void cmis_show_link_len_from_page(const __u8 *page_one_data)
-{
-	cmis_print_smf_cbl_len(page_one_data);
-	sff_show_value_with_unit(page_one_data, CMIS_OM5_LEN_OFFSET,
-				 "Length (OM5)", 2, "m");
-	sff_show_value_with_unit(page_one_data, CMIS_OM4_LEN_OFFSET,
-				 "Length (OM4)", 2, "m");
-	sff_show_value_with_unit(page_one_data, CMIS_OM3_LEN_OFFSET,
-				 "Length (OM3 50/125um)", 2, "m");
-	sff_show_value_with_unit(page_one_data, CMIS_OM2_LEN_OFFSET,
-				 "Length (OM2 50/125um)", 1, "m");
-}
-
-
 /**
  * Print relevant info about the maximum supported fiber media length
  * for each type of fiber media at the maximum module-supported bit rate.
@@ -304,9 +298,19 @@ static void cmis_show_link_len_from_page(const __u8 *page_one_data)
  * [1] CMIS Rev. 3, page 64, section 1.7.4.2, Table 39
  * [2] CMIS Rev. 4, page 99, section 8.4.2, Table 8-27
  */
-static void cmis_show_link_len(const __u8 *id)
+static void cmis_show_link_len(const struct cmis_memory_map *map)
 {
-	cmis_show_link_len_from_page(id);
+	cmis_print_smf_cbl_len(map);
+	if (!map->page_01h)
+		return;
+	sff_show_value_with_unit(map->page_01h, CMIS_OM5_LEN_OFFSET,
+				 "Length (OM5)", 2, "m");
+	sff_show_value_with_unit(map->page_01h, CMIS_OM4_LEN_OFFSET,
+				 "Length (OM4)", 2, "m");
+	sff_show_value_with_unit(map->page_01h, CMIS_OM3_LEN_OFFSET,
+				 "Length (OM3 50/125um)", 2, "m");
+	sff_show_value_with_unit(map->page_01h, CMIS_OM2_LEN_OFFSET,
+				 "Length (OM2 50/125um)", 1, "m");
 }
 
 /**
@@ -314,25 +318,26 @@ static void cmis_show_link_len(const __u8 *id)
  * [1] CMIS Rev. 3, page 56, section 1.7.3, Table 27
  * [2] CMIS Rev. 4, page 91, section 8.2, Table 8-15
  */
-static void cmis_show_vendor_info(const __u8 *id)
+static void cmis_show_vendor_info(const struct cmis_memory_map *map)
 {
-	const char *clei = (const char *)(id + CMIS_CLEI_START_OFFSET);
+	const char *clei;
 
-	sff_show_ascii(id, CMIS_VENDOR_NAME_START_OFFSET,
+	sff_show_ascii(map->page_00h, CMIS_VENDOR_NAME_START_OFFSET,
 		       CMIS_VENDOR_NAME_END_OFFSET, "Vendor name");
-	cmis_show_oui(id);
-	sff_show_ascii(id, CMIS_VENDOR_PN_START_OFFSET,
+	cmis_show_oui(map);
+	sff_show_ascii(map->page_00h, CMIS_VENDOR_PN_START_OFFSET,
 		       CMIS_VENDOR_PN_END_OFFSET, "Vendor PN");
-	sff_show_ascii(id, CMIS_VENDOR_REV_START_OFFSET,
+	sff_show_ascii(map->page_00h, CMIS_VENDOR_REV_START_OFFSET,
 		       CMIS_VENDOR_REV_END_OFFSET, "Vendor rev");
-	sff_show_ascii(id, CMIS_VENDOR_SN_START_OFFSET,
+	sff_show_ascii(map->page_00h, CMIS_VENDOR_SN_START_OFFSET,
 		       CMIS_VENDOR_SN_END_OFFSET, "Vendor SN");
-	sff_show_ascii(id, CMIS_DATE_YEAR_OFFSET,
+	sff_show_ascii(map->page_00h, CMIS_DATE_YEAR_OFFSET,
 		       CMIS_DATE_VENDOR_LOT_OFFSET + 1, "Date code");
 
+	clei = (const char *)(map->page_00h + CMIS_CLEI_START_OFFSET);
 	if (*clei && strncmp(clei, CMIS_CLEI_BLANK, CMIS_CLEI_LEN))
-		sff_show_ascii(id, CMIS_CLEI_START_OFFSET, CMIS_CLEI_END_OFFSET,
-			       "CLEI code");
+		sff_show_ascii(map->page_00h, CMIS_CLEI_START_OFFSET,
+			       CMIS_CLEI_END_OFFSET, "CLEI code");
 }
 
 static void cmis_memory_map_init_buf(struct cmis_memory_map *map,
@@ -363,16 +368,16 @@ void cmis_show_all_ioctl(const __u8 *id)
 
 	cmis_memory_map_init_buf(&map, id);
 
-	cmis_show_identifier(id);
-	cmis_show_power_info(id);
-	cmis_show_connector(id);
-	cmis_show_cbl_asm_len(id);
-	cmis_show_sig_integrity(id);
-	cmis_show_mit_compliance(id);
-	cmis_show_mod_lvl_monitors(id);
-	cmis_show_link_len(id);
-	cmis_show_vendor_info(id);
-	cmis_show_rev_compliance(id);
+	cmis_show_identifier(&map);
+	cmis_show_power_info(&map);
+	cmis_show_connector(&map);
+	cmis_show_cbl_asm_len(&map);
+	cmis_show_sig_integrity(&map);
+	cmis_show_mit_compliance(&map);
+	cmis_show_mod_lvl_monitors(&map);
+	cmis_show_link_len(&map);
+	cmis_show_vendor_info(&map);
+	cmis_show_rev_compliance(&map);
 }
 
 static void
@@ -403,22 +408,18 @@ cmis_memory_map_init_pages(struct cmis_memory_map *map,
 void cmis_show_all_nl(const struct ethtool_module_eeprom *page_zero,
 		      const struct ethtool_module_eeprom *page_one)
 {
-	const __u8 *page_zero_data = page_zero->data;
 	struct cmis_memory_map map = {};
 
 	cmis_memory_map_init_pages(&map, page_zero, page_one);
 
-	cmis_show_identifier(page_zero_data);
-	cmis_show_power_info(page_zero_data);
-	cmis_show_connector(page_zero_data);
-	cmis_show_cbl_asm_len(page_zero_data);
-	cmis_show_sig_integrity(page_zero_data);
-	cmis_show_mit_compliance(page_zero_data);
-	cmis_show_mod_lvl_monitors(page_zero_data);
-
-	if (page_one)
-		cmis_show_link_len_from_page(page_one->data - 0x80);
-
-	cmis_show_vendor_info(page_zero_data);
-	cmis_show_rev_compliance(page_zero_data);
+	cmis_show_identifier(&map);
+	cmis_show_power_info(&map);
+	cmis_show_connector(&map);
+	cmis_show_cbl_asm_len(&map);
+	cmis_show_sig_integrity(&map);
+	cmis_show_mit_compliance(&map);
+	cmis_show_mod_lvl_monitors(&map);
+	cmis_show_link_len(&map);
+	cmis_show_vendor_info(&map);
+	cmis_show_rev_compliance(&map);
 }
diff --git a/cmis.h b/cmis.h
index 53cbb5f57127..c878e3bc5afd 100644
--- a/cmis.h
+++ b/cmis.h
@@ -100,7 +100,6 @@
  * that are unique to active modules and cable assemblies.
  * GlobalOffset = 2 * 0x80 + LocalOffset
  */
-#define PAG01H_UPPER_OFFSET			(0x02 * 0x80)
 
 /* Supported Link Length (Page 1) */
 #define CMIS_SMF_LEN_OFFSET			0x84
-- 
2.31.1

[PATCH ethtool-next 04/14] cmis: Consolidate code between IOCTL and netlink paths

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Now that both the netlink and IOCTL paths use the same memory map
structure for parsing, the code can be easily consolidated.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 cmis.c | 38 ++++++++++++++++----------------------
 1 file changed, 16 insertions(+), 22 deletions(-)

diff --git a/cmis.c b/cmis.c
index 2e01446b2315..eb7791dd59df 100644
--- a/cmis.c
+++ b/cmis.c
@@ -340,6 +340,20 @@ static void cmis_show_vendor_info(const struct cmis_memory_map *map)
 			       CMIS_CLEI_END_OFFSET, "CLEI code");
 }
 
+static void cmis_show_all_common(const struct cmis_memory_map *map)
+{
+	cmis_show_identifier(map);
+	cmis_show_power_info(map);
+	cmis_show_connector(map);
+	cmis_show_cbl_asm_len(map);
+	cmis_show_sig_integrity(map);
+	cmis_show_mit_compliance(map);
+	cmis_show_mod_lvl_monitors(map);
+	cmis_show_link_len(map);
+	cmis_show_vendor_info(map);
+	cmis_show_rev_compliance(map);
+}
+
 static void cmis_memory_map_init_buf(struct cmis_memory_map *map,
 				     const __u8 *id)
 {
@@ -367,17 +381,7 @@ void cmis_show_all_ioctl(const __u8 *id)
 	struct cmis_memory_map map = {};
 
 	cmis_memory_map_init_buf(&map, id);
-
-	cmis_show_identifier(&map);
-	cmis_show_power_info(&map);
-	cmis_show_connector(&map);
-	cmis_show_cbl_asm_len(&map);
-	cmis_show_sig_integrity(&map);
-	cmis_show_mit_compliance(&map);
-	cmis_show_mod_lvl_monitors(&map);
-	cmis_show_link_len(&map);
-	cmis_show_vendor_info(&map);
-	cmis_show_rev_compliance(&map);
+	cmis_show_all_common(&map);
 }
 
 static void
@@ -411,15 +415,5 @@ void cmis_show_all_nl(const struct ethtool_module_eeprom *page_zero,
 	struct cmis_memory_map map = {};
 
 	cmis_memory_map_init_pages(&map, page_zero, page_one);
-
-	cmis_show_identifier(&map);
-	cmis_show_power_info(&map);
-	cmis_show_connector(&map);
-	cmis_show_cbl_asm_len(&map);
-	cmis_show_sig_integrity(&map);
-	cmis_show_mit_compliance(&map);
-	cmis_show_mod_lvl_monitors(&map);
-	cmis_show_link_len(&map);
-	cmis_show_vendor_info(&map);
-	cmis_show_rev_compliance(&map);
+	cmis_show_all_common(&map);
 }
-- 
2.31.1

[PATCH ethtool-next 05/14] sff-8636: Rename SFF-8636 parsing functions

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Currently, there are two SFF-8636 parsing functions. sff8636_show_all()
and sff8636_show_all_paged(). The former is called from the IOCTL path
with a buffer containing EEPROM contents and the latter is called from
the netlink path with pointer to individual EEPROM pages.

Rename them with '_ioctl' and '_nl' suffixes to make the distinction
clear.

In subsequent patches, these two functions will only differ in the way
they initialize the SFF-8636 memory map for parsing, while the parsing
code itself will be shared between the two.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 ethtool.c               | 4 ++--
 internal.h              | 6 +++---
 netlink/module-eeprom.c | 2 +-
 qsfp.c                  | 6 +++---
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index 46887c7263e1..e3347db78fc3 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4908,8 +4908,8 @@ static int do_getmodule(struct cmd_context *ctx)
 				break;
 			case ETH_MODULE_SFF_8436:
 			case ETH_MODULE_SFF_8636:
-				sff8636_show_all(eeprom->data,
-						 modinfo.eeprom_len);
+				sff8636_show_all_ioctl(eeprom->data,
+						       modinfo.eeprom_len);
 				break;
 #endif
 			default:
diff --git a/internal.h b/internal.h
index 33e619b3ac53..7ca6066d4e12 100644
--- a/internal.h
+++ b/internal.h
@@ -390,9 +390,9 @@ void sff8079_show_all(const __u8 *id);
 void sff8472_show_all(const __u8 *id);
 
 /* QSFP Optics diagnostics */
-void sff8636_show_all(const __u8 *id, __u32 eeprom_len);
-void sff8636_show_all_paged(const struct ethtool_module_eeprom *page_zero,
-			    const struct ethtool_module_eeprom *page_three);
+void sff8636_show_all_ioctl(const __u8 *id, __u32 eeprom_len);
+void sff8636_show_all_nl(const struct ethtool_module_eeprom *page_zero,
+			 const struct ethtool_module_eeprom *page_three);
 
 /* FUJITSU Extended Socket network device */
 int fjes_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
index fc4ef1a53aff..18b1abbe1252 100644
--- a/netlink/module-eeprom.c
+++ b/netlink/module-eeprom.c
@@ -328,7 +328,7 @@ static void decoder_print(void)
 	case SFF8024_ID_QSFP:
 	case SFF8024_ID_QSFP28:
 	case SFF8024_ID_QSFP_PLUS:
-		sff8636_show_all_paged(page_zero, page_three);
+		sff8636_show_all_nl(page_zero, page_three);
 		break;
 	case SFF8024_ID_QSFP_DD:
 	case SFF8024_ID_DSFP:
diff --git a/qsfp.c b/qsfp.c
index 27fdd3bd1771..dc6407d3ef6f 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -853,7 +853,7 @@ static void sff8636_show_page_zero(const __u8 *id)
 
 }
 
-void sff8636_show_all(const __u8 *id, __u32 eeprom_len)
+void sff8636_show_all_ioctl(const __u8 *id, __u32 eeprom_len)
 {
 	if (id[SFF8636_ID_OFFSET] == SFF8024_ID_QSFP_DD) {
 		cmis_show_all_ioctl(id);
@@ -871,8 +871,8 @@ void sff8636_show_all(const __u8 *id, __u32 eeprom_len)
 	}
 }
 
-void sff8636_show_all_paged(const struct ethtool_module_eeprom *page_zero,
-			    const struct ethtool_module_eeprom *page_three)
+void sff8636_show_all_nl(const struct ethtool_module_eeprom *page_zero,
+			 const struct ethtool_module_eeprom *page_three)
 {
 	sff8636_show_identifier(page_zero->data);
 	sff8636_show_page_zero(page_zero->data);
-- 
2.31.1

[PATCH ethtool-next 06/14] sff-8636: Initialize SFF-8636 memory map

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

The SFF-8636 memory map [1] consists of Lower Memory and Upper Memory.

The content of the Lower Memory is fixed and can be addressed using an
offset between 0 and 127 (inclusive).

The Upper Memory is variable and optional and can be addressed by
specifying a page number and an offset between 128 and 255 (inclusive).

Create a structure describing this memory map and initialize it with
pointers to available pages.

In the IOCTL path, the structure holds pointers to regions of the
continuous buffer passed to user space via the 'ETHTOOL_GMODULEEEPROM'
command.

In the netlink path, the structure holds pointers to individual pages
passed to user space via the 'MODULE_EEPROM_GET' message.

This structure will later allow us to consolidate the IOCTL and netlink
parsing code paths and also easily support additional EEPROM pages, when
needed.

[1] SFF-8636 Rev. 2.10a, pag. 30, section 6.1, Figure 6-1

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 qsfp.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/qsfp.c b/qsfp.c
index dc6407d3ef6f..80000d40f6e8 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -60,6 +60,15 @@
 #include "qsfp.h"
 #include "cmis.h"
 
+struct sff8636_memory_map {
+	const __u8 *lower_memory;
+	const __u8 *upper_memory[4];
+#define page_00h upper_memory[0x0]
+#define page_03h upper_memory[0x3]
+};
+
+#define SFF8636_PAGE_SIZE	0x80
+
 #define MAX_DESC_SIZE	42
 
 static struct sff8636_aw_flags {
@@ -853,13 +862,40 @@ static void sff8636_show_page_zero(const __u8 *id)
 
 }
 
+static void sff8636_memory_map_init_buf(struct sff8636_memory_map *map,
+					const __u8 *id, __u32 eeprom_len)
+{
+	/* Lower Memory and Page 00h are always present.
+	 *
+	 * Offset into Upper Memory is between page size and twice the page
+	 * size. Therefore, set the base address of each page to base address
+	 * plus page size multiplied by the page number.
+	 */
+	map->lower_memory = id;
+	map->page_00h = id;
+
+	/* Page 03h is only present when the module memory model is paged and
+	 * not flat and when we got a big enough buffer from the kernel.
+	 */
+	if (map->lower_memory[SFF8636_STATUS_2_OFFSET] &
+	    SFF8636_STATUS_PAGE_3_PRESENT ||
+	    eeprom_len != ETH_MODULE_SFF_8636_MAX_LEN)
+		return;
+
+	map->page_03h = id + 3 * SFF8636_PAGE_SIZE;
+}
+
 void sff8636_show_all_ioctl(const __u8 *id, __u32 eeprom_len)
 {
+	struct sff8636_memory_map map = {};
+
 	if (id[SFF8636_ID_OFFSET] == SFF8024_ID_QSFP_DD) {
 		cmis_show_all_ioctl(id);
 		return;
 	}
 
+	sff8636_memory_map_init_buf(&map, id, eeprom_len);
+
 	sff8636_show_identifier(id);
 	switch (id[SFF8636_ID_OFFSET]) {
 	case SFF8024_ID_QSFP:
@@ -871,9 +907,38 @@ void sff8636_show_all_ioctl(const __u8 *id, __u32 eeprom_len)
 	}
 }
 
+static void
+sff8636_memory_map_init_pages(struct sff8636_memory_map *map,
+			      const struct ethtool_module_eeprom *page_zero,
+			      const struct ethtool_module_eeprom *page_three)
+{
+	/* Lower Memory and Page 00h are always present.
+	 *
+	 * Offset into Upper Memory is between page size and twice the page
+	 * size. Therefore, set the base address of each page to its base
+	 * address minus page size. For Page 00h, this is the address of the
+	 * Lower Memory.
+	 */
+	map->lower_memory = page_zero->data;
+	map->page_00h = page_zero->data;
+
+	/* Page 03h is only present when the module memory model is paged and
+	 * not flat.
+	 */
+	if (map->lower_memory[SFF8636_STATUS_2_OFFSET] &
+	    SFF8636_STATUS_PAGE_3_PRESENT)
+		return;
+
+	map->page_03h = page_three->data - SFF8636_PAGE_SIZE;
+}
+
 void sff8636_show_all_nl(const struct ethtool_module_eeprom *page_zero,
 			 const struct ethtool_module_eeprom *page_three)
 {
+	struct sff8636_memory_map map = {};
+
+	sff8636_memory_map_init_pages(&map, page_zero, page_three);
+
 	sff8636_show_identifier(page_zero->data);
 	sff8636_show_page_zero(page_zero->data);
 	if (page_three)
-- 
2.31.1

[PATCH ethtool-next 07/14] sff-8636: Use memory map during parsing

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Instead of passing one large buffer to the individual parsing functions,
use the memory map structure from the previous patch.

This has the added benefit of checking which optional pages are actually
available and it will also allow us to consolidate the IOCTL and netlink
parsing code paths.

Tested by making sure that there are no differences in output in both
the IOCTL and netlink paths before and after the patch.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 qsfp.c | 368 +++++++++++++++++++++++++++++++--------------------------
 1 file changed, 201 insertions(+), 167 deletions(-)

diff --git a/qsfp.c b/qsfp.c
index 80000d40f6e8..354b3b1ce9ff 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -205,20 +205,21 @@ static struct sff8636_aw_flags {
 	{ NULL, 0, 0 },
 };
 
-static void sff8636_show_identifier(const __u8 *id)
+static void sff8636_show_identifier(const struct sff8636_memory_map *map)
 {
-	sff8024_show_identifier(id, SFF8636_ID_OFFSET);
+	sff8024_show_identifier(map->lower_memory, SFF8636_ID_OFFSET);
 }
 
-static void sff8636_show_ext_identifier(const __u8 *id)
+static void sff8636_show_ext_identifier(const struct sff8636_memory_map *map)
 {
 	printf("\t%-41s : 0x%02x\n", "Extended identifier",
-			id[SFF8636_EXT_ID_OFFSET]);
+	       map->page_00h[SFF8636_EXT_ID_OFFSET]);
 
 	static const char *pfx =
 		"\tExtended identifier description           :";
 
-	switch (id[SFF8636_EXT_ID_OFFSET] & SFF8636_EXT_ID_PWR_CLASS_MASK) {
+	switch (map->page_00h[SFF8636_EXT_ID_OFFSET] &
+		SFF8636_EXT_ID_PWR_CLASS_MASK) {
 	case SFF8636_EXT_ID_PWR_CLASS_1:
 		printf("%s 1.5W max. Power consumption\n", pfx);
 		break;
@@ -233,17 +234,18 @@ static void sff8636_show_ext_identifier(const __u8 *id)
 		break;
 	}
 
-	if (id[SFF8636_EXT_ID_OFFSET] & SFF8636_EXT_ID_CDR_TX_MASK)
+	if (map->page_00h[SFF8636_EXT_ID_OFFSET] & SFF8636_EXT_ID_CDR_TX_MASK)
 		printf("%s CDR present in TX,", pfx);
 	else
 		printf("%s No CDR in TX,", pfx);
 
-	if (id[SFF8636_EXT_ID_OFFSET] & SFF8636_EXT_ID_CDR_RX_MASK)
+	if (map->page_00h[SFF8636_EXT_ID_OFFSET] & SFF8636_EXT_ID_CDR_RX_MASK)
 		printf(" CDR present in RX\n");
 	else
 		printf(" No CDR in RX\n");
 
-	switch (id[SFF8636_EXT_ID_OFFSET] & SFF8636_EXT_ID_EPWR_CLASS_MASK) {
+	switch (map->page_00h[SFF8636_EXT_ID_OFFSET] &
+		SFF8636_EXT_ID_EPWR_CLASS_MASK) {
 	case SFF8636_EXT_ID_PWR_CLASS_LEGACY:
 		printf("%s", pfx);
 		break;
@@ -257,18 +259,19 @@ static void sff8636_show_ext_identifier(const __u8 *id)
 		printf("%s 5.0W max. Power consumption, ", pfx);
 		break;
 	}
-	if (id[SFF8636_PWR_MODE_OFFSET] & SFF8636_HIGH_PWR_ENABLE)
+	if (map->lower_memory[SFF8636_PWR_MODE_OFFSET] &
+	    SFF8636_HIGH_PWR_ENABLE)
 		printf(" High Power Class (> 3.5 W) enabled\n");
 	else
 		printf(" High Power Class (> 3.5 W) not enabled\n");
 }
 
-static void sff8636_show_connector(const __u8 *id)
+static void sff8636_show_connector(const struct sff8636_memory_map *map)
 {
-	sff8024_show_connector(id, SFF8636_CTOR_OFFSET);
+	sff8024_show_connector(map->page_00h, SFF8636_CTOR_OFFSET);
 }
 
-static void sff8636_show_transceiver(const __u8 *id)
+static void sff8636_show_transceiver(const struct sff8636_memory_map *map)
 {
 	static const char *pfx =
 		"\tTransceiver type                          :";
@@ -276,33 +279,41 @@ static void sff8636_show_transceiver(const __u8 *id)
 	printf("\t%-41s : 0x%02x 0x%02x 0x%02x " \
 			"0x%02x 0x%02x 0x%02x 0x%02x 0x%02x\n",
 			"Transceiver codes",
-			id[SFF8636_ETHERNET_COMP_OFFSET],
-			id[SFF8636_SONET_COMP_OFFSET],
-			id[SFF8636_SAS_COMP_OFFSET],
-			id[SFF8636_GIGE_COMP_OFFSET],
-			id[SFF8636_FC_LEN_OFFSET],
-			id[SFF8636_FC_TECH_OFFSET],
-			id[SFF8636_FC_TRANS_MEDIA_OFFSET],
-			id[SFF8636_FC_SPEED_OFFSET]);
+			map->page_00h[SFF8636_ETHERNET_COMP_OFFSET],
+			map->page_00h[SFF8636_SONET_COMP_OFFSET],
+			map->page_00h[SFF8636_SAS_COMP_OFFSET],
+			map->page_00h[SFF8636_GIGE_COMP_OFFSET],
+			map->page_00h[SFF8636_FC_LEN_OFFSET],
+			map->page_00h[SFF8636_FC_TECH_OFFSET],
+			map->page_00h[SFF8636_FC_TRANS_MEDIA_OFFSET],
+			map->page_00h[SFF8636_FC_SPEED_OFFSET]);
 
 	/* 10G/40G Ethernet Compliance Codes */
-	if (id[SFF8636_ETHERNET_COMP_OFFSET] & SFF8636_ETHERNET_10G_LRM)
+	if (map->page_00h[SFF8636_ETHERNET_COMP_OFFSET] &
+	    SFF8636_ETHERNET_10G_LRM)
 		printf("%s 10G Ethernet: 10G Base-LRM\n", pfx);
-	if (id[SFF8636_ETHERNET_COMP_OFFSET] & SFF8636_ETHERNET_10G_LR)
+	if (map->page_00h[SFF8636_ETHERNET_COMP_OFFSET] &
+	    SFF8636_ETHERNET_10G_LR)
 		printf("%s 10G Ethernet: 10G Base-LR\n", pfx);
-	if (id[SFF8636_ETHERNET_COMP_OFFSET] & SFF8636_ETHERNET_10G_SR)
+	if (map->page_00h[SFF8636_ETHERNET_COMP_OFFSET] &
+	    SFF8636_ETHERNET_10G_SR)
 		printf("%s 10G Ethernet: 10G Base-SR\n", pfx);
-	if (id[SFF8636_ETHERNET_COMP_OFFSET] & SFF8636_ETHERNET_40G_CR4)
+	if (map->page_00h[SFF8636_ETHERNET_COMP_OFFSET] &
+	    SFF8636_ETHERNET_40G_CR4)
 		printf("%s 40G Ethernet: 40G Base-CR4\n", pfx);
-	if (id[SFF8636_ETHERNET_COMP_OFFSET] & SFF8636_ETHERNET_40G_SR4)
+	if (map->page_00h[SFF8636_ETHERNET_COMP_OFFSET] &
+	    SFF8636_ETHERNET_40G_SR4)
 		printf("%s 40G Ethernet: 40G Base-SR4\n", pfx);
-	if (id[SFF8636_ETHERNET_COMP_OFFSET] & SFF8636_ETHERNET_40G_LR4)
+	if (map->page_00h[SFF8636_ETHERNET_COMP_OFFSET] &
+	    SFF8636_ETHERNET_40G_LR4)
 		printf("%s 40G Ethernet: 40G Base-LR4\n", pfx);
-	if (id[SFF8636_ETHERNET_COMP_OFFSET] & SFF8636_ETHERNET_40G_ACTIVE)
+	if (map->page_00h[SFF8636_ETHERNET_COMP_OFFSET] &
+	    SFF8636_ETHERNET_40G_ACTIVE)
 		printf("%s 40G Ethernet: 40G Active Cable (XLPPI)\n", pfx);
 	/* Extended Specification Compliance Codes from SFF-8024 */
-	if (id[SFF8636_ETHERNET_COMP_OFFSET] & SFF8636_ETHERNET_RSRVD) {
-		switch (id[SFF8636_OPTION_1_OFFSET]) {
+	if (map->page_00h[SFF8636_ETHERNET_COMP_OFFSET] &
+	    SFF8636_ETHERNET_RSRVD) {
+		switch (map->page_00h[SFF8636_OPTION_1_OFFSET]) {
 		case SFF8636_ETHERNET_UNSPECIFIED:
 			printf("%s (reserved or unknown)\n", pfx);
 			break;
@@ -493,113 +504,122 @@ static void sff8636_show_transceiver(const __u8 *id)
 	}
 
 	/* SONET Compliance Codes */
-	if (id[SFF8636_SONET_COMP_OFFSET] & (SFF8636_SONET_40G_OTN))
+	if (map->page_00h[SFF8636_SONET_COMP_OFFSET] &
+	    (SFF8636_SONET_40G_OTN))
 		printf("%s 40G OTN (OTU3B/OTU3C)\n", pfx);
-	if (id[SFF8636_SONET_COMP_OFFSET] & (SFF8636_SONET_OC48_LR))
+	if (map->page_00h[SFF8636_SONET_COMP_OFFSET] & (SFF8636_SONET_OC48_LR))
 		printf("%s SONET: OC-48, long reach\n", pfx);
-	if (id[SFF8636_SONET_COMP_OFFSET] & (SFF8636_SONET_OC48_IR))
+	if (map->page_00h[SFF8636_SONET_COMP_OFFSET] & (SFF8636_SONET_OC48_IR))
 		printf("%s SONET: OC-48, intermediate reach\n", pfx);
-	if (id[SFF8636_SONET_COMP_OFFSET] & (SFF8636_SONET_OC48_SR))
+	if (map->page_00h[SFF8636_SONET_COMP_OFFSET] & (SFF8636_SONET_OC48_SR))
 		printf("%s SONET: OC-48, short reach\n", pfx);
 
 	/* SAS/SATA Compliance Codes */
-	if (id[SFF8636_SAS_COMP_OFFSET] & (SFF8636_SAS_6G))
+	if (map->page_00h[SFF8636_SAS_COMP_OFFSET] & (SFF8636_SAS_6G))
 		printf("%s SAS 6.0G\n", pfx);
-	if (id[SFF8636_SAS_COMP_OFFSET] & (SFF8636_SAS_3G))
+	if (map->page_00h[SFF8636_SAS_COMP_OFFSET] & (SFF8636_SAS_3G))
 		printf("%s SAS 3.0G\n", pfx);
 
 	/* Ethernet Compliance Codes */
-	if (id[SFF8636_GIGE_COMP_OFFSET] & SFF8636_GIGE_1000_BASE_T)
+	if (map->page_00h[SFF8636_GIGE_COMP_OFFSET] & SFF8636_GIGE_1000_BASE_T)
 		printf("%s Ethernet: 1000BASE-T\n", pfx);
-	if (id[SFF8636_GIGE_COMP_OFFSET] & SFF8636_GIGE_1000_BASE_CX)
+	if (map->page_00h[SFF8636_GIGE_COMP_OFFSET] & SFF8636_GIGE_1000_BASE_CX)
 		printf("%s Ethernet: 1000BASE-CX\n", pfx);
-	if (id[SFF8636_GIGE_COMP_OFFSET] & SFF8636_GIGE_1000_BASE_LX)
+	if (map->page_00h[SFF8636_GIGE_COMP_OFFSET] & SFF8636_GIGE_1000_BASE_LX)
 		printf("%s Ethernet: 1000BASE-LX\n", pfx);
-	if (id[SFF8636_GIGE_COMP_OFFSET] & SFF8636_GIGE_1000_BASE_SX)
+	if (map->page_00h[SFF8636_GIGE_COMP_OFFSET] & SFF8636_GIGE_1000_BASE_SX)
 		printf("%s Ethernet: 1000BASE-SX\n", pfx);
 
 	/* Fibre Channel link length */
-	if (id[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_LEN_VERY_LONG)
+	if (map->page_00h[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_LEN_VERY_LONG)
 		printf("%s FC: very long distance (V)\n", pfx);
-	if (id[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_LEN_SHORT)
+	if (map->page_00h[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_LEN_SHORT)
 		printf("%s FC: short distance (S)\n", pfx);
-	if (id[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_LEN_INT)
+	if (map->page_00h[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_LEN_INT)
 		printf("%s FC: intermediate distance (I)\n", pfx);
-	if (id[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_LEN_LONG)
+	if (map->page_00h[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_LEN_LONG)
 		printf("%s FC: long distance (L)\n", pfx);
-	if (id[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_LEN_MED)
+	if (map->page_00h[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_LEN_MED)
 		printf("%s FC: medium distance (M)\n", pfx);
 
 	/* Fibre Channel transmitter technology */
-	if (id[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_TECH_LONG_LC)
+	if (map->page_00h[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_TECH_LONG_LC)
 		printf("%s FC: Longwave laser (LC)\n", pfx);
-	if (id[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_TECH_ELEC_INTER)
+	if (map->page_00h[SFF8636_FC_LEN_OFFSET] & SFF8636_FC_TECH_ELEC_INTER)
 		printf("%s FC: Electrical inter-enclosure (EL)\n", pfx);
-	if (id[SFF8636_FC_TECH_OFFSET] & SFF8636_FC_TECH_ELEC_INTRA)
+	if (map->page_00h[SFF8636_FC_TECH_OFFSET] & SFF8636_FC_TECH_ELEC_INTRA)
 		printf("%s FC: Electrical intra-enclosure (EL)\n", pfx);
-	if (id[SFF8636_FC_TECH_OFFSET] & SFF8636_FC_TECH_SHORT_WO_OFC)
+	if (map->page_00h[SFF8636_FC_TECH_OFFSET] &
+	    SFF8636_FC_TECH_SHORT_WO_OFC)
 		printf("%s FC: Shortwave laser w/o OFC (SN)\n", pfx);
-	if (id[SFF8636_FC_TECH_OFFSET] & SFF8636_FC_TECH_SHORT_W_OFC)
+	if (map->page_00h[SFF8636_FC_TECH_OFFSET] & SFF8636_FC_TECH_SHORT_W_OFC)
 		printf("%s FC: Shortwave laser with OFC (SL)\n", pfx);
-	if (id[SFF8636_FC_TECH_OFFSET] & SFF8636_FC_TECH_LONG_LL)
+	if (map->page_00h[SFF8636_FC_TECH_OFFSET] & SFF8636_FC_TECH_LONG_LL)
 		printf("%s FC: Longwave laser (LL)\n", pfx);
 
 	/* Fibre Channel transmission media */
-	if (id[SFF8636_FC_TRANS_MEDIA_OFFSET] & SFF8636_FC_TRANS_MEDIA_TW)
+	if (map->page_00h[SFF8636_FC_TRANS_MEDIA_OFFSET] &
+	    SFF8636_FC_TRANS_MEDIA_TW)
 		printf("%s FC: Twin Axial Pair (TW)\n", pfx);
-	if (id[SFF8636_FC_TRANS_MEDIA_OFFSET] & SFF8636_FC_TRANS_MEDIA_TP)
+	if (map->page_00h[SFF8636_FC_TRANS_MEDIA_OFFSET] &
+	    SFF8636_FC_TRANS_MEDIA_TP)
 		printf("%s FC: Twisted Pair (TP)\n", pfx);
-	if (id[SFF8636_FC_TRANS_MEDIA_OFFSET] & SFF8636_FC_TRANS_MEDIA_MI)
+	if (map->page_00h[SFF8636_FC_TRANS_MEDIA_OFFSET] &
+	    SFF8636_FC_TRANS_MEDIA_MI)
 		printf("%s FC: Miniature Coax (MI)\n", pfx);
-	if (id[SFF8636_FC_TRANS_MEDIA_OFFSET] & SFF8636_FC_TRANS_MEDIA_TV)
+	if (map->page_00h[SFF8636_FC_TRANS_MEDIA_OFFSET] &
+	    SFF8636_FC_TRANS_MEDIA_TV)
 		printf("%s FC: Video Coax (TV)\n", pfx);
-	if (id[SFF8636_FC_TRANS_MEDIA_OFFSET] & SFF8636_FC_TRANS_MEDIA_M6)
+	if (map->page_00h[SFF8636_FC_TRANS_MEDIA_OFFSET] &
+	    SFF8636_FC_TRANS_MEDIA_M6)
 		printf("%s FC: Multimode, 62.5m (M6)\n", pfx);
-	if (id[SFF8636_FC_TRANS_MEDIA_OFFSET] & SFF8636_FC_TRANS_MEDIA_M5)
+	if (map->page_00h[SFF8636_FC_TRANS_MEDIA_OFFSET] &
+	    SFF8636_FC_TRANS_MEDIA_M5)
 		printf("%s FC: Multimode, 50m (M5)\n", pfx);
-	if (id[SFF8636_FC_TRANS_MEDIA_OFFSET] & SFF8636_FC_TRANS_MEDIA_OM3)
+	if (map->page_00h[SFF8636_FC_TRANS_MEDIA_OFFSET] &
+	    SFF8636_FC_TRANS_MEDIA_OM3)
 		printf("%s FC: Multimode, 50um (OM3)\n", pfx);
-	if (id[SFF8636_FC_TRANS_MEDIA_OFFSET] & SFF8636_FC_TRANS_MEDIA_SM)
+	if (map->page_00h[SFF8636_FC_TRANS_MEDIA_OFFSET] &
+	    SFF8636_FC_TRANS_MEDIA_SM)
 		printf("%s FC: Single Mode (SM)\n", pfx);
 
 	/* Fibre Channel speed */
-	if (id[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_1200_MBPS)
+	if (map->page_00h[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_1200_MBPS)
 		printf("%s FC: 1200 MBytes/sec\n", pfx);
-	if (id[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_800_MBPS)
+	if (map->page_00h[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_800_MBPS)
 		printf("%s FC: 800 MBytes/sec\n", pfx);
-	if (id[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_1600_MBPS)
+	if (map->page_00h[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_1600_MBPS)
 		printf("%s FC: 1600 MBytes/sec\n", pfx);
-	if (id[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_400_MBPS)
+	if (map->page_00h[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_400_MBPS)
 		printf("%s FC: 400 MBytes/sec\n", pfx);
-	if (id[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_200_MBPS)
+	if (map->page_00h[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_200_MBPS)
 		printf("%s FC: 200 MBytes/sec\n", pfx);
-	if (id[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_100_MBPS)
+	if (map->page_00h[SFF8636_FC_SPEED_OFFSET] & SFF8636_FC_SPEED_100_MBPS)
 		printf("%s FC: 100 MBytes/sec\n", pfx);
 }
 
-static void sff8636_show_encoding(const __u8 *id)
+static void sff8636_show_encoding(const struct sff8636_memory_map *map)
 {
-	sff8024_show_encoding(id, SFF8636_ENCODING_OFFSET, ETH_MODULE_SFF_8636);
+	sff8024_show_encoding(map->page_00h, SFF8636_ENCODING_OFFSET,
+			      ETH_MODULE_SFF_8636);
 }
 
-static void sff8636_show_rate_identifier(const __u8 *id)
+static void sff8636_show_rate_identifier(const struct sff8636_memory_map *map)
 {
 	/* TODO: Need to fix rate select logic */
 	printf("\t%-41s : 0x%02x\n", "Rate identifier",
-			id[SFF8636_EXT_RS_OFFSET]);
+	       map->page_00h[SFF8636_EXT_RS_OFFSET]);
 }
 
-static void sff8636_show_oui(const __u8 *id, int id_offset)
-{
-	sff8024_show_oui(id, id_offset);
-}
-
-static void sff8636_show_wavelength_or_copper_compliance(const __u8 *id)
+static void
+sff8636_show_wavelength_or_copper_compliance(const struct sff8636_memory_map *map)
 {
 	printf("\t%-41s : 0x%02x", "Transmitter technology",
-		(id[SFF8636_DEVICE_TECH_OFFSET] & SFF8636_TRANS_TECH_MASK));
+	       map->page_00h[SFF8636_DEVICE_TECH_OFFSET] &
+	       SFF8636_TRANS_TECH_MASK);
 
-	switch (id[SFF8636_DEVICE_TECH_OFFSET] & SFF8636_TRANS_TECH_MASK) {
+	switch (map->page_00h[SFF8636_DEVICE_TECH_OFFSET] &
+		SFF8636_TRANS_TECH_MASK) {
 	case SFF8636_TRANS_850_VCSEL:
 		printf(" (850 nm VCSEL)\n");
 		break;
@@ -650,31 +670,26 @@ static void sff8636_show_wavelength_or_copper_compliance(const __u8 *id)
 		break;
 	}
 
-	if ((id[SFF8636_DEVICE_TECH_OFFSET] & SFF8636_TRANS_TECH_MASK)
-			>= SFF8636_TRANS_COPPER_PAS_UNEQUAL) {
+	if ((map->page_00h[SFF8636_DEVICE_TECH_OFFSET] &
+	     SFF8636_TRANS_TECH_MASK) >= SFF8636_TRANS_COPPER_PAS_UNEQUAL) {
 		printf("\t%-41s : %udb\n", "Attenuation at 2.5GHz",
-			id[SFF8636_WAVELEN_HIGH_BYTE_OFFSET]);
+			map->page_00h[SFF8636_WAVELEN_HIGH_BYTE_OFFSET]);
 		printf("\t%-41s : %udb\n", "Attenuation at 5.0GHz",
-			id[SFF8636_WAVELEN_LOW_BYTE_OFFSET]);
+			map->page_00h[SFF8636_WAVELEN_LOW_BYTE_OFFSET]);
 		printf("\t%-41s : %udb\n", "Attenuation at 7.0GHz",
-			id[SFF8636_WAVE_TOL_HIGH_BYTE_OFFSET]);
+			map->page_00h[SFF8636_WAVE_TOL_HIGH_BYTE_OFFSET]);
 		printf("\t%-41s : %udb\n", "Attenuation at 12.9GHz",
-			id[SFF8636_WAVE_TOL_LOW_BYTE_OFFSET]);
+		       map->page_00h[SFF8636_WAVE_TOL_LOW_BYTE_OFFSET]);
 	} else {
 		printf("\t%-41s : %.3lfnm\n", "Laser wavelength",
-			(((id[SFF8636_WAVELEN_HIGH_BYTE_OFFSET] << 8) |
-				id[SFF8636_WAVELEN_LOW_BYTE_OFFSET])*0.05));
+		       (((map->page_00h[SFF8636_WAVELEN_HIGH_BYTE_OFFSET] << 8) |
+			 map->page_00h[SFF8636_WAVELEN_LOW_BYTE_OFFSET]) * 0.05));
 		printf("\t%-41s : %.3lfnm\n", "Laser wavelength tolerance",
-			(((id[SFF8636_WAVE_TOL_HIGH_BYTE_OFFSET] << 8) |
-			id[SFF8636_WAVE_TOL_LOW_BYTE_OFFSET])*0.005));
+		       (((map->page_00h[SFF8636_WAVE_TOL_HIGH_BYTE_OFFSET] << 8) |
+			 map->page_00h[SFF8636_WAVE_TOL_LOW_BYTE_OFFSET]) * 0.005));
 	}
 }
 
-static void sff8636_show_revision_compliance(const __u8 *id)
-{
-	sff_show_revision_compliance(id, SFF8636_REV_COMPLIANCE_OFFSET);
-}
-
 /*
  * 2-byte internal temperature conversions:
  * First byte is a signed 8-bit integer, which is the temp decimal part
@@ -683,39 +698,65 @@ static void sff8636_show_revision_compliance(const __u8 *id)
 #define SFF8636_OFFSET_TO_TEMP(offset) ((__s16)OFFSET_TO_U16(offset))
 #define OFFSET_TO_U16_PTR(ptr, offset) (ptr[offset] << 8 | ptr[(offset) + 1])
 
-static void sff8636_dom_parse(const __u8 *id, const __u8 *page_three, struct sff_diags *sd)
+static void sff8636_dom_parse(const struct sff8636_memory_map *map,
+			      struct sff_diags *sd)
 {
+	const __u8 *id = map->lower_memory;
 	int i = 0;
 
 	/* Monitoring Thresholds for Alarms and Warnings */
 	sd->sfp_voltage[MCURR] = OFFSET_TO_U16_PTR(id, SFF8636_VCC_CURR);
-	sd->sfp_voltage[HALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_VCC_HALRM);
-	sd->sfp_voltage[LALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_VCC_LALRM);
-	sd->sfp_voltage[HWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_VCC_HWARN);
-	sd->sfp_voltage[LWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_VCC_LWARN);
-
 	sd->sfp_temp[MCURR] = SFF8636_OFFSET_TO_TEMP(SFF8636_TEMP_CURR);
-	sd->sfp_temp[HALRM] = (__s16)OFFSET_TO_U16_PTR(page_three, SFF8636_TEMP_HALRM);
-	sd->sfp_temp[LALRM] = (__s16)OFFSET_TO_U16_PTR(page_three, SFF8636_TEMP_LALRM);
-	sd->sfp_temp[HWARN] = (__s16)OFFSET_TO_U16_PTR(page_three, SFF8636_TEMP_HWARN);
-	sd->sfp_temp[LWARN] = (__s16)OFFSET_TO_U16_PTR(page_three, SFF8636_TEMP_LWARN);
-
-	sd->bias_cur[HALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_BIAS_HALRM);
-	sd->bias_cur[LALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_BIAS_LALRM);
-	sd->bias_cur[HWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_BIAS_HWARN);
-	sd->bias_cur[LWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_BIAS_LWARN);
-
-	sd->tx_power[HALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_PWR_HALRM);
-	sd->tx_power[LALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_PWR_LALRM);
-	sd->tx_power[HWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_PWR_HWARN);
-	sd->tx_power[LWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_TX_PWR_LWARN);
-
-	sd->rx_power[HALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_RX_PWR_HALRM);
-	sd->rx_power[LALRM] = OFFSET_TO_U16_PTR(page_three, SFF8636_RX_PWR_LALRM);
-	sd->rx_power[HWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_RX_PWR_HWARN);
-	sd->rx_power[LWARN] = OFFSET_TO_U16_PTR(page_three, SFF8636_RX_PWR_LWARN);
-
 
+	if (!map->page_03h)
+		goto out;
+
+	sd->sfp_voltage[HALRM] = OFFSET_TO_U16_PTR(map->page_03h,
+						   SFF8636_VCC_HALRM);
+	sd->sfp_voltage[LALRM] = OFFSET_TO_U16_PTR(map->page_03h,
+						   SFF8636_VCC_LALRM);
+	sd->sfp_voltage[HWARN] = OFFSET_TO_U16_PTR(map->page_03h,
+						   SFF8636_VCC_HWARN);
+	sd->sfp_voltage[LWARN] = OFFSET_TO_U16_PTR(map->page_03h,
+						   SFF8636_VCC_LWARN);
+
+	sd->sfp_temp[HALRM] = (__s16)OFFSET_TO_U16_PTR(map->page_03h,
+						       SFF8636_TEMP_HALRM);
+	sd->sfp_temp[LALRM] = (__s16)OFFSET_TO_U16_PTR(map->page_03h,
+						       SFF8636_TEMP_LALRM);
+	sd->sfp_temp[HWARN] = (__s16)OFFSET_TO_U16_PTR(map->page_03h,
+						       SFF8636_TEMP_HWARN);
+	sd->sfp_temp[LWARN] = (__s16)OFFSET_TO_U16_PTR(map->page_03h,
+						       SFF8636_TEMP_LWARN);
+
+	sd->bias_cur[HALRM] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_TX_BIAS_HALRM);
+	sd->bias_cur[LALRM] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_TX_BIAS_LALRM);
+	sd->bias_cur[HWARN] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_TX_BIAS_HWARN);
+	sd->bias_cur[LWARN] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_TX_BIAS_LWARN);
+
+	sd->tx_power[HALRM] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_TX_PWR_HALRM);
+	sd->tx_power[LALRM] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_TX_PWR_LALRM);
+	sd->tx_power[HWARN] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_TX_PWR_HWARN);
+	sd->tx_power[LWARN] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_TX_PWR_LWARN);
+
+	sd->rx_power[HALRM] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_RX_PWR_HALRM);
+	sd->rx_power[LALRM] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_RX_PWR_LALRM);
+	sd->rx_power[HWARN] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_RX_PWR_HWARN);
+	sd->rx_power[LWARN] = OFFSET_TO_U16_PTR(map->page_03h,
+						SFF8636_RX_PWR_LWARN);
+
+out:
 	/* Channel Specific Data */
 	for (i = 0; i < MAX_CHANNEL_NUM; i++) {
 		u8 rx_power_offset, tx_bias_offset;
@@ -749,7 +790,7 @@ static void sff8636_dom_parse(const __u8 *id, const __u8 *page_three, struct sff
 	}
 }
 
-static void sff8636_show_dom(const __u8 *id, const __u8 *page_three, __u32 eeprom_len)
+static void sff8636_show_dom(const struct sff8636_memory_map *map)
 {
 	struct sff_diags sd = {0};
 	char *rx_power_string = NULL;
@@ -763,20 +804,15 @@ static void sff8636_show_dom(const __u8 *id, const __u8 *page_three, __u32 eepro
 	 * and thresholds
 	 * If pagging support exists, then supports_alarms is marked as 1
 	 */
+	if (map->page_03h)
+		sd.supports_alarms = 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;
-		}
-	}
+	sd.rx_power_type = map->page_00h[SFF8636_DIAG_TYPE_OFFSET] &
+			   SFF8636_RX_PWR_TYPE_MASK;
+	sd.tx_power_type = map->page_00h[SFF8636_DIAG_TYPE_OFFSET] &
+			   SFF8636_RX_PWR_TYPE_MASK;
 
-	sd.rx_power_type = id[SFF8636_DIAG_TYPE_OFFSET] &
-						SFF8636_RX_PWR_TYPE_MASK;
-	sd.tx_power_type = id[SFF8636_DIAG_TYPE_OFFSET] &
-						SFF8636_RX_PWR_TYPE_MASK;
-
-	sff8636_dom_parse(id, page_three, &sd);
+	sff8636_dom_parse(map, &sd);
 
 	PRINT_TEMP("Module temperature", sd.sfp_temp[MCURR]);
 	PRINT_VCC("Module voltage", sd.sfp_voltage[MCURR]);
@@ -819,7 +855,7 @@ static void sff8636_show_dom(const __u8 *id, const __u8 *page_three, __u32 eepro
 	if (sd.supports_alarms) {
 		for (i = 0; sff8636_aw_flags[i].str; ++i) {
 			printf("\t%-41s : %s\n", sff8636_aw_flags[i].str,
-			       id[sff8636_aw_flags[i].offset]
+			       map->lower_memory[sff8636_aw_flags[i].offset]
 			       & sff8636_aw_flags[i].value ? "On" : "Off");
 		}
 
@@ -827,39 +863,39 @@ static void sff8636_show_dom(const __u8 *id, const __u8 *page_three, __u32 eepro
 	}
 }
 
-static void sff8636_show_page_zero(const __u8 *id)
+static void sff8636_show_page_zero(const struct sff8636_memory_map *map)
 {
-	sff8636_show_ext_identifier(id);
-	sff8636_show_connector(id);
-	sff8636_show_transceiver(id);
-	sff8636_show_encoding(id);
-	sff_show_value_with_unit(id, SFF8636_BR_NOMINAL_OFFSET,
-			"BR, Nominal", 100, "Mbps");
-	sff8636_show_rate_identifier(id);
-	sff_show_value_with_unit(id, SFF8636_SM_LEN_OFFSET,
-		     "Length (SMF,km)", 1, "km");
-	sff_show_value_with_unit(id, SFF8636_OM3_LEN_OFFSET,
-			"Length (OM3 50um)", 2, "m");
-	sff_show_value_with_unit(id, SFF8636_OM2_LEN_OFFSET,
-			"Length (OM2 50um)", 1, "m");
-	sff_show_value_with_unit(id, SFF8636_OM1_LEN_OFFSET,
-		     "Length (OM1 62.5um)", 1, "m");
-	sff_show_value_with_unit(id, SFF8636_CBL_LEN_OFFSET,
-		     "Length (Copper or Active cable)", 1, "m");
-	sff8636_show_wavelength_or_copper_compliance(id);
-	sff_show_ascii(id, SFF8636_VENDOR_NAME_START_OFFSET,
+	sff8636_show_ext_identifier(map);
+	sff8636_show_connector(map);
+	sff8636_show_transceiver(map);
+	sff8636_show_encoding(map);
+	sff_show_value_with_unit(map->page_00h, SFF8636_BR_NOMINAL_OFFSET,
+				 "BR, Nominal", 100, "Mbps");
+	sff8636_show_rate_identifier(map);
+	sff_show_value_with_unit(map->page_00h, SFF8636_SM_LEN_OFFSET,
+				 "Length (SMF,km)", 1, "km");
+	sff_show_value_with_unit(map->page_00h, SFF8636_OM3_LEN_OFFSET,
+				 "Length (OM3 50um)", 2, "m");
+	sff_show_value_with_unit(map->page_00h, SFF8636_OM2_LEN_OFFSET,
+				 "Length (OM2 50um)", 1, "m");
+	sff_show_value_with_unit(map->page_00h, SFF8636_OM1_LEN_OFFSET,
+				 "Length (OM1 62.5um)", 1, "m");
+	sff_show_value_with_unit(map->page_00h, SFF8636_CBL_LEN_OFFSET,
+				 "Length (Copper or Active cable)", 1, "m");
+	sff8636_show_wavelength_or_copper_compliance(map);
+	sff_show_ascii(map->page_00h, SFF8636_VENDOR_NAME_START_OFFSET,
 		       SFF8636_VENDOR_NAME_END_OFFSET, "Vendor name");
-	sff8636_show_oui(id, SFF8636_VENDOR_OUI_OFFSET);
-	sff_show_ascii(id, SFF8636_VENDOR_PN_START_OFFSET,
+	sff8024_show_oui(map->page_00h, SFF8636_VENDOR_OUI_OFFSET);
+	sff_show_ascii(map->page_00h, SFF8636_VENDOR_PN_START_OFFSET,
 		       SFF8636_VENDOR_PN_END_OFFSET, "Vendor PN");
-	sff_show_ascii(id, SFF8636_VENDOR_REV_START_OFFSET,
+	sff_show_ascii(map->page_00h, SFF8636_VENDOR_REV_START_OFFSET,
 		       SFF8636_VENDOR_REV_END_OFFSET, "Vendor rev");
-	sff_show_ascii(id, SFF8636_VENDOR_SN_START_OFFSET,
+	sff_show_ascii(map->page_00h, SFF8636_VENDOR_SN_START_OFFSET,
 		       SFF8636_VENDOR_SN_END_OFFSET, "Vendor SN");
-	sff_show_ascii(id, SFF8636_DATE_YEAR_OFFSET,
+	sff_show_ascii(map->page_00h, SFF8636_DATE_YEAR_OFFSET,
 		       SFF8636_DATE_VENDOR_LOT_OFFSET + 1, "Date code");
-	sff8636_show_revision_compliance(id);
-
+	sff_show_revision_compliance(map->lower_memory,
+				     SFF8636_REV_COMPLIANCE_OFFSET);
 }
 
 static void sff8636_memory_map_init_buf(struct sff8636_memory_map *map,
@@ -896,13 +932,13 @@ void sff8636_show_all_ioctl(const __u8 *id, __u32 eeprom_len)
 
 	sff8636_memory_map_init_buf(&map, id, eeprom_len);
 
-	sff8636_show_identifier(id);
-	switch (id[SFF8636_ID_OFFSET]) {
+	sff8636_show_identifier(&map);
+	switch (map.lower_memory[SFF8636_ID_OFFSET]) {
 	case SFF8024_ID_QSFP:
 	case SFF8024_ID_QSFP_PLUS:
 	case SFF8024_ID_QSFP28:
-		sff8636_show_page_zero(id);
-		sff8636_show_dom(id, id + 3 * 0x80, eeprom_len);
+		sff8636_show_page_zero(&map);
+		sff8636_show_dom(&map);
 		break;
 	}
 }
@@ -939,9 +975,7 @@ void sff8636_show_all_nl(const struct ethtool_module_eeprom *page_zero,
 
 	sff8636_memory_map_init_pages(&map, page_zero, page_three);
 
-	sff8636_show_identifier(page_zero->data);
-	sff8636_show_page_zero(page_zero->data);
-	if (page_three)
-		sff8636_show_dom(page_zero->data, page_three->data - 0x80,
-				 ETH_MODULE_SFF_8636_MAX_LEN);
+	sff8636_show_identifier(&map);
+	sff8636_show_page_zero(&map);
+	sff8636_show_dom(&map);
 }
-- 
2.31.1

[PATCH ethtool-next 08/14] sff-8636: Consolidate code between IOCTL and netlink paths

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Now that both the netlink and IOCTL paths use the same memory map
structure for parsing, the code can be easily consolidated.

Note that the switch-case statement is not necessary for the netlink
path, as the netlink code (i.e., netlink/module-eeprom.c) already
performed the check, but it is required for the IOCTL path.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 qsfp.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/qsfp.c b/qsfp.c
index 354b3b1ce9ff..4aa49351e6b7 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -898,6 +898,19 @@ static void sff8636_show_page_zero(const struct sff8636_memory_map *map)
 				     SFF8636_REV_COMPLIANCE_OFFSET);
 }
 
+static void sff8636_show_all_common(const struct sff8636_memory_map *map)
+{
+	sff8636_show_identifier(map);
+	switch (map->lower_memory[SFF8636_ID_OFFSET]) {
+	case SFF8024_ID_QSFP:
+	case SFF8024_ID_QSFP_PLUS:
+	case SFF8024_ID_QSFP28:
+		sff8636_show_page_zero(map);
+		sff8636_show_dom(map);
+		break;
+	}
+}
+
 static void sff8636_memory_map_init_buf(struct sff8636_memory_map *map,
 					const __u8 *id, __u32 eeprom_len)
 {
@@ -931,16 +944,7 @@ void sff8636_show_all_ioctl(const __u8 *id, __u32 eeprom_len)
 	}
 
 	sff8636_memory_map_init_buf(&map, id, eeprom_len);
-
-	sff8636_show_identifier(&map);
-	switch (map.lower_memory[SFF8636_ID_OFFSET]) {
-	case SFF8024_ID_QSFP:
-	case SFF8024_ID_QSFP_PLUS:
-	case SFF8024_ID_QSFP28:
-		sff8636_show_page_zero(&map);
-		sff8636_show_dom(&map);
-		break;
-	}
+	sff8636_show_all_common(&map);
 }
 
 static void
@@ -974,8 +978,5 @@ void sff8636_show_all_nl(const struct ethtool_module_eeprom *page_zero,
 	struct sff8636_memory_map map = {};
 
 	sff8636_memory_map_init_pages(&map, page_zero, page_three);
-
-	sff8636_show_identifier(&map);
-	sff8636_show_page_zero(&map);
-	sff8636_show_dom(&map);
+	sff8636_show_all_common(&map);
 }
-- 
2.31.1

[PATCH ethtool-next 09/14] sff-8079: Split SFF-8079 parsing function

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

SFF-8079, unlike CMIS and SFF-8636, only has a single page and therefore
its parsing function (i.e., sff8079_show_all()) is called from both the
IOCTL and netlink paths with a buffer pointing to that single page.

In future patches, the netlink code (i.e., netlink/module-eeprom.c) will
no longer call the SFF-8079 code with a buffer pointing to the first 128
bytes of the EEPROM. Instead, the SFF-8079 code will need to request the
needed EEPROM data, as will be done in CMIS and SFF-8636.

Therefore, as a preparation for this change, split the main parsing
function into IOCTL and netlink variants.

No functional changes intended.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 ethtool.c               |  4 ++--
 internal.h              |  3 ++-
 netlink/module-eeprom.c |  2 +-
 sfpid.c                 | 12 +++++++++++-
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/ethtool.c b/ethtool.c
index e3347db78fc3..064bc697926e 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -4900,10 +4900,10 @@ static int do_getmodule(struct cmd_context *ctx)
 			switch (modinfo.type) {
 #ifdef ETHTOOL_ENABLE_PRETTY_DUMP
 			case ETH_MODULE_SFF_8079:
-				sff8079_show_all(eeprom->data);
+				sff8079_show_all_ioctl(eeprom->data);
 				break;
 			case ETH_MODULE_SFF_8472:
-				sff8079_show_all(eeprom->data);
+				sff8079_show_all_ioctl(eeprom->data);
 				sff8472_show_all(eeprom->data);
 				break;
 			case ETH_MODULE_SFF_8436:
diff --git a/internal.h b/internal.h
index 7ca6066d4e12..a77efd385698 100644
--- a/internal.h
+++ b/internal.h
@@ -384,7 +384,8 @@ int rxclass_rule_ins(struct cmd_context *ctx,
 int rxclass_rule_del(struct cmd_context *ctx, __u32 loc);
 
 /* Module EEPROM parsing code */
-void sff8079_show_all(const __u8 *id);
+void sff8079_show_all_ioctl(const __u8 *id);
+void sff8079_show_all_nl(const __u8 *id);
 
 /* Optics diagnostics */
 void sff8472_show_all(const __u8 *id);
diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
index 18b1abbe1252..101d5943c2bc 100644
--- a/netlink/module-eeprom.c
+++ b/netlink/module-eeprom.c
@@ -323,7 +323,7 @@ static void decoder_print(void)
 
 	switch (module_id) {
 	case SFF8024_ID_SFP:
-		sff8079_show_all(page_zero->data);
+		sff8079_show_all_nl(page_zero->data);
 		break;
 	case SFF8024_ID_QSFP:
 	case SFF8024_ID_QSFP28:
diff --git a/sfpid.c b/sfpid.c
index da2b3f4df3d2..c214820226d1 100644
--- a/sfpid.c
+++ b/sfpid.c
@@ -396,7 +396,7 @@ static void sff8079_show_options(const __u8 *id)
 		printf("%s Power level 3 requirement\n", pfx);
 }
 
-void sff8079_show_all(const __u8 *id)
+static void sff8079_show_all_common(const __u8 *id)
 {
 	sff8079_show_identifier(id);
 	if (((id[0] == 0x02) || (id[0] == 0x03)) && (id[1] == 0x04)) {
@@ -439,3 +439,13 @@ void sff8079_show_all(const __u8 *id)
 		sff8079_show_ascii(id, 84, 91, "Date code");
 	}
 }
+
+void sff8079_show_all_ioctl(const __u8 *id)
+{
+	sff8079_show_all_common(id);
+}
+
+void sff8079_show_all_nl(const __u8 *id)
+{
+	sff8079_show_all_common(id);
+}
-- 
2.31.1

[PATCH ethtool-next 10/14] netlink: eeprom: Export a function to request an EEPROM page

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

The function will be used by the EEPROM parsing code (e.g., cmis.c) to
request a specific page for parsing.

All the data buffers used to store EEPROM page contents are stored on a
linked list that is flushed on exit. This relieves callers from the need
to explicitly free the requested pages.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 netlink/extapi.h        |  11 +++++
 netlink/module-eeprom.c | 105 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/netlink/extapi.h b/netlink/extapi.h
index 91bf02b5e3be..129e2931d01d 100644
--- a/netlink/extapi.h
+++ b/netlink/extapi.h
@@ -48,6 +48,9 @@ int nl_getmodule(struct cmd_context *ctx);
 
 void nl_monitor_usage(void);
 
+int nl_get_eeprom_page(struct cmd_context *ctx,
+		       struct ethtool_module_eeprom *request);
+
 #else /* ETHTOOL_ENABLE_NETLINK */
 
 static inline void netlink_run_handler(struct cmd_context *ctx __maybe_unused,
@@ -73,6 +76,14 @@ static inline void nl_monitor_usage(void)
 {
 }
 
+static inline int
+nl_get_eeprom_page(struct cmd_context *ctx __maybe_unused,
+		   struct ethtool_module_eeprom *request __maybe_unused)
+{
+	fprintf(stderr, "Netlink not supported by ethtool.\n");
+	return -EOPNOTSUPP;
+}
+
 #define nl_gset			NULL
 #define nl_sset			NULL
 #define nl_permaddr		NULL
diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
index 101d5943c2bc..ee5508840157 100644
--- a/netlink/module-eeprom.c
+++ b/netlink/module-eeprom.c
@@ -341,6 +341,110 @@ static void decoder_print(void)
 }
 #endif
 
+static struct list_head eeprom_page_list = LIST_HEAD_INIT(eeprom_page_list);
+
+struct eeprom_page_entry {
+	struct list_head list;	/* Member of eeprom_page_list */
+	void *data;
+};
+
+static int eeprom_page_list_add(void *data)
+{
+	struct eeprom_page_entry *entry;
+
+	entry = malloc(sizeof(*entry));
+	if (!entry)
+		return -ENOMEM;
+
+	entry->data = data;
+	list_add(&entry->list, &eeprom_page_list);
+
+	return 0;
+}
+
+static void eeprom_page_list_flush(void)
+{
+	struct eeprom_page_entry *entry;
+	struct list_head *head, *next;
+
+	list_for_each_safe(head, next, &eeprom_page_list) {
+		entry = (struct eeprom_page_entry *) head;
+		free(entry->data);
+		list_del(head);
+		free(entry);
+	}
+}
+
+static int get_eeprom_page_reply_cb(const struct nlmsghdr *nlhdr, void *data)
+{
+	const struct nlattr *tb[ETHTOOL_A_MODULE_EEPROM_DATA + 1] = {};
+	struct ethtool_module_eeprom *request = data;
+	DECLARE_ATTR_TB_INFO(tb);
+	u8 *eeprom_data;
+	int ret;
+
+	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
+	if (ret < 0)
+		return ret;
+
+	if (!tb[ETHTOOL_A_MODULE_EEPROM_DATA])
+		return MNL_CB_ERROR;
+
+	eeprom_data = mnl_attr_get_payload(tb[ETHTOOL_A_MODULE_EEPROM_DATA]);
+	request->data = malloc(request->length);
+	if (!request->data)
+		return MNL_CB_ERROR;
+	memcpy(request->data, eeprom_data, request->length);
+
+	ret = eeprom_page_list_add(request->data);
+	if (ret < 0)
+		goto err_list_add;
+
+	return MNL_CB_OK;
+
+err_list_add:
+	free(request->data);
+	return MNL_CB_ERROR;
+}
+
+int nl_get_eeprom_page(struct cmd_context *ctx,
+		       struct ethtool_module_eeprom *request)
+{
+	struct nl_context *nlctx = ctx->nlctx;
+	struct nl_socket *nlsock;
+	struct nl_msg_buff *msg;
+	int ret;
+
+	if (!request || request->i2c_address > ETH_I2C_MAX_ADDRESS)
+		return -EINVAL;
+
+	nlsock = nlctx->ethnl_socket;
+	msg = &nlsock->msgbuff;
+
+	ret = nlsock_prep_get_request(nlsock, ETHTOOL_MSG_MODULE_EEPROM_GET,
+				      ETHTOOL_A_MODULE_EEPROM_HEADER, 0);
+	if (ret < 0)
+		return ret;
+
+	if (ethnla_put_u32(msg, ETHTOOL_A_MODULE_EEPROM_LENGTH,
+			   request->length) ||
+	    ethnla_put_u32(msg, ETHTOOL_A_MODULE_EEPROM_OFFSET,
+			   request->offset) ||
+	    ethnla_put_u8(msg, ETHTOOL_A_MODULE_EEPROM_PAGE,
+			  request->page) ||
+	    ethnla_put_u8(msg, ETHTOOL_A_MODULE_EEPROM_BANK,
+			  request->bank) ||
+	    ethnla_put_u8(msg, ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS,
+			  request->i2c_address))
+		return -EMSGSIZE;
+
+	ret = nlsock_sendmsg(nlsock, NULL);
+	if (ret < 0)
+		return ret;
+	return nlsock_process_reply(nlsock, get_eeprom_page_reply_cb,
+				    (void *)request);
+}
+
 int nl_getmodule(struct cmd_context *ctx)
 {
 	struct cmd_params getmodule_cmd_params = {};
@@ -425,6 +529,7 @@ int nl_getmodule(struct cmd_context *ctx)
 	}
 
 cleanup:
+	eeprom_page_list_flush();
 	cache_free();
 	return ret;
 }
-- 
2.31.1

[PATCH ethtool-next 11/14] cmis: Request specific pages for parsing in netlink path

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

In the netlink path, unlike the IOCTL path, user space requests specific
EEPROM pages from the kernel. The presence of optional and banked pages
is advertised via various bits in the EEPROM contents.

Currently, for CMIS, the Lower Memory, Page 00h and the optional Page
01h are requested by the netlink code (i.e., netlink/module-eeprom.c)
and passed to the CMIS code (i.e., cmis.c) as two arguments for parsing.

This is problematic for several reasons. First, this approach is not
very scaleable as CMIS supports a lot of optional and banked pages.
Passing them as separate arguments to the CMIS code is not going to
work.

Second, the knowledge of which optional and banked pages are available
is encapsulated in the CMIS parsing code. As such, the common netlink
code has no business of fetching optional and banked pages that might be
invalid.

Instead, pass the command context to the CMIS parsing function and allow
it to fetch only valid pages via the 'MODULE_EEPROM_GET' netlink
message.

Tested by making sure that the output of 'ethtool -m' does not change
before and after the patch.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 cmis.c                  | 60 ++++++++++++++++++++++++++++++++---------
 cmis.h                  |  3 +--
 netlink/module-eeprom.c |  7 +++--
 3 files changed, 51 insertions(+), 19 deletions(-)

diff --git a/cmis.c b/cmis.c
index eb7791dd59df..4798fd4c7d68 100644
--- a/cmis.c
+++ b/cmis.c
@@ -9,9 +9,11 @@
 
 #include <stdio.h>
 #include <math.h>
+#include <errno.h>
 #include "internal.h"
 #include "sff-common.h"
 #include "cmis.h"
+#include "netlink/extapi.h"
 
 struct cmis_memory_map {
 	const __u8 *lower_memory;
@@ -21,6 +23,7 @@ struct cmis_memory_map {
 };
 
 #define CMIS_PAGE_SIZE		0x80
+#define CMIS_I2C_ADDRESS	0x50
 
 static void cmis_show_identifier(const struct cmis_memory_map *map)
 {
@@ -384,36 +387,67 @@ void cmis_show_all_ioctl(const __u8 *id)
 	cmis_show_all_common(&map);
 }
 
-static void
-cmis_memory_map_init_pages(struct cmis_memory_map *map,
-			   const struct ethtool_module_eeprom *page_zero,
-			   const struct ethtool_module_eeprom *page_one)
+static void cmis_request_init(struct ethtool_module_eeprom *request, u8 bank,
+			      u8 page, u32 offset)
 {
+	request->offset = offset;
+	request->length = CMIS_PAGE_SIZE;
+	request->page = page;
+	request->bank = bank;
+	request->i2c_address = CMIS_I2C_ADDRESS;
+	request->data = NULL;
+}
+
+static int
+cmis_memory_map_init_pages(struct cmd_context *ctx,
+			   struct cmis_memory_map *map)
+{
+	struct ethtool_module_eeprom request;
+	int ret;
+
 	/* Lower Memory and Page 00h are always present.
 	 *
 	 * Offset into Upper Memory is between page size and twice the page
 	 * size. Therefore, set the base address of each page to its base
-	 * address minus page size. For Page 00h, this is the address of the
-	 * Lower Memory.
+	 * address minus page size.
 	 */
-	map->lower_memory = page_zero->data;
-	map->page_00h = page_zero->data;
+	cmis_request_init(&request, 0, 0x0, 0);
+	ret = nl_get_eeprom_page(ctx, &request);
+	if (ret < 0)
+		return ret;
+	map->lower_memory = request.data;
+
+	cmis_request_init(&request, 0, 0x0, CMIS_PAGE_SIZE);
+	ret = nl_get_eeprom_page(ctx, &request);
+	if (ret < 0)
+		return ret;
+	map->page_00h = request.data - CMIS_PAGE_SIZE;
 
 	/* Page 01h is only present when the module memory model is paged and
 	 * not flat.
 	 */
 	if (map->lower_memory[CMIS_MEMORY_MODEL_OFFSET] &
 	    CMIS_MEMORY_MODEL_MASK)
-		return;
+		return 0;
+
+	cmis_request_init(&request, 0, 0x1, CMIS_PAGE_SIZE);
+	ret = nl_get_eeprom_page(ctx, &request);
+	if (ret < 0)
+		return ret;
+	map->page_01h = request.data - CMIS_PAGE_SIZE;
 
-	map->page_01h = page_one->data - CMIS_PAGE_SIZE;
+	return 0;
 }
 
-void cmis_show_all_nl(const struct ethtool_module_eeprom *page_zero,
-		      const struct ethtool_module_eeprom *page_one)
+int cmis_show_all_nl(struct cmd_context *ctx)
 {
 	struct cmis_memory_map map = {};
+	int ret;
 
-	cmis_memory_map_init_pages(&map, page_zero, page_one);
+	ret = cmis_memory_map_init_pages(ctx, &map);
+	if (ret < 0)
+		return ret;
 	cmis_show_all_common(&map);
+
+	return 0;
 }
diff --git a/cmis.h b/cmis.h
index c878e3bc5afd..911491dc5c8f 100644
--- a/cmis.h
+++ b/cmis.h
@@ -123,7 +123,6 @@
 
 void cmis_show_all_ioctl(const __u8 *id);
 
-void cmis_show_all_nl(const struct ethtool_module_eeprom *page_zero,
-		      const struct ethtool_module_eeprom *page_one);
+int cmis_show_all_nl(struct cmd_context *ctx);
 
 #endif /* CMIS_H__ */
diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
index ee5508840157..a8e2662e0b8c 100644
--- a/netlink/module-eeprom.c
+++ b/netlink/module-eeprom.c
@@ -314,11 +314,10 @@ static int decoder_prefetch(struct nl_context *nlctx)
 	return page_fetch(nlctx, &request);
 }
 
-static void decoder_print(void)
+static void decoder_print(struct cmd_context *ctx)
 {
 	struct ethtool_module_eeprom *page_three = cache_get(3, 0, ETH_I2C_ADDRESS_LOW);
 	struct ethtool_module_eeprom *page_zero = cache_get(0, 0, ETH_I2C_ADDRESS_LOW);
-	struct ethtool_module_eeprom *page_one = cache_get(1, 0, ETH_I2C_ADDRESS_LOW);
 	u8 module_id = page_zero->data[SFF8636_ID_OFFSET];
 
 	switch (module_id) {
@@ -332,7 +331,7 @@ static void decoder_print(void)
 		break;
 	case SFF8024_ID_QSFP_DD:
 	case SFF8024_ID_DSFP:
-		cmis_show_all_nl(page_zero, page_one);
+		cmis_show_all_nl(ctx);
 		break;
 	default:
 		dump_hex(stdout, page_zero->data, page_zero->length, page_zero->offset);
@@ -524,7 +523,7 @@ int nl_getmodule(struct cmd_context *ctx)
 		ret = decoder_prefetch(nlctx);
 		if (ret)
 			goto cleanup;
-		decoder_print();
+		decoder_print(ctx);
 #endif
 	}
 
-- 
2.31.1

[PATCH ethtool-next 12/14] sff-8636: Request specific pages for parsing in netlink path

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

In the netlink path, unlike the IOCTL path, user space requests specific
EEPROM pages from the kernel. The presence of optional pages is
advertised via various bits in the EEPROM contents.

Currently, for SFF-8636, the Lower Memory, Page 00h and the optional
Page 03h are requested by the netlink code (i.e.,
netlink/module-eeprom.c) and passed to the SFF-8636 code (i.e., qsfp.c)
as two arguments for parsing.

This is problematic for several reasons. First, this approach is not
very scaleable as SFF-8636 supports a lot of optional pages. Passing
them as separate arguments to the SFF-8636 code is not going to work.

Second, the knowledge of which optional pages are available is
encapsulated in the SFF-8636 parsing code. As such, the common netlink
code has no business of fetching optional pages that might be invalid.

Instead, pass the command context to the SFF-8636 parsing function and
allow it to fetch only valid pages via the 'MODULE_EEPROM_GET' netlink
message.

Tested by making sure that the output of 'ethtool -m' does not change
before and after the patch.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 internal.h              |  3 +--
 netlink/module-eeprom.c |  3 +--
 qsfp.c                  | 60 ++++++++++++++++++++++++++++++++---------
 3 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/internal.h b/internal.h
index a77efd385698..2407d3c223fa 100644
--- a/internal.h
+++ b/internal.h
@@ -392,8 +392,7 @@ void sff8472_show_all(const __u8 *id);
 
 /* QSFP Optics diagnostics */
 void sff8636_show_all_ioctl(const __u8 *id, __u32 eeprom_len);
-void sff8636_show_all_nl(const struct ethtool_module_eeprom *page_zero,
-			 const struct ethtool_module_eeprom *page_three);
+int sff8636_show_all_nl(struct cmd_context *ctx);
 
 /* FUJITSU Extended Socket network device */
 int fjes_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
index a8e2662e0b8c..f04f8e134223 100644
--- a/netlink/module-eeprom.c
+++ b/netlink/module-eeprom.c
@@ -316,7 +316,6 @@ static int decoder_prefetch(struct nl_context *nlctx)
 
 static void decoder_print(struct cmd_context *ctx)
 {
-	struct ethtool_module_eeprom *page_three = cache_get(3, 0, ETH_I2C_ADDRESS_LOW);
 	struct ethtool_module_eeprom *page_zero = cache_get(0, 0, ETH_I2C_ADDRESS_LOW);
 	u8 module_id = page_zero->data[SFF8636_ID_OFFSET];
 
@@ -327,7 +326,7 @@ static void decoder_print(struct cmd_context *ctx)
 	case SFF8024_ID_QSFP:
 	case SFF8024_ID_QSFP28:
 	case SFF8024_ID_QSFP_PLUS:
-		sff8636_show_all_nl(page_zero, page_three);
+		sff8636_show_all_nl(ctx);
 		break;
 	case SFF8024_ID_QSFP_DD:
 	case SFF8024_ID_DSFP:
diff --git a/qsfp.c b/qsfp.c
index 4aa49351e6b7..e7c2f51cd9c6 100644
--- a/qsfp.c
+++ b/qsfp.c
@@ -55,10 +55,12 @@
  **/
 #include <stdio.h>
 #include <math.h>
+#include <errno.h>
 #include "internal.h"
 #include "sff-common.h"
 #include "qsfp.h"
 #include "cmis.h"
+#include "netlink/extapi.h"
 
 struct sff8636_memory_map {
 	const __u8 *lower_memory;
@@ -68,6 +70,7 @@ struct sff8636_memory_map {
 };
 
 #define SFF8636_PAGE_SIZE	0x80
+#define SFF8636_I2C_ADDRESS	0x50
 
 #define MAX_DESC_SIZE	42
 
@@ -947,36 +950,67 @@ void sff8636_show_all_ioctl(const __u8 *id, __u32 eeprom_len)
 	sff8636_show_all_common(&map);
 }
 
-static void
-sff8636_memory_map_init_pages(struct sff8636_memory_map *map,
-			      const struct ethtool_module_eeprom *page_zero,
-			      const struct ethtool_module_eeprom *page_three)
+static void sff8636_request_init(struct ethtool_module_eeprom *request, u8 page,
+				 u32 offset)
+{
+	request->offset = offset;
+	request->length = SFF8636_PAGE_SIZE;
+	request->page = page;
+	request->bank = 0;
+	request->i2c_address = SFF8636_I2C_ADDRESS;
+	request->data = NULL;
+}
+
+static int
+sff8636_memory_map_init_pages(struct cmd_context *ctx,
+			      struct sff8636_memory_map *map)
 {
+	struct ethtool_module_eeprom request;
+	int ret;
+
 	/* Lower Memory and Page 00h are always present.
 	 *
 	 * Offset into Upper Memory is between page size and twice the page
 	 * size. Therefore, set the base address of each page to its base
-	 * address minus page size. For Page 00h, this is the address of the
-	 * Lower Memory.
+	 * address minus page size.
 	 */
-	map->lower_memory = page_zero->data;
-	map->page_00h = page_zero->data;
+	sff8636_request_init(&request, 0x0, 0);
+	ret = nl_get_eeprom_page(ctx, &request);
+	if (ret < 0)
+		return ret;
+	map->lower_memory = request.data;
+
+	sff8636_request_init(&request, 0x0, SFF8636_PAGE_SIZE);
+	ret = nl_get_eeprom_page(ctx, &request);
+	if (ret < 0)
+		return ret;
+	map->page_00h = request.data - SFF8636_PAGE_SIZE;
 
 	/* Page 03h is only present when the module memory model is paged and
 	 * not flat.
 	 */
 	if (map->lower_memory[SFF8636_STATUS_2_OFFSET] &
 	    SFF8636_STATUS_PAGE_3_PRESENT)
-		return;
+		return 0;
 
-	map->page_03h = page_three->data - SFF8636_PAGE_SIZE;
+	sff8636_request_init(&request, 0x3, SFF8636_PAGE_SIZE);
+	ret = nl_get_eeprom_page(ctx, &request);
+	if (ret < 0)
+		return ret;
+	map->page_03h = request.data - SFF8636_PAGE_SIZE;
+
+	return 0;
 }
 
-void sff8636_show_all_nl(const struct ethtool_module_eeprom *page_zero,
-			 const struct ethtool_module_eeprom *page_three)
+int sff8636_show_all_nl(struct cmd_context *ctx)
 {
 	struct sff8636_memory_map map = {};
+	int ret;
 
-	sff8636_memory_map_init_pages(&map, page_zero, page_three);
+	ret = sff8636_memory_map_init_pages(ctx, &map);
+	if (ret < 0)
+		return ret;
 	sff8636_show_all_common(&map);
+
+	return 0;
 }
-- 
2.31.1

[PATCH ethtool-next 13/14] sff-8079: Request specific pages for parsing in netlink path

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

Convert the SFF-8079 code to request the required EEPROM contents in the
netlink path as was done for CMIS and SFF-8636. It will allow us to
remove standard-specific code from the netlink code (i.e.,
netlink/module-eeprom.c).

In addition, in the future, it will allow the netlink path to support
parsing of SFF-8472.

Tested by making sure that the output of 'ethtool -m' does not change
before and after the patch.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 internal.h              |  2 +-
 netlink/module-eeprom.c |  2 +-
 sfpid.c                 | 20 ++++++++++++++++++--
 3 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/internal.h b/internal.h
index 2407d3c223fa..0d9d816ab563 100644
--- a/internal.h
+++ b/internal.h
@@ -385,7 +385,7 @@ int rxclass_rule_del(struct cmd_context *ctx, __u32 loc);
 
 /* Module EEPROM parsing code */
 void sff8079_show_all_ioctl(const __u8 *id);
-void sff8079_show_all_nl(const __u8 *id);
+int sff8079_show_all_nl(struct cmd_context *ctx);
 
 /* Optics diagnostics */
 void sff8472_show_all(const __u8 *id);
diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
index f04f8e134223..6d76b8a96461 100644
--- a/netlink/module-eeprom.c
+++ b/netlink/module-eeprom.c
@@ -321,7 +321,7 @@ static void decoder_print(struct cmd_context *ctx)
 
 	switch (module_id) {
 	case SFF8024_ID_SFP:
-		sff8079_show_all_nl(page_zero->data);
+		sff8079_show_all_nl(ctx);
 		break;
 	case SFF8024_ID_QSFP:
 	case SFF8024_ID_QSFP28:
diff --git a/sfpid.c b/sfpid.c
index c214820226d1..621d1e86c278 100644
--- a/sfpid.c
+++ b/sfpid.c
@@ -8,8 +8,13 @@
  */
 
 #include <stdio.h>
+#include <errno.h>
 #include "internal.h"
 #include "sff-common.h"
+#include "netlink/extapi.h"
+
+#define SFF8079_PAGE_SIZE	0x80
+#define SFF8079_I2C_ADDRESS_LOW	0x50
 
 static void sff8079_show_identifier(const __u8 *id)
 {
@@ -445,7 +450,18 @@ void sff8079_show_all_ioctl(const __u8 *id)
 	sff8079_show_all_common(id);
 }
 
-void sff8079_show_all_nl(const __u8 *id)
+int sff8079_show_all_nl(struct cmd_context *ctx)
 {
-	sff8079_show_all_common(id);
+	struct ethtool_module_eeprom request = {
+		.length = SFF8079_PAGE_SIZE,
+		.i2c_address = SFF8079_I2C_ADDRESS_LOW,
+	};
+	int ret;
+
+	ret = nl_get_eeprom_page(ctx, &request);
+	if (ret < 0)
+		return ret;
+	sff8079_show_all_common(request.data);
+
+	return 0;
 }
-- 
2.31.1

[PATCH ethtool-next 14/14] netlink: eeprom: Defer page requests to individual parsers

[Ido Schimmel]

From: Ido Schimmel <idosch@nvidia.com>

The individual EEPROM parsers (e.g., CMIS, SFF-8636) now request the
EEPROM pages they intend to parse and populate their memory maps before
parsing them.

Therefore, there is no need for the common netlink code to request
potentially invalid pages and pass them as blobs to these parsers.

Instead, only query the SFF-8024 Identifier Value which is located at
I2C address 0x50, byte 0 and dispatch to the relevant EEPROM parser.

Tested by making sure that the output of 'ethtool -m' does not change
for SFF-8079, SFF-8636 and CMIS before and after the patch.

Signed-off-by: Ido Schimmel <idosch@nvidia.com>
---
 netlink/module-eeprom.c | 347 +++++++---------------------------------
 1 file changed, 59 insertions(+), 288 deletions(-)

diff --git a/netlink/module-eeprom.c b/netlink/module-eeprom.c
index 6d76b8a96461..f359aeec4ddf 100644
--- a/netlink/module-eeprom.c
+++ b/netlink/module-eeprom.c
@@ -19,7 +19,6 @@
 #include "parser.h"
 
 #define ETH_I2C_ADDRESS_LOW	0x50
-#define ETH_I2C_ADDRESS_HIGH	0x51
 #define ETH_I2C_MAX_ADDRESS	0x7F
 
 struct cmd_params {
@@ -78,267 +77,6 @@ static const struct param_parser getmodule_params[] = {
 	{}
 };
 
-struct page_entry {
-	struct list_head link;
-	struct ethtool_module_eeprom *page;
-};
-
-static struct list_head page_list = LIST_HEAD_INIT(page_list);
-
-static int cache_add(struct ethtool_module_eeprom *page)
-{
-	struct page_entry *list_element;
-
-	if (!page)
-		return -1;
-	list_element = malloc(sizeof(*list_element));
-	if (!list_element)
-		return -ENOMEM;
-	list_element->page = page;
-
-	list_add(&list_element->link, &page_list);
-	return 0;
-}
-
-static void page_free(struct ethtool_module_eeprom *page)
-{
-	free(page->data);
-	free(page);
-}
-
-static void cache_del(struct ethtool_module_eeprom *page)
-{
-	struct ethtool_module_eeprom *entry;
-	struct list_head *head, *next;
-
-	list_for_each_safe(head, next, &page_list) {
-		entry = ((struct page_entry *)head)->page;
-		if (entry == page) {
-			list_del(head);
-			free(head);
-			page_free(entry);
-			break;
-		}
-	}
-}
-
-static void cache_free(void)
-{
-	struct ethtool_module_eeprom *entry;
-	struct list_head *head, *next;
-
-	list_for_each_safe(head, next, &page_list) {
-		entry = ((struct page_entry *)head)->page;
-		list_del(head);
-		free(head);
-		page_free(entry);
-	}
-}
-
-static struct ethtool_module_eeprom *page_join(struct ethtool_module_eeprom *page_a,
-					       struct ethtool_module_eeprom *page_b)
-{
-	struct ethtool_module_eeprom *joined_page;
-	u32 total_length;
-
-	if (!page_a || !page_b ||
-	    page_a->page != page_b->page ||
-	    page_a->bank != page_b->bank ||
-	    page_a->i2c_address != page_b->i2c_address)
-		return NULL;
-
-	total_length = page_a->length + page_b->length;
-	joined_page = calloc(1, sizeof(*joined_page));
-	joined_page->data = calloc(1, total_length);
-	joined_page->page = page_a->page;
-	joined_page->bank = page_a->bank;
-	joined_page->length = total_length;
-	joined_page->i2c_address = page_a->i2c_address;
-
-	if (page_a->offset < page_b->offset) {
-		memcpy(joined_page->data, page_a->data, page_a->length);
-		memcpy(joined_page->data + page_a->length, page_b->data, page_b->length);
-		joined_page->offset = page_a->offset;
-	} else {
-		memcpy(joined_page->data, page_b->data, page_b->length);
-		memcpy(joined_page->data + page_b->length, page_a->data, page_a->length);
-		joined_page->offset = page_b->offset;
-	}
-
-	return joined_page;
-}
-
-static struct ethtool_module_eeprom *cache_get(u32 page, u32 bank, u8 i2c_address)
-{
-	struct ethtool_module_eeprom *entry;
-	struct list_head *head, *next;
-
-	list_for_each_safe(head, next, &page_list) {
-		entry = ((struct page_entry *)head)->page;
-		if (entry->page == page && entry->bank == bank &&
-		    entry->i2c_address == i2c_address)
-			return entry;
-	}
-
-	return NULL;
-}
-
-static int getmodule_page_fetch_reply_cb(const struct nlmsghdr *nlhdr,
-					 void *data)
-{
-	const struct nlattr *tb[ETHTOOL_A_MODULE_EEPROM_DATA + 1] = {};
-	DECLARE_ATTR_TB_INFO(tb);
-	struct ethtool_module_eeprom *lower_page;
-	struct ethtool_module_eeprom *response;
-	struct ethtool_module_eeprom *request;
-	struct ethtool_module_eeprom *joined;
-	u8 *eeprom_data;
-	int ret;
-
-	ret = mnl_attr_parse(nlhdr, GENL_HDRLEN, attr_cb, &tb_info);
-	if (ret < 0)
-		return ret;
-
-	if (!tb[ETHTOOL_A_MODULE_EEPROM_DATA]) {
-		fprintf(stderr, "Malformed netlink message (getmodule)\n");
-		return MNL_CB_ERROR;
-	}
-
-	response = calloc(1, sizeof(*response));
-	if (!response)
-		return -ENOMEM;
-
-	request = (struct ethtool_module_eeprom *)data;
-	response->offset = request->offset;
-	response->page = request->page;
-	response->bank = request->bank;
-	response->i2c_address = request->i2c_address;
-	response->length = mnl_attr_get_payload_len(tb[ETHTOOL_A_MODULE_EEPROM_DATA]);
-	eeprom_data = mnl_attr_get_payload(tb[ETHTOOL_A_MODULE_EEPROM_DATA]);
-
-	response->data = malloc(response->length);
-	if (!response->data) {
-		free(response);
-		return -ENOMEM;
-	}
-	memcpy(response->data, eeprom_data, response->length);
-
-	if (!request->page) {
-		lower_page = cache_get(request->page, request->bank, response->i2c_address);
-		if (lower_page) {
-			joined = page_join(lower_page, response);
-			page_free(response);
-			cache_del(lower_page);
-			return cache_add(joined);
-		}
-	}
-
-	return cache_add(response);
-}
-
-static int page_fetch(struct nl_context *nlctx, const struct ethtool_module_eeprom *request)
-{
-	struct nl_socket *nlsock = nlctx->ethnl_socket;
-	struct nl_msg_buff *msg = &nlsock->msgbuff;
-	struct ethtool_module_eeprom *page;
-	int ret;
-
-	if (!request || request->i2c_address > ETH_I2C_MAX_ADDRESS)
-		return -EINVAL;
-
-	/* Satisfy request right away, if region is already in cache */
-	page = cache_get(request->page, request->bank, request->i2c_address);
-	if (page && page->offset <= request->offset &&
-	    page->offset + page->length >= request->offset + request->length) {
-		return 0;
-	}
-
-	ret = nlsock_prep_get_request(nlsock, ETHTOOL_MSG_MODULE_EEPROM_GET,
-				      ETHTOOL_A_MODULE_EEPROM_HEADER, 0);
-	if (ret < 0)
-		return ret;
-
-	if (ethnla_put_u32(msg, ETHTOOL_A_MODULE_EEPROM_LENGTH, request->length) ||
-	    ethnla_put_u32(msg, ETHTOOL_A_MODULE_EEPROM_OFFSET, request->offset) ||
-	    ethnla_put_u8(msg, ETHTOOL_A_MODULE_EEPROM_PAGE, request->page) ||
-	    ethnla_put_u8(msg, ETHTOOL_A_MODULE_EEPROM_BANK, request->bank) ||
-	    ethnla_put_u8(msg, ETHTOOL_A_MODULE_EEPROM_I2C_ADDRESS, request->i2c_address))
-		return -EMSGSIZE;
-
-	ret = nlsock_sendmsg(nlsock, NULL);
-	if (ret < 0)
-		return ret;
-	ret = nlsock_process_reply(nlsock, getmodule_page_fetch_reply_cb, (void *)request);
-	if (ret < 0)
-		return ret;
-
-	return nlsock_process_reply(nlsock, nomsg_reply_cb, NULL);
-}
-
-#ifdef ETHTOOL_ENABLE_PRETTY_DUMP
-static int decoder_prefetch(struct nl_context *nlctx)
-{
-	struct ethtool_module_eeprom *page_zero_lower = cache_get(0, 0, ETH_I2C_ADDRESS_LOW);
-	struct ethtool_module_eeprom request = {0};
-	u8 module_id = page_zero_lower->data[0];
-	int err = 0;
-
-	/* Fetch rest of page 00 */
-	request.i2c_address = ETH_I2C_ADDRESS_LOW;
-	request.offset = 128;
-	request.length = 128;
-	err = page_fetch(nlctx, &request);
-	if (err)
-		return err;
-
-	switch (module_id) {
-	case SFF8024_ID_QSFP:
-	case SFF8024_ID_QSFP28:
-	case SFF8024_ID_QSFP_PLUS:
-		memset(&request, 0, sizeof(request));
-		request.i2c_address = ETH_I2C_ADDRESS_LOW;
-		request.offset = 128;
-		request.length = 128;
-		request.page = 3;
-		break;
-	case SFF8024_ID_QSFP_DD:
-	case SFF8024_ID_DSFP:
-		memset(&request, 0, sizeof(request));
-		request.i2c_address = ETH_I2C_ADDRESS_LOW;
-		request.offset = 128;
-		request.length = 128;
-		request.page = 1;
-		break;
-	}
-
-	return page_fetch(nlctx, &request);
-}
-
-static void decoder_print(struct cmd_context *ctx)
-{
-	struct ethtool_module_eeprom *page_zero = cache_get(0, 0, ETH_I2C_ADDRESS_LOW);
-	u8 module_id = page_zero->data[SFF8636_ID_OFFSET];
-
-	switch (module_id) {
-	case SFF8024_ID_SFP:
-		sff8079_show_all_nl(ctx);
-		break;
-	case SFF8024_ID_QSFP:
-	case SFF8024_ID_QSFP28:
-	case SFF8024_ID_QSFP_PLUS:
-		sff8636_show_all_nl(ctx);
-		break;
-	case SFF8024_ID_QSFP_DD:
-	case SFF8024_ID_DSFP:
-		cmis_show_all_nl(ctx);
-		break;
-	default:
-		dump_hex(stdout, page_zero->data, page_zero->length, page_zero->offset);
-		break;
-	}
-}
-#endif
-
 static struct list_head eeprom_page_list = LIST_HEAD_INIT(eeprom_page_list);
 
 struct eeprom_page_entry {
@@ -443,14 +181,64 @@ int nl_get_eeprom_page(struct cmd_context *ctx,
 				    (void *)request);
 }
 
+static int eeprom_dump_hex(struct cmd_context *ctx)
+{
+	struct ethtool_module_eeprom request = {
+		.length = 128,
+		.i2c_address = ETH_I2C_ADDRESS_LOW,
+	};
+	int ret;
+
+	ret = nl_get_eeprom_page(ctx, &request);
+	if (ret < 0)
+		return ret;
+
+	dump_hex(stdout, request.data, request.length, request.offset);
+
+	return 0;
+}
+
+static int eeprom_parse(struct cmd_context *ctx)
+{
+	struct ethtool_module_eeprom request = {
+		.length = 1,
+		.i2c_address = ETH_I2C_ADDRESS_LOW,
+	};
+	int ret;
+
+	/* Fetch the SFF-8024 Identifier Value. For all supported standards, it
+	 * is located at I2C address 0x50, byte 0. See section 4.1 in SFF-8024,
+	 * revision 4.9.
+	 */
+	ret = nl_get_eeprom_page(ctx, &request);
+	if (ret < 0)
+		return ret;
+
+	switch (request.data[0]) {
+#ifdef ETHTOOL_ENABLE_PRETTY_DUMP
+	case SFF8024_ID_SFP:
+		return sff8079_show_all_nl(ctx);
+	case SFF8024_ID_QSFP:
+	case SFF8024_ID_QSFP28:
+	case SFF8024_ID_QSFP_PLUS:
+		return sff8636_show_all_nl(ctx);
+	case SFF8024_ID_QSFP_DD:
+	case SFF8024_ID_DSFP:
+		return cmis_show_all_nl(ctx);
+#endif
+	default:
+		/* If we cannot recognize the memory map, default to dumping
+		 * the first 128 bytes in hex.
+		 */
+		return eeprom_dump_hex(ctx);
+	}
+}
+
 int nl_getmodule(struct cmd_context *ctx)
 {
 	struct cmd_params getmodule_cmd_params = {};
 	struct ethtool_module_eeprom request = {0};
-	struct ethtool_module_eeprom *reply_page;
 	struct nl_context *nlctx = ctx->nlctx;
-	u32 dump_length;
-	u8 *eeprom_data;
 	int ret;
 
 	if (netlink_cmd_check(ctx, ETHTOOL_MSG_MODULE_EEPROM_GET, false))
@@ -479,12 +267,6 @@ int nl_getmodule(struct cmd_context *ctx)
 		return -EOPNOTSUPP;
 	}
 
-	request.i2c_address = ETH_I2C_ADDRESS_LOW;
-	request.length = 128;
-	ret = page_fetch(nlctx, &request);
-	if (ret)
-		goto cleanup;
-
 #ifdef ETHTOOL_ENABLE_PRETTY_DUMP
 	if (getmodule_cmd_params.page || getmodule_cmd_params.bank ||
 	    getmodule_cmd_params.offset || getmodule_cmd_params.length)
@@ -501,33 +283,22 @@ int nl_getmodule(struct cmd_context *ctx)
 		request.offset = 128;
 
 	if (getmodule_cmd_params.dump_hex || getmodule_cmd_params.dump_raw) {
-		ret = page_fetch(nlctx, &request);
+		ret = nl_get_eeprom_page(ctx, &request);
 		if (ret < 0)
 			goto cleanup;
-		reply_page = cache_get(request.page, request.bank, request.i2c_address);
-		if (!reply_page) {
-			ret = -EINVAL;
-			goto cleanup;
-		}
 
-		eeprom_data = reply_page->data + (request.offset - reply_page->offset);
-		dump_length = reply_page->length < request.length ? reply_page->length
-								  : request.length;
 		if (getmodule_cmd_params.dump_raw)
-			fwrite(eeprom_data, 1, request.length, stdout);
+			fwrite(request.data, 1, request.length, stdout);
 		else
-			dump_hex(stdout, eeprom_data, dump_length, request.offset);
+			dump_hex(stdout, request.data, request.length,
+				 request.offset);
 	} else {
-#ifdef ETHTOOL_ENABLE_PRETTY_DUMP
-		ret = decoder_prefetch(nlctx);
-		if (ret)
+		ret = eeprom_parse(ctx);
+		if (ret < 0)
 			goto cleanup;
-		decoder_print(ctx);
-#endif
 	}
 
 cleanup:
 	eeprom_page_list_flush();
-	cache_free();
 	return ret;
 }
-- 
2.31.1

Re: [PATCH ethtool-next 00/14] ethtool: Use memory maps for EEPROM parsing

[Michal Kubecek]

[-- Attachment #1: Type: text/plain, Size: 639 bytes --]

On Tue, Oct 12, 2021 at 04:25:11PM +0300, Ido Schimmel wrote:
> From: Ido Schimmel <idosch@nvidia.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).

I still need to take a closer look at some of the patches but just to be
sure: the only reason to leave this series for 5.16 cycle is that it's
rather big and intrusive change (i.e. it does not depend on any kernel
functionality not present in 5.15), right?

Michal


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH ethtool-next 00/14] ethtool: Use memory maps for EEPROM parsing

[Ido Schimmel]

On Wed, Oct 27, 2021 at 10:30:45PM +0200, Michal Kubecek wrote:
> On Tue, Oct 12, 2021 at 04:25:11PM +0300, Ido Schimmel wrote:
> > From: Ido Schimmel <idosch@nvidia.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).
> 
> I still need to take a closer look at some of the patches but just to be
> sure: the only reason to leave this series for 5.16 cycle is that it's
> rather big and intrusive change (i.e. it does not depend on any kernel
> functionality not present in 5.15), right?

Right, it does not depend on any kernel functionality not present in
5.15

Re: [PATCH ethtool-next 00/14] ethtool: Use memory maps for EEPROM parsing

[Ido Schimmel]

On Thu, Oct 28, 2021 at 01:01:05AM +0300, Ido Schimmel wrote:
> On Wed, Oct 27, 2021 at 10:30:45PM +0200, Michal Kubecek wrote:
> > On Tue, Oct 12, 2021 at 04:25:11PM +0300, Ido Schimmel wrote:
> > > From: Ido Schimmel <idosch@nvidia.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).
> > 
> > I still need to take a closer look at some of the patches but just to be
> > sure: the only reason to leave this series for 5.16 cycle is that it's
> > rather big and intrusive change (i.e. it does not depend on any kernel
> > functionality not present in 5.15), right?
> 
> Right, it does not depend on any kernel functionality not present in
> 5.15

Can the patchset be applied to ethtool-next please? It is marked as
"Under Review" for over a month now and I would like to make progress
with other patches I have in the queue.

Thanks

Re: [PATCH ethtool-next 00/14] ethtool: Use memory maps for EEPROM parsing

[patchwork-bot+netdevbpf]

Hello:

This series was applied to ethtool/ethtool.git (master)
by Michal Kubecek <mkubecek@suse.cz>:

On Tue, 12 Oct 2021 16:25:11 +0300 you wrote:
> From: Ido Schimmel <idosch@nvidia.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).
> 
> [...]

Here is the summary with links:
  - [ethtool-next,01/14] cmis: Rename CMIS parsing functions
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=795f42092f20
  - [ethtool-next,02/14] cmis: Initialize CMIS memory map
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=369b43a1a066
  - [ethtool-next,03/14] cmis: Use memory map during parsing
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=da1628840bd6
  - [ethtool-next,04/14] cmis: Consolidate code between IOCTL and netlink paths
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=6acaeb94402a
  - [ethtool-next,05/14] sff-8636: Rename SFF-8636 parsing functions
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=d7d15f737ab7
  - [ethtool-next,06/14] sff-8636: Initialize SFF-8636 memory map
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=4230597fe952
  - [ethtool-next,07/14] sff-8636: Use memory map during parsing
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=b74c040256de
  - [ethtool-next,08/14] sff-8636: Consolidate code between IOCTL and netlink paths
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=799572f86647
  - [ethtool-next,09/14] sff-8079: Split SFF-8079 parsing function
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=9fdf45ca1726
  - [ethtool-next,10/14] netlink: eeprom: Export a function to request an EEPROM page
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=2ccda2570d65
  - [ethtool-next,11/14] cmis: Request specific pages for parsing in netlink path
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=86792dbbebf3
  - [ethtool-next,12/14] sff-8636: Request specific pages for parsing in netlink path
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=6e2b32a0d0ea
  - [ethtool-next,13/14] sff-8079: Request specific pages for parsing in netlink path
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=c2170d40b6a1
  - [ethtool-next,14/14] netlink: eeprom: Defer page requests to individual parsers
    https://git.kernel.org/pub/scm/network/ethtool/ethtool.git/commit/?id=9538f384b535

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html