[PATCH] sis900: re-enable high throughput

[PATCH] sis900: re-enable high throughput

[Sergej Benilov]

Since commit 605ad7f184b60cfaacbc038aa6c55ee68dee3c89 "tcp: refine TSO autosizing",
the TSQ limit is computed as the smaller of
sysctl_tcp_limit_output_bytes and max(2 * skb->truesize, sk->sk_pacing_rate >> 10).
For some connections this approach sets a low limit, reducing throughput dramatically.

Add a call to skb_orphan() to sis900_start_xmit()
to speed up packets delivery from the kernel to the driver.

Test:
netperf -H remote -l -2000000 -- -s 1000000

before patch:

MIGRATED TCP STREAM TEST from 0.0.0.0 () port 0 AF_INET to remote () port 0 AF_INET : demo
Recv   Send    Send
Socket Socket  Message  Elapsed
Size   Size    Size     Time     Throughput
bytes  bytes   bytes    secs.    10^6bits/sec

 87380 327680 327680    341.79      0.05

after patch:

MIGRATED TCP STREAM TEST from 0.0.0.0 () port 0 AF_INET to remote () port 0 AF_INET : demo
Recv   Send    Send
Socket Socket  Message  Elapsed
Size   Size    Size     Time     Throughput
bytes  bytes   bytes    secs.    10^6bits/sec

 87380 327680 327680    1.29       12.54

Signed-off-by: Sergej Benilov <sergej.benilov@googlemail.com>
---
 drivers/net/ethernet/sis/sis900.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
index fd812d2e..ca17b50c 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -1604,6 +1604,7 @@ sis900_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
 	unsigned int  index_cur_tx, index_dirty_tx;
 	unsigned int  count_dirty_tx;
 
+	skb_orphan(skb);
 	spin_lock_irqsave(&sis_priv->lock, flags);
 
 	/* Calculate the next Tx descriptor entry. */
-- 
2.17.1

Re: [PATCH] sis900: re-enable high throughput

[Sergej Benilov]

On Fri, 7 Jun 2019 at 19:26, Sergej Benilov
<sergej.benilov@googlemail.com> wrote:
>
> Since commit 605ad7f184b60cfaacbc038aa6c55ee68dee3c89 "tcp: refine TSO autosizing",
> the TSQ limit is computed as the smaller of
> sysctl_tcp_limit_output_bytes and max(2 * skb->truesize, sk->sk_pacing_rate >> 10).
> For some connections this approach sets a low limit, reducing throughput dramatically.
>
> Add a call to skb_orphan() to sis900_start_xmit()
> to speed up packets delivery from the kernel to the driver.
>
> Test:
> netperf -H remote -l -2000000 -- -s 1000000
>
> before patch:
>
> MIGRATED TCP STREAM TEST from 0.0.0.0 () port 0 AF_INET to remote () port 0 AF_INET : demo
> Recv   Send    Send
> Socket Socket  Message  Elapsed
> Size   Size    Size     Time     Throughput
> bytes  bytes   bytes    secs.    10^6bits/sec
>
>  87380 327680 327680    341.79      0.05
>
> after patch:
>
> MIGRATED TCP STREAM TEST from 0.0.0.0 () port 0 AF_INET to remote () port 0 AF_INET : demo
> Recv   Send    Send
> Socket Socket  Message  Elapsed
> Size   Size    Size     Time     Throughput
> bytes  bytes   bytes    secs.    10^6bits/sec
>
>  87380 327680 327680    1.29       12.54
>
> Signed-off-by: Sergej Benilov <sergej.benilov@googlemail.com>
> ---
>  drivers/net/ethernet/sis/sis900.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
> index fd812d2e..ca17b50c 100644
> --- a/drivers/net/ethernet/sis/sis900.c
> +++ b/drivers/net/ethernet/sis/sis900.c
> @@ -1604,6 +1604,7 @@ sis900_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
>         unsigned int  index_cur_tx, index_dirty_tx;
>         unsigned int  count_dirty_tx;
>
> +       skb_orphan(skb);
>         spin_lock_irqsave(&sis_priv->lock, flags);
>
>         /* Calculate the next Tx descriptor entry. */
> --
> 2.17.1
>

