On Thu, Jun 04, 2026 at 12:45:03PM +0200, Paolo Abeni wrote:
> On 6/1/26 12:27 PM, Shradha Gupta wrote:
> > In mana driver, the number of IRQs allocated is capped by the
> > min(num_cpu + 1, queue count). In cases, where the IRQ count is greater
> > than the vcpu count, we want to utilize all the vCPUs, irrespective of
> > their NUMA/core bindings.
> >
> > This is important, especially in the envs where number of vCPUs are so
> > few that the softIRQ handling overhead on two IRQs on the same vCPU is
> > much more than their overheads if they were spread across sibling vCPUs.
> >
> > This behaviour is more evident with dynamic IRQ allocation. Since MANA
> > IRQs are assigned at a later stage compared to static allocation, other
> > device IRQs may already be affinitized to the vCPUs. As a result, IRQ
> > weights become imbalanced, causing multiple MANA IRQs to land on the
> > same vCPU, while some vCPUs have none.
> >
> > In such cases when many parallel TCP connections are tested, the
> > throughput drops significantly.
> >
> > Test envs:
> > =======================================================
> > Case 1: without this patch
> > =======================================================
> > 4 vcpu(2 cores), 5 MANA IRQs (1 HWC + 4 Queue)
> >
> > TYPE effective vCPU aff
> > =======================================================
> > IRQ0: HWC 0
> > IRQ1: mana_q1 0
> > IRQ2: mana_q2 2
> > IRQ3: mana_q3 0
> > IRQ4: mana_q4 3
> >
> > %soft on each vCPU(mpstat -P ALL 1) on receiver
> > vCPU 0 1 2 3
> > =======================================================
> > pass 1: 38.85 0.03 24.89 24.65
> > pass 2: 39.15 0.03 24.57 25.28
> > pass 3: 40.36 0.03 23.20 23.17
> >
> > =======================================================
> > Case 2: with this patch
> > =======================================================
> > 4 vcpu(2 cores), 5 MANA IRQs (1 HWC + 4 Queue)
> >
> > TYPE effective vCPU aff
> > =======================================================
> > IRQ0: HWC 0
> > IRQ1: mana_q1 0
> > IRQ2: mana_q2 1
> > IRQ3: mana_q3 2
> > IRQ4: mana_q4 3
> >
> > %soft on each vCPU(mpstat -P ALL 1) on receiver
> > vCPU 0 1 2 3
> > =======================================================
> > pass 1: 15.42 15.85 14.99 14.51
> > pass 2: 15.53 15.94 15.81 15.93
> > pass 3: 16.41 16.35 16.40 16.36
> >
> > =======================================================
> > Throughput Impact(in Gbps, same env)
> > =======================================================
> > TCP conn with patch w/o patch
> > 20480 15.65 7.73
> > 10240 15.63 8.93
> > 8192 15.64 9.69
> > 6144 15.64 13.16
> > 4096 15.69 15.75
> > 2048 15.69 15.83
> > 1024 15.71 15.28
> >
> > Fixes: 755391121038 ("net: mana: Allocate MSI-X vectors dynamically")
> > Cc: [email protected]
> > Co-developed-by: Erni Sri Satya Vennela <[email protected]>
> > Signed-off-by: Erni Sri Satya Vennela <[email protected]>
> > Signed-off-by: Shradha Gupta <[email protected]>
> > Reviewed-by: Haiyang Zhang <[email protected]>
> > Reviewed-by: Simon Horman <[email protected]>
>
> Why do you consider this patch a fix? To me is a configuration
> improvement and should land on net-next.
Hi Paolo,
This is a fix for commit 755391121038 ("net: mana: Allocate MSI-X
vectors dynamically"). Before that commit, IRQs were statically
allocated and clustering of MANA IRQs happened less often on low vCPU
configs. With dynamic allocation, MANA IRQs are assigned at a later
stage when other device IRQs have already occupied vCPUs. The NUMA-aware
affinity logic in that commit increased the probability of IRQ
clustering, causing a 2x throughput regression (15.65 vs 7.73 Gbps) on
low vCPU Azure SKUs at high connection counts.
>
> > @@ -1717,11 +1719,24 @@ static int irq_setup(unsigned int *irqs, unsigned
> > int len, int node,
> > return 0;
> > }
> >
> > +/* should be called with cpus_read_lock() held */
>
> Minor nit: s/should/must/ or just drop the comment, as
> `for_each_online_cpu()` usage implies that.
>
Thanks, will change this in next version.
> > +static void irq_setup_linear(unsigned int *irqs, unsigned int len)
> > +{
> > + int cpu;
> > +
> > + for_each_online_cpu(cpu) {
> > + if (len == 0)
> > + break;
> > +
> > + irq_set_affinity_and_hint(*irqs++, cpumask_of(cpu));
> > + len--;
> > + }
>
> As this is another heuristic regarding irq spreading, why don't you
> implement that inside irq_setup()?
>
irq_setup() already handles multiple cases - dynamic, static, and HWC
affinity logic with NUMA-aware sibling group spreading. Adding the
linear case there would make it more complex and harder to follow.
Keeping it as a separate function makes both, NUMA aware and linear
paths easier to understand and maintain.
Happy to reconsider if you feel strongly about it.
> > @@ -1767,13 +1784,42 @@ static int mana_gd_setup_dyn_irqs(struct pci_dev
> > *pdev, int nvec)
> > * first CPU sibling group since they are already affinitized to HWC IRQ
> > */
> > cpus_read_lock();
> > - if (gc->num_msix_usable <= num_online_cpus())
> > - skip_first_cpu = true;
> > + if (gc->num_msix_usable <= num_online_cpus()) {
> > + err = irq_setup(irqs, nvec, gc->numa_node, true);
> > + if (err) {
> > + cpus_read_unlock();
> > + goto free_irq;
> > + }
> > + } else {
> > + /*
> > + * When num_msix_usable are more than num_online_cpus, our
> > + * queue IRQs should be equal to num of online vCPUs.
> > + * We try to make sure queue IRQs spread across all vCPUs.
> > + * In such a case NUMA or CPU core affinity does not matter.
> > + * Note: in this case the total mana IRQ should always be
> > + * num_online_cpus + 1. The first HWC IRQ is already handled
> > + * in HWC setup calls
> > + * However, if CPUs went offline since num_msix_usable was
> > + * computed, queue IRQs will be more than num_online_cpus().
> > + * In such cases remaining extra IRQs will retain their default
> > + * affinity.
> > + */
> > + int first_unassigned = num_online_cpus();
> > + if (nvec > first_unassigned) {
>
> An empty line is needed between the variable declaration and the code.
noted, Thanks.
>
> /P
