Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#3022 closed defect (fixed)

Caches should no longer cache v2 networkstatus documents

Reported by: nickm Owned by:
Priority: High Milestone: Tor: 0.2.2.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Right now, v2 networkstatus documents are used by nobody but authorities; we hope that we'll use the "opinion vote" design to make them needless even there, but for now, there's no point in havign caches download or store them.

We think.

We should make sure.

This could save some sweet sweet bandwidth.

Child Tickets

Change History (17)

comment:1 Changed 8 years ago by nickm

Status: newneeds_review

See branch bug3022 in my public repository.

comment:2 Changed 8 years ago by Sebastian

I don't think this is a good patch. It will break our dnsel tors more, because they can't fetch the v2 directories anymore which they rely on for learning about descriptors. Not sure if bwauths also set these options

comment:3 Changed 8 years ago by nickm

Thanks for catching that. In that case, what if we made fetching v2 votes a config option, defaulting to except for authorities, and made it so that that if you _do_ fetch v2 votes, you always do so from authorities?

Also, anything that currently depends on getting v2 votes really needs to migrate away from them. There are not so many v2 authorities left, and we're hoping to remove them entirely once we can get proposal 147 implemented.

Thoughts?

comment:4 Changed 8 years ago by nickm

New branch: bug3022_v2. This time, it's optional, so if you need your tor to fetch v2 votes, you can. Please review?

comment:5 Changed 8 years ago by Sebastian

Maybe we should use FetchUselessDescriptors for that? Because with v3, you can't fetch non-running descriptors because they aren't in the consensus, so setting FetchUselessDescriptors without FetchV2Networkstatus makes no sense. I think there's no use-case left for FetchV2Networkstatus otherwise.

comment:6 Changed 8 years ago by nickm

That sounds plausible. V2 votes aren't exactly "descriptors", but that's probably no disaster.

I wonder what karsten and mikeperry think about whether this affects their tools.

comment:7 Changed 8 years ago by mikeperry

For the bw auths, I think that instead of FetchUselessDescriptors and v2 networkstatuses, we just want to move to FetchDirInfoEarly and FetchDirInfoExtraEarly.

Is there current logic in tor that might cause it to not fetch listed Running consensus descriptors? Or is FetchUselessDescriptors currently only about fetching non-Running descriptors?

comment:8 in reply to:  7 ; Changed 8 years ago by arma

Replying to mikeperry:

For the bw auths, I think that instead of FetchUselessDescriptors and v2 networkstatuses, we just want to move to FetchDirInfoEarly and FetchDirInfoExtraEarly.

Is there current logic in tor that might cause it to not fetch listed Running consensus descriptors? Or is FetchUselessDescriptors currently only about fetching non-Running descriptors?

FetchUselessDescriptors does that, and also one other thing:

int
directory_too_idle_to_fetch_descriptors(or_options_t *options, time_t now)
{
  return !directory_caches_dir_info(options) &&
         !options->FetchUselessDescriptors &&
         rep_hist_circbuilding_dormant(now);
}

which causes update_consensus_router_descriptor_downloads() to short-circuit if true. So, no fetching descriptors (even running ones) if you haven't needed circuits lately.

comment:9 Changed 8 years ago by karsten

I don't think that any of the tools I maintain are affected. metrics-db doesn't download v2 network statuses at all, and it uses its own logic to download descriptors. (metrics-db fails to download non-referenced descriptors, but that's unrelated to changing tor.) None of the other tools would need v2 network statuses or descriptors that are not referenced in the consensus.

comment:10 in reply to:  8 Changed 8 years ago by mikeperry

Replying to arma:

Replying to mikeperry:

Is there current logic in tor that might cause it to not fetch listed Running consensus descriptors? Or is FetchUselessDescriptors currently only about fetching non-Running descriptors?

FetchUselessDescriptors does that, and also one other thing:

int
directory_too_idle_to_fetch_descriptors(or_options_t *options, time_t now)
{
  return !directory_caches_dir_info(options) &&
         !options->FetchUselessDescriptors &&
         rep_hist_circbuilding_dormant(now);
}

which causes update_consensus_router_descriptor_downloads() to short-circuit if true. So, no fetching descriptors (even running ones) if you haven't needed circuits lately.

Does rep_hist_circbuilding_dormant(now) care about circuit purpose and who attaches streams? What if only controller circuits are built, and tor only sees port 443 in use, and the controller always has a 443 circuit ready before attempted use, and furthermore, if LeaveStreamsUnattached is set, so tor never even sees that it needs to try to attach port 443 streams?

It looks to me as if rep_hist_note_used_port() will not be called if LeaveStreamsUnattached is set.

Am I right in this? This means that I won't fetch any descriptors if only the controller is building circuits and attaching them, unless FetchUselessDescriptors is set?

Wrt this bug, I don't think the bw auth use of the option anything to do with v2 networkstatus documents, nor does it have anything to do with the need to fetch non-Running descriptors.. It is only about making sure Tor doesn't decide to stop downloading descriptors, which I believe I've experienced in the past, early on in the exit scanner and bw auth development. The code paths I mention above seem to show that this is still true.

comment:11 Changed 8 years ago by mikeperry

To summarize the above comment, it is fine for the bw auths if FetchV2Networkstatus is a separate option from FetchUnusedDescriptors. We need FetchUselessDescriptors but not FetchV2Networkstatus. So keeping the two separate should cause less directory activity by the bw auths.

We also only scan nodes in the consensus, so we don't even need the non-Running fetch property of FetchUselessDescriptors.

comment:12 Changed 8 years ago by nickm

So it sounds like both Mike and Karsten believe bug3022_v2 would not affect their code.

Sebastian, do you know any remaining issues?

comment:13 Changed 8 years ago by Sebastian

No, other than that I still think adding yet another option isn't really warranted. But if we think it is cleaner this way, then let's do it.

comment:14 Changed 8 years ago by nickm

IMO I think a separate option is cleaner: A V2 Network Status isn't really a Descriptor of any kind, Useless or otherwise.

comment:15 Changed 8 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Merging into maint-0.2.2 and later. Thanks for the review and research, everybody!

comment:16 Changed 7 years ago by nickm

Keywords: tor-relay added

comment:17 Changed 7 years ago by nickm

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