Thanks to Eric Dumazet for suggesting this patch

Re: [PATCH] sis900: re-enable high throughput

[Eric Dumazet]

On 6/7/19 10:31 AM, Sergej Benilov wrote:
> On Fri, 7 Jun 2019 at 19:26, Sergej Benilov
> <sergej.benilov@googlemail.com> wrote:
>>
>> Since commit 605ad7f184b60cfaacbc038aa6c55ee68dee3c89 "tcp: refine TSO autosizing",
>> the TSQ limit is computed as the smaller of
>> sysctl_tcp_limit_output_bytes and max(2 * skb->truesize, sk->sk_pacing_rate >> 10).
>> For some connections this approach sets a low limit, reducing throughput dramatically.
>>
>> Add a call to skb_orphan() to sis900_start_xmit()
>> to speed up packets delivery from the kernel to the driver.
>>
>> Test:
>> netperf -H remote -l -2000000 -- -s 1000000
>>
>> before patch:
>>
>> MIGRATED TCP STREAM TEST from 0.0.0.0 () port 0 AF_INET to remote () port 0 AF_INET : demo
>> Recv   Send    Send
>> Socket Socket  Message  Elapsed
>> Size   Size    Size     Time     Throughput
>> bytes  bytes   bytes    secs.    10^6bits/sec
>>
>>  87380 327680 327680    341.79      0.05
>>
>> after patch:
>>
>> MIGRATED TCP STREAM TEST from 0.0.0.0 () port 0 AF_INET to remote () port 0 AF_INET : demo
>> Recv   Send    Send
>> Socket Socket  Message  Elapsed
>> Size   Size    Size     Time     Throughput
>> bytes  bytes   bytes    secs.    10^6bits/sec
>>
>>  87380 327680 327680    1.29       12.54
>>
>> Signed-off-by: Sergej Benilov <sergej.benilov@googlemail.com>
>> ---
>>  drivers/net/ethernet/sis/sis900.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/net/ethernet/sis/sis900.c b/drivers/net/ethernet/sis/sis900.c
>> index fd812d2e..ca17b50c 100644
>> --- a/drivers/net/ethernet/sis/sis900.c
>> +++ b/drivers/net/ethernet/sis/sis900.c
>> @@ -1604,6 +1604,7 @@ sis900_start_xmit(struct sk_buff *skb, struct net_device *net_dev)
>>         unsigned int  index_cur_tx, index_dirty_tx;
>>         unsigned int  count_dirty_tx;
>>
>> +       skb_orphan(skb);
>>         spin_lock_irqsave(&sis_priv->lock, flags);
>>
>>         /* Calculate the next Tx descriptor entry. */
>> --
>> 2.17.1
>>
> 
> Thanks to Eric Dumazet for suggesting this patch

Note that this suggests the driver is not performing TX completion fast enough.

Looking at the driver, I do not see anything requesting interrupt mitigation,
so this might also be caused by a race in the driver (some skbs being not TX completed until
another unrelated xmit is requested)

Re: [PATCH] sis900: re-enable high throughput

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 7 Jun 2019 12:18:21 -0700

> Note that this suggests the driver is not performing TX completion
> fast enough.
> 
> Looking at the driver, I do not see anything requesting interrupt
> mitigation, so this might also be caused by a race in the driver
> (some skbs being not TX completed until another unrelated xmit is
> requested)

I think the INTR bit needs to be set in the individual TX descriptors.

This is a classic case of a chip that only interrupts when the entire
TX queue is emptied and the TX engine enters the idle state.

I don't like this skb_orphan() solution as it kills UDP flow control.

I'm not applying this.