Skip to content

Fixing issue #215#449

Open
LocutusOfBorg wants to merge 1 commit into
Ettercap:masterfrom
LocutusOfBorg:fix-215
Open

Fixing issue #215#449
LocutusOfBorg wants to merge 1 commit into
Ettercap:masterfrom
LocutusOfBorg:fix-215

Conversation

@LocutusOfBorg

Copy link
Copy Markdown
Contributor

I don't know, does this code makes sense?

I would like to test it, but I don't know how to do it properly

Comment thread src/ec_decode.c Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Wouldn't this cause stats_half_start() and stats_half_end() to be called without processing any additional packets?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes, so how to solve this? Adding packets in a queue?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What do you think about calling stats_start when count is 0 and end when it has reached the packet limit?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done, I hope
but this solution needs indeed LOT of testing

@eaescob

eaescob commented Jan 8, 2014

Copy link
Copy Markdown
Contributor

I don't think travis actually failed, a build stalled.

@LocutusOfBorg

Copy link
Copy Markdown
Contributor Author

In the last few days travis has needed a lot of rebuilds, and is using two concurrent builds instead of 5... I think there is some overread in the system! Anyway, I restarted the build!

@koeppea

koeppea commented Mar 6, 2018

Copy link
Copy Markdown
Member

Rebased onto current master and renamed commit.
Need to test if it really has a positive effect when ettercap has to serve some bandwidth.

@koeppea

koeppea commented Apr 30, 2018

Copy link
Copy Markdown
Member

Rebased over latest master and resolved conflicts + two errors.
Testing now the impact on transfer performance...

@koeppea

koeppea commented May 19, 2018

Copy link
Copy Markdown
Member

Comparism test done:

1GByte via FTP from one VM(FreeBSD) to another VM(Debian 64-bit) on the same host
w/o Ettercap(IPv6):         duration: 00:50 (19.68 MiB/s)
w/o Ettercap(IPv4):         duration: 00:46 (21.63 MiB/s)
w/  Ettercap(master/IPv6):  duration: 04:45 ( 3.50 MiB/s)
w/  Ettercap(master/IPv4):  duration: 04:25 ( 3.76 MiB/s)
w/  Ettercap(fix-215/IPv6): duration: 04:31 ( 3.68 MiB/s)
w/  Ettercap(fix-215/IPv4): duration: 04:44 ( 3.52 MiB/s)

I also promised myself more out of it. However, since it's improving in both cases (IPv4 & IPv6) against master, I vote for merge in any case.

@koeppea

koeppea commented May 19, 2018

Copy link
Copy Markdown
Member

Sorry I'fuzzed up.
IPv4 even worse. Maybe random. I think the pulll request doesn't improve the performance of Ettercap, unfortunately.

@sgeto

sgeto commented May 21, 2018

Copy link
Copy Markdown
Contributor

IPv4 even worse.

Worse than before applying this PR?

@koeppea

koeppea commented May 25, 2018

Copy link
Copy Markdown
Member

Well I think this PR may have an effect on performance.
But at the moment the issue lies somewhere else. I'm afraid now it's time to profile Ettercap to find the real bottleneck. When this is found and potentially fixed, this PR could make a difference, then relativiately to the improved throughput.

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.

4 participants