* [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
@ 2020-12-21 22:25 Nick Lowe
2021-01-31 23:17 ` Matt Corallo
0 siblings, 1 reply; 15+ messages in thread
From: Nick Lowe @ 2020-12-21 22:25 UTC (permalink / raw)
To: netdev
Cc: anthony.l.nguyen, kuba, jesse.brandeburg, intel-wired-lan, davem,
Nick Lowe
The Intel I211 Ethernet Controller supports 2 Receive Side Scaling (RSS) queues.
It should not be excluded from having this feature enabled.
Via commit c883de9fd787b6f49bf825f3de3601aeb78a7114
E1000_MRQC_ENABLE_RSS_4Q was renamed to E1000_MRQC_ENABLE_RSS_MQ to
indicate that this is a generic bit flag to enable queues and not
a flag that is specific to devices that support 4 queues
The bit flag enables 2, 4 or 8 queues appropriately depending on the part.
Tested with a multicore CPU and frames were then distributed as expected.
This issue appears to have been introduced because of confusion caused
by the prior name.
Signed-off-by: Nick Lowe <nick.lowe@gmail.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 03f78fdb0dcd..87ac1d3e25cb 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -4482,8 +4482,7 @@ static void igb_setup_mrqc(struct igb_adapter *adapter)
else
mrqc |= E1000_MRQC_ENABLE_VMDQ;
} else {
- if (hw->mac.type != e1000_i211)
- mrqc |= E1000_MRQC_ENABLE_RSS_MQ;
+ mrqc |= E1000_MRQC_ENABLE_RSS_MQ;
}
igb_vmm_control(adapter);
--
2.29.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
2020-12-21 22:25 [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller Nick Lowe
@ 2021-01-31 23:17 ` Matt Corallo
2021-02-01 16:47 ` Jakub Kicinski
0 siblings, 1 reply; 15+ messages in thread
From: Matt Corallo @ 2021-01-31 23:17 UTC (permalink / raw)
To: Nick Lowe, netdev
Cc: anthony.l.nguyen, kuba, jesse.brandeburg, intel-wired-lan, davem
Given this fixes a major (albeit ancient) performance regression, is it not a candidate for backport? It landed on
Tony's dev-queue branch with a Fixes tag but no stable CC.
Thanks,
Matt
On 12/21/20 5:25 PM, Nick Lowe wrote:
> The Intel I211 Ethernet Controller supports 2 Receive Side Scaling (RSS) queues.
> It should not be excluded from having this feature enabled.
>
> Via commit c883de9fd787b6f49bf825f3de3601aeb78a7114
> E1000_MRQC_ENABLE_RSS_4Q was renamed to E1000_MRQC_ENABLE_RSS_MQ to
> indicate that this is a generic bit flag to enable queues and not
> a flag that is specific to devices that support 4 queues
>
> The bit flag enables 2, 4 or 8 queues appropriately depending on the part.
>
> Tested with a multicore CPU and frames were then distributed as expected.
>
> This issue appears to have been introduced because of confusion caused
> by the prior name.
>
> Signed-off-by: Nick Lowe <nick.lowe@gmail.com>
> ---
> drivers/net/ethernet/intel/igb/igb_main.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index 03f78fdb0dcd..87ac1d3e25cb 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -4482,8 +4482,7 @@ static void igb_setup_mrqc(struct igb_adapter *adapter)
> else
> mrqc |= E1000_MRQC_ENABLE_VMDQ;
> } else {
> - if (hw->mac.type != e1000_i211)
> - mrqc |= E1000_MRQC_ENABLE_RSS_MQ;
> + mrqc |= E1000_MRQC_ENABLE_RSS_MQ;
> }
> igb_vmm_control(adapter);
>
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
2021-01-31 23:17 ` Matt Corallo
@ 2021-02-01 16:47 ` Jakub Kicinski
2021-02-01 18:32 ` Nguyen, Anthony L
0 siblings, 1 reply; 15+ messages in thread
From: Jakub Kicinski @ 2021-02-01 16:47 UTC (permalink / raw)
To: Matt Corallo
Cc: Nick Lowe, netdev, anthony.l.nguyen, jesse.brandeburg,
intel-wired-lan, davem
On Sun, 31 Jan 2021 18:17:11 -0500 Matt Corallo wrote:
> Given this fixes a major (albeit ancient) performance regression, is
> it not a candidate for backport? It landed on Tony's dev-queue branch
> with a Fixes tag but no stable CC.
Tony's tree needs to get fed into net, then net into Linus's tree and
then we can do the backport :(
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
2021-02-01 16:47 ` Jakub Kicinski
@ 2021-02-01 18:32 ` Nguyen, Anthony L
2021-02-01 19:45 ` Jakub Kicinski
0 siblings, 1 reply; 15+ messages in thread
From: Nguyen, Anthony L @ 2021-02-01 18:32 UTC (permalink / raw)
To: kuba, linux-wired-list
Cc: netdev, davem, Brandeburg, Jesse, intel-wired-lan, nick.lowe
On Mon, 2021-02-01 at 08:47 -0800, Jakub Kicinski wrote:
> On Sun, 31 Jan 2021 18:17:11 -0500 Matt Corallo wrote:
> > Given this fixes a major (albeit ancient) performance regression,
> > is
> > it not a candidate for backport? It landed on Tony's dev-queue
> > branch
> > with a Fixes tag but no stable CC.
>
> Tony's tree needs to get fed into net, then net into Linus's tree and
> then we can do the backport :(
The behavior looks to have always been since support was introduced
with f96a8a0b7854 ("igb: Add Support for new i210/i211 devices."). I
was unable to determine the original reason for excluding it so I was
planning on sending through net-next as added functionality, however, I
will go ahead and send this through net and add it to the current
series that I need to make changes to.
Thanks,
Tony
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
2021-02-01 18:32 ` Nguyen, Anthony L
@ 2021-02-01 19:45 ` Jakub Kicinski
2021-02-01 20:06 ` Nick Lowe
2021-02-01 20:21 ` Matt Corallo
0 siblings, 2 replies; 15+ messages in thread
From: Jakub Kicinski @ 2021-02-01 19:45 UTC (permalink / raw)
To: Nguyen, Anthony L
Cc: linux-wired-list, netdev, davem, Brandeburg, Jesse,
intel-wired-lan, nick.lowe
On Mon, 1 Feb 2021 18:32:50 +0000 Nguyen, Anthony L wrote:
> On Mon, 2021-02-01 at 08:47 -0800, Jakub Kicinski wrote:
> > On Sun, 31 Jan 2021 18:17:11 -0500 Matt Corallo wrote:
> > > Given this fixes a major (albeit ancient) performance regression,
> > > is
> > > it not a candidate for backport? It landed on Tony's dev-queue
> > > branch
> > > with a Fixes tag but no stable CC.
> >
> > Tony's tree needs to get fed into net, then net into Linus's tree and
> > then we can do the backport :(
>
> The behavior looks to have always been since support was introduced
> with f96a8a0b7854 ("igb: Add Support for new i210/i211 devices."). I
> was unable to determine the original reason for excluding it so I was
> planning on sending through net-next as added functionality, however, I
> will go ahead and send this through net and add it to the current
> series that I need to make changes to.
Hm, no need for net if it's not really a regression IMO. Not after -rc6.
Matt, would you mind clarifying if this is indeed a regression for i211?
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
2021-02-01 19:45 ` Jakub Kicinski
@ 2021-02-01 20:06 ` Nick Lowe
2021-02-01 20:21 ` Matt Corallo
1 sibling, 0 replies; 15+ messages in thread
From: Nick Lowe @ 2021-02-01 20:06 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Nguyen, Anthony L, linux-wired-list, netdev, davem, Brandeburg,
Jesse, intel-wired-lan
Hi all,
It is correct that this is not a regression, it instead has never worked.
My suggestion would be to merge it during the 5.11 cycle rather than
5.12 where possible because the patch is non-invasive and should be
lower risk.
There are significant and tangible real-world benefits to RSS
functioning with the i211 so I do not personally think, on balance,
that it is proportionate and worth waiting for 5.12 to conclude here.
Any thoughts?
More importantly though, after this does release in a stable kernel
(regardless of whether that is 5.11 or 5.12) and this change has had
some time to soak in the field, I do suggest consideration to then
backport this to the 5.4 and 5.10 LTS kernels so that RSS starts to
function there. 5.4 and 5.10 are the release branches that will, of
course, get far more use for the next couple of years.
Best,
Nick
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
2021-02-01 19:45 ` Jakub Kicinski
2021-02-01 20:06 ` Nick Lowe
@ 2021-02-01 20:21 ` Matt Corallo
2021-02-01 20:23 ` Nick Lowe
1 sibling, 1 reply; 15+ messages in thread
From: Matt Corallo @ 2021-02-01 20:21 UTC (permalink / raw)
To: Jakub Kicinski, Nguyen, Anthony L
Cc: linux-wired-list, netdev, davem, Brandeburg, Jesse,
intel-wired-lan, nick.lowe
On 2/1/21 2:45 PM, Jakub Kicinski wrote:
> Matt, would you mind clarifying if this is indeed a regression for i211?
>
Admittedly, I didn't go all the way back to test that it is, indeed, a regression. The Fixes commit that it was tagged
with on Tony's tree was something more recent than initial igb landing (its a refactor of RSS) and there are numerous
posts online indicating common I211 hardware (eg PCEngines APU2) support RSS and properly load multiple cores, so I
naively assumed that it is a regression of some form.
Did you test kernels odler than c883de9fd787, Nick?
Given that, and the non-invasive nature of the patch, I figured it was worth trying to land on LTS trees and going ahead
with it for 5.11, but I don't feel strongly on how it ends up on LTS, it just seems a waste to not land it there.
Matt
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
2021-02-01 20:21 ` Matt Corallo
@ 2021-02-01 20:23 ` Nick Lowe
2021-02-01 20:33 ` Jakub Kicinski
0 siblings, 1 reply; 15+ messages in thread
From: Nick Lowe @ 2021-02-01 20:23 UTC (permalink / raw)
To: Matt Corallo
Cc: Jakub Kicinski, Nguyen, Anthony L, netdev, davem, Brandeburg,
Jesse, intel-wired-lan
I personally tested with mainline and 5.10, but not 5.4
Best,
Nick
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
2021-02-01 20:23 ` Nick Lowe
@ 2021-02-01 20:33 ` Jakub Kicinski
2021-05-03 12:32 ` Nick Lowe
0 siblings, 1 reply; 15+ messages in thread
From: Jakub Kicinski @ 2021-02-01 20:33 UTC (permalink / raw)
To: Nick Lowe
Cc: Matt Corallo, Nguyen, Anthony L, netdev, davem, Brandeburg,
Jesse, intel-wired-lan
On Mon, 1 Feb 2021 20:23:51 +0000 Nick Lowe wrote:
> I personally tested with mainline and 5.10, but not 5.4
My preference would be to merge into net-next, and then do the
backport after 5.11 is released.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
2021-02-01 20:33 ` Jakub Kicinski
@ 2021-05-03 12:32 ` Nick Lowe
2021-05-03 18:30 ` Jakub Kicinski
0 siblings, 1 reply; 15+ messages in thread
From: Nick Lowe @ 2021-05-03 12:32 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Matt Corallo, Nguyen, Anthony L, netdev, davem, Brandeburg,
Jesse, intel-wired-lan
Hi all,
Now that the 5.12 kernel has released, please may we consider
backporting commit 6e6026f2dd2005844fb35c3911e8083c09952c6c to both
the 5.4 and 5.10 LTS kernels so that RSS starts to function with the
i211?
Best,
Nick
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
2021-05-03 12:32 ` Nick Lowe
@ 2021-05-03 18:30 ` Jakub Kicinski
2021-05-03 19:53 ` Stable inclusion request - " Nick Lowe
0 siblings, 1 reply; 15+ messages in thread
From: Jakub Kicinski @ 2021-05-03 18:30 UTC (permalink / raw)
To: Nick Lowe
Cc: Matt Corallo, Nguyen, Anthony L, netdev, davem, Brandeburg,
Jesse, intel-wired-lan
On Mon, 3 May 2021 13:32:24 +0100 Nick Lowe wrote:
> Hi all,
>
> Now that the 5.12 kernel has released, please may we consider
> backporting commit 6e6026f2dd2005844fb35c3911e8083c09952c6c to both
> the 5.4 and 5.10 LTS kernels so that RSS starts to function with the
> i211?
No objections here. Please submit the backport request to stable@.
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-2
^ permalink raw reply [flat|nested] 15+ messages in thread
* Stable inclusion request - igb: Enable RSS for Intel I211 Ethernet Controller
2021-05-03 18:30 ` Jakub Kicinski
@ 2021-05-03 19:53 ` Nick Lowe
2021-05-04 4:43 ` Greg KH
0 siblings, 1 reply; 15+ messages in thread
From: Nick Lowe @ 2021-05-03 19:53 UTC (permalink / raw)
To: stable, Matt Corallo, netdev, David S. Miller, intel-wired-lan,
Nguyen, Anthony L, Brandeburg, Jesse
Hi,
Please may we request that commit 6e6026f2dd20 ("igb: Enable RSS for
Intel I211 Ethernet Controller") be backported to the 5.4 and 5.10 LTS
kernels?
The Intel i211 Ethernet Controller supports 2 Receive Side Scaling
(RSS) queues, the patch corrects the issue that the i211 should not be
excluded from having this feature enabled.
Best regards,
Nick
---------- Forwarded message ---------
From: Jakub Kicinski <kuba@kernel.org>
Date: Mon, 3 May 2021 at 19:30
Subject: Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
To: Nick Lowe <nick.lowe@gmail.com>
Cc: Matt Corallo <linux-wired-list@bluematt.me>, Nguyen, Anthony L
<anthony.l.nguyen@intel.com>, netdev@vger.kernel.org
<netdev@vger.kernel.org>, davem@davemloft.net <davem@davemloft.net>,
Brandeburg, Jesse <jesse.brandeburg@intel.com>,
intel-wired-lan@lists.osuosl.org <intel-wired-lan@lists.osuosl.org>
On Mon, 3 May 2021 13:32:24 +0100 Nick Lowe wrote:
> Hi all,
>
> Now that the 5.12 kernel has released, please may we consider
> backporting commit 6e6026f2dd2005844fb35c3911e8083c09952c6c to both
> the 5.4 and 5.10 LTS kernels so that RSS starts to function with the
> i211?
No objections here. Please submit the backport request to stable@.
https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#option-2
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: Stable inclusion request - igb: Enable RSS for Intel I211 Ethernet Controller
2021-05-03 19:53 ` Stable inclusion request - " Nick Lowe
@ 2021-05-04 4:43 ` Greg KH
0 siblings, 0 replies; 15+ messages in thread
From: Greg KH @ 2021-05-04 4:43 UTC (permalink / raw)
To: Nick Lowe
Cc: stable, Matt Corallo, netdev, David S. Miller, intel-wired-lan,
Nguyen, Anthony L, Brandeburg, Jesse
On Mon, May 03, 2021 at 08:53:48PM +0100, Nick Lowe wrote:
> Hi,
>
> Please may we request that commit 6e6026f2dd20 ("igb: Enable RSS for
> Intel I211 Ethernet Controller") be backported to the 5.4 and 5.10 LTS
> kernels?
>
Also added to 5.11 as it's still alive for another week or so :)
thanks,
greg k-h
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
@ 2020-12-21 19:15 Nick Lowe
2020-12-21 22:11 ` Jakub Kicinski
0 siblings, 1 reply; 15+ messages in thread
From: Nick Lowe @ 2020-12-21 19:15 UTC (permalink / raw)
To: netdev; +Cc: Nick Lowe
The Intel I211 Ethernet Controller supports 2 Receive Side Scaling (RSS) queues.
It should not be excluded from having this feature enabled.
Via commit c883de9fd787b6f49bf825f3de3601aeb78a7114
E1000_MRQC_ENABLE_RSS_4Q was renamed to E1000_MRQC_ENABLE_RSS_MQ to
indicate that this is a generic bit flag to enable queues and not
a flag that is specific to devices that support 4 queues
The bit flag enables 2, 4 or 8 queues appropriately depending on the part.
Tested with a multicore CPU and frames were then distributed as expected.
This issue appears to have been introduced because of confusion caused
by the prior name.
Signed-off-by: Nick Lowe <nick.lowe@gmail.com>
---
drivers/net/ethernet/intel/igb/igb_main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 03f78fdb0dcd..87ac1d3e25cb 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -4482,8 +4482,7 @@ static void igb_setup_mrqc(struct igb_adapter *adapter)
else
mrqc |= E1000_MRQC_ENABLE_VMDQ;
} else {
- if (hw->mac.type != e1000_i211)
- mrqc |= E1000_MRQC_ENABLE_RSS_MQ;
+ mrqc |= E1000_MRQC_ENABLE_RSS_MQ;
}
igb_vmm_control(adapter);
--
2.29.2
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller
2020-12-21 19:15 [PATCH net] " Nick Lowe
@ 2020-12-21 22:11 ` Jakub Kicinski
0 siblings, 0 replies; 15+ messages in thread
From: Jakub Kicinski @ 2020-12-21 22:11 UTC (permalink / raw)
To: Nick Lowe; +Cc: netdev
On Mon, 21 Dec 2020 19:15:49 +0000 Nick Lowe wrote:
> The Intel I211 Ethernet Controller supports 2 Receive Side Scaling (RSS) queues.
> It should not be excluded from having this feature enabled.
>
> Via commit c883de9fd787b6f49bf825f3de3601aeb78a7114
> E1000_MRQC_ENABLE_RSS_4Q was renamed to E1000_MRQC_ENABLE_RSS_MQ to
> indicate that this is a generic bit flag to enable queues and not
> a flag that is specific to devices that support 4 queues
>
> The bit flag enables 2, 4 or 8 queues appropriately depending on the part.
>
> Tested with a multicore CPU and frames were then distributed as expected.
>
> This issue appears to have been introduced because of confusion caused
> by the prior name.
>
> Signed-off-by: Nick Lowe <nick.lowe@gmail.com>
Please repost CCing the maintainers.
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2021-05-04 4:44 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-21 22:25 [PATCH net] igb: Enable RSS for Intel I211 Ethernet Controller Nick Lowe
2021-01-31 23:17 ` Matt Corallo
2021-02-01 16:47 ` Jakub Kicinski
2021-02-01 18:32 ` Nguyen, Anthony L
2021-02-01 19:45 ` Jakub Kicinski
2021-02-01 20:06 ` Nick Lowe
2021-02-01 20:21 ` Matt Corallo
2021-02-01 20:23 ` Nick Lowe
2021-02-01 20:33 ` Jakub Kicinski
2021-05-03 12:32 ` Nick Lowe
2021-05-03 18:30 ` Jakub Kicinski
2021-05-03 19:53 ` Stable inclusion request - " Nick Lowe
2021-05-04 4:43 ` Greg KH
-- strict thread matches above, loose matches on Subject: below --
2020-12-21 19:15 [PATCH net] " Nick Lowe
2020-12-21 22:11 ` Jakub Kicinski
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).