You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2017/12/15 02:20:52 UTC

[trafficserver] branch quic-latest updated: draft-08: Support new ACK frame format

This is an automated email from the ASF dual-hosted git repository.

masaori pushed a commit to branch quic-latest
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/quic-latest by this push:
     new 5b9acb9  draft-08: Support new ACK frame format
5b9acb9 is described below

commit 5b9acb9618f5c54e6d34a531374edee19d702e6f
Author: Masaori Koshiba <ma...@apache.org>
AuthorDate: Fri Dec 15 11:20:34 2017 +0900

    draft-08: Support new ACK frame format
---
 iocore/net/quic/QUICFrame.cc                     | 307 ++++++++++++-----------
 iocore/net/quic/QUICFrame.h                      | 101 ++++----
 iocore/net/quic/QUICLossDetector.cc              |   2 +-
 iocore/net/quic/test/test_QUICAckFrameCreator.cc |  10 +-
 iocore/net/quic/test/test_QUICFrame.cc           | 138 ++++++----
 5 files changed, 300 insertions(+), 258 deletions(-)

diff --git a/iocore/net/quic/QUICFrame.cc b/iocore/net/quic/QUICFrame.cc
index fd237ad..5160763 100644
--- a/iocore/net/quic/QUICFrame.cc
+++ b/iocore/net/quic/QUICFrame.cc
@@ -327,8 +327,7 @@ void
 QUICAckFrame::reset(const uint8_t *buf, size_t len)
 {
   QUICFrame::reset(buf, len);
-  this->_ack_block_section =
-    new AckBlockSection(buf + this->_get_ack_block_section_offset(), this->num_blocks(), this->_get_ack_block_length());
+  this->_ack_block_section = new AckBlockSection(buf + this->_get_ack_block_section_offset(), this->ack_block_count());
 }
 
 QUICFrameType
@@ -340,104 +339,62 @@ QUICAckFrame::type() const
 size_t
 QUICAckFrame::size() const
 {
-  if (this->_buf) {
-    return this->_get_ack_block_section_offset() + this->ack_block_section()->size();
-  } else {
-    // TODO Not implemented
-    return 0;
-  }
+  return this->_get_ack_block_section_offset() + this->_ack_block_section->size();
 }
 
 void
 QUICAckFrame::store(uint8_t *buf, size_t *len) const
 {
-  size_t n;
   uint8_t *p = buf;
+  size_t n;
+  *p = static_cast<uint8_t>(QUICFrameType::ACK);
+  ++p;
 
-  // Build Frame Type: "101NLLMM"
-  buf[0] = static_cast<uint8_t>(QUICFrameType::ACK);
-  p += 1;
-
-  // "N" of "101NLLMM"
-  if (this->_ack_block_section->count() > 0) {
-    buf[0] += 0x10;
-    *p = this->_ack_block_section->count();
-    p += 1;
-  }
-
-  // "LL" of "101NLLMM"
-  if (this->_largest_acknowledged <= 0xff) {
-    QUICTypeUtil::write_uint_as_nbytes(this->_largest_acknowledged, 1, p, &n);
-  } else if (this->_largest_acknowledged <= 0xffff) {
-    buf[0] += 0x01 << 2;
-    QUICTypeUtil::write_uint_as_nbytes(this->_largest_acknowledged, 2, p, &n);
-  } else if (this->_largest_acknowledged <= 0xffffffff) {
-    buf[0] += 0x02 << 2;
-    QUICTypeUtil::write_uint_as_nbytes(this->_largest_acknowledged, 4, p, &n);
-  } else {
-    buf[0] += 0x03 << 2;
-    QUICTypeUtil::write_uint_as_nbytes(this->_largest_acknowledged, 8, p, &n);
-  }
+  QUICTypeUtil::write_QUICVariableInt(this->_largest_acknowledged, p, &n);
   p += n;
-
-  QUICTypeUtil::write_uint_as_nbytes(this->_ack_delay, 2, p, &n);
+  QUICTypeUtil::write_QUICVariableInt(this->_ack_delay, p, &n);
+  p += n;
+  QUICTypeUtil::write_QUICVariableInt(this->ack_block_count(), p, &n);
   p += n;
-
-  // "MM" of "101NLLMM"
-  // use 32 bit length for now
-  // TODO The length should be returned by ackBlockSection
-  buf[0] += 0x02;
   this->_ack_block_section->store(p, &n);
   p += n;
 
   *len = p - buf;
-}
 
-uint8_t
-QUICAckFrame::num_blocks() const
-{
-  if (this->has_ack_blocks()) {
-    if (this->_buf) {
-      return this->_buf[1];
-    } else {
-      return this->_ack_block_section->count();
-    }
-  } else {
-    return 0;
-  }
+  return;
 }
 
 QUICPacketNumber
 QUICAckFrame::largest_acknowledged() const
 {
   if (this->_buf) {
-    return QUICTypeUtil::read_QUICPacketNumber(this->_buf + this->_get_largest_acknowledged_offset(),
-                                               this->_get_largest_acknowledged_length());
+    uint64_t largest_acknowledged;
+    size_t encoded_len;
+    QUICVariableInt::decode(largest_acknowledged, encoded_len, this->_buf + this->_get_largest_acknowledged_offset(),
+                            this->_len - this->_get_largest_acknowledged_offset());
+    return largest_acknowledged;
   } else {
     return this->_largest_acknowledged;
   }
 }
 
