Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement QName compression in minmdns #12445

Merged
merged 23 commits into from
Dec 3, 2021
Merged
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
180e9ed
Revamp constants: qname length constant is NOT used
andy31415 Dec 1, 2021
07d5408
Merge branch 'master' into qname_compression
andy31415 Dec 1, 2021
a1b3129
First pass on an indirection to use a dedicated writer. No test upda…
andy31415 Dec 1, 2021
b8871a1
Updates to use the record writer, more tests pass now
andy31415 Dec 1, 2021
559f582
Merge branch 'master' into qname_compression
andy31415 Dec 1, 2021
7082e7c
Compressed writer with unit tests
andy31415 Dec 1, 2021
c6c358c
Fix memory leak in UDP responders when a multicast join fails
andy31415 Dec 1, 2021
6d178c3
Add a harder unit test
andy31415 Dec 1, 2021
bcf032d
Make the complex reuse also validate that we can write things other t…
andy31415 Dec 1, 2021
587e987
Merge branch 'master' into qname_compression
andy31415 Dec 2, 2021
e218582
Compression applied on all answers (no queries yet)
andy31415 Dec 2, 2021
12fb985
Prepare to make query writing do compression as well, add more unit t…
andy31415 Dec 2, 2021
5b48d1a
Record compression for writing serialized qnames as well. Tested that…
andy31415 Dec 2, 2021
62ee0cb
Fix unit tests for resource records for writing QName compression
andy31415 Dec 2, 2021
9a98ea1
Code review comments, fix android compile
andy31415 Dec 2, 2021
623b5e3
Fix test advertiser to consider the entire valid packet range when va…
andy31415 Dec 2, 2021
fcbac5d
Only remember non-fully compressed qnames
andy31415 Dec 2, 2021
3e623ec
Address some code review comments
andy31415 Dec 2, 2021
2084279
Add a fit validation for response building
andy31415 Dec 2, 2021
2e74910
update the code to have RecordWriter be more transparent/easier to us…
andy31415 Dec 2, 2021
cb73e42
Merge branch 'master' into qname_compression
andy31415 Dec 2, 2021
e129b91
Merge branch 'master' into qname_compression
andy31415 Dec 2, 2021
bc38e47
Merge branch 'master' into qname_compression
andy31415 Dec 2, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .vscode/settings.json
Original file line number Diff line number Diff line change
@@ -156,5 +156,6 @@
"clang-format.fallbackStyle": "WebKit",
"files.trimFinalNewlines": true,
"C_Cpp.default.cppStandard": "gnu++14",
"C_Cpp.default.cStandard": "gnu11"
"C_Cpp.default.cStandard": "gnu11",
"cmake.configureOnOpen": false
}
15 changes: 8 additions & 7 deletions src/lib/dnssd/minimal_mdns/Parser.cpp
Original file line number Diff line number Diff line change
@@ -66,23 +66,24 @@ bool QueryData::Parse(const BytesRange & validData, const uint8_t ** start)
return true;
}

bool QueryData::Append(HeaderRef & hdr, chip::Encoding::BigEndian::BufferWriter & out) const
bool QueryData::Append(HeaderRef & hdr, RecordWriter & out) const
{
if ((hdr.GetAdditionalCount() != 0) || (hdr.GetAnswerCount() != 0) || (hdr.GetAuthorityCount() != 0))
{
return false;
}

GetName().Put(out);
out.Put16(static_cast<uint16_t>(mType));
out.Put16(static_cast<uint16_t>(mClass) | (mAnswerViaUnicast ? kQClassUnicastAnswerFlag : 0));
out.WriteQName(GetName())
.Put16(static_cast<uint16_t>(mType))
.Put16(static_cast<uint16_t>(mClass) | (mAnswerViaUnicast ? kQClassUnicastAnswerFlag : 0));

if (out.Fit())
if (!out.Fit())
{
hdr.SetQueryCount(static_cast<uint16_t>(hdr.GetQueryCount() + 1));
return false;
}

return out.Fit();
hdr.SetQueryCount(static_cast<uint16_t>(hdr.GetQueryCount() + 1));
return true;
}

bool ResourceData::Parse(const BytesRange & validData, const uint8_t ** start)
3 changes: 2 additions & 1 deletion src/lib/dnssd/minimal_mdns/Parser.h
Original file line number Diff line number Diff line change
@@ -20,6 +20,7 @@
#include <lib/dnssd/minimal_mdns/core/Constants.h>
#include <lib/dnssd/minimal_mdns/core/DnsHeader.h>
#include <lib/dnssd/minimal_mdns/core/QName.h>
#include <lib/dnssd/minimal_mdns/core/RecordWriter.h>

