-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
UDP Connect retry mechanism #1402
Open
davidBar-On
wants to merge
5
commits into
esnet:master
Choose a base branch
from
davidBar-On:issue-1144-UDP-connect-retry-mechanisms
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
UDP Connect retry mechanism #1402
davidBar-On
wants to merge
5
commits into
esnet:master
from
davidBar-On:issue-1144-UDP-connect-retry-mechanisms
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
Luflosi
reviewed
Aug 7, 2024
Luflosi
reviewed
Aug 7, 2024
Luflosi
reviewed
Aug 7, 2024
Luflosi
reviewed
Aug 7, 2024
i_errno = IESTREAMCNCTSEND; | ||
return -1; | ||
} | ||
if (repeat_flag) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
if (repeat_flag) { | |
if (repeat_flag) { |
Luflosi
reviewed
Aug 7, 2024
/* | ||
* iperf_udp_acceppt_all_streams_connected_msgs | ||
* | ||
* Accepts the "all UDP streams connected" msgs/replies. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
* Accepts the "all UDP streams connected" msgs/replies. | |
* Accepts the "all UDP streams connected" msgs/replies. |
Luflosi
reviewed
Aug 7, 2024
* | ||
* Accepts the "all UDP streams connected" msgs/replies. | ||
* Return value: >0 sucess, 0 - no msg received. | ||
* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested change
* | |
* |
Co-authored-by: Luflosi <luflosi@luflosi.de>
Co-authored-by: Luflosi <luflosi@luflosi.de>
Co-authored-by: Luflosi <luflosi@luflosi.de>
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.
Version of iperf3 (or development branch, such as
master
or3.1-STABLE
) to which this pull request applies:master
Issues fixed (if any):
Enhancement for iperf_udp: implement UDP connect retry mechanism #1144
Brief description of code changes (suitable for use as a commit message):
Suggested enhancement to add retries to the UDP streams connection protocol - inspired by the enhancement suggested in PR #1144. When retries are used, the change should also solve issues such as out of order packets and make PR #1260 redundant.
A
--udp-retry[=#[/#]]
option is added to specify that a UDP connection should be retried when needed. Two optional arguments are allowed:When
--udp-retry
is not specified or--udp-retry=1
is specified, the functionality should be 100% compatible to the functionality before this change. Therefore, the risk of including this change in the mainline should be very low. This is important as my system does not have the timing issues and I only simulated some basic issues. Therefore, it is expected that there are scenarios that I didn't anticipate. In such cases, the user can just revert to the legacy default behavior.The suggested algorithm is based on the current algorithm where the connection related messages are sent on the UDP streams. An alternative could be to create a "TCP control stream" for sending the connection messages instead of adding the retries mechanism. However such approach requires another mechanism for passing the Stream IP parameters, so I decided not to try this approach.
The suggested retry algorithm is (when number of retries is greater then 1):
Per Stream on the Client side:
1.1 Send
UDP_CONNECT_MSG
to the server.1.2 Wait for
UDP_CONNECT_REPLY
with timeout.1.3 While waiting, if any control message is received on any of the previously connected Stream, it is discarded (expected to be redundant
UDP_CONNECT_REPLY
). This makes sure that redundant control messages retries will not be received later as test data (when-R
--bidir
are used).1.4 When receiving
UDP_CONNECT_REPLY
the Client repeats the process for the next Stream.1.5 If receiving
UDP_CONNECT_REPLY
times out, the Client retries sending theUDP_CONNECT_MSG
.Per Stream on the Server side:
2.1 Wait for
UDP_CONNECT_MSG
from the Client.3.2 While waiting, if any control message is received on any of the previously connected Stream, it is discarded to make sure that redundant control messages retries will not be received later as test data (when not in reverse mode or
--bidir
is used). However, if the discarded message isUDP_CONNECT_MSG
, aUDP_CONNECT_REPLY
is sent to the Client (on the proper Stream), as it may be that the Client did not receive the reply for the previous Stream so it retried the connection.2.3 When receiving
UDP_CONNECT_MSG
the the Server replies withUDP_CONNECT_REPLY
.When all streams were connected:
Note: this functionality was added so both Client and Server will be in sync that all Streams are established and to make sure (as much as possible) that no redundant data is still available in any of the Streams that will be used to send test data before any test data is sent.
3.1 When the Client detects that all Streams are successfully connected, it sends
UDP_ALL_STREAMS_CONNECTED_MSG
messages to the server on a new created socket (that is not used for the test data). The message is repeated "retry number" times. The Client then waits for up to "retry number"UDP_ALL_STREAMS_CONNECTED_REPLY
messages from the Server.3.2 When the Server detects that all Streams are successfully connected, it waits for up to "retry number"
UDP_ALL_STREAMS_CONNECTED_MSG
messages from theprot_listener
socket (note that this socket will not be not used for tests data).3.3 While the Server is waiting, if any control message is received on any of the previously connected Stream, it is handled the same as when waiting for
UDP_CONNECT_MSG
.3.4 When the Server receives all
UDP_ALL_STREAMS_CONNECTED_MSG
, or when receive times out: if at least one message was received, the server sends "retry number"UDP_ALL_STREAMS_CONNECTED_REPLY
messages and starts the test.3.5 When the Client receives all
UDP_ALL_STREAMS_CONNECTED_REPLY
, or when receive times out: if at least one message was received, the Client starts the test.