-uint16_t
+uint64_t
 QUICAckFrame::ack_delay() const
 {
   if (this->_buf) {
-    return QUICTypeUtil::read_nbytes_as_uint(this->_buf + this->_get_ack_delay_offset(), 2);
+    return QUICTypeUtil::read_QUICVariableInt(this->_buf + this->_get_ack_delay_offset());
   } else {
     return this->_ack_delay;
   }
 }
 
-/**
- * N of 101NLLMM
- */
-bool
-QUICAckFrame::has_ack_blocks() const
+uint64_t
+QUICAckFrame::ack_block_count() const
 {
   if (this->_buf) {
-    return (this->_buf[0] & 0x10) != 0;
+    return QUICTypeUtil::read_QUICVariableInt(this->_buf + this->_get_ack_block_count_offset());
   } else {
-    return this->_ack_block_section->count() != 0;
+    return this->_ack_block_section->count();
   }
 }
 
@@ -453,101 +410,123 @@ QUICAckFrame::ack_block_section() const
   return this->_ack_block_section;
 }
 
-/**
- * LL of 101NLLMM
- */
+size_t
+QUICAckFrame::_get_largest_acknowledged_offset() const
+{
+  return sizeof(QUICFrameType);
+}
+
 size_t
 QUICAckFrame::_get_largest_acknowledged_length() const
 {
-  /*
-   * 0 -> 1 byte
-   * 1 -> 2 byte
-   * 2 -> 4 byte
-   * 3 -> 8 byte
-   */
-  int n = (this->_buf[0] & 0x0c) >> 2;
-  return 0x01 << n;
+  if (this->_buf) {
+    return QUICVariableInt::size(this->_buf + this->_get_largest_acknowledged_offset());
+  } else {
+    return QUICVariableInt::size(this->_largest_acknowledged);
+  }
 }
 
 size_t
