netdev - Re: [PATCH v2] tcp: Add TCP_INFO counter for packets received out-of-order

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]

Date:   Tue, 17 Sep 2019 13:12:28 -0400
From:   Jason Baron <jbaron@...mai.com>
To:     Eric Dumazet <edumazet@...gle.com>, Thomas Higdon <tph@...com>
Cc:     netdev <netdev@...r.kernel.org>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        Dave Jones <dsj@...com>
Subject: Re: [PATCH v2] tcp: Add TCP_INFO counter for packets received
 out-of-order

On 9/10/19 4:38 PM, Eric Dumazet wrote:
> On Tue, Sep 10, 2019 at 10:11 PM Thomas Higdon <tph@...com> wrote:
>>
>>
> ...
>> Because an additional 32-bit member in struct tcp_info would cause
>> a hole on 64-bit systems, we reserve a struct member '_reserved'.
> ...
>> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
>> index b3564f85a762..990a5bae3ac1 100644
>> --- a/include/uapi/linux/tcp.h
>> +++ b/include/uapi/linux/tcp.h
>> @@ -270,6 +270,9 @@ struct tcp_info {
>>         __u64   tcpi_bytes_retrans;  /* RFC4898 tcpEStatsPerfOctetsRetrans */
>>         __u32   tcpi_dsack_dups;     /* RFC4898 tcpEStatsStackDSACKDups */
>>         __u32   tcpi_reord_seen;     /* reordering events seen */
>> +
>> +       __u32   _reserved;           /* Reserved for future 32-bit member. */
>> +       __u32   tcpi_rcv_ooopack;    /* Out-of-order packets received */
>>  };
>>
> 
> Unfortunately we won't be able to use this hole, because the way the
> TCP_INFO works,
> 
> The kernel will report the same size after the reserved field is
> renamed to something else.
> 
> User space code is able to detect which fields are there or not based
> on what the kernel
> returns for the size of the structure.
> 

Hi,

I was interested in adding a field to tcp_info around the TFO state of a
socket. So for the server side it would indicate if TFO was used to
create the socket and on the client side it would report whether TFO
worked and if not that it failed with maybe some additional states
around why it failed. I'm thinking it would be maybe 3 bits.

My question is whether its reasonable to use the unused bits of
__u8    tcpi_delivery_rate_app_limited:1;. Or is this not good because
the size hasn't changed? What if I avoided using 0 for the new field to
avoid the possibility of not knowing if 0 because its the old kernel or
0 because that's now its a TFO state? IE the new field could always be >
0 for the new kernel.

Thanks,

-Jason