Add sysctl to disable the TCP hostcache
ClosedPublic
Actions

Authored by j-nitrology.com on May 4 2016, 4:19 AM.

Details

Reviewers

hiren
rrs
sbruno
bz

Group Reviewers

transport

Commits

rS306464: This adds a sysctl which allows you to disable the TCP hostcache. This is handy

Summary

This adds a sysctl which allows you to disable the TCP hostcache. This is handy during testing of network related changes where cached entries may pollute your results, or during known congestion events where you don't want to unfairly penalize hosts.

Prior to r232346 this would have meant you would break any connection with a sub 1500 MTU, as the hostcache was authoritative. All entries as they stand today should simply be used to pre populate values for efficiency.

Test Plan

This has been running in production at Limelight

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

j-nitrology.com updated this revision to Diff 15882.May 4 2016, 4:19 AM

j-nitrology.com retitled this revision from to Add sysctl to disable the TCP hostcache.

j-nitrology.com updated this object.

j-nitrology.com edited the test plan for this revision. (Show Details)

j-nitrology.com added a reviewer: hiren.

Herald added a reviewer: transport. · View Herald TranscriptMay 4 2016, 4:19 AM

Herald added a subscriber: imp. · View Herald Transcript

j-nitrology.com added a comment.May 4 2016, 4:23 AM

This comment was removed by j-nitrology.com.

The default needs to maintain the current behavior

Herald edited edge metadata. · View Herald TranscriptMay 4 2016, 4:29 AM

bz requested changes to this revision.May 4 2016, 10:01 AM

bz added a reviewer: bz.

bz added a subscriber: bz.

bz added inline comments.

sys/netinet/tcp_hostcache.c
283 ↗	(On Diff #15883)	This needs to go after variable declarations
341 ↗	(On Diff #15883)	This needs to go after the variable declarations; same applies for all further down; I'll stop repeating myself ;)
470 ↗	(On Diff #15883)	Maybe we should also fix the comment to say zero instead of NULL

This revision now requires changes to proceed.May 4 2016, 10:01 AM

This seems like a good idea, but fixing the style bugs (e.g., local variable definitions should be at the start of the function before any executable code) is necessary.

Making it per-VNET caused me slight pondering, but seems like the right overall choice.

I'm glad that the description of the patch indicates that code calling into the cache has been inspected for poor interactions with the cache being disabled. I assume experimentation has taken place on both IPv4 and IPv6?

I've moved all functionality to post declaration and updated the comment about the return on tcp_hc_getmtu().
@rwatson yes I just figure it's a near free feature should you be using jails to do various testing. It has been tested with both v4 and v6 traffic.

j-nitrology.com updated this object.May 4 2016, 6:23 PM

j-nitrology.com edited edge metadata.

When someone has a moment, might this look good after the updates?

Seems like this is good to go in. @bz @rwatson ?

I'll go ahead with a commit in a couple days if I don't hear anything back. Thank you for looking into it.

All review comments have been addressed by submitter.

rrs accepted this revision.Jul 5 2016, 1:33 PM

rrs added a reviewer: rrs.

Closed by commit rS306464: This adds a sysctl which allows you to disable the TCP hostcache. This is handy (authored by hiren). · Explain WhySep 30 2016, 12:11 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

head/

sys/

netinet/

tcp_hostcache.c

26 lines

Diff 20834

View Options

head/sys/netinet/tcp_hostcache.c

Add sysctl to disable the TCP hostcacheClosedPublicActions