Skip to content
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
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

davidBar-On
Copy link
Contributor

  • Version of iperf3 (or development branch, such as master or
    3.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:

  1. Number of retries. Suggested default is 3 retries. One retry is the same as not specifying this option.
  2. Timeout before retrying to connect. Suggested default is 10 seconds.

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):

  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 redundantUDP_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 the UDP_CONNECT_MSG.

  2. 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 is UDP_CONNECT_MSG, a UDP_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 with UDP_CONNECT_REPLY.

  3. 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 the prot_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.

src/iperf_server_api.c Outdated Show resolved Hide resolved
src/iperf_api.c Outdated Show resolved Hide resolved
src/iperf_api.h Outdated Show resolved Hide resolved
i_errno = IESTREAMCNCTSEND;
return -1;
}
if (repeat_flag) {
Copy link

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) {

/*
* iperf_udp_acceppt_all_streams_connected_msgs
*
* Accepts the "all UDP streams connected" msgs/replies.
Copy link

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.

*
* Accepts the "all UDP streams connected" msgs/replies.
* Return value: >0 sucess, 0 - no msg received.
*
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
*
*

davidBar-On and others added 3 commits August 13, 2024 19:07
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants