Opened 7 months ago

Closed 6 months ago

#32928 closed task (implemented)

Tor Manual: Split Circuit Timeout options into their own subsection

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.4.3.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: documentation tor-client manpage easy 043-can extra-review
Cc: catalyst, swati Actual Points:
Parent ID: #4310 Points: 0.5
Reviewer: catalyst, nickm Sponsor:

Description

Let's put the *Circuit*Timeout options in their own manpage section.

For context, see:
https://trac.torproject.org/projects/tor/ticket/32846#comment:11

Child Tickets

Change History (19)

comment:1 Changed 6 months ago by swati

Added a new section called Timeout Options. See PR - https://github.com/torproject/tor/pull/1702
Currently, there are 7 options in this section.

Some questions:

  • Is the Timeout Options an appropriate title for this category?
  • Do the following options also belong to the Timeout Options category?

MaxCircuitDirtiness
MaxClientCircuitsPending
CircuitPadding
ReducedCircuitPadding
NewCircuitPeriod
PathsNeededToBuildCircuits

If yes, I will have to add these too.

Last edited 6 months ago by swati (previous) (diff)

comment:2 Changed 6 months ago by swati

Status: newneeds_review

comment:3 Changed 6 months ago by swati

Reviewer: teor, catalyst

comment:4 Changed 6 months ago by catalyst

Keywords: extra-review added

Thanks!

It mostly looks good. I think it's better to have a title like "Circuit timeout options". (I think we don't currently have any timeout options other than circuit timeout options, but we might in the future.)

I'll try to comment more after the weekend.

comment:5 Changed 6 months ago by teor

Reviewer: teor, catalystcatalyst

Hi, I'm busy with Sponsor 55 right now, can you find someone else to review?

comment:6 in reply to:  4 ; Changed 6 months ago by swati

Thanks, Taylor. Regarding your comment about renaming the section to 'Circuit Timeout Options', I see that I have added DormantClientTimeout and DormantTimeoutDisabledByIdleStreams options to this section. Are these two circuit options as well?

Also, should I add the following circuit options to this section:?

MaxCircuitDirtiness
MaxClientCircuitsPending
CircuitPadding
ReducedCircuitPadding
NewCircuitPeriod
PathsNeededToBuildCircuits

Replying to catalyst:

Thanks!

It mostly looks good. I think it's better to have a title like "Circuit timeout options". (I think we don't currently have any timeout options other than circuit timeout options, but we might in the future.)

I'll try to comment more after the weekend.

comment:7 in reply to:  5 Changed 6 months ago by swati

Taylor, Could you please add any other reviewers?

Replying to teor:

Hi, I'm busy with Sponsor 55 right now, can you find someone else to review?

comment:8 in reply to:  6 ; Changed 6 months ago by catalyst

Replying to swati:

Thanks, Taylor. Regarding your comment about renaming the section to 'Circuit Timeout Options', I see that I have added DormantClientTimeout and DormantTimeoutDisabledByIdleStreams options to this section. Are these two circuit options as well?

I see. I think you're right there. They're client options that aren't directly related to circuit timeouts. They might belong in a separate section about Dormant mode?

Also, should I add the following circuit options to this section:?

Unfortunately, I think these are a little bit tricky to classify. Maybe we want to expand the section heading to include client circuit behavior overall? (The Nodes options have to do with the details of building a circuit, while many of these operations are at a higher level of abstraction that ignores individual nodes in a circuit.)

MaxCircuitDirtiness

I think this is not directly timing related? This is related to reusing a circuit for multiple application streams. I think there is a tradeoff between circuit reuse (for efficiency and decreased latency) and traffic analysis risk.

MaxClientCircuitsPending

I'm not quite sure how this one interacts with timing.

CircuitPadding
ReducedCircuitPadding

I think these aren't directly timing-related. These options are related to a countermeasure that we use against traffic analysis. There is a tradeoff because the countermeasure can increase bandwidth consumption. (I guess in situations of highly constrained bandwidth it could decrease performance in a user-visible way.)

NewCircuitPeriod
PathsNeededToBuildCircuits

These help tor decide when to build new circuits. PathsNeededToBuildCircuits is a bit more strongly related to the bootstrapping process than the other options here, so I'm not quite sure whether it belongs with the others.

comment:9 Changed 6 months ago by catalyst

Reviewer: catalystcatalyst, nickm

comment:10 in reply to:  8 ; Changed 6 months ago by swati

Replying to catalyst:

Replying to swati:

Thanks, Taylor. Regarding your comment about renaming the section to 'Circuit Timeout Options', I see that I have added DormantClientTimeout and DormantTimeoutDisabledByIdleStreams options to this section. Are these two circuit options as well?

I see. I think you're right there. They're client options that aren't directly related to circuit timeouts. They might belong in a separate section about Dormant mode?

Currently, there are only four Dormant mode options in the manual - DormantCanceledByStartup, DormantOnFirstStartup, DormantClientTimeout and DormantTimeoutDisabledByIdleStreams. Do you think we can have a separate section for dormant mode with just these four options? If yes, I'll put them out from here and create a new section.

Also, should I add the following circuit options to this section:?

Unfortunately, I think these are a little bit tricky to classify. Maybe we want to expand the section heading to include client circuit behavior overall? (The Nodes options have to do with the details of building a circuit, while many of these operations are at a higher level of abstraction that ignores individual nodes in a circuit.)

MaxCircuitDirtiness

I think this is not directly timing related? This is related to reusing a circuit for multiple application streams. I think there is a tradeoff between circuit reuse (for efficiency and decreased latency) and traffic analysis risk.

MaxClientCircuitsPending

I'm not quite sure how this one interacts with timing.