-QUICAckFrame::_get_largest_acknowledged_offset() const
+QUICAckFrame::_get_ack_delay_offset() const
 {
-  if (this->has_ack_blocks()) {
-    return 2;
+  return this->_get_largest_acknowledged_offset() + this->_get_largest_acknowledged_length();
+}
+
+size_t
+QUICAckFrame::_get_ack_delay_length() const
+{
+  if (this->_buf) {
+    return QUICVariableInt::size(this->_buf + this->_get_ack_delay_offset());
   } else {
-    return 1;
+    return QUICVariableInt::size(this->_ack_delay);
   }
 }
 
-/**
- * MM of 101NLLMM
- */
 size_t
-QUICAckFrame::_get_ack_block_length() const
+QUICAckFrame::_get_ack_block_count_offset() const
 {
-  /*
-   * 0 -> 1 byte
-   * 1 -> 2 byte
-   * 2 -> 4 byte
-   * 3 -> 8 byte
-   */
-  int n = this->_buf[0] & 0x03;
-  return 0x01 << n;
+  return this->_get_ack_delay_offset() + this->_get_ack_delay_length();
 }
 
 size_t
-QUICAckFrame::_get_ack_delay_offset() const
+QUICAckFrame::_get_ack_block_count_length() const
 {
-  return this->_get_largest_acknowledged_offset() + this->_get_largest_acknowledged_length();
+  if (this->_buf) {
+    return QUICVariableInt::size(this->_buf + this->_get_ack_block_count_offset());
+  } else {
+    return QUICVariableInt::size(this->ack_block_count());
+  }
 }
 
 size_t
 QUICAckFrame::_get_ack_block_section_offset() const
 {
-  return this->_get_ack_delay_offset() + 2;
+  return this->_get_ack_block_count_offset() + this->_get_ack_block_count_length();
 }
 
-QUICAckFrame::AckBlockSection::AckBlockSection(const uint8_t *buf, uint8_t num_blocks, uint8_t ack_block_length)
+//
+// QUICAckFrame::AckBlock
+//
+uint64_t
+QUICAckFrame::AckBlock::gap() const
 {
-  this->_buf              = buf;
-  this->_num_blocks       = num_blocks;
-  this->_ack_block_length = ack_block_length;
+  if (this->_buf) {
+    return QUICTypeUtil::read_QUICVariableInt(this->_buf);
+  } else {
+    return this->_gap;
+  }
 }
 
-QUICAckFrame::AckBlockSection::AckBlockSection(uint64_t first_ack_block_length)
+uint64_t
+QUICAckFrame::AckBlock::length() const
 {
-  this->_first_ack_block_length = first_ack_block_length;
+  if (this->_buf) {
+    return QUICTypeUtil::read_QUICVariableInt(this->_buf + this->_get_gap_size());
+  } else {
+    return this->_length;
+  }
 }
 
-QUICAckFrame::AckBlock::AckBlock(const uint8_t *buf, uint8_t ack_block_length)
+size_t
+QUICAckFrame::AckBlock::size() const
 {
-  this->_gap    = buf[0];
-  this->_length = QUICTypeUtil::read_nbytes_as_uint(buf + 1, ack_block_length);
+  return this->_get_gap_size() + this->_get_length_size();
 }
 
-QUICAckFrame::AckBlock::AckBlock(uint8_t gap, uint64_t length)
+const uint8_t *
+QUICAckFrame::AckBlock::buf() const
 {
-  this->_gap    = gap;
-  this->_length = length;
+  return this->_buf;
 }
 
-uint8_t
-QUICAckFrame::AckBlock::gap() const
+size_t
+QUICAckFrame::AckBlock::_get_gap_size() const
 {
-  return this->_gap;
+  if (this->_buf) {
+    return QUICVariableInt::size(this->_buf);
+  } else {
+    return QUICVariableInt::size(this->_gap);
+  }
 }
 
-uint64_t
-QUICAckFrame::AckBlock::length() const
+size_t
+QUICAckFrame::AckBlock::_get_length_size() const
 {
-  return this->_length;
+  if (this->_buf) {
+    return QUICVariableInt::size(this->_buf + this->_get_gap_size());
+  } else {
+    return QUICVariableInt::size(this->_gap);
+  }
 }
 
+//
+// QUICAckFrame::AckBlockSection
+//
 uint8_t
 QUICAckFrame::AckBlockSection::count() const
 {
   if (this->_buf) {
-    return this->_num_blocks;
+    return this->_ack_block_count;
   } else {
     return this->_ack_blocks.size();
   }
@@ -556,27 +535,33 @@ QUICAckFrame::AckBlockSection::count() const
 size_t
 QUICAckFrame::AckBlockSection::size() const
 {
-  if (this->_buf) {
-    return this->_ack_block_length + (this->_ack_block_length + 1) * this->_num_blocks;
-  } else {
-    // TODO Which block length should we use?
-    return 48 + (48 + 1) * this->_ack_blocks.size();
+  size_t n = 0;
+
+  n += this->_get_first_ack_block_length_size();
+
+  for (auto &&block : *this) {
+    n += block.size();
   }
+
+  return n;
 }
 
 void
 QUICAckFrame::AckBlockSection::store(uint8_t *buf, size_t *len) const
 {
+  size_t n;
   uint8_t *p = buf;
-  size_t dummy;
-  QUICTypeUtil::write_uint_as_nbytes(this->_first_ack_block_length, 4, buf, &dummy);
-  p += 4;
+
+  QUICTypeUtil::write_QUICVariableInt(this->_first_ack_block_length, p, &n);
+  p += n;
+
   for (auto &&block : *this) {
-    p[0] = block.gap();
-    p += 1;
-    QUICTypeUtil::write_uint_as_nbytes(block.length(), 4, buf, &dummy);
-    p += 4;
+    QUICTypeUtil::write_QUICVariableInt(block.gap(), p, &n);
+    p += n;
+    QUICTypeUtil::write_QUICVariableInt(block.length(), p, &n);
+    p += n;
   }
+
   *len = p - buf;
 }
 
@@ -584,7 +569,7 @@ uint64_t
 QUICAckFrame::AckBlockSection::first_ack_block_length() const
 {
   if (this->_buf) {
-    return QUICTypeUtil::read_nbytes_as_uint(this->_buf, this->_ack_block_length);
+    return QUICTypeUtil::read_QUICVariableInt(this->_buf);
   } else {
     return this->_first_ack_block_length;
   }
@@ -600,7 +585,7 @@ QUICAckFrame::AckBlockSection::const_iterator
 QUICAckFrame::AckBlockSection::begin() const
 {
   if (this->_buf) {
-    return const_iterator(0, this->_buf, this->_num_blocks, this->_ack_block_length);
+    return const_iterator(0, this->_buf + this->_get_first_ack_block_length_size(), this->_ack_block_count);
   } else {
     return const_iterator(0, &this->_ack_blocks);
   }
@@ -610,39 +595,79 @@ QUICAckFrame::AckBlockSection::const_iterator
 QUICAckFrame::AckBlockSection::end() const
 {
   if (this->_buf) {
-    return const_iterator(this->_num_blocks, this->_buf, this->_num_blocks, this->_ack_block_length);
+    return const_iterator(this->_ack_block_count, this->_buf, this->_ack_block_count);
   } else {
     return const_iterator(this->_ack_blocks.size(), &this->_ack_blocks);
   }
 }
 
-QUICAckFrame::AckBlockSection::const_iterator::const_iterator(uint8_t index, const uint8_t *buf, uint8_t num_blocks,
-                                                              uint8_t ack_block_length)
+size_t
+QUICAckFrame::AckBlockSection::_get_first_ack_block_length_size() const
 {
-  this->_index            = index;
-  this->_buf              = buf;
-  this->_ack_block_length = ack_block_length;
-  if (index < num_blocks) {
-    this->_current_block = AckBlock(buf + ack_block_length + (1 + ack_block_length) * index, ack_block_length);
+  if (this->_buf) {
+    return QUICVariableInt::size(this->_buf);
   } else {
-    this->_current_block = {static_cast<uint8_t>(0), 0ULL};
+    return QUICVariableInt::size(this->_first_ack_block_length);
   }
 }
 
-QUICAckFrame::AckBlockSection::const_iterator::const_iterator(uint8_t index, const std::vector<QUICAckFrame::AckBlock> *ack_block)
+//
+// QUICAckFrame::AckBlockSection::const_iterator
+//
+QUICAckFrame::AckBlockSection::const_iterator::const_iterator(uint8_t index, const uint8_t *buf, uint8_t ack_block_count)
+  : _index(index), _buf(buf)
+{
+  if (index == 0) {
+    this->_current_block = AckBlock(this->_buf);
+  } else if (index < ack_block_count) {
+    this->_current_block = AckBlock(this->_current_block.buf() + this->_current_block.size());
+  } else {
+    this->_current_block = {UINT64_C(0), UINT64_C(0)};
+  }
+}
+
+QUICAckFrame::AckBlockSection::const_iterator::const_iterator(uint8_t index, const std::vector<QUICAckFrame::AckBlock> *ack_blocks)
+  : _index(index), _ack_blocks(ack_blocks)
 {
-  this->_index      = index;
-  this->_buf        = nullptr;
-  this->_ack_blocks = ack_block;
   if (this->_ack_blocks->size()) {
     if (this->_ack_blocks->size() == this->_index) {
-      this->_current_block = {static_cast<uint8_t>(0), 0ULL};
+      this->_current_block = {UINT64_C(0), UINT64_C(0)};
     } else {
       this->_current_block = this->_ack_blocks->at(this->_index);
     }
   }
 }
 
+// FIXME: something wrong with clang-format?
+const QUICAckFrame::AckBlock &QUICAckFrame::AckBlockSection::const_iterator::operator++()
+{
+  ++(this->_index);
+
+  if (this->_buf) {
+    this->_current_block = AckBlock(this->_current_block.buf() + this->_current_block.size());
+  } else {
+    if (this->_ack_blocks->size() == this->_index) {
+      this->_current_block = {UINT64_C(0), UINT64_C(0)};
+    } else {
+      this->_current_block = this->_ack_blocks->at(this->_index);
+    }
+  }
+
+  return this->_current_block;
+}
+
+const bool
+QUICAckFrame::AckBlockSection::const_iterator::operator!=(const const_iterator &ite) const
+{
+  return this->_index != ite._index;
+}
+
+const bool
+QUICAckFrame::AckBlockSection::const_iterator::operator==(const const_iterator &ite) const
+{
+  return this->_index == ite._index;
+}
+
 //
 // RST_STREAM frame
 //
diff --git a/iocore/net/quic/QUICFrame.h b/iocore/net/quic/QUICFrame.h
index 151d317..74e03a8 100644
--- a/iocore/net/quic/QUICFrame.h
+++ b/iocore/net/quic/QUICFrame.h
@@ -99,15 +99,23 @@ public:
   class AckBlock
   {
   public:
-    AckBlock(const uint8_t *buf, uint8_t ack_block_length);
-    AckBlock(uint8_t gap, uint64_t length);
-    uint8_t gap() const;
+    AckBlock(const uint8_t *b) : _buf(b) {}
+    AckBlock(uint64_t g, uint64_t l) : _gap(g), _length(l) {}
+
+    uint64_t gap() const;
     uint64_t length() const;
+    size_t size() const;
+    const uint8_t *buf() const;
+
     LINK(QUICAckFrame::AckBlock, link);
 
   private:
-    uint8_t _gap     = 0;
-    uint64_t _length = 0;
+    size_t _get_gap_size() const;
+    size_t _get_length_size() const;
+
+    const uint8_t *_buf = nullptr;
+    uint64_t _gap       = 0;
+    uint64_t _length    = 0;
   };
 
   class AckBlockSection
@@ -116,51 +124,24 @@ public:
     class const_iterator : public std::iterator<std::input_iterator_tag, QUICAckFrame::AckBlock>
     {
     public:
-      const_iterator(uint8_t index, const uint8_t *buf, uint8_t num_blocks, uint8_t ack_block_length);
+      const_iterator(uint8_t index, const uint8_t *buf, uint8_t ack_block_count);
       const_iterator(uint8_t index, const std::vector<QUICAckFrame::AckBlock> *ack_blocks);
 
       const QUICAckFrame::AckBlock &operator*() const { return this->_current_block; };
       const QUICAckFrame::AckBlock *operator->() const { return &this->_current_block; };
-      const QUICAckFrame::AckBlock &operator++()
-      {
-        ++(this->_index);
-
-        if (this->_buf) {
-          this->_current_block =
-            AckBlock(this->_buf + this->_ack_block_length + (1 + this->_ack_block_length) * this->_index, this->_ack_block_length);
-        } else {
-          if (this->_ack_blocks->size() == this->_index) {
-            this->_current_block = {static_cast<uint8_t>(0), 0ULL};
-          } else {
-            this->_current_block = this->_ack_blocks->at(this->_index);
-          }
-        }
-
-        return this->_current_block;
-      };
-
-      const bool
-      operator!=(const const_iterator &ite) const
-      {
-        return this->_index != ite._index;
-      };
-
-      const bool
-      operator==(const const_iterator &ite) const
-      {
-        return this->_index == ite._index;
-      };
+      const QUICAckFrame::AckBlock &operator++();
+      const bool operator!=(const const_iterator &ite) const;
+      const bool operator==(const const_iterator &ite) const;
 
     private:
-      uint8_t _index;
-      const uint8_t *_buf;
-      uint8_t _ack_block_length;
+      uint8_t _index                                         = 0;
+      const uint8_t *_buf                                    = nullptr;
+      QUICAckFrame::AckBlock _current_block                  = {UINT64_C(0), UINT64_C(0)};
       const std::vector<QUICAckFrame::AckBlock> *_ack_blocks = nullptr;
-      QUICAckFrame::AckBlock _current_block                  = {static_cast<uint8_t>(0), 0ULL};
     };
 
-    AckBlockSection(uint64_t first_ack_block_length);
-    AckBlockSection(const uint8_t *buf, uint8_t num_blocks, uint8_t ack_block_length);
+    AckBlockSection(uint64_t first_ack_block_length) : _first_ack_block_length(first_ack_block_length) {}
+    AckBlockSection(const uint8_t *buf, uint8_t ack_block_count) : _buf(buf), _ack_block_count(ack_block_count) {}
     uint8_t count() const;
     size_t size() const;
     void store(uint8_t *buf, size_t *len) const;
@@ -170,36 +151,41 @@ public:
     const_iterator end() const;
 
   private:
+    size_t _get_first_ack_block_length_size() const;
+
     const uint8_t *_buf              = nullptr;
     uint64_t _first_ack_block_length = 0;
-    uint8_t _num_blocks              = 0;
-    uint8_t _ack_block_length        = 0;
+    uint8_t _ack_block_count         = 0;
     std::vector<QUICAckFrame::AckBlock> _ack_blocks;
   };
 
   QUICAckFrame() : QUICFrame() {}
   QUICAckFrame(const uint8_t *buf, size_t len);
   QUICAckFrame(QUICPacketNumber largest_acknowledged, uint16_t ack_delay, uint64_t first_ack_block_length);
+
   virtual ~QUICAckFrame();
   virtual void reset(const uint8_t *buf, size_t len) override;
   virtual QUICFrameType type() const override;
   virtual size_t size() const override;
   virtual void store(uint8_t *buf, size_t *len) const override;
-  uint8_t num_blocks() const;
+
   QUICPacketNumber largest_acknowledged() const;
-  uint16_t ack_delay() const;
+  uint64_t ack_delay() const;
+  uint64_t ack_block_count() const;
   const AckBlockSection *ack_block_section() const;
   AckBlockSection *ack_block_section();
-  bool has_ack_blocks() const;
 
 private:
   size_t _get_largest_acknowledged_offset() const;
   size_t _get_largest_acknowledged_length() const;
-  size_t _get_ack_block_length() const;
   size_t _get_ack_delay_offset() const;
+  size_t _get_ack_delay_length() const;
+  size_t _get_ack_block_count_offset() const;
+  size_t _get_ack_block_count_length() const;
   size_t _get_ack_block_section_offset() const;
+
   QUICPacketNumber _largest_acknowledged = 0;
-  uint16_t _ack_delay                    = 0;
+  uint64_t _ack_delay                    = 0;
   AckBlockSection *_ack_block_section    = nullptr;
 };
 
@@ -228,7 +214,7 @@ private:
   size_t _get_error_code_field_offset() const;
   size_t _get_final_offset_field_offset() const;
   size_t _get_final_offset_field_length() const;
-  
+
   QUICStreamId _stream_id      = 0;
   QUICAppErrorCode _error_code = 0;
   QUICOffset _final_offset     = 0;
@@ -281,7 +267,7 @@ private:
   size_t _get_reason_phrase_length_field_offset() const;
   size_t _get_reason_phrase_length_field_length() const;
   size_t _get_reason_phrase_field_offset() const;
-  
+
   QUICTransErrorCode _error_code;
   uint64_t _reason_phrase_length = 0;
   const char *_reason_phrase     = nullptr;
@@ -390,8 +376,8 @@ class QUICBlockedFrame : public QUICFrame
 public:
   QUICBlockedFrame() : QUICFrame() {}
   QUICBlockedFrame(const uint8_t *buf, size_t len) : QUICFrame(buf, len) {}
-  QUICBlockedFrame(QUICOffset offset) : _offset(offset) {};
-  
+  QUICBlockedFrame(QUICOffset offset) : _offset(offset){};
+
   virtual QUICFrameType type() const override;
   virtual size_t size() const override;
   virtual void store(uint8_t *buf, size_t *len) const override;
@@ -413,7 +399,7 @@ class QUICStreamBlockedFrame : public QUICFrame
 public:
   QUICStreamBlockedFrame() : QUICFrame() {}
   QUICStreamBlockedFrame(const uint8_t *buf, size_t len) : QUICFrame(buf, len) {}
-  QUICStreamBlockedFrame(QUICStreamId s, QUICOffset o): _stream_id(s), _offset(o) {};
+  QUICStreamBlockedFrame(QUICStreamId s, QUICOffset o) : _stream_id(s), _offset(o){};
 
   virtual QUICFrameType type() const override;
   virtual size_t size() const override;
@@ -428,7 +414,7 @@ private:
   size_t _get_offset_field_length() const;
 
   QUICStreamId _stream_id = 0;
-  QUICOffset _offset = 0;  
+  QUICOffset _offset      = 0;
 };
 
 //
@@ -463,7 +449,7 @@ public:
   QUICNewConnectionIdFrame() : QUICFrame() {}
   QUICNewConnectionIdFrame(const uint8_t *buf, size_t len) : QUICFrame(buf, len) {}
   QUICNewConnectionIdFrame(uint64_t seq, QUICConnectionId id, QUICStatelessResetToken token)
-      : _sequence(seq), _connection_id(id), _stateless_reset_token(token) {};
+    : _sequence(seq), _connection_id(id), _stateless_reset_token(token){};
 
   virtual QUICFrameType type() const override;
   virtual size_t size() const override;
@@ -504,7 +490,7 @@ private:
   size_t _get_stream_id_field_length() const;
   size_t _get_error_code_field_offset() const;
 
-  QUICStreamId _stream_id = 0;
+  QUICStreamId _stream_id      = 0;
   QUICAppErrorCode _error_code = 0;
 };
 
@@ -729,7 +715,8 @@ public:
   /*
    * Creates a STREAM_BLOCKED frame.
    */
-  static std::unique_ptr<QUICStreamBlockedFrame, QUICFrameDeleterFunc> create_stream_blocked_frame(QUICStreamId stream_id, QUICOffset offset);
+  static std::unique_ptr<QUICStreamBlockedFrame, QUICFrameDeleterFunc> create_stream_blocked_frame(QUICStreamId stream_id,
+                                                                                                   QUICOffset offset);
 
   /*
    * Creates a STREAM_ID_BLOCKED frame.
diff --git a/iocore/net/quic/QUICLossDetector.cc b/iocore/net/quic/QUICLossDetector.cc
index d735ae4..45896b0 100644
--- a/iocore/net/quic/QUICLossDetector.cc
+++ b/iocore/net/quic/QUICLossDetector.cc
@@ -194,7 +194,7 @@ QUICLossDetector::_on_ack_received(const std::shared_ptr<const QUICAckFrame> &ac
   if (pi != this->_sent_packets.end()) {
     this->_latest_rtt = Thread::get_hrtime() - pi->second->time;
     // _latest_rtt is nanosecond but ack_frame->ack_delay is millisecond
-    if (this->_latest_rtt > HRTIME_MSECONDS(ack_frame->ack_delay())) {
+    if (this->_latest_rtt > static_cast<ink_hrtime>(HRTIME_MSECONDS(ack_frame->ack_delay()))) {
       this->_latest_rtt -= HRTIME_MSECONDS(ack_frame->ack_delay());
     }
     this->_update_rtt(this->_latest_rtt);
diff --git a/iocore/net/quic/test/test_QUICAckFrameCreator.cc b/iocore/net/quic/test/test_QUICAckFrameCreator.cc
index 09e742d..6153460 100644
--- a/iocore/net/quic/test/test_QUICAckFrameCreator.cc
+++ b/iocore/net/quic/test/test_QUICAckFrameCreator.cc
@@ -39,7 +39,7 @@ TEST_CASE("QUICAckFrameCreator", "[quic]")
   creator.update(1, true);
   frame = creator.create();
   CHECK(frame != nullptr);
-  CHECK(frame->num_blocks() == 0);
+  CHECK(frame->ack_block_count() == 0);
   CHECK(frame->largest_acknowledged() == 1);
   CHECK(frame->ack_block_section()->first_ack_block_length() == 0);
 
@@ -53,7 +53,7 @@ TEST_CASE("QUICAckFrameCreator", "[quic]")
   creator.update(4, true);
   frame = creator.create();
   CHECK(frame != nullptr);
-  CHECK(frame->num_blocks() == 0);
+  CHECK(frame->ack_block_count() == 0);
   CHECK(frame->largest_acknowledged() == 5);
   CHECK(frame->ack_block_section()->first_ack_block_length() == 3);
 
@@ -63,7 +63,7 @@ TEST_CASE("QUICAckFrameCreator", "[quic]")
   creator.update(10, true);
   frame = creator.create();
   CHECK(frame != nullptr);
-  CHECK(frame->num_blocks() == 1);
+  CHECK(frame->ack_block_count() == 1);
   CHECK(frame->largest_acknowledged() == 10);
   CHECK(frame->ack_block_section()->first_ack_block_length() == 0);
   CHECK(frame->ack_block_section()->begin()->gap() == 1);
@@ -86,7 +86,7 @@ TEST_CASE("QUICAckFrameCreator_loss_recover", "[quic]")
 
   frame = creator.create();
   CHECK(frame != nullptr);
-  CHECK(frame->num_blocks() == 2);
+  CHECK(frame->ack_block_count() == 2);
   CHECK(frame->largest_acknowledged() == 9);
   CHECK(frame->ack_block_section()->first_ack_block_length() == 1);
   CHECK(frame->ack_block_section()->begin()->gap() == 0);
@@ -98,7 +98,7 @@ TEST_CASE("QUICAckFrameCreator_loss_recover", "[quic]")
   creator.update(4, true);
   frame = creator.create();
   CHECK(frame != nullptr);
-  CHECK(frame->num_blocks() == 1);
+  CHECK(frame->ack_block_count() == 1);
   CHECK(frame->largest_acknowledged() == 7);
   CHECK(frame->ack_block_section()->first_ack_block_length() == 0);
   CHECK(frame->ack_block_section()->begin()->gap() == 1);
diff --git a/iocore/net/quic/test/test_QUICFrame.cc b/iocore/net/quic/test/test_QUICFrame.cc
index 280aae4..f9daf5f 100644
--- a/iocore/net/quic/test/test_QUICFrame.cc
+++ b/iocore/net/quic/test/test_QUICFrame.cc
@@ -267,98 +267,128 @@ TEST_CASE("Load Ack Frame 1", "[quic]")
   SECTION("0 Ack Block, 8 bit packet number length, 8 bit block length")
   {
     uint8_t buf1[] = {
-      0xA0,       // 101NLLMM
+      0x0e,       // Type
       0x12,       // Largest Acknowledged
-      0x34, 0x56, // Ack Delay
+      0x74, 0x56, // Ack Delay
+      0x00,       // Ack Block Count
       0x00,       // Ack Block Section
     };
     std::shared_ptr<const QUICFrame> frame1 = QUICFrameFactory::create(buf1, sizeof(buf1));
     CHECK(frame1->type() == QUICFrameType::ACK);
-    CHECK(frame1->size() == 5);
+    CHECK(frame1->size() == 6);
     std::shared_ptr<const QUICAckFrame> ackFrame1 = std::dynamic_pointer_cast<const QUICAckFrame>(frame1);
     CHECK(ackFrame1 != nullptr);
-    CHECK(ackFrame1->has_ack_blocks() == false);
+    CHECK(ackFrame1->ack_block_count() == 0);
     CHECK(ackFrame1->largest_acknowledged() == 0x12);
     CHECK(ackFrame1->ack_delay() == 0x3456);
   }
 
+  SECTION("0 Ack Block, 8 bit packet number length, 8 bit block length")
+  {
+    uint8_t buf1[] = {
+      0x0e,                   // Type
+      0x80, 0x00, 0x00, 0x01, // Largest Acknowledged
+      0x41, 0x71,             // Ack Delay
+      0x00,                   // Ack Block Count
+      0x80, 0x00, 0x00, 0x01, // Ack Block Section (First ACK Block Length)
+    };
+    std::shared_ptr<const QUICFrame> frame1 = QUICFrameFactory::create(buf1, sizeof(buf1));
+    CHECK(frame1->type() == QUICFrameType::ACK);
+    CHECK(frame1->size() == 12);
+
+    std::shared_ptr<const QUICAckFrame> ackFrame1 = std::dynamic_pointer_cast<const QUICAckFrame>(frame1);
+    CHECK(ackFrame1 != nullptr);
+    CHECK(ackFrame1->largest_acknowledged() == 0x01);
+    CHECK(ackFrame1->ack_delay() == 0x0171);
+    CHECK(ackFrame1->ack_block_count() == 0);
+
+    const QUICAckFrame::AckBlockSection *section = ackFrame1->ack_block_section();
+    CHECK(section->first_ack_block_length() == 0x01);
+  }
+
   SECTION("2 Ack Block, 8 bit packet number length, 8 bit block length")
   {
     uint8_t buf1[] = {
-      0xB0,       // 101NLLMM
-      0x02,       // Num Blocks
-      0x12,       // Largest Acknowledged
-      0x34, 0x56, // Ack Delay
-      0x01,       // Ack Block Section (First ACK Block Length)
-      0x02,       // Ack Block Section (Gap 1)
-      0x03,       // Ack Block Section (ACK Block 1 Length)
-      0x04,       // Ack Block Section (Gap 2)
-      0x05,       // Ack Block Section (ACK Block 2 Length)
+      0x0e,                                           // Type
+      0x12,                                           // Largest Acknowledged
+      0x74, 0x56,                                     // Ack Delay
+      0x02,                                           // Ack Block Count
+      0x01,                                           // Ack Block Section (First ACK Block Length)
+      0x02,                                           // Ack Block Section (Gap 1)
+      0x43, 0x04,                                     // Ack Block Section (ACK Block 1 Length)
+      0x85, 0x06, 0x07, 0x08,                         // Ack Block Section (Gap 2)
+      0xc9, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, // Ack Block Section (ACK Block 2 Length)
     };
 
     std::shared_ptr<const QUICFrame> frame1 = QUICFrameFactory::create(buf1, sizeof(buf1));
     CHECK(frame1->type() == QUICFrameType::ACK);
-    CHECK(frame1->size() == 10);
+    CHECK(frame1->size() == 21);
     std::shared_ptr<const QUICAckFrame> ackFrame1 = std::dynamic_pointer_cast<const QUICAckFrame>(frame1);
     CHECK(ackFrame1 != nullptr);
     CHECK(ackFrame1->largest_acknowledged() == 0x12);
     CHECK(ackFrame1->ack_delay() == 0x3456);
-    CHECK(ackFrame1->num_blocks() == 2);
-    CHECK(ackFrame1->has_ack_blocks() == true);
+    CHECK(ackFrame1->ack_block_count() == 2);
+
     const QUICAckFrame::AckBlockSection *section = ackFrame1->ack_block_section();
     CHECK(section->first_ack_block_length() == 0x01);
     auto ite = section->begin();
     CHECK(ite != section->end());
-    CHECK(ite->gap() == 2);
-    CHECK(ite->length() == 3);
+    CHECK(ite->gap() == 0x02);
+    CHECK(ite->length() == 0x0304);
     ++ite;
     CHECK(ite != section->end());
-    CHECK(ite->gap() == 4);
-    CHECK(ite->length() == 5);
+    CHECK(ite->gap() == 0x05060708);
+    CHECK(ite->length() == 0x090a0b0c0d0e0f10);
     ++ite;
     CHECK(ite == section->end());
   }
 }
 
-TEST_CASE("Load Ack Frame 2", "[quic]")
+TEST_CASE("Store Ack Frame", "[quic]")
 {
-  // 0 Ack Block, 8 bit packet number length, 8 bit block length
-  uint8_t buf1[] = {
-    0xAA,                   // 101NLLMM '0b10101010' { N: 0, LL: 10, MM:10 }
-    0x00, 0x00, 0x00, 0x01, // Largest Acknowledged
-    0x01, 0x71,             // Ack Delay
-    0x00, 0x00, 0x00, 0x01, // ACK Block
-  };
-  std::shared_ptr<const QUICFrame> frame1 = QUICFrameFactory::create(buf1, sizeof(buf1));
-  CHECK(frame1->type() == QUICFrameType::ACK);
-  CHECK(frame1->size() == 11);
-  std::shared_ptr<const QUICAckFrame> ackFrame1 = std::dynamic_pointer_cast<const QUICAckFrame>(frame1);
-  CHECK(ackFrame1 != nullptr);
-  CHECK(ackFrame1->has_ack_blocks() == false);
-  CHECK(ackFrame1->largest_acknowledged() == 0x01);
-  CHECK(ackFrame1->ack_delay() == 0x0171);
+  SECTION("0 Ack Block, 8 bit packet number length, 8 bit block length")
+  {
+    uint8_t buf[32] = {0};
+    size_t len;
 
-  // TODO: 1 Ack Block
-}
+    uint8_t expected[] = {
+      0x0e,       // Type
+      0x12,       // Largest Acknowledged
+      0x74, 0x56, // Ack Delay
+      0x00,       // Ack Block Count
+      0x00,       // Ack Block Section
+    };
+    QUICAckFrame ackFrame(0x12, 0x3456, 0);
+    ackFrame.store(buf, &len);
+    CHECK(len == 6);
+    CHECK(memcmp(buf, expected, len) == 0);
+  }
 
-TEST_CASE("Store Ack Frame", "[quic]")
-{
-  uint8_t buf[65535];
-  size_t len;
+  SECTION("2 Ack Block, 8 bit packet number length, 8 bit block length")
+  {
+    uint8_t buf[32] = {0};
+    size_t len;
 
-  // 0 Ack Block, 8 bit packet number length, 8 bit block length
-  uint8_t expected[] = {
-    0xA2,                   // 101NLLMM
-    0x12,                   // Largest Acknowledged
-    0x34, 0x56,             // Ack Delay
-    0x00, 0x00, 0x00, 0x00, // Ack Block Section
-  };
-  QUICAckFrame ackFrame(0x12, 0x3456, 0);
-  ackFrame.store(buf, &len);
-  CHECK(len == 8);
-  CHECK(memcmp(buf, expected, len) == 0);
+    uint8_t expected[] = {
+      0x0e,                                           // Type
+      0x12,                                           // Largest Acknowledged
+      0x74, 0x56,                                     // Ack Delay
+      0x02,                                           // Ack Block Count
+      0x01,                                           // Ack Block Section (First ACK Block Length)
+      0x02,                                           // Ack Block Section (Gap 1)
+      0x43, 0x04,                                     // Ack Block Section (ACK Block 1 Length)
+      0x85, 0x06, 0x07, 0x08,                         // Ack Block Section (Gap 2)
+      0xc9, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, // Ack Block Section (ACK Block 2 Length)
+    };
+    QUICAckFrame ackFrame(0x12, 0x3456, 0x01);
+    QUICAckFrame::AckBlockSection *section = ackFrame.ack_block_section();
+    section->add_ack_block({0x02, 0x0304});
+    section->add_ack_block({0x05060708, 0x090a0b0c0d0e0f10});
 
-  // TODO: Add ack blocks
+    ackFrame.store(buf, &len);
+    CHECK(len == 21);
+    CHECK(memcmp(buf, expected, len) == 0);
+  }
 }
 
 TEST_CASE("Load RST_STREAM Frame", "[quic]")

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].