Skip to contentSkip to conversation

add module EEPROM ops for ice

PATCH Applied Steve Yang 4 participants 8 messages Jan 20, 2022 – Feb 3, 2022 Network drivers

lore
Share & tools
lore

Steve Yang

author
#0 lore raw

cover add module EEPROM ops for ice

Added the following 2 items of ice_eth_dev_ops for ice:
- ice_get_module_info
- ice_get_module_eeprom

Fixed stack overflow error when displaying a large size info.

Steve Yang (2):
net/ice: add module EEPROM ops for ice app/testpmd: fix stack overflow for EEPROM display

 app/test-pmd/config.c        |  22 ++++-
 drivers/net/ice/ice_ethdev.c | 160 +++++++++++++++++++++++++++++++++++
 drivers/net/ice/ice_ethdev.h |  25 ++++++
 3 files changed, 203 insertions(+), 4 deletions(-)
Signature · 1 line
2.27.0
author
#1 lore raw

PATCH 1/2 net/ice: add module EEPROM ops for ice

Add new callbacks for eth_dev_ops of ice to get the information and data of plugin module EEPROM.

Signed-off-by Steve Yang <[email protected]>
---
 drivers/net/ice/ice_ethdev.c | 160 +++++++++++++++++++++++++++++++++++
 drivers/net/ice/ice_ethdev.h |  25 ++++++
 2 files changed, 185 insertions(+)
