Make the recorders work again - #18
Open
stupalov wants to merge 1 commit into
Open
Conversation
Both recording commands are dead on pysnmp 7, and both fail quietly rather than with an error. snmpsim-record-commands sends its first GETNEXT, writes nothing and hangs forever. It reads the response as the old table of rows, which pysnmp now delivers as a flat list of var-binds; and pysnmp no longer continues a walk by itself, nor does the asyncio dispatcher return from run_dispatcher() when the last job is done. So the walk is now driven from the command: one place issues each request, and the run ends by stopping the dispatcher once the agent is walked, the stop OID is reached, or an error is final. Two side effects of it never having run: the IPv6 transport class no longer exists under the name it used, and the summary counted every OID twice. It also needs an event loop of its own, since Python no longer hands out one which was never created. snmpsim-record-traffic cannot even start: it wants pylibpcap 0.6.4, which has not been installable for years, and behind that it parses packet headers with Python 2 byte semantics, overwrites its own argument namespace with a packet tuple mid-loop, and cannot create its output directory. Capture files are now read directly - the format is a global header and a length in front of every packet, which needs no extension module - and pylibpcap is required only for live capture off an interface. Also fix --debug in the recorder, which died on a pysnmp function renamed years ago. Tests record a live agent through both GETNEXT and GETBULK, and turn a capture file built in the test into a recording.
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.
snmpsim-record-commands
It sends its first GETNEXT, writes an empty file and then hangs forever — no error, no traceback.
Three things have to change for a walk to complete:
for row in table: for oid, value in row:unpacks an OID into two names and the record is never written.NextCommandGenerator.process_response_varbinds()now ends right after calling the callback — there is no follow-up request. This is deliberate on the pysnmp side: asked about it in lextudio/pysnmp#251, the maintainer confirmed "This change is intentional." So the recorder has to issue each step itself; the three copies of the "send a request" block that were already in the file are replaced by one place that does it.AsyncioDispatcher.run_dispatcher()runs its loop until the loop is stopped; it does not return when the last job finishes. So the command stops the dispatcher itself once the agent is walked, the stop OID is reached, or an error is final.Two more things surfaced once it ran at all: the IPv6 transport class no longer exists under the name used (
udp6.Udp6SocketTransport), and the summary line counted every OID twice. It also needs an event loop of its own — pysnmp builds transports around the loop of the calling thread, and Python no longer hands out one that was never created.snmpsim-record-traffic
This one cannot start at all: it requires pylibpcap 0.6.4, a SourceForge release from 2011 which does not build on any current Python. Behind that check sit three more breakages — packet headers parsed with Python 2 byte semantics (
ord()overbytes), the argument namespace overwritten by a packet tuple inside the read loop (args = pcap_obj.next()), and an output directory that is never created becauseos.mkdir()cannot create a missing parent.A capture file is a global header followed by a length in front of every packet, so reading one needs no extension module:
CaptureFiledoes it in 40 lines, handling both byte orders and both timestamp resolutions, and pointing ateditcapwhen handed a pcapng file. pylibpcap is now needed only for live capture off an interface, which needs root anyway —tcpdump -wplus--capture-fileis the usual path and now works out of the box.Tests
tests/test_cmd2rec.pyrecords a live agent through both GETNEXT and GETBULK and compares the result with what the agent serves.tests/test_pcap2rec.pybuilds a capture file in the test out of real SNMP responses and checks the recording produced from it. The stub intests/test_issue_224.pyis updated to the attribute name pysnmp 7 uses.