Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/9032 )
Change subject: IMPALA-6219: Use AES-GCM for spill-to-disk encryption ...................................................................... Patch Set 6: Code-Review+1 (3 comments) Thanks for applying these changes! http://gerrit.cloudera.org:8080/#/c/9032/6/be/src/util/openssl-util.h File be/src/util/openssl-util.h: http://gerrit.cloudera.org:8080/#/c/9032/6/be/src/util/openssl-util.h@140 PS6, Line 140: Return the a Returns the http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util.cc File be/src/util/openssl-util.cc: http://gerrit.cloudera.org:8080/#/c/9032/4/be/src/util/openssl-util.cc@173 PS4, Line 173: LOG(WARNING) << "This mode is not supported, fall back to the default mode."; : mode_ = GetSupportedDefaultMode(); : } : } : : bool EncryptionKey::IsModeSupported(AES_CIPHER_MODE m) const { : switch (m) { : case AES_256_GCM: : return (CpuInfo::IsSupported(CpuInfo::PCLMULQDQ) : && SSLeay() >= OPENSSL_VERSION_1_0_1D && EVP_aes_256_gcm); : : case AES_256_CTR: : > good suggestion. Thanks a lot! duplicated logic is refactored. "falling cha Yeah, I think this is reasonable. Thanks for applying the changes. http://gerrit.cloudera.org:8080/#/c/9032/6/be/src/util/openssl-util.cc File be/src/util/openssl-util.cc: http://gerrit.cloudera.org:8080/#/c/9032/6/be/src/util/openssl-util.cc@173 PS6, Line 173: LOG(WARNING) << "This mode is not supported, fall back to the default mode."; Maybe you could create a 'string ToString(enum AES_CIPHER_MODE)' helper function and output the not supported mode and the default mode as well. For this you need string formatting, by convention we use the Substitute() function for that in Impala. See example in L59. -- To view, visit http://gerrit.cloudera.org:8080/9032 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1ea87b82a8897ee8bfa187715ac1c52883790d24 Gerrit-Change-Number: 9032 Gerrit-PatchSet: 6 Gerrit-Owner: Xianda Ke <[email protected]> Gerrit-Reviewer: Sailesh Mukil <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Reviewer: Xianda Ke <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Mon, 22 Jan 2018 12:46:54 +0000 Gerrit-HasComments: Yes
