Back when I added this logic in 20c0581a79, the rule was that whenever
a circuit finished building, we cleared its isolation info. I did that
so that we would still use the circuit even if all the streams that
had previously led us to tentatively set its isolation info had closed.
But there were problems with that approach: We could pretty easily get
into a case where S1 had led us to launch C1 and S2 had led us to
launch C2, but when C1 finished, we cleared its isolation and attached
S2 first. Since C2 was still marked in a way that made S1
unattachable to it, we'd then launch another circuit needlessly.
So instead, we try the following approach now: when a circuit is done
building, we try to attach streams to it. If it remains unused after
we try attaching streams, then we clear its isolation info, and try
again to attach streams.
Thanks to Sebastian for helping me figure this out.
Our old "do we need to launch a circuit for stream S" logic was,
more or less, that if we had a pending circuit that could handle S,
we didn't need to launch a new one.
But now that we have streams isolated from one another, we need
something stronger here: It's possible that some pending C can
handle either S1 or S2, but not both.
This patch reuses the existing isolation logic for a simple
solution: when we decide during circuit launching that some pending
C would satisfy stream S1, we "hypothetically" mark C as though S1
had been connected to it. Now if S2 is incompatible with S1, it
won't be something that can attach to C, and so we'll launch a new
stream.
When the circuit becomes OPEN for the first time (with no streams
attached to it), we reset the circuit's isolation status. I'm not
too sure about this part: I wanted some way to be sure that, if all
streams that would have used a circuit die before the circuit is
done, the circuit can still get used. But I worry that this
approach could also lead to us launching too many circuits. Careful
thought needed here.
This patch adds fields to track how streams should be isolated, and
ensures that those fields are set correctly. It also adds fields to
track what streams can go on a circuit, and adds functions to see
whether a streams can go on a circuit and update the circuit
accordingly. Those functions aren't yet called.
This lets us make a lot of other stuff const, allows the compiler to
generate (slightly) better code, and will make me get slightly fewer
patches from folks who stick mutable stuff into or_options_t.
const: because not every input is an output!
The conflicts were mainly caused by the routerinfo->node transition.
Conflicts:
src/or/circuitbuild.c
src/or/command.c
src/or/connection_edge.c
src/or/directory.c
src/or/dirserv.c
src/or/relay.c
src/or/rendservice.c
src/or/routerlist.c
This patch introduces a few new functions in router.c to produce a
more helpful description of a node than its nickame, and then tweaks
nearly all log messages taking a nickname as an argument to call these
functions instead.
There are a few cases where I left the old log messages alone: in
these cases, the nickname was that of an authority (whose nicknames
are useful and unique), or the message already included an identity
and/or an address. I might have missed a couple more too.
This is a fix for bug 3045.
Conflicts in various places, mainly node-related. Resolved them in
favor of HEAD, with copying of tor_mem* operations from bug3122_memcmp_022.
src/common/Makefile.am
src/or/circuitlist.c
src/or/connection_edge.c
src/or/directory.c
src/or/microdesc.c
src/or/networkstatus.c
src/or/router.c
src/or/routerlist.c
src/test/test_util.c
Conflicts throughout. All resolved in favor of taking HEAD and
adding tor_mem* or fast_mem* ops as appropriate.
src/common/Makefile.am
src/or/circuitbuild.c
src/or/directory.c
src/or/dirserv.c
src/or/dirvote.c
src/or/networkstatus.c
src/or/rendclient.c
src/or/rendservice.c
src/or/router.c
src/or/routerlist.c
src/or/routerparse.c
src/or/test.c
Here I looked at the results of the automated conversion and cleaned
them up as follows:
If there was a tor_memcmp or tor_memeq that was in fact "safe"[*] I
changed it to a fast_memcmp or fast_memeq.
Otherwise if there was a tor_memcmp that could turn into a
tor_memneq or tor_memeq, I converted it.
This wants close attention.
[*] I'm erring on the side of caution here, and leaving some things
as tor_memcmp that could in my opinion use the data-dependent
fast_memcmp variant.
Resolved conflicts in:
doc/tor.1.txt
src/or/circuitbuild.c
src/or/circuituse.c
src/or/connection_edge.c
src/or/connection_edge.h
src/or/directory.c
src/or/rendclient.c
src/or/routerlist.c
src/or/routerlist.h
These were mostly releated to the routerinfo_t->node_t conversion.
We've got millisecond timers now, we might as well use them.
This change won't actually make circuits get expiered with microsecond
precision, since we only call the expiry functions once per second.
Still, it should avoid the situation where we have a circuit get
expired too early because of rounding.
A couple of the expiry functions now call tor_gettimeofday: this
should be cheap since we're only doing it once per second. If it gets
to be called more often, though, we should onsider having the current
time be an argument again.
We need to make sure that the worst thing that a weird consensus param
can do to us is to break our Tor (and only if the other Tors are
reliably broken in the same way) so that the majority of directory
authorities can't pull any attacks that are worse than the DoS that
they can trigger by simply shutting down.
One of these worse things was the cbtnummodes parameter, which could
lead to heap corruption on some systems if the value was sufficiently
large.
This commit fixes this particular issue and also introduces sanity
checking for all consensus parameters.
There's no reason to keep a time_t and a struct timeval to represent
the same value: highres_created.tv_sec was the same as timestamp_created.
This should save a few bytes per circuit.
A node_t is an abstraction over routerstatus_t, routerinfo_t, and
microdesc_t. It should try to present a consistent interface to all
of them. There should be a node_t for a server whenever there is
* A routerinfo_t for it in the routerlist
* A routerstatus_t in the current_consensus.
(note that a microdesc_t alone isn't enough to make a node_t exist,
since microdescriptors aren't usable on their own.)
There are three ways to get a node_t right now: looking it up by ID,
looking it up by nickname, and iterating over the whole list of
microdescriptors.
All (or nearly all) functions that are supposed to return "a router"
-- especially those used in building connections and circuits --
should return a node_t, not a routerinfo_t or a routerstatus_t.
A node_t should hold all the *mutable* flags about a node. This
patch moves the is_foo flags from routerinfo_t into node_t. The
flags in routerstatus_t remain, but they get set from the consensus
and should not change.
Some other highlights of this patch are:
* Looking up routerinfo and routerstatus by nickname is now
unified and based on the "look up a node by nickname" function.
This tries to look only at the values from current consensus,
and not get confused by the routerinfo_t->is_named flag, which
could get set for other weird reasons. This changes the
behavior of how authorities (when acting as clients) deal with
nodes that have been listed by nickname.
* I tried not to artificially increase the size of the diff here
by moving functions around. As a result, some functions that
now operate on nodes are now in the wrong file -- they should
get moved to nodelist.c once this refactoring settles down.
This moving should happen as part of a patch that moves
functions AND NOTHING ELSE.
* Some old code is now left around inside #if 0/1 blocks, and
should get removed once I've verified that I don't want it
sitting around to see how we used to do things.
There are still some unimplemented functions: these are flagged
with "UNIMPLEMENTED_NODELIST()." I'll work on filling in the
implementation here, piece by piece.
I wish this patch could have been smaller, but there did not seem to
be any piece of it that was independent from the rest. Moving flags
forces many functions that once returned routerinfo_t * to return
node_t *, which forces their friends to change, and so on.
At best, this patch helps us avoid sending queued relayed cells that
would get ignored during the time between when a destroy cell is
sent and when the circuit is finally freed. At worst, it lets us
release some memory a little earlier than it would otherwise.
Fix for bug #1184. Bugfix on 0.2.0.1-alpha.
In rare cases, we could cannibalize a one-hop circuit, ending up
with a two-hop circuit. This circuit would not be actually used,
but we should prevent its creation in the first place.
Thanks to outofwords and swissknife for helping to analyse this.
The new rule is: safe_str_X() means "this string is a piece of X
information; make it safe to log." safe_str() on its own means
"this string is a piece of who-knows-what; make it safe to log".
There are two big changes here:
- We store active circuits in a priority queue for each or_conn,
rather than doing a linear search over all the active circuits
before we send each cell.
- Rather than multiplying every circuit's cell-ewma by a decay
factor every time we send a cell (thus normalizing the value of a
current cell to 1.0 and a past cell to alpha^t), we instead
only scale down the cell-ewma every tick (ten seconds atm),
normalizing so that a cell sent at the start of the tick has
value 1.0).
Each circuit is ranked in terms of how many cells from it have been
relayed recently, using a time-weighted average.
This patch has been tested this on a private Tor network on PlanetLab,
and gotten improvements of 12-35% in time it takes to fetch a small
web page while there's a simultaneous large data transfer going on
simultaneously.
[Commit msg by nickm based on mail from Ian Goldberg.]
Some *_free functions threw asserts when passed NULL. Now all of them
accept NULL as input and perform no action when called that way.
This gains us consistence for our free functions, and allows some
code simplifications where an explicit null check is no longer necessary.
Avoid crashing if the client is trying to upload many bytes and the
circuit gets torn down at the same time, or if the flip side
happens on the exit relay. Bugfix on 0.2.0.1-alpha; fixes bug 1150.
If the networkstatus consensus tells us that we should use a
negative circuit package window, ignore it. Otherwise we'll
believe it and then trigger an assert.
Also, change the interface for networkstatus_get_param() so we
don't have to lookup the consensus beforehand.
Tor now reads the "circwindow" parameter out of the consensus,
and uses that value for its circuit package window rather than the
default of 1000 cells. Begins the implementation of proposal 168.
The subversion $Id$ fields made every commit force a rebuild of
whatever file got committed. They were not actually useful for
telling the version of Tor files in the wild.
svn:r17867
Initial conversion of uint32_t addr to tor_addr_t addr in connection_t and related types. Most of the Tor wire formats using these new types are in, but the code to generate and use it is not. This is a big patch. Let me know what it breaks for you.
svn:r16435
Move n_addr, n_port, and n_conn_id_digest fields of circuit_t into a separately allocated extend_info_t. Saves 22 bytes per connected circuit_t on 32-bit platforms, and makes me more comfortable with using tor_addr_t in place of uint32_t n_addr.
svn:r16257
Patch from Christian Wilms: remove (HiddenService|Rend)(Exclude)?Nodes options. They never worked properly, and nobody seems to be using them. Resolves bug 754.
svn:r16144
Never allow a circuit to be created with the same circid as a circuit that has been marked for close. May be a fix for bug 779. Needs testing. Backport candidate.
svn:r16136
Fix all but 2 DOCDOC items; defer many XXX020s (particularly those where fixing them would fix no bugs at the risk of introducing some bugs).
svn:r13529
Be more thorough about memory poisoning and clearing. Add an in-place version of aes_crypt in order to remove a memcpy from relay_crypt_one_payload.
svn:r13414
Initial attempts to track down bug 600, and refactor possibly offending code. 1) complain early if circuit state is set to OPEN when an onionskin is pending. 2) refactor onionskin field into one only used when n_conn is pending, and a separate onionskin field waiting for attention by a cpuworker. This might even fix the bug. More likely, it will make it fail with a more useful core.
svn:r13394
Refactor circuit_launch* functions to take a bitfield of flags rather than 4 separate nonconsecutive flags arguments. Also, note a possible but in circuit_find_to_cannibalize, which seems to be ignoring its purpose argument.
svn:r12948
When sending destroy cells from a circuit's origin, don't include
the reason for tearing down the circuit. The spec says we didn't,
and now we actually don't. Reported by lodger. [Bugfix on 0.1.2.x]
svn:r10790
next little while, make it only take effect _when there's an attached
origin circuit_, rather than from the first attached origin circuit
until death of the conn.
svn:r10116
Yet another attempted Bug 411 fix: Under some circumstances, a circuit can have cells without being active. The likeliest is that it has been unlinked from all connections in preparation for closing. Therefore, stop enforcing this non-invariant.
svn:r9936
Initial version of circuit-based cell queues. Instead of hammering or_conns with piles of cells, queue cells on their corresponding circuits, and append them to the or_conn as needed. This seems to work so far, but needs a bit more work. This will break the memory-use-limitation patch for begin_dir conns: the solution will be a fun but fiddly.
svn:r9904
Make all LD_BUG log messsages get prefixed with "Bug: ". Remove manually-generated "Bug: "s from log-messages. (Apparently, we remembered to add them about 40% of the time.)
svn:r9733
Fix an XXXX012, and make circuits_pending_or_conns a static variable. In addition to cleaning up the code, this may also resolve Bug 386 if Roger has the right intuition there.
svn:r9482
Removing the last DOCDOC comment hurt so much that I had to use Doxygen to identify undocumented macros and comments, and add 150 more DOCDOCs to point out where they were. Oops. Hey, kids! Fixing some of these could be your first Tor patch!
svn:r9477
Have connection_about_to_close use an end_reason field in edge_connection_t to tell what reason to tell the controller for closing the stream. Set end_reason in connection_edge_end, connection_mark_unattached_ap, and everwhere we set edge_has_sent_end. Add a changelog entry.
svn:r8779
Fix an XXX in handling destroy cells: when we get a destroy cell with reason FOO, do not tell the controller REASON=FOO. Instead, say REASON=DESTROYED REMOTE_REASON=FOO. Suggested by a conversation with Mike Perry.
svn:r8760
Apply patch from Mike Perry: add more reasons for circuit destroys. (Slightly tweaked to avoid allocating a number for an "internal" reason.)
svn:r8739
Touch up last patch (to add REASON to CIRC events): make some reasons
more sensible, send reasons only to controllers that have enabled
extended events, and clean up whitespace.
svn:r8672
Refactor connection_t into edge, or, dir, control, and base subtypes. This might save some RAM on busy exit servers, but really matters most in terms of correctness.
svn:r6906
No circuit can be both an intro point and a rend point, so we can merge both the cookie and the pk digest into one "rend_token" field for or circuits. This saves another 20 bytes per or circuit.
svn:r6904
circuit_t into origin_circuit_t and or_circuit_t. I fixed some
segaults; there may be more. We still need to move more rendezvous
stuff into subtypes.
This is a trial run for splitting up connection_t; if the approach is
insane, please say so soon so we can do something smarter.
Also, this discards the old HALF_OPEN code, which nobody seems to
want.
svn:r6817
sometimes decrement conn->n_circuits even when there was no circuit
originally. This caused conn->n_circuits to go negative.
We noticed this because we were checking if connections can be
closed based on conn->n_circuits == 0, so we were never closing
any connection that had ever had a circuit on it.
svn:r5931
we screwed up the formatting in wild and unpredictable ways.
fix it before it becomes convention to format logs in wild and
unpredictable ways.
still need to do src/common/ someday.
svn:r5551
Weasel says circuit_get_by_conn is his main timesink. Most of its
users were just checking whether OR conns had circuits, so add a
circuit count to OR conns, and check that. One was
circuit_about_to_close_conn, which was doing an O(n^2) series of calls
to get all circs on an OR conn, so make an O(n) function for that.
Finally, circuit_get_by_edge_conn was using it as a sanity test that
has been around for a while but never found any actualy insanity, so
kill that.
circuit_get_by_conn is finally dead, which is good, since it was never
sane to begin with.
svn:r5460
separately. It's important to keep them separate because internal
circuits have their last hops picked like middle hops, rather than like
exit hops. So exiting on them will break the user's expectations.
- Stop cannibalizing internal circuits for general exits, and stop
cannibalizing exit circuits for rendezvous stuff.
- Don't let new exit streams attach to internal circuits.
- When deciding if we have enough circuits for internal and for exit,
don't count the wrong ones.
- Treat predicted resolves as predicted port 80 exits.
svn:r5457
