DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] drivers/net: fix FW version get
@ 2021-04-21 16:20 Ferruh Yigit
  2021-04-21 16:30 ` Ajit Khaparde
                   ` (7 more replies)
  0 siblings, 8 replies; 11+ messages in thread
From: Ferruh Yigit @ 2021-04-21 16:20 UTC (permalink / raw)
  To: Igor Russkikh, Pavel Belous, Somalapuram Amaranath,
	Ajit Khaparde, Somnath Kotur, Hemant Agrawal, Sachin Saxena,
	Jeff Guo, Haiyue Wang, John Daley, Hyong Youb Kim,
	Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Beilei Xing, Qiming Yang, Qi Zhang,
	Andrew Boyer, Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K,
	Rasesh Mody, Devendra Singh Rawat, Andrew Rybchenko, Jiawen Wu,
	Jian Wang, Thomas Monjalon, Selwin Sebastian, Remy Horton,
	Chunsong Feng, Huisong Li, Hao Chen, Wei Hu (Xavier),
	Jingjing Wu, Wenzhuo Lu, Xiaoyun Li, Alvin Zhang, Shannon Nelson,
	Alfredo Cardigliano, Vamsi Attunuru, Yash Sharma, Ivan Malov,
	Andrew Lee
  Cc: Ferruh Yigit, dev, stable

Fixes a few different things:
* Remove 'fw_version' NULL checks, it is allowed if the 'fw_size' is
  zero, 'fw_version' being NULL but 'fw_size' not zero condition checked
  in ethdev layer
* Be sure required buffer size is returned if provided one is not big
  enough, instead of returning success (0)
* Document in doxygen comment the '-EINVAL' is a valid return type
* Take into account that 'snprintf' can return negative value
* Cast length to 'size_t' to compare it with 'fw_size'

Fixes: bb42aa9ffe4e ("net/atlantic: configure device start/stop")
Fixes: ff70acdf4299 ("net/axgbe: support reading FW version")
Fixes: e2652b0a20a0 ("net/bnxt: support get FW version")
Fixes: cf0fab1d2ca5 ("net/dpaa: support firmware version get API")
Fixes: 748eccb97cdc ("net/dpaa2: add support for firmware version get")
Fixes: b883c0644a24 ("net/e1000: add firmware version get")
Fixes: 293430677e9c ("net/enic: add handler to return firmware version")
Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
Fixes: bd5b86732bc7 ("net/hns3: modify format for firmware version")
Fixes: ed0dfdd0e976 ("net/i40e: add firmware version get")
Fixes: e31cb9a36298 ("net/ice: support FW version getting")
Fixes: 4f09bc55ac3d ("net/igc: implement device base operations")
Fixes: eec10fb0ce6b ("net/ionic: support FW version")
Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
Fixes: 4d9f5b8adc02 ("net/octeontx2: add FW version get operation")
Fixes: f97b56f9f12e ("net/qede: support FW version query")
Fixes: 83fef46a22b2 ("net/sfc: add callback to retrieve FW version")
Fixes: bc84ac0fadef ("net/txgbe: support getting FW version")
Fixes: 21913471202f ("ethdev: add firmware version get")
Cc: stable@dpdk.org

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
Cc: pavel.belous@aquantia.com
Cc: selwin.sebastian@amd.com
Cc: hemant.agrawal@nxp.com
Cc: qiming.yang@intel.com
Cc: hyonkim@cisco.com
Cc: xavier.huwei@huawei.com
Cc: wenzhuo.lu@intel.com
Cc: alvinx.zhang@intel.com
Cc: cardigliano@ntop.org
Cc: vattunuru@marvell.com
Cc: rmody@marvell.com
Cc: ivan.malov@oktetlabs.ru
Cc: jiawenwu@trustnetic.com
---
 drivers/net/atlantic/atl_ethdev.c       |  7 ++++---
 drivers/net/axgbe/axgbe_rxtx.c          |  4 ----
 drivers/net/bnxt/bnxt_ethdev.c          |  4 +++-
 drivers/net/dpaa/dpaa_ethdev.c          |  6 ++++--
 drivers/net/dpaa2/dpaa2_ethdev.c        |  4 +++-
 drivers/net/e1000/igb_ethdev.c          |  4 +++-
 drivers/net/enic/enic_ethdev.c          | 15 ++++++++++-----
 drivers/net/hns3/hns3_ethdev.c          |  5 ++++-
 drivers/net/hns3/hns3_ethdev_vf.c       |  5 ++++-
 drivers/net/i40e/i40e_ethdev.c          |  4 +++-
 drivers/net/ice/ice_ethdev.c            |  4 +++-
 drivers/net/igc/igc_ethdev.c            |  4 +++-
 drivers/net/ionic/ionic_ethdev.c        | 15 +++++++++------
 drivers/net/ixgbe/ixgbe_ethdev.c        |  4 +++-
 drivers/net/octeontx2/otx2_ethdev_ops.c |  2 +-
 drivers/net/qede/qede_ethdev.c          |  3 ---
 drivers/net/sfc/sfc_ethdev.c            |  8 --------
 drivers/net/txgbe/txgbe_ethdev.c        |  4 +++-
 lib/librte_ethdev/rte_ethdev.h          |  1 +
 19 files changed, 61 insertions(+), 42 deletions(-)

diff --git a/drivers/net/atlantic/atl_ethdev.c b/drivers/net/atlantic/atl_ethdev.c
index 473f6209f6aa..ce7f814f255d 100644
--- a/drivers/net/atlantic/atl_ethdev.c
+++ b/drivers/net/atlantic/atl_ethdev.c
@@ -1073,7 +1073,7 @@ atl_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
 {
 	struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	uint32_t fw_ver = 0;
-	unsigned int ret = 0;
+	int ret = 0;
 
 	ret = hw_atl_utils_get_fw_version(hw, &fw_ver);
 	if (ret)
@@ -1081,10 +1081,11 @@ atl_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
 
 	ret = snprintf(fw_version, fw_size, "%u.%u.%u", fw_ver >> 24,
 		       (fw_ver >> 16) & 0xFFU, fw_ver & 0xFFFFU);
+	if (ret < 0)
+		return -EINVAL;
 
 	ret += 1; /* add string null-terminator */
-
-	if (fw_size < ret)
+	if (fw_size < (size_t)ret)
 		return ret;
 
 	return 0;
diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
index e34bb6d448fc..33f709a6bb02 100644
--- a/drivers/net/axgbe/axgbe_rxtx.c
+++ b/drivers/net/axgbe/axgbe_rxtx.c
@@ -623,9 +623,6 @@ int axgbe_dev_fw_version_get(struct rte_eth_dev *eth_dev,
 	pdata = (struct axgbe_port *)eth_dev->data->dev_private;
 	hw_feat = &pdata->hw_feat;
 
-	if (fw_version == NULL)
-		return -EINVAL;
-
 	ret = snprintf(fw_version, fw_size, "%d.%d.%d",
 			AXGMAC_GET_BITS(hw_feat->version, MAC_VR, USERVER),
 			AXGMAC_GET_BITS(hw_feat->version, MAC_VR, DEVID),
@@ -634,7 +631,6 @@ int axgbe_dev_fw_version_get(struct rte_eth_dev *eth_dev,
 		return -EINVAL;
 
 	ret += 1; /* add the size of '\0' */
-
 	if (fw_size < (size_t)ret)
 		return ret;
 	else
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index f5d2dc85907d..dba5b9f94df4 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -2794,9 +2794,11 @@ bnxt_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
 
 	ret = snprintf(fw_version, fw_size, "%d.%d.%d.%d",
 			fw_major, fw_minor, fw_updt, fw_rsvd);
+	if (ret < 0)
+		return -EINVAL;
 
 	ret += 1; /* add the size of '\0' */
-	if (fw_size < (uint32_t)ret)
+	if (fw_size < (size_t)ret)
 		return ret;
 	else
 		return 0;
diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
index 176943a8eae6..57ee660c5e57 100644
--- a/drivers/net/dpaa/dpaa_ethdev.c
+++ b/drivers/net/dpaa/dpaa_ethdev.c
@@ -532,9 +532,11 @@ dpaa_fw_version_get(struct rte_eth_dev *dev __rte_unused,
 
 	ret = snprintf(fw_version, fw_size, "SVR:%x-fman-v%x",
 		       svr_ver, fman_ip_rev);
-	ret += 1; /* add the size of '\0' */
+	if (ret < 0)
+		return -EINVAL;
 
-	if (fw_size < (uint32_t)ret)
+	ret += 1; /* add the size of '\0' */
+	if (fw_size < (size_t)ret)
 		return ret;
 	else
 		return 0;
diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
index 9011dcfc1242..95e8d9abe869 100644
--- a/drivers/net/dpaa2/dpaa2_ethdev.c
+++ b/drivers/net/dpaa2/dpaa2_ethdev.c
@@ -226,9 +226,11 @@ dpaa2_fw_version_get(struct rte_eth_dev *dev,
 		       mc_ver_info.major,
 		       mc_ver_info.minor,
 		       mc_ver_info.revision);
+	if (ret < 0)
+		return -EINVAL;
 
 	ret += 1; /* add the size of '\0' */
-	if (fw_size < (uint32_t)ret)
+	if (fw_size < (size_t)ret)
 		return ret;
 	else
 		return 0;
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 3fdffe329483..10ee0f33415a 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -2159,9 +2159,11 @@ eth_igb_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
 		}
 		break;
 	}
+	if (ret < 0)
+		return -EINVAL;
 
 	ret += 1; /* add the size of '\0' */
-	if (fw_size < (u32)ret)
+	if (fw_size < (size_t)ret)
 		return ret;
 	else
 		return 0;
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index d91c2cdc8c54..e1a393b84762 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -1050,16 +1050,21 @@ static int enicpmd_dev_fw_version_get(struct rte_eth_dev *eth_dev,
 	int ret;
 
 	ENICPMD_FUNC_TRACE();
-	if (fw_version == NULL || fw_size <= 0)
-		return -EINVAL;
+
 	enic = pmd_priv(eth_dev);
 	ret = vnic_dev_fw_info(enic->vdev, &info);
 	if (ret)
 		return ret;
-	snprintf(fw_version, fw_size, "%s %s",
+	ret = snprintf(fw_version, fw_size, "%s %s",
 		 info->fw_version, info->fw_build);
-	fw_version[fw_size - 1] = '\0';
-	return 0;
+	if (ret < 0)
+		return -EINVAL;
+
+	ret += 1; /* add the size of '\0' */
+	if (fw_size < (size_t)ret)
+		return ret;
+	else
+		return 0;
 }
 
 static const struct eth_dev_ops enicpmd_eth_dev_ops = {
diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
index 60267e1ec115..bd0699af5844 100644
--- a/drivers/net/hns3/hns3_ethdev.c
+++ b/drivers/net/hns3/hns3_ethdev.c
@@ -2828,8 +2828,11 @@ hns3_fw_version_get(struct rte_eth_dev *eth_dev, char *fw_version,
 				      HNS3_FW_VERSION_BYTE1_S),
 		       hns3_get_field(version, HNS3_FW_VERSION_BYTE0_M,
 				      HNS3_FW_VERSION_BYTE0_S));
+	if (ret < 0)
+		return -EINVAL;
+
 	ret += 1; /* add the size of '\0' */
-	if (fw_size < (uint32_t)ret)
+	if (fw_size < (size_t)ret)
 		return ret;
 	else
 		return 0;
diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
index 7a7a6bfa7cc0..06a26fb9be92 100644
--- a/drivers/net/hns3/hns3_ethdev_vf.c
+++ b/drivers/net/hns3/hns3_ethdev_vf.c
@@ -2181,8 +2181,11 @@ hns3vf_fw_version_get(struct rte_eth_dev *eth_dev, char *fw_version,
 				      HNS3_FW_VERSION_BYTE1_S),
 		       hns3_get_field(version, HNS3_FW_VERSION_BYTE0_M,
 				      HNS3_FW_VERSION_BYTE0_S));
+	if (ret < 0)
+		return -EINVAL;
+
 	ret += 1; /* add the size of '\0' */
-	if (fw_size < (uint32_t)ret)
+	if (fw_size < (size_t)ret)
 		return ret;
 	else
 		return 0;
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index e6206a7e5100..66d23d698ea0 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -3687,9 +3687,11 @@ i40e_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
 		 ((hw->nvm.version >> 4) & 0xff),
 		 (hw->nvm.version & 0xf), hw->nvm.eetrack,
 		 ver, build, patch);
+	if (ret < 0)
+		return -EINVAL;
 
 	ret += 1; /* add the size of '\0' */
-	if (fw_size < (u32)ret)
+	if (fw_size < (size_t)ret)
 		return ret;
 	else
 		return 0;
diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
index da9e85bd7c9e..c37a2e09ceb0 100644
--- a/drivers/net/ice/ice_ethdev.c
+++ b/drivers/net/ice/ice_ethdev.c
@@ -4646,10 +4646,12 @@ ice_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
 			hw->flash.nvm.minor,
 			hw->flash.nvm.eetrack,
 			ver, build, patch);
+	if (ret < 0)
+		return -EINVAL;
 
 	/* add the size of '\0' */
 	ret += 1;
-	if (fw_size < (u32)ret)
+	if (fw_size < (size_t)ret)
 		return ret;
 	else
 		return 0;
diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c
index 31c99dca099f..b1c58fb3deb9 100644
--- a/drivers/net/igc/igc_ethdev.c
+++ b/drivers/net/igc/igc_ethdev.c
@@ -1476,9 +1476,11 @@ eth_igc_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
 				 fw.eep_build);
 		}
 	}
