netdev - Re: Linux ECN Handling

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACJspmKn=WhhK+-0XkcuYEu5D1K0QVpAbN=+8Cg9x3k0-D0cUA@mail.gmail.com>
Date:   Wed, 20 Dec 2017 11:20:31 -0800
From:   Steve Ibanez <sibanez@...nford.edu>
To:     Neal Cardwell <ncardwell@...gle.com>
Cc:     Eric Dumazet <edumazet@...gle.com>,
        Yuchung Cheng <ycheng@...gle.com>,
        Daniel Borkmann <daniel@...earbox.net>,
        Netdev <netdev@...r.kernel.org>, Florian Westphal <fw@...len.de>,
        Mohammad Alizadeh <alizadeh@...il.mit.edu>,
        Lawrence Brakmo <brakmo@...com>
Subject: Re: Linux ECN Handling

Hi Neal,

I added in some more printk statements and it does indeed look like
all of these calls you listed are being invoked successfully. I guess
this isn't too surprising given what the inet_csk_schedule_ack() and
inet_csk_ack_scheduled() functions are doing:

static inline void inet_csk_schedule_ack(struct sock *sk)
{
        inet_csk(sk)->icsk_ack.pending |= ICSK_ACK_SCHED;
}

static inline int inet_csk_ack_scheduled(const struct sock *sk)
{
        return inet_csk(sk)->icsk_ack.pending & ICSK_ACK_SCHED;
}

So through the code path that you listed, the inet_csk_schedule_ack()
function sets the ICSK_ACK_SCHED bit and then the tcp_ack_snd_check()
function just checks that the ICSK_ACK_SCHED bit is indeed set.

Do you know how I can verify that setting the ICSK_ACK_SCHED bit
actually results in an ACK being sent?

Thanks,
-Steve

On Tue, Dec 19, 2017 at 4:08 PM, Neal Cardwell <ncardwell@...gle.com> wrote:
> On Tue, Dec 19, 2017 at 5:00 PM, Steve Ibanez <sibanez@...nford.edu> wrote:
>> Hi Neal,
>>
>> I managed to track down the code path that the unACKed CWR packet is
>> taking. The tcp_rcv_established() function calls tcp_ack_snd_check()
>> at the end of step5 and then the return statement indicated below is
>> invoked, which prevents the __tcp_ack_snd_check() function from
>> running.
>>
>> static inline void tcp_ack_snd_check(struct sock *sk)
>> {
>>         if (!inet_csk_ack_scheduled(sk)) {
>>                 /* We sent a data segment already. */
>>                 return;   /* <=== here */
>>         }
>>         __tcp_ack_snd_check(sk, 1);
>> }
>>
>> So somehow tcp_ack_snd_check() thinks that a data segment was already
>> sent when in fact it wasn't. Do you see a way around this issue?
>
> Thanks for tracking that down! AFAICT in this case the call chain we
> are trying to achieve is as follows:
>
> tcp_rcv_established()
>  -> tcp_data_queue()
>  -> tcp_event_data_recv()
>  -> inet_csk_schedule_ack()
>
> The only think I can think of would be to add printks that fire for
> CWR packets, to isolate why the code bails out before it reaches those
> calls...
>
> thanks,
> neal