[tor-bugs] #33414 [Core Tor/Tor]: Split router.c, and disable it (mostly) when building without relay support.

[Tor Bug Tracker & Wiki]

#33414: Split router.c, and disable it (mostly) when building without relay
support.
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:  assigned
 Priority:  Medium   |  Milestone:  Tor: 0.4.4.x-final
Component:  Core |Version:
  Tor/Tor|   Keywords:  tor-design, network-team-roadmap-
 Severity:  Normal   |  2020Q1, 043-deferred
Actual Points:  0.5  |  Parent ID:  #31851
   Points:  0.5  |   Reviewer:
  Sponsor:   |
-+-
 The last ''big'' piece of code in feature/relay that we want to turn off
 when --disable-module-relay is set is `router.c`.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33414 [Core Tor/Tor]: Split router.c, and disable it (mostly) when building without relay support.

[Tor Bug Tracker & Wiki]

#33414: Split router.c, and disable it (mostly) when building without relay
support.
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:
 |  assigned
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-design, network-team-roadmap-|  Actual Points:  0.5
  2020Q1, 043-deferred   |
Parent ID:  #31851   | Points:  0.5
 Reviewer:   |Sponsor:
-+-

Comment (by nickm):

 I have a branch called `extract_router` with a PR at
 https://github.com/torproject/tor/pull/1762`.

 There are a few notes here:
   * The branch starts out by splitting the descriptor-generation code into
 its own file.
   * There is some code (currently in clientkey.[ch]) that I was not able
 to extract, since it is used when running as a client.  I believe this
 code to be useless for clients, and I'll open another ticket to look at
 removing it or making it relay only.  But that refactoring seems
 orthogonal to this one.
   * I've tried using some helper macros to generate the stubs in router.h.
 If we like those, we could extract them and use them to generate stubs in
 other files.

 I'm traveling today, so I won't be able to check whether or not CI has
 passed.  I'll put this into needs_review if it does.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33414 [Core Tor/Tor]: Split router.c, and disable it (mostly) when building without relay support.

[Tor Bug Tracker & Wiki]

#33414: Split router.c, and disable it (mostly) when building without relay
support.
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-design, network-team-roadmap-|  Actual Points:  0.5
  2020Q1, 043-deferred   |
Parent ID:  #31851   | Points:  0.5
 Reviewer:   |Sponsor:
-+-
Changes (by nickm):

 * status:  assigned => needs_review


Comment:

 CI has passed; I've added a changes file and am putting this in
 needs_review.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33414 [Core Tor/Tor]: Split router.c, and disable it (mostly) when building without relay support.

[Tor Bug Tracker & Wiki]

#33414: Split router.c, and disable it (mostly) when building without relay
support.
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-design, network-team-roadmap-|  Actual Points:  0.5
  2020Q1, 043-deferred   |
Parent ID:  #31851   | Points:  0.5
 Reviewer:  catalyst |Sponsor:
-+-
Changes (by dgoulet):

 * reviewer:   => catalyst


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33414 [Core Tor/Tor]: Split router.c, and disable it (mostly) when building without relay support.

[Tor Bug Tracker & Wiki]

#33414: Split router.c, and disable it (mostly) when building without relay
support.
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:
 |  merge_ready
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-design, network-team-roadmap-|  Actual Points:  0.5
  2020Q1, 043-deferred   |
Parent ID:  #31851   | Points:  0.5
 Reviewer:  catalyst |Sponsor:
-+-
Changes (by catalyst):

 * status:  needs_review => merge_ready


Comment:

 Looks good! There's a typo that I commented on in the pull request.

 I manually tested building with `--disable-module-relay`, but I didn't
 verify that the removed code was absent from the executable.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33414 [Core Tor/Tor]: Split router.c, and disable it (mostly) when building without relay support.

[Tor Bug Tracker & Wiki]

#33414: Split router.c, and disable it (mostly) when building without relay
support.
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:
 |  merge_ready
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-design, network-team-roadmap-|  Actual Points:  0.5
  2020Q1, 043-deferred   |
Parent ID:  #31851   | Points:  0.5
 Reviewer:  catalyst |Sponsor:
-+-

Comment (by teor):

 I think there's a missing keys lock when relay mode is disabled.
 And there might be merge conflict with #32588.

 Let's also open a ticket for the TODO dirauth change?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33414 [Core Tor/Tor]: Split router.c, and disable it (mostly) when building without relay support.

[Tor Bug Tracker & Wiki]

