netdev - Re: [net-next PATCH v4 2/3] net: TCP thin linear timeouts

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: <4B7D0779.6010609@lastsummer.de>
Date:	Thu, 18 Feb 2010 10:25:13 +0100
From:	Franco Fichtner <franco@...tsummer.de>
To:	Andreas Petlund <apetlund@...ula.no>
CC:	Ilpo Järvinen <ilpo.jarvinen@...sinki.fi>,
	Franco Fichtner <franco@...tsummer.de>,
	Netdev <netdev@...r.kernel.org>, eric.dumazet@...il.com,
	hannemann@...s.rwth-aachen.de, LKML <linux-kernel@...r.kernel.org>,
	shemminger@...tta.com, william.allen.simpson@...il.com,
	damian@....rwth-aachen.de, ebiederm@...ssion.com,
	David Miller <davem@...emloft.net>
Subject: Re: [net-next PATCH v4 2/3] net: TCP thin linear timeouts

Andreas Petlund wrote:
> On 02/18/2010 10:09 AM, Ilpo Järvinen wrote:
>   
>> On Thu, 18 Feb 2010, Franco Fichtner wrote:
>>
>>     
>>> Andreas Petlund wrote:
>>>       
>>>> On 02/18/2010 09:41 AM, Ilpo Järvinen wrote:
>>>>   
>>>>         
>>>>> On Wed, 17 Feb 2010, David Miller wrote:
>>>>>
>>>>>     
>>>>>           
>>>>>> From: Andreas Petlund <apetlund@...ula.no>
>>>>>> Date: Tue, 16 Feb 2010 15:40:41 +0100
>>>>>>
>>>>>>       
>>>>>>             
>>>>>>> @@ -341,6 +342,8 @@ struct tcp_sock {
>>>>>>>  	u16	advmss;		/* Advertised MSS
>>>>>>> */
>>>>>>>  	u8	frto_counter;	/* Number of new acks after RTO */
>>>>>>>  	u8	nonagle;	/* Disable Nagle algorithm?
>>>>>>> */
>>>>>>> +	u8      thin_lto    : 1,/* Use linear timeouts for thin
>>>>>>> streams */
>>>>>>> +		thin_undef  : 7;
>>>>>>>          
>>>>>>>               
>>>>>> There is now a gap of 3 unused bytes here in this critical
>>>>>> core TCP socket data structure.
>>>>>>
>>>>>> Please either find a way to avoid this hole, or document
>>>>>> it with a comment.
>>>>>>       
>>>>>>             
>>>>> There would be multiple bits free for use in both frto_counter and nonagle
>>>>> byte.
>>>>>
>>>>>     
>>>>>           
>>>> I was playing aroud with this setup:
>>>>
>>>> =========
>>>> u8	nonagle     : 4,/* Disable Nagle algorithm?             */
>>>> 	thin_lto    : 1,/* Use linear timeouts for thin streams */
>>>> 	thin_dupack : 1,/* Fast retransmit on first dupack      */
>>>> 	thin_undef  : 2;
>>>> =========
>>>>
>>>> Do you think that would do the trick?
>>>>   
>>>>         
>>> According to Ilpo, it would be ok to reduce both ftro_counter and
>>> nonagle, so why not join all these into u16 and leave the remaining
>>> free bits documented for other people. Like this:
>>>
>>>    u16 frto_counter:x; /* Number of new acks after RTO */
>>>    u16 nonagle:y; /* Disable Nagle algorithm? */
>>>    u16 thin_lto:1; /* Use linear timeouts for thin streams */
>>>    u16 unused:15-x-y;
>>>
>>> Not sure about the y and x. Ilpo, can you comment on those values?
>>>       
>> I don't remember top of the hat how much of nonagle used, but for 
>> frto_counter max value was 3 iirc. 
>>     
>
> I think nonagle uses 4 bits:
> ======
> #define TCP_NAGLE_OFF           1       /* Nagle's algo is disabled */
> #define TCP_NAGLE_CORK          2       /* Socket is corked         */
> #define TCP_NAGLE_PUSH          4       /* Cork is overridden for already queued data */
> ======
>
>   
That would be 3 bits. :)

>> However, I'm unsure if compiler is 
>> nowadays wise enough to handle bitfields in some not all so stupid way.
>>     
>
> Would you then recommend to use a byte for each value, thus avoiding the bitfields?
>   
No, he meant he's not sure if the compiler can merge the bitfields in a 
clever
way if written like this.

I use this style all the time at work and according to pahole it's okay. 
But then,
I'm not sure if pahole can be trusted or if there is some compiler twist on
non-intel architectures.


Franco
--
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