[tor-bugs] #13737 [Tor]: Implement circuit building crypto to worker

[Tor Bug Tracker & Wiki]

#13737: Implement circuit building crypto to worker
-------------------------------------------------+-------------------------
 Reporter:  dgoulet                              |          Owner:
     Type:  enhancement                          |         Status:
 Priority:  Medium                               |  needs_revision
Component:  Tor                                  |      Milestone:  Tor:
 Severity:  Normal                               |  0.2.8.x-final
 Keywords:  tor-client, tor-hs, multicore,       |        Version:  Tor:
  performance, 027-triaged-1-in                  |  0.2.7
Parent ID:                                       |     Resolution:
  Sponsor:                                       |  Actual Points:
                                                 |         Points:  small-
                                                 |  remaning
-------------------------------------------------+-------------------------
Changes (by nickm):

 * status:  needs_review => needs_revision


Comment:

 1b150bcb9d54ccfe132b3e58ee9ef5f001f70221 -- looks ok.

 3e75d22bd496c1238cabee25b8616ea6b02805d8
    * The fields in origin_entry_t need documentation. What are they for?
 Who sets them?  The type itself could also use some explicitness. How do
 they get removed? How do they get removed when the origin_circuit_dies?
    * origin_entry_t and origin_queue could use a friendlier name, maybe
 with "pending_cpu" or "pending_crypto" in it.
    * All arguments of origin_pending_add should be documented.  It should
 document which items it takes ownership of.  It should also document
 whether it gives items to cpuworkers, or if not, who does?
    * In origin_next_task, must the caller also free hop_out?
    * origin_pending_remove has no documentation.
    * I am fine with having this linear search in origin_pending_remove(),
 but we should probably have an XXX about it.
    * Since CIRC_STATE_ONIONSKIN_PENDING can now mean something a little
 different than before, its documentation should be updated.
    * I am not in love with having total_pending_tasks and
 max_pending_tasks be shared by both types of work. (This speaks to the
 'queue_pending_tasks' thing.)
    * I would like to have queue_pending_*_tasks have a more obvious end
 condition; I'm worried about infinite loops.
    * [META]: I assume this is duplicated from existing code, but I don't
 like how assign_onionskin_client_to_cpuworker() can return -1 for "I
 queued the work for later and -1 for "I tried to add the job, but
 threadpool_queue_work failed!"

 META:
    * Do we really need to have two separate queues here, one for pre-
 queued stuff and one for stuff  that's on a workqueue?  Why not just put
 stuff straight on the workqueue?

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/13737#comment:16>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online