Opened 7 months ago

Closed 5 months ago

Last modified 5 months ago

#21662 closed task (implemented)

prop278: Add support for LZMA2 and/or Zstandard

Reported by: ahf Owned by: ahf
Priority: Medium Milestone: Tor: 0.3.1.x-final
Component: Core Tor/Tor Version:
Severity: Normal Keywords: TorCoreTeam201703, prop278
Cc: Actual Points:
Parent ID: Points: 2
Reviewer: Sponsor: Sponsor4

Description

Add support for the compression schemes needed to implement prop#278.

See: http://facebook.github.io/zstd/ and http://7-zip.org/sdk.html for the respective libraries.

Child Tickets

Change History (13)

comment:1 Changed 7 months ago by ahf

Owner: set to ahf
Status: newaccepted

comment:2 Changed 7 months ago by ahf

Type: defecttask

comment:3 Changed 6 months ago by ahf

Keywords: prop278 added

Add prop278 keyword.

comment:4 Changed 5 months ago by ahf

Starting to track the cleaned up commits in: https://gitlab.com/ahf/tor/merge_requests/2

Currently LZMA support is available - Zstandard coming up.

Note that this code depends upon the code that is currently being reviewed as part of #21663.

comment:5 Changed 5 months ago by ahf

Status: acceptedneeds_review

comment:6 Changed 5 months ago by nickm

Status: needs_reviewneeds_revision

Looks good! I've made some initial comments and asked some questions. Also, I should take another pass or two over this code when I'm a little less tired, and think about the pointer arithmetic very carefully.

comment:7 Changed 5 months ago by ahf

Status: needs_revisionneeds_review

I believe I've been over everything you have commented on in the reviews now. We've postponed merging the test functions into a simplified function that tests all backends and we have postponed doing coverage "hints" in the code.

I've added a clean-up patch of some of the Zstandard code with regards to flushing the internal buffer in a more eager manner.

Let me know if this looks acceptable for merge and I'll happily do the squashing as well (whatever you find easiest!)

Note that this is build on the comments already made in both https://gitlab.com/ahf/tor/merge_requests/1 and https://gitlab.com/ahf/tor/merge_requests/2 - Gitlab seems to hide that a bit.

Once this code lands we can close both #21662, #21663 and #21664.

comment:8 Changed 5 months ago by nickm

Merging!

comment:9 Changed 5 months ago by nickm

Resolution: implemented
Status: needs_reviewclosed

comment:10 Changed 5 months ago by atagar

Neat! Friendly reminder that this needs to be added to the dir-spec or it doesn't exist. ;)

comment:11 Changed 5 months ago by nickm

atagar: It isn't actually used yet; this ticket is just for the backend code.

comment:12 Changed 5 months ago by atagar

Ahhh, gotcha. So just to be clear: it's not actually being exposed on the DirPort yet?

comment:13 Changed 5 months ago by ahf

That's correct.

Note: See TracTickets for help on using tickets.