#33414: Split router.c, and disable it (mostly) when building without relay
support.
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-design, network-team-roadmap-|  Actual Points:  0.5
  2020Q1, 043-deferred   |
Parent ID:  #31851   | Points:  0.5
 Reviewer:  catalyst |Sponsor:
-+-
Changes (by teor):

 * status:  merge_ready => needs_revision


--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33414 [Core Tor/Tor]: Split router.c, and disable it (mostly) when building without relay support.

[Tor Bug Tracker & Wiki]

#33414: Split router.c, and disable it (mostly) when building without relay
support.
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-design, network-team-roadmap-|  Actual Points:  0.5
  2020Q1, 043-deferred   |
Parent ID:  #31851   | Points:  0.5
 Reviewer:  catalyst |Sponsor:
-+-
Changes (by nickm):

 * status:  needs_revision => needs_review


Comment:

 I've answered the comment about the missing lock: that lock is only used
 to protect onion keys, which clients don't have.

 I've also fixed the typo and rebased to master.  I checked the conflicting
 diff in question (07ab22bfd3ce43a7c3f8a5e7dd8608ceb1de18b6) with --color-
 moved to make sure I didn't mess up any other code, but an extra look-see
 wouldn't hurt.

 I opened #33739 for moving the dirauth-only keys to the dirauth module.

 The new branch is `extract_router_rebased` with PR at
 https://github.com/torproject/tor/pull/1839

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33414 [Core Tor/Tor]: Split router.c, and disable it (mostly) when building without relay support.

[Tor Bug Tracker & Wiki]

#33414: Split router.c, and disable it (mostly) when building without relay
support.
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-design, network-team-roadmap-|  Actual Points:  0.5
  2020Q1, 043-deferred   |
Parent ID:  #31851   | Points:  0.5
 Reviewer:  catalyst |Sponsor:
-+-

Comment (by teor):

 >  I've answered the comment about the missing lock: that lock is only
 used to protect onion keys, which clients don't have.

 Thanks!

 Can we add some comments that document this information?

 init_keys_common() and init_keys_client() don't have function comments,
 and init_keys_common() is a really confusing name for a relay-only
 function.

 Let's open another ticket?

 Otherwise the changes look good here.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33414 [Core Tor/Tor]: Split router.c, and disable it (mostly) when building without relay support.

[Tor Bug Tracker & Wiki]

#33414: Split router.c, and disable it (mostly) when building without relay
support.
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-design, network-team-roadmap-|  Actual Points:  0.5
  2020Q1, 043-deferred   |
Parent ID:  #31851   | Points:  0.5
 Reviewer:  catalyst |Sponsor:
-+-

Comment (by teor):

 I also opened #33789, to move the address-picking functions to their own
 files.

 nickm, catalyst, if you have an opinion on making these functions client-
 level functions, please comment on that ticket:
 * router_new_address_suggestion()
 * router_guess_address_from_dir_headers()

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #33414 [Core Tor/Tor]: Split router.c, and disable it (mostly) when building without relay support.

[Tor Bug Tracker & Wiki]

#33414: Split router.c, and disable it (mostly) when building without relay
support.
-+-
 Reporter:  nickm|  Owner:  nickm
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.4.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  tor-design, network-team-roadmap-|  Actual Points:  0.5
  2020Q1, 043-deferred   |
Parent ID:  #31851   | Points:  0.5
 Reviewer:  catalyst |Sponsor:
-+-
Changes (by catalyst):

 * status:  needs_review => needs_revision


Comment:

 Mostly looks good!

 Replying to [comment:8 teor]:
 > >  I've answered the comment about the missing lock: that lock is only
 used to protect onion keys, which clients don't have.
 >
 > Thanks!
 >
 > Can we add some comments that document this information?
 +1
 > init_keys_common() and init_keys_client() don't have function comments,
 and init_keys_common() is a really confusing name for a relay-only
 function.
 It looks like `init_keys_client()` has a function comment. (It was
 previously in the header file.) I'm not sure I understand how
 `init_keys_common()` is a relay-only function, though. It looks like
 `init_keys_client()` calls it?

 OK I looked a bit more closely and it's more confusing than I thought. I
 think it really could use better documentation in comments.

 The `init_keys_common()` definition in router.c is not built on clients,
 because router.c is not built on clients. However, clientkey.c includes
 router.h, which has a macro to stub out `init_keys_common()`. Also oddly
 enough, these patches change `init_keys_common()` to remove the `static`
 qualifier, which seems unnecessary to change?

 Or maybe rewrite things so that the naming isn't so confusing?

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs