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

Modify handling of dead hosts. #252

Merged
merged 8 commits into from
May 11, 2020
Merged

Conversation

jjnicola
Copy link
Member

@jjnicola jjnicola commented May 6, 2020

Depends on PR greenbone/ospd#266

A finished host could be alive or dead, when the classic alive test method is uses.
Depending on the host progress it will be set as dead or alive(finished).

When Boreas is used, do not send log and host details message of dead hosts to the client.

In any case, the dead host are not taken in account for the scan progress calculation

@jjnicola jjnicola requested a review from bjoernricks as a code owner May 6, 2020 11:15
@jjnicola jjnicola added the Work in progress Waiting for Feature Specification validation. label May 6, 2020
@codecov
Copy link

codecov bot commented May 6, 2020

Codecov Report

Merging #252 into master will increase coverage by 0.41%.
The diff coverage is 62.50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #252      +/-   ##
==========================================
+ Coverage   81.84%   82.26%   +0.41%     
==========================================
  Files           9        9              
  Lines        1449     1438      -11     
==========================================
- Hits         1186     1183       -3     
+ Misses        263      255       -8     
Impacted Files Coverage Δ
ospd_openvas/preferencehandler.py 86.76% <ø> (-0.29%) ⬇️
ospd_openvas/daemon.py 61.79% <62.50%> (+1.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d2f5c9...d6e857c. Read the comment docs.

Copy link
Contributor

@mattmundell mattmundell left a comment

Choose a reason for hiding this comment

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

The dead hosts part works, but the progress is stuck at 2%. gvmd uses the progress to see when the scan is done, so maybe don't merge yet.

@janowagner
Copy link
Member

I tried the patch and it accelerates my Host Discovery scan substantially.

…single host list.

This reduce the amount of memory used for storing the dead hosts and the processing time.
No messages HOST_START, dead host, HOST_END are sent to the client if the dead hosts are handle in batch.
This is the method use by Boreas method in OpenVAS.
…shed()

A finished host at this point could be alive or dead.
Depending on the host progress it will be set as dead or alive(finished).
Finished hosts are only for progress calculation.
Only the host in the exclude host list will not be scanned.
When resuming a stopped task, the client should sent in the exclude
host list, the list of host to exclude, and the list of finished host,
if they must not be scanned again.
@jjnicola jjnicola changed the title Do not send log and host details message of dead hosts to the client. Modify handling of dead hosts. May 8, 2020
@jjnicola jjnicola removed the Work in progress Waiting for Feature Specification validation. label May 11, 2020
Copy link
Member

@bjoernricks bjoernricks left a comment

Choose a reason for hiding this comment

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

Again only a small change should be done :-)

ospd_openvas/daemon.py Outdated Show resolved Hide resolved
Also catch TypeError when cast the progress value to float.
@jjnicola jjnicola requested a review from bjoernricks May 11, 2020 13:40
@bjoernricks bjoernricks merged commit d8ae65c into greenbone:master May 11, 2020
@jjnicola jjnicola deleted the dead-hosts branch May 11, 2020 13:41
ArnoStiefvater pushed a commit to ArnoStiefvater/ospd-openvas that referenced this pull request Oct 25, 2021
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.

None yet

4 participants