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

Adaptive padding prob distr tests #543

Closed

Conversation

asn-d6
Copy link
Member

@asn-d6 asn-d6 commented Nov 26, 2018

No description provided.

Mike Perry and others added 30 commits November 26, 2018 14:37
We need this for padding negotiation so that we can have later machine
revisions supercede earlier ones.
This is a good code review start point, to get an overview of the interfaces
and types used in circuit padding.
This helps us to determine if a middle node can pad to us or not.
These event callbacks allow circuit padding to decide when to attempt to
launch and negotiate new padding machines, and when to tear old ones down.
These callbacks allow the padding state machines to react to various types of
sent and received relay cells.
This implements all of the event handling, state machines, and padding
decisions for circuit padding.

I recommend reviewing this after you look at the call-in points into it from
the rest of Tor.
* Tighten and justify cutoff for approximation in GPD sampler.
* Use expm1(-xi log U) to evaluate U^{-xi} - 1 without precision loss.
* Use log1p(x) to evaluate log(1 + x) precisely even for x near 0.
* Avoid 0 by rejection sampling, not by choosing to return 0 instead.
  (This avoids the problematic 0 return from crypto_rand_double()
   into the log() without introducing distribution bias).
Hope is this will make it easier to test on the live tor network.

Does not need to be merged if we don't want to, but will come in handy
for researchers.
Note to self/others: don't merge this.
Rename all the variables.
Rename all the variables.
Rename all the variables.
Factor end state cleanup code into its own function.
Fix issues with bin<->usec conversion.
Better tests for bin conversion.
Padding support is for version >= 1.
Clean up limit checks; also avoid overflow.
Add helper macro for infinity bin.
Use helper macro for infinity bin.
Make RTT calc slightly better.
Use range_usec instead of range_sec.
Mike Perry and others added 10 commits November 26, 2018 14:37
Fix negotiation failure due to not checking return values.
Invert transition array.
Invert transition array.
Invert transition array.
Add the code as is with no modifications as of Riastradh's commit
b0f98cdb1d54c2b6b8f36cb183bbbc341a520f16 in
https://mumble.net/~campbell/git/tor-dists.git.
- Shadowing warnings
- Unused ok variable warnings
- Disable some printfs that appear when tests pass.
This test only checks the uniform distribution and the way it's used by the
circuitpadding subsystem. It doesn't test the other distributions.
@asn-d6 asn-d6 force-pushed the adaptive_padding-prob-distr-tests branch from 83f78f8 to ef86fbd Compare November 26, 2018 12:37
asn-d6 and others added 16 commits November 26, 2018 14:40
They were very slow (11s on my laptop) and also had 0.1% chance of failing.
There are various places where we legitimately test for equality:

- comparison with zero for tests that should return zero
- comparison with +/-infinity for tests that should return it
- testing for probability p = 1 when p is limited to [0,1]

There are also various places where we legitimately convert between
integer and floating-point types:

- choosing a geometric sample from ceil(log(...))
- choosing an integer bin from a continuous sample

This patch:

1. Avoids unnecessary equality like p != 0 when p > 0 would work.
2. Uses silly comparisons like p >= 1 where p == 1 is meant.
3. Marks the silly cases with a comment `XXX -Wfloat-equal'.
4. Inserts explicit casts for intended conversion between integer and
   floating-point types.

This patch does not address -Wbad-function-casts because it needlessly
clutters up the code: when I write (size_t)floor((x - lo)/w), it's
clear that I want the integer as an integer type, and clear _which_
integer I want.
Whitespace-only change: view with `git diff -b' to confirm.
The remainder are URLs that are too long to fit in 80 columns plus a
comment marker.
1. For p near 0 or 1/2, there's nothing wrong with the translated cdf,
   so enable these tests.

   (For small x, i.e. p near 1/2, there would be a problem with
   translation if we used something halfway between the cdf and the sf
   that gives \int_0^x p instead of the cdf \int_{-\infty}^x p or the
   sf \int_x^{+\infty} p.  But we're not testing that.)

2. Fix the tests where either logit or logithalf is bad, so that we
   test both if the input is the same and only the preciser one if
   translation by 1/2 rounds one or the other.

3. Test logistic(-x) = 1 - p even if p == 1, as long as x is infinite.
I have a hard time following boolean parameters like that!
(only in tests, not for user input)
No reason to have an XXX over every -Wfloat-equal warning.
@asn-d6 asn-d6 force-pushed the adaptive_padding-prob-distr-tests branch from ef86fbd to cbe0d0e Compare November 26, 2018 12:41
@coveralls
Copy link

coveralls commented Nov 26, 2018

Pull Request Test Coverage Report for Build 3103

  • 812 of 995 (81.61%) changed or added relevant lines in 13 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.3%) to 62.523%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/feature/nodelist/routerlist.c 1 2 50.0%
src/core/or/circuituse.c 3 5 60.0%
src/core/or/connection_edge.c 0 2 0.0%
src/lib/math/prob_distr.c 206 208 99.04%
src/core/or/relay.c 4 11 36.36%
src/core/or/circuitbuild.c 2 13 15.38%
src/core/or/circuitpadding.c 584 742 78.71%
Files with Coverage Reduction New Missed Lines %
src/core/or/circuitbuild.c 2 42.12%
Totals Coverage Status
Change from base Build 3082: 0.3%
Covered Lines: 45251
Relevant Lines: 72375

💛 - Coveralls

@asn-d6 asn-d6 closed this Nov 27, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants