-
Notifications
You must be signed in to change notification settings - Fork 30
Remove INET_ADDRSTRLEN limit for symbolic hostnames and optimize socket connections.
#578
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
7253f5f
53aee73
c0d284c
a930ffd
fa343e0
bb1ae56
3726122
f6b6b8a
bb0d53d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -133,6 +133,9 @@ typedef struct socket_connection_params_t { | |
|
|
||
| /** @brief Hostname of the remote server. */ | ||
| const char* server_hostname; | ||
|
|
||
| /** @brief IP address of the remote server. If provided, bypasses DNS resolution. */ | ||
| struct in_addr* server_ip_addr; | ||
| } socket_connection_params_t; | ||
|
Comment on lines
134
to
139
|
||
|
|
||
| /** | ||
|
|
@@ -149,8 +152,6 @@ typedef struct socket_priv_t { | |
| uint16_t port; | ||
| /** @brief The desired port specified by the user on the command line. */ | ||
| uint16_t user_specified_port; | ||
| /** @brief Human-readable IP address of the federate's socket server. */ | ||
| char server_hostname[INET_ADDRSTRLEN]; | ||
| /** @brief Port number of the socket server of the federate. The port number will be -1 if there is no server or if | ||
| * the RTI has not been informed of the port number. */ | ||
| int32_t server_port; | ||
|
|
@@ -215,10 +216,11 @@ int accept_socket(int socket); | |
| * | ||
| * @param sock The socket file descriptor that has already been created (using `socket()`). | ||
| * @param hostname The hostname or IP address of the server to connect to. | ||
| * @param ip_addr The IPv4 address to connect to. If non-NULL, bypasses DNS lookup of hostname. | ||
| * @param port The port number to connect to. If 0 is specified, a default port range will be used. | ||
| * @return 0 on success, -1 on failure, and `errno` is set to indicate the specific error. | ||
| */ | ||
| int connect_to_socket(int sock, const char* hostname, int port); | ||
| int connect_to_socket(int sock, const char* hostname, struct in_addr* ip_addr, int port); | ||
|
|
||
| /** | ||
| * @brief Read the specified number of bytes from the specified socket into the specified buffer. | ||
|
|
@@ -296,7 +298,7 @@ bool is_socket_open(int socket); | |
| * @ingroup Network | ||
| * | ||
| * Get the connected peer name from the connected socket. | ||
| * Set it to the server_ip_addr. Also, set server_hostname if LOG_LEVEL is higher than LOG_LEVEL_DEBUG. | ||
| * Set it to the server_ip_addr. Also, print server's hostname if LOG_LEVEL is higher than LOG_LEVEL_DEBUG. | ||
| * | ||
| * @param priv The socket_priv struct. | ||
| * @return 0 for success, -1 for failure. | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -177,14 +177,10 @@ int get_peer_address(socket_priv_t* priv) { | |||||||||||||||||||||||||||||||||||
| priv->server_ip_addr = peer_addr.sin_addr; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| #if LOG_LEVEL >= LOG_LEVEL_DEBUG | ||||||||||||||||||||||||||||||||||||
| // Create the human readable format and copy that into | ||||||||||||||||||||||||||||||||||||
| // the .server_hostname field of the federate. | ||||||||||||||||||||||||||||||||||||
| // Create the human readable format for logging purposes | ||||||||||||||||||||||||||||||||||||
| char str[INET_ADDRSTRLEN + 1]; | ||||||||||||||||||||||||||||||||||||
| inet_ntop(AF_INET, &priv->server_ip_addr, str, INET_ADDRSTRLEN); | ||||||||||||||||||||||||||||||||||||
| strncpy(priv->server_hostname, str, INET_ADDRSTRLEN - 1); // Copy up to INET_ADDRSTRLEN - 1 characters | ||||||||||||||||||||||||||||||||||||
| priv->server_hostname[INET_ADDRSTRLEN - 1] = '\0'; // Null-terminate explicitly | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| LF_PRINT_DEBUG("Got address %s", priv->server_hostname); | ||||||||||||||||||||||||||||||||||||
| LF_PRINT_DEBUG("Got address %s", str); | ||||||||||||||||||||||||||||||||||||
| #endif | ||||||||||||||||||||||||||||||||||||
| return 0; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
@@ -218,20 +214,28 @@ int accept_socket(int socket) { | |||||||||||||||||||||||||||||||||||
| return socket_id; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| int connect_to_socket(int sock, const char* hostname, int port) { | ||||||||||||||||||||||||||||||||||||
| int connect_to_socket(int sock, const char* hostname, struct in_addr* ip_addr, int port) { | ||||||||||||||||||||||||||||||||||||
| struct addrinfo hints; | ||||||||||||||||||||||||||||||||||||
| struct addrinfo* result; | ||||||||||||||||||||||||||||||||||||
| struct addrinfo* result = NULL; | ||||||||||||||||||||||||||||||||||||
| int ret = -1; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| memset(&hints, 0, sizeof(hints)); | ||||||||||||||||||||||||||||||||||||
| hints.ai_family = AF_INET; /* Allow IPv4 */ | ||||||||||||||||||||||||||||||||||||
| hints.ai_socktype = SOCK_STREAM; /* Stream socket */ | ||||||||||||||||||||||||||||||||||||
| hints.ai_protocol = IPPROTO_TCP; /* TCP protocol */ | ||||||||||||||||||||||||||||||||||||
| hints.ai_addr = NULL; | ||||||||||||||||||||||||||||||||||||
| hints.ai_next = NULL; | ||||||||||||||||||||||||||||||||||||
| hints.ai_flags = AI_NUMERICSERV; /* Allow only numeric port numbers */ | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| uint16_t used_port = (port == 0) ? DEFAULT_PORT : (uint16_t)port; | ||||||||||||||||||||||||||||||||||||
| struct sockaddr_in direct_addr; | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| if (ip_addr == NULL) { | ||||||||||||||||||||||||||||||||||||
| memset(&hints, 0, sizeof(hints)); | ||||||||||||||||||||||||||||||||||||
| hints.ai_family = AF_INET; /* Allow IPv4 */ | ||||||||||||||||||||||||||||||||||||
| hints.ai_socktype = SOCK_STREAM; /* Stream socket */ | ||||||||||||||||||||||||||||||||||||
| hints.ai_protocol = IPPROTO_TCP; /* TCP protocol */ | ||||||||||||||||||||||||||||||||||||
| hints.ai_addr = NULL; | ||||||||||||||||||||||||||||||||||||
| hints.ai_next = NULL; | ||||||||||||||||||||||||||||||||||||
| hints.ai_flags = AI_NUMERICSERV; /* Allow only numeric port numbers */ | ||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||
| memset(&direct_addr, 0, sizeof(direct_addr)); | ||||||||||||||||||||||||||||||||||||
| direct_addr.sin_family = AF_INET; | ||||||||||||||||||||||||||||||||||||
| direct_addr.sin_port = htons(used_port); | ||||||||||||||||||||||||||||||||||||
| direct_addr.sin_addr = *ip_addr; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| instant_t start_connect = lf_time_physical(); | ||||||||||||||||||||||||||||||||||||
| // while (!_lf_termination_executed) { // Not working... | ||||||||||||||||||||||||||||||||||||
|
|
@@ -240,30 +244,43 @@ int connect_to_socket(int sock, const char* hostname, int port) { | |||||||||||||||||||||||||||||||||||
| lf_print_error("Failed to connect with timeout: " PRINTF_TIME ". Giving up.", CONNECT_TIMEOUT); | ||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||
| // Convert port number to string. | ||||||||||||||||||||||||||||||||||||
| char str[6]; | ||||||||||||||||||||||||||||||||||||
| snprintf(str, sizeof(str), "%u", used_port); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Get address structure matching hostname and hints criteria, and | ||||||||||||||||||||||||||||||||||||
| // set port to the port number provided in str. There should only | ||||||||||||||||||||||||||||||||||||
| // ever be one matching address structure, and we connect to that. | ||||||||||||||||||||||||||||||||||||
| if (getaddrinfo(hostname, (const char*)&str, &hints, &result)) { | ||||||||||||||||||||||||||||||||||||
| lf_print_error("No host matching given hostname: %s", hostname); | ||||||||||||||||||||||||||||||||||||
| break; | ||||||||||||||||||||||||||||||||||||
| if (ip_addr != NULL) { | ||||||||||||||||||||||||||||||||||||
| // Safe to type cast specific protocols (e.g., sockaddr_in) to the generic sockaddr. | ||||||||||||||||||||||||||||||||||||
| ret = connect(sock, (struct sockaddr*)&direct_addr, sizeof(direct_addr)); | ||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||
| // Convert port number to string. | ||||||||||||||||||||||||||||||||||||
| char str[6]; | ||||||||||||||||||||||||||||||||||||
| snprintf(str, sizeof(str), "%u", used_port); | ||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||
| // Get address structure matching hostname and hints criteria, and | ||||||||||||||||||||||||||||||||||||
| // set port to the port number provided in str. There should only | ||||||||||||||||||||||||||||||||||||
| // ever be one matching address structure, and we connect to that. | ||||||||||||||||||||||||||||||||||||
| if (getaddrinfo(hostname, (const char*)&str, &hints, &result)) { | ||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
| if (getaddrinfo(hostname, (const char*)&str, &hints, &result)) { | |
| if (getaddrinfo(hostname, str, &hints, &result)) { |
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.
Fixed.
Copilot
AI
Apr 13, 2026
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.
connect_to_socket() logs "Connected to ..." unconditionally, even when ret remains < 0 due to timeout or repeated failures. This can mislead operators and test logs. Consider only printing the success message when ret == 0, and print a failure/timeout message (or nothing) otherwise; also handle inet_ntop() failure before using host_str.
| if (ip_addr != NULL) { | |
| char host_str[INET_ADDRSTRLEN]; | |
| inet_ntop(AF_INET, ip_addr, host_str, INET_ADDRSTRLEN); | |
| lf_print_info("Connected to %s:%d.", host_str, used_port); | |
| } else { | |
| lf_print_info("Connected to %s:%d.", hostname, used_port); | |
| if (ret == 0) { | |
| if (ip_addr != NULL) { | |
| char host_str[INET_ADDRSTRLEN]; | |
| if (inet_ntop(AF_INET, ip_addr, host_str, INET_ADDRSTRLEN) != NULL) { | |
| lf_print_info("Connected to %s:%d.", host_str, used_port); | |
| } else { | |
| lf_print_info("Connected to port %d.", used_port); | |
| } | |
| } else { | |
| lf_print_info("Connected to %s:%d.", hostname, used_port); | |
| } |
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.
I don't think we need this? On line 267, which is not shown here,
if (ret < 0) {
lf_sleep(CONNECT_RETRY_INTERVAL);
lf_print_warning("Could not connect. Will try again every " PRINTF_TIME " nanoseconds. Connecting to port %d.\n",
CONNECT_RETRY_INTERVAL, used_port);
continue;
} else {
break;
}
}
If ret<0, it continues, so it wouldn't get to this part if ret<0.
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.
If
accept_net()returns NULL (e.g., because the server socket was closed andaccept()starts failing with EBADF),continuewill immediately retry in a tight loop, potentially spamming logs and burning CPU until_lf_termination_executedflips. Consider breaking/returning on NULL (as before), or adding an explicit backoff + exit condition based on errno/termination state.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.
I think it should still be
continue, however, as it says it will keep looping when there are spammingconnect()attempts.How about adding a short sleep like 100 ms? @edwardalee
Also besides of that after rti_failed(), I think it should be
return NULL;, because it shouldn't print the log that all remote federates are connected.