Refactor PCB hash read lock (the epoch)
ClosedPublic
Actions

Authored by glebius on Oct 30 2019, 5:54 PM.

Details

Reviewers

None

Group Reviewers

network
transport

Commits

rS354477: Since r353292 on input path we are always in network epoch, when

Summary

Since r335356 INP_INFO_RLOCK() actually means entering network epoch.
Since r353292 we are in epoch whenever we process any incoming packet.
And we are moving towards entering epoch earlier for outgoing packets.

This branch reviews all places that do PCB hash locking. In some places
we just mechanically convert from INP_INFO_RLOCK() macro to NET_EPOCH_ENTER()
macro, to remove ambiguity. In the code that is supposed to already run
in the epoch, we remove the macro and we assert the epoch.

Since "read-locking" the PCB hash is no longer mutually exclusive
to write-locking it, some code can be greatly simplified. See
udp_output() and udp_output6(). These two functions now also
call into ip_output() and ip_output6() in epoch. For now that
creates recursion, but in the future all upper level protcols
will do that and IP level would assert epoch.

The change isn't intended to be committed at once. The split
into series of commits with more comments can be viewed here:

https://github.com/glebius/FreeBSD/commits/pcb_epoch

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 27282
Build 25543: arc lint + arc unit

Event Timeline

glebius created this revision.Oct 30 2019, 5:54 PM

Herald added 1 blocking reviewer(s): transport. · View Herald TranscriptOct 30 2019, 5:54 PM

Herald added a subscriber: ae. · View Herald Transcript

Harbormaster completed remote builds in B27282: Diff 63806.Oct 30 2019, 5:54 PM

In general I really like the changes. I've not looked into the large chunks of sysctls / UDP / TCP in too much detail. I'll try to do them based on their individual git changes the next days (at least sysctl and UDP). Hope someone else will do the TCP bits.

sys/netinet/in_pcb.c
2257	I was contemplating if the assertion should (also) be in the callers of this private function (at least for documentation purposes) given their callers (at least in some individual changes in git) explicitly acquire the et around the calls?
sys/netinet/tcp_input.c
1424	Unrelated whitespace change.
sys/netinet/toecore.c
394	Why are we losing that assertion on what in git is a "mechanical replacement"? Is there a purpose for losing it here?

glebius marked an inline comment as done.Nov 5 2019, 10:29 PM

glebius added inline comments.

sys/netinet/in_pcb.c
2257	I also contemplated that. AFAIK, doesn't make sense, as there is no code flow that would pass the assertion. See my other reply on comment.
sys/netinet/toecore.c
394	I usually removed the assertions in case when we: a) asserted it somewhere earlier b) would assert it in header of a function called on next line We can spread assertions virtually everywhere, but at some point they become superfluous.

This revision was not accepted when it landed; it landed in state Needs Review.Nov 7 2019, 8:50 PM

Closed by commit rS354477: Since r353292 on input path we are always in network epoch, when (authored by glebius). · Explain Why

This revision was automatically updated to reflect the committed changes.

glebius marked an inline comment as done.

glebius added a commit: rS354477: Since r353292 on input path we are always in network epoch, when.

Herald added a subscriber: imp. · View Herald TranscriptNov 7 2019, 8:50 PM

Revision Contents
Changeset List

Path

Size

sys/

dev/

cxgbe/

cxgbei/

cxgbei.c

4 lines

tom/

4 lines

20 lines

14 lines

4 lines

kern/

uipc_ktls.c

4 lines

netinet/

10 lines

13 lines

82 lines

77 lines

28 lines

109 lines

4 lines

tcp_stacks/

4 lines

17 lines

17 lines

122 lines

2 lines

47 lines

24 lines

38 lines

48 lines

6 lines

228 lines

netinet6/

5 lines

2 lines

5 lines

115 lines

Commit	Tree	Parents	Author	Summary	Date
ba1d3001a420	a63f205f7844	f1a87ddfb9eb	Gleb Smirnoff	Remove now unused INP_INFO_RLOCK macros.	Oct 25 2019, 5:38 AM
f1a87ddfb9eb	38412d340188	ca133b3223af	Gleb Smirnoff	In TCP HPTS enter the epoch in tcp_hpts_thread() and assert it in the leaf… (Show More…)	Oct 25 2019, 5:34 AM
ca133b3223af	9bf0e6806dbd	48029a1814d4	Gleb Smirnoff	Mechanically convert INP_INFO_RLOCK() to NET_EPOCH_ENTER() in TCP timewait… (Show More…)	Oct 25 2019, 5:23 AM
48029a1814d4	d765eba5835e	22f3c30f6af9	Gleb Smirnoff	Since pfslowtimo() runs in the network epoch, tcp_slowtimo() also does. This… (Show More…)	Oct 25 2019, 5:21 AM
22f3c30f6af9	325213280e40	347424d3a4ad	Gleb Smirnoff	The pcblist sysctls can be greatly simplified. All the previous double cycling… (Show More…)	Oct 24 2019, 9:46 PM
347424d3a4ad	430a8441910f	4f9d6f9da08b	Gleb Smirnoff	All of the tcp_input() and its branches are now executed in the network epoch. (Show More…)	Oct 24 2019, 11:17 PM
4f9d6f9da08b	9f0a1a26def5	89e7d7ec9d74	Gleb Smirnoff	udp_input() must be executed in the network epoch.	Oct 22 2019, 7:01 PM
89e7d7ec9d74	b10eb666c51f	6e7486c027a2	Gleb Smirnoff	Now with epoch synchronized PCB lookup tables we can greatly simplify locking… (Show More…)	Oct 17 2019, 5:09 PM
6e7486c027a2	4bbee2d627b1	b613a4c1a42f	Gleb Smirnoff	Add INP_UNLOCK() which will do whatever R/W unlock is required.	Oct 15 2019, 4:10 PM
b613a4c1a42f	652e593463c8	ff3b727ee08d	Gleb Smirnoff	Now with r353292 on input path we are always in network epoch, when we lookup… (Show More…)	Oct 15 2019, 2:58 AM
ff3b727ee08d	eba7828f2132	0d9d2bbb9e20	Gleb Smirnoff	divert_packet() is called from packet filters, thus always runs in the network… (Show More…)	Oct 24 2019, 11:26 PM
0d9d2bbb9e20	55f821f8d573	9785b31d8ae0	Gleb Smirnoff	icmp6_rip6_input() should run in the network epoch.	Oct 21 2019, 9:28 PM
9785b31d8ae0	5869812fdfe5	1cc44328ca43	Gleb Smirnoff	rip_input() and rip6_input() should run in the network epoch.	Oct 21 2019, 9:27 PM
1cc44328ca43	b380c36782ea	bd4aa536400e	Gleb Smirnoff	udp6_input() shall always run in network epoch.	Oct 21 2019, 9:16 PM
bd4aa536400e	e99f1f56f7e6	fb4e1bdf2294	Gleb Smirnoff	TCP timers are executed in callout context, so they need to enter network epoch… (Show More…)	Oct 25 2019, 12:10 AM
fb4e1bdf2294	08560bca6de1	8c6edd53ff8f	Gleb Smirnoff	Mechanically convert INP_INFO_RLOCK() to NET_EPOCH_ENTER() in TCP functions… (Show More…)	Oct 25 2019, 12:16 AM
8c6edd53ff8f	082144d4c41d	8a547ec5d92b	Gleb Smirnoff	Mechanically convert INP_INFO_RLOCK() to NET_EPOCH_ENTER(). No functional… (Show More…)	Oct 18 2019, 3:13 PM