+	if (ret < 0)
+		return -EINVAL;
 
 	ret += 1; /* add the size of '\0' */
-	if (fw_size < (u32)ret)
+	if (fw_size < (size_t)ret)
 		return ret;
 	else
 		return 0;
diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
index cffe899c078c..5ec474dbf539 100644
--- a/drivers/net/ionic/ionic_ethdev.c
+++ b/drivers/net/ionic/ionic_ethdev.c
@@ -215,15 +215,18 @@ ionic_dev_fw_version_get(struct rte_eth_dev *eth_dev,
 {
 	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
 	struct ionic_adapter *adapter = lif->adapter;
+	int ret;
 
-	if (fw_version == NULL || fw_size <= 0)
-		return -EINVAL;
-
-	snprintf(fw_version, fw_size, "%s",
+	ret = snprintf(fw_version, fw_size, "%s",
 		 adapter->fw_version);
-	fw_version[fw_size - 1] = '\0';
+	if (ret < 0)
+		return -EINVAL;
 
-	return 0;
+	ret += 1; /* add the size of '\0' */
+	if (fw_size < (size_t)ret)
+		return ret;
+	else
+		return 0;
 }
 
 /*
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index ff65145f5526..a5c191fe1b17 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -3814,9 +3814,11 @@ ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
 
 	etrack_id = (eeprom_verh << 16) | eeprom_verl;
 	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
+	if (ret < 0)
+		return -EINVAL;
 
 	ret += 1; /* add the size of '\0' */
-	if (fw_size < (u32)ret)
+	if (fw_size < (size_t)ret)
 		return ret;
 	else
 		return 0;
diff --git a/drivers/net/octeontx2/otx2_ethdev_ops.c b/drivers/net/octeontx2/otx2_ethdev_ops.c
index 6f0cdc585422..5a4501208e9e 100644
--- a/drivers/net/octeontx2/otx2_ethdev_ops.c
+++ b/drivers/net/octeontx2/otx2_ethdev_ops.c
@@ -453,7 +453,7 @@ otx2_nix_fw_version_get(struct rte_eth_dev *eth_dev, char *fw_version,
 	rc = strlcpy(fw_version, (char *)dev->mkex_pfl_name, rc);
 
 	rc += 1; /* Add the size of '\0' */
-	if (fw_size < (uint32_t)rc)
+	if (fw_size < (size_t)rc)
 		return rc;
 
 	return 0;
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index 057a7b00e2dc..c2c8de867501 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -237,9 +237,6 @@ qede_fw_version_get(struct rte_eth_dev *dev, char *fw_ver, size_t fw_size)
 	static char ver_str[QEDE_PMD_DRV_VER_STR_SIZE];
 	size_t size;
 
-	if (fw_ver == NULL)
-		return 0;
-
 	if (IS_PF(edev))
 		snprintf(ver_str, QEDE_PMD_DRV_VER_STR_SIZE, "%s",
 			 QEDE_PMD_FW_VERSION);
diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
index 52ba7b67b052..c50ecea0b993 100644
--- a/drivers/net/sfc/sfc_ethdev.c
+++ b/drivers/net/sfc/sfc_ethdev.c
@@ -46,14 +46,6 @@ sfc_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
 	int ret;
 	int rc;
 
-	/*
-	 * Return value of the callback is likely supposed to be
-	 * equal to or greater than 0, nevertheless, if an error
-	 * occurs, it will be desirable to pass it to the caller
-	 */
-	if ((fw_version == NULL) || (fw_size == 0))
-		return -EINVAL;
-
 	rc = efx_nic_get_fw_version(sa->nic, &enfi);
 	if (rc != 0)
 		return -rc;
diff --git a/drivers/net/txgbe/txgbe_ethdev.c b/drivers/net/txgbe/txgbe_ethdev.c
index 97796f040b43..8dbe3da5c2c9 100644
--- a/drivers/net/txgbe/txgbe_ethdev.c
+++ b/drivers/net/txgbe/txgbe_ethdev.c
@@ -2582,9 +2582,11 @@ txgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
 	hw->phy.get_fw_version(hw, &etrack_id);
 
 	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
+	if (ret < 0)
+		return -EINVAL;
 
 	ret += 1; /* add the size of '\0' */
-	if (fw_size < (u32)ret)
+	if (fw_size < (size_t)ret)
 		return ret;
 	else
 		return 0;
diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
index a0d01d26675f..80841b44b680 100644
--- a/lib/librte_ethdev/rte_ethdev.h
+++ b/lib/librte_ethdev/rte_ethdev.h
@@ -3069,6 +3069,7 @@ int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
  *   - (-ENOTSUP) if operation is not supported.
  *   - (-ENODEV) if *port_id* invalid.
  *   - (-EIO) if device is removed.
+ *   - (-EINVAL) if failed to get version.
  *   - (>0) if *fw_size* is not enough to store firmware version, return
  *          the size of the non truncated string.
  */
-- 
2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] drivers/net: fix FW version get
  2021-04-21 16:20 [dpdk-dev] [PATCH] drivers/net: fix FW version get Ferruh Yigit
@ 2021-04-21 16:30 ` Ajit Khaparde
  2021-04-22  8:53   ` Andrew Rybchenko
  2021-04-22 11:44 ` Wang, Haiyue
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 11+ messages in thread
From: Ajit Khaparde @ 2021-04-21 16:30 UTC (permalink / raw)
  To: Ferruh Yigit
  Cc: Igor Russkikh, Pavel Belous, Somalapuram Amaranath,
	Somnath Kotur, Hemant Agrawal, Sachin Saxena, Jeff Guo,
	Haiyue Wang, John Daley, Hyong Youb Kim, Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Beilei Xing, Qiming Yang, Qi Zhang,
	Andrew Boyer, Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K,
	Rasesh Mody, Devendra Singh Rawat, Andrew Rybchenko, Jiawen Wu,
	Jian Wang, Thomas Monjalon, Selwin Sebastian, Remy Horton,
	Chunsong Feng, Huisong Li, Hao Chen, Wei Hu (Xavier),
	Jingjing Wu, Wenzhuo Lu, Xiaoyun Li, Alvin Zhang, Shannon Nelson,
	Alfredo Cardigliano, Vamsi Attunuru, Yash Sharma, Ivan Malov,
	Andrew Lee, dpdk-dev, dpdk stable

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

On Wed, Apr 21, 2021 at 9:21 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>
> Fixes a few different things:
> * Remove 'fw_version' NULL checks, it is allowed if the 'fw_size' is
>   zero, 'fw_version' being NULL but 'fw_size' not zero condition checked
>   in ethdev layer
> * Be sure required buffer size is returned if provided one is not big
>   enough, instead of returning success (0)
> * Document in doxygen comment the '-EINVAL' is a valid return type
> * Take into account that 'snprintf' can return negative value
> * Cast length to 'size_t' to compare it with 'fw_size'
>
> Fixes: bb42aa9ffe4e ("net/atlantic: configure device start/stop")
> Fixes: ff70acdf4299 ("net/axgbe: support reading FW version")
> Fixes: e2652b0a20a0 ("net/bnxt: support get FW version")
> Fixes: cf0fab1d2ca5 ("net/dpaa: support firmware version get API")
> Fixes: 748eccb97cdc ("net/dpaa2: add support for firmware version get")
> Fixes: b883c0644a24 ("net/e1000: add firmware version get")
> Fixes: 293430677e9c ("net/enic: add handler to return firmware version")
> Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
> Fixes: bd5b86732bc7 ("net/hns3: modify format for firmware version")
> Fixes: ed0dfdd0e976 ("net/i40e: add firmware version get")
> Fixes: e31cb9a36298 ("net/ice: support FW version getting")
> Fixes: 4f09bc55ac3d ("net/igc: implement device base operations")
> Fixes: eec10fb0ce6b ("net/ionic: support FW version")
> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> Fixes: 4d9f5b8adc02 ("net/octeontx2: add FW version get operation")
> Fixes: f97b56f9f12e ("net/qede: support FW version query")
> Fixes: 83fef46a22b2 ("net/sfc: add callback to retrieve FW version")
> Fixes: bc84ac0fadef ("net/txgbe: support getting FW version")
> Fixes: 21913471202f ("ethdev: add firmware version get")
> Cc: stable@dpdk.org
>
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
Thanks!
For bnxt,
Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> ---
> Cc: pavel.belous@aquantia.com
> Cc: selwin.sebastian@amd.com
> Cc: hemant.agrawal@nxp.com
> Cc: qiming.yang@intel.com
> Cc: hyonkim@cisco.com
> Cc: xavier.huwei@huawei.com
> Cc: wenzhuo.lu@intel.com
> Cc: alvinx.zhang@intel.com
> Cc: cardigliano@ntop.org
> Cc: vattunuru@marvell.com
> Cc: rmody@marvell.com
> Cc: ivan.malov@oktetlabs.ru
> Cc: jiawenwu@trustnetic.com
> ---
>  drivers/net/atlantic/atl_ethdev.c       |  7 ++++---
>  drivers/net/axgbe/axgbe_rxtx.c          |  4 ----
>  drivers/net/bnxt/bnxt_ethdev.c          |  4 +++-
>  drivers/net/dpaa/dpaa_ethdev.c          |  6 ++++--
>  drivers/net/dpaa2/dpaa2_ethdev.c        |  4 +++-
>  drivers/net/e1000/igb_ethdev.c          |  4 +++-
>  drivers/net/enic/enic_ethdev.c          | 15 ++++++++++-----
>  drivers/net/hns3/hns3_ethdev.c          |  5 ++++-
>  drivers/net/hns3/hns3_ethdev_vf.c       |  5 ++++-
>  drivers/net/i40e/i40e_ethdev.c          |  4 +++-
>  drivers/net/ice/ice_ethdev.c            |  4 +++-
>  drivers/net/igc/igc_ethdev.c            |  4 +++-
>  drivers/net/ionic/ionic_ethdev.c        | 15 +++++++++------
>  drivers/net/ixgbe/ixgbe_ethdev.c        |  4 +++-
>  drivers/net/octeontx2/otx2_ethdev_ops.c |  2 +-
>  drivers/net/qede/qede_ethdev.c          |  3 ---
>  drivers/net/sfc/sfc_ethdev.c            |  8 --------
>  drivers/net/txgbe/txgbe_ethdev.c        |  4 +++-
>  lib/librte_ethdev/rte_ethdev.h          |  1 +
>  19 files changed, 61 insertions(+), 42 deletions(-)
>
> diff --git a/drivers/net/atlantic/atl_ethdev.c b/drivers/net/atlantic/atl_ethdev.c
> index 473f6209f6aa..ce7f814f255d 100644
> --- a/drivers/net/atlantic/atl_ethdev.c
> +++ b/drivers/net/atlantic/atl_ethdev.c
> @@ -1073,7 +1073,7 @@ atl_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
>  {
>         struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data->dev_private);
>         uint32_t fw_ver = 0;
> -       unsigned int ret = 0;
> +       int ret = 0;
>
>         ret = hw_atl_utils_get_fw_version(hw, &fw_ver);
>         if (ret)
> @@ -1081,10 +1081,11 @@ atl_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
>
>         ret = snprintf(fw_version, fw_size, "%u.%u.%u", fw_ver >> 24,
>                        (fw_ver >> 16) & 0xFFU, fw_ver & 0xFFFFU);
> +       if (ret < 0)
> +               return -EINVAL;
>
>         ret += 1; /* add string null-terminator */
> -
> -       if (fw_size < ret)
> +       if (fw_size < (size_t)ret)
>                 return ret;
>
>         return 0;
> diff --git a/drivers/net/axgbe/axgbe_rxtx.c b/drivers/net/axgbe/axgbe_rxtx.c
> index e34bb6d448fc..33f709a6bb02 100644
> --- a/drivers/net/axgbe/axgbe_rxtx.c
> +++ b/drivers/net/axgbe/axgbe_rxtx.c
> @@ -623,9 +623,6 @@ int axgbe_dev_fw_version_get(struct rte_eth_dev *eth_dev,
>         pdata = (struct axgbe_port *)eth_dev->data->dev_private;
>         hw_feat = &pdata->hw_feat;
>
> -       if (fw_version == NULL)
> -               return -EINVAL;
> -
>         ret = snprintf(fw_version, fw_size, "%d.%d.%d",
>                         AXGMAC_GET_BITS(hw_feat->version, MAC_VR, USERVER),
>                         AXGMAC_GET_BITS(hw_feat->version, MAC_VR, DEVID),
> @@ -634,7 +631,6 @@ int axgbe_dev_fw_version_get(struct rte_eth_dev *eth_dev,
>                 return -EINVAL;
>
>         ret += 1; /* add the size of '\0' */
> -
>         if (fw_size < (size_t)ret)
>                 return ret;
>         else
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
> index f5d2dc85907d..dba5b9f94df4 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -2794,9 +2794,11 @@ bnxt_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
>
>         ret = snprintf(fw_version, fw_size, "%d.%d.%d.%d",
>                         fw_major, fw_minor, fw_updt, fw_rsvd);
> +       if (ret < 0)
> +               return -EINVAL;
>
>         ret += 1; /* add the size of '\0' */
> -       if (fw_size < (uint32_t)ret)
> +       if (fw_size < (size_t)ret)
>                 return ret;
>         else
>                 return 0;
> diff --git a/drivers/net/dpaa/dpaa_ethdev.c b/drivers/net/dpaa/dpaa_ethdev.c
> index 176943a8eae6..57ee660c5e57 100644
> --- a/drivers/net/dpaa/dpaa_ethdev.c
> +++ b/drivers/net/dpaa/dpaa_ethdev.c
> @@ -532,9 +532,11 @@ dpaa_fw_version_get(struct rte_eth_dev *dev __rte_unused,
>
>         ret = snprintf(fw_version, fw_size, "SVR:%x-fman-v%x",
>                        svr_ver, fman_ip_rev);
> -       ret += 1; /* add the size of '\0' */
> +       if (ret < 0)
> +               return -EINVAL;
>
> -       if (fw_size < (uint32_t)ret)
> +       ret += 1; /* add the size of '\0' */
> +       if (fw_size < (size_t)ret)
>                 return ret;
>         else
>                 return 0;
> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c b/drivers/net/dpaa2/dpaa2_ethdev.c
> index 9011dcfc1242..95e8d9abe869 100644
> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
> @@ -226,9 +226,11 @@ dpaa2_fw_version_get(struct rte_eth_dev *dev,
>                        mc_ver_info.major,
>                        mc_ver_info.minor,
>                        mc_ver_info.revision);
> +       if (ret < 0)
> +               return -EINVAL;
>
>         ret += 1; /* add the size of '\0' */
> -       if (fw_size < (uint32_t)ret)
> +       if (fw_size < (size_t)ret)
>                 return ret;
>         else
>                 return 0;
> diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
> index 3fdffe329483..10ee0f33415a 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -2159,9 +2159,11 @@ eth_igb_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
>                 }
>                 break;
>         }
> +       if (ret < 0)
> +               return -EINVAL;
>
>         ret += 1; /* add the size of '\0' */
> -       if (fw_size < (u32)ret)
> +       if (fw_size < (size_t)ret)
>                 return ret;
>         else
>                 return 0;
> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
> index d91c2cdc8c54..e1a393b84762 100644
> --- a/drivers/net/enic/enic_ethdev.c
> +++ b/drivers/net/enic/enic_ethdev.c
> @@ -1050,16 +1050,21 @@ static int enicpmd_dev_fw_version_get(struct rte_eth_dev *eth_dev,
>         int ret;
>
>         ENICPMD_FUNC_TRACE();
> -       if (fw_version == NULL || fw_size <= 0)
> -               return -EINVAL;
> +
>         enic = pmd_priv(eth_dev);
>         ret = vnic_dev_fw_info(enic->vdev, &info);
>         if (ret)
>                 return ret;
> -       snprintf(fw_version, fw_size, "%s %s",
> +       ret = snprintf(fw_version, fw_size, "%s %s",
>                  info->fw_version, info->fw_build);
> -       fw_version[fw_size - 1] = '\0';
> -       return 0;
> +       if (ret < 0)
> +               return -EINVAL;
> +
> +       ret += 1; /* add the size of '\0' */
> +       if (fw_size < (size_t)ret)
> +               return ret;
> +       else
> +               return 0;
>  }
>
>  static const struct eth_dev_ops enicpmd_eth_dev_ops = {
> diff --git a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c
> index 60267e1ec115..bd0699af5844 100644
> --- a/drivers/net/hns3/hns3_ethdev.c
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -2828,8 +2828,11 @@ hns3_fw_version_get(struct rte_eth_dev *eth_dev, char *fw_version,
>                                       HNS3_FW_VERSION_BYTE1_S),
>                        hns3_get_field(version, HNS3_FW_VERSION_BYTE0_M,
>                                       HNS3_FW_VERSION_BYTE0_S));
> +       if (ret < 0)
> +               return -EINVAL;
> +
>         ret += 1; /* add the size of '\0' */
> -       if (fw_size < (uint32_t)ret)
> +       if (fw_size < (size_t)ret)
>                 return ret;
>         else
>                 return 0;
> diff --git a/drivers/net/hns3/hns3_ethdev_vf.c b/drivers/net/hns3/hns3_ethdev_vf.c
> index 7a7a6bfa7cc0..06a26fb9be92 100644
> --- a/drivers/net/hns3/hns3_ethdev_vf.c
> +++ b/drivers/net/hns3/hns3_ethdev_vf.c
> @@ -2181,8 +2181,11 @@ hns3vf_fw_version_get(struct rte_eth_dev *eth_dev, char *fw_version,
>                                       HNS3_FW_VERSION_BYTE1_S),
>                        hns3_get_field(version, HNS3_FW_VERSION_BYTE0_M,
>                                       HNS3_FW_VERSION_BYTE0_S));
> +       if (ret < 0)
> +               return -EINVAL;
> +
>         ret += 1; /* add the size of '\0' */
> -       if (fw_size < (uint32_t)ret)
> +       if (fw_size < (size_t)ret)
>                 return ret;
>         else
>                 return 0;
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index e6206a7e5100..66d23d698ea0 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -3687,9 +3687,11 @@ i40e_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
>                  ((hw->nvm.version >> 4) & 0xff),
>                  (hw->nvm.version & 0xf), hw->nvm.eetrack,
>                  ver, build, patch);
> +       if (ret < 0)
> +               return -EINVAL;
>
>         ret += 1; /* add the size of '\0' */
> -       if (fw_size < (u32)ret)
> +       if (fw_size < (size_t)ret)
>                 return ret;
>         else
>                 return 0;
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> index da9e85bd7c9e..c37a2e09ceb0 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -4646,10 +4646,12 @@ ice_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
>                         hw->flash.nvm.minor,
>                         hw->flash.nvm.eetrack,
>                         ver, build, patch);
> +       if (ret < 0)
> +               return -EINVAL;
>
>         /* add the size of '\0' */
>         ret += 1;
> -       if (fw_size < (u32)ret)
> +       if (fw_size < (size_t)ret)
>                 return ret;
>         else
>                 return 0;
> diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c
> index 31c99dca099f..b1c58fb3deb9 100644
> --- a/drivers/net/igc/igc_ethdev.c
> +++ b/drivers/net/igc/igc_ethdev.c
> @@ -1476,9 +1476,11 @@ eth_igc_fw_version_get(struct rte_eth_dev *dev, char *fw_version,
>                                  fw.eep_build);
>                 }
>         }
> +       if (ret < 0)
> +               return -EINVAL;
>
>         ret += 1; /* add the size of '\0' */
> -       if (fw_size < (u32)ret)
> +       if (fw_size < (size_t)ret)
>                 return ret;
>         else
>                 return 0;
> diff --git a/drivers/net/ionic/ionic_ethdev.c b/drivers/net/ionic/ionic_ethdev.c
> index cffe899c078c..5ec474dbf539 100644
> --- a/drivers/net/ionic/ionic_ethdev.c
> +++ b/drivers/net/ionic/ionic_ethdev.c
> @@ -215,15 +215,18 @@ ionic_dev_fw_version_get(struct rte_eth_dev *eth_dev,
>  {
>         struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
>         struct ionic_adapter *adapter = lif->adapter;
> +       int ret;
>
> -       if (fw_version == NULL || fw_size <= 0)
> -               return -EINVAL;
> -
> -       snprintf(fw_version, fw_size, "%s",
> +       ret = snprintf(fw_version, fw_size, "%s",
>                  adapter->fw_version);
> -       fw_version[fw_size - 1] = '\0';
> +       if (ret < 0)
> +               return -EINVAL;
>
> -       return 0;
> +       ret += 1; /* add the size of '\0' */
> +       if (fw_size < (size_t)ret)
> +               return ret;
> +       else
> +               return 0;
>  }
>
>  /*
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
> index ff65145f5526..a5c191fe1b17 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -3814,9 +3814,11 @@ ixgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
>
>         etrack_id = (eeprom_verh << 16) | eeprom_verl;
>         ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
> +       if (ret < 0)
> +               return -EINVAL;
>
>         ret += 1; /* add the size of '\0' */
> -       if (fw_size < (u32)ret)
> +       if (fw_size < (size_t)ret)
>                 return ret;
>         else
>                 return 0;
> diff --git a/drivers/net/octeontx2/otx2_ethdev_ops.c b/drivers/net/octeontx2/otx2_ethdev_ops.c
> index 6f0cdc585422..5a4501208e9e 100644
> --- a/drivers/net/octeontx2/otx2_ethdev_ops.c
> +++ b/drivers/net/octeontx2/otx2_ethdev_ops.c
> @@ -453,7 +453,7 @@ otx2_nix_fw_version_get(struct rte_eth_dev *eth_dev, char *fw_version,
>         rc = strlcpy(fw_version, (char *)dev->mkex_pfl_name, rc);
>
>         rc += 1; /* Add the size of '\0' */
> -       if (fw_size < (uint32_t)rc)
> +       if (fw_size < (size_t)rc)
>                 return rc;
>
>         return 0;
> diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
> index 057a7b00e2dc..c2c8de867501 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -237,9 +237,6 @@ qede_fw_version_get(struct rte_eth_dev *dev, char *fw_ver, size_t fw_size)
>         static char ver_str[QEDE_PMD_DRV_VER_STR_SIZE];
>         size_t size;
>
> -       if (fw_ver == NULL)
> -               return 0;
> -
>         if (IS_PF(edev))
>                 snprintf(ver_str, QEDE_PMD_DRV_VER_STR_SIZE, "%s",
>                          QEDE_PMD_FW_VERSION);
> diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
> index 52ba7b67b052..c50ecea0b993 100644
> --- a/drivers/net/sfc/sfc_ethdev.c
> +++ b/drivers/net/sfc/sfc_ethdev.c
> @@ -46,14 +46,6 @@ sfc_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
>         int ret;
>         int rc;
>
> -       /*
> -        * Return value of the callback is likely supposed to be
> -        * equal to or greater than 0, nevertheless, if an error
> -        * occurs, it will be desirable to pass it to the caller
> -        */
> -       if ((fw_version == NULL) || (fw_size == 0))
> -               return -EINVAL;
> -
>         rc = efx_nic_get_fw_version(sa->nic, &enfi);
>         if (rc != 0)
>                 return -rc;
> diff --git a/drivers/net/txgbe/txgbe_ethdev.c b/drivers/net/txgbe/txgbe_ethdev.c
> index 97796f040b43..8dbe3da5c2c9 100644
> --- a/drivers/net/txgbe/txgbe_ethdev.c
> +++ b/drivers/net/txgbe/txgbe_ethdev.c
> @@ -2582,9 +2582,11 @@ txgbe_fw_version_get(struct rte_eth_dev *dev, char *fw_version, size_t fw_size)
>         hw->phy.get_fw_version(hw, &etrack_id);
>
>         ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
> +       if (ret < 0)
> +               return -EINVAL;
>
>         ret += 1; /* add the size of '\0' */
> -       if (fw_size < (u32)ret)
> +       if (fw_size < (size_t)ret)
>                 return ret;
>         else
>                 return 0;
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index a0d01d26675f..80841b44b680 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -3069,6 +3069,7 @@ int rte_eth_dev_info_get(uint16_t port_id, struct rte_eth_dev_info *dev_info);
>   *   - (-ENOTSUP) if operation is not supported.
>   *   - (-ENODEV) if *port_id* invalid.
>   *   - (-EIO) if device is removed.
> + *   - (-EINVAL) if failed to get version.
>   *   - (>0) if *fw_size* is not enough to store firmware version, return
>   *          the size of the non truncated string.
>   */
> --
> 2.30.2
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] drivers/net: fix FW version get
  2021-04-21 16:30 ` Ajit Khaparde
@ 2021-04-22  8:53   ` Andrew Rybchenko
  0 siblings, 0 replies; 11+ messages in thread
From: Andrew Rybchenko @ 2021-04-22  8:53 UTC (permalink / raw)
  To: Ajit Khaparde, Ferruh Yigit
  Cc: Igor Russkikh, Pavel Belous, Somalapuram Amaranath,
	Somnath Kotur, Hemant Agrawal, Sachin Saxena, Jeff Guo,
	Haiyue Wang, John Daley, Hyong Youb Kim, Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Beilei Xing, Qiming Yang, Qi Zhang,
	Andrew Boyer, Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K,
	Rasesh Mody, Devendra Singh Rawat, Jiawen Wu, Jian Wang,
	Thomas Monjalon, Selwin Sebastian, Remy Horton, Chunsong Feng,
	Huisong Li, Hao Chen, Wei Hu (Xavier),
	Jingjing Wu, Wenzhuo Lu, Xiaoyun Li, Alvin Zhang, Shannon Nelson,
	Alfredo Cardigliano, Vamsi Attunuru, Yash Sharma, Ivan Malov,
	Andrew Lee, dpdk-dev, dpdk stable

On 4/21/21 7:30 PM, Ajit Khaparde wrote:
> On Wed, Apr 21, 2021 at 9:21 AM Ferruh Yigit <ferruh.yigit@intel.com> wrote:
>>
>> Fixes a few different things:
>> * Remove 'fw_version' NULL checks, it is allowed if the 'fw_size' is
>>   zero, 'fw_version' being NULL but 'fw_size' not zero condition checked
>>   in ethdev layer
>> * Be sure required buffer size is returned if provided one is not big
>>   enough, instead of returning success (0)
>> * Document in doxygen comment the '-EINVAL' is a valid return type
>> * Take into account that 'snprintf' can return negative value
>> * Cast length to 'size_t' to compare it with 'fw_size'
>>
>> Fixes: bb42aa9ffe4e ("net/atlantic: configure device start/stop")
>> Fixes: ff70acdf4299 ("net/axgbe: support reading FW version")
>> Fixes: e2652b0a20a0 ("net/bnxt: support get FW version")
>> Fixes: cf0fab1d2ca5 ("net/dpaa: support firmware version get API")
>> Fixes: 748eccb97cdc ("net/dpaa2: add support for firmware version get")
>> Fixes: b883c0644a24 ("net/e1000: add firmware version get")
>> Fixes: 293430677e9c ("net/enic: add handler to return firmware version")
>> Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
>> Fixes: bd5b86732bc7 ("net/hns3: modify format for firmware version")
>> Fixes: ed0dfdd0e976 ("net/i40e: add firmware version get")
>> Fixes: e31cb9a36298 ("net/ice: support FW version getting")
>> Fixes: 4f09bc55ac3d ("net/igc: implement device base operations")
>> Fixes: eec10fb0ce6b ("net/ionic: support FW version")
>> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
>> Fixes: 4d9f5b8adc02 ("net/octeontx2: add FW version get operation")
>> Fixes: f97b56f9f12e ("net/qede: support FW version query")
>> Fixes: 83fef46a22b2 ("net/sfc: add callback to retrieve FW version")
>> Fixes: bc84ac0fadef ("net/txgbe: support getting FW version")
>> Fixes: 21913471202f ("ethdev: add firmware version get")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> Thanks!
> For bnxt,
> Acked-by: Ajit Khaparde <ajit.khaparde@broadcom.com>

For net/sfc,
Acked-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] drivers/net: fix FW version get
  2021-04-21 16:20 [dpdk-dev] [PATCH] drivers/net: fix FW version get Ferruh Yigit
  2021-04-21 16:30 ` Ajit Khaparde
@ 2021-04-22 11:44 ` Wang, Haiyue
  2021-04-22 17:01 ` [dpdk-dev] [EXT] " Rasesh Mody
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Wang, Haiyue @ 2021-04-22 11:44 UTC (permalink / raw)
  To: Yigit, Ferruh, Igor Russkikh, Pavel Belous,
	Somalapuram Amaranath, Ajit Khaparde, Somnath Kotur,
	Hemant Agrawal, Sachin Saxena, Guo, Jia, Daley, John,
	Hyong Youb Kim, Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Xing, Beilei, Yang, Qiming, Zhang, Qi Z,
	Andrew Boyer, Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K,
	Rasesh Mody, Devendra Singh Rawat, Andrew Rybchenko, Jiawen Wu,
	Jian Wang, Thomas Monjalon, Selwin Sebastian, Remy Horton,
	Chunsong Feng, Huisong Li, Hao Chen, Wei Hu (Xavier),
	Wu, Jingjing, Lu, Wenzhuo, Li, Xiaoyun, Zhang, AlvinX,
	Shannon Nelson, Alfredo Cardigliano, Vamsi Attunuru, Yash Sharma,
	Ivan Malov, Andrew Lee
  Cc: dev, stable

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Thursday, April 22, 2021 00:21
> To: Igor Russkikh <igor.russkikh@aquantia.com>; Pavel Belous <pavel.belous@aquantia.com>; Somalapuram
> Amaranath <asomalap@amd.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>; Somnath Kotur
> <somnath.kotur@broadcom.com>; Hemant Agrawal <hemant.agrawal@nxp.com>; Sachin Saxena
> <sachin.saxena@oss.nxp.com>; Guo, Jia <jia.guo@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>; Daley,
> John <johndale@cisco.com>; Hyong Youb Kim <hyonkim@cisco.com>; Min Hu (Connor) <humin29@huawei.com>;
> Yisen Zhuang <yisen.zhuang@huawei.com>; Lijun Ou <oulijun@huawei.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Andrew Boyer <aboyer@pensando.io>; Jerin Jacob <jerinj@marvell.com>; Nithin Dabilpuram
> <ndabilpuram@marvell.com>; Kiran Kumar K <kirankumark@marvell.com>; Rasesh Mody <rmody@marvell.com>;
> Devendra Singh Rawat <dsinghrawat@marvell.com>; Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>;
> Jiawen Wu <jiawenwu@trustnetic.com>; Jian Wang <jianwang@trustnetic.com>; Thomas Monjalon
> <thomas@monjalon.net>; Selwin Sebastian <selwin.sebastian@amd.com>; Remy Horton
> <remy.horton@intel.com>; Chunsong Feng <fengchunsong@huawei.com>; Huisong Li <lihuisong@huawei.com>;
> Hao Chen <chenhao164@huawei.com>; Wei Hu (Xavier) <xavier.huwei@huawei.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>; Zhang,
> AlvinX <alvinx.zhang@intel.com>; Shannon Nelson <snelson@pensando.io>; Alfredo Cardigliano
> <cardigliano@ntop.org>; Vamsi Attunuru <vattunuru@marvell.com>; Yash Sharma <ysharma@marvell.com>;
> Ivan Malov <ivan.malov@oktetlabs.ru>; Andrew Lee <alee@solarflare.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH] drivers/net: fix FW version get
> 
> Fixes a few different things:
> * Remove 'fw_version' NULL checks, it is allowed if the 'fw_size' is
>   zero, 'fw_version' being NULL but 'fw_size' not zero condition checked
>   in ethdev layer
> * Be sure required buffer size is returned if provided one is not big
>   enough, instead of returning success (0)
> * Document in doxygen comment the '-EINVAL' is a valid return type
> * Take into account that 'snprintf' can return negative value
> * Cast length to 'size_t' to compare it with 'fw_size'
> 
> Fixes: bb42aa9ffe4e ("net/atlantic: configure device start/stop")
> Fixes: ff70acdf4299 ("net/axgbe: support reading FW version")
> Fixes: e2652b0a20a0 ("net/bnxt: support get FW version")
> Fixes: cf0fab1d2ca5 ("net/dpaa: support firmware version get API")
> Fixes: 748eccb97cdc ("net/dpaa2: add support for firmware version get")
> Fixes: b883c0644a24 ("net/e1000: add firmware version get")
> Fixes: 293430677e9c ("net/enic: add handler to return firmware version")
> Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
> Fixes: bd5b86732bc7 ("net/hns3: modify format for firmware version")
> Fixes: ed0dfdd0e976 ("net/i40e: add firmware version get")
> Fixes: e31cb9a36298 ("net/ice: support FW version getting")
> Fixes: 4f09bc55ac3d ("net/igc: implement device base operations")
> Fixes: eec10fb0ce6b ("net/ionic: support FW version")
> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> Fixes: 4d9f5b8adc02 ("net/octeontx2: add FW version get operation")
> Fixes: f97b56f9f12e ("net/qede: support FW version query")
> Fixes: 83fef46a22b2 ("net/sfc: add callback to retrieve FW version")
> Fixes: bc84ac0fadef ("net/txgbe: support getting FW version")
> Fixes: 21913471202f ("ethdev: add firmware version get")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: pavel.belous@aquantia.com
> Cc: selwin.sebastian@amd.com
> Cc: hemant.agrawal@nxp.com
> Cc: qiming.yang@intel.com
> Cc: hyonkim@cisco.com
> Cc: xavier.huwei@huawei.com
> Cc: wenzhuo.lu@intel.com
> Cc: alvinx.zhang@intel.com
> Cc: cardigliano@ntop.org
> Cc: vattunuru@marvell.com
> Cc: rmody@marvell.com
> Cc: ivan.malov@oktetlabs.ru
> Cc: jiawenwu@trustnetic.com
> ---
>  drivers/net/atlantic/atl_ethdev.c       |  7 ++++---
>  drivers/net/axgbe/axgbe_rxtx.c          |  4 ----
>  drivers/net/bnxt/bnxt_ethdev.c          |  4 +++-
>  drivers/net/dpaa/dpaa_ethdev.c          |  6 ++++--
>  drivers/net/dpaa2/dpaa2_ethdev.c        |  4 +++-
>  drivers/net/e1000/igb_ethdev.c          |  4 +++-
>  drivers/net/enic/enic_ethdev.c          | 15 ++++++++++-----
>  drivers/net/hns3/hns3_ethdev.c          |  5 ++++-
>  drivers/net/hns3/hns3_ethdev_vf.c       |  5 ++++-
>  drivers/net/i40e/i40e_ethdev.c          |  4 +++-
>  drivers/net/ice/ice_ethdev.c            |  4 +++-
>  drivers/net/igc/igc_ethdev.c            |  4 +++-
>  drivers/net/ionic/ionic_ethdev.c        | 15 +++++++++------
>  drivers/net/ixgbe/ixgbe_ethdev.c        |  4 +++-
>  drivers/net/octeontx2/otx2_ethdev_ops.c |  2 +-
>  drivers/net/qede/qede_ethdev.c          |  3 ---
>  drivers/net/sfc/sfc_ethdev.c            |  8 --------
>  drivers/net/txgbe/txgbe_ethdev.c        |  4 +++-
>  lib/librte_ethdev/rte_ethdev.h          |  1 +
>  19 files changed, 61 insertions(+), 42 deletions(-)
> 

For e1000/igc/ixgbe

Acked-by: Haiyue Wang <haiyue.wang@intel.com>

> --
> 2.30.2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH] drivers/net: fix FW version get
  2021-04-21 16:20 [dpdk-dev] [PATCH] drivers/net: fix FW version get Ferruh Yigit
  2021-04-21 16:30 ` Ajit Khaparde
  2021-04-22 11:44 ` Wang, Haiyue