drivers/net/ice/ice_ethdev.c +160 -0
index 13a7a9702a..70c162d0a8 100644
@@ -139,6 +139,10 @@ static int ice_vlan_pvid_set(struct rte_eth_dev *dev,
 static int ice_get_eeprom_length(struct rte_eth_dev *dev);
 static int ice_get_eeprom(struct rte_eth_dev *dev,
 			  struct rte_dev_eeprom_info *eeprom);
+static int ice_get_module_info(struct rte_eth_dev *dev,
+			       struct rte_eth_dev_module_info *modinfo);
+static int ice_get_module_eeprom(struct rte_eth_dev *dev,
+				 struct rte_dev_eeprom_info *info);
 static int ice_stats_get(struct rte_eth_dev *dev,
 			 struct rte_eth_stats *stats);
 static int ice_stats_reset(struct rte_eth_dev *dev);

@@ -238,6 +242,8 @@ static const struct eth_dev_ops ice_eth_dev_ops = {
 	.tx_burst_mode_get            = ice_tx_burst_mode_get,
 	.get_eeprom_length            = ice_get_eeprom_length,
 	.get_eeprom                   = ice_get_eeprom,
+	.get_module_info              = ice_get_module_info,
+	.get_module_eeprom            = ice_get_module_eeprom,
 	.stats_get                    = ice_stats_get,
 	.stats_reset                  = ice_stats_reset,
 	.xstats_get                   = ice_xstats_get,

@@ -4934,6 +4940,160 @@ ice_get_eeprom(struct rte_eth_dev *dev,
 	return 0;
 }
 
+static int
+ice_get_module_info(struct rte_eth_dev *dev,
+		    struct rte_eth_dev_module_info *modinfo)
+{
+	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+	enum ice_status status;
+	u8 sff8472_comp = 0;
+	u8 sff8472_swap = 0;
+	u8 sff8636_rev = 0;
+	u8 value = 0;
+
+	status = ice_aq_sff_eeprom(hw, 0, ICE_I2C_EEPROM_DEV_ADDR, 0x00, 0x00,
+				   0, &value, 1, 0, NULL);
+	if (status)
+		return -EIO;
+
+	switch (value) {
+	case ICE_MODULE_TYPE_SFP:
+		status = ice_aq_sff_eeprom(hw, 0, ICE_I2C_EEPROM_DEV_ADDR,
+					   ICE_MODULE_SFF_8472_COMP, 0x00, 0,
+					   &sff8472_comp, 1, 0, NULL);
+		if (status)
+			return -EIO;
+		status = ice_aq_sff_eeprom(hw, 0, ICE_I2C_EEPROM_DEV_ADDR,
+					   ICE_MODULE_SFF_8472_SWAP, 0x00, 0,
+					   &sff8472_swap, 1, 0, NULL);
+		if (status)
+			return -EIO;
+
+		if (sff8472_swap & ICE_MODULE_SFF_ADDR_MODE) {
+			modinfo->type = ICE_MODULE_SFF_8079;
+			modinfo->eeprom_len = ICE_MODULE_SFF_8079_LEN;
+		} else if (sff8472_comp &&
+			   (sff8472_swap & ICE_MODULE_SFF_DIAG_CAPAB)) {
+			modinfo->type = ICE_MODULE_SFF_8472;
+			modinfo->eeprom_len = ICE_MODULE_SFF_8472_LEN;
+		} else {
+			modinfo->type = ICE_MODULE_SFF_8079;
+			modinfo->eeprom_len = ICE_MODULE_SFF_8079_LEN;
+		}
+		break;
+	case ICE_MODULE_TYPE_QSFP_PLUS:
+	case ICE_MODULE_TYPE_QSFP28:
+		status = ice_aq_sff_eeprom(hw, 0, ICE_I2C_EEPROM_DEV_ADDR,
+					   ICE_MODULE_REVISION_ADDR, 0x00, 0,
+					   &sff8636_rev, 1, 0, NULL);
+		if (status)
+			return -EIO;
+		/* Check revision compliance */
+		if (sff8636_rev > 0x02) {
+			/* Module is SFF-8636 compliant */
+			modinfo->type = ICE_MODULE_SFF_8636;
+			modinfo->eeprom_len = ICE_MODULE_QSFP_MAX_LEN;
+		} else {
+			modinfo->type = ICE_MODULE_SFF_8436;
+			modinfo->eeprom_len = ICE_MODULE_QSFP_MAX_LEN;
+		}
+		break;
+	default:
+		PMD_DRV_LOG(WARNING, "SFF Module Type not recognized.\n");
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int
+ice_get_module_eeprom(struct rte_eth_dev *dev,
+		      struct rte_dev_eeprom_info *info)
+{
+	struct ice_hw *hw = ICE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
+#define SFF_READ_BLOCK_SIZE 8
+#define I2C_BUSY_TRY_TIMES 4
+#define I2C_USLEEP_MIN_TIME 1500
+#define I2C_USLEEP_MAX_TIME 2500
+	uint8_t value[SFF_READ_BLOCK_SIZE] = {0};
+	uint8_t addr = ICE_I2C_EEPROM_DEV_ADDR;
+	uint8_t *data = info->data;
+	enum ice_status status;
+	bool is_sfp = false;
+	uint32_t i, j;
+	uint32_t offset = 0;
+	uint8_t page = 0;
+
+	if (!info || !info->length || !data)
+		return -EINVAL;
+
+	status = ice_aq_sff_eeprom(hw, 0, addr, offset, page, 0, value, 1, 0,
+				   NULL);
+	if (status)
+		return -EIO;
+
+	if (value[0] == ICE_MODULE_TYPE_SFP)
+		is_sfp = true;
+
+	memset(data, 0, info->length);
+	for (i = 0; i < info->length; i += SFF_READ_BLOCK_SIZE) {
+		offset = i + info->offset;
+		page = 0;
+
+		/* Check if we need to access the other memory page */
+		if (is_sfp) {
+			if (offset >= ICE_MODULE_SFF_8079_LEN) {
+				offset -= ICE_MODULE_SFF_8079_LEN;
+				addr = ICE_I2C_EEPROM_DEV_ADDR2;
+			}
+		} else {
+			while (offset >= ICE_MODULE_SFF_8436_LEN) {
+				/* Compute memory page number and offset. */
+				offset -= ICE_MODULE_SFF_8436_LEN / 2;
+				page++;
+			}
+		}
+
+		/* Bit 2 of eeprom address 0x02 declares upper
+		 * pages are disabled on QSFP modules.
+		 * SFP modules only ever use page 0.
+		 */
+		if (page == 0 || !(data[0x2] & 0x4)) {
+			/* If i2c bus is busy due to slow page change or
+			 * link management access, call can fail.
+			 * This is normal. So we retry this a few times.
+			 */
+			for (j = 0; j < I2C_BUSY_TRY_TIMES; j++) {
+				status = ice_aq_sff_eeprom(hw, 0, addr, offset,
+							   page, !is_sfp, value,
+							   SFF_READ_BLOCK_SIZE,
+							   0, NULL);
+				PMD_DRV_LOG(DEBUG, "SFF %02X %02X %02X %X = "
+					"%02X%02X%02X%02X."
+					"%02X%02X%02X%02X (%X)\n",
+					addr, offset, page, is_sfp,
+					value[0], value[1],
+					value[2], value[3],
+					value[4], value[5],
+					value[6], value[7],
+					status);
+				if (status) {
+					usleep_range(I2C_USLEEP_MIN_TIME,
+						     I2C_USLEEP_MAX_TIME);
+					memset(value, 0, SFF_READ_BLOCK_SIZE);
+					continue;
+				}
+				break;
+			}
+
+			/* Make sure we have enough room for the new block */
+			if ((i + SFF_READ_BLOCK_SIZE) < info->length)
+				memcpy(data + i, value, SFF_READ_BLOCK_SIZE);
+		}
+	}
+
+	return 0;
+}
+
 static void
 ice_stat_update_32(struct ice_hw *hw,
 		   uint32_t reg,
drivers/net/ice/ice_ethdev.h +25 -0
index 1242177b42..3ed580d438 100644
@@ -143,6 +143,31 @@
 /* Max number of flexible descriptor rxdid */
 #define ICE_FLEX_DESC_RXDID_MAX_NUM 64
 
+#define ICE_I2C_EEPROM_DEV_ADDR		0xA0
+#define ICE_I2C_EEPROM_DEV_ADDR2	0xA2
+#define ICE_MODULE_TYPE_SFP		0x03
+#define ICE_MODULE_TYPE_QSFP_PLUS	0x0D
+#define ICE_MODULE_TYPE_QSFP28		0x11
+#define ICE_MODULE_SFF_ADDR_MODE	0x04
+#define ICE_MODULE_SFF_DIAG_CAPAB	0x40
+#define ICE_MODULE_REVISION_ADDR	0x01
+#define ICE_MODULE_SFF_8472_COMP	0x5E
+#define ICE_MODULE_SFF_8472_SWAP	0x5C
+#define ICE_MODULE_QSFP_MAX_LEN		640
+
+/* EEPROM Standards for plug in modules */
+#define ICE_MODULE_SFF_8079		0x1
+#define ICE_MODULE_SFF_8079_LEN		256
+#define ICE_MODULE_SFF_8472		0x2
+#define ICE_MODULE_SFF_8472_LEN		512
+#define ICE_MODULE_SFF_8636		0x3
+#define ICE_MODULE_SFF_8636_LEN		256
+#define ICE_MODULE_SFF_8636_MAX_LEN     640
+#define ICE_MODULE_SFF_8436		0x4
+#define ICE_MODULE_SFF_8436_LEN		256
+#define ICE_MODULE_SFF_8436_MAX_LEN     640
+
+
 /* Per-channel register definitions */
 #define GLTSYN_AUX_OUT(_chan, _idx)     (GLTSYN_AUX_OUT_0(_idx) + ((_chan) * 8))
 #define GLTSYN_CLKO(_chan, _idx)        (GLTSYN_CLKO_0(_idx) + ((_chan) * 8))
Signature · 1 line
2.27.0
Acked-by on v1 1/2 Applied
#2 lore raw

RE: [PATCH 1/2] net/ice: add module EEPROM ops for ice

Quoted 11 lines

-----Original Message-----
From: Yang, SteveX <[email protected]>
Sent: Thursday, January 20, 2022 11:00 AM
To: [email protected]
Cc: Zhang, Yuying <[email protected]>; Singh, Aman Deep <[email protected]>; Yang, Qiming <[email protected]>; Zhang, Qi Z <[email protected]>; Yang, SteveX <[email protected]>
Subject: [PATCH 1/2] net/ice: add module EEPROM ops for ice

Quoted 4 lines

Add new callbacks for eth_dev_ops of ice to get the information and data of plugin module EEPROM.

Signed-off-by Steve Yang <[email protected]>
Acked-by Qi Zhang <[email protected]>

Applied to dpdk-next-net-intel.

Thanks
Qi

#3 lore raw

Re: [PATCH 1/2] net/ice: add module EEPROM ops for ice

In reply to Steve Yang
Quoted 8 lines · Steve Yang

Add new callbacks for eth_dev_ops of ice to get the information and data of plugin module EEPROM.

Signed-off-by Steve Yang<[email protected]>
---
  drivers/net/ice/ice_ethdev.c | 160 +++++++++++++++++++++++++++++++++++
  drivers/net/ice/ice_ethdev.h |  25 ++++++
  2 files changed, 185 insertions(+)

In the 'doc/guides/nics/features/ice.ini' file, the "Module EEPROM dump" feature already documented as supported, but it is this patch add that support.

It looks like documenting "Module EEPROM dump" was mistake [1], intention was to document "EEPROM dump" feature.

Steve, can you please send another patch to fix it?

[1]
Fixes d0dd1c8e1997 ("net/ice: support EEPROM information getting")

Thanks,
ferruh

author
#4 lore raw

RE: [PATCH 1/2] net/ice: add module EEPROM ops for ice

Quoted 21 lines

-----Original Message-----
From: Yigit, Ferruh <[email protected]>
Sent: Tuesday, January 25, 2022 10:49 PM
To: Yang, SteveX <[email protected]>; [email protected]
Cc: Zhang, Yuying <[email protected]>; Singh, Aman Deep <[email protected]>; Yang, Qiming <[email protected]>; Zhang, Qi Z <[email protected]>; Lu, Wenzhuo <[email protected]>
Subject: Re: [PATCH 1/2] net/ice: add module EEPROM ops for ice

Quoted 6 lines · Steve Yang

Add new callbacks for eth_dev_ops of ice to get the information and data of plugin module EEPROM.

Signed-off-by Steve Yang<[email protected]>
---
  drivers/net/ice/ice_ethdev.c | 160

+++++++++++++++++++++++++++++++++++

Quoted 2 lines
  drivers/net/ice/ice_ethdev.h |  25 ++++++
  2 files changed, 185 insertions(+)
Quoted 8 lines

In the 'doc/guides/nics/features/ice.ini' file, the "Module EEPROM dump" feature already documented as supported, but it is this patch add that support.

It looks like documenting "Module EEPROM dump" was mistake [1], intention was to document "EEPROM dump" feature.

Steve, can you please send another patch to fix it?

Ok, I'll add "EEPROM dump" line into the ice.ini to fix it within another patch later. Thanks.

Quoted 9 lines
[1]
Fixes d0dd1c8e1997 ("net/ice: support EEPROM information getting")

Thanks,
ferruh

author
#5 lore raw

PATCH 2/2 app/testpmd: fix stack overflow for EEPROM display

In reply to Steve Yang

When the size of EEPROM exceeds the default thread stack size(8MB), e.g.: 10Mb size, it will be cashed with stack overflow.

Allocate the data of EPPROM information on the heap.

Fixes 6b67721dee2a ("app/testpmd: add EEPROM command")
Signed-off-by Steve Yang <[email protected]>
---
 app/test-pmd/config.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)
app/test-pmd/config.c +18 -4
index 1722d6c8f8..e812f57151 100644
@@ -912,10 +912,15 @@ port_eeprom_display(portid_t port_id)
 		return;
 	}
 
-	char buf[len_eeprom];
 	einfo.offset = 0;
 	einfo.length = len_eeprom;
-	einfo.data = buf;
+	einfo.data = calloc(1, len_eeprom);
+	if (!einfo.data) {
+		fprintf(stderr,
+			"Allocation of port %u eeprom data failed\n",
+			port_id);
+		return;
+	}
 
 	ret = rte_eth_dev_get_eeprom(port_id, &einfo);
 	if (ret != 0) {

@@ -933,10 +938,12 @@ port_eeprom_display(portid_t port_id)
 			fprintf(stderr, "Unable to get EEPROM: %d\n", ret);
 			break;
 		}
+		free(einfo.data);
 		return;
 	}
 	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
 	printf("Finish -- Port: %d EEPROM length: %d bytes\n", port_id, len_eeprom);
+	free(einfo.data);
 }
 
 void

@@ -972,10 +979,15 @@ port_module_eeprom_display(portid_t port_id)
 		return;
 	}
 
