Skip to content

Support new Shadow configuration formats#8

Merged
stevenengler merged 11 commits intoshadow:mainfrom
stevenengler:phantom
Jun 15, 2021
Merged

Support new Shadow configuration formats#8
stevenengler merged 11 commits intoshadow:mainfrom
stevenengler:phantom

Conversation

@stevenengler
Copy link
Copy Markdown
Contributor

No description provided.

@stevenengler stevenengler marked this pull request as ready for review June 15, 2021 19:00
@stevenengler stevenengler requested a review from robgjansen June 15, 2021 19:08
Copy link
Copy Markdown
Member

@robgjansen robgjansen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, looks good to me!

TOR_DIR_PORT=8080
TOR_GUARD_MIN_CONSBW=2000

PROC_ENV="OPENSSL_ia32cap=~0x200000200000000;EVENT_NOSELECT=1;EVENT_NOPOLL=1;EVENT_NOKQUEUE=1;EVENT_NODEVPOLL=1;EVENT_NOEVPORT=1;EVENT_NOWIN32=1"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't remember, but did we decide that Tor actually doesn't need any of these env variables to be set in order to work correctly? If so, should we simplify and remove them?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added them only because they were there originally. The OPENSSL_ia32cap=~0x200000200000000 to disable hardware AES might be useful once we have a preload that skips encryption.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we decided here that preloading the AES cipher does not actually improve performance, so it's easier to just not deal with the extra complexity of another preload library. Without a preload, I think we would want to enable hardware AES to make sure it's as efficient as possible, right?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I forgot about that. I've removed these environment variables.

@stevenengler stevenengler merged commit bfc257a into shadow:main Jun 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants