* [Intel-wired-lan] [PATCH net v3] e1000e: Disable TSO on i219-LM card to increase speed
@ 2023-02-22 14:07 Mateusz Palczewski
2023-02-22 14:22 ` Paul Menzel
0 siblings, 1 reply; 6+ messages in thread
From: Mateusz Palczewski @ 2023-02-22 14:07 UTC (permalink / raw)
To: intel-wired-lan; +Cc: pmenzel, Sebastian Basierski
From: Sebastian Basierski <sebastianx.basierski@intel.com>
While using i219-LM card currently it was only possible to achieve
about 60% of maximum speed due to regression introduced in Linux 5.8-rc1.
This was caused by TSO not being disabled by default despite commit
f29801030ac6 ("e1000e: Disable TSO for buffer overrun workaround")
implementation. Fix that by disabling TSO during driver probe.
Signed-off-by: Sebastian Basierski <sebastianx.basierski@intel.com>
Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
---
v3: Changed the patch to disable TSO during the probe
v2: Fixed commit message and comment inside the code
---
drivers/net/ethernet/intel/e1000e/netdev.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
index 04acd1a992fa..863796acf8d7 100644
--- a/drivers/net/ethernet/intel/e1000e/netdev.c
+++ b/drivers/net/ethernet/intel/e1000e/netdev.c
@@ -7529,6 +7529,14 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
NETIF_F_RXCSUM |
NETIF_F_HW_CSUM);
+ /* Disable TSO for i219 to avoid transfer speed
+ * being capped at 60%.
+ */
+ if (hw->mac.type == e1000_pch_spt) {
+ netdev->features &= ~NETIF_F_TSO;
+ netdev->features &= ~NETIF_F_TSO6;
+ }
+
/* Set user-changeable features (subset of all device features) */
netdev->hw_features = netdev->features;
netdev->hw_features |= NETIF_F_RXFCS;
--
2.31.1
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v3] e1000e: Disable TSO on i219-LM card to increase speed
2023-02-22 14:07 [Intel-wired-lan] [PATCH net v3] e1000e: Disable TSO on i219-LM card to increase speed Mateusz Palczewski
@ 2023-02-22 14:22 ` Paul Menzel
2023-02-22 14:54 ` Kai-Heng Feng
0 siblings, 1 reply; 6+ messages in thread
From: Paul Menzel @ 2023-02-22 14:22 UTC (permalink / raw)
To: Mateusz Palczewski; +Cc: Kai-Heng Feng, intel-wired-lan, Sebastian Basierski
[Cc: +Kai-Heng]
Dear Mateusz, dear Sebastian,
Thank you for the patch.
Am 22.02.23 um 15:07 schrieb Mateusz Palczewski:
> From: Sebastian Basierski <sebastianx.basierski@intel.com>
>
> While using i219-LM card currently it was only possible to achieve
> about 60% of maximum speed due to regression introduced in Linux 5.8-rc1.
> This was caused by TSO not being disabled by default despite commit
> f29801030ac6 ("e1000e: Disable TSO for buffer overrun workaround")
> implementation. Fix that by disabling TSO during driver probe.
Can the code added by the referenced commit then be removed?
Also the questions from the discussion of v2(?) was not answered, why
the conditions in the if statement of the code added by commit
f29801030ac6 where not true.
/* disable TSO for pcie and 10/100 speeds, to avoid
* some hardware issues
*/
if (!(adapter->flags & FLAG_TSO_FORCE)) {
Missing Fixes: tag below.
> Signed-off-by: Sebastian Basierski <sebastianx.basierski@intel.com>
> Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
> ---
> v3: Changed the patch to disable TSO during the probe
> v2: Fixed commit message and comment inside the code
> ---
> drivers/net/ethernet/intel/e1000e/netdev.c | 8 ++++++++
> 1 file changed, 8 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 04acd1a992fa..863796acf8d7 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -7529,6 +7529,14 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> NETIF_F_RXCSUM |
> NETIF_F_HW_CSUM);
>
> + /* Disable TSO for i219 to avoid transfer speed
> + * being capped at 60%.
> + */
> + if (hw->mac.type == e1000_pch_spt) {
> + netdev->features &= ~NETIF_F_TSO;
> + netdev->features &= ~NETIF_F_TSO6;
> + }
> +
> /* Set user-changeable features (subset of all device features) */
> netdev->hw_features = netdev->features;
> netdev->hw_features |= NETIF_F_RXFCS;
Kind regards,
Paul
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v3] e1000e: Disable TSO on i219-LM card to increase speed
2023-02-22 14:22 ` Paul Menzel
@ 2023-02-22 14:54 ` Kai-Heng Feng
2023-03-15 12:44 ` Palczewski, Mateusz
0 siblings, 1 reply; 6+ messages in thread
From: Kai-Heng Feng @ 2023-02-22 14:54 UTC (permalink / raw)
To: Paul Menzel; +Cc: intel-wired-lan, Sebastian Basierski
Hi,
On Wed, Feb 22, 2023 at 10:22 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> [Cc: +Kai-Heng]
Thanks for adding me to Cc list.
Please add me to the Cc list if there's next revision.
>
>
> Dear Mateusz, dear Sebastian,
>
>
> Thank you for the patch.
>
> Am 22.02.23 um 15:07 schrieb Mateusz Palczewski:
> > From: Sebastian Basierski <sebastianx.basierski@intel.com>
> >
> > While using i219-LM card currently it was only possible to achieve
> > about 60% of maximum speed due to regression introduced in Linux 5.8-rc1.
> > This was caused by TSO not being disabled by default despite commit
> > f29801030ac6 ("e1000e: Disable TSO for buffer overrun workaround")
> > implementation. Fix that by disabling TSO during driver probe.
Does that mean "watchdog_timer" isn't scheduled?
>
> Can the code added by the referenced commit then be removed?
>
> Also the questions from the discussion of v2(?) was not answered, why
> the conditions in the if statement of the code added by commit
> f29801030ac6 where not true.
>
> /* disable TSO for pcie and 10/100 speeds, to avoid
> * some hardware issues
> */
> if (!(adapter->flags & FLAG_TSO_FORCE)) {
Yea, my idea was to take FLAG_TSO_FORCE into consideration hence the
adding the change to this if block.
Maybe someone still wants to enable TSO despite of the downside?
Kai-Heng
>
> Missing Fixes: tag below.
>
> > Signed-off-by: Sebastian Basierski <sebastianx.basierski@intel.com>
> > Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
> > ---
> > v3: Changed the patch to disable TSO during the probe
> > v2: Fixed commit message and comment inside the code
> > ---
> > drivers/net/ethernet/intel/e1000e/netdev.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> > index 04acd1a992fa..863796acf8d7 100644
> > --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> > @@ -7529,6 +7529,14 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > NETIF_F_RXCSUM |
> > NETIF_F_HW_CSUM);
> >
> > + /* Disable TSO for i219 to avoid transfer speed
> > + * being capped at 60%.
> > + */
> > + if (hw->mac.type == e1000_pch_spt) {
> > + netdev->features &= ~NETIF_F_TSO;
> > + netdev->features &= ~NETIF_F_TSO6;
> > + }
> > +
> > /* Set user-changeable features (subset of all device features) */
> > netdev->hw_features = netdev->features;
> > netdev->hw_features |= NETIF_F_RXFCS;
>
>
> Kind regards,
>
> Paul
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v3] e1000e: Disable TSO on i219-LM card to increase speed
2023-02-22 14:54 ` Kai-Heng Feng
@ 2023-03-15 12:44 ` Palczewski, Mateusz
2023-03-17 7:34 ` Kai-Heng Feng
0 siblings, 1 reply; 6+ messages in thread
From: Palczewski, Mateusz @ 2023-03-15 12:44 UTC (permalink / raw)
To: Kai-Heng Feng, Paul Menzel; +Cc: intel-wired-lan, Basierski, SebastianX
Hi,
Sorry for late response
> Hi,
>
> On Wed, Feb 22, 2023 at 10:22 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
> >
> > [Cc: +Kai-Heng]
>
> Thanks for adding me to Cc list.
>
> Please add me to the Cc list if there's next revision.
Sure, will do.
>
> >
> >
> > Dear Mateusz, dear Sebastian,
> >
> >
> > Thank you for the patch.
> >
> > Am 22.02.23 um 15:07 schrieb Mateusz Palczewski:
> > > From: Sebastian Basierski <sebastianx.basierski@intel.com>
> > >
> > > While using i219-LM card currently it was only possible to achieve
> > > about 60% of maximum speed due to regression introduced in Linux 5.8-rc1.
> > > This was caused by TSO not being disabled by default despite commit
> > > f29801030ac6 ("e1000e: Disable TSO for buffer overrun workaround")
> > > implementation. Fix that by disabling TSO during driver probe.
>
> Does that mean "watchdog_timer" isn't scheduled?
I think it is, but for some reason forcing TSO to be disabled in e1000_watchdog_task() does work only after reloading the driver on server and not right away after it was booted.
I believe with the solution posted in the new patch it should work right away.
>
> >
> > Can the code added by the referenced commit then be removed?
You are right, I will prepare new patch version with removal of the code that disables TSO in watchdog task and instead does this during probe.
> >
> > Also the questions from the discussion of v2(?) was not answered, why
> > the conditions in the if statement of the code added by commit
> > f29801030ac6 where not true.
> >
> > /* disable TSO for pcie and 10/100 speeds, to avoid
> > * some hardware issues
> > */
> > if (!(adapter->flags & FLAG_TSO_FORCE)) {
>
> Yea, my idea was to take FLAG_TSO_FORCE into consideration hence the adding the change to this if block.
>
> Maybe someone still wants to enable TSO despite of the downside?
By disabling TSO during probe we are not shutting it down completly, if a user wants to use it anyway despite speed decrease it can be done manually with ethtool.
>
> Kai-Heng
>
> >
> > Missing Fixes: tag below.
Thanks, will add that in v4 version.
> >
> > > Signed-off-by: Sebastian Basierski <sebastianx.basierski@intel.com>
> > > Signed-off-by: Mateusz Palczewski <mateusz.palczewski@intel.com>
> > > ---
> > > v3: Changed the patch to disable TSO during the probe
> > > v2: Fixed commit message and comment inside the code
> > > ---
> > > drivers/net/ethernet/intel/e1000e/netdev.c | 8 ++++++++
> > > 1 file changed, 8 insertions(+)
> > >
> > > diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> > > b/drivers/net/ethernet/intel/e1000e/netdev.c
> > > index 04acd1a992fa..863796acf8d7 100644
> > > --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> > > +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> > > @@ -7529,6 +7529,14 @@ static int e1000_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > NETIF_F_RXCSUM |
> > > NETIF_F_HW_CSUM);
> > >
> > > + /* Disable TSO for i219 to avoid transfer speed
> > > + * being capped at 60%.
> > > + */
> > > + if (hw->mac.type == e1000_pch_spt) {
> > > + netdev->features &= ~NETIF_F_TSO;
> > > + netdev->features &= ~NETIF_F_TSO6;
> > > + }
> > > +
> > > /* Set user-changeable features (subset of all device features) */
> > > netdev->hw_features = netdev->features;
> > > netdev->hw_features |= NETIF_F_RXFCS;
> >
> >
> > Kind regards,
> >
> > Paul
>
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v3] e1000e: Disable TSO on i219-LM card to increase speed
2023-03-15 12:44 ` Palczewski, Mateusz
@ 2023-03-17 7:34 ` Kai-Heng Feng
2023-03-22 12:04 ` Palczewski, Mateusz
0 siblings, 1 reply; 6+ messages in thread
From: Kai-Heng Feng @ 2023-03-17 7:34 UTC (permalink / raw)
To: Palczewski, Mateusz; +Cc: Paul Menzel, intel-wired-lan, Basierski, SebastianX
Hi Mateusz,
On Wed, Mar 15, 2023 at 8:44 PM Palczewski, Mateusz
<mateusz.palczewski@intel.com> wrote:
[snipped]
> > > Also the questions from the discussion of v2(?) was not answered, why
> > > the conditions in the if statement of the code added by commit
> > > f29801030ac6 where not true.
> > >
> > > /* disable TSO for pcie and 10/100 speeds, to avoid
> > > * some hardware issues
> > > */
> > > if (!(adapter->flags & FLAG_TSO_FORCE)) {
> >
> > Yea, my idea was to take FLAG_TSO_FORCE into consideration hence the adding the change to this if block.
> >
> > Maybe someone still wants to enable TSO despite of the downside?
>
> By disabling TSO during probe we are not shutting it down completly, if a user wants to use it anyway despite speed decrease it can be done manually with ethtool.
Maybe move the whole block of "FLAG_TSO_FORCE" to probe routine? So
the code is logically grouped.
Kai-Heng
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v3] e1000e: Disable TSO on i219-LM card to increase speed
2023-03-17 7:34 ` Kai-Heng Feng
@ 2023-03-22 12:04 ` Palczewski, Mateusz
0 siblings, 0 replies; 6+ messages in thread
From: Palczewski, Mateusz @ 2023-03-22 12:04 UTC (permalink / raw)
To: Kai-Heng Feng; +Cc: Paul Menzel, intel-wired-lan, Basierski, SebastianX
Hi,
>Hi Mateusz,
>
>On Wed, Mar 15, 2023 at 8:44 PM Palczewski, Mateusz <mateusz.palczewski@intel.com> wrote:
>[snipped]
>> > > Also the questions from the discussion of v2(?) was not answered,
>> > > why the conditions in the if statement of the code added by commit
>> > > f29801030ac6 where not true.
>> > >
>> > > /* disable TSO for pcie and 10/100 speeds, to avoid
>> > > * some hardware issues
>> > > */
>> > > if (!(adapter->flags & FLAG_TSO_FORCE)) {
>> >
>> > Yea, my idea was to take FLAG_TSO_FORCE into consideration hence the adding the change to this if block.
>> >
>> > Maybe someone still wants to enable TSO despite of the downside?
>>
>> By disabling TSO during probe we are not shutting it down completly, if a user wants to use it anyway despite speed decrease it can be done manually with ethtool.
>
>Maybe move the whole block of "FLAG_TSO_FORCE" to probe routine? So the code is logically grouped.
Sure, I will test how this works on my setup and then if everything will be fine I will post new version on IWL.
>
>Kai-Heng
>
Regards,
Mateusz
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-03-22 12:04 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-22 14:07 [Intel-wired-lan] [PATCH net v3] e1000e: Disable TSO on i219-LM card to increase speed Mateusz Palczewski
2023-02-22 14:22 ` Paul Menzel
2023-02-22 14:54 ` Kai-Heng Feng
2023-03-15 12:44 ` Palczewski, Mateusz
2023-03-17 7:34 ` Kai-Heng Feng
2023-03-22 12:04 ` Palczewski, Mateusz
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.