netdev - Re: [PATCH net-next v3 04/17] mptcp: Add handling of outgoing MP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4439b5b8-f737-2a3d-be24-b10aa2214aca@gmail.com>
Date:   Wed, 27 Oct 2021 13:38:23 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Paolo Abeni <pabeni@...hat.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Mat Martineau <mathew.j.martineau@...ux.intel.com>,
        netdev@...r.kernel.org
Cc:     Peter Krystad <peter.krystad@...ux.intel.com>,
        Florian Westphal <fw@...len.de>,
        Matthieu Baerts <matthieu.baerts@...sares.net>
Subject: Re: [PATCH net-next v3 04/17] mptcp: Add handling of outgoing MP_JOIN
 requests



On 10/27/21 1:45 AM, Paolo Abeni wrote:
> On Tue, 2021-10-26 at 17:54 -0700, Eric Dumazet wrote:
>>

>>
>> What about this fix ?
>>
>> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
>> index 422f4acfb3e6d6d41f6f5f820828eaa40ffaa6b9..9f5edcf562c9f98539256074b8f587c0a64a8693 100644
>> --- a/net/mptcp/options.c
>> +++ b/net/mptcp/options.c
>> @@ -434,12 +434,13 @@ static void schedule_3rdack_retransmission(struct sock *sk)
>>  
>>         /* reschedule with a timeout above RTT, as we must look only for drop */
>>         if (tp->srtt_us)
>> -               timeout = tp->srtt_us << 1;
>> +               timeout = usecs_to_jiffies(tp->srtt_us >> (3-1));
>>         else
>>                 timeout = TCP_TIMEOUT_INIT;
>>  
>>         WARN_ON_ONCE(icsk->icsk_ack.pending & ICSK_ACK_TIMER);
>>         icsk->icsk_ack.pending |= ICSK_ACK_SCHED | ICSK_ACK_TIMER;
>> +       timeout += jiffies;
>>         icsk->icsk_ack.timeout = timeout;
>>         sk_reset_timer(sk, &icsk->icsk_delack_timer, timeout);
>>  }
> 
> Thanks Eric for catching this! We need better packetdrill coverage
> here. I would be courious to learn how did you get it? 
> 

I found this because I am evaluating converting the delack timer
to hrtimer for TCP.

Kind of extending the ack compression schem I added for SACK,
but for pure ACKS.

Reducing ACK rate is key to performance, especially for drivers not using GRO.

So I was looking at all uses of icsk_delack_timer in the stack,
seeing how much work I would have to do.


> The patch LGTM, would you formally submit it, or do you prefer we cope
> with the process?
> 
> Thanks!
> 
> Paolo
>