@ 2021-04-22 17:01 ` Rasesh Mody
  2021-04-23  1:49 ` [dpdk-dev] " Jiawen Wu
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Rasesh Mody @ 2021-04-22 17:01 UTC (permalink / raw)
  To: Ferruh Yigit, Igor Russkikh, Pavel Belous, Somalapuram Amaranath,
	Ajit Khaparde, Somnath Kotur, Hemant Agrawal, Sachin Saxena,
	Jeff Guo, Haiyue Wang, John Daley, Hyong Youb Kim,
	Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Beilei Xing, Qiming Yang, Qi Zhang,
	Andrew Boyer, Jerin Jacob Kollanukkaran, Nithin Kumar Dabilpuram,
	Kiran Kumar Kokkilagadda, Devendra Singh Rawat, Andrew Rybchenko,
	Jiawen Wu, Jian Wang, Thomas Monjalon, Selwin Sebastian,
	Remy Horton, Chunsong Feng, Huisong Li, Hao Chen, Wei Hu (Xavier),
	Jingjing Wu, Wenzhuo Lu, Xiaoyun Li, Alvin Zhang, Shannon Nelson,
	Alfredo Cardigliano, Vamsi Krishna Attunuru, Yash Sharma,
	Ivan Malov, Andrew Lee
  Cc: dev, stable

