Implement missing parts for Windows. Change SO_REUSEPORT to SO_REUSEA…

oatpp / oatpp-ssdp

Oat++ extension module to work with SSDP protocol.

https://oatpp.io/

Apache License 2.0

2 stars 2 forks source link

Implement missing parts for Windows. Change SO_REUSEPORT to SO_REUSEA… #3

Closed beckdave closed 3 years ago

beckdave commented 3 years ago

…DDR, because SO_REUSEPORT is not available for Windows. I hope, it is the same behavior.

lganzzzo commented 3 years ago

Hey @beckdave ,

This looks great. Lets have Benedikt's input on this.

@bamkrs

beckdave commented 3 years ago

Thank you for your answers. I was looking for some information about SO_REUSEPORT. I found this text on https://stackoverflow.com/questions/14388706/how-do-so-reuseaddr-and-so-reuseport-differ

"The meaning of SO_REUSEADDR changes for multicast addresses as it allows multiple sockets to be bound to exactly the same combination of source multicast address and port. In other words, for multicast addresses SO_REUSEADDR behaves exactly as SO_REUSEPORT for unicast addresses. Actually, the code treats SO_REUSEADDR and SO_REUSEPORT identically for multicast addresses, that means you could say that SO_REUSEADDR implies SO_REUSEPORT for all multicast addresses and the other way round."

For me, it sounds like changing SO_REUSEPORT to SO_REUSEADDR has no difference in case of multicast.

What do you think?

bamkrs commented 3 years ago

Its not about SO_REUSEPORT to SO_REUSEADDR. Its about adding SO_BROADCAST as another socketop for Windows. Albeit its only for sending, one may end up in the situation of sending broadcasts with this socket. And since we are aiming for true Windows-compatibility, this is a necessary socketop.

beckdave commented 3 years ago

Thank you for the Information. I will add it, only for Windows.

beckdave commented 3 years ago

Some Questions:

The SO_REUSEADDR socket option is already set in SimpleUdpStreamProvider. I think, it is not necessary to set it again in SimpleSsdpUdpStreamProvider.
Why is SO_BROADCAST needed for Windows, but not for Linux?

bamkrs commented 3 years ago

Why is SO_BROADCAST needed for Windows, but not for Linux?

Oh dang! TIL it's also a flag for Linux. I've always read it in company with windows and must have missed it in Linux. You are right, SO_BROADCAST should also be set for Linux!

beckdave commented 3 years ago

I pushed a commit:

I removed SO_REUSEADDR in SimpleSsdpUdpStreamProvider, because it is already set in SimpleUdpStreamProvider. I think SO_REUSEADDR shall even be set before bind(). But in SimpleSsdpUdpStreamProvider it is set after bind(). So, the right place to set SO_REUSEADDR is SimpleUdpStreamProvider.
I enabled SO_BROADCAST for all plattforms.

bamkrs commented 3 years ago

I totally agree, nice catch. Good work! The code was refactored and rewritten multiple times because @lganzzzo and I tried so many different things how to implement a non-stream based protocol in Oat++ which is entirely build upon streams. So its nice to have another pair of eyes looking through it and fix stuff we missed because we were obsessed with other details.

beckdave commented 3 years ago

Thank you for your comment. I appreciated to improve oatpp-ssdp a little bit. Thank you very much for oatpp, it is a great lib. If you are interested in my communication lib, just check it out: https://github.com/bexoft/finalmq.