[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: Patches for proposal 155: Four Improvements of Hidden Service Performance
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Roger Dingledine wrote:
>> Index: /home/karsten/tor/tor-trunk-155-patch1/ChangeLog
>> ===================================================================
>> + - Reduce extension timeout for introduction circuits from 60 to 30
>> + seconds.
>
> Applied and in. I changed it a bit, but not in any substantial way
> I think.
Yay, looks good!
>> Index: /home/karsten/tor/tor-trunk-155-patch2/ChangeLog
>> ===================================================================
>> + - Start extending introduction circuits in parallel after a delay of
>> + 15 seconds (based on work by Christian Wilms).
>
> Isn't this patch a smaller version of patch4? That is, if we started
> enough circuits to begin with, can we just skip this step? It seems
> odd to apply both patch2 and patch4, and it seems like patch4 is the
> more general.
Ah no, this patch affects circuit extension to an introduction point on
client side, whereas patch 4 deals with establishing introduction points
by the service. On client side, this process needs to be done on demand,
because the client cannot know the introduction point to build a circuit
to in advance. That's why this step is one of the most critical in the
protocol where we lose up to an entire minute when establishing a
connection (or half a minute with patch 1).
>> Index: /home/karsten/tor/tor-trunk-155-patch3/ChangeLog
>> ===================================================================
>> + - Increase number of server-side introduction points based on number
>> + of recent hidden service requests (based on work by Christian Wilms).
>
> I think this is a very useful change, since we really do want our hidden
> services to adapt the number of preemptive circuits they hold open to
> the amount of attention they get. But the patch here looks much more
> complex than it needs to be.
>
> Rather than keeping a whole history with exact timestamps and everything,
> why not keep a counter and a timestamp in rephist (or rend*), and let
> the counter decay over time.
>
> That is, when a circuit is requested we call a function to increment the
> counter (but never past some cap), and when we're considering making
> a new circuit we call a function to get the current counter value,
> and every time either of these functions is called we check a static
> time_t and move the counter value down by 1 (but not less than 0) for
> every interval that's passed since we last called one of the functions.
>
> It's not quite as precise, but it's way less code. :)
Alas, I have to say that I'm not convinced of the usefulness of this
change anymore. It sounds like a good idea to be adaptive with the
number of pre-built circuits, but it does not reflect current reality.
The improvements are based on two assumptions, that might be wrong:
First, we assumed that a popular hidden service cannot make use of
cannibalization and therefore needs to extend all hops of a circuit to a
rendezvous point. That's right, but we forgot that extending a
previously cannibalized circuit takes time, too. That means that the
performance improvement is not as high as expected.
Second, the hidden service needs to be really popular for being in a
situation where he doesn't have circuits ready for client requests. With
this patch a hidden service needs to see 300 requests per hour to build
a 4th internal circuit instead of only 3. Given the average number of
fetch requests to the directory, I'd guess that there is not a single
hidden service out there which has this load. And the choice of 300 (or
5 per minute) is probably still low to see an effect of the additional
circuit at all.
All in all, I'd say we hold back this patch for now until we know better
whether this is a problem at all. It probably wouldn't hurt to add it.
But it doesn't make sense to add this much new complexity (your proposed
code change reduces that to some extend, but still) for an uncertain effect.
>> Index: /home/karsten/tor/tor-trunk-155-patch4/ChangeLog
>> ===================================================================
>> + - Start building more server-side introduction circuits than needed
>> + (five), pick the first three that succeed, and use the others as
>> + general-purpose circuits.
>
> Looks plausible. I'm going to review the patch more tomorrow and then
> check it (or something like it) in.
>
>> /* Remember how many introduction circuits we started with. */
>> prev_intro_nodes = smartlist_len(service->intro_nodes);
>> -
>> - /* The directory is now here. Pick three ORs as intro points. */
>> - for (j=prev_intro_nodes; j < NUM_INTRO_POINTS; ++j) {
>> + /* The directory is now here. Pick three ORs as intro points (plus, if
>> + * we currently have none at all, two more so that we can pick the first
>> + * three afterwards). */
>> +#define NUM_INTRO_POINTS_INIT (NUM_INTRO_POINTS + 2)
>> + for (j=prev_intro_nodes; j < (prev_intro_nodes == 0 ?
>> + NUM_INTRO_POINTS_INIT : NUM_INTRO_POINTS); ++j) {
>> router_crn_flags_t flags = CRN_NEED_UPTIME;
>> if (get_options()->_AllowInvalid & ALLOW_INVALID_INTRODUCTION)
>> flags |= CRN_ALLOW_INVALID;
>
> This is the part that makes me nervous still. We make 5 intro points,
> and as soon as three are ready, we discard (change to general) any others
> that finish. What if the three that are ready are at the end of the intro
> points list? Don't we need to remove the intro point from the list too,
> if we decide not to use its intro circuit?
Ah, the idea is to build 5 introduction *circuits* and use the first 3
to establish introduction points, not to establish 5 introduction
*points* and throw away 2 of them. That's a big difference---in my
personal vocabulary that I apparently missed to share with you. ;) That
means that we only send ESTABLISH_INTRO cells to the first 3 completed
circuits, not to the 2 other circuits. As an effect, those 2 don't make
it to the introduction points list at all. They are declared as general
circuits, so that they might be used for other purposes---including
being cannibalized soon after if one of the introduction point should fail.
Best,
- --Karsten
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFI9aq80M+WPffBEmURAtwVAJ4nBSPgcX33fZ7WfOKD8HReDyUTrgCeJjuj
yTE6lqlbdCcvZfzgun0oMTI=
=v+NX
-----END PGP SIGNATURE-----