netdev - Re: [PATCH iwl-next] ice: use DSN instead of PCI BDF for ice

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: <pcmfqg3b5wg4cyzzjrpw23c6dwan62567vakbgnmto3khbwysk@dloxz3hqifdf>
Date: Fri, 7 Mar 2025 13:39:10 +0100
From: Jiri Pirko <jiri@...nulli.us>
To: Przemek Kitszel <przemyslaw.kitszel@...el.com>
Cc: intel-wired-lan@...ts.osuosl.org, 
	Tony Nguyen <anthony.l.nguyen@...el.com>, netdev@...r.kernel.org, Jacob Keller <jacob.e.keller@...el.com>, 
	Jakub Kicinski <kuba@...nel.org>, Aleksandr Loktionov <aleksandr.loktionov@...el.com>, 
	Karol Kolacinski <karol.kolacinski@...el.com>, Grzegorz Nitka <grzegorz.nitka@...el.com>, 
	Michal Schmidt <mschmidt@...hat.com>, Sergey Temerkhanov <sergey.temerkhanov@...el.com>
Subject: Re: [PATCH iwl-next] ice: use DSN instead of PCI BDF for ice_adapter
 index

Thu, Mar 06, 2025 at 10:11:46PM +0100, przemyslaw.kitszel@...el.com wrote:
>Use Device Serial Number instead of PCI bus/device/function for
>index of struct ice_adapter.
>Functions on the same physical device should point to the very same
>ice_adapter instance.
>
>This is not only simplification, but also fixes things up when PF
>is passed to VM (and thus has a random BDF).
>
>Suggested-by: Jacob Keller <jacob.e.keller@...el.com>
>Suggested-by: Jakub Kicinski <kuba@...nel.org>
>Suggested-by: Jiri Pirko <jiri@...nulli.us>
>Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@...el.com>
>Signed-off-by: Przemek Kitszel <przemyslaw.kitszel@...el.com>

>From my perspective, this is a bug fix, makes sense to me to send to
-net tree.


>---
>CC: Karol Kolacinski <karol.kolacinski@...el.com>
>CC: Grzegorz Nitka <grzegorz.nitka@...el.com>
>CC: Michal Schmidt <mschmidt@...hat.com>
>CC: Sergey Temerkhanov <sergey.temerkhanov@...el.com>
>---
> drivers/net/ethernet/intel/ice/ice_adapter.h |  4 +--
> drivers/net/ethernet/intel/ice/ice_adapter.c | 29 +++-----------------
> 2 files changed, 6 insertions(+), 27 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.h b/drivers/net/ethernet/intel/ice/ice_adapter.h
>index e233225848b3..1935163bd32f 100644
>--- a/drivers/net/ethernet/intel/ice/ice_adapter.h
>+++ b/drivers/net/ethernet/intel/ice/ice_adapter.h
>@@ -42,7 +42,7 @@ struct ice_adapter {
> 	struct ice_port_list ports;
> };
> 
>-struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev);
>-void ice_adapter_put(const struct pci_dev *pdev);
>+struct ice_adapter *ice_adapter_get(struct pci_dev *pdev);
>+void ice_adapter_put(struct pci_dev *pdev);
> 
> #endif /* _ICE_ADAPTER_H */
>diff --git a/drivers/net/ethernet/intel/ice/ice_adapter.c b/drivers/net/ethernet/intel/ice/ice_adapter.c
>index 01a08cfd0090..b668339ed0ef 100644
>--- a/drivers/net/ethernet/intel/ice/ice_adapter.c
>+++ b/drivers/net/ethernet/intel/ice/ice_adapter.c
>@@ -1,7 +1,6 @@
> // SPDX-License-Identifier: GPL-2.0-only
> // SPDX-FileCopyrightText: Copyright Red Hat
> 
>-#include <linux/bitfield.h>
> #include <linux/cleanup.h>
> #include <linux/mutex.h>
> #include <linux/pci.h>
>@@ -14,29 +13,9 @@
> static DEFINE_XARRAY(ice_adapters);
> static DEFINE_MUTEX(ice_adapters_mutex);
> 
>-/* PCI bus number is 8 bits. Slot is 5 bits. Domain can have the rest. */
>-#define INDEX_FIELD_DOMAIN GENMASK(BITS_PER_LONG - 1, 13)
>-#define INDEX_FIELD_DEV    GENMASK(31, 16)
>-#define INDEX_FIELD_BUS    GENMASK(12, 5)
>-#define INDEX_FIELD_SLOT   GENMASK(4, 0)
>-
>-static unsigned long ice_adapter_index(const struct pci_dev *pdev)
>+static unsigned long ice_adapter_index(struct pci_dev *pdev)
> {
>-	unsigned int domain = pci_domain_nr(pdev->bus);
>-
>-	WARN_ON(domain > FIELD_MAX(INDEX_FIELD_DOMAIN));
>-
>-	switch (pdev->device) {
>-	case ICE_DEV_ID_E825C_BACKPLANE:
>-	case ICE_DEV_ID_E825C_QSFP:
>-	case ICE_DEV_ID_E825C_SFP:
>-	case ICE_DEV_ID_E825C_SGMII:
>-		return FIELD_PREP(INDEX_FIELD_DEV, pdev->device);
>-	default:
>-		return FIELD_PREP(INDEX_FIELD_DOMAIN, domain) |
>-		       FIELD_PREP(INDEX_FIELD_BUS,    pdev->bus->number) |
>-		       FIELD_PREP(INDEX_FIELD_SLOT,   PCI_SLOT(pdev->devfn));
>-	}
>+	return (unsigned long)pci_get_dsn(pdev);

How do you ensure there is no xarray index collision then you
cut the number like this?


> }
> 
> static struct ice_adapter *ice_adapter_new(void)
>@@ -77,7 +56,7 @@ static void ice_adapter_free(struct ice_adapter *adapter)
>  * Return:  Pointer to ice_adapter on success.
>  *          ERR_PTR() on error. -ENOMEM is the only possible error.
>  */
>-struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev)
>+struct ice_adapter *ice_adapter_get(struct pci_dev *pdev)
> {
> 	unsigned long index = ice_adapter_index(pdev);
> 	struct ice_adapter *adapter;
>@@ -110,7 +89,7 @@ struct ice_adapter *ice_adapter_get(const struct pci_dev *pdev)
>  *
>  * Context: Process, may sleep.
>  */
>-void ice_adapter_put(const struct pci_dev *pdev)
>+void ice_adapter_put(struct pci_dev *pdev)
> {
> 	unsigned long index = ice_adapter_index(pdev);
> 	struct ice_adapter *adapter;
>-- 
>2.46.0
>