-	char buf[minfo.eeprom_len];
 	einfo.offset = 0;
 	einfo.length = minfo.eeprom_len;
-	einfo.data = buf;
+	einfo.data = calloc(1, minfo.eeprom_len);
+	if (!einfo.data) {
+		fprintf(stderr,
+			"Allocation of port %u eeprom data failed\n",
+			port_id);
+		return;
+	}
 
 	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
 	if (ret != 0) {

@@ -994,11 +1006,13 @@ port_module_eeprom_display(portid_t port_id)
 				ret);
 			break;
 		}
+		free(einfo.data);
 		return;
 	}
 
 	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
 	printf("Finish -- Port: %d MODULE EEPROM length: %d bytes\n", port_id, einfo.length);
+	free(einfo.data);
 }
 
 int
Signature · 1 line
2.27.0
Acked-by on v1 2/2
#6 lore raw

Re: [PATCH 2/2] app/testpmd: fix stack overflow for EEPROM display

Hi Steve
The patch looks ok to me.

Quoted 2 lines · Steve Yang

When the size of EEPROM exceeds the default thread stack size(8MB), e.g.: 10Mb size, it will be cashed with stack overflow.

Just spelling: 10Mb/10MB, cashed/crashed
Can even rephrase, like: it will crash due to stack overflow

