netdev - Re: [PATCH 0/2] Replace driver's usage of hard-coded device IDs to #defines

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: <20170616200820.GE11129@bhelgaas-glaptop.roam.corp.google.com>
Date:   Fri, 16 Jun 2017 15:08:20 -0500
From:   Bjorn Helgaas <helgaas@...nel.org>
To:     Myron Stowe <mstowe@...hat.com>
Cc:     David Miller <davem@...emloft.net>, myron.stowe@...hat.com,
        linux-pci@...r.kernel.org, netdev@...r.kernel.org,
        bhelgaas@...gle.com, saeedm@...lanox.com, noaos@...lanox.com,
        tariqt@...lanox.com
Subject: Re: [PATCH 0/2] Replace driver's usage of hard-coded device IDs to
 #defines

On Thu, May 25, 2017 at 09:56:55AM -0600, Myron Stowe wrote:
> On Wed, 24 May 2017 20:02:49 -0400 (EDT)
> David Miller <davem@...emloft.net> wrote:
> 
> > From: Myron Stowe <myron.stowe@...hat.com>
> > Date: Wed, 24 May 2017 16:47:34 -0600
> > 
> > > Noa Osherovich introduced a series of new Mellanox device ID
> > > definitions to help differentiate specific controllers that needed
> > > INTx masking quirks [1].
> > > 
> > > Bjorn Helgaas followed on, using the device ID definitions Noa
> > > provided to replace hard-coded values within the mxl4 ID table [2].
> > >     
> > > This patch continues along similar lines, adding a few additional
> > > Mellanox device ID definitions and converting the net/mlx5e
> > > driver's mlx5 ID table to use the defines so tools like 'grep' and
> > > 'cscope' can be used to help identify relationships with other
> > > aspects (such as INTx masking).  
> > 
> > If you're adding pci_ids.h defines, it's only valid to do so if you
> > actually use the defines in more than one location.
> > 
> > This patch series is not doing that.
> 
> Hi David,
> 
> Yes, now that you mention that again I do vaguely remember past
> conversations stating similar constraints which is a little odd as
> Noa's series did exactly that.  It was Bjorn, in a separate patch, that
> made the connection to the driver with commit c19e4b9037f
> ("net/mlx4_core: Use device ID defines") [1] and even after such, some
> of the introduced #defines are still currently singular in usage.
> 
> Anyway, the part I'm interested in is creating a more transparent
> association between the Mellanox controllers that need the INTx masking
> quirk and their drivers, something that remains very opaque currently
> for a few of the remaining instances (PCI_DEVICE_ID_MELLANOX_CONNECTIB,
> PCI_DEVICE_ID_MELLANOX_CONNECTX4, and
> PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX).

I think what you want is the patch below (your patch 2, after removing
CONNECTX5, CONNECTX5_EX, and CONNECTX6 since they're only used in one
place).

We added definitions for CONNECTIB, CONNECTX4, and CONNECTX4_LX and uses of
them in a quirk via:

  7254383341bc ("PCI: Add Mellanox device IDs")
  d76d2fe05fd9 ("PCI: Convert Mellanox broken INTx quirks to be for listed
  devices only")

But somehow we missed using those in mlx5/core/main.c.

The patch below doesn't touch PCI, so it would be just for netdev.

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/main.c b/drivers/net/ethernet/mellanox/mlx5/core/main.c
index 0c123d571b4c..8a4e292f26b8 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/main.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/main.c
@@ -1508,11 +1508,11 @@ static void shutdown(struct pci_dev *pdev)
 }
 
 static const struct pci_device_id mlx5_core_pci_table[] = {
-	{ PCI_VDEVICE(MELLANOX, 0x1011) },			/* Connect-IB */
+	{ PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTIB) },
 	{ PCI_VDEVICE(MELLANOX, 0x1012), MLX5_PCI_DEV_IS_VF},	/* Connect-IB VF */
-	{ PCI_VDEVICE(MELLANOX, 0x1013) },			/* ConnectX-4 */
+	{ PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTX4) },
 	{ PCI_VDEVICE(MELLANOX, 0x1014), MLX5_PCI_DEV_IS_VF},	/* ConnectX-4 VF */
-	{ PCI_VDEVICE(MELLANOX, 0x1015) },			/* ConnectX-4LX */
+	{ PCI_VDEVICE(MELLANOX, PCI_DEVICE_ID_MELLANOX_CONNECTX4_LX) },
 	{ PCI_VDEVICE(MELLANOX, 0x1016), MLX5_PCI_DEV_IS_VF},	/* ConnectX-4LX VF */
 	{ PCI_VDEVICE(MELLANOX, 0x1017) },			/* ConnectX-5, PCIe 3.0 */
 	{ PCI_VDEVICE(MELLANOX, 0x1018), MLX5_PCI_DEV_IS_VF},	/* ConnectX-5 VF */