namespace mdns {
namespace Minimal {
@@ -56,7 +57,7 @@ class QueryData
bool Parse(const BytesRange & validData, const uint8_t ** start);

/// Write out this query data back into an output buffer.
bool Append(HeaderRef & hdr, chip::Encoding::BigEndian::BufferWriter & out) const;
bool Append(HeaderRef & hdr, RecordWriter & out) const;

private:
QType mType = QType::ANY;
17 changes: 9 additions & 8 deletions src/lib/dnssd/minimal_mdns/Query.h
Original file line number Diff line number Diff line change
@@ -21,6 +21,7 @@

#include <lib/dnssd/minimal_mdns/core/Constants.h>
#include <lib/dnssd/minimal_mdns/core/QName.h>
#include <lib/dnssd/minimal_mdns/core/RecordWriter.h>

namespace mdns {
namespace Minimal {
@@ -56,25 +57,25 @@ class Query
///
/// @param hdr will be updated with a query count
/// @param out where to write the query data
bool Append(HeaderRef & hdr, chip::Encoding::BigEndian::BufferWriter & out) const
bool Append(HeaderRef & hdr, RecordWriter & out) const
{
// Questions can only be appended before any other data is added
if ((hdr.GetAdditionalCount() != 0) || (hdr.GetAnswerCount() != 0) || (hdr.GetAuthorityCount() != 0))
{
return false;
}

mQName.Output(out);
out.WriteQName(mQName)
.Put16(static_cast<uint16_t>(mType))
.Put16(static_cast<uint16_t>(static_cast<uint16_t>(mClass) | (mAnswerViaUnicast ? kQClassUnicastAnswerFlag : 0)));

out.Put16(static_cast<uint16_t>(mType));
out.Put16(static_cast<uint16_t>(static_cast<uint16_t>(mClass) | (mAnswerViaUnicast ? kQClassUnicastAnswerFlag : 0)));

if (out.Fit())
if (!out.Fit())
{
hdr.SetQueryCount(static_cast<uint16_t>(hdr.GetQueryCount() + 1));
return false;
}

return out.Fit();
hdr.SetQueryCount(static_cast<uint16_t>(hdr.GetQueryCount() + 1));
return true;
}

private:
3 changes: 2 additions & 1 deletion src/lib/dnssd/minimal_mdns/QueryBuilder.h
Original file line number Diff line number Diff line change
@@ -68,8 +68,9 @@ class QueryBuilder
}

chip::Encoding::BigEndian::BufferWriter out(mPacket->Start() + mPacket->DataLength(), mPacket->AvailableDataLength());
RecordWriter writer(&out);

if (!query.Append(mHeader, out))
if (!query.Append(mHeader, writer))
{
mQueryBuildOk = false;
}
31 changes: 21 additions & 10 deletions src/lib/dnssd/minimal_mdns/ResponseBuilder.h
Original file line number Diff line number Diff line change
@@ -29,8 +29,12 @@ namespace Minimal {
class ResponseBuilder
{
public:
ResponseBuilder() : mHeader(nullptr) {}
ResponseBuilder(chip::System::PacketBufferHandle && packet) : mHeader(nullptr) { Reset(std::move(packet)); }
ResponseBuilder() : mHeader(nullptr), mEndianOutput(nullptr, 0), mWriter(&mEndianOutput) {}
ResponseBuilder(chip::System::PacketBufferHandle && packet) :
mHeader(nullptr), mEndianOutput(nullptr, 0), mWriter(&mEndianOutput)
{
Reset(std::move(packet));
}

ResponseBuilder & Reset(chip::System::PacketBufferHandle && packet)
{
@@ -49,6 +53,13 @@ class ResponseBuilder
}

mHeader.SetFlags(mHeader.GetFlags().SetResponse());

mEndianOutput =
chip::Encoding::BigEndian::BufferWriter(mPacket->Start(), mPacket->DataLength() + mPacket->AvailableDataLength());
mEndianOutput.Skip(mPacket->DataLength());

mWriter.Reset();

return *this;
}

@@ -77,16 +88,16 @@ class ResponseBuilder
return *this;
}

chip::Encoding::BigEndian::BufferWriter out(mPacket->Start() + mPacket->DataLength(), mPacket->AvailableDataLength());

if (!record.Append(mHeader, type, out))
if (!record.Append(mHeader, type, mWriter))
{
mBuildOk = false;
}
else
{
mPacket->SetDataLength(static_cast<uint16_t>(mPacket->DataLength() + out.Needed()));
VerifyOrDie(mEndianOutput.Fit()); // should be guaranteed because record Append succeeded
mPacket->SetDataLength(static_cast<uint16_t>(mEndianOutput.Needed()));
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
}

return *this;
}

@@ -97,15 +108,13 @@ class ResponseBuilder
return *this;
}

chip::Encoding::BigEndian::BufferWriter out(mPacket->Start() + mPacket->DataLength(), mPacket->AvailableDataLength());

if (!query.Append(mHeader, out))
if (!query.Append(mHeader, mWriter))
{
mBuildOk = false;
}
else
{
mPacket->SetDataLength(static_cast<uint16_t>(mPacket->DataLength() + out.Needed()));
mPacket->SetDataLength(static_cast<uint16_t>(mEndianOutput.Needed()));
}
return *this;
}
@@ -116,6 +125,8 @@ class ResponseBuilder
private:
chip::System::PacketBufferHandle mPacket;
HeaderRef mHeader;
chip::Encoding::BigEndian::BufferWriter mEndianOutput;
RecordWriter mWriter;
bool mBuildOk = false;
};

