Skip to content

Use SWRX timestamps in server tests to match production - #532

Closed
leoleovich wants to merge 1 commit into
facebook:mainfrom
leoleovich:export-D107533486
Closed

leoleovich wants to merge 1 commit into
facebook:mainfrom
leoleovich:export-D107533486

Conversation

@leoleovich

Copy link
Copy Markdown
Contributor

Summary:
TestServer fails non-deterministically (and consistently on newer kernels,
e.g. Fedora rawhide where it breaks the RPM %check stage) with a UDP read
timeout at server_test.go:223.

Root cause: the tests configure the server with TimestampType: timestamp.SW,
a value production never uses. cmd/ntpresponder defaults to SWRX and
Config.Validate() only accepts SWRX/HWRX. SW routes through
EnableSWTimestamps, which additionally turns on TX software timestamping
(SOF_TIMESTAMPING_TX_SOFTWARE). Every response the worker sends then makes
the kernel clone the packet onto the socket error queue. The responder never
drains MSG_ERRQUEUE, so those clones accumulate; error-queue skbs are charged
to sk_rmem_alloc and bounded by SO_RCVBUF. Once that budget fills, the kernel
silently drops incoming requests, the listener never sees them, and the
client's synchronous read blocks until the 10s deadline. How quickly it fills
relative to the 1000-request loop depends on skb truesize / rmem accounting,
which is why it passes on some kernels and fails on others; the earlier 1MB
buffer bump only delays the fill.

Measured difference (1MB rcvbuf, 4000 sends, no draining):

Mode sk_rmem_alloc after sends
SWRX (production, RX only) 0 bytes
SW (test, TX+RX) 2,096,640 bytes (entire 2MB rcvbuf)

Switch TestServer and TestListener to timestamp.SWRX so the tests exercise
the real production code path and no longer accumulate TX timestamps.

Differential Revision: D107533486

Summary:
TestServer fails non-deterministically (and consistently on newer kernels,
e.g. Fedora rawhide where it breaks the RPM %check stage) with a UDP read
timeout at server_test.go:223.

Root cause: the tests configure the server with TimestampType: timestamp.SW,
a value production never uses. cmd/ntpresponder defaults to SWRX and
Config.Validate() only accepts SWRX/HWRX. SW routes through
EnableSWTimestamps, which additionally turns on TX software timestamping
(SOF_TIMESTAMPING_TX_SOFTWARE). Every response the worker sends then makes
the kernel clone the packet onto the socket error queue. The responder never
drains MSG_ERRQUEUE, so those clones accumulate; error-queue skbs are charged
to sk_rmem_alloc and bounded by SO_RCVBUF. Once that budget fills, the kernel
silently drops incoming requests, the listener never sees them, and the
client's synchronous read blocks until the 10s deadline. How quickly it fills
relative to the 1000-request loop depends on skb truesize / rmem accounting,
which is why it passes on some kernels and fails on others; the earlier 1MB
buffer bump only delays the fill.

Measured difference (1MB rcvbuf, 4000 sends, no draining):

| Mode | sk_rmem_alloc after sends |
|------|---------------------------|
| SWRX (production, RX only) | 0 bytes |
| SW (test, TX+RX)           | 2,096,640 bytes (entire 2MB rcvbuf) |

Switch TestServer and TestListener to timestamp.SWRX so the tests exercise
the real production code path and no longer accumulate TX timestamps.

Differential Revision: D107533486
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 4, 2026
@meta-codesync

meta-codesync Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

@leoleovich has exported this pull request. If you are a Meta employee, you can view the originating Diff in D107533486.

@meta-codesync

meta-codesync Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

This pull request has been merged in 67daa0f.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged meta-exported

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant