Page MenuHomeFreeBSD
Differential D41059

tcp: improve layout of struct tcpcb
ClosedPublic
Actions

Authored by tuexen on Jul 17 2023, 6:45 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, May 14, 12:38 PM
Unknown Object (File)
Wed, May 13, 10:29 PM
Unknown Object (File)
Wed, May 13, 10:29 PM
Unknown Object (File)
Wed, May 13, 10:22 PM
Unknown Object (File)
Fri, May 8, 6:16 PM
Unknown Object (File)
Thu, May 7, 9:13 AM
Unknown Object (File)
Thu, May 7, 2:00 AM
Unknown Object (File)
Wed, May 6, 8:08 AM
View All Files
Subscribers
emaste
melifaro

Details

Reviewers
imp
jhb
rrs
rscheff
cc
jtl
glebius
gallatin
Group Reviewers
transport
Commits
rGe4a873bf10b7: tcp: improve layout of struct tcpcb
Summary

Move the conditional stuff to the end. I hope we can remove TCPPCAP in FreeBSD 15.

Not using #ifdef for TCP_ACCOUNTING and TCP_REQUEST_TRK would increase the size of the struct. I am open to it, but based on a discussion with rrs@, the compromise was to move them to the end.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tuexen created this revision.Jul 17 2023, 6:45 PM
Herald added a reviewer: transport. · View Herald TranscriptJul 17 2023, 6:45 PM
Herald added a subscriber: melifaro. · View Herald Transcript
tuexen requested review of this revision.Jul 17 2023, 6:45 PM
tuexen added a reviewer: gallatin.
tuexen edited the summary of this revision. (Show Details)Jul 17 2023, 6:47 PM
jhb added a comment.Jul 17 2023, 6:53 PM

Moving to the end is certainly sensible. Have you checked for any padding holes on at least LP64 with ptype /o in GDB?

emaste added a subscriber: emaste.Jul 17 2023, 7:49 PM
gallatin added inline comments.Jul 17 2023, 7:52 PM
sys/netinet/tcp_var.h
473

You're creating a 7 byte alignment hole.. I'd try to keep the uint8_t's together. Eg, put TCP_ACCOUNTING in a different place, and put the tcp_hybrid* after tcp_sendfile_track.

tuexen added inline comments.Jul 17 2023, 8:39 PM
sys/netinet/tcp_var.h
473

When moving TCP_ACCOUNTING around, it is not at the end anymore... That is the only intention here...

tuexen updated this revision to Diff 124788.Jul 17 2023, 9:48 PM

Avoid holes on 64-bit systems when all options are defined.

tuexen added inline comments.Jul 17 2023, 9:49 PM
sys/netinet/tcp_var.h
473

Moved things around, now avoiding any holes on 64-bit platforms when TCP_ACCOUNTING and TCP_REQUEST_TRK is defined.

gallatin accepted this revision.Jul 17 2023, 10:53 PM
imp accepted this revision.Jul 18 2023, 6:01 AM

I think this will work much better. Thanks

cc accepted this revision as: cc.Jul 18 2023, 2:17 PM
glebius accepted this revision.Jul 18 2023, 3:37 PM

About t_osd. It seems to be used only by alternative cc modules. Can we move it into their software context structures?

This revision was not accepted when it landed; it landed in state Needs Review.Jul 19 2023, 12:51 PM
Closed by commit rGe4a873bf10b7: tcp: improve layout of struct tcpcb (authored by tuexen). · Explain Why
This revision was automatically updated to reflect the committed changes.
tuexen added a commit: rGe4a873bf10b7: tcp: improve layout of struct tcpcb.

Revision Contents
Changeset List

PathSize
sys/
netinet/
24 lines

Diff 124841

View Options

sys/netinet/tcp_var.h

Loading...