Opened 8 months ago

Last modified 6 weeks ago

#24732 needs_revision enhancement

Remove unused IPv6 DirPort code

Reported by: teor Owned by:
Priority: Medium Milestone: Tor: 0.3.5.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: ipv6, tor-relay, 034-triage-20180328, 035-triaged-in-20180711
Cc: Actual Points:
Parent ID: #24403 Points: 1
Reviewer: ahf Sponsor: SponsorV-can


IPv6 DirPorts aren't used by Tor: clients use IPv4/IPv6 ORPorts, and relays use IPv4 DirPorts (and IPv4 ORPorts).

There is code that implements this algorithm in commit 36ba50c820 of my bug23975_tree branch.

Child Tickets

Change History (19)

comment:1 Changed 8 months ago by teor

This would also be a good time to get rid of other unused address functions, like:

  • fascist_firewall_choose_address_dir_server

comment:2 Changed 7 months ago by teor

Milestone: Tor: 0.3.3.x-finalTor: 0.3.4.x-final

The 0.3.3 freeze deadline has passed, all these children of #24403 belong in 0.3.4

comment:3 Changed 6 months ago by meryemz

I have my first change here concerning removing the unused function: fascist_firewall_choose_address_dir_server()

And i made the necessary tests.

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

comment:4 Changed 6 months ago by teor

Reviewer: teor

Since the original code is mine, I guess I should review this.

But I wonder if you looked at commit 36ba50c820 of my bug23975_tree branch on ?
Because it removes a lot of the other functions, so there may be conflicts,

comment:5 Changed 6 months ago by meryemz

Yes I did, I don't think there will be any conflicts since the function I removed is not related to those you removed.

comment:6 Changed 6 months ago by teor

Status: newneeds_revision

Someone needs to combine both commits in this ticket, and check that they work together.

comment:7 Changed 5 months ago by meryemz

I combined both modifications in :

Please have a look at it.

comment:8 Changed 5 months ago by teor

Status: needs_revisionneeds_review

Thanks! I've marked this as needing review.

The first thing that someone will do is run:

make check
make test-network-all

You can do them yourself if you like. Please let us know if either of them fails.

You will need chutney for the second test:

comment:9 Changed 5 months ago by meryemz

I ran the make check and got a total of 19 tests with 16 Pass and 3 skip and no FAIL OR ERRORE but i get the error of make check-spaces at the end.
As for the chutney test it works fine.

comment:10 Changed 5 months ago by teor

Please fix the make check-spaces errors, and let us know when that's done.

comment:11 Changed 5 months ago by meryemz

I have corrected the make check-spaces errors and ran again the chutney test, everything works fine.
Here is the last commit:

comment:12 Changed 5 months ago by teor

Thanks! A bunch of us are at a meeting this week, so it might be a few weeks before your patch gets reviewed.

comment:13 Changed 5 months ago by nickm

Keywords: 034-triage-20180328 added

comment:14 Changed 3 months ago by ahf

Reviewer: teorahf

comment:16 Changed 3 months ago by ahf

Status: needs_reviewmerge_ready

LGTM! Maybe Teor still wants to have a round of review of this?

comment:17 Changed 3 months ago by dgoulet

Status: merge_readyneeds_revision

This commit bf40049c77160afe seems to only be formatting weirdly some part of the code... I think without it, everything should be good?

Also @meryemz, you might want to set a valid email address (if you want) to your git configuration with:

git config --global ""

... because right now it is: Meryem Zeroual <meryem@localhost>.

comment:18 Changed 3 months ago by nickm

Milestone: Tor: 0.3.4.x-finalTor: 0.3.5.x-final

Move most needs_revision tickets from 0.3.4 to 0.3.5: we're about to hit the feature freeze.

If you really need to get this into 0.3.4, please drop me a line. :)

comment:19 Changed 6 weeks ago by nickm

Keywords: 035-triaged-in-20180711 added
Note: See TracTickets for help on using tickets.