netdev - Re: [PATCH v4 net-next 04/10] qed: implement devlink info request

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200731130402.2288f44a@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Fri, 31 Jul 2020 13:04:02 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Igor Russkikh <irusskikh@...vell.com>
Cc:     <netdev@...r.kernel.org>, "David S . Miller" <davem@...emloft.net>,
        Ariel Elior <aelior@...vell.com>,
        Michal Kalderon <mkalderon@...vell.com>,
        Denis Bolotin <dbolotin@...vell.com>,
        Jiri Pirko <jiri@...nulli.us>,
        Alexander Lobakin <alobakin@...vell.com>,
        Michal Kalderon <michal.kalderon@...vell.com>
Subject: Re: [PATCH v4 net-next 04/10] qed: implement devlink info request

On Fri, 31 Jul 2020 08:53:55 +0300 Igor Russkikh wrote:
> Here we return existing fw & mfw versions, we also fetch device's
> serial number.
> 
> The base device specific structure (qed_dev_info) was not directly
> available to the base driver before.
> Thus, here we create and store a private copy of this structure
> in qed_dev root object.

Please include example output of devlink info on you device.

> diff --git a/drivers/net/ethernet/qlogic/qed/qed_devlink.c b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
> index a62c47c61edf..57ef2c56c884 100644
> --- a/drivers/net/ethernet/qlogic/qed/qed_devlink.c
> +++ b/drivers/net/ethernet/qlogic/qed/qed_devlink.c
> @@ -45,7 +45,55 @@ static const struct devlink_param qed_devlink_params[] = {
>  			     qed_dl_param_get, qed_dl_param_set, NULL),
>  };
>  
> -static const struct devlink_ops qed_dl_ops;
> +static int qed_devlink_info_get(struct devlink *devlink,
> +				struct devlink_info_req *req,
> +				struct netlink_ext_ack *extack)
> +{
> +	struct qed_devlink *qed_dl = devlink_priv(devlink);
> +	struct qed_dev *cdev = qed_dl->cdev;
> +	struct qed_dev_info *dev_info;
> +	char buf[100];
> +	int err;
> +
> +	dev_info = &cdev->common_dev_info;
> +
> +	err = devlink_info_driver_name_put(req, KBUILD_MODNAME);
> +	if (err)
> +		return err;
> +
> +	memcpy(buf, cdev->hwfns[0].hw_info.part_num, sizeof(cdev->hwfns[0].hw_info.part_num));
> +	buf[sizeof(cdev->hwfns[0].hw_info.part_num)] = 0;

Part number != serial number. What's the thing you're reporting here
actually identifying.

> +
> +	snprintf(buf, sizeof(buf), "%d.%d.%d.%d",
> +		 GET_MFW_FIELD(dev_info->mfw_rev, QED_MFW_VERSION_3),
> +		 GET_MFW_FIELD(dev_info->mfw_rev, QED_MFW_VERSION_2),
> +		 GET_MFW_FIELD(dev_info->mfw_rev, QED_MFW_VERSION_1),
> +		 GET_MFW_FIELD(dev_info->mfw_rev, QED_MFW_VERSION_0));
> +
> +	err = devlink_info_version_stored_put(req,
> +					      DEVLINK_INFO_VERSION_GENERIC_FW_MGMT, buf);
> +	if (err)
> +		return err;

Assuming MFW means management FW - this looks good.

> +	snprintf(buf, sizeof(buf), "%d.%d.%d.%d",
> +		 dev_info->fw_major,
> +		 dev_info->fw_minor,
> +		 dev_info->fw_rev,
> +		 dev_info->fw_eng);
> +
> +	return devlink_info_version_running_put(req,
> +						DEVLINK_INFO_VERSION_GENERIC_FW, buf);

But what's this one?