Use bind() instead of setsockopt() for binding raw sockets on Linux
#1646
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #1646 +/- ##
=======================================
Coverage 83.16% 83.16%
=======================================
Files 277 277
Lines 48189 48191 +2
Branches 9731 9754 +23
=======================================
+ Hits 40074 40079 +5
+ Misses 7224 7222 -2
+ Partials 891 890 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
seladb
requested review from
clementperon,
tigercosmos,
Dimi1010 and
egecetin
Pcap++/src/RawSocketDevice.cpp
Outdated
| saddr.sll_protocol = htons(ETH_P_ALL); | ||
| saddr.sll_ifindex = if_nametoindex(ifaceName.c_str()); | ||
|
|
||
| if (bind(fd, reinterpret_cast<struct sockaddr*>(&saddr), sizeof(saddr)) < 0) |
There was a problem hiding this comment.
nit: Do we need the struct keyword here and on L508?
| @@ -456,7 +456,7 @@ namespace pcpp | |||
| int fd = socket(AF_PACKET, SOCK_RAW, htobe16(ETH_P_ALL)); | |||
| if (fd < 0) | |||
| { | |||
| PCPP_LOG_ERROR("Failed to create raw socket. Error code was " << errno); | |||
| PCPP_LOG_ERROR("Failed to create raw socket. Error code was " << strerror(errno)); | |||
There was a problem hiding this comment.
Not checked the full code so maybe not important but strerror is not thread safe
There was a problem hiding this comment.
I guess errno isn't thread safe either, so we should handle it differently if we want to be fully thread safe. In this case it might not be required beacuse it's the creation and binding of the socket...
There was a problem hiding this comment.
Actually errno is thread safe since it is thread local but there are other strerror functions in library so better to fix it all later
There was a problem hiding this comment.
yes, I think you're right. I'll open a separate PR for it
Fixes #1640 (implementing the solution proposed in the issue)