Re: [PATCH net V2 2/4] net/mlx5: Lag, limit aggregated TX speed by PCIe bandwidth

[Bjorn Helgaas]

On Thu, Oct 22, 2026 at 10:30:51PM +0300, Tariq Toukan wrote:
> From: Or Har-Toov <ohartoov@xxxxxxxxxx>
> 
> FW initializes max_tx_speed to the minimum of the port speed and the
> PCI bandwidth. Modifying it with a value computed by different rules
> changes the meaning of the field rather than updating it.
> 
> Limit the aggregated LAG TX speed by the PCIe bandwidth. Clamping each
> uplink separately and then summing could let the aggregate reach a
> multiple of the link's real bandwidth, so clamp the sum once instead,
> counting only the ports that contribute to it.
> 
> This assumes the bonded PFs share a single PCIe link, which holds when
> they are functions of the same device. In Socket Direct, each port spans
> multiple mdevs on separate PCIe links, so the PCIe budget for that port
> is the sum of its mdevs' PCIe bandwidths. Where Socket Direct ports do
> not have equal PCIe budgets, the aggregate is clamped to the narrowest
> port, which under-reports rather than over-reports the available
> bandwidth.
> 
> pcie_bandwidth_mbps() reads the endpoint link only, without traversing
> upstream bridges, and uses the nominal per-lane transfer rate times the
> link width, without encoding overhead. FW initializes max_tx_speed from
> the same endpoint-only link status, so using the same calculation keeps
> the clamp consistent with the value FW programs before LAG. Ethernet
> speeds and max_tx_speed are likewise nominal rates, so the comparison is
> like for like. A more accurate bandwidth estimate would make the
> aggregate differ from the per-vport value FW sets up for the same link.
> 
> If the PCIe bandwidth of a contributing port cannot be read, the speed
> calculation fails instead of returning an unclamped or partial result.
> A device that has no Link Status register, an RC integrated endpoint,
> has no PCIe budget to apply and is not clamped.
> 
> Fixes: 50f1d188c580 ("net/mlx5: Propagate LAG effective max_tx_speed to vports")
> Signed-off-by: Or Har-Toov <ohartoov@xxxxxxxxxx>
> Reviewed-by: Mark Bloch <mbloch@xxxxxxxxxx>
> Reviewed-by: Shay Drori <shayd@xxxxxxxxxx>
> Signed-off-by: Tariq Toukan <tariqt@xxxxxxxxxx>
> ...

> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8cc6a89130b5..064eef237d6f 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -6006,6 +6006,22 @@ int pcie_link_speed_mbps(struct pci_dev *pdev)
>  }
>  EXPORT_SYMBOL(pcie_link_speed_mbps);
>  
> +u32 pcie_bandwidth_mbps(struct pci_dev *pdev)
> +{
> +	u16 lnksta;
> +	u32 width;
> +	int speed;
> +
> +	if (pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnksta))
> +		return 0;
> +	speed = pcie_dev_speed_mbps(to_pcie_link_speed(lnksta));
> +	if (speed <= 0)
> +		return 0;
> +	width = FIELD_GET(PCI_EXP_LNKSTA_NLW, lnksta);
> +	return speed * width;
> +}
> +EXPORT_SYMBOL_GPL(pcie_bandwidth_mbps);

Can you split this into its own patch?

It looks like cxl_pci_get_bandwidth() and maybe even
pcie_bandwidth_available() could be refactored to use this.

>  /**
>   * pcie_bandwidth_available - determine minimum link settings of a PCIe
>   *			      device and its bandwidth limitation
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 5ffcf5ce8c96..202bc8680601 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1479,6 +1479,8 @@ u32 pcie_bandwidth_available(struct pci_dev *dev, struct pci_dev **limiting_dev,
>  			     enum pci_bus_speed *speed,
>  			     enum pcie_link_width *width);
>  int pcie_link_speed_mbps(struct pci_dev *pdev);
> +u32 pcie_bandwidth_mbps(struct pci_dev *pdev);
> +
>  void pcie_print_link_status(struct pci_dev *dev);
>  int pcie_reset_flr(struct pci_dev *dev, bool probe);
>  int pcie_flr(struct pci_dev *dev);
> -- 
> 2.44.0
>