-
Notifications
You must be signed in to change notification settings - Fork 24
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Refactoring of socket related functions to a separate socket_common.c #505
Merged
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
Jakio815
changed the title
Draft:Refactor only comm type
Draft: Refactor socket related functions to a separate socket_common.c
Dec 17, 2024
Jakio815
force-pushed
the
refactor-only-comm-type
branch
from
December 21, 2024 01:09
a9291bd
to
7eb4d55
Compare
edwardalee
approved these changes
Dec 23, 2024
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.
This looks much better. I've added some minor suggestions, but otherwise, this is good to merge.
Co-authored-by: Edward A. Lee <eal@eecs.berkeley.edu>
hokeun
changed the title
Refactor socket related functions to a separate socket_common.c
Refactoring of socket related functions to a separate socket_common.c
Dec 23, 2024
edwardalee
approved these changes
Dec 27, 2024
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.
LGTM, except for a typo.
Co-authored-by: Edward A. Lee <eal@eecs.berkeley.edu>
# for free
to join this conversation on GitHub.
Already have an account?
# 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.
This PR should be merged with lf-lang/lingua-franca#2449.
Motivation
The original codebase had socket-related functionality directly embedded in various parts of the application. This tight coupling led to code duplication and hindered modularity. In this refactor:
Overview of Approach
Three major functions were introduced to handle the socket-related operations:
create_TCP_server()
accept_rti_socket() / accept_federate_socket()
connect_to_socket()
These functions are used both in the RTI (Real-Time Infrastructure) server and the federate components, enhancing reusability and simplifying the network-related logic in these modules.
Detailed Approach
New integrated function:
create_TCP_server()
create_UDP_server()
void create_TCP_server(uint16_t port, int* final_socket, uint16_t* final_port);
void create_UDP_server(uint16_t port, int* final_socket, uint16_t* final_port);
Arguments
port
: The port number to bind the server socket to.DEFAULT_PORT
.final_socket
: A pointer to an integer where the server socket file descriptor will be stored upon success.final_port
: A pointer to a uint16_t where the successfully bound port number will be stored.increment_port_on_retry
: Boolean to retry port increment.Return Values
PORT_BIND_RETRY_LIMIT
times. If all attempts fail, the function prints an error and exits.Function Description
This function creates a TCP/UDP server socket and binds it to a port, enabling it to listen for incoming connections. The process includes:
increment_port_on_retry
is true, it starts fromDEFAULT_PORT
(15045) and increments the port number up toMAX_NUM_PORT_ADDRESSES
times, cycling back to DEFAULT_PORT if necessary.increment_port_on_retry
is false, it means the federate is trying to bind a port assigned by the OS.PORT_BIND_RETRY_LIMIT
times with aPORT_BIND_RETRY_INTERVAL
delay between attempts when cycling ports.Where It’s Used
lf_create_server()
infederate.c
: Initializes a server socket for federates.start_rti_server()
inrti_remote.c
: Sets up the RTI server socket for listening. Bothcreate_TCP_server()
andcreate_UDP_server()
is called.New integrated function:
accept_rti_socket()
accept_federate_socket()
int accept_rti_socket(int socket);
int accept_federate_socket(int socket, int rti_socket);
Arguments
socket
: The server socket file descriptor that is already set up and listening for incoming connections.rti_socket
: The RTI socket descriptor is used to check if the RTI socket is still open/alive.Return Values
errno
.Function Description
This function blocks and waits for an incoming connection request on the given server socket:
accept()
to accept an incoming client connection.EAGAIN
,EWOULDBLOCK
, orEINTR
) occurs, the function retries.Where It’s Used
lf_connect_to_federates()
inrti_remote.c
: Waits for federates to connect to the RTI server.respond_to_erroneous_connections()
inrti_remote.c
: Accepts connections who are attempting to join the wrong federation.lf_handle_p2p_connections_from_federates()
infederate.c
: Accepts connections from federates that send messages directly to this federate (not through the RTI).New integrated function:
connect_to_socket()
int connect_to_socket(int sock, const char* hostname, int port)
Inputs
sock
: The socket file descriptor created usingsocket()
.hostname
: The target hostname or IP address of the server to connect to.port
: The port number to connect to. If 0 is specified, it is trying to connect to the RTI. The function iterates through a range of default ports starting atDEFAULT_PORT
.Outputs
errno
.Function Description
This function attempts to establish a TCP connection to the specified hostname and port:
getaddrinfo()
to resolve the hostname to a valid network address.DEFAULT_PORT
.CONNECT_TIMEOUT
is reached.CONNECT_RETRY_INTERVAL
.Where It’s Used
lf_connect_to_federate()
infederate.c
: Federates use this function to establish connections with other federates.lf_connect_to_rti()
infederate.c
: Federates use this function to establish connections with the RTI server.Minor Logic Changed
The original code for
lf_connect_to_rti()
andlf_connect_to_federate()
, there was a large while loop retrying the connect() and also sending theMSG_TYPE_FED_IDS
andMSG_TYPE_P2P_SENDING_FED_ID
messages respectively. Below is a simplified version.The refactored version has two while loops, one inside the
connect_to_socket()
function itself and the original while loop. Below is the new simplified version.Follow Up
This PR yet did not handle the
shutdown()
andclose()
functions. This will be done in the next PR as soon as possible.UPDATE: This is done in #506