RE: [PATCH v5 12/14] fm10k: Report PCIe link properties with pcie_print_link_status()

[Keller, Jacob E]

> -----Original Message-----
> From: netdev-owner@xxxxxxxxxxxxxxx [mailto:netdev-owner@xxxxxxxxxxxxxxx]
> On Behalf Of Bjorn Helgaas
> Sent: Monday, April 02, 2018 1:32 PM
> To: Keller, Jacob E <jacob.e.keller@xxxxxxxxx>
> Cc: Tal Gilboa <talgi@xxxxxxxxxxxx>; Tariq Toukan <tariqt@xxxxxxxxxxxx>; Ariel
> Elior <ariel.elior@xxxxxxxxxx>; Ganesh Goudar <ganeshgr@xxxxxxxxxxx>;
> Kirsher, Jeffrey T <jeffrey.t.kirsher@xxxxxxxxx>; everest-linux-l2@xxxxxxxxxx;
> intel-wired-lan@xxxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH v5 12/14] fm10k: Report PCIe link properties with
> pcie_print_link_status()
> 
> On Mon, Apr 02, 2018 at 03:56:06PM +0000, Keller, Jacob E wrote:
> > > -----Original Message-----
> > > From: Bjorn Helgaas [mailto:helgaas@xxxxxxxxxx]
> > > Sent: Friday, March 30, 2018 2:06 PM
> > > To: Tal Gilboa <talgi@xxxxxxxxxxxx>
> > > Cc: Tariq Toukan <tariqt@xxxxxxxxxxxx>; Keller, Jacob E
> > > <jacob.e.keller@xxxxxxxxx>; Ariel Elior <ariel.elior@xxxxxxxxxx>; Ganesh
> > > Goudar <ganeshgr@xxxxxxxxxxx>; Kirsher, Jeffrey T
> > > <jeffrey.t.kirsher@xxxxxxxxx>; everest-linux-l2@xxxxxxxxxx; intel-wired-
> > > lan@xxxxxxxxxxxxxxxx; netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx;
> > > linux-pci@xxxxxxxxxxxxxxx
> > > Subject: [PATCH v5 12/14] fm10k: Report PCIe link properties with
> > > pcie_print_link_status()
> > >
> > > From: Bjorn Helgaas <bhelgaas@xxxxxxxxxx>
> > >
> > > Use pcie_print_link_status() to report PCIe link speed and possible
> > > limitations instead of implementing this in the driver itself.
> > >
> > > Note that pcie_get_minimum_link() can return misleading information
> because
> > > it finds the slowest link and the narrowest link without considering the
> > > total bandwidth of the link.  If the path contains a 16 GT/s x1 link and a
> > > 2.5 GT/s x16 link, pcie_get_minimum_link() returns 2.5 GT/s x1, which
> > > corresponds to 250 MB/s of bandwidth, not the actual available bandwidth of
> > > about 2000 MB/s for a 16 GT/s x1 link.
> >
> > This comment is about what's being fixed, so it would have been easier to
> > parse if it were written to more clearly indicate that we're removing
> > (and not adding) this behavior.
> 
> Good point.  Is this any better?
> 
>   fm10k: Report PCIe link properties with pcie_print_link_status()
> 
>   Previously the driver used pcie_get_minimum_link() to warn when the NIC
>   is in a slot that can't supply as much bandwidth as the NIC could use.
> 
>   pcie_get_minimum_link() can be misleading because it finds the slowest link
>   and the narrowest link (which may be different links) without considering
>   the total bandwidth of each link.  For a path with a 16 GT/s x1 link and a
>   2.5 GT/s x16 link, it returns 2.5 GT/s x1, which corresponds to 250 MB/s of
>   bandwidth, not the true available bandwidth of about 1969 MB/s for a
>   16 GT/s x1 link.
> 
>   Use pcie_print_link_status() to report PCIe link speed and possible
>   limitations instead of implementing this in the driver itself.  This finds
>   the slowest link in the path to the device by computing the total bandwidth
>   of each link and compares that with the capabilities of the device.
> 
>   Note that the driver previously used dev_warn() to suggest using a
>   different slot, but pcie_print_link_status() uses dev_info() because if the
>   platform has no faster slot available, the user can't do anything about the
>   warning and may not want to be bothered with it.

Perfect! Thanks!

-Jake