-
Notifications
You must be signed in to change notification settings - Fork 340
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
Consideration of initial Read() buffer size #45
Comments
I've done some more real world testing and experimenting. I'll put together a pull request to make a minor buffer size change building on @benjamin-thomas wonderful addition. In short, when we're walking (GetBulk) the pdu count is only 1, but the returned number of PDUs will be many more (MaxRepetitions). This leads to many retries. It's an easy fix. Also while playing I found a Windows specific bug. A read on a buffer that's too small results in an error rather than a partial buffer read. I'll fix this in the same pull request. |
OK. I've just filed a pull-request for @soniah to do a quick review (#46). Summary of findings:
|
Closed a while back in #46 |
Following on from comment here:
3e6fc99
Auto scaling the initial buffer size based on the oid count is clever (marshal.go => dispatch() ). It's a compromise between memory usage vs possible network re-requests. For typical numeric values the current defaults will be fine but it may cause issues with OctetStrings?
For reference, both snmp4j and net-snmp defaults to ~64k buffers (e.g. max datagram size). Max OctetString in the RFC is 65535
I have a dozen or so different snmp devices in here. I'll do some scanning and see what pops up.
The text was updated successfully, but these errors were encountered: