[asterisk-dev] [Code Review]: Add the ability to capture and analyze packets

Terry Wilson reviewboard at asterisk.org
Thu Dec 15 12:24:39 CST 2011 


> On Dec. 15, 2011, 10:28 a.m., Matt Jordan wrote:
> > /asterisk/trunk/lib/python/pcap_listener.py, line 4
> > <https://reviewboard.asterisk.org/r/1623/diff/1/?file=22223#file22223line4>
> >
> >     Since these classes are in the /lib/ directory (and are going to be used by multiple people), commenting them would be nice

I'll add some comments. People *shouldn't* need to know anything (or much, anyway) about the pcap_listener. Its only purpose is to integrate with Twisted and throw packets at the callback. Hopefully we can hide pretty much all of it from anyone writing tests.


> On Dec. 15, 2011, 10:28 a.m., Matt Jordan wrote:
> > /asterisk/trunk/lib/python/sip_message.py, line 55
> > <https://reviewboard.asterisk.org/r/1623/diff/1/?file=22224#file22224line55>
> >
> >     Would it make sense having this inherit from unittest.TestCase?

Maybe? Probably? I'll have to actually read up on unittest now. :-)


> On Dec. 15, 2011, 10:28 a.m., Matt Jordan wrote:
> > /asterisk/trunk/lib/python/sip_message.py, lines 64-71
> > <https://reviewboard.asterisk.org/r/1623/diff/1/?file=22224#file22224line64>
> >
> >     Red blobs and such

fixed


> On Dec. 15, 2011, 10:28 a.m., Matt Jordan wrote:
> > /asterisk/trunk/tests/channels/SIP/pcap_demo/test-config.yaml, line 9
> > <https://reviewboard.asterisk.org/r/1623/diff/1/?file=22228#file22228line9>
> >
> >     The problem with having yappcap be a dependency here is that if a build agent doesn't have it, the test flat out won't run - which in reality, we really want it as a feature that only enables if the library is present.
> >     
> >     What do you think about putting a detection for the library into PcapListener?  If the library isn't available, it can either throw an exception (handled by TestCase) or just log the fact that the library isn't installed and not attempt to listen for packets.  That way we won't disable any tests by default, and we can have the pcap capture capability put in automatically across a large number of tests.

> The problem with having yappcap be a dependency here is that if a build agent doesn't have it, the test flat out won't run - which in reality, we really want it as a feature that only enables if the library is present.

Well, that depends. There are two cases. 1) Using pcap for keeping a record for later viewing and 2) using pcap functionality for the test itself. In case 1, yes, we only want to enable it when possible. In case 2, the test depends upon the functionality being available and should not run otherwise. The test requires there be a Python object (yappcap.Pcap, in this case) that has a "datalink" property so it can ensure that we are decoding an ethernet packet, and another Python object (yappcap.PcapPcaket in this case) that has a "data" property that contains the raw packet data. What I could do is move this stuff further back up into the pcap_listener stuff so that we only pass application-layer data for testing (since I doubt we really are going to be running tests on whether we are getting Ethernet or Token Ring packets or not) but TCP streams, IP fragmentation, and packet dissection are problems...but, if we just say that tests are only supported for non-fragmented (and the packets would have to be huge to be fragmented over the loopback interface) UDP packets all is well with that. :-)

> What do you think about putting a detection for the library into PcapListener?  If the library isn't available, it can either throw an exception (handled by TestCase) or just log the fact that the library isn't installed and not attempt to listen for packets.  That way we won't disable any tests by default, and we can have the pcap capture capability put in automatically across a large number of tests.

I think I should add the detection, and throw an exception which TestCase can use to set a flag AND make that flag available as a dependency in case tests rely on being able to test pcap packets so that we don't run them if functionality isn't available. If we are going to try to turn capturing on by default as basically an additional logger, I'll have to change things around a little bit so that it doesn't interfere with people actually writing tests based on the pcap stuff (like not giving the test itself the option to create the savefile--except by accessing yappcap directly and creating their own PcapDumper().


> On Dec. 15, 2011, 10:28 a.m., Matt Jordan wrote:
> > /asterisk/trunk/lib/python/asterisk/TestCase.py, line 276
> > <https://reviewboard.asterisk.org/r/1623/diff/1/?file=22222#file22222line276>
> >
> >     This probably should be a .debug, so as not to spam the messages.txt file (which generally is set to INFO and higher)
> >     
> >     Also: can packet have its __str__ method implemented?  That way you could send out to debug a brief textual representation of the packet, which I imagine would be pretty useful in a debug log

I'll switch to debug. I can implement a __str__ that includes things like timestamp and capture length. Including the data would require knowing lots of things about the packet which we don't at this point.


- Terry


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/1623/#review5013
-----------------------------------------------------------


On Dec. 14, 2011, 10:30 p.m., Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1623/
> -----------------------------------------------------------
> 
> (Updated Dec. 14, 2011, 10:30 p.m.)
> 
> 
> Review request for Asterisk Developers and Paul Belanger.
> 
> 
> Summary
> -------
> 
> This patch allows one to capture live network traffic, write tests based on that traffic, and to save capture files for later use. Currently the savefile directory must be specified by the test, so it might be nice to add the ability to easily record he pcap file somewhere where runtests.py will archive it.
> 
> There is also a pseudo SIP message parser for breaking generally well-formed SIP messages into the request/response line, headers, and body to facilitate writing SIP-based pcap tests. A cheesy demo test is included as a proof of concept.
> 
> The pcap functionality relies on the yappcap library (which I also wrote) which is available at https://github.com/otherwiseguy/yappcap . The library is still under active development and I desperately need to document it, but the API in use by the tests should not change. Other python pcap libraries were missing one or more features I needed/wanted.
> 
> The pcap demo test uses the Construct python library to navigate to the application-layer data in the raw packet because I didn't want to bother writing code to do that myself.
> 
> I've been staring at and obsessing over this code (and yappcap) too long. It is time for some other eyes.
> 
> 
> Diffs
> -----
> 
>   /asterisk/trunk/lib/python/asterisk/TestCase.py 2892 
>   /asterisk/trunk/lib/python/pcap_listener.py PRE-CREATION 
>   /asterisk/trunk/lib/python/sip_message.py PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/pcap_demo/configs/ast1/extensions.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/pcap_demo/configs/ast1/sip.conf PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/pcap_demo/run-test PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/pcap_demo/test-config.yaml PRE-CREATION 
>   /asterisk/trunk/tests/channels/SIP/tests.yaml 2892 
> 
> Diff: https://reviewboard.asterisk.org/r/1623/diff
> 
> 
> Testing
> -------
> 
> I ran the included test and opened the saved capture file to make sure it looked right.
> 
> 
> Thanks,
> 
> Terry
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20111215/4e2b1809/attachment-0001.htm>

More information about the asterisk-dev mailing list