tqchen commented on a change in pull request #6334:
URL: https://github.com/apache/incubator-tvm/pull/6334#discussion_r483882571



##########
File path: include/tvm/runtime/crt/rpc_common/framing.h
##########
@@ -0,0 +1,267 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file framing.h
+ * \brief Framing for RPC.
+ */
+
+#ifndef TVM_RUNTIME_CRT_RPC_COMMON_FRAMING_H_
+#define TVM_RUNTIME_CRT_RPC_COMMON_FRAMING_H_
+
+#include <crc16.h>
+#include <inttypes.h>
+#include <stddef.h>
+#include <tvm/runtime/crt/error_codes.h>
+#include <tvm/runtime/crt/rpc_common/write_stream.h>
+
+namespace tvm {
+namespace runtime {
+
+enum class Escape : uint8_t { kEscapeStart = 0xff, kEscapeNop = 0xfe, 
kPacketStart = 0xfd };
+
+class PacketFieldSizeBytes {
+ public:
+  static constexpr const size_t kPayloadLength = sizeof(uint32_t);
+  static constexpr const size_t kCrc = sizeof(uint16_t);
+};
+
+class Unframer {
+ public:
+  explicit Unframer(WriteStream* stream)
+      : stream_{stream},
+        state_{State::kFindPacketStart},
+        saw_escape_start_{false},
+        num_buffer_bytes_valid_{0} {}
+
+  /*!
+   * \brief Push data into unframer and try to decode one packet.
+   *
+   * This function will return when exactly one packet has been decoded. It 
may not consume all of
+   * `data` in this case, and valid bytes may remain at the end of data.
+   *
+   * \param data The new data to unframe and send downstream.
+   * \param data_size_bytes The number of valid bytes in data.
+   * \param bytes_consumed Pointer written with the number of bytes consumed 
from data.
+   * \return
+   *     - kTvmErrorNoError when successful -- continue writing data.
+   *     - kTvmErrorFramingInvalidState when the Unframer was in or enters an 
invalid state
+   *       (probably indicates memory corruption).
+   *     - kTvmErrorFramingShortPacket when a new packet started before the 
current one ended.
+   *     - kTvmErrorFramingInvalidEscape when an invalid escape sequence was 
seen
+   */
+  tvm_crt_error_t Write(const uint8_t* data, size_t data_size_bytes, size_t* 
bytes_consumed);
+
+  /*! \brief Reset unframer to initial state. */
+  void Reset();
+
+  /*! \brief Return an underestimate of the number of bytes needed from the 
wire. */
+  size_t BytesNeeded();
+
+ private:
+  tvm_crt_error_t FindPacketStart();
+  tvm_crt_error_t FindPacketLength();
+  tvm_crt_error_t FindPacketCrc();
+  tvm_crt_error_t FindCrcEnd();
+
+  inline bool is_buffer_full(size_t buffer_full_bytes) {
+    return num_buffer_bytes_valid_ >= buffer_full_bytes;
+  }
+
+  /*! \brief Consume input into buffer_ until buffer_ has buffer_full_bytes. */
+  tvm_crt_error_t AddToBuffer(size_t buffer_full_bytes, bool update_crc);
+
+  void ClearBuffer();
+
+  /*! \brief Unescape and consume input bytes, storing into buffer.
+   *
+   * \param buffer A buffer to fill with consumed, unescaped bytes.
+   * \param buffer_size_bytes Size of buffer, in bytes.
+   * \param bytes_filled A pointer to an accumulator to which is added the 
number of bytes written
+   *      to `buffer`.
+   * \param update_crc true when the CRC should be updated with the escaped 
bytes.
+   * \return
+   *     - kTvmErrorNoError if successful
+   *     - kTvmErrorFramingShortPacket if a start-of-packet escape code was 
encountered. If so,
+   *       *bytes_filled indicates the number of bytes before the 
Escape::kEscapeStart byte.
+   *     - kTvmErrorFramingInvalidEscape if an invalid escape sequence was 
seen.
+   *     - kTvmErrorWriteStreamShortWrite if the WriteStream passed to 
constructor's Write()
+   *       function returns 0.
+   *     - kTvmErrorWriteStreamShortWrite if the WriteStream passed to 
constructor's Write()
+   *       function returns an invalid positive number.
+   *     - Any negative value (i.e. with bits in kTvmErrorSystemErrorMask set) 
returned by the
+   *       WriteStream's Write() function.
+   */

Review comment:
       Given rpc_common is quite internal to the uTVM RPC protocol, perhaps we 
should move them as internal header.

##########
File path: src/runtime/crt/utvm_rpc_common/framing.cc
##########
@@ -0,0 +1,409 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file framing.cc
+ * \brief Framing for RPC.
+ */
+
+#include <string.h>
+#include <tvm/runtime/crt/logging.h>
+#include <tvm/runtime/crt/rpc_common/framing.h>
+
+#include "crt_config.h"
+
+// For debugging purposes, Framer logs can be enabled, but this should only be 
done when
+// running from the host. This is done differently from TVMLogf() because 
TVMLogf() uses the
+// framer in its implementation.
+#ifdef TVM_CRT_FRAMER_ENABLE_LOGS
+#include <cstdio>
+#define TVM_FRAMER_DEBUG_LOG(msg, ...) fprintf(stderr, "utvm framer: " msg " 
\n", ##__VA_ARGS__)
+#define TVM_UNFRAMER_DEBUG_LOG(msg, ...) fprintf(stderr, "utvm unframer: " msg 
" \n", ##__VA_ARGS__)
+#else
+#define TVM_FRAMER_DEBUG_LOG(msg, ...)
+#define TVM_UNFRAMER_DEBUG_LOG(msg, ...)
+#endif
+
+namespace tvm {
+namespace runtime {
+
+template <typename E>
+static constexpr uint8_t to_integral(E e) {
+  return static_cast<uint8_t>(e);
+}
+
+void Unframer::Reset() {
+  state_ = State::kFindPacketStart;
+  saw_escape_start_ = false;
+  num_buffer_bytes_valid_ = 0;
+}
+
+tvm_crt_error_t Unframer::Write(const uint8_t* data, size_t data_size_bytes,
+                                size_t* bytes_consumed) {
+  tvm_crt_error_t return_code = kTvmErrorNoError;
+  input_ = data;
+  input_size_bytes_ = data_size_bytes;
+
+  while (return_code == kTvmErrorNoError && input_size_bytes_ > 0) {
+    TVM_UNFRAMER_DEBUG_LOG("state: %02x size 0x%02zx", to_integral(state_), 
input_size_bytes_);
+    switch (state_) {
+      case State::kFindPacketStart:
+        return_code = FindPacketStart();
+        break;
+      case State::kFindPacketLength:
+        return_code = FindPacketLength();
+        break;
+      case State::kFindPacketCrc:
+        return_code = FindPacketCrc();
+        break;
+      case State::kFindCrcEnd:
+        return_code = FindCrcEnd();
+        break;
+      default:
+        return_code = kTvmErrorFramingInvalidState;
+        break;
+    }
+  }
+
+  *bytes_consumed = data_size_bytes - input_size_bytes_;
+  input_ = nullptr;
+  input_size_bytes_ = 0;
+
+  if (return_code != kTvmErrorNoError) {
+    state_ = State::kFindPacketStart;
+    ClearBuffer();
+  }
+
+  return return_code;
+}
+
+tvm_crt_error_t Unframer::FindPacketStart() {
+  size_t i;
+  for (i = 0; i < input_size_bytes_; i++) {

Review comment:
       prefer ++i over i++

##########
File path: include/tvm/runtime/crt/rpc_common/session.h
##########
@@ -0,0 +1,236 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+/*!
+ * \file session.h
+ * \brief RPC Session
+ */
+
+#ifndef TVM_RUNTIME_CRT_RPC_COMMON_SESSION_H_
+#define TVM_RUNTIME_CRT_RPC_COMMON_SESSION_H_
+
+#include <inttypes.h>
+#include <tvm/runtime/crt/error_codes.h>
+#include <tvm/runtime/crt/rpc_common/buffer.h>
+#include <tvm/runtime/crt/rpc_common/framing.h>
+#include <tvm/runtime/crt/rpc_common/write_stream.h>
+
+namespace tvm {
+namespace runtime {
+
+enum class MessageType : uint8_t {
+  kStartSessionInit = 0x00,
+  kStartSessionReply = 0x01,
+  kTerminateSession = 0x02,
+  kLog = 0x03,
+  kNormal = 0x10,
+};
+
+typedef struct SessionHeader {
+  uint16_t session_id;
+  MessageType message_type;
+} __attribute__((packed)) SessionHeader;
+
+/*!
+ * \brief CRT communication session management class.
+ * Assumes the following properties provided by the underlying transport:
+ *  - in-order delivery.
+ *  - reliable delivery.
+ *
+ * Specifically, designed for use with UARTs. Will probably work over 
semihosting, USB, and TCP;
+ * will probably not work reliably enough over UDP.
+ */
+class Session {

Review comment:
       Session is a quite generic class name, let us either add a prefix, or 
add a micro namespace




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to