netdev - RE: [PATCH iwl-next v1 02/13] ixgbe: add handler for devlink .info

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 for Android: free password hash cracker in your pocket

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

Message-ID: <DS0PR11MB7785F42A55BDDBA791D14AE9F0F22@DS0PR11MB7785.namprd11.prod.outlook.com>
Date: Mon, 10 Feb 2025 11:06:47 +0000
From: "Jagielski, Jedrzej" <jedrzej.jagielski@...el.com>
To: Simon Horman <horms@...nel.org>
CC: "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
	"Nguyen, Anthony L" <anthony.l.nguyen@...el.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "Polchlopek, Mateusz"
	<mateusz.polchlopek@...el.com>
Subject: RE: [PATCH iwl-next v1 02/13] ixgbe: add handler for devlink
 .info_get()

From: Simon Horman <horms@...nel.org> 
Sent: Sunday, February 9, 2025 5:27 PM
>On Mon, Feb 03, 2025 at 04:03:17PM +0100, Jedrzej Jagielski wrote:
>> Provide devlink .info_get() callback implementation to allow the
>> driver to report detailed version information. The following info
>> is reported:
>> 
>>  "serial_number" -> The PCI DSN of the adapter
>>  "fw.bundle_id" -> Unique identifier for the combined flash image
>>  "fw.undi" -> Version of the Option ROM containing the UEFI driver
>>  "board.id" -> The PBA ID string
>> 
>> Reviewed-by: Mateusz Polchlopek <mateusz.polchlopek@...el.com>
>> Signed-off-by: Jedrzej Jagielski <jedrzej.jagielski@...el.com>
>
>...
>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c b/drivers/net/ethernet/intel/ixgbe/devlink/devlink.c
>
>...
>
>> +static void ixgbe_info_nvm_ver(struct ixgbe_adapter *adapter,
>> +			       struct ixgbe_info_ctx *ctx)
>> +{
>> +	struct ixgbe_hw *hw = &adapter->hw;
>> +	struct ixgbe_nvm_version nvm_ver;
>> +
>> +	ixgbe_get_oem_prod_version(hw, &nvm_ver);
>> +	if (nvm_ver.oem_valid) {
>> +		snprintf(ctx->buf, sizeof(ctx->buf), "%x.%x.%x",
>> +			 nvm_ver.oem_major, nvm_ver.oem_minor,
>> +			 nvm_ver.oem_release);
>> +
>> +		return;
>> +	}
>> +
>> +	ixgbe_get_orom_version(hw, &nvm_ver);
>> +	if (nvm_ver.or_valid)
>> +		snprintf(ctx->buf, sizeof(ctx->buf), "%d.%d.%d",
>> +			 nvm_ver.or_major, nvm_ver.or_build, nvm_ver.or_patch);
>
>Hi Jedrzej,
>
>If neither of the conditions above are met then it seems that ctx->buf will
>contain whatever string was present when the function was called. Is
>something like the following needed here?
>
>	ctx->buf[0] = '\0';

Hi Simon,
thanks for suggestion, it's definitely worth do be incorporated in the next
revision.

Thanks!

>
>> +}
>> +
>> +static void ixgbe_info_eetrack(struct ixgbe_adapter *adapter,
>> +			       struct ixgbe_info_ctx *ctx)
>> +{
>> +	struct ixgbe_hw *hw = &adapter->hw;
>> +	struct ixgbe_nvm_version nvm_ver;
>> +
>> +	ixgbe_get_oem_prod_version(hw, &nvm_ver);
>> +	/* No ETRACK version for OEM */
>> +	if (nvm_ver.oem_valid)
>> +		return;
>
>Likewise, here.
>
>> +
>> +	ixgbe_get_etk_id(hw, &nvm_ver);
>> +	snprintf(ctx->buf, sizeof(ctx->buf), "0x%08x", nvm_ver.etk_id);
>> +}
>> +
>> +static int ixgbe_devlink_info_get(struct devlink *devlink,
>> +				  struct devlink_info_req *req,
>> +				  struct netlink_ext_ack *extack)
>> +{
>> +	struct ixgbe_devlink_priv *devlink_private = devlink_priv(devlink);
>> +	struct ixgbe_adapter *adapter = devlink_private->adapter;
>> +	struct ixgbe_hw *hw = &adapter->hw;
>> +	struct ixgbe_info_ctx *ctx;
>> +	int err;
>> +
>> +	ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
>> +	if (!ctx)
>> +		return -ENOMEM;
>> +
>> +	ixgbe_info_get_dsn(adapter, ctx);
>> +	err = devlink_info_serial_number_put(req, ctx->buf);
>> +	if (err)
>> +		goto free_ctx;
>> +
>> +	ixgbe_info_nvm_ver(adapter, ctx);
>> +	err = ixgbe_devlink_info_put(req, IXGBE_DL_VERSION_RUNNING,
>> +				     DEVLINK_INFO_VERSION_GENERIC_FW_UNDI,
>> +				     ctx->buf);
>> +	if (err)
>> +		goto free_ctx;
>> +
>> +	ixgbe_info_eetrack(adapter, ctx);
>> +	err = ixgbe_devlink_info_put(req, IXGBE_DL_VERSION_RUNNING,
>> +				     DEVLINK_INFO_VERSION_GENERIC_FW_BUNDLE_ID,
>> +				     ctx->buf);
>> +	if (err)
>> +		goto free_ctx;
>> +
>> +	err = ixgbe_read_pba_string_generic(hw, ctx->buf, sizeof(ctx->buf));
>> +	if (err)
>> +		goto free_ctx;
>> +
>> +	err = ixgbe_devlink_info_put(req, IXGBE_DL_VERSION_FIXED,
>> +				     DEVLINK_INFO_VERSION_GENERIC_BOARD_ID,
>> +				     ctx->buf);
>> +free_ctx:
>> +	kfree(ctx);
>> +	return err;
>> +}
>
>...