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

Problem with UDP broadcast sockets #11

Closed
MarkRivers opened this issue Dec 8, 2015 · 4 comments
Closed

Problem with UDP broadcast sockets #11

MarkRivers opened this issue Dec 8, 2015 · 4 comments

Comments

@MarkRivers
Copy link
Member

There is a problem with UDP broadcast sockets created with drvAsynIPPortConfigure using the IP protocol string UDP*. Broadcast messages written to that socket work correctly. However, replies from clients to such broadcasts are rejected by the server that sent the broadcast. The problem is that the drvAsynIPPort calls connect(), send(), and recv() on such sockets. This is incorrect, it should not call connect(), and it should call sendto() and recvfrom() rather than send() and recv().

Pull request #10 fixes this problem by doing the following for all UDP ports, not just ports configured for broadcasts:

  • Does not call connect()
  • Calls sendto() rather than send()
  • Calls recvfrom() rather than recv()
@MarkRivers
Copy link
Member Author

Changed implementation. Now always call recvfrom() rather than recv(), no longer conditional.
Fixes for Windows, add source IP address to ASYN_TRACEIO_DRIVER output on read operation. This shows what IP address sent a UDP message.
Tagged R4-28beta2.

@tboegi
Copy link
Contributor

tboegi commented Dec 9, 2015

I think there are 2 minor comments:
a) In flushIt(), the sender address is ignored anyway, so recv() works fine.
b) In readIt(), the print may be wrong or confusing on a stream socket.
From nam recvfrom():
If address is not a null pointer and the socket is not connection-oriented, the source address of the message is filled
in. The address_len argument is a value-result argument, initialized to the size of the buffer associated with address,
and modified on return to indicate the actual size of the address stored there.

Solution: Separate stream from datagram.
In short:

https://github.com/tboegi/asyn/tree/151210_2116_udp_fixes

@MarkRivers
Copy link
Member Author

I agree with your comments. Eric wanted to use recvfrom everywhere if we used it at all, and not have the conditional (which I originally had). But since the debug info could be confusing for TCP sockets I have re-implemented the conditional as you suggested. I also changed flushIt back to recv(). I have committed the changes.

@MarkRivers
Copy link
Member Author

Fixed in R4-28.

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

No branches or pull requests

2 participants