Page MenuHomeFreeBSD

Support printing of the TCP fast open client side cache
ClosedPublic

Authored by tuexen on Mar 1 2018, 9:01 AM.

Details

Summary

Add support to print the TCP fast open client-side cache via the sysctl interface. This is similar to the TCP host cache.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

tuexen created this revision.Mar 1 2018, 9:01 AM
pkelsey added inline comments.Mar 3 2018, 3:27 AM
sys/netinet/tcp_fastopen.c
141 ↗(On Diff #39853)

It is not a connection cache, it is the client-side cache of TFO cookies. I think this should read "Print the client cookie cache."

Also, please maintain the alphabetical sort of sysctls in this documentation block.

356 ↗(On Diff #39853)

"List of all client cookie cache entries"

1155 ↗(On Diff #39853)

I think we should also only allow root to list the contents. I think allowing any user to list the client cookie cache lowers the bar on initiating TFO attacks that require obtaining valid TFO cookies for known {client, server} tuples.

1166 ↗(On Diff #39853)

Is there any reason not to keep it simple, skip the estimated entry count and just use an auto-extending sbuf here?

1178 ↗(On Diff #39853)

Wouldn't it be simpler to just use inet_ntop(cce->af, cce->client_ip, clt_buf), and the same approach for the server address?

1194 ↗(On Diff #39853)

I think it would be useful also to show the current value of the disable time in seconds when the state is disabled.

tuexen updated this revision to Diff 39917.Mar 3 2018, 3:43 PM

Address most of the issues raised by Patrick

tuexen added inline comments.Mar 3 2018, 3:48 PM
sys/netinet/tcp_fastopen.c
141 ↗(On Diff #39853)

I updated to comment and moved it to the correct place.

356 ↗(On Diff #39853)

Fixed.

1155 ↗(On Diff #39853)

OK, I added that restriction.

1166 ↗(On Diff #39853)

The problem is that WITNESS then complains about using the uma allocator while holding a non-sleepable lock (the CCB_LOCK). So I kept it this way.

1178 ↗(On Diff #39853)

Done.

1194 ↗(On Diff #39853)

I added that. Changed the column title from "Status" to "Disabled".

tuexen edited the summary of this revision. (Show Details)Mar 3 2018, 4:24 PM
tuexen retitled this revision from Support printing of the TCP fast open connection cache to Support printing of the TCP fast open client side cache.
pkelsey accepted this revision.Mar 6 2018, 3:46 AM

Looks good. Thanks.

This revision is now accepted and ready to land.Mar 6 2018, 3:46 AM
kbowling accepted this revision.Jul 10 2018, 3:00 AM
This revision was automatically updated to reflect the committed changes.