Skip to content
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

Allocate much larger buffer for event data, remove incorrect logic #17

Closed
wants to merge 1 commit into from

Conversation

stolowski
Copy link
Contributor

We have recently been alerted about an issue where go-udev returns the following error:
udev event error: Unable to parse uevent, err: no buffer space available (see https://bugs.launchpad.net/snapd/+bug/1833707).

The reference implementation of udev monitor coming from systemd uses a large buffer for events - see https://github.com/systemd/systemd/blob/master/src/udev/udevadm-monitor.c#L73 ; while go-udev implements logic for resizing the buffer, it is based on a wrong assumption regarding MSG_PEEK flag; the meaning of the flag is simply the following:

"This flag causes the receive operation to return data from the beginning of the receive queue without removing that data from the queue. Thus, a subsequent receive call will return the same data."

This PR pre-allocates the buffer based on reference implementation from systemd, and removes the resizing logic. Also, the allocation is moved to the Connect method so it's done only once.

I think that maybe MSG_PEEK should be OR'ed with MSG_TRUNC flag and then the resizing logic would work, but I'm not able to reproduce this error case, so I chose a conservative fix.

…e implementation from systemd. Remove resizing logic as it was based on wrong assumption regarding MSG_PEEK semantics.
@stolowski
Copy link
Contributor Author

Please hold on with reviewing this, the problem may be actually deeper and existing logic based on MSG_PEEK may be correct after all... Discussing this with my peers.

@stolowski
Copy link
Contributor Author

Closing, problem caused by low value of /proc/sys/net/core/rmem_default size. Sorry for the noise!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant