Re: [PATCH net-next 3/5] i40e: Add handler for devlink .info_get

From: Ivan Vecera
Date: Wed Oct 18 2023 - 07:59:48 EST




On 17. 10. 23 19:17, Jacob Keller wrote:


On 10/13/2023 10:07 AM, Ivan Vecera wrote:
Provide devlink .info_get callback to allow the driver to report
detailed version information. The following info is reported:

"serial_number" -> The PCI DSN of the adapter
"fw.mgmt" -> The version of the firmware
"fw.mgmt.api" -> The API version of interface exposed over the AdminQ
"fw.psid" -> The version of the NVM image
"fw.bundle_id" -> Unique identifier for the combined flash image
"fw.undi" -> The combo image version

With this, 'devlink dev info' provides at least the same amount
information as is reported by ETHTOOL_GDRVINFO:

$ ethtool -i enp2s0f0 | egrep '(driver|firmware)'
driver: i40e
firmware-version: 9.30 0x8000e5f3 1.3429.0

$ devlink dev info pci/0000:02:00.0
pci/0000:02:00.0:
driver i40e
serial_number c0-de-b7-ff-ff-ef-ec-3c
versions:
running:
fw.mgmt 9.130.73618

The ice driver used fw.mgmt.build for the fw_build value, rather than
combining it into the fw.mgmt value.

OK, will fix by follow up.

fw.mgmt.api 1.15
fw.psid 9.30

As discussed in the other thread, ice used fw.psid.api

OK, will change it to fw.psid.api.

fw.bundle_id 0x8000e5f3
fw.undi 1.3429.0


Does i40e have a netlist? The ice driver reports netlist versions as
well. It also reports the DDP version information, but I don't think
i40e supports that either if I recall..

i40e supports to load DDP in runtime by ethtool flash function and the
name and version of DDP package could be provided IMHO.


Signed-off-by: Ivan Vecera <ivecera@xxxxxxxxxx>
---
.../net/ethernet/intel/i40e/i40e_devlink.c | 90 +++++++++++++++++++
1 file changed, 90 insertions(+)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_devlink.c b/drivers/net/ethernet/intel/i40e/i40e_devlink.c
index 66b7f5be45ae..fb6144d74c98 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_devlink.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_devlink.c
@@ -5,7 +5,97 @@
#include "i40e.h"
#include "i40e_devlink.h"
+static void i40e_info_get_dsn(struct i40e_pf *pf, char *buf, size_t len)
+{
+ u8 dsn[8];
+
+ put_unaligned_be64(pci_get_dsn(pf->pdev), dsn);
+
+ snprintf(buf, len, "%8phD", dsn);
+}
+
+static void i40e_info_fw_mgmt(struct i40e_hw *hw, char *buf, size_t len)
+{
+ struct i40e_adminq_info *aq = &hw->aq;
+
+ snprintf(buf, len, "%u.%u.%05d",
+ aq->fw_maj_ver, aq->fw_min_ver, aq->fw_build);
+}
+
+static void i40e_info_fw_api(struct i40e_hw *hw, char *buf, size_t len)
+{
+ struct i40e_adminq_info *aq = &hw->aq;
+
+ snprintf(buf, len, "%u.%u", aq->api_maj_ver, aq->api_min_ver);
+}
+
+enum i40e_devlink_version_type {
+ I40E_DL_VERSION_RUNNING,
+};
+
+static int i40e_devlink_info_put(struct devlink_info_req *req,
+ enum i40e_devlink_version_type type,
+ const char *key, const char *value)
+{
+ if (!strlen(value))
+ return 0;
+
+ switch (type) {
+ case I40E_DL_VERSION_RUNNING:
+ return devlink_info_version_running_put(req, key, value);
+ }
+ return 0;
+}
+
+static int i40e_devlink_info_get(struct devlink *dl,
+ struct devlink_info_req *req,
+ struct netlink_ext_ack *extack)
+{
+ struct i40e_pf *pf = devlink_priv(dl);
+ struct i40e_hw *hw = &pf->hw;
+ char buf[32];
+ int err;
+
+ i40e_info_get_dsn(pf, buf, sizeof(buf));
+ err = devlink_info_serial_number_put(req, buf);
+ if (err)
+ return err;
+
+ i40e_info_fw_mgmt(hw, buf, sizeof(buf));
+ err = i40e_devlink_info_put(req, I40E_DL_VERSION_RUNNING,
+ DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, buf);
+ if (err)
+ return err;
+
+ i40e_info_fw_api(hw, buf, sizeof(buf));
+ err = i40e_devlink_info_put(req, I40E_DL_VERSION_RUNNING,
+ DEVLINK_INFO_VERSION_GENERIC_FW_MGMT_API,
+ buf);
+ if (err)
+ return err;
+
+ i40e_info_nvm_ver(hw, buf, sizeof(buf));
+ err = i40e_devlink_info_put(req, I40E_DL_VERSION_RUNNING,
+ DEVLINK_INFO_VERSION_GENERIC_FW_PSID, buf);
+ if (err)
+ return err;
+
+ i40e_info_eetrack(hw, buf, sizeof(buf));
+ err = i40e_devlink_info_put(req, I40E_DL_VERSION_RUNNING,
+ DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID,
+ buf);
+ if (err)
+ return err;
+
+ i40e_info_civd_ver(hw, buf, sizeof(buf));
+ err = i40e_devlink_info_put(req, I40E_DL_VERSION_RUNNING,
+ DEVLINK_INFO_VERSION_GENERIC_FW_UNDI, buf);
+
+ return err;
+}

The ice driver created a struct list and loop flow to iterate this. I'm
wondering if it could make sense to extract that logic into devlink
core, so that drivers just need to implement a map between version names
and functions which extract the name.

It seems like it would be straight forward to implement with a setup,
the list mapping info names to version getters, and a teardown.

Hmm...

+
static const struct devlink_ops i40e_devlink_ops = {
+ .info_get = i40e_devlink_info_get,
};
/**