Changes between Initial Version and Version 1 of Ticket #21425, comment 24


Ignore:
Timestamp:
Feb 17, 2018, 9:31:56 PM (15 months ago)
Author:
teor
Comment:

Legend:

Unmodified
Added
Removed
Modified
  • Ticket #21425, comment 24

    initial v1  
    11Replying to [comment:23 asn]:
    2 > Hello people,
    3 >
    4 > two comments on the patch so that I understand a bit better what's going on because it's been a while and I'm a bit confuse:
    5 >
    62> 1) What are we trying to achieve with this new `entry_list_is_constrained()` logic? Can someone explain to me why we consider the entry list constrained if it contains more than 50 guards, and why this logic makes sense given the way `entry_list_is_constrained()` is used in the rest of the code? I think this is a very important question and ideally we should have a clear and concise answer :)
    73
     
    128Neel, please explain why we chose 50 in the comments in the patch.
    139
    14 Replying to [ticket:21425 nickm]:
    15 > We use entry_list_is_constrained() in a few places to find out if our list of entry points is highly limited (e.g., to a few bridges or a few EntryNodes).  But it doesn't do that very well:  instead, it looks to see if EntryNodes is set or UseBridges is set.
    16 >
    17 > We have better ways: we should be looking at the size of the guard sample, or something.
     10Here's the background:
    1811
    19 > > 2. If #1 is okay, which capacity size should {{{sampled_entry_guards}}} be to return 1? Right now, I am guessing 3 guards. Should it be more? Less?
    20 >
    21 > If we want to consider Tor Browser bridge users constrained, the answer is around 10 or 20.
    22 > If not, the answer is around 3.
     12> Replying to [ticket:21425 nickm]:
     13> > We use entry_list_is_constrained() in a few places to find out if our list of entry points is highly limited (e.g., to a few bridges or a few EntryNodes).  But it doesn't do that very well:  instead, it looks to see if EntryNodes is set or UseBridges is set.
     14> >
     15> > We have better ways: we should be looking at the size of the guard sample, or something.
    2316
    24 Replying to [comment:16 teor]:
    25 > There are 27 obfs4 bridges in Tor Browser at the moment:
    26 > https://gitweb.torproject.org/builders/tor-browser-build.git/tree/projects/tor-browser/Bundle-Data/PTConfigs/bridge_prefs.js
    27 > The other bridge types have 1-5 bridges.
    28 >
    29 > So let's set the limit to 50 or 100?
     17> Replying to [comment:15 teor]:
     18> > Replying to [comment:14 neel]:
     19> > > 2. If #1 is okay, which capacity size should {{{sampled_entry_guards}}} be to return 1? Right now, I am guessing 3 guards. Should it be more? Less?
     20> >
     21> > If we want to consider Tor Browser bridge users constrained, the answer is around 10 or 20.
     22> > If not, the answer is around 3.
    3023
     24> Replying to [comment:16 teor]:
     25> > There are 27 obfs4 bridges in Tor Browser at the moment:
     26> > https://gitweb.torproject.org/builders/tor-browser-build.git/tree/projects/tor-browser/Bundle-Data/PTConfigs/bridge_prefs.js
     27> > The other bridge types have 1-5 bridges.
     28> >
     29> > So let's set the limit to 50 or 100?
     30
     31----
     32
     33Replying to [comment:23 asn]:
    3134> 2) Why are we using the `capacity` field of a smartlist? We should (almost) never dig into the guts of smartlists. If we want to check the current number of sampled guards we should use `smartlist_len()`. Is that we are trying to do here?
    3235
    3336I asked for this change already, but I forgot to check that it had happened:
    3437
    35 Replying to [comment:15 teor]:
    36 > Replying to [comment:14 neel]:
    37 > > 1. Is checking the capacity of {{{sampled_entry_guards}}} from the {{{guard_selection_t}}} object okay for this patch?
    38 >
    39 > Checking the current size of sampled entry guards is ok.
    40 > (The capacity of a smartlist is the size it can grow to without allocating additional memory.)
     38> Replying to [comment:15 teor]:
     39> > Replying to [comment:14 neel]:
     40> > > 1. Is checking the capacity of {{{sampled_entry_guards}}} from the {{{guard_selection_t}}} object okay for this patch?
     41> >
     42> > Checking the current size of sampled entry guards is ok.
     43> > (The capacity of a smartlist is the size it can grow to without allocating additional memory.)
     44
     45Edit: fix formatting