Re: [tor-bugs] #24975 [Core Tor/Tor]: sched: scheduler_notify_networkstatus_changed() calls select_scheduler() without the new consensus

[Tor Bug Tracker & Wiki]

#24975: sched: scheduler_notify_networkstatus_changed() calls select_scheduler()
without the new consensus
-+
 Reporter:  dgoulet  |  Owner:  dgoulet
 Type:  defect   | Status:  closed
 Priority:  High |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  032-backport, tor-sched  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  armadev  |Sponsor:
-+
Changes (by dgoulet):

 * status:  needs_review => closed
 * resolution:   => fixed


Comment:

 Replying to [comment:9 nickm]:
 > I've merged to 0.3.2 and forward (on the theory that having only one
 version of this to maintain will make us happier).  I put the
 dos_consensus_has_changed() call in notify_before but please let's change
 it as appropriate?
 >
 > Leaving this open till somebody confirms the dos_consensus_has_changed()
 issue.

 Yes I do confirm that it is properly placed. Using the new consensus it
 was needs to be done so good!

--
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] #24975 [Core Tor/Tor]: sched: scheduler_notify_networkstatus_changed() calls select_scheduler() without the new consensus

[Tor Bug Tracker & Wiki]

#24975: sched: scheduler_notify_networkstatus_changed() calls select_scheduler()
without the new consensus
-+
 Reporter:  dgoulet  |  Owner:  dgoulet
 Type:  defect   | Status:  needs_review
 Priority:  High |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  032-backport, tor-sched  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  armadev  |Sponsor:
-+

Comment (by nickm):

 I've merged to 0.3.2 and forward (on the theory that having only one
 version of this to maintain will make us happier).  I put the
 dos_consensus_has_changed() call in notify_before but please let's change
 it as appropriate?

 Leaving this open till somebody confirms the dos_consensus_has_changed()
 issue.

--
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] #24975 [Core Tor/Tor]: sched: scheduler_notify_networkstatus_changed() calls select_scheduler() without the new consensus

[Tor Bug Tracker & Wiki]

#24975: sched: scheduler_notify_networkstatus_changed() calls select_scheduler()
without the new consensus
-+
 Reporter:  dgoulet  |  Owner:  dgoulet
 Type:  defect   | Status:  needs_review
 Priority:  High |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  032-backport, tor-sched  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  armadev  |Sponsor:
-+

Comment (by arma):

 Oh, and eventually all of those other "things we do with a new consensus"
 should work their way out of networkstatus_set_current_consensus() into
 one of these new functions.

--
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] #24975 [Core Tor/Tor]: sched: scheduler_notify_networkstatus_changed() calls select_scheduler() without the new consensus

[Tor Bug Tracker & Wiki]

#24975: sched: scheduler_notify_networkstatus_changed() calls select_scheduler()
without the new consensus
-+
 Reporter:  dgoulet  |  Owner:  dgoulet
 Type:  defect   | Status:  needs_review
 Priority:  High |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  032-backport, tor-sched  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  armadev  |Sponsor:
-+

Comment (by arma):

 New branch looks ok.

 The changelog says "This lead [sic] to the scheduler failing to notice any
 consensus parameters that might have changed between consensuses" but I
 think it isn't quite that bad -- the bug meant that you picked up the new
 consensus params on the following consensus fetch.

 So it could count as a minor bugfix and go only on master if we wanted to
 be more conservative.

 Speaking of conflicts, when this gets merged there will be a conflict with
 the new dos_consensus_has_changed() call, which can be put in either the
 notify_before or notify_after function, so long as it's done right.

--
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] #24975 [Core Tor/Tor]: sched: scheduler_notify_networkstatus_changed() calls select_scheduler() without the new consensus

[Tor Bug Tracker & Wiki]

#24975: sched: scheduler_notify_networkstatus_changed() calls select_scheduler()
without the new consensus
-+
 Reporter:  dgoulet  |  Owner:  dgoulet
 Type:  defect   | Status:  needs_review
 Priority:  High |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  032-backport, tor-sched  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  armadev  |Sponsor:
-+
Changes (by dgoulet):

 * status:  reopened => 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] #24975 [Core Tor/Tor]: sched: scheduler_notify_networkstatus_changed() calls select_scheduler() without the new consensus

[Tor Bug Tracker & Wiki]

#24975: sched: scheduler_notify_networkstatus_changed() calls select_scheduler()
without the new consensus
-+
 Reporter:  dgoulet  |  Owner:  dgoulet
 Type:  defect   | Status:  reopened
 Priority:  High |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  032-backport, tor-sched  |  Actual Points:
Parent ID:   | Points:
 Reviewer:  armadev  |Sponsor:
-+
Changes (by dgoulet):

 * status:  closed => reopened
 * reviewer:   => armadev
 * resolution:  fixed =>