> From: Ferruh Yigit <ferruh.yigit@intel.com>
> Sent: Wednesday, April 21, 2021 9:51 PM
> 
> Fixes a few different things:
> * Remove 'fw_version' NULL checks, it is allowed if the 'fw_size' is
>   zero, 'fw_version' being NULL but 'fw_size' not zero condition checked
>   in ethdev layer
> * Be sure required buffer size is returned if provided one is not big
>   enough, instead of returning success (0)
> * Document in doxygen comment the '-EINVAL' is a valid return type
> * Take into account that 'snprintf' can return negative value
> * Cast length to 'size_t' to compare it with 'fw_size'
> 
> Fixes: bb42aa9ffe4e ("net/atlantic: configure device start/stop")
> Fixes: ff70acdf4299 ("net/axgbe: support reading FW version")
> Fixes: e2652b0a20a0 ("net/bnxt: support get FW version")
> Fixes: cf0fab1d2ca5 ("net/dpaa: support firmware version get API")
> Fixes: 748eccb97cdc ("net/dpaa2: add support for firmware version get")
> Fixes: b883c0644a24 ("net/e1000: add firmware version get")
> Fixes: 293430677e9c ("net/enic: add handler to return firmware version")
> Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
> Fixes: bd5b86732bc7 ("net/hns3: modify format for firmware version")
> Fixes: ed0dfdd0e976 ("net/i40e: add firmware version get")
> Fixes: e31cb9a36298 ("net/ice: support FW version getting")
> Fixes: 4f09bc55ac3d ("net/igc: implement device base operations")
> Fixes: eec10fb0ce6b ("net/ionic: support FW version")
> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> Fixes: 4d9f5b8adc02 ("net/octeontx2: add FW version get operation")
> Fixes: f97b56f9f12e ("net/qede: support FW version query")
> Fixes: 83fef46a22b2 ("net/sfc: add callback to retrieve FW version")
> Fixes: bc84ac0fadef ("net/txgbe: support getting FW version")
> Fixes: 21913471202f ("ethdev: add firmware version get")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---

