From d68bb2ab3cc9715731dd380b69d85d1afefc2956 Mon Sep 17 00:00:00 2001
From: Ngo The Trung <tngo@zendesk.com>
Date: Thu, 17 Aug 2017 03:21:18 +0000
Subject: [PATCH 1/7] Support binary protocol

---
 mcrouter/lib/IOBufUtil.cpp                |   1 +
 mcrouter/lib/IOBufUtil.h                  |  33 +++
 mcrouter/lib/Makefile.am                  |   2 +
 mcrouter/lib/network/ClientMcParser-inl.h |   5 +
 mcrouter/lib/network/ClientMcParser.h     |   1 +
 mcrouter/lib/network/McAsciiParser-inl.h  |  35 +--
 mcrouter/lib/network/McAsciiParser.h      |  13 +-
 mcrouter/lib/network/McAsciiParser.rl     |   1 +
 mcrouter/lib/network/McBinaryParser.cpp   | 254 ++++++++++++++++++++++
 mcrouter/lib/network/McBinaryParser.h     | 179 +++++++++++++++
 mcrouter/lib/network/McParser.cpp         |   4 +
 mcrouter/lib/network/McParser.h           |  11 +
 mcrouter/lib/network/ServerMcParser-inl.h |  20 ++
 mcrouter/lib/network/ServerMcParser.h     |   3 +
 mcrouter/lib/network/gen/CommonMessages.h |   2 +-
 15 files changed, 519 insertions(+), 45 deletions(-)
 create mode 100644 mcrouter/lib/network/McBinaryParser.cpp
 create mode 100644 mcrouter/lib/network/McBinaryParser.h

diff --git a/mcrouter/lib/IOBufUtil.cpp b/mcrouter/lib/IOBufUtil.cpp
index adf9c3dd6..3b94b6e80 100644
--- a/mcrouter/lib/IOBufUtil.cpp
+++ b/mcrouter/lib/IOBufUtil.cpp
@@ -79,5 +79,6 @@ coalesceIovecs(const struct iovec* iov, size_t iovcnt, size_t destCapacity) {
   }
   return coalesceSlow(iov, iovcnt, destCapacity);
 }
+
 }
 } // facebook::memcache
diff --git a/mcrouter/lib/IOBufUtil.h b/mcrouter/lib/IOBufUtil.h
index e29dd27e8..11973a6dd 100644
--- a/mcrouter/lib/IOBufUtil.h
+++ b/mcrouter/lib/IOBufUtil.h
@@ -104,5 +104,38 @@ copyAsString(const folly::IOBuf& source, const uint8_t* begin, size_t size) {
  */
 folly::IOBuf
 coalesceIovecs(const struct iovec* iov, size_t iovcnt, size_t destCapacity);
+
+/**
+ * Trim IOBuf to reference only data from range [posStart, posEnd).
+ */
+inline void trimIOBufToRange(
+    folly::IOBuf& buffer,
+    const char* posStart,
+    const char* posEnd) {
+  buffer.trimStart(posStart - reinterpret_cast<const char*>(buffer.data()));
+  buffer.trimEnd(buffer.length() - (posEnd - posStart));
+}
+
+
+inline void appendKeyPiece(
+    const folly::IOBuf& from,
+    folly::IOBuf& to,
+    const char* posStart,
+    const char* posEnd) {
+  // No need to process empty piece.
+  if (UNLIKELY(posEnd == posStart)) {
+    return;
+  }
+
+  if (LIKELY(to.length() == 0)) {
+    from.cloneOneInto(to);
+    trimIOBufToRange(to, posStart, posEnd);
+  } else {
+    auto nextPiece = from.cloneOne();
+    trimIOBufToRange(*nextPiece, posStart, posEnd);
+    to.prependChain(std::move(nextPiece));
+  }
+}
+
 }
 } // facebook::memcache
diff --git a/mcrouter/lib/Makefile.am b/mcrouter/lib/Makefile.am
index 9d5f4598b..690f30c6b 100644
--- a/mcrouter/lib/Makefile.am
+++ b/mcrouter/lib/Makefile.am
@@ -164,6 +164,8 @@ libmcrouter_a_SOURCES = \
   network/McAsciiParser-inl.h \
   network/McAsciiParser.cpp \
   network/McAsciiParser.h \
+  network/McBinaryParser.cpp \
+  network/McBinaryParser.h \
   network/McClientRequestContext-inl.h \
   network/McClientRequestContext.cpp \
   network/McClientRequestContext.h \
diff --git a/mcrouter/lib/network/ClientMcParser-inl.h b/mcrouter/lib/network/ClientMcParser-inl.h
index d31b15fff..cf1ff9d47 100644
--- a/mcrouter/lib/network/ClientMcParser-inl.h
+++ b/mcrouter/lib/network/ClientMcParser-inl.h
@@ -275,6 +275,11 @@ void ClientMcParser<Callback>::handleAscii(folly::IOBuf& readBuffer) {
   }
 }
 
