tcpcb cleanup?

[Randall Stewart]

That should change soon :)

R

> On Jul 30, 2019, at 2:45 PM, Scheffenegger, Richard <Richard.Scheffenegger at netapp.com> wrote:
> 
> I believe the alignment of some pointers is also off, further shifting the data...
> 
> I started some work in D21117 around this - reducing the size of this t_rttupdated in tcpcb down from ulong to uint8_t, as it really only needs to track the initial few rounds, and then stay fixed at the UCHAR_MAX.
> 
> Also, I made other types, particularly pure counters, uint32_t (from int), giving them more range.
> 
> It seems that some variables, that are only (currently) used in the NF RACK stack, have "leaked" out already 😊 (t_maxunacktime is referenced to, but apparently never set, in the upstreamed RACK stack).
> 
> Best regards,
> 
> 
> Richard Scheffenegger
> 
> -----Original Message-----
> From: Randall Stewart <rrs at netflix.com> 
> Sent: Dienstag, 30. Juli 2019 08:41
> To: Scheffenegger, Richard <Richard.Scheffenegger at netapp.com>
> Cc: freebsd-transport at freebsd.org
> Subject: Re: tcpcb cleanup?
> 
> NetApp Security WARNING: This is an external email. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> 
> 
> Yes
> 
> As I said in comments to your removal of the sack field, they have drifted since my first work.
> 
> I am not too concerned since the first 3 cache lines are the most used and most important.
> 
> I do intend to do a re-analysis once I get BBR and the latest Rack in to see if we could improve things.. though it is doubtful that we will gain much (but you never know) ;)
> 
> R
> 
>> On Jul 29, 2019, at 11:13 PM, Scheffenegger, Richard <Richard.Scheffenegger at netapp.com> wrote:
>> 
>> Hi,
>> 
>> I've been looking into the cache line alignment today (because D18811).
>> 
>> Found that the commented cache lines only align to line 3 - then they drift from the comments.
>> 
>> For example, t_rttupdated is defined as u_long (8 bytes), while it really only tracks if at least some (small) number of rtt samples were collected, to start the  use of the rtt vars...
>> 
>> Realistically, a uint8_t with a limited increment (if (x<255) x++) would serve the very same function...
>> 
>> Other example: t_sndzerowin (in tcpcb) appears to be only a counter, better placed into the tcpstat structure, where non-functional counters belong IMHO. (perhaps a per-session variant).
>> 
>> 
>> 
>> 
>> And the other variables could probably be defined in the explicit types (uint16/32/64), to be more certain of the alignment - plus making sure that alignment boundaries between different sized types don't result in unintentional shifting of the alignment (like it seems currently to be the case).
>> 
>> Another prime example: struct sackhint is 40 bytes, only 24 of these are actually used; and ideally, sackblks, snd_fack share the same line, while sackhint and snd_holes would do the same (currently, both are distributed across 2)        ....
>> 
>> 
>> Any appetite to get the (higher) cache lines aligned in tcpcb?
>> 
>> Best regards,
>>  Richard
>> 
>> 
>> _______________________________________________
>> freebsd-transport at freebsd.org mailing list 
>> https://lists.freebsd.org/mailman/listinfo/freebsd-transport
>> To unsubscribe, send any mail to "freebsd-transport-unsubscribe at freebsd.org"
> 
> ------
> Randall Stewart
> rrs at netflix.com
> 
> 
> 

------
Randall Stewart
rrs at netflix.com