For qede, thanks ferruh.
Acked-by: Rasesh Mody <rmody@marvell.com>

> Cc: pavel.belous@aquantia.com
> Cc: selwin.sebastian@amd.com
> Cc: hemant.agrawal@nxp.com
> Cc: qiming.yang@intel.com
> Cc: hyonkim@cisco.com
> Cc: xavier.huwei@huawei.com
> Cc: wenzhuo.lu@intel.com
> Cc: alvinx.zhang@intel.com
> Cc: cardigliano@ntop.org
> Cc: vattunuru@marvell.com
> Cc: rmody@marvell.com
> Cc: ivan.malov@oktetlabs.ru
> Cc: jiawenwu@trustnetic.com
> ---
>  drivers/net/atlantic/atl_ethdev.c       |  7 ++++---
>  drivers/net/axgbe/axgbe_rxtx.c          |  4 ----
>  drivers/net/bnxt/bnxt_ethdev.c          |  4 +++-
>  drivers/net/dpaa/dpaa_ethdev.c          |  6 ++++--
>  drivers/net/dpaa2/dpaa2_ethdev.c        |  4 +++-
>  drivers/net/e1000/igb_ethdev.c          |  4 +++-
>  drivers/net/enic/enic_ethdev.c          | 15 ++++++++++-----
>  drivers/net/hns3/hns3_ethdev.c          |  5 ++++-
>  drivers/net/hns3/hns3_ethdev_vf.c       |  5 ++++-
>  drivers/net/i40e/i40e_ethdev.c          |  4 +++-
>  drivers/net/ice/ice_ethdev.c            |  4 +++-
>  drivers/net/igc/igc_ethdev.c            |  4 +++-
>  drivers/net/ionic/ionic_ethdev.c        | 15 +++++++++------
>  drivers/net/ixgbe/ixgbe_ethdev.c        |  4 +++-
>  drivers/net/octeontx2/otx2_ethdev_ops.c |  2 +-
>  drivers/net/qede/qede_ethdev.c          |  3 ---
>  drivers/net/sfc/sfc_ethdev.c            |  8 --------
>  drivers/net/txgbe/txgbe_ethdev.c        |  4 +++-
>  lib/librte_ethdev/rte_ethdev.h          |  1 +
>  19 files changed, 61 insertions(+), 42 deletions(-)
> 
> diff --git a/drivers/net/atlantic/atl_ethdev.c
> b/drivers/net/atlantic/atl_ethdev.c
> index 473f6209f6aa..ce7f814f255d 100644
> --- a/drivers/net/atlantic/atl_ethdev.c
> +++ b/drivers/net/atlantic/atl_ethdev.c
> @@ -1073,7 +1073,7 @@ atl_fw_version_get(struct rte_eth_dev *dev, char
> *fw_version, size_t fw_size)  {
>  	struct aq_hw_s *hw = ATL_DEV_PRIVATE_TO_HW(dev->data-
> >dev_private);
>  	uint32_t fw_ver = 0;
> -	unsigned int ret = 0;
> +	int ret = 0;
> 
>  	ret = hw_atl_utils_get_fw_version(hw, &fw_ver);
>  	if (ret)
> @@ -1081,10 +1081,11 @@ atl_fw_version_get(struct rte_eth_dev *dev,
> char *fw_version, size_t fw_size)
> 
>  	ret = snprintf(fw_version, fw_size, "%u.%u.%u", fw_ver >> 24,
>  		       (fw_ver >> 16) & 0xFFU, fw_ver & 0xFFFFU);
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	ret += 1; /* add string null-terminator */
> -
> -	if (fw_size < ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
> 
>  	return 0;
> diff --git a/drivers/net/axgbe/axgbe_rxtx.c
> b/drivers/net/axgbe/axgbe_rxtx.c index e34bb6d448fc..33f709a6bb02
> 100644
> --- a/drivers/net/axgbe/axgbe_rxtx.c
> +++ b/drivers/net/axgbe/axgbe_rxtx.c
> @@ -623,9 +623,6 @@ int axgbe_dev_fw_version_get(struct rte_eth_dev
> *eth_dev,
>  	pdata = (struct axgbe_port *)eth_dev->data->dev_private;
>  	hw_feat = &pdata->hw_feat;
> 
> -	if (fw_version == NULL)
> -		return -EINVAL;
> -
>  	ret = snprintf(fw_version, fw_size, "%d.%d.%d",
>  			AXGMAC_GET_BITS(hw_feat->version, MAC_VR,
> USERVER),
>  			AXGMAC_GET_BITS(hw_feat->version, MAC_VR,
> DEVID), @@ -634,7 +631,6 @@ int axgbe_dev_fw_version_get(struct
> rte_eth_dev *eth_dev,
>  		return -EINVAL;
> 
>  	ret += 1; /* add the size of '\0' */
> -
>  	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
> diff --git a/drivers/net/bnxt/bnxt_ethdev.c
> b/drivers/net/bnxt/bnxt_ethdev.c index f5d2dc85907d..dba5b9f94df4
> 100644
> --- a/drivers/net/bnxt/bnxt_ethdev.c
> +++ b/drivers/net/bnxt/bnxt_ethdev.c
> @@ -2794,9 +2794,11 @@ bnxt_fw_version_get(struct rte_eth_dev *dev,
> char *fw_version, size_t fw_size)
> 
>  	ret = snprintf(fw_version, fw_size, "%d.%d.%d.%d",
>  			fw_major, fw_minor, fw_updt, fw_rsvd);
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (uint32_t)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;
> diff --git a/drivers/net/dpaa/dpaa_ethdev.c
> b/drivers/net/dpaa/dpaa_ethdev.c index 176943a8eae6..57ee660c5e57
> 100644
> --- a/drivers/net/dpaa/dpaa_ethdev.c
> +++ b/drivers/net/dpaa/dpaa_ethdev.c
> @@ -532,9 +532,11 @@ dpaa_fw_version_get(struct rte_eth_dev *dev
> __rte_unused,
> 
>  	ret = snprintf(fw_version, fw_size, "SVR:%x-fman-v%x",
>  		       svr_ver, fman_ip_rev);
> -	ret += 1; /* add the size of '\0' */
> +	if (ret < 0)
> +		return -EINVAL;
> 
> -	if (fw_size < (uint32_t)ret)
> +	ret += 1; /* add the size of '\0' */
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;
> diff --git a/drivers/net/dpaa2/dpaa2_ethdev.c
> b/drivers/net/dpaa2/dpaa2_ethdev.c
> index 9011dcfc1242..95e8d9abe869 100644
> --- a/drivers/net/dpaa2/dpaa2_ethdev.c
> +++ b/drivers/net/dpaa2/dpaa2_ethdev.c
> @@ -226,9 +226,11 @@ dpaa2_fw_version_get(struct rte_eth_dev *dev,
>  		       mc_ver_info.major,
>  		       mc_ver_info.minor,
>  		       mc_ver_info.revision);
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (uint32_t)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;
> diff --git a/drivers/net/e1000/igb_ethdev.c
> b/drivers/net/e1000/igb_ethdev.c index 3fdffe329483..10ee0f33415a 100644
> --- a/drivers/net/e1000/igb_ethdev.c
> +++ b/drivers/net/e1000/igb_ethdev.c
> @@ -2159,9 +2159,11 @@ eth_igb_fw_version_get(struct rte_eth_dev
> *dev, char *fw_version,
>  		}
>  		break;
>  	}
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (u32)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;
> diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
> index d91c2cdc8c54..e1a393b84762 100644
> --- a/drivers/net/enic/enic_ethdev.c
> +++ b/drivers/net/enic/enic_ethdev.c
> @@ -1050,16 +1050,21 @@ static int enicpmd_dev_fw_version_get(struct
> rte_eth_dev *eth_dev,
>  	int ret;
> 
>  	ENICPMD_FUNC_TRACE();
> -	if (fw_version == NULL || fw_size <= 0)
> -		return -EINVAL;
> +
>  	enic = pmd_priv(eth_dev);
>  	ret = vnic_dev_fw_info(enic->vdev, &info);
>  	if (ret)
>  		return ret;
> -	snprintf(fw_version, fw_size, "%s %s",
> +	ret = snprintf(fw_version, fw_size, "%s %s",
>  		 info->fw_version, info->fw_build);
> -	fw_version[fw_size - 1] = '\0';
> -	return 0;
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	ret += 1; /* add the size of '\0' */
> +	if (fw_size < (size_t)ret)
> +		return ret;
> +	else
> +		return 0;
>  }
> 
>  static const struct eth_dev_ops enicpmd_eth_dev_ops = { diff --git
> a/drivers/net/hns3/hns3_ethdev.c b/drivers/net/hns3/hns3_ethdev.c index
> 60267e1ec115..bd0699af5844 100644
> --- a/drivers/net/hns3/hns3_ethdev.c
> +++ b/drivers/net/hns3/hns3_ethdev.c
> @@ -2828,8 +2828,11 @@ hns3_fw_version_get(struct rte_eth_dev
> *eth_dev, char *fw_version,
>  				      HNS3_FW_VERSION_BYTE1_S),
>  		       hns3_get_field(version, HNS3_FW_VERSION_BYTE0_M,
>  				      HNS3_FW_VERSION_BYTE0_S));
> +	if (ret < 0)
> +		return -EINVAL;
> +
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (uint32_t)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;
> diff --git a/drivers/net/hns3/hns3_ethdev_vf.c
> b/drivers/net/hns3/hns3_ethdev_vf.c
> index 7a7a6bfa7cc0..06a26fb9be92 100644
> --- a/drivers/net/hns3/hns3_ethdev_vf.c
> +++ b/drivers/net/hns3/hns3_ethdev_vf.c
> @@ -2181,8 +2181,11 @@ hns3vf_fw_version_get(struct rte_eth_dev
> *eth_dev, char *fw_version,
>  				      HNS3_FW_VERSION_BYTE1_S),
>  		       hns3_get_field(version, HNS3_FW_VERSION_BYTE0_M,
>  				      HNS3_FW_VERSION_BYTE0_S));
> +	if (ret < 0)
> +		return -EINVAL;
> +
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (uint32_t)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index e6206a7e5100..66d23d698ea0 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -3687,9 +3687,11 @@ i40e_fw_version_get(struct rte_eth_dev *dev,
> char *fw_version, size_t fw_size)
>  		 ((hw->nvm.version >> 4) & 0xff),
>  		 (hw->nvm.version & 0xf), hw->nvm.eetrack,
>  		 ver, build, patch);
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (u32)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;
> diff --git a/drivers/net/ice/ice_ethdev.c b/drivers/net/ice/ice_ethdev.c
> index da9e85bd7c9e..c37a2e09ceb0 100644
> --- a/drivers/net/ice/ice_ethdev.c
> +++ b/drivers/net/ice/ice_ethdev.c
> @@ -4646,10 +4646,12 @@ ice_fw_version_get(struct rte_eth_dev *dev,
> char *fw_version, size_t fw_size)
>  			hw->flash.nvm.minor,
>  			hw->flash.nvm.eetrack,
>  			ver, build, patch);
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	/* add the size of '\0' */
>  	ret += 1;
> -	if (fw_size < (u32)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;
> diff --git a/drivers/net/igc/igc_ethdev.c b/drivers/net/igc/igc_ethdev.c index
> 31c99dca099f..b1c58fb3deb9 100644
> --- a/drivers/net/igc/igc_ethdev.c
> +++ b/drivers/net/igc/igc_ethdev.c
> @@ -1476,9 +1476,11 @@ eth_igc_fw_version_get(struct rte_eth_dev
> *dev, char *fw_version,
>  				 fw.eep_build);
>  		}
>  	}
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (u32)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;
> diff --git a/drivers/net/ionic/ionic_ethdev.c
> b/drivers/net/ionic/ionic_ethdev.c
> index cffe899c078c..5ec474dbf539 100644
> --- a/drivers/net/ionic/ionic_ethdev.c
> +++ b/drivers/net/ionic/ionic_ethdev.c
> @@ -215,15 +215,18 @@ ionic_dev_fw_version_get(struct rte_eth_dev
> *eth_dev,  {
>  	struct ionic_lif *lif = IONIC_ETH_DEV_TO_LIF(eth_dev);
>  	struct ionic_adapter *adapter = lif->adapter;
> +	int ret;
> 
> -	if (fw_version == NULL || fw_size <= 0)
> -		return -EINVAL;
> -
> -	snprintf(fw_version, fw_size, "%s",
> +	ret = snprintf(fw_version, fw_size, "%s",
>  		 adapter->fw_version);
> -	fw_version[fw_size - 1] = '\0';
> +	if (ret < 0)
> +		return -EINVAL;
> 
> -	return 0;
> +	ret += 1; /* add the size of '\0' */
> +	if (fw_size < (size_t)ret)
> +		return ret;
> +	else
> +		return 0;
>  }
> 
>  /*
> diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c
> b/drivers/net/ixgbe/ixgbe_ethdev.c
> index ff65145f5526..a5c191fe1b17 100644
> --- a/drivers/net/ixgbe/ixgbe_ethdev.c
> +++ b/drivers/net/ixgbe/ixgbe_ethdev.c
> @@ -3814,9 +3814,11 @@ ixgbe_fw_version_get(struct rte_eth_dev *dev,
> char *fw_version, size_t fw_size)
> 
>  	etrack_id = (eeprom_verh << 16) | eeprom_verl;
>  	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (u32)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;
> diff --git a/drivers/net/octeontx2/otx2_ethdev_ops.c
> b/drivers/net/octeontx2/otx2_ethdev_ops.c
> index 6f0cdc585422..5a4501208e9e 100644
> --- a/drivers/net/octeontx2/otx2_ethdev_ops.c
> +++ b/drivers/net/octeontx2/otx2_ethdev_ops.c
> @@ -453,7 +453,7 @@ otx2_nix_fw_version_get(struct rte_eth_dev
> *eth_dev, char *fw_version,
>  	rc = strlcpy(fw_version, (char *)dev->mkex_pfl_name, rc);
> 
>  	rc += 1; /* Add the size of '\0' */
> -	if (fw_size < (uint32_t)rc)
> +	if (fw_size < (size_t)rc)
>  		return rc;
> 
>  	return 0;
> diff --git a/drivers/net/qede/qede_ethdev.c
> b/drivers/net/qede/qede_ethdev.c index 057a7b00e2dc..c2c8de867501
> 100644
> --- a/drivers/net/qede/qede_ethdev.c
> +++ b/drivers/net/qede/qede_ethdev.c
> @@ -237,9 +237,6 @@ qede_fw_version_get(struct rte_eth_dev *dev, char
> *fw_ver, size_t fw_size)
>  	static char ver_str[QEDE_PMD_DRV_VER_STR_SIZE];
>  	size_t size;
> 
> -	if (fw_ver == NULL)
> -		return 0;
> -
>  	if (IS_PF(edev))
>  		snprintf(ver_str, QEDE_PMD_DRV_VER_STR_SIZE, "%s",
>  			 QEDE_PMD_FW_VERSION);
> diff --git a/drivers/net/sfc/sfc_ethdev.c b/drivers/net/sfc/sfc_ethdev.c
> index 52ba7b67b052..c50ecea0b993 100644
> --- a/drivers/net/sfc/sfc_ethdev.c
> +++ b/drivers/net/sfc/sfc_ethdev.c
> @@ -46,14 +46,6 @@ sfc_fw_version_get(struct rte_eth_dev *dev, char
> *fw_version, size_t fw_size)
>  	int ret;
>  	int rc;
> 
> -	/*
> -	 * Return value of the callback is likely supposed to be
> -	 * equal to or greater than 0, nevertheless, if an error
> -	 * occurs, it will be desirable to pass it to the caller
> -	 */
> -	if ((fw_version == NULL) || (fw_size == 0))
> -		return -EINVAL;
> -
>  	rc = efx_nic_get_fw_version(sa->nic, &enfi);
>  	if (rc != 0)
>  		return -rc;
> diff --git a/drivers/net/txgbe/txgbe_ethdev.c
> b/drivers/net/txgbe/txgbe_ethdev.c
> index 97796f040b43..8dbe3da5c2c9 100644
> --- a/drivers/net/txgbe/txgbe_ethdev.c
> +++ b/drivers/net/txgbe/txgbe_ethdev.c
> @@ -2582,9 +2582,11 @@ txgbe_fw_version_get(struct rte_eth_dev *dev,
> char *fw_version, size_t fw_size)
>  	hw->phy.get_fw_version(hw, &etrack_id);
> 
>  	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (u32)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;
> diff --git a/lib/librte_ethdev/rte_ethdev.h b/lib/librte_ethdev/rte_ethdev.h
> index a0d01d26675f..80841b44b680 100644
> --- a/lib/librte_ethdev/rte_ethdev.h
> +++ b/lib/librte_ethdev/rte_ethdev.h
> @@ -3069,6 +3069,7 @@ int rte_eth_dev_info_get(uint16_t port_id, struct
> rte_eth_dev_info *dev_info);
>   *   - (-ENOTSUP) if operation is not supported.
>   *   - (-ENODEV) if *port_id* invalid.
>   *   - (-EIO) if device is removed.
> + *   - (-EINVAL) if failed to get version.
>   *   - (>0) if *fw_size* is not enough to store firmware version, return
>   *          the size of the non truncated string.
>   */
> --
> 2.30.2



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] drivers/net: fix FW version get
  2021-04-21 16:20 [dpdk-dev] [PATCH] drivers/net: fix FW version get Ferruh Yigit
                   ` (2 preceding siblings ...)
  2021-04-22 17:01 ` [dpdk-dev] [EXT] " Rasesh Mody
@ 2021-04-23  1:49 ` Jiawen Wu
  2021-04-23  2:00 ` Xing, Beilei
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Jiawen Wu @ 2021-04-23  1:49 UTC (permalink / raw)
  To: 'Ferruh Yigit', 'Igor Russkikh',
	'Pavel Belous', 'Somalapuram Amaranath',
	'Ajit Khaparde', 'Somnath Kotur',
	'Hemant Agrawal', 'Sachin Saxena',
	'Jeff Guo', 'Haiyue Wang', 'John Daley',
	'Hyong Youb Kim', 'Min Hu (Connor)',
	'Yisen Zhuang', 'Lijun Ou', 'Beilei Xing',
	'Qiming Yang', 'Qi Zhang', 'Andrew Boyer',
	'Jerin Jacob', 'Nithin Dabilpuram',
	'Kiran Kumar K', 'Rasesh Mody',
	'Devendra Singh Rawat', 'Andrew Rybchenko',
	'Jian Wang', 'Thomas Monjalon',
	'Selwin Sebastian', 'Remy Horton',
	'Chunsong Feng', 'Huisong Li', 'Hao Chen',
	'Wei Hu (Xavier)', 'Jingjing Wu',
	'Wenzhuo Lu', 'Xiaoyun Li', 'Alvin Zhang',
	'Shannon Nelson', 'Alfredo Cardigliano',
	'Vamsi Attunuru', 'Yash Sharma',
	'Ivan Malov', 'Andrew Lee'
  Cc: dev, stable

On April 22, 2021 12:21 AM, Ferruh Yigit wrote:
> Fixes a few different things:
> * Remove 'fw_version' NULL checks, it is allowed if the 'fw_size' is
>   zero, 'fw_version' being NULL but 'fw_size' not zero condition checked
>   in ethdev layer
> * Be sure required buffer size is returned if provided one is not big
>   enough, instead of returning success (0)
> * Document in doxygen comment the '-EINVAL' is a valid return type
> * Take into account that 'snprintf' can return negative value
> * Cast length to 'size_t' to compare it with 'fw_size'
> 
> Fixes: bb42aa9ffe4e ("net/atlantic: configure device start/stop")
> Fixes: ff70acdf4299 ("net/axgbe: support reading FW version")
> Fixes: e2652b0a20a0 ("net/bnxt: support get FW version")
> Fixes: cf0fab1d2ca5 ("net/dpaa: support firmware version get API")
> Fixes: 748eccb97cdc ("net/dpaa2: add support for firmware version get")
> Fixes: b883c0644a24 ("net/e1000: add firmware version get")
> Fixes: 293430677e9c ("net/enic: add handler to return firmware version")
> Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
> Fixes: bd5b86732bc7 ("net/hns3: modify format for firmware version")
> Fixes: ed0dfdd0e976 ("net/i40e: add firmware version get")
> Fixes: e31cb9a36298 ("net/ice: support FW version getting")
> Fixes: 4f09bc55ac3d ("net/igc: implement device base operations")
> Fixes: eec10fb0ce6b ("net/ionic: support FW version")
> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> Fixes: 4d9f5b8adc02 ("net/octeontx2: add FW version get operation")
> Fixes: f97b56f9f12e ("net/qede: support FW version query")
> Fixes: 83fef46a22b2 ("net/sfc: add callback to retrieve FW version")
> Fixes: bc84ac0fadef ("net/txgbe: support getting FW version")
> Fixes: 21913471202f ("ethdev: add firmware version get")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

<...>

> diff --git a/drivers/net/txgbe/txgbe_ethdev.c
> b/drivers/net/txgbe/txgbe_ethdev.c
> index 97796f040b43..8dbe3da5c2c9 100644
> --- a/drivers/net/txgbe/txgbe_ethdev.c
> +++ b/drivers/net/txgbe/txgbe_ethdev.c
> @@ -2582,9 +2582,11 @@ txgbe_fw_version_get(struct rte_eth_dev *dev,
> char *fw_version, size_t fw_size)
>  	hw->phy.get_fw_version(hw, &etrack_id);
> 
>  	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (u32)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;

For txgbe,
Acked-by: Jiawen Wu <jiawenwu@trustnetic.com>





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] drivers/net: fix FW version get
  2021-04-21 16:20 [dpdk-dev] [PATCH] drivers/net: fix FW version get Ferruh Yigit
                   ` (3 preceding siblings ...)
  2021-04-23  1:49 ` [dpdk-dev] " Jiawen Wu
@ 2021-04-23  2:00 ` Xing, Beilei
  2021-04-23  2:19 ` Jiawen Wu
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 11+ messages in thread
From: Xing, Beilei @ 2021-04-23  2:00 UTC (permalink / raw)
  To: Yigit, Ferruh, Igor Russkikh, Pavel Belous,
	Somalapuram Amaranath, Ajit Khaparde, Somnath Kotur,
	Hemant Agrawal, Sachin Saxena, Guo, Jia, Wang, Haiyue, Daley,
	John, Hyong Youb Kim, Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Yang, Qiming, Zhang, Qi Z, Andrew Boyer,
	Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K, Rasesh Mody,
	Devendra Singh Rawat, Andrew Rybchenko, Jiawen Wu, Jian Wang,
	Thomas Monjalon, Selwin Sebastian, Remy Horton, Chunsong Feng,
	Huisong Li, Hao Chen, Wei Hu (Xavier),
	Wu, Jingjing, Lu, Wenzhuo, Li, Xiaoyun, Zhang, AlvinX,
	Shannon Nelson, Alfredo Cardigliano, Vamsi Attunuru, Yash Sharma,
	Ivan Malov, Andrew Lee
  Cc: dev, stable



> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Thursday, April 22, 2021 12:21 AM
> To: Igor Russkikh <igor.russkikh@aquantia.com>; Pavel Belous
> <pavel.belous@aquantia.com>; Somalapuram Amaranath
> <asomalap@amd.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>;
> Somnath Kotur <somnath.kotur@broadcom.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; Sachin Saxena <sachin.saxena@oss.nxp.com>;
> Guo, Jia <jia.guo@intel.com>; Wang, Haiyue <haiyue.wang@intel.com>;
> Daley, John <johndale@cisco.com>; Hyong Youb Kim <hyonkim@cisco.com>;
> Min Hu (Connor) <humin29@huawei.com>; Yisen Zhuang
> <yisen.zhuang@huawei.com>; Lijun Ou <oulijun@huawei.com>; Xing, Beilei
> <beilei.xing@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>; Andrew Boyer <aboyer@pensando.io>; Jerin Jacob
> <jerinj@marvell.com>; Nithin Dabilpuram <ndabilpuram@marvell.com>;
> Kiran Kumar K <kirankumark@marvell.com>; Rasesh Mody
> <rmody@marvell.com>; Devendra Singh Rawat <dsinghrawat@marvell.com>;
> Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>; Jiawen Wu
> <jiawenwu@trustnetic.com>; Jian Wang <jianwang@trustnetic.com>;
> Thomas Monjalon <thomas@monjalon.net>; Selwin Sebastian
> <selwin.sebastian@amd.com>; Remy Horton <remy.horton@intel.com>;
> Chunsong Feng <fengchunsong@huawei.com>; Huisong Li
> <lihuisong@huawei.com>; Hao Chen <chenhao164@huawei.com>; Wei Hu
> (Xavier) <xavier.huwei@huawei.com>; Wu, Jingjing <jingjing.wu@intel.com>;
> Lu, Wenzhuo <wenzhuo.lu@intel.com>; Li, Xiaoyun <xiaoyun.li@intel.com>;
> Zhang, AlvinX <alvinx.zhang@intel.com>; Shannon Nelson
> <snelson@pensando.io>; Alfredo Cardigliano <cardigliano@ntop.org>;
> Vamsi Attunuru <vattunuru@marvell.com>; Yash Sharma
> <ysharma@marvell.com>; Ivan Malov <ivan.malov@oktetlabs.ru>; Andrew
> Lee <alee@solarflare.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH] drivers/net: fix FW version get
> 
> Fixes a few different things:
> * Remove 'fw_version' NULL checks, it is allowed if the 'fw_size' is
>   zero, 'fw_version' being NULL but 'fw_size' not zero condition checked
>   in ethdev layer
> * Be sure required buffer size is returned if provided one is not big
>   enough, instead of returning success (0)
> * Document in doxygen comment the '-EINVAL' is a valid return type
> * Take into account that 'snprintf' can return negative value
> * Cast length to 'size_t' to compare it with 'fw_size'
> 
> Fixes: bb42aa9ffe4e ("net/atlantic: configure device start/stop")
> Fixes: ff70acdf4299 ("net/axgbe: support reading FW version")
> Fixes: e2652b0a20a0 ("net/bnxt: support get FW version")
> Fixes: cf0fab1d2ca5 ("net/dpaa: support firmware version get API")
> Fixes: 748eccb97cdc ("net/dpaa2: add support for firmware version get")
> Fixes: b883c0644a24 ("net/e1000: add firmware version get")
> Fixes: 293430677e9c ("net/enic: add handler to return firmware version")
> Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
> Fixes: bd5b86732bc7 ("net/hns3: modify format for firmware version")
> Fixes: ed0dfdd0e976 ("net/i40e: add firmware version get")
> Fixes: e31cb9a36298 ("net/ice: support FW version getting")
> Fixes: 4f09bc55ac3d ("net/igc: implement device base operations")
> Fixes: eec10fb0ce6b ("net/ionic: support FW version")
> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> Fixes: 4d9f5b8adc02 ("net/octeontx2: add FW version get operation")
> Fixes: f97b56f9f12e ("net/qede: support FW version query")
> Fixes: 83fef46a22b2 ("net/sfc: add callback to retrieve FW version")
> Fixes: bc84ac0fadef ("net/txgbe: support getting FW version")
> Fixes: 21913471202f ("ethdev: add firmware version get")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---

<...>

> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index e6206a7e5100..66d23d698ea0 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -3687,9 +3687,11 @@ i40e_fw_version_get(struct rte_eth_dev *dev,
> char *fw_version, size_t fw_size)
>  		 ((hw->nvm.version >> 4) & 0xff),
>  		 (hw->nvm.version & 0xf), hw->nvm.eetrack,
>  		 ver, build, patch);
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (u32)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;