+template <class Callback>
+void ClientMcParser<Callback>::handleBinary(folly::IOBuf& readBuffer) {
+  LOG(ERROR) << "handleBinary() not available to client parser";
+}
+
 template <class Callback>
 void ClientMcParser<Callback>::parseError(
     mc_res_t result,
diff --git a/mcrouter/lib/network/ClientMcParser.h b/mcrouter/lib/network/ClientMcParser.h
index 360506463..e2093879e 100644
--- a/mcrouter/lib/network/ClientMcParser.h
+++ b/mcrouter/lib/network/ClientMcParser.h
@@ -113,6 +113,7 @@ class ClientMcParser : private McParser::ParserCallback {
       const UmbrellaMessageInfo& headerInfo,
       const folly::IOBuf& buffer) final;
   void handleAscii(folly::IOBuf& readBuffer) final;
+  void handleBinary(folly::IOBuf& readBuffer) final;
   void parseError(mc_res_t result, folly::StringPiece reason) final;
 
   bool shouldReadToAsciiBuffer() const;
diff --git a/mcrouter/lib/network/McAsciiParser-inl.h b/mcrouter/lib/network/McAsciiParser-inl.h
index ca1fae35d..ca7e61e25 100644
--- a/mcrouter/lib/network/McAsciiParser-inl.h
+++ b/mcrouter/lib/network/McAsciiParser-inl.h
@@ -8,6 +8,7 @@
 #include <arpa/inet.h>
 
 #include "mcrouter/lib/fbi/cpp/util.h"
+#include "mcrouter/lib/IOBufUtil.h"
 
 namespace facebook {
 namespace memcache {
@@ -136,40 +137,6 @@ void McClientAsciiParser::initializeReplyParser() {
       typeid(Request).name());
 }
 
-/**
- * Append piece of IOBuf in range [posStart, posEnd) to destination IOBuf.
- */
-inline void McAsciiParserBase::appendKeyPiece(
-    const folly::IOBuf& from,
-    folly::IOBuf& to,
-    const char* posStart,
-    const char* posEnd) {
-  // No need to process empty piece.
-  if (UNLIKELY(posEnd == posStart)) {
-    return;
-  }
-
-  if (LIKELY(to.length() == 0)) {
-    from.cloneOneInto(to);
-    trimIOBufToRange(to, posStart, posEnd);
-  } else {
-    auto nextPiece = from.cloneOne();
-    trimIOBufToRange(*nextPiece, posStart, posEnd);
-    to.prependChain(std::move(nextPiece));
-  }
-}
-
-/**
- * Trim IOBuf to reference only data from range [posStart, posEnd).
- */
-inline void McAsciiParserBase::trimIOBufToRange(
-    folly::IOBuf& buffer,
-    const char* posStart,
-    const char* posEnd) {
-  buffer.trimStart(posStart - reinterpret_cast<const char*>(buffer.data()));
-  buffer.trimEnd(buffer.length() - (posEnd - posStart));
-}
-
 template <class Callback>
 McServerAsciiParser::McServerAsciiParser(Callback& callback)
     : callback_(
diff --git a/mcrouter/lib/network/McAsciiParser.h b/mcrouter/lib/network/McAsciiParser.h
index 516745214..98c7666d3 100644
--- a/mcrouter/lib/network/McAsciiParser.h
+++ b/mcrouter/lib/network/McAsciiParser.h
@@ -72,16 +72,6 @@ class McAsciiParserBase {
   bool readValue(folly::IOBuf& buffer, folly::IOBuf& to);
   bool readValue(folly::IOBuf& buffer, folly::Optional<folly::IOBuf>& to);
 
-  static void appendKeyPiece(
-      const folly::IOBuf& from,
-      folly::IOBuf& to,
-      const char* posStart,
-      const char* posEnd);
-  static void trimIOBufToRange(
-      folly::IOBuf& buffer,
-      const char* posStart,
-      const char* posEnd);
-
   std::string currentErrorDescription_;
 
   uint64_t currentUInt_{0};
@@ -177,6 +167,9 @@ class McClientAsciiParser : public McAsciiParserBase {
 namespace detail {
 template <class RequestList>
 class CallbackBase;
+
+template <class Callback, class Requests>
+class CallbackWrapper;
 } // detail
 
 class McServerAsciiParser : public McAsciiParserBase {
diff --git a/mcrouter/lib/network/McAsciiParser.rl b/mcrouter/lib/network/McAsciiParser.rl
index 552299fdf..669249092 100644
--- a/mcrouter/lib/network/McAsciiParser.rl
+++ b/mcrouter/lib/network/McAsciiParser.rl
@@ -10,6 +10,7 @@
 #include "mcrouter/lib/mc/msg.h"
 #include "mcrouter/lib/McOperation.h"
 #include "mcrouter/lib/network/gen/Memcache.h"
+#include "mcrouter/lib/IOBufUtil.h"
 
 namespace facebook { namespace memcache {
 
diff --git a/mcrouter/lib/network/McBinaryParser.cpp b/mcrouter/lib/network/McBinaryParser.cpp
new file mode 100644
index 000000000..8c68623e0
--- /dev/null
+++ b/mcrouter/lib/network/McBinaryParser.cpp
@@ -0,0 +1,254 @@
+/*
+ *  Copyright (c) 2017, Facebook, Inc.
+ *  All rights reserved.
+ *
+ *  This source code is licensed under the BSD-style license found in the
+ *  LICENSE file in the root directory of this source tree. An additional grant
+ *  of patent rights can be found in the PATENTS file in the same directory.
+ *
+ */
+#include "McBinaryParser.h"
+
+#include <folly/String.h>
+
+#include "mcrouter/lib/fbi/cpp/LogFailure.h"
+#include "mcrouter/lib/IOBufUtil.h"
+
+namespace facebook {
+namespace memcache {
+
+McServerBinaryParser::State McServerBinaryParser::consume(folly::IOBuf& buffer) {
+  assert(state_ != State::ERROR);
+  assert(state_ != State::COMPLETE);
+
+  uint64_t avaiBytes = buffer.length();
+  const char *p_ = reinterpret_cast<const char*>(buffer.data());
+
+  if (state_ == State::UNINIT) {
+    header_        = nullptr;
+    sectionLength_ = HeaderLength;
+    sectionStart_  = p_;
+    state_         = State::PARTIAL_HEADER;
+  } else {
+    while (state_ != State::ERROR && state_ != State::COMPLETE
+      && avaiBytes >= sectionLength_) {
+      switch (state_) {
+        case State::PARTIAL_HEADER:
+          if (!parseHeader(p_)) {
+            state_ = State::ERROR;
+          } else {
+            sectionStart_ += sectionLength_;
+            sectionLength_ = getExtrasLength();
+            state_ = State::PARTIAL_EXTRA;
+          }
+          break;
+
+        case State::PARTIAL_EXTRA:
+          appendKeyPiece(
+            buffer, currentValue_,
+            sectionStart_, sectionStart_ + sectionLength_);
+          sectionStart_ += sectionLength_;
+          sectionLength_ = getKeyLength();
+          state_ = State::PARTIAL_KEY;
+          break;
+
+        case State::PARTIAL_KEY:
+          appendKeyPiece(
+            buffer, currentValue_,
+            sectionStart_, sectionStart_ + sectionLength_);
+          sectionStart_ += sectionLength_;
+          sectionLength_ = getValueLength();
+          state_ = State::PARTIAL_VALUE;
+          break;
+
+        case State::PARTIAL_VALUE:
+          appendKeyPiece(
+            buffer, currentValue_,
+            sectionStart_, sectionStart_ + sectionLength_);
+          sectionStart_ += sectionLength_;
+          state_ = State::COMPLETE;
+          (this->*consumer_)();
+          break;
+
+        default:
+          CHECK(false);
+      }
+    }
+  }
+  buffer.trimStart(sectionStart_ - p_);
+
+  return state_;
+}
+
+bool McServerBinaryParser::parseHeader(const char * bytes) {
+  header_ = reinterpret_cast<const RequestHeader*>(bytes);
+
+  if (getMagic() != 0x80 || getMagic() != 0x81 || getDataType() != 0x00) {
+    return false;
+  }
+
+  // TODO validate command constraint (i.e. no extras, no value)
+  switch (getOpCode()) {
+    case 0x0a:  // No-op
+      return false;
+
+    case 0x01:  // Set
+    case 0x11:  // SetQ
+    case 0x31:  // RSet
+    case 0x32:  // RSetQ
+      // TODO check the necessary parameters (extras size)
+      currentMessage_.emplace<McSetRequest>();
+      consumer_ = &McServerBinaryParser::consumeSetLike<McSetRequest>;
+      return true;
+    case 0x02:  // Add
+    case 0x12:  // AddQ
+      currentMessage_.emplace<McAddRequest>();
+      consumer_ = &McServerBinaryParser::consumeSetLike<McAddRequest>;
+      return true;
+    case 0x03:  // Replace
+    case 0x13:  // ReplaceQ
+      currentMessage_.emplace<McReplaceRequest>();
+      consumer_ = &McServerBinaryParser::consumeSetLike<McReplaceRequest>;
+      return true;
+    case 0x0e:  // Append
+    case 0x19:  // AppendQ
+    case 0x33:  // RAppend
+    case 0x34:  // RAppendQ
+      currentMessage_.emplace<McAppendRequest>();
+      consumer_ = &McServerBinaryParser::consumeAppendLike<McAppendRequest>;
+      return true;
+    case 0x0f:  // Prepend
+    case 0x1a:  // PrependQ
+    case 0x35:  // RPrepend
+    case 0x36:  // RPrependQ
+      currentMessage_.emplace<McPrependRequest>();
+      consumer_ = &McServerBinaryParser::consumeAppendLike<McPrependRequest>;
+      return true;
+    case 0x00:  // Get
+    case 0x09:  // GetQ
+    case 0x0c:  // GetK
+    case 0x0d:  // GetKQ
+    case 0x30:  // RGet
+      currentMessage_.emplace<McGetRequest>();
+      consumer_ = &McServerBinaryParser::consumeGetLike<McGetRequest>;
+      return true;
+    case 0x04:  // Delete
+    case 0x14:  // DeleteQ
+    case 0x37:  // RDelete
+    case 0x38:  // RDeleteQ
+      currentMessage_.emplace<McDeleteRequest>();
+      consumer_ = &McServerBinaryParser::consumeGetLike<McDeleteRequest>;
+      return true;
+    case 0x05:  // Increment
+    case 0x15:  // IncrementQ
+    case 0x39:  // RIncr
+    case 0x3a:  // RIncrQ
+      currentMessage_.emplace<McIncrRequest>();
+      consumer_ = &McServerBinaryParser::consumeArithLike<McIncrRequest>;
+      return true;
+    case 0x06:  // Decrement
+    case 0x16:  // DecrementQ
+    case 0x3b:  // RDecr
+    case 0x3c:  // RDecrQ
+      currentMessage_.emplace<McDecrRequest>();
+      consumer_ = &McServerBinaryParser::consumeArithLike<McDecrRequest>;
+      return true;
+    case 0x1c:  // Touch
+    case 0x1d:  // GAT
+    case 0x1e:  // GATQ
+      currentMessage_.emplace<McTouchRequest>();
+      consumer_ = &McServerBinaryParser::consumeGetLike<McTouchRequest>;
+      return true;
+    case 0x10:  // Stat
+      currentMessage_.emplace<McStatsRequest>();
+      consumer_ = &McServerBinaryParser::consumeGetLike<McStatsRequest>;
+      return true;
+    case 0x0b:  // Version
+      currentMessage_.emplace<McVersionRequest>();
+      consumer_ = &McServerBinaryParser::consumeUnary<McVersionRequest>;
+      return true;
+    case 0x07:  // Quit
+    case 0x17:  // QuitQ
+      currentMessage_.emplace<McQuitRequest>();
+      consumer_ = &McServerBinaryParser::consumeUnary<McQuitRequest>;
+      return true;
+    case 0x08:  // Flush
+    case 0x18:  // FlushQ
+      currentMessage_.emplace<McFlushAllRequest>();
+      consumer_ = &McServerBinaryParser::consumeFlush;
+      return true;
+    /*
+    case 0x20:  // SASL list mechs
+    case 0x21:  // SASL Auth
+    case 0x22:  // SASL Step
+
+    // v1.6 proposed commands
+    case 0x3d:  // Set VBucket *
+    case 0x45:  // TAP VBucket Set *
+    case 0x3e:  // Get VBucket *
+    case 0x42:  // TAP Delete *
+
+    case 0x1b:  // Verbosity *
+    case 0x43:  // TAP Flush *
+    case 0x3f:  // Del VBucket *
+    case 0x40:  // TAP Connect *
+    case 0x41:  // TAP Mutation *
+    case 0x44:  // TAP Opaque *
+    case 0x46:  // TAP Checkpoint Start *
+    case 0x47:  // TAP Checkpoint End *
+    */
+    default:
+      return false;
+  }
+}
+
+template <class Request>
+void McServerBinaryParser::consumeSetLike() {
+  auto extras       = reinterpret_cast<const SetExtras_t*>(currentExtras_.data());
+  auto& message     = currentMessage_.get<Request>();
+  message.key()     = std::move(currentKey_);
+  message.exptime() = ntohl(extras->exptime);
+  callback_->onRequest(std::move(message));
+}
+
+template <class Request>
+void McServerBinaryParser::consumeAppendLike() {
+  auto& message   = currentMessage_.get<Request>();
+  message.key()   = std::move(currentKey_);
+  message.value() = std::move(currentValue_);
+  callback_->onRequest(std::move(message));
+}
+
+template <class Request>
+void McServerBinaryParser::consumeGetLike() {
+  auto& message   = currentMessage_.get<Request>();
+  message.key()   = std::move(currentKey_);
+  callback_->onRequest(std::move(message));
+}
+
+template <class Request>
+void McServerBinaryParser::consumeArithLike() {
+  auto extras     = reinterpret_cast<const ArithExtras_t*>(currentExtras_.data());
+  auto& message   = currentMessage_.get<Request>();
+  message.key()   = std::move(currentKey_);
+  message.delta() = ntohl(extras->delta);
+  // message.initialValue() = ntohl(extras->initialValue);
+  // message.exptime() = ntohl(extras->exptime);
+  callback_->onRequest(std::move(message));
+}
+
+template <class Request>
+void McServerBinaryParser::consumeUnary() {
+  auto& message = currentMessage_.get<Request>();
+  callback_->onRequest(std::move(message));
+}
+
+void McServerBinaryParser::consumeFlush() {
+  // auto extras       = reinterpret_cast<const FlushExtras_t*>(currentExtras_.data());
+  auto& message     = currentMessage_.get<McFlushAllRequest>();
+  // message.exptime() = ntohl(extras->exptime);
+  callback_->onRequest(std::move(message));
+}
+
+}
+} // facebook::memcache
diff --git a/mcrouter/lib/network/McBinaryParser.h b/mcrouter/lib/network/McBinaryParser.h
new file mode 100644
index 000000000..3d6ea6514
--- /dev/null
+++ b/mcrouter/lib/network/McBinaryParser.h
@@ -0,0 +1,179 @@
+/*
+ *  Copyright (c) 2017, Facebook, Inc.
+ *  All rights reserved.
+ *
+ *  This source code is licensed under the BSD-style license found in the
+ *  LICENSE file in the root directory of this source tree. An additional grant
+ *  of patent rights can be found in the PATENTS file in the same directory.
+ *
+ */
+#pragma once
+
+#include <exception>
+#include <typeindex>
+
+#include <folly/Optional.h>
+#include <folly/io/IOBuf.h>
+
+#include "mcrouter/lib/Operation.h"
+#include "mcrouter/lib/carbon/Variant.h"
+#include "mcrouter/lib/fbi/cpp/TypeList.h"
+#include "mcrouter/lib/network/CarbonMessageList.h"
+#include "mcrouter/lib/network/gen/Memcache.h"
+#include "mcrouter/lib/network/McAsciiParser.h"
+
+namespace facebook {
+namespace memcache {
+
+namespace detail {
+template <class RequestList>
+class CallbackBase;
+} // detail
+
+class McServerBinaryParser {
+ public:
+  enum class State {
+    // The parser is not initialized to parse any messages.
+    UNINIT,
+    // Have partial message, and need more data to complete it.
+    PARTIAL_HEADER,
+    PARTIAL_EXTRA,
+    PARTIAL_KEY,
+    PARTIAL_VALUE,
+    // There was an error on the protocol level.
+    ERROR,
+    // Complete message had been parsed and ready to be returned.
+    COMPLETE,
+  };
+
+  McServerBinaryParser() = default;
+
+  McServerBinaryParser(const McServerBinaryParser&) = delete;
+  McServerBinaryParser& operator=(const McServerBinaryParser&) = delete;
+
+  template <class Callback>
+  McServerBinaryParser(Callback& callback)
+      : callback_(
+            std::make_unique<detail::CallbackWrapper<Callback, McRequestList>>(
+                callback)) {}
+
+  State getCurrentState() const noexcept {
+    return state_;
+  }
+
+  State consume(folly::IOBuf& buffer);
+  std::unique_ptr<detail::CallbackBase<McRequestList>> callback_;
+
+ protected:
+
+  std::string currentErrorDescription_;
+
+  uint64_t currentUInt_{0};
+
+  folly::IOBuf* currentIOBuf_{nullptr};
+  size_t remainingIOBufLength_{0};
+  State state_{State::UNINIT};
+  bool negative_{false};
+
+  const char *sectionStart_;
+  uint64_t sectionLength_;
+
+  uint32_t getMagic() {
+    return header_->magic;
+  }
+  uint8_t getOpCode() {
+    return header_->opCode;
+  }
+  uint16_t getKeyLength() {
+    return ntohs(header_->keyLen);
+  }
+  uint8_t getExtrasLength() {
+    return header_->extrasLen;
+  }
+  uint16_t getValueLength() {
+    return getTotalBodyLength() - getKeyLength() - getExtrasLength();
+  }
+  uint8_t getDataType() {
+    return header_->dataType;
+  }
+  uint16_t getVBucketId() {
+    return ntohs(header_->vBucketId);
+  }
+  uint64_t getTotalBodyLength() {
+    return ntohl(header_->totalBodyLen);
+  }
+  uint32_t getOpaque() {
+    return ntohl(header_->opaque);
+  }
+  uint32_t getCAS() {
+    return ntohl(header_->cas);
+  }
+
+  static constexpr uint64_t HeaderLength = 24;
+
+  typedef struct RequestHeader {
+    uint8_t  magic;
+    uint8_t  opCode;
+    uint16_t keyLen;
+    uint8_t  extrasLen;
+    uint8_t  dataType;
+    uint16_t vBucketId;
+    uint32_t totalBodyLen;
+    uint32_t opaque;
+    uint32_t cas;
+  } __attribute__((__packed__)) RequestHeader_t;
+
+  typedef struct SetExtras {
+    uint32_t flags;
+    uint32_t exptime;
+  } __attribute__((__packed__)) SetExtras_t;
+
+  typedef struct ArithExtras {
+    uint32_t delta;
+    uint32_t initialValue;
+    uint32_t exptime;
+  } __attribute__((__packed__)) ArithExtras_t;
+
+  typedef struct TouchExtras {
+    uint32_t exptime;
+  } __attribute__((__packed__)) TouchExtras_t;
+
+  typedef struct FlushExtras {
+    uint32_t exptime;
+  } __attribute__((__packed__)) FlushExtras_t;
+
+  bool parseHeader(const char* bytes);
+
+  template <class Request>
+  void consumeSetLike();
+
+  template <class Request>
+  void consumeAppendLike();
+
+  template <class Request>
+  void consumeGetLike();
+
+  template <class Request>
+  void consumeArithLike();
+
+  template <class Request>
+  void consumeUnary();
+
+  void consumeFlush();
+
+  // Network byte-ordered fields
+  const RequestHeader_t *header_;
+
+  folly::IOBuf currentExtras_;
+  folly::IOBuf currentKey_;
+  folly::IOBuf currentValue_;
+
+  using ConsumerFunPtr = void (McServerBinaryParser::*)();
+  ConsumerFunPtr consumer_{nullptr};
+
+  using RequestVariant = carbon::makeVariantFromList<McRequestList>;
+  RequestVariant currentMessage_;
+};
+
+}
+} // facebook::memcache
diff --git a/mcrouter/lib/network/McParser.cpp b/mcrouter/lib/network/McParser.cpp
index e6c3008f0..f0c8fb8ae 100644
--- a/mcrouter/lib/network/McParser.cpp
+++ b/mcrouter/lib/network/McParser.cpp
@@ -232,6 +232,10 @@ bool McParser::readDataAvailable(size_t len) {
     callback_.handleAscii(readBuffer_);
     return true;
   }
+  if (protocol_ == mc_binary_protocol) {
+    callback_.handleBinary(readBuffer_);
+    return true;
+  }
   return readUmbrellaOrCaretData();
 }
 
diff --git a/mcrouter/lib/network/McParser.h b/mcrouter/lib/network/McParser.h
index 68914516b..41bc30bf2 100644
--- a/mcrouter/lib/network/McParser.h
+++ b/mcrouter/lib/network/McParser.h
@@ -25,6 +25,9 @@ inline mc_protocol_t determineProtocol(uint8_t firstByte) {
       return mc_caret_protocol;
     case ENTRY_LIST_MAGIC_BYTE:
       return mc_umbrella_protocol_DONOTUSE;
+    case 0x80:
+    case 0x81:
+      return mc_binary_protocol;
     default:
       return mc_ascii_protocol;
   }
@@ -70,6 +73,14 @@ class McParser {
      */
     virtual void handleAscii(folly::IOBuf& readBuffer) = 0;
 
+    /**
+     * Handle binary data read.
+     * The user is responsible for clearing or advancing the readBuffer.
+     *
+     * @param readBuffer  buffer with newly read data that needs to be parsed.
+     */
+    virtual void handleBinary(folly::IOBuf& readBuffer) = 0;
+
     /**
      * Called on fatal parse error (the stream should normally be closed)
      */
diff --git a/mcrouter/lib/network/ServerMcParser-inl.h b/mcrouter/lib/network/ServerMcParser-inl.h
index a9fe450e6..72e124096 100644
--- a/mcrouter/lib/network/ServerMcParser-inl.h
+++ b/mcrouter/lib/network/ServerMcParser-inl.h
@@ -136,6 +136,26 @@ void ServerMcParser<Callback>::handleAscii(folly::IOBuf& readBuffer) {
   }
 }
 
+template <class Callback>
+void ServerMcParser<Callback>::handleBinary(folly::IOBuf& readBuffer) {
+  if (UNLIKELY(parser_.protocol() != mc_binary_protocol)) {
+    std::string reason(folly::sformat(
+        "Expected {} protocol, but received binary!",
+        mc_protocol_to_string(parser_.protocol())));
+    callback_.parseError(mc_res_local_error, reason);
+    return;
+  }
+
+  // Note: McParser never chains IOBufs.
+  auto result = binaryParser_.consume(readBuffer);
+
+  if (result == McServerBinaryParser::State::ERROR) {
+    // Note: we could include actual parsing error instead of
+    // "malformed request" (e.g. asciiParser_.getErrorDescription()).
+    callback_.parseError(mc_res_client_error, "malformed request");
+  }
+}
+
 template <class Callback>
 void ServerMcParser<Callback>::parseError(
     mc_res_t result,
diff --git a/mcrouter/lib/network/ServerMcParser.h b/mcrouter/lib/network/ServerMcParser.h
index 03e384995..ba54a47e4 100644
--- a/mcrouter/lib/network/ServerMcParser.h
+++ b/mcrouter/lib/network/ServerMcParser.h
@@ -9,6 +9,7 @@
 
 #include "mcrouter/lib/network/AsciiSerialized.h"
 #include "mcrouter/lib/network/McAsciiParser.h"
+#include "mcrouter/lib/network/McBinaryParser.h"
 #include "mcrouter/lib/network/McParser.h"
 
 namespace facebook {
@@ -62,6 +63,7 @@ class ServerMcParser : private McParser::ParserCallback {
  private:
   McParser parser_;
   McServerAsciiParser asciiParser_;
+  McServerBinaryParser binaryParser_;
 
   Callback& callback_;
 
@@ -81,6 +83,7 @@ class ServerMcParser : private McParser::ParserCallback {
       const UmbrellaMessageInfo& headerInfo,
       const folly::IOBuf& buffer) final;
   void handleAscii(folly::IOBuf& readBuffer) final;
+  void handleBinary(folly::IOBuf& readBuffer) final;
   void parseError(mc_res_t result, folly::StringPiece reason) final;
   bool shouldReadToAsciiBuffer() const;
 
diff --git a/mcrouter/lib/network/gen/CommonMessages.h b/mcrouter/lib/network/gen/CommonMessages.h
index b7e6bdbee..de9bd1ca6 100644
--- a/mcrouter/lib/network/gen/CommonMessages.h
+++ b/mcrouter/lib/network/gen/CommonMessages.h
@@ -79,7 +79,7 @@ class McVersionRequest : public carbon::RequestCommon {
   void visitFields(V&& v) const;
 
  private:
-  carbon::Keys<folly::IOBuf> key_;
+  carbon::Keys<folly::IOBuf> key_;  // TODO: Why?
 };
 
 class McVersionReply : public carbon::ReplyCommon {

From d594fd7bfd1ee6b833d381f26d883bf60021e09a Mon Sep 17 00:00:00 2001
From: Ngo The Trung <tngo@zendesk.com>
Date: Sat, 26 Aug 2017 05:23:41 +0000
Subject: [PATCH 2/7] Add quiet/returnKey flags

---
 mcrouter/lib/carbon/RequestCommon.h     |  16 +++
 mcrouter/lib/network/McBinaryParser.cpp | 135 +++++++++++++++++-------
 mcrouter/lib/network/McBinaryParser.h   |  15 +--
 3 files changed, 119 insertions(+), 47 deletions(-)

diff --git a/mcrouter/lib/carbon/RequestCommon.h b/mcrouter/lib/carbon/RequestCommon.h
index 0143b5a1b..db855c241 100644
--- a/mcrouter/lib/carbon/RequestCommon.h
+++ b/mcrouter/lib/carbon/RequestCommon.h
@@ -89,6 +89,18 @@ class RequestCommon {
     fbtraceInfo_ = McFbtraceRef::moveRef(carbonFbtraceInfo);
   }
 #endif
+  bool quiet() const {
+    return quiet_;
+  }
+  bool& quiet() {
+    return quiet_;
+  }
+  bool returnKey() const {
+    return returnKey_;
+  }
+  bool& returnKey() {
+    return returnKey_;
+  }
 
   /**
    * Tells whether or not "serializedBuffer()" is dirty, in which case it can't
@@ -130,6 +142,10 @@ class RequestCommon {
 
  private:
   static constexpr size_t kTraceIdSize = 11;
+  bool quiet_{false};
+  bool returnKey_{false};
+};
+
 
   const folly::IOBuf* serializedBuffer_{nullptr};
 
diff --git a/mcrouter/lib/network/McBinaryParser.cpp b/mcrouter/lib/network/McBinaryParser.cpp
index 8c68623e0..1605c99d8 100644
--- a/mcrouter/lib/network/McBinaryParser.cpp
+++ b/mcrouter/lib/network/McBinaryParser.cpp
@@ -93,95 +93,135 @@ bool McServerBinaryParser::parseHeader(const char * bytes) {
       return false;
 
     case 0x01:  // Set
+      currentMessage_.emplace<McSetRequest>();
+      consumer_ = &McServerBinaryParser::consumeSetLike<McSetRequest, false>;
+      return true;
     case 0x11:  // SetQ
-    case 0x31:  // RSet
-    case 0x32:  // RSetQ
-      // TODO check the necessary parameters (extras size)
       currentMessage_.emplace<McSetRequest>();
-      consumer_ = &McServerBinaryParser::consumeSetLike<McSetRequest>;
+      consumer_ = &McServerBinaryParser::consumeSetLike<McSetRequest, true>;
       return true;
     case 0x02:  // Add
+      currentMessage_.emplace<McAddRequest>();
+      consumer_ = &McServerBinaryParser::consumeSetLike<McAddRequest, false>;
+      return true;
     case 0x12:  // AddQ
       currentMessage_.emplace<McAddRequest>();
-      consumer_ = &McServerBinaryParser::consumeSetLike<McAddRequest>;
+      consumer_ = &McServerBinaryParser::consumeSetLike<McAddRequest, true>;
       return true;
     case 0x03:  // Replace
+      currentMessage_.emplace<McReplaceRequest>();
+      consumer_ = &McServerBinaryParser::consumeSetLike<McReplaceRequest, false>;
+      return true;
     case 0x13:  // ReplaceQ
       currentMessage_.emplace<McReplaceRequest>();
-      consumer_ = &McServerBinaryParser::consumeSetLike<McReplaceRequest>;
+      consumer_ = &McServerBinaryParser::consumeSetLike<McReplaceRequest, true>;
       return true;
     case 0x0e:  // Append
+      currentMessage_.emplace<McAppendRequest>();
+      consumer_ = &McServerBinaryParser::consumeAppendLike<McAppendRequest, false>;
+      return true;
     case 0x19:  // AppendQ
-    case 0x33:  // RAppend
-    case 0x34:  // RAppendQ
       currentMessage_.emplace<McAppendRequest>();
-      consumer_ = &McServerBinaryParser::consumeAppendLike<McAppendRequest>;
+      consumer_ = &McServerBinaryParser::consumeAppendLike<McAppendRequest, true>;
       return true;
     case 0x0f:  // Prepend
+      currentMessage_.emplace<McPrependRequest>();
+      consumer_ = &McServerBinaryParser::consumeAppendLike<McPrependRequest, false>;
+      return true;
     case 0x1a:  // PrependQ
-    case 0x35:  // RPrepend
-    case 0x36:  // RPrependQ
       currentMessage_.emplace<McPrependRequest>();
-      consumer_ = &McServerBinaryParser::consumeAppendLike<McPrependRequest>;
+      consumer_ = &McServerBinaryParser::consumeAppendLike<McPrependRequest, true>;
       return true;
     case 0x00:  // Get
+      currentMessage_.emplace<McGetRequest>();
+      consumer_ = &McServerBinaryParser::consumeGetLike<McGetRequest, false, false>;
+      return true;
     case 0x09:  // GetQ
+      currentMessage_.emplace<McGetRequest>();
+      consumer_ = &McServerBinaryParser::consumeGetLike<McGetRequest, true, false>;
+      return true;
     case 0x0c:  // GetK
+      currentMessage_.emplace<McGetRequest>();
+      consumer_ = &McServerBinaryParser::consumeGetLike<McGetRequest, false, true>;
+      return true;
     case 0x0d:  // GetKQ
-    case 0x30:  // RGet
       currentMessage_.emplace<McGetRequest>();
-      consumer_ = &McServerBinaryParser::consumeGetLike<McGetRequest>;
+      consumer_ = &McServerBinaryParser::consumeGetLike<McGetRequest, true, true>;
       return true;
     case 0x04:  // Delete
+      currentMessage_.emplace<McDeleteRequest>();
+      consumer_ = &McServerBinaryParser::consumeGetLike<McDeleteRequest, false, false>;
+      return true;
     case 0x14:  // DeleteQ
-    case 0x37:  // RDelete
-    case 0x38:  // RDeleteQ
       currentMessage_.emplace<McDeleteRequest>();
-      consumer_ = &McServerBinaryParser::consumeGetLike<McDeleteRequest>;
+      consumer_ = &McServerBinaryParser::consumeGetLike<McDeleteRequest, true, false>;
       return true;
     case 0x05:  // Increment
+      currentMessage_.emplace<McIncrRequest>();
+      consumer_ = &McServerBinaryParser::consumeArithLike<McIncrRequest, false>;
+      return true;
     case 0x15:  // IncrementQ
-    case 0x39:  // RIncr
-    case 0x3a:  // RIncrQ
       currentMessage_.emplace<McIncrRequest>();
-      consumer_ = &McServerBinaryParser::consumeArithLike<McIncrRequest>;
+      consumer_ = &McServerBinaryParser::consumeArithLike<McIncrRequest, true>;
       return true;
     case 0x06:  // Decrement
+      currentMessage_.emplace<McDecrRequest>();
+      consumer_ = &McServerBinaryParser::consumeArithLike<McDecrRequest, false>;
+      return true;
     case 0x16:  // DecrementQ
-    case 0x3b:  // RDecr
-    case 0x3c:  // RDecrQ
       currentMessage_.emplace<McDecrRequest>();
-      consumer_ = &McServerBinaryParser::consumeArithLike<McDecrRequest>;
+      consumer_ = &McServerBinaryParser::consumeArithLike<McDecrRequest, true>;
       return true;
     case 0x1c:  // Touch
     case 0x1d:  // GAT
     case 0x1e:  // GATQ
       currentMessage_.emplace<McTouchRequest>();
-      consumer_ = &McServerBinaryParser::consumeGetLike<McTouchRequest>;
+      consumer_ = &McServerBinaryParser::consumeGetLike<McTouchRequest, false, false>;
       return true;
     case 0x10:  // Stat
       currentMessage_.emplace<McStatsRequest>();
-      consumer_ = &McServerBinaryParser::consumeGetLike<McStatsRequest>;
+      consumer_ = &McServerBinaryParser::consumeGetLike<McStatsRequest, false, false>;
       return true;
     case 0x0b:  // Version
       currentMessage_.emplace<McVersionRequest>();
-      consumer_ = &McServerBinaryParser::consumeUnary<McVersionRequest>;
+      consumer_ = &McServerBinaryParser::consumeVersion;
       return true;
     case 0x07:  // Quit
+      currentMessage_.emplace<McQuitRequest>();
+      consumer_ = &McServerBinaryParser::consumeQuit<false>;
+      return true;
     case 0x17:  // QuitQ
       currentMessage_.emplace<McQuitRequest>();
-      consumer_ = &McServerBinaryParser::consumeUnary<McQuitRequest>;
+      consumer_ = &McServerBinaryParser::consumeQuit<true>;
       return true;
     case 0x08:  // Flush
+      currentMessage_.emplace<McFlushAllRequest>();
+      consumer_ = &McServerBinaryParser::consumeFlush<false>;
+      return true;
     case 0x18:  // FlushQ
       currentMessage_.emplace<McFlushAllRequest>();
-      consumer_ = &McServerBinaryParser::consumeFlush;
+      consumer_ = &McServerBinaryParser::consumeFlush<true>;
       return true;
     /*
     case 0x20:  // SASL list mechs
     case 0x21:  // SASL Auth
     case 0x22:  // SASL Step
 
+    // Range operations, not implemented in memcached itself
+    case 0x31:  // RSet
+    case 0x32:  // RSetQ
+    case 0x33:  // RAppend
+    case 0x34:  // RAppendQ
+    case 0x35:  // RPrepend
+    case 0x36:  // RPrependQ
+    case 0x30:  // RGet
+    case 0x37:  // RDelete
+    case 0x38:  // RDeleteQ
+    case 0x39:  // RIncr
+    case 0x3a:  // RIncrQ
+    case 0x3b:  // RDecr
+    case 0x3c:  // RDecrQ
+
     // v1.6 proposed commands
     case 0x3d:  // Set VBucket *
     case 0x45:  // TAP VBucket Set *
@@ -202,51 +242,64 @@ bool McServerBinaryParser::parseHeader(const char * bytes) {
   }
 }
 
-template <class Request>
+template <class Request, bool quiet>
 void McServerBinaryParser::consumeSetLike() {
   auto extras       = reinterpret_cast<const SetExtras_t*>(currentExtras_.data());
   auto& message     = currentMessage_.get<Request>();
   message.key()     = std::move(currentKey_);
   message.exptime() = ntohl(extras->exptime);
+  message.quiet()   = quiet;
   callback_->onRequest(std::move(message));
 }
 
-template <class Request>
+template <class Request, bool quiet>
 void McServerBinaryParser::consumeAppendLike() {
   auto& message   = currentMessage_.get<Request>();
   message.key()   = std::move(currentKey_);
   message.value() = std::move(currentValue_);
+  message.quiet() = quiet;
   callback_->onRequest(std::move(message));
 }
 
-template <class Request>
+template <class Request, bool quiet, bool returnKey>
 void McServerBinaryParser::consumeGetLike() {
-  auto& message   = currentMessage_.get<Request>();
-  message.key()   = std::move(currentKey_);
+  auto& message       = currentMessage_.get<Request>();
+  message.key()       = std::move(currentKey_);
+  message.quiet()     = quiet;
+  message.returnKey() = returnKey;
   callback_->onRequest(std::move(message));
 }
 
-template <class Request>
+template <class Request, bool quiet>
 void McServerBinaryParser::consumeArithLike() {
-  auto extras     = reinterpret_cast<const ArithExtras_t*>(currentExtras_.data());
-  auto& message   = currentMessage_.get<Request>();
-  message.key()   = std::move(currentKey_);
-  message.delta() = ntohl(extras->delta);
+  auto extras            = reinterpret_cast<const ArithExtras_t*>(currentExtras_.data());
+  auto& message          = currentMessage_.get<Request>();
+  message.key()          = std::move(currentKey_);
+  message.delta()        = ntohl(extras->delta);
   // message.initialValue() = ntohl(extras->initialValue);
   // message.exptime() = ntohl(extras->exptime);
+  message.quiet()        = quiet;
+  callback_->onRequest(std::move(message));
+}
+
+template <bool quiet>
+void McServerBinaryParser::consumeQuit() {
+  auto& message   = currentMessage_.get<McQuitRequest>();
+  message.quiet() = quiet;
   callback_->onRequest(std::move(message));
 }
 
-template <class Request>
-void McServerBinaryParser::consumeUnary() {
-  auto& message = currentMessage_.get<Request>();
+void McServerBinaryParser::consumeVersion() {
+  auto& message = currentMessage_.get<McVersionRequest>();
   callback_->onRequest(std::move(message));
 }
 
+template <bool quiet>
 void McServerBinaryParser::consumeFlush() {
   // auto extras       = reinterpret_cast<const FlushExtras_t*>(currentExtras_.data());
   auto& message     = currentMessage_.get<McFlushAllRequest>();
   // message.exptime() = ntohl(extras->exptime);
+  message.quiet()   = quiet;
   callback_->onRequest(std::move(message));
 }
 
diff --git a/mcrouter/lib/network/McBinaryParser.h b/mcrouter/lib/network/McBinaryParser.h
index 3d6ea6514..5a621944c 100644
--- a/mcrouter/lib/network/McBinaryParser.h
+++ b/mcrouter/lib/network/McBinaryParser.h
@@ -144,21 +144,24 @@ class McServerBinaryParser {
 
   bool parseHeader(const char* bytes);
 
-  template <class Request>
+  template <class Request, bool quiet>
   void consumeSetLike();
 
-  template <class Request>
+  template <class Request, bool quiet>
   void consumeAppendLike();
 
-  template <class Request>
+  template <class Request, bool quiet, bool returnKey>
   void consumeGetLike();
 
-  template <class Request>
+  template <class Request, bool quiet>
   void consumeArithLike();
 
-  template <class Request>
-  void consumeUnary();
+  template <bool quiet>
+  void consumeQuit();
 
+  void consumeVersion();
+
+  template <bool quiet>
   void consumeFlush();
 
   // Network byte-ordered fields

From f7b63232230da136b7619ab752ee74ca9a861edc Mon Sep 17 00:00:00 2001
From: Ngo The Trung <tngo@zendesk.com>
Date: Sat, 26 Aug 2017 14:53:16 +0000
Subject: [PATCH 3/7] Enable binary type in WriteBuffer

---
 mcrouter/lib/network/WriteBuffer-inl.h |  4 ++++
 mcrouter/lib/network/WriteBuffer.cpp   | 12 ++++++++++++
 mcrouter/lib/network/WriteBuffer.h     |  2 ++
 3 files changed, 18 insertions(+)

diff --git a/mcrouter/lib/network/WriteBuffer-inl.h b/mcrouter/lib/network/WriteBuffer-inl.h
index 5031e666a..5dca6c37e 100644
--- a/mcrouter/lib/network/WriteBuffer-inl.h
+++ b/mcrouter/lib/network/WriteBuffer-inl.h
@@ -43,6 +43,10 @@ WriteBuffer::prepareTyped(
       return asciiReply_.prepare(
           std::move(reply), ctx_->asciiKey(), iovsBegin_, iovsCount_);
 
+    case mc_binary_protocol:
+      return binaryReply_.prepare(
+          std::move(reply), ctx_->asciiKey(), iovsBegin_, iovsCount_);
+
     case mc_umbrella_protocol_DONOTUSE:
       return umbrellaReply_.prepare(
           std::move(reply), ctx_->reqid_, iovsBegin_, iovsCount_);
diff --git a/mcrouter/lib/network/WriteBuffer.cpp b/mcrouter/lib/network/WriteBuffer.cpp
index f9e0f054e..92356b706 100644
--- a/mcrouter/lib/network/WriteBuffer.cpp
+++ b/mcrouter/lib/network/WriteBuffer.cpp
@@ -22,6 +22,10 @@ WriteBuffer::WriteBuffer(mc_protocol_t protocol) : protocol_(protocol) {
       new (&asciiReply_) AsciiSerializedReply;
       break;
 
+    case mc_binary_protocol:
+      new (&binaryReply_) BinarySerializedReply;
+      break;
+
     case mc_umbrella_protocol_DONOTUSE:
       new (&umbrellaReply_) UmbrellaSerializedMessage;
       break;
@@ -41,6 +45,10 @@ WriteBuffer::~WriteBuffer() {
       asciiReply_.~AsciiSerializedReply();
       break;
 
+    case mc_binary_protocol:
+      binaryReply_.~BinarySerializedReply();
+      break;
+
     case mc_umbrella_protocol_DONOTUSE:
       umbrellaReply_.~UmbrellaSerializedMessage();
       break;
@@ -65,6 +73,10 @@ void WriteBuffer::clear() {
       asciiReply_.clear();
       break;
 
+    case mc_binary_protocol:
+      binaryReply_.clear();
+      break;
+
     case mc_umbrella_protocol_DONOTUSE:
       umbrellaReply_.clear();
       break;
diff --git a/mcrouter/lib/network/WriteBuffer.h b/mcrouter/lib/network/WriteBuffer.h
index e9a3bec06..681bc865d 100644
--- a/mcrouter/lib/network/WriteBuffer.h
+++ b/mcrouter/lib/network/WriteBuffer.h
@@ -15,6 +15,7 @@
 #include "mcrouter/lib/mc/protocol.h"
 #include "mcrouter/lib/mc/umbrella.h"
 #include "mcrouter/lib/network/AsciiSerialized.h"
+#include "mcrouter/lib/network/BinarySerialized.h"
 #include "mcrouter/lib/network/CaretSerializedMessage.h"
 #include "mcrouter/lib/network/McServerRequestContext.h"
 #include "mcrouter/lib/network/UmbrellaProtocol.h"
@@ -118,6 +119,7 @@ class WriteBuffer {
   /* Write buffers */
   union {
     AsciiSerializedReply asciiReply_;
+    BinarySerializedReply binaryeply_;
     UmbrellaSerializedMessage umbrellaReply_;
     CaretSerializedMessage caretReply_;
   };

From 626d8164722c523993aa8ec1a07cffdf3c6eebf7 Mon Sep 17 00:00:00 2001
From: Ngo The Trung <tngo@zendesk.com>
Date: Sun, 27 Aug 2017 04:22:54 +0000
Subject: [PATCH 4/7] Add comment about binary protocol fields

---
 mcrouter/lib/network/McBinaryParser.cpp | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/mcrouter/lib/network/McBinaryParser.cpp b/mcrouter/lib/network/McBinaryParser.cpp
index 1605c99d8..658876db1 100644
--- a/mcrouter/lib/network/McBinaryParser.cpp
+++ b/mcrouter/lib/network/McBinaryParser.cpp
@@ -276,6 +276,8 @@ void McServerBinaryParser::consumeArithLike() {
   auto& message          = currentMessage_.get<Request>();
   message.key()          = std::move(currentKey_);
   message.delta()        = ntohl(extras->delta);
+  // These fields are for binary protocol only, we cannot forward them to
+  // upstream servers because we use the ASCII protocol for upstreams
   // message.initialValue() = ntohl(extras->initialValue);
   // message.exptime() = ntohl(extras->exptime);
   message.quiet()        = quiet;
@@ -298,6 +300,7 @@ template <bool quiet>
 void McServerBinaryParser::consumeFlush() {
   // auto extras       = reinterpret_cast<const FlushExtras_t*>(currentExtras_.data());
   auto& message     = currentMessage_.get<McFlushAllRequest>();
+  // Binary protocol only fields
   // message.exptime() = ntohl(extras->exptime);
   message.quiet()   = quiet;
   callback_->onRequest(std::move(message));

From efd375bfd77c28147b6af97ae35e214010f32b6d Mon Sep 17 00:00:00 2001
From: Ngo The Trung <tngo@zendesk.com>
Date: Sun, 27 Aug 2017 05:17:40 +0000
Subject: [PATCH 5/7] Correct request magic byte

0x80 for request
0x81 for response
---
 mcrouter/lib/network/McBinaryParser.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mcrouter/lib/network/McBinaryParser.cpp b/mcrouter/lib/network/McBinaryParser.cpp
index 658876db1..ec2013b54 100644
--- a/mcrouter/lib/network/McBinaryParser.cpp
+++ b/mcrouter/lib/network/McBinaryParser.cpp
@@ -83,7 +83,7 @@ McServerBinaryParser::State McServerBinaryParser::consume(folly::IOBuf& buffer)
 bool McServerBinaryParser::parseHeader(const char * bytes) {
   header_ = reinterpret_cast<const RequestHeader*>(bytes);
 
-  if (getMagic() != 0x80 || getMagic() != 0x81 || getDataType() != 0x00) {
+  if (getMagic() != 0x80 || getDataType() != 0x00) {
     return false;
   }
 

From c73e0121fb491491138562ef79ff5a6d08d667a1 Mon Sep 17 00:00:00 2001
From: Ngo The Trung <tngo@zendesk.com>
Date: Sun, 27 Aug 2017 05:59:52 +0000
Subject: [PATCH 6/7] Use enum for opcode

---
 mcrouter/lib/mc/msg.h                   |  65 ++++++++++++
 mcrouter/lib/network/McBinaryParser.cpp | 126 +++++++++++-------------
 2 files changed, 125 insertions(+), 66 deletions(-)

diff --git a/mcrouter/lib/mc/msg.h b/mcrouter/lib/mc/msg.h
index 8ce6e1e6e..73bcdbbb7 100644
--- a/mcrouter/lib/mc/msg.h
+++ b/mcrouter/lib/mc/msg.h
@@ -406,4 +406,69 @@ const char* mc_req_err_to_string(const mc_req_err_t err);
  */
 const char* mc_res_to_response_string(const mc_res_t result);
 
+typedef enum mc_opcode_e : uint8_t {
+  mc_opcode_noop               = 0x0a,
+  mc_opcode_set                = 0x01,
+  mc_opcode_setq               = 0x11,
+  mc_opcode_add                = 0x02,
+  mc_opcode_addq               = 0x12,
+  mc_opcode_replace            = 0x03,
+  mc_opcode_replaceq           = 0x13,
+  mc_opcode_append             = 0x0e,
+  mc_opcode_appendq            = 0x19,
+  mc_opcode_prepend            = 0x0f,
+  mc_opcode_prependq           = 0x1a,
+  mc_opcode_get                = 0x00,
+  mc_opcode_getq               = 0x09,
+  mc_opcode_getk               = 0x0c,
+  mc_opcode_getkq              = 0x0d,
+  mc_opcode_delete             = 0x04,
+  mc_opcode_deleteq            = 0x14,
+  mc_opcode_increment          = 0x05,
+  mc_opcode_incrementq         = 0x15,
+  mc_opcode_decrement          = 0x06,
+  mc_opcode_decrementq         = 0x16,
+  mc_opcode_touch              = 0x1c,
+  mc_opcode_gat                = 0x1d,
+  mc_opcode_gatq               = 0x1e,
+  mc_opcode_stat               = 0x10,
+  mc_opcode_version            = 0x0b,
+  mc_opcode_quit               = 0x07,
+  mc_opcode_quitq              = 0x17,
+  mc_opcode_flush              = 0x08,
+  mc_opcode_flushq             = 0x18,
+  // SASL commands
+  mc_opcode_sasllistmechs      = 0x20,
+  mc_opcode_saslauth           = 0x21,
+  mc_opcode_saslstep           = 0x22,
+  // Range commands
+  mc_opcode_rset               = 0x31,
+  mc_opcode_rsetq              = 0x32,
+  mc_opcode_rappend            = 0x33,
+  mc_opcode_rappendq           = 0x34,
+  mc_opcode_rprepend           = 0x35,
+  mc_opcode_rprependq          = 0x36,
+  mc_opcode_rget               = 0x30,
+  mc_opcode_rdelete            = 0x37,
+  mc_opcode_rdeleteq           = 0x38,
+  mc_opcode_rincr              = 0x39,
+  mc_opcode_rincrq             = 0x3a,
+  mc_opcode_rdecr              = 0x3b,
+  mc_opcode_rdecrq             = 0x3c,
+  // v1.6 proposed commands
+  mc_opcode_setvbucket         = 0x3d,
+  mc_opcode_tapvbucketset      = 0x45,
+  mc_opcode_getvbucket         = 0x3e,
+  mc_opcode_tapdelete          = 0x42,
+  mc_opcode_verbosity          = 0x1b,
+  mc_opcode_tapflush           = 0x43,
+  mc_opcode_delvbucket         = 0x3f,
+  mc_opcode_tapconnect         = 0x40,
+  mc_opcode_tapmutation        = 0x41,
+  mc_opcode_tapopaque          = 0x44,
+  mc_opcode_tapcheckpointstart = 0x46,
+  mc_opcode_tapcheckpointend   = 0x47,
+} mc_opcode_t;
+
+
 __END_DECLS
diff --git a/mcrouter/lib/network/McBinaryParser.cpp b/mcrouter/lib/network/McBinaryParser.cpp
index ec2013b54..2644a3b51 100644
--- a/mcrouter/lib/network/McBinaryParser.cpp
+++ b/mcrouter/lib/network/McBinaryParser.cpp
@@ -89,154 +89,148 @@ bool McServerBinaryParser::parseHeader(const char * bytes) {
 
   // TODO validate command constraint (i.e. no extras, no value)
   switch (getOpCode()) {
-    case 0x0a:  // No-op
-      return false;
-
-    case 0x01:  // Set
+    case mc_opcode_set:
       currentMessage_.emplace<McSetRequest>();
       consumer_ = &McServerBinaryParser::consumeSetLike<McSetRequest, false>;
       return true;
-    case 0x11:  // SetQ
+    case mc_opcode_setq:
       currentMessage_.emplace<McSetRequest>();
       consumer_ = &McServerBinaryParser::consumeSetLike<McSetRequest, true>;
       return true;
-    case 0x02:  // Add
+    case mc_opcode_add:
       currentMessage_.emplace<McAddRequest>();
       consumer_ = &McServerBinaryParser::consumeSetLike<McAddRequest, false>;
       return true;
-    case 0x12:  // AddQ
+    case mc_opcode_addq
       currentMessage_.emplace<McAddRequest>();
       consumer_ = &McServerBinaryParser::consumeSetLike<McAddRequest, true>;
       return true;
-    case 0x03:  // Replace
+    case mc_opcode_replace:
       currentMessage_.emplace<McReplaceRequest>();
       consumer_ = &McServerBinaryParser::consumeSetLike<McReplaceRequest, false>;
       return true;
-    case 0x13:  // ReplaceQ
+    case mc_opcode_replaceq:
       currentMessage_.emplace<McReplaceRequest>();
       consumer_ = &McServerBinaryParser::consumeSetLike<McReplaceRequest, true>;
       return true;
-    case 0x0e:  // Append
+    case mc_opcode_append:
       currentMessage_.emplace<McAppendRequest>();
       consumer_ = &McServerBinaryParser::consumeAppendLike<McAppendRequest, false>;
       return true;
-    case 0x19:  // AppendQ
+    case mc_opcode_appendq:
       currentMessage_.emplace<McAppendRequest>();
       consumer_ = &McServerBinaryParser::consumeAppendLike<McAppendRequest, true>;
       return true;
-    case 0x0f:  // Prepend
+    case mc_opcode_prepend:
       currentMessage_.emplace<McPrependRequest>();
       consumer_ = &McServerBinaryParser::consumeAppendLike<McPrependRequest, false>;
       return true;
-    case 0x1a:  // PrependQ
+    case mc_opcode_prependq:
       currentMessage_.emplace<McPrependRequest>();
       consumer_ = &McServerBinaryParser::consumeAppendLike<McPrependRequest, true>;
       return true;
-    case 0x00:  // Get
+    case mc_opcode_get:
       currentMessage_.emplace<McGetRequest>();
       consumer_ = &McServerBinaryParser::consumeGetLike<McGetRequest, false, false>;
       return true;
-    case 0x09:  // GetQ
+    case mc_opcode_getq:
       currentMessage_.emplace<McGetRequest>();
       consumer_ = &McServerBinaryParser::consumeGetLike<McGetRequest, true, false>;
       return true;
-    case 0x0c:  // GetK
+    case mc_opcode_getk:
       currentMessage_.emplace<McGetRequest>();
       consumer_ = &McServerBinaryParser::consumeGetLike<McGetRequest, false, true>;
       return true;
-    case 0x0d:  // GetKQ
+    case mc_opcode_getkq:
       currentMessage_.emplace<McGetRequest>();
       consumer_ = &McServerBinaryParser::consumeGetLike<McGetRequest, true, true>;
       return true;
-    case 0x04:  // Delete
+    case mc_opcode_delete:
       currentMessage_.emplace<McDeleteRequest>();
       consumer_ = &McServerBinaryParser::consumeGetLike<McDeleteRequest, false, false>;
       return true;
-    case 0x14:  // DeleteQ
+    case mc_opcode_deleteq:
       currentMessage_.emplace<McDeleteRequest>();
       consumer_ = &McServerBinaryParser::consumeGetLike<McDeleteRequest, true, false>;
       return true;
-    case 0x05:  // Increment
+    case mc_opcode_increment:
       currentMessage_.emplace<McIncrRequest>();
       consumer_ = &McServerBinaryParser::consumeArithLike<McIncrRequest, false>;
       return true;
-    case 0x15:  // IncrementQ
+    case mc_opcode_incrementq:
       currentMessage_.emplace<McIncrRequest>();
       consumer_ = &McServerBinaryParser::consumeArithLike<McIncrRequest, true>;
       return true;
-    case 0x06:  // Decrement
+    case mc_opcode_decrement:
       currentMessage_.emplace<McDecrRequest>();
       consumer_ = &McServerBinaryParser::consumeArithLike<McDecrRequest, false>;
       return true;
-    case 0x16:  // DecrementQ
+    case mc_opcode_decrementq:
       currentMessage_.emplace<McDecrRequest>();
       consumer_ = &McServerBinaryParser::consumeArithLike<McDecrRequest, true>;
       return true;
-    case 0x1c:  // Touch
-    case 0x1d:  // GAT
-    case 0x1e:  // GATQ
+    case mc_opcode_touch:
+    case mc_opcode_gat:
+    case mc_opcode_gatq:
       currentMessage_.emplace<McTouchRequest>();
       consumer_ = &McServerBinaryParser::consumeGetLike<McTouchRequest, false, false>;
       return true;
-    case 0x10:  // Stat
+    case mc_opcode_stat:
       currentMessage_.emplace<McStatsRequest>();
       consumer_ = &McServerBinaryParser::consumeGetLike<McStatsRequest, false, false>;
       return true;
-    case 0x0b:  // Version
+    case mc_opcode_version:
       currentMessage_.emplace<McVersionRequest>();
       consumer_ = &McServerBinaryParser::consumeVersion;
       return true;
-    case 0x07:  // Quit
+    case mc_opcode_quit:
       currentMessage_.emplace<McQuitRequest>();
       consumer_ = &McServerBinaryParser::consumeQuit<false>;
       return true;
-    case 0x17:  // QuitQ
+    case mc_opcode_quitq:
       currentMessage_.emplace<McQuitRequest>();
       consumer_ = &McServerBinaryParser::consumeQuit<true>;
       return true;
-    case 0x08:  // Flush
+    case mc_opcode_flush:
       currentMessage_.emplace<McFlushAllRequest>();
       consumer_ = &McServerBinaryParser::consumeFlush<false>;
       return true;
-    case 0x18:  // FlushQ
+    case mc_opcode_flushq:
       currentMessage_.emplace<McFlushAllRequest>();
       consumer_ = &McServerBinaryParser::consumeFlush<true>;
       return true;
-    /*
-    case 0x20:  // SASL list mechs
-    case 0x21:  // SASL Auth
-    case 0x22:  // SASL Step
-
-    // Range operations, not implemented in memcached itself
-    case 0x31:  // RSet
-    case 0x32:  // RSetQ
-    case 0x33:  // RAppend
-    case 0x34:  // RAppendQ
-    case 0x35:  // RPrepend
-    case 0x36:  // RPrependQ
-    case 0x30:  // RGet
-    case 0x37:  // RDelete
-    case 0x38:  // RDeleteQ
-    case 0x39:  // RIncr
-    case 0x3a:  // RIncrQ
-    case 0x3b:  // RDecr
-    case 0x3c:  // RDecrQ
-
+    case mc_opcode_noop:
+    // SASL commands
+    case mc_opcode_sasllistmechs:
+    case mc_opcode_saslauth:
+    case mc_opcode_saslstep:
+    // Range commands
+    case mc_opcode_rset:
+    case mc_opcode_rsetq:
+    case mc_opcode_rappend:
+    case mc_opcode_rappendq:
+    case mc_opcode_rprepend:
+    case mc_opcode_rprependq:
+    case mc_opcode_rget:
+    case mc_opcode_rdelete:
+    case mc_opcode_rdeleteq:
+    case mc_opcode_rincr:
+    case mc_opcode_rincrq:
+    case mc_opcode_rdecr:
+    case mc_opcode_rdecrq:
     // v1.6 proposed commands
-    case 0x3d:  // Set VBucket *
-    case 0x45:  // TAP VBucket Set *
-    case 0x3e:  // Get VBucket *
-    case 0x42:  // TAP Delete *
-
-    case 0x1b:  // Verbosity *
-    case 0x43:  // TAP Flush *
-    case 0x3f:  // Del VBucket *
-    case 0x40:  // TAP Connect *
-    case 0x41:  // TAP Mutation *
-    case 0x44:  // TAP Opaque *
-    case 0x46:  // TAP Checkpoint Start *
-    case 0x47:  // TAP Checkpoint End *
-    */
+    case mc_opcode_setvbucket:
+    case mc_opcode_tapvbucketset:
+    case mc_opcode_getvbucket:
+    case mc_opcode_tapdelete:
+    case mc_opcode_verosity:
+    case mc_opcode_tapflush:
+    case mc_opcode_delvbucket:
+    case mc_opcode_tapconnect:
+    case mc_opcode_tapmutation:
+    case mc_opcode_tapopaque:
+    case mc_opcode_tapcheckpointstart:
+    case mc_opcode_tapcheckpointend::
     default:
       return false;
   }

From 113ac12b03764e36a52427aaad5c3154770184b2 Mon Sep 17 00:00:00 2001
From: Ngo The Trung <tngo@zendesk.com>
Date: Sun, 27 Aug 2017 06:54:51 +0000
Subject: [PATCH 7/7] Undo changes to make quiet and returnKey available

Likely better to add new Reply classes to handle this
---
 mcrouter/lib/carbon/RequestCommon.h         |  7 ++++---
 mcrouter/lib/network/McBinaryParser.cpp     | 14 +++++++-------
 mcrouter/lib/network/WriteBuffer-inl.h      |  2 +-
 mcrouter/lib/network/gen/MemcacheMessages.h |  2 +-
 4 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/mcrouter/lib/carbon/RequestCommon.h b/mcrouter/lib/carbon/RequestCommon.h
index db855c241..8a7357f7d 100644
--- a/mcrouter/lib/carbon/RequestCommon.h
+++ b/mcrouter/lib/carbon/RequestCommon.h
@@ -89,6 +89,7 @@ class RequestCommon {
     fbtraceInfo_ = McFbtraceRef::moveRef(carbonFbtraceInfo);
   }
 #endif
+  /*
   bool quiet() const {
     return quiet_;
   }
@@ -100,7 +101,7 @@ class RequestCommon {
   }
   bool& returnKey() {
     return returnKey_;
-  }
+  }*/
 
   /**
    * Tells whether or not "serializedBuffer()" is dirty, in which case it can't
@@ -142,8 +143,8 @@ class RequestCommon {
 
  private:
   static constexpr size_t kTraceIdSize = 11;
-  bool quiet_{false};
-  bool returnKey_{false};
+  // bool quiet_{false};
+  // bool returnKey_{false};
 };
 
 
diff --git a/mcrouter/lib/network/McBinaryParser.cpp b/mcrouter/lib/network/McBinaryParser.cpp
index 2644a3b51..a53804ba1 100644
--- a/mcrouter/lib/network/McBinaryParser.cpp
+++ b/mcrouter/lib/network/McBinaryParser.cpp
@@ -242,7 +242,7 @@ void McServerBinaryParser::consumeSetLike() {
   auto& message     = currentMessage_.get<Request>();
   message.key()     = std::move(currentKey_);
   message.exptime() = ntohl(extras->exptime);
-  message.quiet()   = quiet;
+  // message.quiet()   = quiet;
   callback_->onRequest(std::move(message));
 }
 
@@ -251,7 +251,7 @@ void McServerBinaryParser::consumeAppendLike() {
   auto& message   = currentMessage_.get<Request>();
   message.key()   = std::move(currentKey_);
   message.value() = std::move(currentValue_);
-  message.quiet() = quiet;
+  // message.quiet() = quiet;
   callback_->onRequest(std::move(message));
 }
 
@@ -259,8 +259,8 @@ template <class Request, bool quiet, bool returnKey>
 void McServerBinaryParser::consumeGetLike() {
   auto& message       = currentMessage_.get<Request>();
   message.key()       = std::move(currentKey_);
-  message.quiet()     = quiet;
-  message.returnKey() = returnKey;
+  // message.quiet()     = quiet;
+  // message.returnKey() = returnKey;
   callback_->onRequest(std::move(message));
 }
 
@@ -274,14 +274,14 @@ void McServerBinaryParser::consumeArithLike() {
   // upstream servers because we use the ASCII protocol for upstreams
   // message.initialValue() = ntohl(extras->initialValue);
   // message.exptime() = ntohl(extras->exptime);
-  message.quiet()        = quiet;
+  // message.quiet()        = quiet;
   callback_->onRequest(std::move(message));
 }
 
 template <bool quiet>
 void McServerBinaryParser::consumeQuit() {
   auto& message   = currentMessage_.get<McQuitRequest>();
-  message.quiet() = quiet;
+  // message.quiet() = quiet;
   callback_->onRequest(std::move(message));
 }
 
@@ -296,7 +296,7 @@ void McServerBinaryParser::consumeFlush() {
   auto& message     = currentMessage_.get<McFlushAllRequest>();
   // Binary protocol only fields
   // message.exptime() = ntohl(extras->exptime);
-  message.quiet()   = quiet;
+  // message.quiet()   = quiet;
   callback_->onRequest(std::move(message));
 }
 
diff --git a/mcrouter/lib/network/WriteBuffer-inl.h b/mcrouter/lib/network/WriteBuffer-inl.h
index 5dca6c37e..bbc026fca 100644
--- a/mcrouter/lib/network/WriteBuffer-inl.h
+++ b/mcrouter/lib/network/WriteBuffer-inl.h
@@ -37,7 +37,7 @@ WriteBuffer::prepareTyped(
   typeId_ = static_cast<uint32_t>(Reply::typeId);
 
   // The current congestion control only supports mc_caret_protocol.
-  // May extend to other protocals in the future.
+  // May extend to other protocols in the future.
   switch (protocol_) {
     case mc_ascii_protocol:
       return asciiReply_.prepare(
diff --git a/mcrouter/lib/network/gen/MemcacheMessages.h b/mcrouter/lib/network/gen/MemcacheMessages.h
index b0cdcacfc..e47a877a4 100644
--- a/mcrouter/lib/network/gen/MemcacheMessages.h
+++ b/mcrouter/lib/network/gen/MemcacheMessages.h
@@ -149,7 +149,7 @@ class McGetReply : public carbon::ReplyCommon {
 
  private:
   folly::Optional<folly::IOBuf> value_;
-  uint64_t flags_{0};
+  uint64_t flags_{0};  // FIXME Shouldn't this be 32 bits?
   std::string message_;
   carbon::Result result_{mc_res_unknown};
   int16_t appSpecificErrorCode_{0};