Add an option that makes Tor use fewer connections
Tor is willing all the file descriptors the OS tells it are available for the current user. So to enforce a connection limit, operators need to drop the user file descriptor limit. But if the OS is wrong, or there are multiple tor instances under the user, tor can easily exceed this limit. Or machine hits a kernel, RAM, or CPU limit first, then the user might want to artificially limit connections.
Right now, we have ConnLimit, which looks like it limits connections, but it's actually a minimum. And we have the out of socket check, but DisableOOSCheck is the default.
Does enforcing a connection limit require us to set DisableOOSCheck 0? Then we should move this ticket to 0.3.4, and open one to improve the out of socket check.
Activity
changed milestone to %Tor: unspecified
Trac:
Cc: N/A to neel@neelc.org
Status: new to assigned
Owner: N/A to neelReplying to arma:
This one is tricky, because if we have such an option, and a Tor relay sets it and hits the limit, then the relay will start behaving badly.
We really need our relays to never hit their limit. Anything else is bad.
Thank You.
I noticed this function in
src/common/compat.c:int set_max_file_descriptors(rlim_t limit, int *max_out)Should I be working with the function above? I am thinking about an argument to this function with the total maximum number of connections Tor will take (and using that value as an alternative to the maximum number of file descriptors) which will in turn be used to set
max_socketsinsrc/common/compat.cand theConnLimit_option from the call toset_max_file_descriptors()inconfig.c. The argument will be based on an option I will add (if I work on this patch). If this option is0or left unset, it will default to using the number of file descriptors like Tor does right now.Don't change set_max_file_descriptors(), it works well, and does exactly what arma says: gets the most file descriptors from the system.
Instead, look at the out of socket check code. It is enabled by "DisableOOSCheck 0". It uses the socket limit that set_max_file_descriptors() discovers from the system. But you could add a torrc option that allows the user to set a lower limit for the out of socket check. It would only work if the out of socket check was enabled.
Then the tor process could have the maximum number of sockets available, but choose not to use them. (For example, during the extra client load, I would have set the socket limit to half the number of file descriptors, so that the connection closing logic ran less frequently.)
Since I opened this ticket, we also added DoSConnectionMaxConcurrentCount. Maybe that makes this ticket less important? But it would still be good for operators who run all their tor instances under the same user.
I am still interested in this ticket.
I noticed the following options in
connection_check_oos():get_options()->ConnLimit_high_thresh get_options()->ConnLimit_ get_options()->ConnLimit_low_threshIt seems that the threshold is determined from these values. Should I modify these values directly should my proposed option (connection limit) be set, and calculates a threshold based on the set value rather than the number of file descriptors?
Or should I add new values to the
or_options_tstructure, fill them if my proposed option is set, and makeconnection_check_oos()use the new options instead if the proposed option is set.I prefer the former as it is far simpler, since the latter adds some complexity but the latter is also less destructive.
Which approach do you prefer?
Replying to neel:
I am still interested in this ticket.
I noticed the following options in
connection_check_oos():{{{ get_options()->ConnLimit_high_thresh get_options()->ConnLimit_ get_options()->ConnLimit_low_thresh }}}
It seems that the threshold is determined from these values. Should I modify these values directly should my proposed option (connection limit) be set, and calculates a threshold based on the set value rather than the number of file descriptors? …
If you add another torrc option MaxSockets, then use min(MaxSockets, ConnLimit_) instead of ConnLimit_, all the rest of the code should just work.
Using min(MaxSockets, ConnLimit_) allows us to override the OS when its limit is too high, and it makes sure we don't ever go over the OS limit.
Trac:
b24797-001.patchHi,
The MaxSockets default in the man page is incorrect:
(Default: 1000)It should say "Default: unlimited" or something similar.
I don't think we should fail if ConnLimit_ is lower than MaxSockets: why not just log a notice message, and use ConnLimit_? MaxSockets is a maximum, not a minimum requirement.
Please update the options unit tests to test the new option, and its interaction with DisableOOSCheck.
Trac:
Status: assigned to needs_review
Replying to teor:
I don't think we should fail if ConnLimit_ is lower than MaxSockets: why not just log a notice message, and use ConnLimit_?
Use ConnLimit for what? As the min or as the max?
I'm still thinking we shouldn't add this feature. We periodically have people who want this sort of thing, and if they set it and it gets triggered, they are being a bad relay. Why are we offering them a way to be a bad relay?
Replying to arma:
I'm still thinking we shouldn't add this feature. We periodically have people who want this sort of thing, and if they set it and it gets triggered, they are being a bad relay. Why are we offering them a way to be a bad relay?
What do you mean by being a 'bad relay'? I am talking about a maximum number of sockets should a relay operator desire it (I am sorry if my patch does not make it too clear, it's my ADHD).
I am thinking that for people who run Tor relays on servers with other applications (web, email, etc.), or run multiple Tor instances, this could help with resource limitation without requiring virtualization (VMWare, Xen, etc.) or containerization (Docker, FreeBSD Jails, etc.). But I could understand your concern with this option if you could give an explanation.
Replying to teor:
Hi,
The MaxSockets default in the man page is incorrect: {{{ (Default: 1000) }}} It should say "Default: unlimited" or something similar.
I don't think we should fail if ConnLimit_ is lower than MaxSockets: why not just log a notice message, and use ConnLimit_? MaxSockets is a maximum, not a minimum requirement.
Please update the options unit tests to test the new option, and its interaction with DisableOOSCheck.
Replying to arma:
Replying to teor:
I don't think we should fail if ConnLimit_ is lower than MaxSockets: why not just log a notice message, and use ConnLimit_?
Use ConnLimit for what? As the min or as the max?
ConnLimit_(underscore) is the maximum number of file descriptors discovered from the OS. Tor doesn't go above this limit: it is a maximum.ConnLimit(no underscore) is the minimum required value ofConnLimit_for tor to start.So I suggest replacing
ConnLimit_withmin(MaxSockets, ConnLimit_):If you add another torrc option MaxSockets, then use min(MaxSockets, ConnLimit_) instead of ConnLimit_, all the rest of the code should just work.
Using min(MaxSockets, ConnLimit_) allows us to override the OS when its limit is too high, and it makes sure we don't ever go over the OS limit.
I'm still thinking we shouldn't add this feature. We periodically have people who want this sort of thing, and if they set it and it gets triggered, they are being a bad relay. Why are we offering them a way to be a bad relay?
Because they're about to hit their limit anyway, and they want to fail slightly less awfully using
DisableOOSCheck 0.Some people want to be able to access their relays after tor hits its socket limit (#28367 (moved)). And as neel said, others can't set ulimit on their machines.
Trac:
Cc: neel@neelc.org to neel@neelc.org, starlight@binnacle.cxyou can also use ReducedConnectionPadding 1
ReducedConnectionPadding 0|1
If set to 1, Tor will not not hold OR connections open for very long,
and will send less padding on these connections. Only clients may set
this option. This option should be offered via the UI to mobile users
for use where bandwidth may be expensive. (Default: 0)
Trac:
Cc: neel@neelc.org, starlight@binnacle.cx to starlight@binnacle.cx
Owner: neel to N/A
Status: needs_revision to assignedReplying to cypherpunks:
you can also use ReducedConnectionPadding 1
ReducedConnectionPadding 0|1
If set to 1, Tor will not not hold OR connections open for very long,
and will send less padding on these connections. Only clients may set
this option. This option should be offered via the UI to mobile users
for use where bandwidth may be expensive. (Default: 0)
ReducedConnectionPadding reduces the amount of data sent on each connection. This ticket is about reducing the number of connections.
mentioned in issue #28367 (moved)
moved to tpo/core/tor#24797 (closed)