CircuitPadding
ReducedCircuitPadding

I think these aren't directly timing-related. These options are related to a countermeasure that we use against traffic analysis. There is a tradeoff because the countermeasure can increase bandwidth consumption. (I guess in situations of highly constrained bandwidth it could decrease performance in a user-visible way.)

NewCircuitPeriod
PathsNeededToBuildCircuits

These help tor decide when to build new circuits. PathsNeededToBuildCircuits is a bit more strongly related to the bootstrapping process than the other options here, so I'm not quite sure whether it belongs with the others.

Last edited 6 months ago by swati (previous) (diff)

comment:11 in reply to:  8 Changed 6 months ago by nickm

Replying to catalyst:

Replying to swati:

Thanks, Taylor. Regarding your comment about renaming the section to 'Circuit Timeout Options', I see that I have added DormantClientTimeout and DormantTimeoutDisabledByIdleStreams options to this section. Are these two circuit options as well?

I see. I think you're right there. They're client options that aren't directly related to circuit timeouts. They might belong in a separate section about Dormant mode?

[...]

For what it's worth, I agree with catalyst in their assessment of the options discussed in the comment above.

comment:12 in reply to:  10 Changed 6 months ago by nickm

Replying to swati:

Replying to catalyst:

Replying to swati:

Thanks, Taylor. Regarding your comment about renaming the section to 'Circuit Timeout Options', I see that I have added DormantClientTimeout and DormantTimeoutDisabledByIdleStreams options to this section. Are these two circuit options as well?

I see. I think you're right there. They're client options that aren't directly related to circuit timeouts. They might belong in a separate section about Dormant mode?

Currently, there are only four Dormant mode options in the manual - DormantCanceledByStartup, DormantOnFirstStartup, DormantClientTimeout and DormantTimeoutDisabledByIdleStreams. Do you think we can have a separate section for dormant mode with just these four options? If yes, I'll put them out from here and create a new section.

I think that's a fine idea.

comment:13 Changed 6 months ago by swati

Hey nickm and catalyst,

  • I added a new section called Dormant Mode Options and moved the four options under this section. Does anyone want to suggest a better introductory sentence for this section?
  • Also, I renamed the Timeout Options section to Circuit Timeout Options, as suggested by catalyst. I haven't moved the other options that aren't directly related to timeout under this section. Leaving them where they are for now. Is that okay?
  • Can we consider this ticket resolved? If yes, can we merge the PR so that I can continue working on the rest of the manual?

Thank you,
Swati

Last edited 6 months ago by swati (previous) (diff)

comment:14 Changed 6 months ago by nickm

This looks good to me; I say we should merge if catalyst is okay with it.

comment:15 in reply to:  13 ; Changed 6 months ago by catalyst

Replying to swati:

Thanks for the changes!

  • I added a new section called Dormant Mode Options and moved the four options under this section. Does anyone want to suggest a better introductory sentence for this section?

Dormant mode is a state that tor can enter and leave, and the options in this section modify that behavior. Maybe this?

"Tor can enter a dormant mode to conserve power and network bandwidth. The following options control when tor enters and leaves dormant mode."

  • Also, I renamed the Timeout Options section to Circuit Timeout Options, as suggested by catalyst. I haven't moved the other options that aren't directly related to timeout under this section. Leaving them where they are for now. Is that okay?

I think that's fine. I couldn't come up with a concise heading for an expanded section that would contain the other options you were considering. I would suggest rewording the introductory paragraph to something like this?

"The following options are useful for configuring timeouts related to building Tor circuits and using them."

  • Can we consider this ticket resolved? If yes, can we merge the PR so that I can continue working on the rest of the manual?

I think we're mostly there. There are a couple of minor wording changes I suggested above for the introductory text of the new sections. If you agree, please go ahead and make those changes.

comment:16 in reply to:  15 ; Changed 6 months ago by swati

Replying to catalyst:

Thanks for the changes!

  • I added a new section called Dormant Mode Options and moved the four options under this section. Does anyone want to suggest a better introductory sentence for this section?

Dormant mode is a state that tor can enter and leave, and the options in this section modify that behavior. Maybe this?

"Tor can enter a dormant mode to conserve power and network bandwidth. The following options control when tor enters and leaves dormant mode."

  • Also, I renamed the Timeout Options section to Circuit Timeout Options, as suggested by catalyst. I haven't moved the other options that aren't directly related to timeout under this section. Leaving them where they are for now. Is that okay?

I think that's fine. I couldn't come up with a concise heading for an expanded section that would contain the other options you were considering. I would suggest rewording the introductory paragraph to something like this?

"The following options are useful for configuring timeouts related to building Tor circuits and using them."

  • Can we consider this ticket resolved? If yes, can we merge the PR so that I can continue working on the rest of the manual?

I think we're mostly there. There are a couple of minor wording changes I suggested above for the introductory text of the new sections. If you agree, please go ahead and make those changes.

Reworded your suggestion a bit and updated the introductory text in both the sections.

comment:17 in reply to:  16 Changed 6 months ago by catalyst

Replying to swati:

Reworded your suggestion a bit and updated the introductory text in both the sections.

Thanks! It looks good. There needs to be a changes file, but I can write one and push it to your branch if that's ok.

comment:18 Changed 6 months ago by catalyst

Status: needs_reviewmerge_ready

Note to merger: please merge https://github.com/torproject/tor/pull/1711 instead; it adds a changes file. Thanks!

Swati, I think that should auto-close your pull request if it's merged without additional changes or rebasing.

comment:19 Changed 6 months ago by nickm

Resolution: implemented
Status: merge_readyclosed

Merged!

Note: See TracTickets for help on using tickets.