Page MenuHomeFreeBSD

LRO: Fix lost packets when merging 1 payload with an ACK
ClosedPublic

Authored by rstone on Jan 6 2022, 10:55 PM.
Tags
None
Referenced Files
Unknown Object (File)
Oct 18 2024, 9:16 PM
Unknown Object (File)
Oct 14 2024, 12:25 PM
Unknown Object (File)
Sep 24 2024, 5:24 AM
Unknown Object (File)
Sep 24 2024, 3:21 AM
Unknown Object (File)
Sep 23 2024, 4:04 PM
Unknown Object (File)
Sep 12 2024, 6:03 PM
Unknown Object (File)
Sep 10 2024, 3:33 PM
Unknown Object (File)
Sep 7 2024, 10:22 AM

Details

Summary

To check if it needed to regenerate a packet's header before
sending it up the stack, LRO was checking if more than one payload
had been merged into the packet. This failed in the case where
a single payload was merged with one or more pure ACKs. This
results in lost ACKs.

Fix this by precisely tracking whether header regeneration is
required instead of using an incorrect heuristic.

Found with: Sysunit test

Test Plan

This unit test fails without the fix:
https://github.com/rysto32/freebsd/blob/sysunit/tests/sys/sysunit/tcp_lro_test.cc#L381

$ ./tcp_lro --gtest_filter=*TestIncrPureAck 
Running main() from /srcpool/src/rstone/worktree/sysunit/contrib/googletest/googletest/src/gtest_main.cc
Note: Google Test filter = *TestIncrPureAck
[==========] Running 2 tests from 2 test cases.
[----------] Global test environment set-up.
[----------] 1 test from TcpLroTestSuite/0, where TypeParam = IPv4
[ RUN      ] TcpLroTestSuite/0.TestIncrPureAck
unknown file: Failure

Unexpected mock function call - returning directly.
    Function call: if_input(0x38ca7d33e000)
Google Mock tried the following 1 expectation, but it didn't match:

/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:400: EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(expected)))...
  Expected arg #0: Ethernet/IPv4/TCP/Payload
           Actual: 0x38ca7d33e000, TCP: th_ack field is 965 (expected 3854)
         Expected: to be called once
           Actual: never called - unsatisfied and active
/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:400: Failure
Actual function call count doesn't match EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(expected)))...
         Expected: to be called once
           Actual: never called - unsatisfied and active
[  FAILED  ] TcpLroTestSuite/0.TestIncrPureAck, where TypeParam = IPv4 (1 ms)
[----------] 1 test from TcpLroTestSuite/0 (1 ms total)

[----------] 1 test from TcpLroTestSuite/1, where TypeParam = IPv6
[ RUN      ] TcpLroTestSuite/1.TestIncrPureAck
unknown file: Failure

Unexpected mock function call - returning directly.
    Function call: if_input(0x38ca7d33e000)
Google Mock tried the following 1 expectation, but it didn't match:

/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:400: EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(expected)))...
  Expected arg #0: Ethernet/IPv6/TCP/Payload
           Actual: 0x38ca7d33e000, TCP: th_ack field is 965 (expected 3854)
         Expected: to be called once
           Actual: never called - unsatisfied and active
/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:400: Failure
Actual function call count doesn't match EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(expected)))...
         Expected: to be called once
           Actual: never called - unsatisfied and active
[  FAILED  ] TcpLroTestSuite/1.TestIncrPureAck, where TypeParam = IPv6 (1 ms)
[----------] 1 test from TcpLroTestSuite/1 (1 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 2 test cases ran. (2 ms total)
[  PASSED  ] 0 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] TcpLroTestSuite/0.TestIncrPureAck, where TypeParam = IPv4
[  FAILED  ] TcpLroTestSuite/1.TestIncrPureAck, where TypeParam = IPv6

 2 FAILED TESTS

It passes with the fix:

$ ./tcp_lro --gtest_filter=*TestIncrPureAck
Running main() from /srcpool/src/rstone/worktree/sysunit/contrib/googletest/googletest/src/gtest_main.cc
Note: Google Test filter = *TestIncrPureAck
[==========] Running 2 tests from 2 test cases.
[----------] Global test environment set-up.
[----------] 1 test from TcpLroTestSuite/0, where TypeParam = IPv4
[ RUN      ] TcpLroTestSuite/0.TestIncrPureAck
[       OK ] TcpLroTestSuite/0.TestIncrPureAck (1 ms)
[----------] 1 test from TcpLroTestSuite/0 (1 ms total)

[----------] 1 test from TcpLroTestSuite/1, where TypeParam = IPv6
[ RUN      ] TcpLroTestSuite/1.TestIncrPureAck
[       OK ] TcpLroTestSuite/1.TestIncrPureAck (0 ms)
[----------] 1 test from TcpLroTestSuite/1 (0 ms total)

[----------] Global test environment tear-down
[==========] 2 tests from 2 test cases ran. (1 ms total)
[  PASSED  ] 2 tests.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 43750
Build 40638: arc lint + arc unit

Event Timeline

rstone edited the test plan for this revision. (Show Details)

Ping? I'd like to get this patch and the other LRO patch in by next week.

I'll bring it up on the transport call tomorrow.

This revision is now accepted and ready to land.Jan 13 2022, 3:23 PM