-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Adding support for UDP ephemeral ports #3691
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
Conversation
|
|
||
| struct sockaddr_in sender_addr; | ||
| socklen_t sender_addr_len = sizeof(sender_addr); | ||
| I32 received = static_cast<I32>(::recvfrom(socketDescriptor.fd, data, size, SOCKET_IP_RECV_FLAGS, |
Check warning
Code scanning / CodeQL
Unchecked function argument Warning
|
|
||
| struct sockaddr_in sender_addr; | ||
| socklen_t sender_addr_len = sizeof(sender_addr); | ||
| I32 received = static_cast<I32>(::recvfrom(socketDescriptor.fd, data, size, SOCKET_IP_RECV_FLAGS, |
Check warning
Code scanning / CodeQL
Unchecked function argument Warning
|
|
||
| struct sockaddr_in sender_addr; | ||
| socklen_t sender_addr_len = sizeof(sender_addr); | ||
| I32 received = static_cast<I32>(::recvfrom(socketDescriptor.fd, data, size, SOCKET_IP_RECV_FLAGS, |
Check warning
Code scanning / CodeQL
Unchecked function argument Warning
LeStarch
left a comment
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.
Add update and store and forward
|
@zimri-leisher change to UDP, would you take a look? |
…ureSend so that it maintains the configured timeout_seconds and timeout_microseconds.
LeStarch
left a comment
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.
One minor question, but otherwise looks good.
|
Aside from the confusing internal state of this component, this PR looks good to me. My highest priority is that this not change any existing behavior, and it doesn't appear to. |
|
Thanks @zimri-leisher see #3716 for follow-up. |
Change Description
This change adds the ability to support ephemeral ports when using UdpSocket.
In the sender case, you could set up your receive port as 0 to be assigned an ephemeral port for responses.
In the receiver case, you will be able to reply to the port that is specified in the UDP datagram of the first received message.
Rationale
This improves F' compliance to the UDP specification and allows for the use of ephemeral ports.
Testing/Review Recommendations
Added test case to TestUdp:
TestEphemeralPorts - Set up a receiver specifying 0 for the port to be assigned an ephemeral one. Setup a sender that is full duplex (with an ephemeral receive port). Send a message from the sender and ensure it is received properly by the receiver. Send a response back from the receiver and ensure that it is received properly by the sender.
Future Work
In the current implementation it only supports responding to messages from a single sender. If we need to support being able to reply to multiple senders this would require a different design.