Opened 3 years ago
Closed 2 years ago
#19024 closed enhancement (fixed)
prop224: Refactor rend_data_t so be able to use multiple HS version
Reported by: | dgoulet | Owned by: | dgoulet |
---|---|---|---|
Priority: | Medium | Milestone: | Tor: 0.3.0.x-final |
Component: | Core Tor/Tor | Version: | |
Severity: | Normal | Keywords: | tor-hs, prop224, TorCoreTeam201609, nickm-deferred-20161005 |
Cc: | Actual Points: | ||
Parent ID: | #17238 | Points: | 4 |
Reviewer: | asn | Sponsor: | SponsorR-must |
Description
Break rend_data_t
into something that could looks like this (or maybe without a union...):
struct rend_data_t { uint32_t version; /* XXX: Maybe not necessary if our code flow doesn't * require us to learn the version through that data struct. */ union { hs_data_v2_t v2; hs_data_v3_t v3; } hs; };
Once we have such construction, we can use v3 with that data structure more cleanly.
Child Tickets
Change History (20)
comment:1 Changed 3 years ago by
Points: | → 1 |
---|---|
Status: | new → accepted |
comment:2 follow-up: 3 Changed 3 years ago by
Points: | 1 → 4 |
---|---|
Status: | accepted → needs_review |
comment:3 follow-up: 6 Changed 3 years ago by
Replying to dgoulet:
This is the _first_ building block for the entire HSDir support. I'm submitting a request for review so if we are satisfied with it, it will be the first commit of the upcoming branch for the entire implementation (#17238). It might change slightly as we add more commits but the bulk of the work is here:
I did an initial review here just to keep things rolling.
I will do more review here in the future.
comment:4 Changed 3 years ago by
Reviewer: | → asn |
---|
comment:5 Changed 3 years ago by
Keywords: | review-group-6 added |
---|
comment:6 follow-up: 8 Changed 3 years ago by
Replying to asn:
Replying to dgoulet:
I did an initial review here just to keep things rolling.
I will do more review here in the future.
Thanks for this. I've addressed your comments. See 3 new commits on top I pushed to the merge request. In particular, the addition of a new file for rend_data_t
stuff.
comment:7 Changed 3 years ago by
Keywords: | review-group-6 removed |
---|
comment:8 Changed 3 years ago by
Replying to dgoulet:
Replying to asn:
Replying to dgoulet:
I did an initial review here just to keep things rolling.
I will do more review here in the future.
Thanks for this. I've addressed your comments. See 3 new commits on top I pushed to the merge request. In particular, the addition of a new file for
rend_data_t
stuff.
Great stuff. I did another review on that same merge request. The merge request has been polluted with various unrelated changes from master, but hopefully it's still readable.
Code looks good here! I guess the next step here would be testing it with the current HS system.
comment:9 Changed 3 years ago by
Keywords: | TorCoreTeam201608 added |
---|
comment:10 Changed 3 years ago by
Keywords: | review-group-7 added |
---|
comment:11 Changed 3 years ago by
Keywords: | review-group-7 removed |
---|
Removing this from the review group because this ticket patch is part of the big #17238.
comment:12 Changed 2 years ago by
Keywords: | TorCoreTeam201609 added; TorCoreTeam201608 removed |
---|
Review and merge will happen in September.
comment:13 Changed 2 years ago by
Keywords: | review-group-8 added |
---|
comment:15 Changed 2 years ago by
Keywords: | review-group-9 added; review-group-8 removed |
---|
comment:16 Changed 2 years ago by
Keywords: | nickm-deferred-20161005 added |
---|---|
Milestone: | Tor: 0.2.9.x-final → Tor: 0.3.0.x-final |
Deferring big/risky-feature things (even the ones I really love!) to 0.3.0. Please argue if I'm wrong.
comment:17 Changed 2 years ago by
These have sat in needs_revision for a few weeks at least. Removing from review-group-9, not adding to review-group-10.
comment:18 Changed 2 years ago by
Status: | needs_revision → accepted |
---|
The implementation is in parent ticket which is being reviewed so putting this one in Accepted
state and once parent is merged, we can simply close.
comment:19 Changed 2 years ago by
Keywords: | review-group-9 removed |
---|
comment:20 Changed 2 years ago by
Resolution: | → fixed |
---|---|
Status: | accepted → closed |
Merged as part of #17238 in commit 8293356a
.
This is the _first_ building block for the entire HSDir support. I'm submitting a request for review so if we are satisfied with it, it will be the first commit of the upcoming branch for the entire implementation (#17238). It might change slightly as we add more commits but the bulk of the work is here:
https://gitlab.com/dgoulet/tor/merge_requests/8