Re: [tor-bugs] #30578 [Core Tor/Tor]: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-document, or ___?

[Tor Bug Tracker & Wiki]

#30578: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-
document, or ___?
-+-
 Reporter:  nickm|  Owner:  asn
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.2.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  circuitpadding, hopefully-easy,  |  Actual Points:
  wtf-pad|
Parent ID:   | Points:
 Reviewer:  asn  |Sponsor:
 |  Sponsor2
-+-
Changes (by asn):

 * keywords:  circuitpadding, 041-should, hopefully-easy, wtf-pad =>
 circuitpadding, hopefully-easy, wtf-pad
 * milestone:  Tor: 0.4.1.x-final => Tor: 0.4.2.x-final


Comment:

 I suggest we defer this to 042 since Mike's attempt did not make it work
 properly.

 If we don't want to have this disabled test in 041 whatsoever, then feel
 free to apply my branch from comment:5 in 041, and we can try to fix the
 test in 042.

--
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] #30578 [Core Tor/Tor]: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-document, or ___?

[Tor Bug Tracker & Wiki]

#30578: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-
document, or ___?
-+-
 Reporter:  nickm|  Owner:  asn
 Type:  defect   | Status:
 |  needs_revision
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.1.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  circuitpadding, 041-should,  |  Actual Points:
  hopefully-easy, wtf-pad|
Parent ID:   | Points:
 Reviewer:  asn  |Sponsor:
 |  Sponsor2
-+-
Changes (by asn):

 * status:  needs_review => needs_revision


