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

Multithread sniff #1259

Closed
wants to merge 4 commits into from
Closed

Multithread sniff #1259

wants to merge 4 commits into from

Conversation

gpotter2
Copy link
Member

@gpotter2 gpotter2 commented Mar 17, 2018

This PR was a failure. See #1999

What it does

Split the sniffing process into several threads:

  • first one doing pcap/libpcap/whatever calls, and storing data in a shared list
  • second one constantly dissecting any available packets

To make this work, it was needed to separate dissecting and recieving in the arch/ hooks: introducing recv_async (of course name may be changed), which only recieves the packet as raw bytes. It is called by the older recv() functions, which will keep the same behavior

This allows to split scapy's processing part from API calls. It free the buffer of the APIs into scapy's one.

Benchmark

  • Start 2 scapy instances

  • In the first one:

def stats():
    try:
        i = [0]
        t = time.time()
        on = [True]
        def _t(t=t):
            while on[0]:
                print(str(i[0]/(max(1, time.time()-t))) + " packets / sec")
                time.sleep(3)
        Thread(target=_t).start()
        def _prn(pkt):
            i[0] += 1
        sniff(prn=_prn)
    except KeyboardInterrupt:
        print("Goodbye")
    finally:
        on = [False]
  • In the second one
a = raw(Ether()/IP())
sendp(a, loop=1)
  • Finally, in the first instance, type stats()

Results

  • On secdev/master

no_mthread

  • This branch

mthread

Those results already are an average

Speed test Secdev/master This PR
Average: Recieved packets / sec 647 1820

Edit: this seems wrong, as we are not using multiprocessing but threading, so the measuring methods are slowed down too, making the hole thing clunky.. Redoing the tests in a proper environment (two separate shells) makes the results very different

Conclusion

This PR improves the recieved packet / second rate by 281%

TODO & discussion

  • Switch from threading to multiprocessing ? We need to check the performances of doing so.

@codecov-io
Copy link

codecov-io commented Mar 17, 2018

Codecov Report

Merging #1259 into master will increase coverage by 0.05%.
The diff coverage is 85.71%.

@@            Coverage Diff             @@
##           master    #1259      +/-   ##
==========================================
+ Coverage   85.34%   85.39%   +0.05%     
==========================================
  Files         177      175       -2     
  Lines       40850    40583     -267     
==========================================
- Hits        34862    34657     -205     
+ Misses       5988     5926      -62
Impacted Files Coverage Δ
scapy/supersocket.py 72.83% <ø> (+0.18%) ⬆️
scapy/contrib/cansocket_native.py 80.88% <100%> (+0.88%) ⬆️
scapy/sendrecv.py 81.62% <85.05%> (-1.52%) ⬇️
scapy/__init__.py 83.33% <0%> (-2.39%) ⬇️
scapy/layers/tls/crypto/pkcs1.py 78.67% <0%> (-1.48%) ⬇️
scapy/layers/zigbee.py 42.97% <0%> (-1.21%) ⬇️
scapy/layers/tls/record.py 91.22% <0%> (-0.88%) ⬇️
scapy/layers/l2.py 83.54% <0%> (-0.51%) ⬇️
scapy/layers/tls/handshake_sslv2.py 92.01% <0%> (-0.39%) ⬇️
... and 15 more

@@ -438,34 +438,26 @@ def close(self):
for i in self.iff:
set_promisc(self.ins, i, 0)
SuperSocket.close(self)
def recv(self, x=MTU):
def recv_async(self, x=MTU):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this name is really appropriate... The goal of this function is to receive without dissecting. Any ideas?

@guedou
Copy link
Member

guedou commented Mar 27, 2018 via email

@gpotter2 gpotter2 force-pushed the multithread-sniff branch 3 times, most recently from 4ac8e94 to c3a5d20 Compare March 27, 2018 19:22
@gpotter2 gpotter2 force-pushed the multithread-sniff branch 7 times, most recently from 14232cb to 0f755ae Compare May 7, 2018 19:55
@gpotter2 gpotter2 force-pushed the multithread-sniff branch 2 times, most recently from eb620d3 to 4be4d16 Compare June 11, 2018 10:08
@@ -17,7 +17,8 @@ fi
# Install wireshark data
if [ "$TRAVIS_OS_NAME" = "linux" ] && [ "$TRAVIS_SUDO" = "true" ]
then
sudo apt-get -qy install tshark
# The wireshark packets should be installed with tshark, but Travis sometimes ignores the dependencies
sudo apt-get -qy install libwireshark5 wireshark-common tshark
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See https://travis-ci.org/gpotter2/scapy/jobs/390697554

$ bash .travis/install.sh
Reading package lists...
Building dependency tree...
Reading state information...
Some packages could not be installed. This may mean that you have
requested an impossible situation or if you are using the unstable
distribution that some required packages have not yet been created
or been moved out of Incoming.
The following information may help to resolve the situation:
The following packages have unmet dependencies:
 tshark : Depends: libwireshark5 (>= 1.12.0~rc3) but it is not going to be installed
          Depends: wireshark-common (= 1.12.1+g01b65bf-4+deb8u11ubuntu0.14.04.1) but it is not going to be installed
E: Unable to correct problems, you have held broken packages.

@gpotter2 gpotter2 force-pushed the multithread-sniff branch 3 times, most recently from 2f375de to fefd299 Compare June 11, 2018 10:43
@gpotter2 gpotter2 changed the title [WIP] Multithread sniff Multithread sniff Jun 11, 2018
@gpotter2 gpotter2 requested a review from p-l- June 11, 2018 11:13
- Allow the recv() function to accept an argument from a previously received recv_raw()
- Remove duplicated code from pcapdnet.py
@gpotter2
Copy link
Member Author

gpotter2 commented Jul 7, 2018

I have doubts about the real performance improvement of this PR...

@gpotter2 gpotter2 closed this Jul 17, 2018
@gpotter2
Copy link
Member Author

gpotter2 commented Jul 17, 2018

I can't get this PR to go anywhere, and am only losing performances :/ Will leave it here in the meanwhile...

I do think that using multiprocessing, and keeping this PR will improve the performances, but It seems maybe too hard to implement (sharing objects with multiprocessing is a pain), so I don't think the work is worth it..

@gpotter2 gpotter2 removed the request for review from p-l- June 8, 2019 13:15
@gpotter2 gpotter2 deleted the multithread-sniff branch July 19, 2020 09:19
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.

3 participants