Quoted 5 lines

Allocate the data of EPPROM information on the heap.

Fixes 6b67721dee2a ("app/testpmd: add EEPROM command")
Signed-off-by Steve Yang<[email protected]>
Acked-by Aman Singh <[email protected]>
Quoted 57 lines
---
  app/test-pmd/config.c | 22 ++++++++++++++++++----
  1 file changed, 18 insertions(+), 4 deletions(-)
app/test-pmd/config.c +8 -2
index 1722d6c8f8..e812f57151 100644
@@ -912,10 +912,15 @@ port_eeprom_display(portid_t port_id)
  		return;
  	}
 
-	char buf[len_eeprom];
  	einfo.offset = 0;
  	einfo.length = len_eeprom;
-	einfo.data = buf;
+	einfo.data = calloc(1, len_eeprom);
+	if (!einfo.data) {
+		fprintf(stderr,
+			"Allocation of port %u eeprom data failed\n",
+			port_id);
+		return;
+	}
 
  	ret = rte_eth_dev_get_eeprom(port_id, &einfo);
  	if (ret != 0) {

@@ -933,10 +938,12 @@ port_eeprom_display(portid_t port_id)
  			fprintf(stderr, "Unable to get EEPROM: %d\n", ret);
  			break;
  		}
+		free(einfo.data);
  		return;
  	}
  	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
  	printf("Finish -- Port: %d EEPROM length: %d bytes\n", port_id, len_eeprom); + free(einfo.data); } void @@ -972,10 +979,15

