| Message ID | 1328693798-27323-3-git-send-email-jeffrey.t.kirsher@intel.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
> -----Original Message----- > From: Kirsher, Jeffrey T > Sent: Wednesday, February 08, 2012 1:37 AM > To: davem@davemloft.net > Cc: Rose, Gregory V; netdev@vger.kernel.org; gospo@redhat.com; > sassmann@redhat.com; stable@vger.kernel.org; Kirsher, Jeffrey T > Subject: [net 2/8] igb: fix vf lookup > > From: Greg Rose <gregory.v.rose@intel.com> > > Recent addition of code to find already allocated VFs failed to take > account that systems with 2 or more multi-port SR-IOV capable controllers > might have already enabled VFs. Make sure that the VFs the function is > finding are actually subordinate to the particular instance of the adapter > that is looking for them and not subordinate to some device that has > previously enabled SR-IOV. > > This is applicable to 3.2+ kernels. > > CC: stable@vger.kernel.org > Reported-by: David Ahern <daahern@cisco.com> > Signed-off-by: Greg Rose <gregory.v.rose@intel.com> > Tested-by: Robert E Garrett <robertX.e.garrett@intel.com> > Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com> > --- > drivers/net/ethernet/intel/igb/igb_main.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c > b/drivers/net/ethernet/intel/igb/igb_main.c > index e91d73c..94be6c3 100644 > --- a/drivers/net/ethernet/intel/igb/igb_main.c > +++ b/drivers/net/ethernet/intel/igb/igb_main.c > @@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct igb_adapter > *adapter) > vf_devfn = pdev->devfn + 0x80; > pvfdev = pci_get_device(hw->vendor_id, device_id, NULL); > while (pvfdev) { > - if (pvfdev->devfn == vf_devfn) > + if (pvfdev->devfn == vf_devfn && > + (pvfdev->bus->number >= pdev->bus->number)) > vfs_found++; > vf_devfn += vf_stride; > pvfdev = pci_get_device(hw->vendor_id, > -- > 1.7.7.6 I'll fix this one too. You start leaning on checkpatch and you get lazy I guess. - Greg -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2012-02-08 at 23:42 +0000, Rose, Gregory V wrote: > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c [] > > @@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct igb_adapter > > *adapter) > > vf_devfn = pdev->devfn + 0x80; > > pvfdev = pci_get_device(hw->vendor_id, device_id, NULL); > > while (pvfdev) { > > - if (pvfdev->devfn == vf_devfn) > > + if (pvfdev->devfn == vf_devfn && > > + (pvfdev->bus->number >= pdev->bus->number)) > > vfs_found++; [] > I'll fix this one too. You start leaning on checkpatch and you get lazy I guess. I suppose an indentation rule could be created when arguments on multiple lines don't align at the open parenthesis, but I'm not going to rewrite emacs indentation rules. Presumably it should only be used with --strict. Anyone think multiple line tests with inequivalent uses of parentheses like this one should be noted as well? -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Joe Perches <joe@perches.com> Date: Wed, 08 Feb 2012 15:49:40 -0800 > On Wed, 2012-02-08 at 23:42 +0000, Rose, Gregory V wrote: >> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c > [] >> > @@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct igb_adapter >> > *adapter) >> > vf_devfn = pdev->devfn + 0x80; >> > pvfdev = pci_get_device(hw->vendor_id, device_id, NULL); >> > while (pvfdev) { >> > - if (pvfdev->devfn == vf_devfn) >> > + if (pvfdev->devfn == vf_devfn && >> > + (pvfdev->bus->number >= pdev->bus->number)) >> > vfs_found++; > [] >> I'll fix this one too. You start leaning on checkpatch and you get lazy I guess. > > I suppose an indentation rule could be created when > arguments on multiple lines don't align at the open > parenthesis, but I'm not going to rewrite emacs > indentation rules. > > Presumably it should only be used with --strict. > > Anyone think multiple line tests with inequivalent uses > of parentheses like this one should be noted as well? Actually I thought this case was perfectly fine. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> -----Original Message----- > From: David Miller [mailto:davem@davemloft.net] > Sent: Wednesday, February 08, 2012 3:53 PM > To: joe@perches.com > Cc: Rose, Gregory V; Kirsher, Jeffrey T; netdev@vger.kernel.org; > gospo@redhat.com; sassmann@redhat.com; stable@vger.kernel.org > Subject: Re: [net 2/8] igb: fix vf lookup > > From: Joe Perches <joe@perches.com> > Date: Wed, 08 Feb 2012 15:49:40 -0800 > > > On Wed, 2012-02-08 at 23:42 +0000, Rose, Gregory V wrote: > >> > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c > > [] > >> > @@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct > igb_adapter > >> > *adapter) > >> > vf_devfn = pdev->devfn + 0x80; > >> > pvfdev = pci_get_device(hw->vendor_id, device_id, NULL); > >> > while (pvfdev) { > >> > - if (pvfdev->devfn == vf_devfn) > >> > + if (pvfdev->devfn == vf_devfn && > >> > + (pvfdev->bus->number >= pdev->bus->number)) > >> > vfs_found++; > > [] > >> I'll fix this one too. You start leaning on checkpatch and you get > lazy I guess. > > > > I suppose an indentation rule could be created when > > arguments on multiple lines don't align at the open > > parenthesis, but I'm not going to rewrite emacs > > indentation rules. > > > > Presumably it should only be used with --strict. > > > > Anyone think multiple line tests with inequivalent uses > > of parentheses like this one should be noted as well? > > Actually I thought this case was perfectly fine. The imbalanced parenthesis usage bothers me. And yes, if you're going to have a tool that checks patch formatting it'd be nice if it caught things like this. But then I'm the lazy fool here... - Greg -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, 2012-02-08 at 15:49 -0800, Joe Perches wrote: > On Wed, 2012-02-08 at 23:42 +0000, Rose, Gregory V wrote: > > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c > [] > > > @@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct igb_adapter > > > *adapter) > > > vf_devfn = pdev->devfn + 0x80; > > > pvfdev = pci_get_device(hw->vendor_id, device_id, NULL); > > > while (pvfdev) { > > > - if (pvfdev->devfn == vf_devfn) > > > + if (pvfdev->devfn == vf_devfn && > > > + (pvfdev->bus->number >= pdev->bus->number)) > > > vfs_found++; > [] > > I'll fix this one too. You start leaning on checkpatch and you get lazy I guess. > > I suppose an indentation rule could be created when > arguments on multiple lines don't align at the open > parenthesis, but I'm not going to rewrite emacs > indentation rules. > > Presumably it should only be used with --strict. > > Anyone think multiple line tests with inequivalent uses > of parentheses like this one should be noted as well? > I tried to create this very fix last year, but I must admit my checkpatch.pl foo was not that strong... :) While it would be a good enhancement to checkpatch.pl, I do not see it as a "critical" fix.
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c index e91d73c..94be6c3 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -5012,7 +5012,8 @@ static int igb_find_enabled_vfs(struct igb_adapter *adapter) vf_devfn = pdev->devfn + 0x80; pvfdev = pci_get_device(hw->vendor_id, device_id, NULL); while (pvfdev) { - if (pvfdev->devfn == vf_devfn) + if (pvfdev->devfn == vf_devfn && + (pvfdev->bus->number >= pdev->bus->number)) vfs_found++; vf_devfn += vf_stride; pvfdev = pci_get_device(hw->vendor_id,