Opened 8 years ago

Closed 8 years ago

Last modified 6 years ago

#1757 closed task (implemented)

Microdescriptors: abstract the notion of "Tor node" in the code

Reported by: nickm Owned by: nickm
Priority: Medium Milestone: Deliverable-Sep2010
Component: Core Tor/Tor Version:
Severity: Keywords:
Cc: Actual Points:
Parent ID: #1748 Points:
Reviewer: Sponsor:

Description

Right now we have at least 5 things that we use to indicate a Tor node:

  • A const char * pointing to an ID key digest
  • A routerinfo_t
  • A routerstatus_t
  • An extend_info_t
  • An entry in rephist.c, though those are mostly isolated.

For microdescriptors, we're about to add another thing:

  • A microdescriptor_t

Making our code work properly with all of these variant things is already a bit hairy. It sure would be nice if we had some kind of abstract node_t that could make it so our code worked well with all of the above.

First, we'll need a basic design here. At the minimum we should make routerstatus_t and routerinfo_t (basically) immutable, and add a node_t that abstracts routerstatus_t and routerinfo_t. Then, we can make it so that its interface can be satisfied by microdescriptor_t as well.

Child Tickets

Change History (12)

comment:1 Changed 8 years ago by arma

I wonder if we should take this opportunity to clean up bridge_info_t too.

Probably not, but we should do the above cleanups with an eye toward not making it harder to do the bridge_info_t cleanup.

comment:2 Changed 8 years ago by nickm

Working on this one. The hard part is that there is too much mutable-flag stuff split and shared between routerinfo_t and routerstatus_t. Nearly all mutable stuff needs to move to node_t. (The alternative, adding a third copy of all the mutable flag stuff into microdesc_t, is too awful to contemplate.)

So, the plan becomes:

  • Add node_t as a wrapper around routerinfo_t, and routerstatus_t. A node_t is "A node we could use in a circuit."
  • Have node_ts exist whenever we have a routerstatus_t in our favored consensus, or a routerinfo_t.
  • Move mutable flags and other fields from routerinfo_t into node_t.
  • DO NOT move document-management flags from routerinfo_t into node_t
  • Make is_foo flags in routerstatus_t as immutable as we can.
  • When making decisions about what nodes to use, base them on node_t. This should simplify the choose-a-node code a little.
  • Add a microdesc_t backend for node_t. If we've designed it right, this is easy.

Part done. The design is the hard part, one hopes. The rest: "just a simple matter of programming."

comment:3 Changed 8 years ago by arma

Sounds plausible. I'm not sure I'm following all the details though.

So does that mean the is_running flag in routerstatus_t will reflect what the consensus tells us to think, and the is_running flag in node_t will tell us whether we currently think it is running?

Looking forward to seeing the first cut of the code, so I can get a better intuition about whether I think this is the right direction. :/

comment:4 Changed 8 years ago by nickm

That's right (about the flags).

comment:5 Changed 8 years ago by nickm

Owner: set to nickm
Status: newaccepted

Revised plan and status (strikethrough means "done"):

  • Add node_t as a wrapper around routerinfo_t, and routerstatus_t. A node_t is "A node we could use in a circuit."
  • Have node_ts exist whenever we have a routerstatus_t in our favored consensus, or a routerinfo_t.
  • Move mutable flags and other fields from routerinfo_t into node_t.
    • DO NOT move document-management flags from routerinfo_t into node_t
  • Make is_foo flags in routerstatus_t as immutable as we can.
    • identity what changes
    • Make it not change. (It's just is_running!)
  • When making decisions about what nodes to use, base them on node_t. This should simplify the choose-a-node code a little.
    • Change the code to use node_t
    • Make it all compile (doing now)
    • Implement missing node-based functions
      • Misc glue
      • Families
      • Policies
  • Add a microdesc_t backend for node_t. If we've designed it right, this is easy. (Doing as I go along.)

If you want to kibbitz, have a look at branch "nodes" in my public. I will rebase it a lot, though, so do not get too attached to it. No, it does not compile atm.

comment:6 Changed 8 years ago by nickm

Tracking hard-to-support-things I should ask about when I'm done:

  • Knowing if you cache extrainfo without a routerinfo.
  • Knowing if you allow single-hop exits without a routerinfo
  • Getting a published time without a routerinfo.
  • Getting declared uptime without a routerinfo.

More stuff to do:

  • Audit all users of extend_info_from_node() to make sure they can deal with getting NULL, or that they never get NULL.

comment:7 Changed 8 years ago by nickm

All of this is now done except:

  • Exit policies
  • Dealing with single-hop-exit nodes
  • Dealing with extrainfo caches

comment:8 Changed 8 years ago by nickm

The policy code is done; the extrainfo code is right-by-accident.

The single-hop-exit code wants a patch in the authorities in 0.2.2

comment:9 Changed 8 years ago by nickm

Status: acceptedneeds_review

See branch "nodes" in my public repo. The client-microdescriptor branch will depend on it.

We need to do the single-hop-exit fix for 0.2.2.

This branch needs more testing and fixing. Right now, starting it with an empty .tor directory predictibly throws it into a tizzy where it never actually gets any descriptors. Not sure why yet.

comment:10 Changed 8 years ago by nickm

Okay, thanks to arma and a weekend's rest, I think I've got the observable bugs here nailed down. Please review branch "nodes" in my public repo.

comment:11 Changed 8 years ago by nickm

Resolution: implemented
Status: needs_reviewclosed

Right; finally merged this. If you find any bugs, please open a new ticket.

comment:12 Changed 6 years ago by nickm

Component: Tor ClientTor
Note: See TracTickets for help on using tickets.