Comment:

 Ok sorry, I fucked up and arma noticed so good!

 I've pushed a revert and then a commit that does the right thing.

 The problem was that the function was moved _after_ but was still using
 `get_latest_consensus()` for the parameter of the "old consensus". After
 we set globally the new consensus, that doesn't work so well...

 Solution is to go with a before and after function. To achieve such, the
 scheduler is a bit changed but only parameters that are actually never
 used are removed.

 Branch: `bug24975_032_01`

--
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] #24975 [Core Tor/Tor]: sched: scheduler_notify_networkstatus_changed() calls select_scheduler() without the new consensus

[Tor Bug Tracker & Wiki]

#24975: sched: scheduler_notify_networkstatus_changed() calls select_scheduler()
without the new consensus
-+
 Reporter:  dgoulet  |  Owner:  dgoulet
 Type:  defect   | Status:  closed
 Priority:  High |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:  032-backport, tor-sched  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+
Changes (by nickm):

 * status:  needs_review => closed
 * resolution:   => fixed


Comment:

 merged to 0.3.2 and forward.

--
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] #24975 [Core Tor/Tor]: sched: scheduler_notify_networkstatus_changed() calls select_scheduler() without the new consensus

[Tor Bug Tracker & Wiki]

#24975: sched: scheduler_notify_networkstatus_changed() calls select_scheduler()
without the new consensus
-+
 Reporter:  dgoulet  |  Owner:  dgoulet
 Type:  defect   | Status:  needs_review
 Priority:  High |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  032-backport, tor-sched  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+
Changes (by dgoulet):

 * status:  assigned => needs_review


Comment:

 See branch: `bug24975_032_01`

 Important bug that affects any values set by the consensus for the KIST
 scheduler so we need to backport it.

--
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] #24975 [Core Tor/Tor]: sched: scheduler_notify_networkstatus_changed() calls select_scheduler() without the new consensus

[Tor Bug Tracker & Wiki]

#24975: sched: scheduler_notify_networkstatus_changed() calls select_scheduler()
without the new consensus
-+
 Reporter:  dgoulet  |  Owner:  dgoulet
 Type:  defect   | Status:  assigned
 Priority:  High |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  032-backport, tor-sched  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+

Comment (by arma):

 Quoting the oniongit comment here for ease of history:
 """
 When this function is called, the consensus ***has not yet changed***. It
 is still old_c. It will be new_c soon. This is your chance to take new
 actions.

 So if you ignore new_c and just call stuff that looks at "the consensus",
 you're going to be looking at the current consensus, i.e. old_c.

 I think this is a bug in 0.3.2 with
 scheduler_notify_networkstatus_changed() too, in that set_scheduler() can
 call select_scheduler() which calls scheduler_can_use_kist() which calls
 kist_scheduler_run_interval(NULL) -- so even though
 kist_scheduler_run_interval knows how to receive an ns as an argument, it
 doesn't get new_c 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] #24975 [Core Tor/Tor]: sched: scheduler_notify_networkstatus_changed() calls select_scheduler() without the new consensus

[Tor Bug Tracker & Wiki]

#24975: sched: scheduler_notify_networkstatus_changed() calls select_scheduler()
without the new consensus
-+
 Reporter:  dgoulet  |  Owner:  dgoulet
 Type:  defect   | Status:  assigned
 Priority:  High |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor |Version:
 Severity:  Normal   | Resolution:
 Keywords:  032-backport, tor-sched  |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+

Comment (by arma):

 I recommend if you want a function that gets called after the new
 consensus is in place (i.e. you don't care about what the old consensus
 used to say), move your call to the bottom of
 {{{networkstatus_set_current_consensus}}}.

--
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] #24975 [Core Tor/Tor]: sched: scheduler_notify_networkstatus_changed() calls select_scheduler() without the new consensus

[Tor Bug Tracker & Wiki]

#24975: sched: scheduler_notify_networkstatus_changed() calls select_scheduler()
without the new consensus
--+-
 Reporter:  dgoulet   |  Owner:  dgoulet
 Type:  defect| Status:  assigned
 Priority:  High  |  Milestone:  Tor: 0.3.3.x-final
Component:  Core Tor/Tor  |Version:
 Severity:  Normal|   Keywords:  032-backport, tor-sched
Actual Points:|  Parent ID:
   Points:|   Reviewer:
  Sponsor:|
--+-
 `select_scheduler()` is called when a new consensus arrives with
 `scheduler_notify_networkstatus_changed()` but never takes the new
 consensus for which at that point it is not yet set as the global
 consensus.

 It needs to use `new_c`.

 This is not good because of that, we can't change anything with the
 consensus at runtime.

 Thanks to Roger for finding this out here:
 https://oniongit.eu/dgoulet/tor/merge_requests/17#note_2153

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