mptcp: add_addr: more tolerant with retrans timeout after errors #98
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The only difference was the injected and faulty ADD_ADDR: if it is faulty, it doesn't really matter if it is in v4 or v6, the main point is to send an ADD_ADDR with the wrong IP. While at it, also add 'errors' in the name in preparation to other 'retry' scenarios. Signed-off-by: Matthieu Baerts <[email protected]>
On very slow hosts, injected packets cause troubles because it reset timers in Packetdrill, see: multipath-tcp/mptcp_net-next#312 Add a new "safer" version without these injected packets to monitor if everything is OK like that. Signed-off-by: Matthieu Baerts <[email protected]>
ADD_ADDR restransmissions are expected to come ~1s after the last sent packet. In packetdrill, the timing is compared to the last packet. We would like to tell Packetdrill that the packet is expected to come "1s not after the previous packet but the one before" but that's not possible. In this test the last packet before the ADD_ADDR retransmission is not the ADD_ADDR that is sent before but it is the injected and faulty echo ADD_ADR. With a (very) slow host, it can takes a bit of time to inject the packet. Because of that, the retransmitted packet can arrive earlier than expected: if the faulty echo ADD_ADDR injection takes X sec, the retransmission of the ADD_ADDR by the kernel will take (1 - X) sec. If X is bigger than the tolerance, the test fails which seems to happen regularly on the public CI. add_addr_retry_v4.pkt:22: error handling packet: timing error: expected outbound packet at 3.214190 sec but happened at 2.384757 sec; tolerance 0.800000 sec script packet: 3.214190 . 1:1(0) ack 1 <add_address address_id: 1 ipv4: 192.168.0.3 hmac: 10354569113664661296> actual packet: 2.384757 . 1:1(0) ack 1 win 256 <add_address address_id: 1 ipv4: 192.168.0.3 hmac: 10354569113664661296> We should then accept packets being sent less than one second before the injected and faulty echo ADD_ADDR. Even worst with a debug kernel config: add_addr_retry_v4.pkt:22: error handling packet: timing error: expected outbound packet at 10.335930 sec but happened at 5.908318 sec; tolerance 2.000000 sec script packet: 10.335930 . 1:1(0) ack 1 <add_address address_id: 1 ipv4: 192.168.0.3 hmac: 14923819917020444620> actual packet: 5.908318 . 1:1(0) ack 1 win 256 <add_address address_id: 1 ipv4: 192.168.0.3 hmac: 14923819917020444620> Injecting the faulty echo and processing packets sent by the kernel can take time and we have situations where packets arrive a few seconds before the expected time by Packetdrill! Sadly, we cannot tell Packetdrill the packet is expected to be sent in the past. So we need to increment the tolerance a bit. But that's find to do that because a new test has been added in the parent commit: it is focussing on the ADD_ADDR retransmissions without injecting other packets in between. This other test can have stricter expected time. Closes: multipath-tcp/mptcp_net-next#312 Suggested-by: Paolo Abeni <[email protected]> Signed-off-by: Matthieu Baerts <[email protected]>
pabeni
approved these changes
Nov 3, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ADD_ADDR restransmissions are expected to come ~1s after the last sent packet.
In packetdrill, the timing is compared to the last packet. We would like to tell Packetdrill that the packet is expected to come "1s not after the previous packet but the one before" but that's not possible.
In this test the last packet before the ADD_ADDR retransmission is not the ADD_ADDR that is sent before but it is the injected and faulty echo ADD_ADR. With a (very) slow host, it can takes a bit of time to inject the packet. Because of that, the retransmitted packet can arrive earlier than expected: if the faulty echo ADD_ADDR injection takes X sec, the retransmission of the ADD_ADDR by the kernel will take (1 - X) sec. If X is bigger than the tolerance, the test fails which seems to happen regularly on the public CI.
We should then accept packets being sent less than one second before the injected and faulty echo ADD_ADDR.
Even worst with a debug kernel config:
Injecting the faulty echo and processing packets sent by the kernel can take time and we have situations where packets arrive a few seconds before the expected time by Packetdrill!
Sadly, we cannot tell Packetdrill the packet is expected to be sent in the past. So we need to increment the tolerance a bit. But that's find to do that because a new test has been added in the parent commit: it is focussing on the ADD_ADDR retransmissions without injecting other packets in between. This other test can have stricter expected time.
Closes: multipath-tcp/mptcp_net-next#312
Suggested-by: Paolo Abeni
Signed-off-by: Matthieu Baerts