For i40e,
Acked-by: Beilei Xing <beilei.xing@intel.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] drivers/net: fix FW version get
  2021-04-21 16:20 [dpdk-dev] [PATCH] drivers/net: fix FW version get Ferruh Yigit
                   ` (4 preceding siblings ...)
  2021-04-23  2:00 ` Xing, Beilei
@ 2021-04-23  2:19 ` Jiawen Wu
  2021-04-26 12:02   ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
  2021-04-23  3:22 ` [dpdk-dev] " Hemant Agrawal
  2021-04-28  8:12 ` Yang, Qiming
  7 siblings, 1 reply; 11+ messages in thread
From: Jiawen Wu @ 2021-04-23  2:19 UTC (permalink / raw)
  To: 'Ferruh Yigit', 'Igor Russkikh',
	'Pavel Belous', 'Somalapuram Amaranath',
	'Ajit Khaparde', 'Somnath Kotur',
	'Hemant Agrawal', 'Sachin Saxena',
	'Jeff Guo', 'Haiyue Wang', 'John Daley',
	'Hyong Youb Kim', 'Min Hu (Connor)',
	'Yisen Zhuang', 'Lijun Ou', 'Beilei Xing',
	'Qiming Yang', 'Qi Zhang', 'Andrew Boyer',
	'Jerin Jacob', 'Nithin Dabilpuram',
	'Kiran Kumar K', 'Rasesh Mody',
	'Devendra Singh Rawat', 'Andrew Rybchenko',
	'Jian Wang', 'Thomas Monjalon',
	'Selwin Sebastian', 'Chunsong Feng',
	'Huisong Li', 'Jingjing Wu', 'Wenzhuo Lu',
	'Xiaoyun Li', 'Alvin Zhang',
	'Shannon Nelson', 'Alfredo Cardigliano',
	'Vamsi Attunuru', 'Yash Sharma',
	'Ivan Malov', 'Andrew Lee'
  Cc: dev, stable

On April 22, 2021 12:21 AM, Ferruh Yigit wrote:
> Fixes a few different things:
> * Remove 'fw_version' NULL checks, it is allowed if the 'fw_size' is
>   zero, 'fw_version' being NULL but 'fw_size' not zero condition checked
>   in ethdev layer
> * Be sure required buffer size is returned if provided one is not big
>   enough, instead of returning success (0)
> * Document in doxygen comment the '-EINVAL' is a valid return type
> * Take into account that 'snprintf' can return negative value
> * Cast length to 'size_t' to compare it with 'fw_size'
> 
> Fixes: bb42aa9ffe4e ("net/atlantic: configure device start/stop")
> Fixes: ff70acdf4299 ("net/axgbe: support reading FW version")
> Fixes: e2652b0a20a0 ("net/bnxt: support get FW version")
> Fixes: cf0fab1d2ca5 ("net/dpaa: support firmware version get API")
> Fixes: 748eccb97cdc ("net/dpaa2: add support for firmware version 
> get")
> Fixes: b883c0644a24 ("net/e1000: add firmware version get")
> Fixes: 293430677e9c ("net/enic: add handler to return firmware 
> version")
> Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
> Fixes: bd5b86732bc7 ("net/hns3: modify format for firmware version")
> Fixes: ed0dfdd0e976 ("net/i40e: add firmware version get")
> Fixes: e31cb9a36298 ("net/ice: support FW version getting")
> Fixes: 4f09bc55ac3d ("net/igc: implement device base operations")
> Fixes: eec10fb0ce6b ("net/ionic: support FW version")
> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> Fixes: 4d9f5b8adc02 ("net/octeontx2: add FW version get operation")
> Fixes: f97b56f9f12e ("net/qede: support FW version query")
> Fixes: 83fef46a22b2 ("net/sfc: add callback to retrieve FW version")
> Fixes: bc84ac0fadef ("net/txgbe: support getting FW version")
> Fixes: 21913471202f ("ethdev: add firmware version get")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>

<...>

> diff --git a/drivers/net/txgbe/txgbe_ethdev.c
> b/drivers/net/txgbe/txgbe_ethdev.c
> index 97796f040b43..8dbe3da5c2c9 100644
> --- a/drivers/net/txgbe/txgbe_ethdev.c
> +++ b/drivers/net/txgbe/txgbe_ethdev.c
> @@ -2582,9 +2582,11 @@ txgbe_fw_version_get(struct rte_eth_dev *dev, 
> char *fw_version, size_t fw_size)
>  	hw->phy.get_fw_version(hw, &etrack_id);
> 
>  	ret = snprintf(fw_version, fw_size, "0x%08x", etrack_id);
> +	if (ret < 0)
> +		return -EINVAL;
> 
>  	ret += 1; /* add the size of '\0' */
> -	if (fw_size < (u32)ret)
> +	if (fw_size < (size_t)ret)
>  		return ret;
>  	else
>  		return 0;

For txgbe,
Acked-by: Jiawen Wu <jiawenwu@trustnetic.com>





^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] drivers/net: fix FW version get
  2021-04-21 16:20 [dpdk-dev] [PATCH] drivers/net: fix FW version get Ferruh Yigit
                   ` (5 preceding siblings ...)
  2021-04-23  2:19 ` Jiawen Wu
@ 2021-04-23  3:22 ` Hemant Agrawal
  2021-04-28  8:12 ` Yang, Qiming
  7 siblings, 0 replies; 11+ messages in thread
From: Hemant Agrawal @ 2021-04-23  3:22 UTC (permalink / raw)
  To: dev

Acked-by: Hemant Agrawal <hemant.agrawal@nxp.com>



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [dpdk-stable] [PATCH] drivers/net: fix FW version get
  2021-04-23  2:19 ` Jiawen Wu
