Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Ticket33029 043 02 #1691

Closed
wants to merge 9 commits into from
Closed

Ticket33029 043 02 #1691

wants to merge 9 commits into from

Conversation

Reviewers

@teor2345 teor2345

@armadev armadev

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
5 participants
@dgoulet-tor @coveralls @armadev @teor2345 @torproject-pusher
@dgoulet-tor
Copy link
Contributor

@dgoulet-tor dgoulet-tor commented Jan 28, 2020

No description provided.

@coveralls
Copy link

@coveralls coveralls commented Jan 28, 2020 •
edited

Pull Request Test Coverage Report for Build 7922

  • 39 of 44 (88.64%) changed or added relevant lines in 5 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.04%) to 63.419%
Changes Missing Coverage Covered Lines Changed/Added Lines %
src/core/mainloop/connection.c 5 6 83.33%
src/feature/dircache/dircache.c 5 6 83.33%
src/feature/nodelist/nodelist.c 14 17 82.35%
Totals Coverage Status
Change from base Build 7892: 0.04%
Covered Lines: 50031
Relevant Lines: 78890
💛 - Coveralls

armadev
armadev suggested changes Jan 28, 2020
View changes
Copy link

@armadev armadev left a comment

In the commit message, s/seperate/separate/ and s/addresse/address/

@dgoulet-tor dgoulet-tor force-pushed the ticket33029_043_02 branch 2 times, most recently from 1475457 to 225dedb Jan 28, 2020
teor2345
teor2345 reviewed Jan 29, 2020
View changes
src/feature/nodelist/nodelist.c
!the_nodelist || !the_nodelist->node_addrs) {
return;
}
address_set_add(the_nodelist->node_addrs, addr);
Copy link
Contributor

@teor2345 teor2345 Jan 29, 2020

Your code will be slightly shorter if you do this check here:

Suggested change
address_set_add(the_nodelist->node_addrs, addr);
if (!tor_addr_is_null(addr))
address_set_add(the_nodelist->node_addrs, addr);

teor2345
teor2345 reviewed Jan 29, 2020
View changes
src/feature/nodelist/nodelist.c
if (!the_nodelist || !the_nodelist->node_addrs) {
return;
}
address_set_add_ipv4h(the_nodelist->node_addrs, addr);
Copy link
Contributor

@teor2345 teor2345 Jan 29, 2020

Your code will be slightly shorter if you do this check here:

Suggested change
address_set_add_ipv4h(the_nodelist->node_addrs, addr);
if (addr)
address_set_add_ipv4h(the_nodelist->node_addrs, addr);

@dgoulet-tor dgoulet-tor force-pushed the ticket33029_043_02 branch 2 times, most recently from cd539df to 8373232 Jan 29, 2020
dgoulet-tor added 8 commits Jan 30, 2020
@dgoulet-tor
mainloop: Rename global_write_bucket_low()
774c71b 
That function is only used to test the global bucket write limit for a
directory connection.

It should _not_ be used for anything else since that function looks to see if
we are a directory authority.

Rename it to something more meaningful. No change in behavior at this commit,
only renaming.

Part of #33029

Signed-off-by: David Goulet <dgoulet@torproject.org>
@dgoulet-tor
mainloop: Remove unused parameter from connection_dir_is_global_write…
cb6ec9d 
…_low()

Signed-off-by: David Goulet <dgoulet@torproject.org>
@dgoulet-tor
mainloop: Modernize a bit connection_dir_is_global_write_low()
8a5ccee 
Signed-off-by: David Goulet <dgoulet@torproject.org>
@dgoulet-tor
nodelist: Helper to add an address to the nodelist address set
32758ec 
We separate v4 and v6 because we often use an IPv4 address represented with
a uint32_t instead of a tor_addr_t.

This will be used to also add the trusted directory addresses taken from the
configuration.

The trusted directories from the consensus are already added to the address
set from their descriptor.

Signed-off-by: David Goulet <dgoulet@torproject.org>
@dgoulet-tor
dirlist: Add configured trusted dir to the nodelist address set
061074d 
The configured, within the torrc or hardcoded, directory authorities addresses
are now added to the nodelist address set.

Signed-off-by: David Goulet <dgoulet@torproject.org>
@dgoulet-tor
dirauth: Resume sending 503 directory error code
180fad7 
Authorities were never sending back 503 error code because by design they
should be able to always answer directory requests regardless of bandwidth
capacity.

However, that recently backfired because of a large number of requests from
unknown source using the DirPort that are _not_ getting their 503 code which
overloaded the DirPort leading to the authority to be unable to answer to its
fellow authorities.

This is not a complete solution to the problem but it will help ease off the
load on the authority side by sending back 503 codes *unless* the connection
is from a known relay or an authority.

Fixes #33029

Signed-off-by: David Goulet <dgoulet@torproject.org>
@dgoulet-tor
dirauth: Add option AuthDirRejectRequestsUnderLoad
3d5f622 
This controls the previous feature added that makes dirauth send back a 503
error code on non relay connections if under bandwidth pressure.

Signed-off-by: David Goulet <dgoulet@torproject.org>
@dgoulet-tor
test: Add unit test for connection_dir_is_global_write_low()
b9ec1b4 
Part of #33029

Signed-off-by: David Goulet <dgoulet@torproject.org>
@dgoulet-tor dgoulet-tor force-pushed the ticket33029_043_02 branch from f48c9d8 to b9ec1b4 Jan 30, 2020
@dgoulet-tor
dirauth: Rename function for better clarity
14c8b66 
Signed-off-by: David Goulet <dgoulet@torproject.org>
@torproject-pusher torproject-pusher deleted the branch torproject:master May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment