* [PATCH iwl-next v3] ice: use netif_get_num_default_rss_queues()
@ 2025-10-30 8:30 Michal Swiatkowski
2025-10-30 9:10 ` Paul Menzel
0 siblings, 1 reply; 7+ messages in thread
From: Michal Swiatkowski @ 2025-10-30 8:30 UTC (permalink / raw)
To: intel-wired-lan
Cc: netdev, pmenzel, aleksander.lobakin, przemyslaw.kitszel,
jacob.e.keller, Michal Swiatkowski, Aleksandr Loktionov
On some high-core systems (like AMD EPYC Bergamo, Intel Clearwater
Forest) loading ice driver with default values can lead to queue/irq
exhaustion. It will result in no additional resources for SR-IOV.
In most cases there is no performance reason for more than half
num_cpus(). Limit the default value to it using generic
netif_get_num_default_rss_queues().
Still, using ethtool the number of queues can be changed up to
num_online_cpus(). It can be done by calling:
$ethtool -L ethX combined $(nproc)
This change affects only the default queue amount.
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
---
v2 --> v3:
* use $(nproc) in command example in commit message
v1 --> v2:
* Follow Olek's comment and switch from custom limiting to the generic
netif_...() function.
* Add more info in commit message (Paul)
* Dropping RB tags, as it is different patch now
---
drivers/net/ethernet/intel/ice/ice_irq.c | 5 +++--
drivers/net/ethernet/intel/ice/ice_lib.c | 12 ++++++++----
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/ice/ice_irq.c b/drivers/net/ethernet/intel/ice/ice_irq.c
index 30801fd375f0..1d9b2d646474 100644
--- a/drivers/net/ethernet/intel/ice/ice_irq.c
+++ b/drivers/net/ethernet/intel/ice/ice_irq.c
@@ -106,9 +106,10 @@ static struct ice_irq_entry *ice_get_irq_res(struct ice_pf *pf,
#define ICE_RDMA_AEQ_MSIX 1
static int ice_get_default_msix_amount(struct ice_pf *pf)
{
- return ICE_MIN_LAN_OICR_MSIX + num_online_cpus() +
+ return ICE_MIN_LAN_OICR_MSIX + netif_get_num_default_rss_queues() +
(test_bit(ICE_FLAG_FD_ENA, pf->flags) ? ICE_FDIR_MSIX : 0) +
- (ice_is_rdma_ena(pf) ? num_online_cpus() + ICE_RDMA_AEQ_MSIX : 0);
+ (ice_is_rdma_ena(pf) ? netif_get_num_default_rss_queues() +
+ ICE_RDMA_AEQ_MSIX : 0);
}
/**
diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
index bac481e8140d..e366d089bef9 100644
--- a/drivers/net/ethernet/intel/ice/ice_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_lib.c
@@ -159,12 +159,14 @@ static void ice_vsi_set_num_desc(struct ice_vsi *vsi)
static u16 ice_get_rxq_count(struct ice_pf *pf)
{
- return min(ice_get_avail_rxq_count(pf), num_online_cpus());
+ return min(ice_get_avail_rxq_count(pf),
+ netif_get_num_default_rss_queues());
}
static u16 ice_get_txq_count(struct ice_pf *pf)
{
- return min(ice_get_avail_txq_count(pf), num_online_cpus());
+ return min(ice_get_avail_txq_count(pf),
+ netif_get_num_default_rss_queues());
}
/**
@@ -907,13 +909,15 @@ static void ice_vsi_set_rss_params(struct ice_vsi *vsi)
if (vsi->type == ICE_VSI_CHNL)
vsi->rss_size = min_t(u16, vsi->num_rxq, max_rss_size);
else
- vsi->rss_size = min_t(u16, num_online_cpus(),
+ vsi->rss_size = min_t(u16,
+ netif_get_num_default_rss_queues(),
max_rss_size);
vsi->rss_lut_type = ICE_LUT_PF;
break;
case ICE_VSI_SF:
vsi->rss_table_size = ICE_LUT_VSI_SIZE;
- vsi->rss_size = min_t(u16, num_online_cpus(), max_rss_size);
+ vsi->rss_size = min_t(u16, netif_get_num_default_rss_queues(),
+ max_rss_size);
vsi->rss_lut_type = ICE_LUT_VSI;
break;
case ICE_VSI_VF:
--
2.49.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH iwl-next v3] ice: use netif_get_num_default_rss_queues()
2025-10-30 8:30 [PATCH iwl-next v3] ice: use netif_get_num_default_rss_queues() Michal Swiatkowski
@ 2025-10-30 9:10 ` Paul Menzel
2025-10-30 9:37 ` Michal Swiatkowski
0 siblings, 1 reply; 7+ messages in thread
From: Paul Menzel @ 2025-10-30 9:10 UTC (permalink / raw)
To: Michal Swiatkowski
Cc: intel-wired-lan, netdev, aleksander.lobakin, przemyslaw.kitszel,
jacob.e.keller, Aleksandr Loktionov
Dear Michal,
Thank you for your patch. For the summary, I’d add:
ice: Use netif_get_num_default_rss_queues() to decrease queue number
Am 30.10.25 um 09:30 schrieb Michal Swiatkowski:
> On some high-core systems (like AMD EPYC Bergamo, Intel Clearwater
> Forest) loading ice driver with default values can lead to queue/irq
> exhaustion. It will result in no additional resources for SR-IOV.
Could you please elaborate how to make the queue/irq exhaustion visible?
> In most cases there is no performance reason for more than half
> num_cpus(). Limit the default value to it using generic
> netif_get_num_default_rss_queues().
>
> Still, using ethtool the number of queues can be changed up to
> num_online_cpus(). It can be done by calling:
> $ethtool -L ethX combined $(nproc)
>
> This change affects only the default queue amount.
How would you judge the regression potential, that means for people
where the defaults work good enough, and the queue number is reduced now?
Kind regards,
Paul
> Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> ---
> v2 --> v3:
> * use $(nproc) in command example in commit message
>
> v1 --> v2:
> * Follow Olek's comment and switch from custom limiting to the generic
> netif_...() function.
> * Add more info in commit message (Paul)
> * Dropping RB tags, as it is different patch now
> ---
> drivers/net/ethernet/intel/ice/ice_irq.c | 5 +++--
> drivers/net/ethernet/intel/ice/ice_lib.c | 12 ++++++++----
> 2 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ice/ice_irq.c b/drivers/net/ethernet/intel/ice/ice_irq.c
> index 30801fd375f0..1d9b2d646474 100644
> --- a/drivers/net/ethernet/intel/ice/ice_irq.c
> +++ b/drivers/net/ethernet/intel/ice/ice_irq.c
> @@ -106,9 +106,10 @@ static struct ice_irq_entry *ice_get_irq_res(struct ice_pf *pf,
> #define ICE_RDMA_AEQ_MSIX 1
> static int ice_get_default_msix_amount(struct ice_pf *pf)
> {
> - return ICE_MIN_LAN_OICR_MSIX + num_online_cpus() +
> + return ICE_MIN_LAN_OICR_MSIX + netif_get_num_default_rss_queues() +
> (test_bit(ICE_FLAG_FD_ENA, pf->flags) ? ICE_FDIR_MSIX : 0) +
> - (ice_is_rdma_ena(pf) ? num_online_cpus() + ICE_RDMA_AEQ_MSIX : 0);
> + (ice_is_rdma_ena(pf) ? netif_get_num_default_rss_queues() +
> + ICE_RDMA_AEQ_MSIX : 0);
> }
>
> /**
> diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> index bac481e8140d..e366d089bef9 100644
> --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> @@ -159,12 +159,14 @@ static void ice_vsi_set_num_desc(struct ice_vsi *vsi)
>
> static u16 ice_get_rxq_count(struct ice_pf *pf)
> {
> - return min(ice_get_avail_rxq_count(pf), num_online_cpus());
> + return min(ice_get_avail_rxq_count(pf),
> + netif_get_num_default_rss_queues());
> }
>
> static u16 ice_get_txq_count(struct ice_pf *pf)
> {
> - return min(ice_get_avail_txq_count(pf), num_online_cpus());
> + return min(ice_get_avail_txq_count(pf),
> + netif_get_num_default_rss_queues());
> }
>
> /**
> @@ -907,13 +909,15 @@ static void ice_vsi_set_rss_params(struct ice_vsi *vsi)
> if (vsi->type == ICE_VSI_CHNL)
> vsi->rss_size = min_t(u16, vsi->num_rxq, max_rss_size);
> else
> - vsi->rss_size = min_t(u16, num_online_cpus(),
> + vsi->rss_size = min_t(u16,
> + netif_get_num_default_rss_queues(),
> max_rss_size);
> vsi->rss_lut_type = ICE_LUT_PF;
> break;
> case ICE_VSI_SF:
> vsi->rss_table_size = ICE_LUT_VSI_SIZE;
> - vsi->rss_size = min_t(u16, num_online_cpus(), max_rss_size);
> + vsi->rss_size = min_t(u16, netif_get_num_default_rss_queues(),
> + max_rss_size);
> vsi->rss_lut_type = ICE_LUT_VSI;
> break;
> case ICE_VSI_VF:
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iwl-next v3] ice: use netif_get_num_default_rss_queues()
2025-10-30 9:10 ` Paul Menzel
@ 2025-10-30 9:37 ` Michal Swiatkowski
2025-10-30 10:39 ` Przemek Kitszel
0 siblings, 1 reply; 7+ messages in thread
From: Michal Swiatkowski @ 2025-10-30 9:37 UTC (permalink / raw)
To: Paul Menzel
Cc: Michal Swiatkowski, intel-wired-lan, netdev, aleksander.lobakin,
przemyslaw.kitszel, jacob.e.keller, Aleksandr Loktionov
On Thu, Oct 30, 2025 at 10:10:32AM +0100, Paul Menzel wrote:
> Dear Michal,
>
>
> Thank you for your patch. For the summary, I’d add:
>
> ice: Use netif_get_num_default_rss_queues() to decrease queue number
>
> Am 30.10.25 um 09:30 schrieb Michal Swiatkowski:
> > On some high-core systems (like AMD EPYC Bergamo, Intel Clearwater
> > Forest) loading ice driver with default values can lead to queue/irq
> > exhaustion. It will result in no additional resources for SR-IOV.
>
> Could you please elaborate how to make the queue/irq exhaustion visible?
>
What do you mean? On high core system, lets say num_online_cpus()
returns 288, on 8 ports card we have online 256 irqs per eqch PF (2k in
total). Driver will load with the 256 queues (and irqs) on each PF.
Any VFs creation command will fail due to no free irqs available.
(echo X > /sys/class/net/ethX/device/sriov_numvfs)
> > In most cases there is no performance reason for more than half
> > num_cpus(). Limit the default value to it using generic
> > netif_get_num_default_rss_queues().
> >
> > Still, using ethtool the number of queues can be changed up to
> > num_online_cpus(). It can be done by calling:
> > $ethtool -L ethX combined $(nproc)
> >
> > This change affects only the default queue amount.
>
> How would you judge the regression potential, that means for people where
> the defaults work good enough, and the queue number is reduced now?
>
You can take a look into commit that introduce /2 change in
netif_get_num_default_rss_queues() [1]. There is a good justification
for such situation. In short, heaving physical core number is just a
wasting of CPU resources.
[1] https://lore.kernel.org/netdev/20220315091832.13873-1-ihuguet@redhat.com/
>
> Kind regards,
>
> Paul
>
>
> > Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > Signed-off-by: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> > ---
> > v2 --> v3:
> > * use $(nproc) in command example in commit message
> >
> > v1 --> v2:
> > * Follow Olek's comment and switch from custom limiting to the generic
> > netif_...() function.
> > * Add more info in commit message (Paul)
> > * Dropping RB tags, as it is different patch now
> > ---
> > drivers/net/ethernet/intel/ice/ice_irq.c | 5 +++--
> > drivers/net/ethernet/intel/ice/ice_lib.c | 12 ++++++++----
> > 2 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/ice/ice_irq.c b/drivers/net/ethernet/intel/ice/ice_irq.c
> > index 30801fd375f0..1d9b2d646474 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_irq.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_irq.c
> > @@ -106,9 +106,10 @@ static struct ice_irq_entry *ice_get_irq_res(struct ice_pf *pf,
> > #define ICE_RDMA_AEQ_MSIX 1
> > static int ice_get_default_msix_amount(struct ice_pf *pf)
> > {
> > - return ICE_MIN_LAN_OICR_MSIX + num_online_cpus() +
> > + return ICE_MIN_LAN_OICR_MSIX + netif_get_num_default_rss_queues() +
> > (test_bit(ICE_FLAG_FD_ENA, pf->flags) ? ICE_FDIR_MSIX : 0) +
> > - (ice_is_rdma_ena(pf) ? num_online_cpus() + ICE_RDMA_AEQ_MSIX : 0);
> > + (ice_is_rdma_ena(pf) ? netif_get_num_default_rss_queues() +
> > + ICE_RDMA_AEQ_MSIX : 0);
> > }
> > /**
> > diff --git a/drivers/net/ethernet/intel/ice/ice_lib.c b/drivers/net/ethernet/intel/ice/ice_lib.c
> > index bac481e8140d..e366d089bef9 100644
> > --- a/drivers/net/ethernet/intel/ice/ice_lib.c
> > +++ b/drivers/net/ethernet/intel/ice/ice_lib.c
> > @@ -159,12 +159,14 @@ static void ice_vsi_set_num_desc(struct ice_vsi *vsi)
> > static u16 ice_get_rxq_count(struct ice_pf *pf)
> > {
> > - return min(ice_get_avail_rxq_count(pf), num_online_cpus());
> > + return min(ice_get_avail_rxq_count(pf),
> > + netif_get_num_default_rss_queues());
> > }
> > static u16 ice_get_txq_count(struct ice_pf *pf)
> > {
> > - return min(ice_get_avail_txq_count(pf), num_online_cpus());
> > + return min(ice_get_avail_txq_count(pf),
> > + netif_get_num_default_rss_queues());
> > }
> > /**
> > @@ -907,13 +909,15 @@ static void ice_vsi_set_rss_params(struct ice_vsi *vsi)
> > if (vsi->type == ICE_VSI_CHNL)
> > vsi->rss_size = min_t(u16, vsi->num_rxq, max_rss_size);
> > else
> > - vsi->rss_size = min_t(u16, num_online_cpus(),
> > + vsi->rss_size = min_t(u16,
> > + netif_get_num_default_rss_queues(),
> > max_rss_size);
> > vsi->rss_lut_type = ICE_LUT_PF;
> > break;
> > case ICE_VSI_SF:
> > vsi->rss_table_size = ICE_LUT_VSI_SIZE;
> > - vsi->rss_size = min_t(u16, num_online_cpus(), max_rss_size);
> > + vsi->rss_size = min_t(u16, netif_get_num_default_rss_queues(),
> > + max_rss_size);
> > vsi->rss_lut_type = ICE_LUT_VSI;
> > break;
> > case ICE_VSI_VF:
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iwl-next v3] ice: use netif_get_num_default_rss_queues()
2025-10-30 9:37 ` Michal Swiatkowski
@ 2025-10-30 10:39 ` Przemek Kitszel
2025-10-31 13:17 ` Michal Swiatkowski
0 siblings, 1 reply; 7+ messages in thread
From: Przemek Kitszel @ 2025-10-30 10:39 UTC (permalink / raw)
To: Michal Swiatkowski, Paul Menzel
Cc: intel-wired-lan, netdev, aleksander.lobakin, jacob.e.keller,
Aleksandr Loktionov
On 10/30/25 10:37, Michal Swiatkowski wrote:
> On Thu, Oct 30, 2025 at 10:10:32AM +0100, Paul Menzel wrote:
>> Dear Michal,
>>
>>
>> Thank you for your patch. For the summary, I’d add:
>>
>> ice: Use netif_get_num_default_rss_queues() to decrease queue number
I would instead just say:
ice: cap the default number of queues to 64
as this is exactly what happens. Then next paragraph could be:
Use netif_get_num_default_rss_queues() as a better base (instead of
the number of CPU cores), but still cap it to 64 to avoid excess IRQs
assigned to PF (what would leave, in some cases, nothing for VFs).
sorry for such late nitpicks
and, see below too
>>
>> Am 30.10.25 um 09:30 schrieb Michal Swiatkowski:
>>> On some high-core systems (like AMD EPYC Bergamo, Intel Clearwater
>>> Forest) loading ice driver with default values can lead to queue/irq
>>> exhaustion. It will result in no additional resources for SR-IOV.
>>
>> Could you please elaborate how to make the queue/irq exhaustion visible?
>>
>
> What do you mean? On high core system, lets say num_online_cpus()
> returns 288, on 8 ports card we have online 256 irqs per eqch PF (2k in
> total). Driver will load with the 256 queues (and irqs) on each PF.
> Any VFs creation command will fail due to no free irqs available.
this clearly means this is a -net material,
even if this commit will be rather unpleasant for backports to stable
> (echo X > /sys/class/net/ethX/device/sriov_numvfs)
>
>>> In most cases there is no performance reason for more than half
>>> num_cpus(). Limit the default value to it using generic
>>> netif_get_num_default_rss_queues().
>>>
>>> Still, using ethtool the number of queues can be changed up to
>>> num_online_cpus(). It can be done by calling:
>>> $ethtool -L ethX combined $(nproc)
>>>
>>> This change affects only the default queue amount.
>>
>> How would you judge the regression potential, that means for people where
>> the defaults work good enough, and the queue number is reduced now?
>>
>
> You can take a look into commit that introduce /2 change in
> netif_get_num_default_rss_queues() [1]. There is a good justification
> for such situation. In short, heaving physical core number is just a
> wasting of CPU resources.
>
> [1] https://lore.kernel.org/netdev/20220315091832.13873-1-ihuguet@redhat.com/
>
[...]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iwl-next v3] ice: use netif_get_num_default_rss_queues()
2025-10-30 10:39 ` Przemek Kitszel
@ 2025-10-31 13:17 ` Michal Swiatkowski
2025-11-05 10:14 ` Przemek Kitszel
0 siblings, 1 reply; 7+ messages in thread
From: Michal Swiatkowski @ 2025-10-31 13:17 UTC (permalink / raw)
To: Przemek Kitszel
Cc: Michal Swiatkowski, Paul Menzel, intel-wired-lan, netdev,
aleksander.lobakin, jacob.e.keller, Aleksandr Loktionov
On Thu, Oct 30, 2025 at 11:39:30AM +0100, Przemek Kitszel wrote:
> On 10/30/25 10:37, Michal Swiatkowski wrote:
> > On Thu, Oct 30, 2025 at 10:10:32AM +0100, Paul Menzel wrote:
> > > Dear Michal,
> > >
> > >
> > > Thank you for your patch. For the summary, I’d add:
> > >
> > > ice: Use netif_get_num_default_rss_queues() to decrease queue number
>
> I would instead just say:
> ice: cap the default number of queues to 64
>
> as this is exactly what happens. Then next paragraph could be:
> Use netif_get_num_default_rss_queues() as a better base (instead of
> the number of CPU cores), but still cap it to 64 to avoid excess IRQs
> assigned to PF (what would leave, in some cases, nothing for VFs).
>
> sorry for such late nitpicks
> and, see below too
I moved away from capping to 64, now it is just call to
netif_get_num_default_rss_queues(). Following Olek's comment, dividing
by 2 is just fine now and looks like there is no good reasone to cap it
more in the driver, but let's discuss it here if you have different
opinion.
>
> > >
> > > Am 30.10.25 um 09:30 schrieb Michal Swiatkowski:
> > > > On some high-core systems (like AMD EPYC Bergamo, Intel Clearwater
> > > > Forest) loading ice driver with default values can lead to queue/irq
> > > > exhaustion. It will result in no additional resources for SR-IOV.
> > >
> > > Could you please elaborate how to make the queue/irq exhaustion visible?
> > >
> >
> > What do you mean? On high core system, lets say num_online_cpus()
> > returns 288, on 8 ports card we have online 256 irqs per eqch PF (2k in
> > total). Driver will load with the 256 queues (and irqs) on each PF.
> > Any VFs creation command will fail due to no free irqs available.
>
> this clearly means this is a -net material,
> even if this commit will be rather unpleasant for backports to stable
>
In my opinion it isn't. It is just about default values. Still in the
described case user can call ethtool -L and lower the queues to create
VFs without a problem.
> > (echo X > /sys/class/net/ethX/device/sriov_numvfs)
> >
> > > > In most cases there is no performance reason for more than half
> > > > num_cpus(). Limit the default value to it using generic
> > > > netif_get_num_default_rss_queues().
> > > >
> > > > Still, using ethtool the number of queues can be changed up to
> > > > num_online_cpus(). It can be done by calling:
> > > > $ethtool -L ethX combined $(nproc)
> > > >
> > > > This change affects only the default queue amount.
> > >
> > > How would you judge the regression potential, that means for people where
> > > the defaults work good enough, and the queue number is reduced now?
> > >
> >
> > You can take a look into commit that introduce /2 change in
> > netif_get_num_default_rss_queues() [1]. There is a good justification
> > for such situation. In short, heaving physical core number is just a
> > wasting of CPU resources.
> >
> > [1] https://lore.kernel.org/netdev/20220315091832.13873-1-ihuguet@redhat.com/
> >
> [...]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH iwl-next v3] ice: use netif_get_num_default_rss_queues()
2025-10-31 13:17 ` Michal Swiatkowski
@ 2025-11-05 10:14 ` Przemek Kitszel
2025-12-11 8:48 ` [Intel-wired-lan] " Romanowski, Rafal
0 siblings, 1 reply; 7+ messages in thread
From: Przemek Kitszel @ 2025-11-05 10:14 UTC (permalink / raw)
To: Michal Swiatkowski
Cc: Paul Menzel, intel-wired-lan, netdev, aleksander.lobakin,
jacob.e.keller, Aleksandr Loktionov
On 10/31/25 14:17, Michal Swiatkowski wrote:
> On Thu, Oct 30, 2025 at 11:39:30AM +0100, Przemek Kitszel wrote:
>> On 10/30/25 10:37, Michal Swiatkowski wrote:
>>> On Thu, Oct 30, 2025 at 10:10:32AM +0100, Paul Menzel wrote:
>>>> Dear Michal,
>>>>
>>>>
>>>> Thank you for your patch. For the summary, I’d add:
>>>>
>>>> ice: Use netif_get_num_default_rss_queues() to decrease queue number
>>
>> I would instead just say:
>> ice: cap the default number of queues to 64
>>
>> as this is exactly what happens. Then next paragraph could be:
>> Use netif_get_num_default_rss_queues() as a better base (instead of
>> the number of CPU cores), but still cap it to 64 to avoid excess IRQs
>> assigned to PF (what would leave, in some cases, nothing for VFs).
>>
>> sorry for such late nitpicks
>> and, see below too
>
> I moved away from capping to 64, now it is just call to
> netif_get_num_default_rss_queues(). Following Olek's comment, dividing
> by 2 is just fine now and looks like there is no good reasone to cap it
> more in the driver, but let's discuss it here if you have different
> opinion.
I see, sorry for the confusion
with that I'm fine with the change being -next material, and commit
message is good (not sure if perfect, but it does not need to be)
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>
>>
>>>>
>>>> Am 30.10.25 um 09:30 schrieb Michal Swiatkowski:
>>>>> On some high-core systems (like AMD EPYC Bergamo, Intel Clearwater
>>>>> Forest) loading ice driver with default values can lead to queue/irq
>>>>> exhaustion. It will result in no additional resources for SR-IOV.
>>>>
>>>> Could you please elaborate how to make the queue/irq exhaustion visible?
>>>>
>>>
>>> What do you mean? On high core system, lets say num_online_cpus()
>>> returns 288, on 8 ports card we have online 256 irqs per eqch PF (2k in
>>> total). Driver will load with the 256 queues (and irqs) on each PF.
>>> Any VFs creation command will fail due to no free irqs available.
>>
>> this clearly means this is a -net material,
>> even if this commit will be rather unpleasant for backports to stable
>>
>
> In my opinion it isn't. It is just about default values. Still in the
> described case user can call ethtool -L and lower the queues to create
> VFs without a problem.
>
>>> (echo X > /sys/class/net/ethX/device/sriov_numvfs)
>>>
>>>>> In most cases there is no performance reason for more than half
>>>>> num_cpus(). Limit the default value to it using generic
>>>>> netif_get_num_default_rss_queues().
>>>>>
>>>>> Still, using ethtool the number of queues can be changed up to
>>>>> num_online_cpus(). It can be done by calling:
>>>>> $ethtool -L ethX combined $(nproc)
>>>>>
>>>>> This change affects only the default queue amount.
>>>>
>>>> How would you judge the regression potential, that means for people where
>>>> the defaults work good enough, and the queue number is reduced now?
>>>>
>>>
>>> You can take a look into commit that introduce /2 change in
>>> netif_get_num_default_rss_queues() [1]. There is a good justification
>>> for such situation. In short, heaving physical core number is just a
>>> wasting of CPU resources.
>>>
>>> [1] https://lore.kernel.org/netdev/20220315091832.13873-1-ihuguet@redhat.com/
>>>
>> [...]
^ permalink raw reply [flat|nested] 7+ messages in thread
* RE: [Intel-wired-lan] [PATCH iwl-next v3] ice: use netif_get_num_default_rss_queues()
2025-11-05 10:14 ` Przemek Kitszel
@ 2025-12-11 8:48 ` Romanowski, Rafal
0 siblings, 0 replies; 7+ messages in thread
From: Romanowski, Rafal @ 2025-12-11 8:48 UTC (permalink / raw)
To: Kitszel, Przemyslaw, Michal Swiatkowski
Cc: Paul Menzel, intel-wired-lan@lists.osuosl.org,
netdev@vger.kernel.org, Lobakin, Aleksander, Keller, Jacob E,
Loktionov, Aleksandr
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of
> Przemek Kitszel
> Sent: Wednesday, November 5, 2025 11:14
> To: Michal Swiatkowski <michal.swiatkowski@linux.intel.com>
> Cc: Paul Menzel <pmenzel@molgen.mpg.de>; intel-wired-lan@lists.osuosl.org;
> netdev@vger.kernel.org; Lobakin, Aleksander <aleksander.lobakin@intel.com>;
> Keller, Jacob E <jacob.e.keller@intel.com>; Loktionov, Aleksandr
> <aleksandr.loktionov@intel.com>
> Subject: Re: [Intel-wired-lan] [PATCH iwl-next v3] ice: use
> netif_get_num_default_rss_queues()
>
> On 10/31/25 14:17, Michal Swiatkowski wrote:
> > On Thu, Oct 30, 2025 at 11:39:30AM +0100, Przemek Kitszel wrote:
> >> On 10/30/25 10:37, Michal Swiatkowski wrote:
> >>> On Thu, Oct 30, 2025 at 10:10:32AM +0100, Paul Menzel wrote:
> >>>> Dear Michal,
> >>>>
> >>>>
> >>>> Thank you for your patch. For the summary, I’d add:
> >>>>
> >>>> ice: Use netif_get_num_default_rss_queues() to decrease queue
> >>>> number
> >>
> >> I would instead just say:
> >> ice: cap the default number of queues to 64
> >>
> >> as this is exactly what happens. Then next paragraph could be:
> >> Use netif_get_num_default_rss_queues() as a better base (instead of
> >> the number of CPU cores), but still cap it to 64 to avoid excess IRQs
> >> assigned to PF (what would leave, in some cases, nothing for VFs).
> >>
> >> sorry for such late nitpicks
> >> and, see below too
> >
> > I moved away from capping to 64, now it is just call to
> > netif_get_num_default_rss_queues(). Following Olek's comment, dividing
> > by 2 is just fine now and looks like there is no good reasone to cap
> > it more in the driver, but let's discuss it here if you have different
> > opinion.
>
> I see, sorry for the confusion
> with that I'm fine with the change being -next material, and commit message is
> good (not sure if perfect, but it does not need to be)
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>
> >
> >>
> >>>>
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2025-12-11 8:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-30 8:30 [PATCH iwl-next v3] ice: use netif_get_num_default_rss_queues() Michal Swiatkowski
2025-10-30 9:10 ` Paul Menzel
2025-10-30 9:37 ` Michal Swiatkowski
2025-10-30 10:39 ` Przemek Kitszel
2025-10-31 13:17 ` Michal Swiatkowski
2025-11-05 10:14 ` Przemek Kitszel
2025-12-11 8:48 ` [Intel-wired-lan] " Romanowski, Rafal
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).