Opened 2 years ago

Closed 23 months ago

#9934 closed enhancement (implemented)

Add control command to drop guard nodes

Reported by: ra Owned by:
Priority: minor Milestone: Tor: 0.2.5.x-final
Component: Tor Version:
Keywords: tor-client guards controller Cc:
Actual Points: Parent ID:
Points:

Description

The attached patches add a new control command to Tor "DUMPGUARDS" that tells the server to remove all guard nodes.

Child Tickets

Attachments (3)

0001-add-DUMPGUARDS-command.patch (745 bytes) - added by ra 2 years ago.
patch to control-spec.txt
tor-dumpguards.patch (2.3 KB) - added by ra 2 years ago.
Patch to Tor source
tor-dumpguards.2.patch (2.5 KB) - added by ra 23 months ago.
v2

Download all attachments as: .zip

Change History (9)

Changed 2 years ago by ra

patch to control-spec.txt

Changed 2 years ago by ra

Patch to Tor source

comment:1 follow-up: Changed 2 years ago by nickm

  • Keywords tor-client guards controller added
  • Milestone set to Tor: 0.2.5.x-final
  • Status changed from new to needs_revision

Quick notes:

  • Should there be an ability to drop a specific guard?
  • Right now, if you call this command with arguments, they all get ignored. That can't be right.
  • The loop is strangely structured, given that "i" is never incremented. Perhaps it would make more sense to do while (smartlist_len(entryguards) { entryguard = smartlist_get(entryguards, 0); ... }?
  • Remember that in C, "void fn()" is a function that takes an unspecified number of arguments. If you need to write a function that takes no arguments, you have to say "void fn(void)".
  • Every function should be documented.
  • Perhaps we should explain briefly what the intended use case of this command is.

comment:2 in reply to: ↑ 1 Changed 23 months ago by ra

Replying to nickm:

  • Should there be an ability to drop a specific guard?

Dropping all guards is sufficient for my use case and the one expressed on tor-dev but I can't give a general answer on that.

Replying to nickm:

  • Right now, if you call this command with arguments, they all get ignored. That can't be right.
  • The loop is strangely structured, given that "i" is never incremented. Perhaps it would make more sense to do while (smartlist_len(entryguards) { entryguard = smartlist_get(entryguards, 0); ... }?
  • Remember that in C, "void fn()" is a function that takes an unspecified number of arguments. If you need to write a function that takes no arguments, you have to say "void fn(void)".

I will rewrite and resubmit the patch.

  • Every function should be documented.

I though that documenting "remove_all_entry_guards" is superfluous but I will add a short documentation to it.

  • Perhaps we should explain briefly what the intended use case of this command is.

To the control-spec.txt..?

Changed 23 months ago by ra

v2

comment:3 Changed 23 months ago by nickm

  • Status changed from needs_revision to needs_review

Almost: that one is doing a string length comparison, so it will reject the command if there's space after it which probably isn't what we wanted. (Other commands allow additional space.)

I'll see if it's easy to just tweak that up and merge it.

comment:4 follow-up: Changed 23 months ago by nickm

I had to tweak it a little to make it merge into master. How do you like branch "bug9934_nm" in my public repository ( https://gitweb.torproject.org/nickm/tor.git ) ?

I also renamed the command to DROPGUARDS, since "dump" in programmerese can mean either "discard" or "print".

Please let me know if you like this version of the patch, and whether I should credit the patch to "ra" or to some longer name.

comment:5 in reply to: ↑ 4 Changed 23 months ago by ra

Replying to nickm:

I had to tweak it a little to make it merge into master. How do you like branch "bug9934_nm" in my public repository ( https://gitweb.torproject.org/nickm/tor.git ) ?

The smartlist solution seems cleaner than the string length comparison. I tried but couldn't figure how to do it in a short time. Anyways, the branch looks perfectly fine to me.

I also renamed the command to DROPGUARDS, since "dump" in programmerese can mean either "discard" or "print".

Ack.

Please let me know if you like this version of the patch, and whether I should credit the patch to "ra" or to some longer name.

"ra" is fine.

comment:6 Changed 23 months ago by nickm

  • Resolution set to implemented
  • Status changed from needs_review to closed

Okay, merged. Thanks!

Note: See TracTickets for help on using tickets.