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

Fix docs crypto_policy_* -> srtp_crypto_policy_* #457

Merged
merged 1 commit into from Jul 12, 2019
Merged

Fix docs crypto_policy_* -> srtp_crypto_policy_* #457

merged 1 commit into from Jul 12, 2019

Conversation

Sean-Der
Copy link
Contributor

No description provided.

@Sean-Der
Copy link
Contributor Author

Just a doc change!

I am also writing a benchmark to compare protection profile performance (to encourage AEAD GCM usage in WebRTC). Does anyone have any existing numbers (or programs) to quickly compare! I am hoping that the hw-acceleration from using OpenSSL makes a significant improvement :)

thanks!

Copy link
Member

@pabuhler pabuhler left a comment

Choose a reason for hiding this comment

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

Thanks for the PR, I have approved it but will leave it unmerged for a bit in case others would like to reply to your questions. Personal I do not have any numbers on hand. But it would be interesting if you find some thing.

README.md Outdated
@@ -138,16 +138,16 @@ can also be linked together to form an entire session policy. A linked
list of `srtp_policy_t` structures is equivalent to a session policy.
In such a policy, we refer to a single `srtp_policy_t` as an *element*.

An `srtp_policy_t` strucutre contains two `crypto_policy_t` structures
An `srtp_policy_t` strucutre contains two `srtp_crypto_policy_t` structures
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you also change the typo here, while you're at it? strucutre -> structure
Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed!

@Sean-Der
Copy link
Contributor Author

Sean-Der commented Jul 11, 2019

@pabuhler I did! Here are my results

-- aes_cm_128_hmac_sha1_32
./a.out  1.83s user 0.01s system 99% cpu 1.843 total
./a.out  1.84s user 0.01s system 99% cpu 1.856 total
./a.out  1.83s user 0.00s system 99% cpu 1.837 total
./a.out  1.82s user 0.01s system 99% cpu 1.831 total
./a.out  2.04s user 0.01s system 99% cpu 2.065 total
./a.out  1.82s user 0.01s system 99% cpu 1.834 total
./a.out  1.84s user 0.01s system 99% cpu 1.857 total
./a.out  1.83s user 0.00s system 99% cpu 1.843 total
./a.out  1.85s user 0.01s system 99% cpu 1.860 total
./a.out  1.83s user 0.01s system 99% cpu 1.839 total

-- aes_gcm_128_16
./a.out  0.76s user 0.00s system 99% cpu 0.772 total
./a.out  0.81s user 0.00s system 99% cpu 0.814 total
./a.out  0.75s user 0.00s system 99% cpu 0.762 total
./a.out  0.75s user 0.01s system 99% cpu 0.760 total
./a.out  0.76s user 0.00s system 99% cpu 0.765 total
./a.out  0.76s user 0.00s system 99% cpu 0.765 total
./a.out  0.79s user 0.01s system 99% cpu 0.803 total
./a.out  0.77s user 0.01s system 99% cpu 0.780 total
./a.out  0.75s user 0.00s system 99% cpu 0.762 total
./a.out  0.79s user 0.01s system 99% cpu 0.796 total

Hopefully the test isn't bad either, just pulled it together quickly. https://gist.github.com/Sean-Der/7a42bd70edfe1324ccc6ab399d653c0e

If you are interested the conversation is happening on rtcweb now, would love your input/support there if interested :)

@pabuhler
Copy link
Member

@Sean-Der interesting numbers. The only thing I would do different is to use aes_cm_128_hmac_sha1_80 instead of aes_cm_128_hmac_sha1_32 as I feel that is the most common cm cipher in use and then try with data packets of 1024 bytes to see if that makes a different.
I will merge your change now which will effectively close this PR and this discussion.

@pabuhler pabuhler merged commit e95cf69 into cisco:master Jul 12, 2019
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.

None yet

3 participants