Comment:

 Much better but still not perfect :(

 Running it like this:

 `$ while ./src/test/test --debug
 circuitpadding/circuitpadding_circuitsetup_machine ; do :; done`

 still fails after a few seconds with:

 {{{
 Jun 26 09:57:44.066 [info] circpad_send_padding_cell_for_callback():
 Callback: Sending padding to circuit (1) [length: 18446744073709551615]
 Jun 26 09:57:44.066 [debug] relay_send_command_from_edge___real():
 delivering 10 cell backward.
 Jun 26 09:57:44.066 [info] circpad_machine_spec_transition__real():
 Circpad machine 0 transitioning from 1 to 1
 Jun 26 09:57:44.066 [info] circpad_machine_schedule_padding__real():
 Padding in 540 usec
 Jun 26 09:57:44.066 [info] circpad_machine_schedule_padding__real():
 Padding in 0 sec, 540 usec
 Jun 26 09:57:44.066 [info] circpad_deliver_recognized_relay_cell_events():
 Got padding cell on origin circuit 1.
 Jun 26 09:57:44.066 [debug] relay_send_command_from_edge___real():
 delivering 10 cell forward.
 Jun 26 09:57:44.066 [debug] relay_send_command_from_edge___real(): Sending
 a RELAY_EARLY cell; 4 remaining.
 Jun 26 09:57:44.066 [info] circpad_machine_spec_transition__real():
 Circpad machine 0 transitioning from 2 to 2
 Jun 26 09:57:44.066 [info] circpad_machine_schedule_padding__real():
 Padding in 4154 usec
 Jun 26 09:57:44.066 [info] circpad_machine_schedule_padding__real():
 Padding in 0 sec, 4154 usec
 Jun 26 09:57:44.066 [info] circpad_deliver_recognized_relay_cell_events():
 Got padding cell on non-origin circuit 0.
 Jun 26 09:57:44.066 [info] circpad_send_padding_cell_for_callback():
 Callback: Sending padding to origin circuit 1 (5) [length:
 18446744073709551615]
 FAIL src/test/test_circuitpadding.c:2322: assert(n_client_cells OP_EQ 2):
 3 vs 2`
 }}}

 I'd be more comfortable if we don't rely on machine timing for this test
 given that this can vary wildly and is unpredictable in CI, etc.

--
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] #30578 [Core Tor/Tor]: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-document, or ___?

[Tor Bug Tracker & Wiki]

#30578: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-
document, or ___?
-+-
 Reporter:  nickm|  Owner:  asn
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.1.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  circuitpadding, 041-should,  |  Actual Points:
  hopefully-easy, wtf-pad|
Parent ID:   | Points:
 Reviewer:  asn  |Sponsor:
 |  Sponsor2
-+-

Comment (by mikeperry):

 (The reason for fixing the test is that it adds about 200 lines of
 coverage, taking circuitpadding from 92 to 94%).

--
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] #30578 [Core Tor/Tor]: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-document, or ___?

[Tor Bug Tracker & Wiki]

#30578: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-
document, or ___?
-+-
 Reporter:  nickm|  Owner:  asn
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.1.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  circuitpadding, 041-should,  |  Actual Points:
  hopefully-easy, wtf-pad|
Parent ID:   | Points:
 Reviewer:  asn  |Sponsor:
 |  Sponsor2
-+-
Changes (by mikeperry):

 * reviewer:  mikeperry => asn


Comment:

 Ok, I fixed the test and resolved the flapping issue. Shifting the
 circuitsetup machine by 2ms ensures that the call of
 timers_advaance_and_run(1) from circuit_package_relay_cell_mock() does not
 trigger the circuitsetup relay side machine to send padding (which was the
 problem).

 https://github.com/torproject/tor/pull/1141

 I am going to switch reviewer back to asn for the test flapping fix
 commits. The rest look good to me.

--
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] #30578 [Core Tor/Tor]: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-document, or ___?

[Tor Bug Tracker & Wiki]

#30578: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-
document, or ___?
-+-
 Reporter:  nickm|  Owner:  asn
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.1.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  circuitpadding, 041-should,  |  Actual Points:
  hopefully-easy, wtf-pad|
Parent ID:   | Points:
 Reviewer:  mikeperry|Sponsor:
 |  Sponsor2
-+-
Changes (by dgoulet):

 * reviewer:  asn => mikeperry


--
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] #30578 [Core Tor/Tor]: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-document, or ___?

[Tor Bug Tracker & Wiki]

#30578: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-
document, or ___?
-+-
 Reporter:  nickm|  Owner:  asn
 Type:  defect   | Status:
 |  needs_review
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.1.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  circuitpadding, 041-should,  |  Actual Points:
  hopefully-easy, wtf-pad|
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor2
-+-
Changes (by asn):

 * status:  assigned => needs_review


Comment:

 OK I tried another time to fix the test, but it seems to be a PITA. The
 test is testing some old complicated padding machines, and it's not
 accounting for some state transitions that are possible (in particular the
 relay machine transitions from BURST to GAP and sometimes sends padding
 really fast before the test accounts for it). Fixing it would require
 either huge changes to the test, or changes to the machine.

 IMO we should just remove the test since it's just doing in-depth testing
 of these machines that are now obsolete and only used as test machines. As
 part of my branch I moved these machines to the unittest file since they
 were never used by anything other than the unittests anyway:
 https://github.com/torproject/tor/pull/1086

--
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] #30578 [Core Tor/Tor]: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-document, or ___?

[Tor Bug Tracker & Wiki]

#30578: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-
document, or ___?
-+-
 Reporter:  nickm|  Owner:  asn
 Type:  defect   | Status:
 |  assigned
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.1.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  circuitpadding, 041-should,  |  Actual Points:
  hopefully-easy, wtf-pad|
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor2
-+-

Comment (by asn):

 Hm, I took a look and tried to naively activate the old test, it was still
 giving out failures.

 Fixing this test is possible I think now that #29298 is done, but it will
 still require some non-trivial changes to the test and also to remember
 how this complicated old test was supposed to work. I might need some help
 with Mike who wrote the test. Will work on other things for now because I
 have lots of backlog and come back to this.

--
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] #30578 [Core Tor/Tor]: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-document, or ___?

[Tor Bug Tracker & Wiki]

#30578: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-
document, or ___?
-+-
 Reporter:  nickm|  Owner:  asn
 Type:  defect   | Status:
 |  assigned
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.1.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  circuitpadding, 041-should,  |  Actual Points:
  hopefully-easy, wtf-pad|
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor2
-+-
Changes (by nickm):

 * owner:  (none) => asn
 * status:  new => assigned


Comment:

 Assigning to asn, at least as far as deciding which change to make.

--
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] #30578 [Core Tor/Tor]: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-document, or ___?

[Tor Bug Tracker & Wiki]

#30578: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-
document, or ___?
-+-
 Reporter:  nickm|  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.1.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  circuitpadding, 041-should,  |  Actual Points:
  hopefully-easy, wtf-pad|
Parent ID:   | Points:
 Reviewer:   |Sponsor:
 |  Sponsor2
-+-
Changes (by teor):

 * sponsor:   => Sponsor2


--
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] #30578 [Core Tor/Tor]: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-document, or ___?

[Tor Bug Tracker & Wiki]

#30578: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-
document, or ___?
-+-
 Reporter:  nickm|  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor:
 |  0.4.1.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  circuitpadding, 041-should,  |  Actual Points:
  hopefully-easy, wtf-pad|
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+-
Changes (by nickm):

 * keywords:  circuitpadding, 041-should, hopefully-easy => circuitpadding,
 041-should, hopefully-easy, wtf-pad


--
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

[tor-bugs] #30578 [Core Tor/Tor]: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-document, or ___?

[Tor Bug Tracker & Wiki]

#30578: The circuitpadding_circuitsetup_machine test: Re-enable, remove, re-
document, or ___?
-+-
 Reporter:  nickm|  Owner:  (none)
 Type:  defect   | Status:  new
 Priority:  Medium   |  Milestone:  Tor: 0.4.1.x-final
Component:  Core |Version:
  Tor/Tor|   Keywords:  circuitpadding, 041-should,
 Severity:  Normal   |  hopefully-easy
Actual Points:   |  Parent ID:
   Points:   |   Reviewer:
  Sponsor:   |
-+-
 Our code in `test_circuitpadding.c` says:
 {{{
   /** Disabled unstable test until #29298 is implemented (see #29122) */
   //  TEST_CIRCUITPADDING(circuitpadding_circuitsetup_machine, TT_FORK),
 }}}

 But both #29298 and #29122 are closed now.

 If this test will work now, let's enable it. If it is no longer useful,
 let's remove it. If it is disabled for some reason other than the one
 that's described in the comment, let's adjust the comment.

--
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