| 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
| ||
|
Message-ID:
<BN5PR12MB9486BC66168772767F4D8BAAAFE62@BN5PR12MB9486.namprd12.prod.outlook.com>
Date: Tue, 21 Jan 2025 22:15:01 +0000
From: "Panicker, Manoj" <Manoj.Panicker2@....com>
To: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>, Michael Chan
<michael.chan@...adcom.com>
CC: "davem@...emloft.net" <davem@...emloft.net>, "netdev@...r.kernel.org"
<netdev@...r.kernel.org>, "edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "pavan.chebbi@...adcom.com"
<pavan.chebbi@...adcom.com>, "andrew.gospodarek@...adcom.com"
<andrew.gospodarek@...adcom.com>, "helgaas@...nel.org" <helgaas@...nel.org>,
Somnath Kotur <somnath.kotur@...adcom.com>, "Huang2, Wei"
<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
Hello Michal,
The driver changes currently attempt to enable TPH only for the Interrupt Vector Mode operation when interrupts are being enabled. The check you pointed out should fall through to the rest of the notifier code since the notifier is registered only when the tph_mode is set currently. I believe the check was added in anticipation of further changes in TPH support like NoST mode, for example, but that is not part of this submission.
Thanks
Manoj
-----Original Message-----
From: Michal Swiatkowski <michal.swiatkowski@...ux.intel.com>
Sent: Thursday, January 16, 2025 11:04 PM
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; Panicker, Manoj <Manoj.Panicker2@....com>; Somnath Kotur <somnath.kotur@...adcom.com>; Huang2, Wei <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
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
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