Add packet timestamp support#2
Draft
trondn wants to merge 3 commits into
Draft
Conversation
jimwwalker
reviewed
Jun 8, 2026
jimwwalker
reviewed
Jun 8, 2026
jimwwalker
reviewed
Jun 8, 2026
jimwwalker
reviewed
Jun 8, 2026
jimwwalker
reviewed
Jun 8, 2026
jimwwalker
reviewed
Jun 8, 2026
jimwwalker
reviewed
Jun 8, 2026
jimwwalker
reviewed
Jun 8, 2026
jimwwalker
suggested changes
Jun 8, 2026
jimwwalker
left a comment
There was a problem hiding this comment.
still looking over this - can copilot do anything as well to help?
Author
|
I've done a code review with copilot, gemini and droid and looked at the results |
jimwwalker
suggested changes
Jun 9, 2026
There was a problem hiding this comment.
Pull request overview
This PR adds receive/packet timestamp plumbing to evbuffers and bufferevents (including OpenSSL bufferevents), and adjusts HTTP/SSL tests for OpenSSL 3.x shutdown behavior.
Changes:
- Introduce evbuffer timestamp storage/retrieval APIs and timestamp-aware read path (
evbuffer_read_with_timestamp,evbuffer_get_timestamp). - Add
BEV_OPT_RECV_TIMESTAMPSand enable timestamp capture in socket and OpenSSL bufferevents. - Extend/adjust regression tests (buffer/bufferevent/SSL/HTTP) to exercise timestamp paths and tolerate OpenSSL 3.x shutdown semantics.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
test/regress_ssl.c |
Adds OpenSSL bufferevent timestamp tests (direct + filter). |
test/regress_http.c |
Adjusts HTTPS test bufferevent shutdown behavior and incomplete-request error handling for OpenSSL 3.x. |
test/regress_bufferevent.c |
Adds a socket bufferevent receive-timestamp regression test. |
test/regress_buffer.c |
Adds new evbuffer pullup regression and timestamp read/get regression. |
include/event2/bufferevent.h |
Adds BEV_OPT_RECV_TIMESTAMPS option and related documentation. |
include/event2/buffer.h |
Declares new timestamp-aware evbuffer APIs and documents timestamp semantics. |
event-config.h.cmake |
Adds CMake-configurable defines for SO_TIMESTAMP/SO_TIMESTAMPNS declarations. |
evbuffer-internal.h |
Extends evbuffer_chain with timestamp metadata fields. |
configure.ac |
Adds autoconf checks for SO_TIMESTAMP/SO_TIMESTAMPNS declarations. |
CMakeLists.txt |
Adds CMake checks for SO_TIMESTAMP/SO_TIMESTAMPNS declarations. |
bufferevent-internal.h |
Adds internal flag for timestamp enablement and declares be_socket_enable_timestamps_(). |
bufferevent_sock.c |
Enables SO_TIMESTAMP* and uses evbuffer_read_with_timestamp() when requested. |
bufferevent_openssl.c |
Adds recvmsg-capable BIO for OpenSSL socket bufferevents and propagates timestamps into decrypted input. |
buffer.c |
Implements timestamp-aware commit/read/get logic and propagates timestamps across chain operations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Under OpenSSL 3.0+, unexpected socket closures before a clean SSL/TLS shutdown alert is exchanged are classified as protocol errors rather than clean EOFs. This behavior change causes various regression tests to fail. To resolve these compatibility issues: 1. bufferevent_openssl.c: Identify the SSL_R_UNEXPECTED_EOF_WHILE_READING protocol error and treat it as a dirty shutdown (clean TCP closure) to ensure backward compatibility. 2. test/regress_http.c (https_bev): Enable 'allow_dirty_shutdown = 1' on server bufferevents inside the mock HTTPS server to cleanly handle abrupt client socket closures. 3. test/regress_http.c (http_incomplete_errorcb): Recognize SSL protocol errors arising from raw socket shutdowns on OpenSSL 3.0 as successful terminations during the incomplete HTTP request test.
29108f4 to
3e19184
Compare
865b2f4 to
0b09f26
Compare
0781a12 to
e5278c5
Compare
Implement kernel-measured socket receive timestamps with nanosecond precision on supported platforms via the recvmsg() syscall. Timestamps are stored per-chain in the evbuffer. Infrastructure changes: - evbuffer-internal.h: Add timestamp storage and validity flag to evbuffer_chain structure. - evbuffer_read_with_timestamp(): New internal function for reading data via recvmsg(), parsing control messages for timestamps, and safeguarding against MSG_CTRUNC truncation. - bufferevent-internal.h: Add recv_timestamps_enabled flag to the bufferevent_private structure. - bufferevent_sock.c: Enable receive timestamps and swap standard reads with evbuffer_read_with_timestamp() when requested. - bufferevent_openssl.c: Add a custom socket BIO to extract kernel timestamps during direct socket reads, and propagate timestamps from underlying bufferevents in filtered TLS mode. - Build system: Add CMake and Autotools socket timestamp checks for SO_TIMESTAMP and SO_TIMESTAMPNS option availability. Public API additions: - BEV_OPT_RECV_TIMESTAMPS option flag for socket bufferevents. - evbuffer_get_timestamp() to fetch the receipt timestamp of the oldest data currently in the buffer. - evbuffer_commit_space_with_timespec() to commit reserved space and manually attach a timestamp to the committed chains. Platform support: - Linux 2.6.22+: Nanosecond (SO_TIMESTAMPNS) and microsecond (SO_TIMESTAMP) precision. - macOS 10.12+: Microsecond (SO_TIMESTAMP) precision. Testing: - Added unit tests for evbuffer receive timestamping. - Added direct socket and filtered OpenSSL bufferevent timestamp regress tests. - Verified all 344 unit tests pass successfully.
8c19909 to
5dabc21
Compare
Comment on lines
753
to
+758
| buf->last->off += vec[0].iov_len; | ||
| added = vec[0].iov_len; | ||
| if (ts && added && buf->last->timestamp.valid == 0) { | ||
| buf->last->timestamp.ts = *ts; | ||
| buf->last->timestamp.valid = 1; | ||
| } |
Comment on lines
785
to
+791
| for (i=0; i<n_vecs; ++i) { | ||
| (*chainp)->off += vec[i].iov_len; | ||
| added += vec[i].iov_len; | ||
| if (ts && vec[i].iov_len && (*chainp)->timestamp.valid == 0) { | ||
| (*chainp)->timestamp.ts = *ts; | ||
| (*chainp)->timestamp.valid = 1; | ||
| } |
Comment on lines
2517
to
2532
| if ((ev_ssize_t)space < remaining) { | ||
| (*chainp)->off += space; | ||
| remaining -= (int)space; | ||
| if (ts_found && (*chainp)->timestamp.valid == 0) { | ||
| (*chainp)->timestamp.ts = ts; | ||
| (*chainp)->timestamp.valid = 1; | ||
| } | ||
| } else { | ||
| (*chainp)->off += remaining; | ||
| if (ts_found && (*chainp)->timestamp.valid == 0) { | ||
| (*chainp)->timestamp.ts = ts; | ||
| (*chainp)->timestamp.valid = 1; | ||
| } | ||
| buf->last_with_datap = chainp; | ||
| break; | ||
| } |
Comment on lines
+371
to
+382
| data = BIO_get_data(b); | ||
| if (data) { | ||
| if (BIO_get_shutdown(b)) { | ||
| #ifdef _WIN32 | ||
| closesocket(data->fd); | ||
| #else | ||
| close(data->fd); | ||
| #endif | ||
| } | ||
| mm_free(data); | ||
| BIO_set_data(b, NULL); | ||
| } |
Comment on lines
+556
to
+565
| /* 10. Drain remaining 4 bytes of packet A. | ||
| * The buffer now contains only packet B's bytes. The timestamp should shift to packet B's. */ | ||
| tt_int_op(evbuffer_drain(buf, 4), ==, 0); | ||
| tt_int_op(evbuffer_get_timestamp(buf, &ts2), ==, 0); | ||
| /* Timestamps for B should be valid and greater than or equal to A */ | ||
| tt_assert(ts2.tv_sec >= ts.tv_sec); | ||
| if (ts2.tv_sec == ts.tv_sec) { | ||
| tt_assert(ts2.tv_nsec >= ts.tv_nsec); | ||
| } | ||
| TT_BLATHER(("Captured timestamp B: %lld.%09ld", (long long)ts2.tv_sec, (long)ts2.tv_nsec)); |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FIx the test suite to work with OpenSSL 3.x and add support for the packet timestamp