Page MenuHomeFreeBSD

Remove TFO from the packet processing path when the kernel is not configured with TCP_RFC7413
ClosedPublic

Authored by jtl on Oct 11 2016, 3:51 PM.

Details

Summary

The TFO server-side code contains some changes that are not conditioned on the TCP_RFC7413 kernel option. This change removes those few instructions from the packet processing path.

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

jtl retitled this revision from to Remove TFO from the packet processing path when the kernel is not configured with TCP_RFC7413.Oct 11 2016, 3:51 PM
jtl updated this object.
jtl edited the test plan for this revision. (Show Details)
jtl added reviewers: transport, rrs, gallatin.
jtl updated this revision to Diff 21252.
hiren added a reviewer: hiren.Oct 11 2016, 4:03 PM
hiren accepted this revision.
hiren added a subscriber: hiren.

Thanks for fixing these corner-cases. :-)

sys/netinet/tcp_input.c
3254 ↗(On Diff #21252)

Is this block necessary?

This revision is now accepted and ready to land.Oct 11 2016, 4:03 PM
jtl added inline comments.Oct 11 2016, 5:56 PM
sys/netinet/tcp_input.c
3254 ↗(On Diff #21252)

Its not necessary, but it constrains the macro to the same namespace as the variable it overrides.

jtl edited edge metadata.Oct 11 2016, 6:35 PM
jtl updated this revision to Diff 21264.

I guess we should be consistent and use the new macro everywhere in the packet processing path...

This revision now requires review to proceed.Oct 11 2016, 6:35 PM
hiren edited edge metadata.Oct 11 2016, 11:54 PM
hiren accepted this revision.
hiren added inline comments.
sys/netinet/tcp_input.c
3254 ↗(On Diff #21252)

I don't see us doing this to any other vars. Or probably I am missing something that's special with tfo_syn.

This revision is now accepted and ready to land.Oct 11 2016, 11:54 PM
jtl added inline comments.Oct 12 2016, 1:58 AM
sys/netinet/tcp_input.c
3254 ↗(On Diff #21252)

Normally, variables are confined to the scope in which they are defined. Here, I am trying to eliminate a variable which isn't needed unless TCP_RFC7413 is defined. So, if TCP_RFC7413 isn't defined, I have defined a macro to replace the occurrences of the variable with the constant value false. Just to be tidy, I'm undefining the macro at the end of the function. This gives the macro the same scope as the variable it replaces.

I probably could have achieved similar results with a definition like this:
const bool tfo_syn = false;

However, I thought the macro would be more likely to completely eliminate the instruction in all compilers. (I'm happy to be corrected if there's a more stylistically pleasing way to do this that produces the same functional result. :-) )

hiren added inline comments.Oct 12 2016, 5:01 PM
sys/netinet/tcp_input.c
3254 ↗(On Diff #21252)

Though I find const bool way cleaner, I'd totally leave it up to you to do it in a fashion that makes all compilers happy. :-) Thanks for answering my questions.

This revision was automatically updated to reflect the committed changes.