netdev - Re: [rfc] suspicious indentation in do_tcp

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAK6E8=f=fLXxVeZ1s1LeXLB5ZzsM7YBvqTTB1GvgM3H+z8xyyQ@mail.gmail.com>
Date:	Thu, 5 Sep 2013 10:34:34 -0700
From:	Yuchung Cheng <ycheng@...gle.com>
To:	Neal Cardwell <ncardwell@...gle.com>
Cc:	Joe Perches <joe@...ches.com>, Dave Jones <davej@...hat.com>,
	Netdev <netdev@...r.kernel.org>
Subject: Re: [rfc] suspicious indentation in do_tcp_setsockopt

On Thu, Sep 5, 2013 at 10:24 AM, Neal Cardwell <ncardwell@...gle.com> wrote:
> On Thu, Sep 5, 2013 at 12:43 AM, Joe Perches <joe@...ches.com> wrote:
>> (Adding Yuchung Cheng and Neal Cardwell as the
>>  author and acker of the patch)
>>
>> On Thu, 2013-09-05 at 00:20 -0400, Dave Jones wrote:
>>> What's the intent here ?
>>>
>>> This ?
>>
>> I think the first is most likely.
>
> Yes, exactly. The first version makes more sense. We only need to
> check thin_dupack and potentially disable early retransmit if the
> setsockopt successfully changes thin_dupack.
ack. first version is the intended one. thanks.

>
> neal
>
>
>>>
>>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>>> index b2f6c74..95544e4 100644
>>> --- a/net/ipv4/tcp.c
>>> +++ b/net/ipv4/tcp.c
>>> @@ -2454,10 +2454,11 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
>>>       case TCP_THIN_DUPACK:
>>>               if (val < 0 || val > 1)
>>>                       err = -EINVAL;
>>> -             else
>>> +             else {
>>>                       tp->thin_dupack = val;
>>>                       if (tp->thin_dupack)
>>>                               tcp_disable_early_retrans(tp);
>>> +             }
>>>               break;
>>>
>>>       case TCP_REPAIR:
>>>
>>> Or this ...
>>>
>>> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
>>> index b2f6c74..187c5a4 100644
>>> --- a/net/ipv4/tcp.c
>>> +++ b/net/ipv4/tcp.c
>>> @@ -2456,8 +2456,9 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
>>>                       err = -EINVAL;
>>>               else
>>>                       tp->thin_dupack = val;
>>> -                     if (tp->thin_dupack)
>>> -                             tcp_disable_early_retrans(tp);
>>> +
>>> +             if (tp->thin_dupack)
>>> +                     tcp_disable_early_retrans(tp);
>>>               break;
>>>
>>>       case TCP_REPAIR:
>>>
>>>
>>> I'll submit the right patch in the right form once I know what was intended.
>>>
>>> The former seems more 'correct' to me, but I'm unsure if that could break something.
>>>
>>>       Dave
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo@...r.kernel.org
>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>>
>>
>>
>>
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html