2 changes: 2 additions & 0 deletions src/lib/dnssd/minimal_mdns/core/BUILD.gn
Original file line number Diff line number Diff line change
@@ -21,6 +21,8 @@ static_library("core") {
"DnsHeader.h",
"QName.cpp",
"QName.h",
"RecordWriter.cpp",
"RecordWriter.h",
]

public_deps = [
5 changes: 5 additions & 0 deletions src/lib/dnssd/minimal_mdns/core/BytesRange.h
Original file line number Diff line number Diff line change
@@ -44,6 +44,11 @@ class BytesRange

size_t Size() const { return static_cast<size_t>(mEnd - mStart); }

inline static BytesRange BufferWithSize(const void * buff, size_t len)
{
return BytesRange(static_cast<const uint8_t *>(buff), static_cast<const uint8_t *>(buff) + len);
}

private:
const uint8_t * mStart = nullptr;
const uint8_t * mEnd = nullptr;
2 changes: 0 additions & 2 deletions src/lib/dnssd/minimal_mdns/core/Constants.h
Original file line number Diff line number Diff line change
@@ -66,7 +66,5 @@ enum class ResourceType
kAdditional,
};

constexpr size_t kMaxQNamePartLength = 255;

} // namespace Minimal
} // namespace mdns
34 changes: 34 additions & 0 deletions src/lib/dnssd/minimal_mdns/core/QName.cpp
Original file line number Diff line number Diff line change
@@ -152,6 +152,40 @@ bool SerializedQNameIterator::operator==(const FullQName & other) const
return ((idx == other.nameCount) && !self.Next());
}

bool SerializedQNameIterator::operator==(const SerializedQNameIterator & other) const
{
SerializedQNameIterator a = *this; // allow iteration
SerializedQNameIterator b = other;

while (true)
{
bool hasA = a.Next();
bool hasB = b.Next();

if (hasA ^ hasB)
{
return false; /// one is longer than the other
andy31415 marked this conversation as resolved.
Show resolved Hide resolved
}

if (!a.IsValid() || !b.IsValid())
{
return false; // invalid data
}

if (!hasA || !hasB)
{
break;
}

if (strcasecmp(a.Value(), b.Value()) != 0)
{
return false;
}
}

return a.IsValid() && b.IsValid();
}

bool FullQName::operator==(const FullQName & other) const
{
if (nameCount != other.nameCount)
26 changes: 4 additions & 22 deletions src/lib/dnssd/minimal_mdns/core/QName.h
Original file line number Diff line number Diff line change
@@ -53,17 +53,6 @@ struct FullQName
FullQName(const QNamePart (&data)[N]) : names(data), nameCount(N)
{}

void Output(chip::Encoding::BigEndian::BufferWriter & out) const
{
for (uint16_t i = 0; i < nameCount; i++)
{

out.Put8(static_cast<uint8_t>(strlen(names[i])));
out.Put(names[i]);
}
out.Put8(0); // end of qnames
}

bool operator==(const FullQName & other) const;
bool operator!=(const FullQName & other) const { return !(*this == other); }
};
@@ -108,17 +97,10 @@ class SerializedQNameIterator
bool operator==(const FullQName & other) const;
bool operator!=(const FullQName & other) const { return !(*this == other); }

void Put(chip::Encoding::BigEndian::BufferWriter & out) const
{
SerializedQNameIterator copy = *this;
while (copy.Next())
{

out.Put8(static_cast<uint8_t>(strlen(copy.Value())));
out.Put(copy.Value());
}
out.Put8(0); // end of qnames
}
bool operator==(const SerializedQNameIterator & other) const;
bool operator!=(const SerializedQNameIterator & other) const { return !(*this == other); }

size_t OffsetInCurrentValidData() const { return static_cast<size_t>(mCurrentPosition - mValidData.Start()); }

private:
static constexpr size_t kMaxValueSize = 63;
Loading