Date:   Tue, 12 Oct 2021 16:25:22 +0300
From:   Ido Schimmel <idosch@...sch.org>
To:     netdev@...r.kernel.org
Cc:     mkubecek@...e.cz, popadrian1996@...il.com, andrew@...n.ch,
        mlxsw@...dia.com, moshe@...dia.com,
        Ido Schimmel <idosch@...dia.com>
Subject: [PATCH ethtool-next 11/14] cmis: Request specific pages for parsing in netlink path

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

Powered by blists - more mailing lists