Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#7037 closed defect (fixed)

When we refuse a create cell due to onion queue length, we should say END_CIRC_REASON_RESOURCELIMIT

Reported by: arma Owned by:
Priority: Medium Milestone: Tor: 0.2.3.x-final
Component: Core Tor/Tor Version:
Severity: Keywords: tor-relay
Cc: Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Right now in command_process_create_cell() when assign_onionskin_to_cpuworker() fails, we close the circuit with

      circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_INTERNAL);

But in onion_pending_add() when we've got old onions, we close the circuit with

    circuit_mark_for_close(TO_CIRCUIT(circ), END_CIRC_REASON_RESOURCELIMIT);

Since the primary reason assign_onionskin_to_cpuworker() fails is because onion_pending_add() hits the "ol_length >= get_options()->MaxOnionsPending" clause, I think we should change command_process_create_cell() to claim RESOURCELIMIT rather than INTERNAL in its destroy cell.

And I think we should do this in 0.2.3 so the fix gets out in time to make a difference for the measurement scripts Mike has been playing with.

Child Tickets

Change History (5)

comment:1 Changed 7 years ago by arma

Status: newneeds_review

bug7037 in my arma

comment:2 Changed 7 years ago by nickm

are you sure you pushed it? I don't see any arma/bug7037.

comment:3 Changed 7 years ago by arma

now it's there (i think)

comment:4 Changed 7 years ago by nickm

Resolution: fixed
Status: needs_reviewclosed

Okay, I would want to make sure that clients react to END_CIRC_REASON_RESOURCELIMIT in the same way they react to END_CIRC_REASON_INTERNAL. I've looked over the code grepping for END_CIRC_REASON, and as near as I can tell, that is indeed the case.

Merged to 0.2.3 and forward.

comment:5 Changed 7 years ago by mikeperry

For reference, the client side code change to pass this reason code to the control port is #7039.

Note: See TracTickets for help on using tickets.