From a2cbaea607d628cc621f95779a0254c371d7ef50 Mon Sep 17 00:00:00 2001 From: Alan Frindell Date: Mon, 13 Apr 2020 15:54:16 -0700 Subject: [PATCH 1/3] [qpack] Overhaul the pseudocode * Use the current terminology (insert count, base, encoder stream) * Update to current design (modulo encoding of RIC) * Normalize naming: index vs. idx * Comments Fixes #3544 --- draft-ietf-quic-qpack.md | 68 ++++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/draft-ietf-quic-qpack.md b/draft-ietf-quic-qpack.md index 1beb7eaa40..0a54550eb6 100644 --- a/draft-ietf-quic-qpack.md +++ b/draft-ietf-quic-qpack.md @@ -1304,54 +1304,62 @@ are registered in the "HTTP/3 Error Code" registry established in {{HTTP3}}. # Sample One Pass Encoding Algorithm Pseudo-code for single pass encoding, excluding handling of duplicates, -non-blocking mode, and reference tracking. +non-blocking mode, available encoder stream flow control and reference tracking. ~~~ -baseIndex = dynamicTable.baseIndex -largestReference = 0 +base = dynamicTable.getInsertCount() +requiredInsertCount = 0 for header in headers: - staticIdx = staticTable.getIndex(header) - if staticIdx: - encodeIndexReference(streamBuffer, staticIdx) + staticIndex = staticTable.getIndex(header) + if staticIndex is not None: + encodeIndexReference(streamBuffer, staticIndex) continue - dynamicIdx = dynamicTable.getIndex(header) - if !dynamicIdx: + dynamicIndex = dynamicTable.getIndex(header) + if dynamicIndex is None: # No matching entry. Either insert+index or encode literal - nameIdx = getNameIndex(header) + + # getNameIndex attempts to find an index for this header's name. + # If one exists, it returns the name's (absolute) index and a + # boolean indicating which table has the name. If the name + # can't be found, nameIndex is None + nameIndex, isStaticName = getNameIndex(header.name) if shouldIndex(header) and dynamicTable.canIndex(header): - encodeLiteralWithIncrementalIndex(controlBuffer, nameIdx, - header) + encodeInsert(encoderBuffer, nameIndex, isStaticName, header) dynamicTable.add(header) - dynamicIdx = dynamicTable.baseIndex + dynamicIndex = dynamicTable.getInsertCount() - if !dynamicIdx: + if dynamicIndex is None: # Couldn't index it, literal - if nameIdx <= staticTable.size: + if nameIndex is None or isStaticName: + # Encodes a literal with a static name or literal name encodeLiteral(streamBuffer, nameIndex, header) else: - # encode literal, possibly with nameIdx above baseIndex - encodeDynamicLiteral(streamBuffer, nameIndex, baseIndex, - header) - largestReference = max(largestReference, - dynamicTable.toAbsolute(nameIdx)) + # encode literal with dynamic name, possibly above base + encodeDynamicLiteral(streamBuffer, nameIndex, base, header) + requiredInsertCount = max(requiredInsertCount, nameIndex) else: # Dynamic index reference - assert(dynamicIdx) - largestReference = max(largestReference, dynamicIdx) - # Encode dynamicIdx, possibly with dynamicIdx above baseIndex - encodeDynamicIndexReference(streamBuffer, dynamicIdx, - baseIndex) + assert(dynamicIndex is not None) + requiredInsertCount = max(requiredInsertCount, dynamicIndex) + # Encode dynamicIndex, possibly above base + encodeDynamicIndexReference(streamBuffer, dynamicIndex, base) # encode the prefix -encodeInteger(prefixBuffer, 0x00, largestReference, 8) -if baseIndex >= largestReference: - encodeInteger(prefixBuffer, 0, baseIndex - largestReference, 7) +if requiredInsertCount == 0: + encodeIndexReference(prefixBuffer, 0, 0, 8) + encodeIndexReference(prefixBuffer, 0, 0, 7) else: - encodeInteger(prefixBuffer, 0x80, - largestReference - baseIndex, 7) + wireRIC = + (requiredInsertCount % (2 * getMaxEntries(maxTableCapacity))) + 1; + encodeInteger(prefixBuffer, 0x00, wireRIC, 8) + if base >= requiredInsertCount: + encodeInteger(prefixBuffer, 0, base - requiredInsertCount, 7) + else: + encodeInteger(prefixBuffer, 0x80, + requiredInsertCount - base - 1, 7) -return controlBuffer, prefixBuffer + streamBuffer +return encoderBuffer, prefixBuffer + streamBuffer ~~~ # Change Log From 01df492f9e4f47e65e94ae00a14aa0f42d7832f6 Mon Sep 17 00:00:00 2001 From: Alan Frindell Date: Tue, 14 Apr 2020 15:16:22 -0700 Subject: [PATCH 2/3] Fix lint --- draft-ietf-quic-qpack.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/draft-ietf-quic-qpack.md b/draft-ietf-quic-qpack.md index 0a54550eb6..ee9b06fb4e 100644 --- a/draft-ietf-quic-qpack.md +++ b/draft-ietf-quic-qpack.md @@ -1319,13 +1319,14 @@ for header in headers: if dynamicIndex is None: # No matching entry. Either insert+index or encode literal - # getNameIndex attempts to find an index for this header's name. - # If one exists, it returns the name's (absolute) index and a - # boolean indicating which table has the name. If the name - # can't be found, nameIndex is None + # getNameIndex attempts to find an index for this header's + # name. If one exists, it returns the name's (absolute) + # index and a boolean indicating which table has the name. + # If the name can't be found, nameIndex is None nameIndex, isStaticName = getNameIndex(header.name) if shouldIndex(header) and dynamicTable.canIndex(header): - encodeInsert(encoderBuffer, nameIndex, isStaticName, header) + encodeInsert(encoderBuffer, nameIndex, isStaticName, + header) dynamicTable.add(header) dynamicIndex = dynamicTable.getInsertCount() @@ -1351,7 +1352,8 @@ if requiredInsertCount == 0: encodeIndexReference(prefixBuffer, 0, 0, 7) else: wireRIC = - (requiredInsertCount % (2 * getMaxEntries(maxTableCapacity))) + 1; + (requiredInsertCount % + (2 * getMaxEntries(maxTableCapacity))) + 1; encodeInteger(prefixBuffer, 0x00, wireRIC, 8) if base >= requiredInsertCount: encodeInteger(prefixBuffer, 0, base - requiredInsertCount, 7) From 4398ec6f2d91b08dfce60af512c325a872a0f1b0 Mon Sep 17 00:00:00 2001 From: Alan Frindell Date: Tue, 5 May 2020 15:18:18 -0700 Subject: [PATCH 3/3] Martin's feedback --- draft-ietf-quic-qpack.md | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/draft-ietf-quic-qpack.md b/draft-ietf-quic-qpack.md index ee9b06fb4e..ed3b6e9063 100644 --- a/draft-ietf-quic-qpack.md +++ b/draft-ietf-quic-qpack.md @@ -1310,25 +1310,22 @@ non-blocking mode, available encoder stream flow control and reference tracking. base = dynamicTable.getInsertCount() requiredInsertCount = 0 for header in headers: - staticIndex = staticTable.getIndex(header) + staticIndex = staticTable.findIndex(header) if staticIndex is not None: encodeIndexReference(streamBuffer, staticIndex) continue - dynamicIndex = dynamicTable.getIndex(header) + dynamicIndex = dynamicTable.findIndex(header) if dynamicIndex is None: # No matching entry. Either insert+index or encode literal + staticNameIndex = staticTable.findName(header.name) + if staticNameIndex is None: + dynamicNameIndex = dynamicTable.findName(header.name) - # getNameIndex attempts to find an index for this header's - # name. If one exists, it returns the name's (absolute) - # index and a boolean indicating which table has the name. - # If the name can't be found, nameIndex is None - nameIndex, isStaticName = getNameIndex(header.name) if shouldIndex(header) and dynamicTable.canIndex(header): - encodeInsert(encoderBuffer, nameIndex, isStaticName, - header) - dynamicTable.add(header) - dynamicIndex = dynamicTable.getInsertCount() + encodeInsert(encoderBuffer, staticNameIndex, + dynamicNameIndex, header) + dynamicIndex = dynamicTable.add(header) if dynamicIndex is None: # Couldn't index it, literal @@ -1351,9 +1348,10 @@ if requiredInsertCount == 0: encodeIndexReference(prefixBuffer, 0, 0, 8) encodeIndexReference(prefixBuffer, 0, 0, 7) else: - wireRIC = - (requiredInsertCount % - (2 * getMaxEntries(maxTableCapacity))) + 1; + wireRIC = ( + requiredInsertCount + % (2 * getMaxEntries(maxTableCapacity)) + ) + 1; encodeInteger(prefixBuffer, 0x00, wireRIC, 8) if base >= requiredInsertCount: encodeInteger(prefixBuffer, 0, base - requiredInsertCount, 7)