Remove bind-to-device support where unavailable #773
Conversation
On freeBSD-like network stacks, the SO_BINDTODEVICE socket option is not available. Since there is no suitable replacement for this socket option, we ignore the 'device' configuration option on systems missing the option and emit a warning instead. This makes vsomeip compatible with the new io-sock network stack on QNX 7.1+. Co-author: Daniel Freiermuth [email protected] @danielfr-haleytek
|
@EriComic does this mean that without the SO_BINDTODEVICE, you aren't able to bind and connect to another device? |
|
@duartenfonseca Specifically it is to bind to the interface instead of just binding to the IP like we normally do. After a lot of exhaustive research in trying to find a freeBSD alternative, we found that there sadly isn't a proper replacement for this. The functionality could be brought back but the implementation would be very use-case specific and would not belong here. It is worth noting is that QNX's port of vsomeip also deprecates the feature. So we moved on to evaluating if the functionality is truly needed. As mentioned, this functionality allows binding to the interface instead of just the IP. This allows for things like:
These use cases (and ones like it) are so specialized that traditional IP binding wouldn't work. Therefore, we decided to deprecate the feature and leave re-implementation of it as an option for any party that needs it. TL;DR No, this won't prevent binding and connecting to another device. The option for interface binding is removed which won't affect the more common IP binding. Interface binding is a very niche feature which we decided to drop due to it rarely being needed. |
| if (setsockopt(socket_->native_handle(), | ||
| SOL_SOCKET, SO_BINDTODEVICE, its_device.c_str(), static_cast<socklen_t>(its_device.size())) == -1) { | ||
| VSOMEIP_WARNING << "TCP Client: Could not bind to device \"" << its_device << "\""; | ||
| } | ||
| #else | ||
| VSOMEIP_WARNING << "TCP Client: Could not bind to device \"" << its_device << "\"" |
There was a problem hiding this comment.
All the changes seem ok, but why not use the normal logging? VSOMEIP_WARNING << "tcp_client_endpoint::connect:.." or VSOMEIP_WARNING <<"tcp_client_endpoint::" << func ""..
There was a problem hiding this comment.
I see your point and I had a few ideas on how to make this better as well, but I decided against it for the following reasons:
- The warning log that was already present in the code uses the same formatting I have currently, I believe it makes sense to adhere to what's already there. I believe best practice is to keep logging style/formatting changes in separate commits in order to maintain a "prettier git log".
- Having function signatures in logs is very useful for developers working on the library but it's too noisy for the user using the library. Your proposed log would make more sense for a DEBUG or TRACE level log. Perhaps in addition to the currently proposed WARNING log.
TL;DR I think your suggestion is good but only useful for the developer and not-so-much the user. If you would like the addition of this log as a DEBUG or TRACE, then please add it, but I believe our proposed WARNING should be there as it is.
On freeBSD-like network stacks, the SO_BINDTODEVICE socket option is not available. Since there is no suitable replacement for this socket option, we ignore the 'device' configuration option on systems missing the option and emit a warning instead.
This makes vsomeip compatible with the new io-sock network stack on QNX 7.1+.
Co-author: Daniel Freiermuth [email protected] @danielfr-haleytek