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)
Mon, Dec 1, 6:18 PM
Unknown Object (File)
Sun, Nov 30, 9:35 AM
Unknown Object (File)
Tue, Nov 25, 6:45 AM
Unknown Object (File)
Sun, Nov 23, 9:14 AM
Unknown Object (File)
Thu, Nov 20, 11:32 AM
Unknown Object (File)
Wed, Nov 19, 3:50 PM
Unknown Object (File)
Tue, Nov 18, 11:14 AM
Unknown Object (File)
Sun, Nov 9, 6:06 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 Skipped
Unit
Tests Skipped

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 124788

View Options

sys/netinet/tcp_var.h

Loading...