@@ port_module_eeprom_display(portid_t port_id) return; } - char
buf[minfo.eeprom_len]; einfo.offset = 0; einfo.length = minfo.eeprom_len; - einfo.data = buf; + einfo.data = calloc(1, minfo.eeprom_len); + if (!einfo.data) { + fprintf(stderr, + "Allocation of port %u eeprom data failed\n",

app/test-pmd/config.c +5 -0
+			port_id);
+		return;
+	}
 
  	ret = rte_eth_dev_get_module_eeprom(port_id, &einfo);
  	if (ret != 0) {

@@ -994,11 +1006,13 @@ port_module_eeprom_display(portid_t port_id)
  				ret);
  			break;
  		}
+		free(einfo.data);
  		return;
  	}
 
  	rte_hexdump(stdout, "hexdump", einfo.data, einfo.length);
  	printf("Finish -- Port: %d MODULE EEPROM length: %d bytes\n", port_id, einfo.length);
+	free(einfo.data);
  }
 
  int
Tested-by Applied
#7 lore raw

Re: [PATCH 2/2] app/testpmd: fix stack overflow for EEPROM display

Quoted 8 lines · Aman Deep

Hi Steve
The patch looks ok to me.

Quoted 2 lines · Steve Yang

When the size of EEPROM exceeds the default thread stack size(8MB), e.g.: 10Mb size, it will be cashed with stack overflow.

Just spelling: 10Mb/10MB, cashed/crashed
Can even rephrase, like: it will crash due to stack overflow

fixed while merging

Quoted 8 lines
Quoted 5 lines

Allocate the data of EPPROM information on the heap.

Fixes 6b67721dee2a ("app/testpmd: add EEPROM command")
Signed-off-by Steve Yang<[email protected]>
Acked-by Aman Singh <[email protected]>
Tested-by Ferruh Yigit <[email protected]>

Applied to dpdk-next-net/main, thanks.