Page MenuHomeFreeBSD

LRO: Don't merge ACK and non-ACK packets together
ClosedPublic

Authored by rstone on Jan 6 2022, 10:55 PM.
Tags
None
Referenced Files
F104096202: D33775.diff
Tue, Dec 3, 12:22 PM
Unknown Object (File)
Tue, Nov 26, 2:14 PM
Unknown Object (File)
Thu, Nov 14, 1:39 PM
Unknown Object (File)
Oct 25 2024, 12:58 PM
Unknown Object (File)
Sep 28 2024, 5:28 PM
Unknown Object (File)
Sep 27 2024, 6:36 PM
Unknown Object (File)
Sep 23 2024, 9:24 PM
Unknown Object (File)
Sep 22 2024, 6:00 AM

Details

Summary

LRO was willing to merge ACK and non-ACK packets together. This
can cause incorrect th_ack values to be reported up the stack.
While non-ACKs are quite unlikely to appear in practice, LRO's
behaviour is against the spec. Make LRO unwilling to merge
packets with different TH_ACK flag values in order to fix the
issue.

Found by: 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#L573

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

Unexpected mock function call - returning directly.
    Function call: if_input(0x15c4d634d000)
Google Mock tried the following 2 expectations, but none matched:

/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:600: tried expectation #0: EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(pkt1)))...
  Expected arg #0: Ethernet/IPv4/TCP/Payload
           Actual: 0x15c4d634d000, IPv4: ip_len field is 54 (expected 47)
         Expected: to be called once
           Actual: never called - unsatisfied and active
/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:603: tried expectation #1: EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(pkt2)))...
  Expected arg #0: Ethernet/IPv4/TCP/Payload
           Actual: 0x15c4d634d000, IPv4: ip_len field is 54 (expected 47)
         Expected: to be called once
           Actual: never called - unsatisfied and active
/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:600: Failure
Actual function call count doesn't match EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(pkt1)))...
         Expected: to be called once
           Actual: never called - unsatisfied and active
/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:603: Failure
Actual function call count doesn't match EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(pkt2)))...
         Expected: to be called once
           Actual: never called - unsatisfied and active
[  FAILED  ] TcpLroTestSuite/0.TestNonAck, where TypeParam = IPv4 (1 ms)
[----------] 1 test from TcpLroTestSuite/0 (1 ms total)

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

Unexpected mock function call - returning directly.
    Function call: if_input(0x15c4d634d200)
Google Mock tried the following 2 expectations, but none matched:

/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:600: tried expectation #0: EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(pkt1)))...
  Expected arg #0: Ethernet/IPv6/TCP/Payload
           Actual: 0x15c4d634d200, IPv6: ip6_plen field is 34 (expected 27)
         Expected: to be called once
           Actual: never called - unsatisfied and active
/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:603: tried expectation #1: EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(pkt2)))...
  Expected arg #0: Ethernet/IPv6/TCP/Payload
           Actual: 0x15c4d634d200, IPv6: ip6_plen field is 34 (expected 27)
         Expected: to be called once
           Actual: never called - unsatisfied and active
/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:600: Failure
Actual function call count doesn't match EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(pkt1)))...
         Expected: to be called once
           Actual: never called - unsatisfied and active
/srcpool/src/rstone/worktree/sysunit/tests/sys/sysunit/tcp_lro_test.cc:603: Failure
Actual function call count doesn't match EXPECT_CALL(*this->mockIfp, if_input(PacketMatcher(pkt2)))...
         Expected: to be called once
           Actual: never called - unsatisfied and active
[  FAILED  ] TcpLroTestSuite/1.TestNonAck, where TypeParam = IPv6 (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  ] 0 tests.
[  FAILED  ] 2 tests, listed below:
[  FAILED  ] TcpLroTestSuite/0.TestNonAck, where TypeParam = IPv4
[  FAILED  ] TcpLroTestSuite/1.TestNonAck, where TypeParam = IPv6

 2 FAILED TESTS

It passes with the fix:

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

[----------] 1 test from TcpLroTestSuite/1, where TypeParam = IPv6
[ RUN      ] TcpLroTestSuite/1.TestNonAck
[       OK ] TcpLroTestSuite/1.TestNonAck (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 43751
Build 40639: arc lint + arc unit

Event Timeline

Ping? I'd like to get this off of my queue by some time 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:22 PM