From 88b9ea15bbf261136eb79ccf7eca968ee78a558f Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Sun, 27 Nov 2016 07:49:26 -0800 Subject: [PATCH] network: (fixes #2221) Remove size constraint of Tag objects --- CHANGES.html | 3 ++ RELEASE_NOTES | 1 + src/network/model/packet-tag-list.cc | 46 +++++++++++++------ src/network/model/packet-tag-list.h | 69 +++++++++++++--------------- src/network/model/packet.cc | 3 +- src/network/utils/packet-socket.cc | 3 -- 6 files changed, 67 insertions(+), 58 deletions(-) diff --git a/CHANGES.html b/CHANGES.html index 3989adfb0..e03e75787 100644 --- a/CHANGES.html +++ b/CHANGES.html @@ -84,6 +84,9 @@ us a note on ns-developers mailing list.

See bug 2467 for discussion. +
  • Packet Tag objects are no longer constrained to fit within 21 + bytes; a maximum size is no longer enforced. +
    • diff --git a/RELEASE_NOTES b/RELEASE_NOTES index 0bc9db6be..92292a62d 100644 --- a/RELEASE_NOTES +++ b/RELEASE_NOTES @@ -25,6 +25,7 @@ New user-visible features Bugs fixed ---------- - Bug 2007 - uan: Remove deprecation on SetRxThresholdDb +- Bug 2221 - network: Remove constraint on size of ns3::Packet Tag objects - Bug 2450 - LogDistancePropagationLossModel is not continuous - Bug 2477 - DCF manager assert - Bug 2492 - uan: Make use of RxGain attribute in UanPhyGen class diff --git a/src/network/model/packet-tag-list.cc b/src/network/model/packet-tag-list.cc index ecb3762e1..06b47dd69 100644 --- a/src/network/model/packet-tag-list.cc +++ b/src/network/model/packet-tag-list.cc @@ -34,6 +34,23 @@ namespace ns3 { NS_LOG_COMPONENT_DEFINE ("PacketTagList"); +PacketTagList::TagData * +PacketTagList::CreateTagData (size_t dataSize) +{ + NS_ASSERT_MSG (dataSize + < std::numeric_limits::max (), + "Requested TagData size " << dataSize + << " exceeds maximum " + << std::numeric_limits::max () ); + + void * p = std::malloc (sizeof (TagData) + dataSize - 1); + // The matching frees are in RemoveAll and RemoveWriter + + TagData * tag = new (p) TagData; + tag->size = dataSize; + return tag; +} + bool PacketTagList::COWTraverse (Tag & tag, PacketTagList::COWWriter Writer) { @@ -133,10 +150,11 @@ PacketTagList::COWTraverse (Tag & tag, PacketTagList::COWWriter Writer) NS_ASSERT (cur != 0); NS_ASSERT (cur->count > 1); cur->count--; // unmerge cur - struct TagData * copy = new struct TagData (); + struct TagData * copy = CreateTagData (cur->size); copy->tid = cur->tid; copy->count = 1; - memcpy (copy->data, cur->data, TagData::MAX_SIZE); + copy->size = cur->size; + memcpy (copy->data, cur->data, copy->size); copy->next = cur->next; // merge into tail copy->next->count++; // mark new merge *prevNext = copy; // point prior list at copy @@ -170,14 +188,14 @@ PacketTagList::RemoveWriter (Tag & tag, bool preMerge, // found tid bool found = true; - tag.Deserialize (TagBuffer (cur->data, - cur->data + TagData::MAX_SIZE)); + tag.Deserialize (TagBuffer (cur->data, cur->data + cur->size)); *prevNext = cur->next; // link around cur if (preMerge) { // found tid before first merge, so delete cur - delete cur; + cur->~TagData (); + std::free (cur); } else { @@ -217,19 +235,17 @@ PacketTagList::ReplaceWriter (Tag & tag, bool preMerge, if (preMerge) { // found tid before first merge, so just rewrite - tag.Serialize (TagBuffer (cur->data, - cur->data + tag.GetSerializedSize ())); + tag.Serialize (TagBuffer (cur->data, cur->data + cur->size)); } else { // cur is always a merge at this point // need to copy, replace, and link past cur cur->count--; // unmerge cur - struct TagData * copy = new struct TagData (); + struct TagData * copy = CreateTagData (tag.GetSerializedSize ()); copy->tid = tag.GetInstanceTypeId (); copy->count = 1; - tag.Serialize (TagBuffer (copy->data, - copy->data + tag.GetSerializedSize ())); + tag.Serialize (TagBuffer (copy->data, copy->data + copy->size)); copy->next = cur->next; // merge into tail if (copy->next != 0) { @@ -247,15 +263,15 @@ PacketTagList::Add (const Tag &tag) const // ensure this id was not yet added for (struct TagData *cur = m_next; cur != 0; cur = cur->next) { - NS_ASSERT_MSG (cur->tid != tag.GetInstanceTypeId (), "Error: cannot add the same kind of tag twice."); + NS_ASSERT_MSG (cur->tid != tag.GetInstanceTypeId (), + "Error: cannot add the same kind of tag twice."); } - struct TagData * head = new struct TagData (); + struct TagData * head = CreateTagData (tag.GetSerializedSize ()); head->count = 1; head->next = 0; head->tid = tag.GetInstanceTypeId (); head->next = m_next; - NS_ASSERT (tag.GetSerializedSize () <= TagData::MAX_SIZE); - tag.Serialize (TagBuffer (head->data, head->data + tag.GetSerializedSize ())); + tag.Serialize (TagBuffer (head->data, head->data + head->size)); const_cast (this)->m_next = head; } @@ -270,7 +286,7 @@ PacketTagList::Peek (Tag &tag) const if (cur->tid == tid) { /* found tag */ - tag.Deserialize (TagBuffer (cur->data, cur->data + TagData::MAX_SIZE)); + tag.Deserialize (TagBuffer (cur->data, cur->data + cur->size)); return true; } } diff --git a/src/network/model/packet-tag-list.h b/src/network/model/packet-tag-list.h index 7967af88d..a40f63de7 100644 --- a/src/network/model/packet-tag-list.h +++ b/src/network/model/packet-tag-list.h @@ -119,12 +119,6 @@ class Tag; * The portion of the list between the first branch and the target is * shared. This portion is copied before the #Remove or #Replace is * performed. - * - * \par Memory Management: - * \n - * Packet tags must serialize to a finite maximum size, see TagData - * - * This documentation entitles the original author to a free beer. */ class PacketTagList { @@ -134,37 +128,23 @@ public: * * See PacketTagList for a discussion of the data structure. * - * See TagData::TagData_e for a discussion of the size limit on - * tag serialization. + * \internal + * Unfortunately this has to be public, because + * PacketTagIterator::Item::GetTag() needs the data and size values. + * The Item nested class can't be forward declared, so friending isn't + * possible. + * + * We use placement new so we can allocate enough room for the Tag + * type which will be serialized into data. See Object::Aggregates + * for a similar construction. */ struct TagData { - /** - * \brief Packet Tag maximum size - * - * The maximum size (in bytes) of a Tag is stored - * in this constant. - * - * \internal - * Ideally, TagData would be 32 bytes in size, so they require - * no padding on 64-bit architectures. (The architecture - * affects the size because of the \c #next pointer.) - * This would leave 18 bytes for \c #data. However, - * ns3:Ipv6PacketInfoTag needs 19 bytes! The current - * implementation allows 20 bytes, which gives TagData - * a size of 30 bytes on 32-byte machines (which gets - * padded with 2 bytes), and 34 bytes on 64-bit machines, which - * gets padded to 40 bytes. - */ - enum TagData_e - { - MAX_SIZE = 21 /**< Size of serialization buffer #data */ - }; - - uint8_t data[MAX_SIZE]; /**< Serialization buffer */ - struct TagData * next; /**< Pointer to next in list */ - TypeId tid; /**< Type of the tag serialized into #data */ - uint32_t count; /**< Number of incoming links */ + struct TagData * next; /**< Pointer to next in list */ + uint32_t count; /**< Number of incoming links */ + TypeId tid; /**< Type of the tag serialized into #data */ + uint32_t size; /**< Size of the \c data buffer */ + uint8_t data[1]; /**< Serialization buffer */ }; /* struct TagData */ /** @@ -239,6 +219,16 @@ public: const struct PacketTagList::TagData *Head (void) const; private: + /** + * Allocate and construct a TagData struct, sizing the data area + * large enough to serialize dataSize bytes from a Tag. + * + * \param [in] dataSize The serialized size of the Tag. + * \returns The newly constructed TagData object. + */ + static + TagData * CreateTagData (size_t dataSize); + /** * Typedef of method function pointer for copy-on-write operations * @@ -272,7 +262,7 @@ private: * pointing to \pname{cur}. * \returns True, since tag will definitely be removed. */ - bool RemoveWriter (Tag & tag, bool preMerge, + bool RemoveWriter (Tag & tag, bool preMerge, struct TagData * cur, struct TagData ** prevNext); /** * Copy-on-write implementing Replace @@ -285,7 +275,8 @@ private: * pointing to \pname{cur}. * \returns True, since tag value will definitely be replaced. */ - bool ReplaceWriter (Tag & tag, bool preMerge, struct TagData * cur, struct TagData ** prevNext); + bool ReplaceWriter (Tag & tag, bool preMerge, + struct TagData * cur, struct TagData ** prevNext); /** * Pointer to first \ref TagData on the list @@ -350,13 +341,15 @@ PacketTagList::RemoveAll (void) } if (prev != 0) { - delete prev; + prev->~TagData (); + std::free (prev); } prev = cur; } if (prev != 0) { - delete prev; + prev->~TagData (); + std::free (prev); } m_next = 0; } diff --git a/src/network/model/packet.cc b/src/network/model/packet.cc index 51ed315ff..eefa4aa32 100644 --- a/src/network/model/packet.cc +++ b/src/network/model/packet.cc @@ -113,8 +113,7 @@ PacketTagIterator::Item::GetTag (Tag &tag) const { NS_ASSERT (tag.GetInstanceTypeId () == m_data->tid); tag.Deserialize (TagBuffer ((uint8_t*)m_data->data, - (uint8_t*)m_data->data - + PacketTagList::TagData::MAX_SIZE)); + (uint8_t*)m_data->data + m_data->size)); } diff --git a/src/network/utils/packet-socket.cc b/src/network/utils/packet-socket.cc index b1d4541ff..7137c7bba 100644 --- a/src/network/utils/packet-socket.cc +++ b/src/network/utils/packet-socket.cc @@ -643,7 +643,6 @@ uint32_t DeviceNameTag::GetSerializedSize (void) const { uint32_t s = 1 + m_deviceName.size(); // +1 for name length field - s = std::min (s, (uint32_t)PacketTagList::TagData::MAX_SIZE); return s; } void @@ -652,8 +651,6 @@ DeviceNameTag::Serialize (TagBuffer i) const const char *n = m_deviceName.c_str(); uint8_t l = (uint8_t) m_deviceName.size (); - l = std::min ((uint32_t)l, (uint32_t)PacketTagList::TagData::MAX_SIZE - 1); - i.WriteU8 (l); i.Write ( (uint8_t*) n , (uint32_t) l); }