search

hgn / captcp

A open source program for TCP analysis of PCAP files
http://research.protocollabs.com/captcp/
GNU General Public License v3.0
113 stars 40 forks source link

Fix a couple of issues in throughput module #10

Closed ykasap closed 11 years ago

ykasap commented 11 years ago

I noticed some questionable behavior in throughput module and want to fix them.

  1. The data_len of the first packet belongs to the next time slot is added to the data of current time slot.
  2. The final summary includes user-supplied unit name, but the values are not converted.
  3. Graphs generated by different "-s" values are not directly comparable because the values are not the average per unit time.
hgn commented 11 years ago

Thank you Yoshiaki! I like the patch set, but patch 3 is incompatible with the current behavior. Think about actual user. Two ways: if we think that the current behavior is crap I take the patch. Another solution is to add a new option, e.g. --normalize or --per-second

What do you think?

ykasap commented 11 years ago

I have a mixed feeling about it.

Actually I'm familiar with per-second value and other software I know (MRTG, munin etc) all uses per-second value for throughput, so I was surprised when I realized the values from captcp wasn't normalized when I used -s option (and I misinterpreted the resulted graph for a while).

On the other hand, if a user wants per-minute value, he can use "-s 60" and see what he wants with the current behavior.

So maybe it is better to make it as an option. I prefer --per-second because --normalize seems a bit ambiguous, but not a strong preference.

hgn commented 11 years ago

Ok, than we take the later approach - make it explicit. Yoshiaki can you provide a patch for "--per-second" feature? Another idea on top of this: we drop a warning when this option is not activated (e.g. "warning: graph is scaled to byte/!second" (or something more understandable).

ykasap commented 11 years ago

I'll try to make a patch... Actually I'm a newbie of git/github, so I'm sorry if I break something (still googling how to revise my pull request). Do you think the following option and its description acceptable?

-p, --per-second      data is normalized as per-second average
hgn commented 11 years ago

Yes, the description is perfect. You can open a new pull request - just delete your last commit (via reset --hard, commit 01a2fc3), work out the new feature and re-submit.

ykasap commented 11 years ago

Hmm, it seems that this pull-request is also updated after I revised my topic branch and did "git push --force".

hgn commented 11 years ago

Thank you Yoshiaki!