[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Z4oA8U3opS/7Ike0@mev-dev.igk.intel.com>
Date: Fri, 17 Jan 2025 08:04:17 +0100
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
To: Michael Chan <michael.chan@...adcom.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
kuba@...nel.org, pabeni@...hat.com, andrew+netdev@...n.ch,
pavan.chebbi@...adcom.com, andrew.gospodarek@...adcom.com,
helgaas@...nel.org, Manoj Panicker <manoj.panicker2@....com>,
Somnath Kotur <somnath.kotur@...adcom.com>,
Wei Huang <wei.huang2@....com>,
Ajit Khaparde <ajit.khaparde@...adcom.com>
Subject: Re: [PATCH net-next v2 10/10] bnxt_en: Add TPH support in BNXT driver
On Thu, Jan 16, 2025 at 11:23:43AM -0800, Michael Chan wrote:
> From: Manoj Panicker <manoj.panicker2@....com>
>
> Add TPH support to the Broadcom BNXT device driver. This allows the
> driver to utilize TPH functions for retrieving and configuring Steering
> Tags when changing interrupt affinity. With compatible NIC firmware,
> network traffic will be tagged correctly with Steering Tags, resulting
> in significant memory bandwidth savings and other advantages as
> demonstrated by real network benchmarks on TPH-capable platforms.
>
> Co-developed-by: Somnath Kotur <somnath.kotur@...adcom.com>
> Signed-off-by: Somnath Kotur <somnath.kotur@...adcom.com>
> Co-developed-by: Wei Huang <wei.huang2@....com>
> Signed-off-by: Wei Huang <wei.huang2@....com>
> Signed-off-by: Manoj Panicker <manoj.panicker2@....com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@...adcom.com>
> Reviewed-by: Andy Gospodarek <andrew.gospodarek@...adcom.com>
> Signed-off-by: Michael Chan <michael.chan@...adcom.com>
> ---
> Cc: Bjorn Helgaas <helgaas@...nel.org>
>
> Previous driver series fixing rtnl_lock and empty release function:
>
> https://lore.kernel.org/netdev/20241115200412.1340286-1-wei.huang2@amd.com/
>
> v5 of the PCI series using netdev_rx_queue_restart():
>
> https://lore.kernel.org/netdev/20240916205103.3882081-5-wei.huang2@amd.com/
>
> v1 of the PCI series using open/close:
>
> https://lore.kernel.org/netdev/20240509162741.1937586-9-wei.huang2@amd.com/
> ---
> drivers/net/ethernet/broadcom/bnxt/bnxt.c | 105 ++++++++++++++++++++++
> drivers/net/ethernet/broadcom/bnxt/bnxt.h | 7 ++
> 2 files changed, 112 insertions(+)
>
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 0a10a4cffcc8..8c24642b8812 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> @@ -55,6 +55,8 @@
> #include <net/page_pool/helpers.h>
> #include <linux/align.h>
> #include <net/netdev_queues.h>
> +#include <net/netdev_rx_queue.h>
> +#include <linux/pci-tph.h>
>
> #include "bnxt_hsi.h"
> #include "bnxt.h"
> @@ -11330,6 +11332,83 @@ static int bnxt_tx_queue_start(struct bnxt *bp, int idx)
> return 0;
> }
>
> +static void bnxt_irq_affinity_notify(struct irq_affinity_notify *notify,
> + const cpumask_t *mask)
> +{
> + struct bnxt_irq *irq;
> + u16 tag;
> + int err;
> +
> + irq = container_of(notify, struct bnxt_irq, affinity_notify);
> +
> + if (!irq->bp->tph_mode)
> + return;
> +
Can it not be set? The notifier is registered only if it is set, can
mode change while irq notifier is registered? Maybe I am missing sth,
but it looks like it can't.
> + cpumask_copy(irq->cpu_mask, mask);
> +
> + if (irq->ring_nr >= irq->bp->rx_nr_rings)
> + return;
> +
> + if (pcie_tph_get_cpu_st(irq->bp->pdev, TPH_MEM_TYPE_VM,
> + cpumask_first(irq->cpu_mask), &tag))
> + return;
> +
> + if (pcie_tph_set_st_entry(irq->bp->pdev, irq->msix_nr, tag))
> + return;
> +
> + rtnl_lock();
> + if (netif_running(irq->bp->dev)) {
> + err = netdev_rx_queue_restart(irq->bp->dev, irq->ring_nr);
> + if (err)
> + netdev_err(irq->bp->dev,
> + "RX queue restart failed: err=%d\n", err);
> + }
> + rtnl_unlock();
> +}
> +
> +static void bnxt_irq_affinity_release(struct kref *ref)
> +{
> + struct irq_affinity_notify *notify =
> + container_of(ref, struct irq_affinity_notify, kref);
> + struct bnxt_irq *irq;
> +
> + irq = container_of(notify, struct bnxt_irq, affinity_notify);
> +
> + if (!irq->bp->tph_mode)
The same here.
> + return;
> +
> + if (pcie_tph_set_st_entry(irq->bp->pdev, irq->msix_nr, 0)) {
> + netdev_err(irq->bp->dev,
> + "Setting ST=0 for MSIX entry %d failed\n",
> + irq->msix_nr);
> + return;
> + }
> +}
> +
> +static void bnxt_release_irq_notifier(struct bnxt_irq *irq)
> +{
> + irq_set_affinity_notifier(irq->vector, NULL);
> +}
> +
> +static void bnxt_register_irq_notifier(struct bnxt *bp, struct bnxt_irq *irq)
> +{
> + struct irq_affinity_notify *notify;
> +
> + irq->bp = bp;
> +
> + /* Nothing to do if TPH is not enabled */
> + if (!bp->tph_mode)
> + return;
> +
> + /* Register IRQ affinity notifier */
> + notify = &irq->affinity_notify;
> + notify->irq = irq->vector;
> + notify->notify = bnxt_irq_affinity_notify;
> + notify->release = bnxt_irq_affinity_release;
> +
> + irq_set_affinity_notifier(irq->vector, notify);
> +}
> +
> static void bnxt_free_irq(struct bnxt *bp)
> {
> struct bnxt_irq *irq;
> @@ -11352,11 +11431,18 @@ static void bnxt_free_irq(struct bnxt *bp)
> free_cpumask_var(irq->cpu_mask);
> irq->have_cpumask = 0;
> }
> +
> + bnxt_release_irq_notifier(irq);
> +
> free_irq(irq->vector, bp->bnapi[i]);
> }
>
> irq->requested = 0;
> }
> +
> + /* Disable TPH support */
> + pcie_disable_tph(bp->pdev);
> + bp->tph_mode = 0;
> }
>
> static int bnxt_request_irq(struct bnxt *bp)
> @@ -11376,6 +11462,12 @@ static int bnxt_request_irq(struct bnxt *bp)
> #ifdef CONFIG_RFS_ACCEL
> rmap = bp->dev->rx_cpu_rmap;
> #endif
> +
> + /* Enable TPH support as part of IRQ request */
> + rc = pcie_enable_tph(bp->pdev, PCI_TPH_ST_IV_MODE);
> + if (!rc)
> + bp->tph_mode = PCI_TPH_ST_IV_MODE;
> +
> for (i = 0, j = 0; i < bp->cp_nr_rings; i++) {
> int map_idx = bnxt_cp_num_to_irq_num(bp, i);
> struct bnxt_irq *irq = &bp->irq_tbl[map_idx];
> @@ -11399,8 +11491,11 @@ static int bnxt_request_irq(struct bnxt *bp)
>
> if (zalloc_cpumask_var(&irq->cpu_mask, GFP_KERNEL)) {
> int numa_node = dev_to_node(&bp->pdev->dev);
> + u16 tag;
>
> irq->have_cpumask = 1;
> + irq->msix_nr = map_idx;
> + irq->ring_nr = i;
> cpumask_set_cpu(cpumask_local_spread(i, numa_node),
> irq->cpu_mask);
> rc = irq_update_affinity_hint(irq->vector, irq->cpu_mask);
> @@ -11410,6 +11505,16 @@ static int bnxt_request_irq(struct bnxt *bp)
> irq->vector);
> break;
> }
> +
> + bnxt_register_irq_notifier(bp, irq);
> +
> + /* Init ST table entry */
> + if (pcie_tph_get_cpu_st(irq->bp->pdev, TPH_MEM_TYPE_VM,
> + cpumask_first(irq->cpu_mask),
> + &tag))
> + continue;
> +
> + pcie_tph_set_st_entry(irq->bp->pdev, irq->msix_nr, tag);
> }
> }
> return rc;
> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> index 826ae030fc09..02dc2ed9c75d 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
> @@ -1234,6 +1234,11 @@ struct bnxt_irq {
> u8 have_cpumask:1;
> char name[IFNAMSIZ + BNXT_IRQ_NAME_EXTRA];
> cpumask_var_t cpu_mask;
> +
> + struct bnxt *bp;
> + int msix_nr;
> + int ring_nr;
> + struct irq_affinity_notify affinity_notify;
> };
>
> #define HWRM_RING_ALLOC_TX 0x1
> @@ -2229,6 +2234,8 @@ struct bnxt {
> struct net_device *dev;
> struct pci_dev *pdev;
>
> + u8 tph_mode;
> +
> atomic_t intr_sem;
>
> u32 flags;
> --
> 2.30.1
>
Powered by blists - more mailing lists