-
Notifications
You must be signed in to change notification settings - Fork 844
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
Set keepalives on rpcap's control socket #773
Conversation
Regardless of the other points, TCP keep-alives date back to at least 1989, and Linux isn't the only OS that implements them in present-day Internet. |
Yup, I only have a Linux for now, but I guess I could probably check the API for *BSD and Windows. I was more interested in getting your feedback on the basic idea than to implement something exhaustive if I must rewrite most of it because it doesn't fit libpcap's API. |
How do you think, where would a custom keep-alive interval be used: at the client side or the server side or both? |
I'm not part of the project anymore, but @fjolliton might want to hear about that. |
Rebased on the current master branch. Any objections to merging this change as it is? |
As it turns out, this code needs some amendments to build on OSes other than Linux. |
We no longer have |
SOL_TCP is a Linuxism; NetBSD and FreeBSD don't define it, although they define TCP_KEEPCNT, TCP_KEEPIDLE and TCP_KEEPINTVL. The matter is, even on Linux the man page says: "For example, to indicate that an option is to be interpreted by the TCP protocol, level should be set to the protocol number of TCP; see getprotoent(3)."
That seems to mean that |
Except Windows, this change now looks much better CI-wise. When the platform does not support setting keepalive, would it be more appropriate to have the error compile-time instead of run-time? |
APIs that affect contents of the private data pointed to by the You'll have to add a new Then add a new Then, add a
and set Then have add a
Then rename Finally, add a message string for That way, if somebody calls (Also, make |
Thank you for the detailed comments, clearly this change is far from being ready for merging. If nobody volunteers to amend it as described, I might try doing that later. |
Closes: the-tcpdump-group#773 Signed-off-by: Kevin Boulain <[email protected]> Signed-off-by: Gabriel Ganne <[email protected]>
@ether42, do you prefer to complete this change yourself or to let @GabrielGanne do it? |
As explained in #773 (comment) I'm not working on this project anymore so anyone can take over. |
Thank you for confirming, closing this pull request then. Please excuse us for not being able to work on it earlier. @GabrielGanne, please address Guy Harris feedback above in your version of these changes. |
Closes: the-tcpdump-group#773 Signed-off-by: Kevin Boulain <[email protected]> Signed-off-by: Gabriel Ganne <[email protected]>
Would you be interested in the following extension of libpcap's API? It builds up on #772 and closes #766 (but doesn't hard-code anything in libpcap and leave it to the users, which seems to be a much more sensible approach).
Since #772 now verifies the integrity of the TCP control connection, optionally allowing keep-alives on it will allow to more efficiently react on a broken network connectivity (the data being transported over UDP or TCP).
If you prefer this commit to follow #772, feel free to close this pull request (but #772 is useful on its own for UDP-based data transport when the network is regarded as perfect).
On Linux, it seems the first keep-alive is sent after 2 hours of idling and a broken connection will be detected 20 minutes later (from
man 7 tcp
), if and only ifSO_KEEPALIVE
is set, so a libpcap application retrieving packets from a remote rpcapd may be stuck for quite a long while.The usage is quite simple:
pcap_set_control_keepalive
(happy to get suggestions about a better name) needs to be called after a successfulpcap_open
with some values (for example, something more aggressive but still reasonable would bepcap_set_control_keepalive(handle, 1, 5, 15, 5)
which would wait 15 seconds of idle time before sending a keep-alive, dropping the connection after 5 missed keep-alives each one spaced by 5 seconds).The keep-alives settings probably only works on Linux.
If the idea is validated, I'll push another commit for the corresponding man pages.