@ 2021-04-26 12:02   ` Ferruh Yigit
  0 siblings, 0 replies; 11+ messages in thread
From: Ferruh Yigit @ 2021-04-26 12:02 UTC (permalink / raw)
  To: Jiawen Wu, 'Igor Russkikh', 'Pavel Belous',
	'Somalapuram Amaranath', 'Ajit Khaparde',
	'Somnath Kotur', 'Hemant Agrawal',
	'Sachin Saxena', 'Jeff Guo',
	'Haiyue Wang', 'John Daley',
	'Hyong Youb Kim', 'Min Hu (Connor)',
	'Yisen Zhuang', 'Lijun Ou', 'Beilei Xing',
	'Qiming Yang', 'Qi Zhang', 'Andrew Boyer',
	'Jerin Jacob', 'Nithin Dabilpuram',
	'Kiran Kumar K', 'Rasesh Mody',
	'Devendra Singh Rawat', 'Andrew Rybchenko',
	'Jian Wang', 'Thomas Monjalon',
	'Selwin Sebastian', 'Chunsong Feng',
	'Huisong Li', 'Jingjing Wu', 'Wenzhuo Lu',
	'Xiaoyun Li', 'Alvin Zhang',
	'Shannon Nelson', 'Alfredo Cardigliano',
	'Vamsi Attunuru', 'Yash Sharma',
	'Ivan Malov', 'Andrew Lee'
  Cc: dev, stable

On 4/23/2021 3:19 AM, Jiawen Wu wrote:
> On April 22, 2021 12:21 AM, Ferruh Yigit wrote:
>> Fixes a few different things:
>> * Remove 'fw_version' NULL checks, it is allowed if the 'fw_size' is
>>   zero, 'fw_version' being NULL but 'fw_size' not zero condition checked
>>   in ethdev layer
>> * Be sure required buffer size is returned if provided one is not big
>>   enough, instead of returning success (0)
>> * Document in doxygen comment the '-EINVAL' is a valid return type
>> * Take into account that 'snprintf' can return negative value
>> * Cast length to 'size_t' to compare it with 'fw_size'
>>
>> Fixes: bb42aa9ffe4e ("net/atlantic: configure device start/stop")
>> Fixes: ff70acdf4299 ("net/axgbe: support reading FW version")
>> Fixes: e2652b0a20a0 ("net/bnxt: support get FW version")
>> Fixes: cf0fab1d2ca5 ("net/dpaa: support firmware version get API")
>> Fixes: 748eccb97cdc ("net/dpaa2: add support for firmware version 
>> get")
>> Fixes: b883c0644a24 ("net/e1000: add firmware version get")
>> Fixes: 293430677e9c ("net/enic: add handler to return firmware 
>> version")
>> Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
>> Fixes: bd5b86732bc7 ("net/hns3: modify format for firmware version")
>> Fixes: ed0dfdd0e976 ("net/i40e: add firmware version get")
>> Fixes: e31cb9a36298 ("net/ice: support FW version getting")
>> Fixes: 4f09bc55ac3d ("net/igc: implement device base operations")
>> Fixes: eec10fb0ce6b ("net/ionic: support FW version")
>> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
>> Fixes: 4d9f5b8adc02 ("net/octeontx2: add FW version get operation")
>> Fixes: f97b56f9f12e ("net/qede: support FW version query")
>> Fixes: 83fef46a22b2 ("net/sfc: add callback to retrieve FW version")
>> Fixes: bc84ac0fadef ("net/txgbe: support getting FW version")
>> Fixes: 21913471202f ("ethdev: add firmware version get")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> 
> <...>
> 
> 
> For txgbe,
> Acked-by: Jiawen Wu <jiawenwu@trustnetic.com>
> 

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [dpdk-dev] [PATCH] drivers/net: fix FW version get
  2021-04-21 16:20 [dpdk-dev] [PATCH] drivers/net: fix FW version get Ferruh Yigit
                   ` (6 preceding siblings ...)
  2021-04-23  3:22 ` [dpdk-dev] " Hemant Agrawal
@ 2021-04-28  8:12 ` Yang, Qiming
  7 siblings, 0 replies; 11+ messages in thread
