[dpdk-dev,RFC] examples/ethtool: enhance ethtool app in i40e

Message ID 1474356637-48797-1-git-send-email-qiming.yang@intel.com (mailing list archive)
State RFC, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Qiming Yang Sept. 20, 2016, 7:30 a.m. UTC
  Now, run the example/ethtool, the drvinfo can not show the fireware
information. From customer point of view, it should be better if we
can have the same as kernel version ethtool show the bus-info and
firmware-version. We need to add a variable in struct rte_eth_dev_info
to get the fw version.
I’m interested in:
a) this approach is appropriate?
b) would prefer a change of the API?

Signed-off-by: Qiming Yang <qiming.yang@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c        | 5 +++++
 examples/ethtool/ethtool-app/ethapp.c | 2 ++
 examples/ethtool/lib/rte_ethtool.c    | 2 ++
 lib/librte_ether/rte_ethdev.h         | 1 +
 4 files changed, 10 insertions(+)
  

Comments

Remy Horton Sept. 21, 2016, 10:20 a.m. UTC | #1
On 20/09/2016 08:30, Qiming Yang wrote:
> Now, run the example/ethtool, the drvinfo can not show the fireware
> information. From customer point of view, it should be better if we
> can have the same as kernel version ethtool show the bus-info and
> firmware-version. We need to add a variable in struct rte_eth_dev_info
> to get the fw version.
> I’m interested in:
> a) this approach is appropriate?
> b) would prefer a change of the API?

The approach is nice and clean, but the changes to rte_eth_dev_info are 
an ABI break.

One alternative is to add a new function (and associated driver function 
pointers) to fetch the version string, but my feeling is if one is to go 
down that road, they may as well generalise it into a driver info 
querying mini-API.

..Remy
  

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index d0aeb70..0f66ecf 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -2556,6 +2556,11 @@  i40e_dev_info_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info)
 	struct i40e_hw *hw = I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct i40e_vsi *vsi = pf->main_vsi;
 
+	snprintf(dev_info->fw_version, 20,
+		 "%d.%d%d 0x%04x",
+		 ((hw->nvm.version >> 12) & 0xf),
+		 ((hw->nvm.version >> 4) & 0xff),
+		 (hw->nvm.version & 0xf), hw->nvm.eetrack);
 	dev_info->max_rx_queues = vsi->nb_qps;
 	dev_info->max_tx_queues = vsi->nb_qps;
 	dev_info->min_rx_bufsize = I40E_BUF_SIZE_MIN;
diff --git a/examples/ethtool/ethtool-app/ethapp.c b/examples/ethtool/ethtool-app/ethapp.c
index 38e466c..9b77385 100644
--- a/examples/ethtool/ethtool-app/ethapp.c
+++ b/examples/ethtool/ethtool-app/ethapp.c
@@ -184,6 +184,8 @@  pcmd_drvinfo_callback(__rte_unused void *ptr_params,
 		printf("Port %i driver: %s (ver: %s)\n",
 			id_port, info.driver, info.version
 		      );
+		printf("bus-info: %s\n", info.bus_info);
+		printf("firmware-version: %s\n", info.fw_version);
 	}
 }
 
diff --git a/examples/ethtool/lib/rte_ethtool.c b/examples/ethtool/lib/rte_ethtool.c
index a1f91d4..72c3f94 100644
--- a/examples/ethtool/lib/rte_ethtool.c
+++ b/examples/ethtool/lib/rte_ethtool.c
@@ -57,6 +57,8 @@  rte_ethtool_get_drvinfo(uint8_t port_id, struct ethtool_drvinfo *drvinfo)
 	memset(&dev_info, 0, sizeof(dev_info));
 	rte_eth_dev_info_get(port_id, &dev_info);
 
+	snprintf(drvinfo->fw_version, sizeof(drvinfo->fw_version), "%s",
+			dev_info.fw_version);
 	snprintf(drvinfo->driver, sizeof(drvinfo->driver), "%s",
 		dev_info.driver_name);
 	snprintf(drvinfo->version, sizeof(drvinfo->version), "%s",
diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index b0fe033..1cb85ba 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -869,6 +869,7 @@  struct rte_eth_conf {
  * Ethernet device information
  */
 struct rte_eth_dev_info {
+	char fw_version[20]; /**< NIC firmware version. */
 	struct rte_pci_device *pci_dev; /**< Device PCI information. */
 	const char *driver_name; /**< Device Driver name. */
 	unsigned int if_index; /**< Index to bound host interface, or 0 if none.