From: Yang, Qiming @ 2021-04-28  8:12 UTC (permalink / raw)
  To: Yigit, Ferruh, Igor Russkikh, Pavel Belous,
	Somalapuram Amaranath, Ajit Khaparde, Somnath Kotur,
	Hemant Agrawal, Sachin Saxena, Guo, Jia, Wang, Haiyue, Daley,
	John, Hyong Youb Kim, Min Hu (Connor),
	Yisen Zhuang, Lijun Ou, Xing, Beilei, Zhang, Qi Z, Andrew Boyer,
	Jerin Jacob, Nithin Dabilpuram, Kiran Kumar K, Rasesh Mody,
	Devendra Singh Rawat, Andrew Rybchenko, Jiawen Wu, Jian Wang,
	Thomas Monjalon, Selwin Sebastian, Remy Horton, Chunsong Feng,
	Huisong Li, Hao Chen, Wei Hu (Xavier),
	Wu, Jingjing, Lu, Wenzhuo, Li, Xiaoyun, Zhang, AlvinX,
	Shannon Nelson, Alfredo Cardigliano, Vamsi Attunuru, Yash Sharma,
	Ivan Malov, Andrew Lee
  Cc: dev, stable

Hi, 

> -----Original Message-----
> From: Yigit, Ferruh <ferruh.yigit@intel.com>
> Sent: Thursday, April 22, 2021 00:21
> To: Igor Russkikh <igor.russkikh@aquantia.com>; Pavel Belous
> <pavel.belous@aquantia.com>; Somalapuram Amaranath
> <asomalap@amd.com>; Ajit Khaparde <ajit.khaparde@broadcom.com>;
> Somnath Kotur <somnath.kotur@broadcom.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; Sachin Saxena
> <sachin.saxena@oss.nxp.com>; Guo, Jia <jia.guo@intel.com>; Wang, Haiyue
> <haiyue.wang@intel.com>; Daley, John <johndale@cisco.com>; Hyong Youb
> Kim <hyonkim@cisco.com>; Min Hu (Connor) <humin29@huawei.com>;
> Yisen Zhuang <yisen.zhuang@huawei.com>; Lijun Ou
> <oulijun@huawei.com>; Xing, Beilei <beilei.xing@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Andrew
> Boyer <aboyer@pensando.io>; Jerin Jacob <jerinj@marvell.com>; Nithin
> Dabilpuram <ndabilpuram@marvell.com>; Kiran Kumar K
> <kirankumark@marvell.com>; Rasesh Mody <rmody@marvell.com>;
> Devendra Singh Rawat <dsinghrawat@marvell.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Jiawen Wu
> <jiawenwu@trustnetic.com>; Jian Wang <jianwang@trustnetic.com>;
> Thomas Monjalon <thomas@monjalon.net>; Selwin Sebastian
> <selwin.sebastian@amd.com>; Remy Horton <remy.horton@intel.com>;
> Chunsong Feng <fengchunsong@huawei.com>; Huisong Li
> <lihuisong@huawei.com>; Hao Chen <chenhao164@huawei.com>; Wei Hu
> (Xavier) <xavier.huwei@huawei.com>; Wu, Jingjing
> <jingjing.wu@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>; Li,
> Xiaoyun <xiaoyun.li@intel.com>; Zhang, AlvinX <alvinx.zhang@intel.com>;
> Shannon Nelson <snelson@pensando.io>; Alfredo Cardigliano
> <cardigliano@ntop.org>; Vamsi Attunuru <vattunuru@marvell.com>; Yash
> Sharma <ysharma@marvell.com>; Ivan Malov <ivan.malov@oktetlabs.ru>;
> Andrew Lee <alee@solarflare.com>
> Cc: Yigit, Ferruh <ferruh.yigit@intel.com>; dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH] drivers/net: fix FW version get
> 
> Fixes a few different things:
> * Remove 'fw_version' NULL checks, it is allowed if the 'fw_size' is
>   zero, 'fw_version' being NULL but 'fw_size' not zero condition checked
>   in ethdev layer
> * Be sure required buffer size is returned if provided one is not big
>   enough, instead of returning success (0)
> * Document in doxygen comment the '-EINVAL' is a valid return type
> * Take into account that 'snprintf' can return negative value
> * Cast length to 'size_t' to compare it with 'fw_size'
> 
> Fixes: bb42aa9ffe4e ("net/atlantic: configure device start/stop")
> Fixes: ff70acdf4299 ("net/axgbe: support reading FW version")
> Fixes: e2652b0a20a0 ("net/bnxt: support get FW version")
> Fixes: cf0fab1d2ca5 ("net/dpaa: support firmware version get API")
> Fixes: 748eccb97cdc ("net/dpaa2: add support for firmware version get")
> Fixes: b883c0644a24 ("net/e1000: add firmware version get")
> Fixes: 293430677e9c ("net/enic: add handler to return firmware version")
> Fixes: 1f5ca0b460cd ("net/hns3: support some device operations")
> Fixes: bd5b86732bc7 ("net/hns3: modify format for firmware version")
> Fixes: ed0dfdd0e976 ("net/i40e: add firmware version get")
> Fixes: e31cb9a36298 ("net/ice: support FW version getting")
> Fixes: 4f09bc55ac3d ("net/igc: implement device base operations")
> Fixes: eec10fb0ce6b ("net/ionic: support FW version")
> Fixes: 8b0b56574269 ("net/ixgbe: add firmware version get")
> Fixes: 4d9f5b8adc02 ("net/octeontx2: add FW version get operation")
> Fixes: f97b56f9f12e ("net/qede: support FW version query")
> Fixes: 83fef46a22b2 ("net/sfc: add callback to retrieve FW version")
> Fixes: bc84ac0fadef ("net/txgbe: support getting FW version")
> Fixes: 21913471202f ("ethdev: add firmware version get")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
> Cc: pavel.belous@aquantia.com
> Cc: selwin.sebastian@amd.com
> Cc: hemant.agrawal@nxp.com
> Cc: qiming.yang@intel.com
> Cc: hyonkim@cisco.com
> Cc: xavier.huwei@huawei.com
> Cc: wenzhuo.lu@intel.com
> Cc: alvinx.zhang@intel.com
> Cc: cardigliano@ntop.org
> Cc: vattunuru@marvell.com
> Cc: rmody@marvell.com
> Cc: ivan.malov@oktetlabs.ru
> Cc: jiawenwu@trustnetic.com
> ---
>  drivers/net/atlantic/atl_ethdev.c       |  7 ++++---
>  drivers/net/axgbe/axgbe_rxtx.c          |  4 ----
>  drivers/net/bnxt/bnxt_ethdev.c          |  4 +++-
>  drivers/net/dpaa/dpaa_ethdev.c          |  6 ++++--
>  drivers/net/dpaa2/dpaa2_ethdev.c        |  4 +++-
>  drivers/net/e1000/igb_ethdev.c          |  4 +++-
>  drivers/net/enic/enic_ethdev.c          | 15 ++++++++++-----
>  drivers/net/hns3/hns3_ethdev.c          |  5 ++++-
>  drivers/net/hns3/hns3_ethdev_vf.c       |  5 ++++-
>  drivers/net/i40e/i40e_ethdev.c          |  4 +++-
>  drivers/net/ice/ice_ethdev.c            |  4 +++-
>  drivers/net/igc/igc_ethdev.c            |  4 +++-
>  drivers/net/ionic/ionic_ethdev.c        | 15 +++++++++------
>  drivers/net/ixgbe/ixgbe_ethdev.c        |  4 +++-
>  drivers/net/octeontx2/otx2_ethdev_ops.c |  2 +-
>  drivers/net/qede/qede_ethdev.c          |  3 ---
>  drivers/net/sfc/sfc_ethdev.c            |  8 --------
>  drivers/net/txgbe/txgbe_ethdev.c        |  4 +++-
>  lib/librte_ethdev/rte_ethdev.h          |  1 +
>  19 files changed, 61 insertions(+), 42 deletions(-)
> 

For ice driver,
Acked-by: Qiming Yang <qiming.yang@intel.com>

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-04-28  8:12 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-21 16:20 [dpdk-dev] [PATCH] drivers/net: fix FW version get Ferruh Yigit
2021-04-21 16:30 ` Ajit Khaparde
2021-04-22  8:53   ` Andrew Rybchenko
2021-04-22 11:44 ` Wang, Haiyue
2021-04-22 17:01 ` [dpdk-dev] [EXT] " Rasesh Mody
2021-04-23  1:49 ` [dpdk-dev] " Jiawen Wu
2021-04-23  2:00 ` Xing, Beilei
2021-04-23  2:19 ` Jiawen Wu
2021-04-26 12:02   ` [dpdk-dev] [dpdk-stable] " Ferruh Yigit
2021-04-23  3:22 ` [dpdk-dev] " Hemant Agrawal
2021-04-28  8:12 ` Yang, Qiming

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).