From 2ecedb3e874d86c17140cfc40de139fea8683948 Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Tue, 13 Nov 2012 23:44:58 -0800 Subject: [PATCH 1/8] Trim excess copies from PacketTagList::Remove, add PacketTagList::Replace, documentation. --- doc/doxygen.conf | 11 +- src/network/model/byte-tag-list.h | 19 +- src/network/model/packet-tag-list.cc | 283 ++++++++++++++++++++----- src/network/model/packet-tag-list.h | 284 ++++++++++++++++++++++++-- src/network/model/packet.cc | 96 ++++----- src/network/model/packet.h | 138 ++++++++----- src/network/model/tag.h | 2 +- src/network/test/packet-test-suite.cc | 217 +++++++++++++++++++- 8 files changed, 848 insertions(+), 202 deletions(-) diff --git a/doc/doxygen.conf b/doc/doxygen.conf index 94e213d8c..be1ae1497 100644 --- a/doc/doxygen.conf +++ b/doc/doxygen.conf @@ -193,9 +193,14 @@ TAB_SIZE = 4 # will result in a user-defined paragraph with heading "Side Effects:". # You can put \n's in the value part of an alias to insert newlines. -ALIASES = \ - "intern=\internal \par \b Internal:" \ - "pname{1}=\1" +ALIASES = + +# Set off \internal docs +ALIASES += intern="\internal \par \b Internal:" + +# Typeset parameter name in docs as in the "Parameters:" list +# Usage: /** \param [in/out] tag If found, \pname{tag} is ... */ +ALIASES += pname{1}="\1" # This tag can be used to specify a number of word-keyword mappings (TCL only). # A mapping has the form "name=value". For example adding diff --git a/src/network/model/byte-tag-list.h b/src/network/model/byte-tag-list.h index 37df359aa..8d20e2117 100644 --- a/src/network/model/byte-tag-list.h +++ b/src/network/model/byte-tag-list.h @@ -31,7 +31,7 @@ struct ByteTagListData; /** * \ingroup packet * - * \brief keep track of the tags stored in a packet. + * \brief keep track of the byte tags stored in a packet. * * This class is mostly private to the Packet implementation and users * should never have to access it directly. @@ -40,15 +40,15 @@ struct ByteTagListData; * The implementation of this class is a bit tricky so, there are a couple * of things to keep in mind here: * - * - it stores all tags in a single byte buffer: each tag is stored + * - It stores all tags in a single byte buffer: each tag is stored * as 4 32bit integers (TypeId, tag data size, start, end) followed * by the tag data as generated by Tag::Serialize. * - * - the struct ByteTagListData structure which contains the tag byte buffer + * - The struct ByteTagListData structure which contains the tag byte buffer * is shared and, thus, reference-counted. This data structure is unshared * as-needed to emulate COW semantics. * - * - each tag tags a unique set of bytes identified by the pair of offsets + * - Each tag tags a unique set of bytes identified by the pair of offsets * (start,end). These offsets are provided by Buffer::GetCurrentStartOffset * and Buffer::GetCurrentEndOffset which means that they are relative to * the start of the 'virtual byte buffer' as explained in the documentation @@ -59,7 +59,7 @@ struct ByteTagListData; * the Packet class calls ByteTagList::AddAtEnd and ByteTagList::AddAtStart to update * the byte offsets of each tag in the ByteTagList. * - * - whenever bytes are removed from the packet byte buffer, the ByteTagList offsets + * - Whenever bytes are removed from the packet byte buffer, the ByteTagList offsets * are never updated because we rely on the fact that they will be updated in * either the next call to Packet::AddHeader or Packet::AddTrailer or when * the user iterates the tag list with Packet::GetTagIterator and @@ -68,10 +68,9 @@ struct ByteTagListData; class ByteTagList { public: - class Iterator { -public: + public: struct Item { TypeId tid; @@ -80,14 +79,14 @@ public: int32_t end; TagBuffer buf; Item (TagBuffer buf); -private: + private: friend class ByteTagList; friend class ByteTagList::Iterator; }; bool HasNext (void) const; struct ByteTagList::Iterator::Item Next (void); uint32_t GetOffsetStart (void) const; -private: + private: friend class ByteTagList; Iterator (uint8_t *start, uint8_t *end, int32_t offsetStart, int32_t offsetEnd); void PrepareForNext (void); @@ -158,7 +157,7 @@ private: bool IsDirtyAtStart (int32_t prependOffset); ByteTagList::Iterator BeginAll (void) const; - struct ByteTagListData *Allocate (uint32_t size); + struct ByteTagListData * Allocate (uint32_t size); void Deallocate (struct ByteTagListData *data); uint16_t m_used; diff --git a/src/network/model/packet-tag-list.cc b/src/network/model/packet-tag-list.cc index e8247c662..97ee625d6 100644 --- a/src/network/model/packet-tag-list.cc +++ b/src/network/model/packet-tag-list.cc @@ -17,6 +17,12 @@ * * Author: Mathieu Lacage */ + +/** +\file packet-tag-list.cc +\brief Implements a linked list of Packet tags, including copy-on-write semantics. +*/ + #include "packet-tag-list.h" #include "tag-buffer.h" #include "tag.h" @@ -28,25 +34,29 @@ NS_LOG_COMPONENT_DEFINE ("PacketTagList"); namespace ns3 { -#ifdef USE_FREE_LIST +#ifndef USE_FREE_LIST +#define USE_FREE_LIST 0 +#endif + +#if USE_FREE_LIST -struct PacketTagList::TagData *PacketTagList::g_free = 0; +struct PacketTagList::TagData * PacketTagList::g_free = 0; uint32_t PacketTagList::g_nfree = 0; - + struct PacketTagList::TagData * PacketTagList::AllocData (void) const { - NS_LOG_FUNCTION_NOARGS (); - struct PacketTagList::TagData *retval; + NS_LOG_FUNCTION (g_nfree); + struct TagData * retval; if (g_free != 0) { retval = g_free; - g_free = g_free->m_next; + g_free = g_free->next; g_nfree--; } else { - retval = new struct PacketTagList::TagData (); + retval = new struct TagData (); } return retval; } @@ -54,97 +64,258 @@ PacketTagList::AllocData (void) const void PacketTagList::FreeData (struct TagData *data) const { - NS_LOG_FUNCTION (data); - if (g_nfree > 1000) + NS_LOG_FUNCTION (g_nfree << data); + if (g_nfree > FREE_LIST_MAX) { delete data; return; } g_nfree++; data->next = g_free; - data->tid = TypeId (); g_free = data; + memset (data->data, 0, TagData::MAX_SIZE); + data->tid = TypeId (); + data->count = 0; } -#else +#else // if USE_FREE_LIST + struct PacketTagList::TagData * PacketTagList::AllocData (void) const { - NS_LOG_FUNCTION (this); - struct PacketTagList::TagData *retval; - retval = new struct PacketTagList::TagData (); - return retval; + NS_LOG_FUNCTION_NOARGS (); + return new struct TagData (); } void PacketTagList::FreeData (struct TagData *data) const { - NS_LOG_FUNCTION (this << data); + NS_LOG_FUNCTION (data); delete data; } -#endif +#endif // if USE_FREE_LIST bool -PacketTagList::Remove (Tag &tag) +PacketTagList::COWTraverse (Tag & tag, PacketTagList::COWWriter_fp Writer) { - NS_LOG_FUNCTION (this << &tag); TypeId tid = tag.GetInstanceTypeId (); - bool found = false; - for (struct TagData *cur = m_next; cur != 0; cur = cur->next) - { - if (cur->tid == tid) - { - found = true; - tag.Deserialize (TagBuffer (cur->data, cur->data+PACKET_TAG_MAX_SIZE)); - } - } - if (!found) + NS_LOG_FUNCTION (this << tid); + NS_LOG_INFO ("looking for " << tid); + + // trivial case when list is empty + if (m_next == 0) { return false; } - struct TagData *start = 0; - struct TagData **prevNext = &start; - for (struct TagData *cur = m_next; cur != 0; cur = cur->next) + + bool found = false; + + struct TagData ** prevNext = &m_next; // previous node's next pointer + struct TagData * cur = m_next; // cursor to current node + struct TagData * it = 0; // utility + + // Search from the head of the list until we find tid or a merge + while (cur != 0) { - if (cur->tid == tid) + if (cur->count > 1) { - /** - * XXX - * Note: I believe that we could optimize this to - * avoid copying each TagData located after the target id - * and just link the already-copied list to the next tag. - */ - continue; + // found merge + NS_LOG_INFO ("found initial merge before tid"); + break; } - struct TagData *copy = AllocData (); + else if (cur->tid == tid) + { + NS_LOG_INFO ("found tid before initial merge, calling writer"); + found = (this->*Writer)(tag, true, cur, prevNext); + break; + } + else + { + // no merge or tid found yet, move on + prevNext = &cur->next; + cur = cur->next; + } + } // while !found && !cow + + // did we find it or run out of tags? + if (cur == 0 || found) + { + NS_LOG_INFO ("returning after header with found: " << found); + return found; + } + + // From here on out, we have to copy the list + // until we find tid, then link past it + + // Before we do all that work, let's make sure tid really exists + for (it = cur; it != 0; it = it->next) + { + if (it->tid == tid) + { + break; + } + } + if (it == 0) + { + // got to end of list without finding tid + NS_LOG_INFO ("tid not found after first merge"); + return found; + } + + // At this point cur is a merge, but untested for tid + NS_ASSERT (cur != 0); + NS_ASSERT (cur->count > 1); + + /* + Walk the remainder of the list, copying, until we find tid + As we put a copy of the cur node onto our list, + we move the merge point down the list. + + Starting position End position + T1 is a merge T1.count decremented + T2 is a merge + T1' is a copy of T1 + + other other + \ \ + Prev -> T1 -> T2 -> ... T1 -> T2 -> ... + / / /| + pNext cur Prev -> T1' --/ | + / | + pNext cur + + When we reach tid, we link past it, decrement count, and we're done. + */ + + // Should normally check for null cur pointer, + // but since we know tid exists, we'll skip this test + while ( /* cur && */ cur->tid != tid) + { + NS_ASSERT (cur != 0); + NS_ASSERT (cur->count > 1); + cur->count--; // unmerge cur + struct TagData * copy = AllocData (); copy->tid = cur->tid; copy->count = 1; - copy->next = 0; - std::memcpy (copy->data, cur->data, PACKET_TAG_MAX_SIZE); - *prevNext = copy; - prevNext = ©->next; + memcpy (copy->data, cur->data, TagData::MAX_SIZE); + copy->next = cur->next; // merge into tail + copy->next->count++; // mark new merge + *prevNext = copy; // point prior list at copy + prevNext = ©->next; // advance + cur = copy->next; } - *prevNext = 0; - RemoveAll (); - m_next = start; - return true; + // Sanity check: + NS_ASSERT (cur != 0); // cur should be non-zero + NS_ASSERT (cur->tid == tid); // cur->tid should be tid + NS_ASSERT (cur->count > 1); // cur should be a merge + + // link around tid, removing it from our list + found = (this->*Writer)(tag, false, cur, prevNext); + return found; + +} + +bool +PacketTagList::Remove (Tag & tag) +{ + return COWTraverse (tag, &PacketTagList::RemoveWriter); +} + +// COWWriter implementing Remove +bool +PacketTagList::RemoveWriter (Tag & tag, bool preMerge, + struct PacketTagList::TagData * cur, + struct PacketTagList::TagData ** prevNext) +{ + NS_LOG_FUNCTION_NOARGS (); + + // found tid + bool found = true; + tag.Deserialize (TagBuffer (cur->data, + cur->data + TagData::MAX_SIZE)); + *prevNext = cur->next; // link around cur + + if (preMerge) + { + // found tid before first merge, so delete cur + FreeData (cur); + } + else + { + // cur is always a merge at this point + // unmerge cur, since we linked around it already + cur->count--; + if (cur->next != 0) + { + // there's a next, so make it a merge + cur->next->count++; + } + } + return found; +} + +bool +PacketTagList::Replace (Tag & tag) +{ + bool found = COWTraverse (tag, &PacketTagList::ReplaceWriter); + if (!found) + { + Add (tag); + } + return found; +} + +// COWWriter implementing Replace +bool +PacketTagList::ReplaceWriter (Tag & tag, bool preMerge, + struct PacketTagList::TagData * cur, + struct PacketTagList::TagData ** prevNext) +{ + NS_LOG_FUNCTION_NOARGS (); + + // found tid + bool found = true; + if (preMerge) + { + // found tid before first merge, so just rewrite + tag.Serialize (TagBuffer (cur->data, + cur->data + tag.GetSerializedSize ())); + } + else + { + // cur is always a merge at this point + // need to copy, replace, and link past cur + cur->count--; // unmerge cur + struct TagData * copy = AllocData (); + copy->tid = tag.GetInstanceTypeId (); + copy->count = 1; + tag.Serialize (TagBuffer (copy->data, + copy->data + tag.GetSerializedSize ())); + copy->next = cur->next; // merge into tail + if (copy->next != 0) + { + copy->next->count++; // mark new merge + } + *prevNext = copy; // point prior list at copy + } + return found; } void PacketTagList::Add (const Tag &tag) const { - NS_LOG_FUNCTION (this << &tag); + NS_LOG_FUNCTION (this << tag.GetInstanceTypeId ()); // ensure this id was not yet added for (struct TagData *cur = m_next; cur != 0; cur = cur->next) { NS_ASSERT (cur->tid != tag.GetInstanceTypeId ()); } - struct TagData *head = AllocData (); + struct TagData * head = AllocData (); head->count = 1; head->next = 0; head->tid = tag.GetInstanceTypeId (); head->next = m_next; - NS_ASSERT (tag.GetSerializedSize () <= PACKET_TAG_MAX_SIZE); - tag.Serialize (TagBuffer (head->data, head->data+tag.GetSerializedSize ())); + NS_ASSERT (tag.GetSerializedSize () <= TagData::MAX_SIZE); + tag.Serialize (TagBuffer (head->data, head->data + tag.GetSerializedSize ())); const_cast (this)->m_next = head; } @@ -152,14 +323,14 @@ PacketTagList::Add (const Tag &tag) const bool PacketTagList::Peek (Tag &tag) const { - NS_LOG_FUNCTION (this << &tag); + NS_LOG_FUNCTION (this << tag.GetInstanceTypeId ()); TypeId tid = tag.GetInstanceTypeId (); for (struct TagData *cur = m_next; cur != 0; cur = cur->next) { if (cur->tid == tid) { /* found tag */ - tag.Deserialize (TagBuffer (cur->data, cur->data+PACKET_TAG_MAX_SIZE)); + tag.Deserialize (TagBuffer (cur->data, cur->data + TagData::MAX_SIZE)); return true; } } @@ -173,5 +344,5 @@ PacketTagList::Head (void) const return m_next; } -} // namespace ns3 +} /* namespace ns3 */ diff --git a/src/network/model/packet-tag-list.h b/src/network/model/packet-tag-list.h index 838e84752..2895b8f72 100644 --- a/src/network/model/packet-tag-list.h +++ b/src/network/model/packet-tag-list.h @@ -20,6 +20,11 @@ #ifndef PACKET_TAG_LIST_H #define PACKET_TAG_LIST_H +/** +\file packet-tag-list.h +\brief Defines a linked list of Packet tags, including copy-on-write semantics. +*/ + #include #include #include "ns3/type-id.h" @@ -28,46 +33,285 @@ namespace ns3 { class Tag; -/** - * \ingroup constants - * \brief Tag maximum size - * The maximum size (in bytes) of a Tag is stored - * in this constant. - */ -#define PACKET_TAG_MAX_SIZE 20 +/** + * \ingroup packet + * + * \brief List of the packet tags stored in a packet. + * + * This class is mostly private to the Packet implementation and users + * should never have to access it directly. + * + * \intern + * + * The implementation of this class is a bit tricky. Refer to this + * diagram in the discussion that follows. + * + * \dot + * digraph { + * rankdir = "LR"; + * clusterrank = local; + * node [ shape = record, fontname="FreeSans", fontsize="10" ]; + * oth [ label=" Other branch | next | ..." ]; + * PTL1 [ label=" PacketTagList A | m_next" , shape=Mrecord]; + * PTL2 [ label=" PacketTagList B | m_next" , shape=Mrecord]; + * oth:n -> T7:l ; + * PTL2:n -> T6:l ; + * PTL1:n -> T5:l ; + * T1 [ label=" T1 | next | count = 1" ]; + * T2 [ label=" T2 | next | count = 1" ]; + * T3 [ label=" T3 | next | count = 2" ]; + * T4 [ label=" T4 | next | count = 1" ]; + * T5 [ label=" T5 | next | count = 2" ]; + * T6 [ label=" T6 | next | count = 1" ]; + * T7 [ label=" T7 | next | count = 1" ]; + * NULL [ label="0", shape = ellipse ]; + * subgraph cluster_list { + * penwidth = 0; + * T6:n -> T5:l ; + * T5:n -> T4:l ; + * T4:n -> T3:l ; + * T7:n -> T3:l ; + * T3:n -> T2:l ; + * T2:n -> T1:l ; + * T1:n -> NULL ; + * }; + * }; + * \enddot + * + * - Tags are stored in serialized form in a tree of #TagData + * structures. (T1-T7 in the diagram) + * + * - Each #TagData points (\c next pointers in the diagram) + * toward the root of the tree, which is a null pointer. + * + * - \c count is the number of incoming pointers to this + * #TagData. Branch points, where branches merge or join, have + * count \> 1 (\c T3, \c T5); succesive links along + * a branch have count = 1 (\c T1, \c T2, \c T4, \c T6, \c T7). + * + * - Each PacketTagList points to a specific #TagData, + * which is the most recent Tag added to the packet. (T5-T7) + * + * - Conceptually, therefore, each Packet has a PacketTagList which + * points to a singly-linked list of #TagData. + * + * \par Copy-on-write is implemented as follows: + * + * - #Add prepends the new tag to the list (growing that branch of the tree, + * as \c T6). This is a constant time operation, and does not affect + * any other #PacketTagList's, hence this is a \c const function. + * + * - Copy constructor (PacketTagList(const PacketTagList & o)) + * and assignment (#operator=(const PacketTagList & o) + * simply join the tree at the same place as the original + * PacketTagList \c o, incrementing the \c count. + * For assignment, the old branch is deleted, up to + * the first branch point, which has its \c count decremented. + * (PacketTagList \c B started as a copy of PacketTagList \c A, + * before \c T6 was added to \c B). + * + * - #Remove and #Replace are a little tricky, depending on where the + * target tag is found relative to the first branch point: + * - \e Target before the first branch point: \n + * The target is just dealt with in place (linked around and deleted, + * in the case of #Remove; rewritten in the case of #Replace). + * - \e Target at or after the first branch point: \n + * 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 + * If the preprocessor symbol \link packet-tag-list.cc#USE_FREE_LIST + * \c USE_FREE_LIST \endlink is true, #FreeData'd #TagData's + * are added to the static free list #g_free. The free list size is bounded. + * Using the free list avoids \c new'ing and \c delete'ing #TagData's + * every time a tag is added/removed from a PacketTagList. + * \n\n + * Packet tags must serialize to a finite maximum size, see #TagData + * + * This documentation entitles the original author to a free beer. + */ class PacketTagList { public: - struct TagData { - uint8_t data[PACKET_TAG_MAX_SIZE]; - struct TagData *next; - TypeId tid; - uint32_t count; + /** + * Tree node for sharing serialized tags. + * + * See PacketTagList for a discussion of the data structure. + * + * See TagData::TagData_e for a discussion of the size limit on + * tag serialization. + */ + struct TagData + { + /** + * \brief Packet Tag maximum size + * + * The maximum size (in bytes) of a Tag is stored + * in this constant. + * + * \intern + * Ideally, #TagData would be 32 bytes in size, so they + * can be added/removed from a single free list, and 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 = 20 /**< 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 */ + + /** + * Create a new PacketTagList. + */ inline PacketTagList (); + /** + * Copy constructor + * + * \param [in] o The PacketTagList to copy. + * + * This makes a light-weight copy by #RemoveAll, then + * pointing to the same #TagData as \pname{o}. + */ inline PacketTagList (PacketTagList const &o); + /** + * Assignment + * + * \param [in] o The PacketTagList to copy. + * + * This makes a light-weight copy by #RemoveAll, then + * pointing to the same #TagData as \pname{o}. + */ inline PacketTagList &operator = (PacketTagList const &o); + /** + * Destructor + * + * #RemoveAll's the tags up to the first merge. + */ inline ~PacketTagList (); + /** + * Add a tag to the head of this branch. + * + * \param [in] tag The tag to add + */ void Add (Tag const&tag) const; + /** + * Remove (the first instance of) tag from the list. + * + * \param [in,out] tag The tag type to remove. If found, + * \pname{tag} is set to the value of the tag found. + * \returns True if \pname{tag} is found, false otherwise. + */ bool Remove (Tag &tag); + /** + * Replace the value of a tag. + * + * \param [in] tag The tag type to replace. To get the old + * value of the tag, use #Peek first. + * \returns True if \pname{tag} is found, false otherwise. + * If \pname{tag} wasn't found, Add is performed instead (so + * the list is guaranteed to have the new tag value either way). + */ + bool Replace (Tag &tag); + /** + * Find a tag and return its value. + * + * \param [in,out] tag The tag type to find. If found, + * \pname{tag} is set to the value of the tag found. + * \returns True if \pname{tag} is found, false otherwise. + */ bool Peek (Tag &tag) const; + /** + * Remove all tags from this list (up to the first merge). + */ inline void RemoveAll (void); - + /** + * \returns pointer to head of tag list + */ const struct PacketTagList::TagData *Head (void) const; private: + /** + * \returns A pointer to a new #TagData + */ + struct TagData * AllocData (void) const; + /** + * Free a #TagData, adding it to the free list. + */ + void FreeData (struct TagData * data) const; - bool Remove (TypeId tid); - struct PacketTagList::TagData *AllocData (void) const; - void FreeData (struct TagData *data) const; + /** + * Typedef of method function pointer for copy-on-write operations + * + * \param [in] tag The tag type to operate on. + * \param [in] preMerge True if \pname{tag} was found before the first merge, + * false otherwise. + * \param [in] cur Pointer to the tag. + * \param [in] prevNext Pointer to the struct TagData.next pointer + * pointing to \pname{cur}. + * \returns True if operation successful, false otherwise + */ + typedef bool (PacketTagList::*COWWriter_fp) + (Tag & tag, bool preMerge, + struct TagData * cur, struct TagData ** prevNext); + /** + * Traverse the list implementing copy-on-write, using \pname{Writer}. + * + * \param [in] tag The tag type to operate on. + * \param [in] Writer The copy-on-write function to use. + * \returns True if \pname{tag} found, false otherwise. + */ + bool COWTraverse (Tag & tag, PacketTagList::COWWriter_fp Writer); + /** + * Copy-on-write implementing Remove. + * + * \param [in] tag The target tag type to remove. + * \param [in] preMerge True if \pname{tag} was found before the first merge, + * false otherwise. + * \param [in] cur Pointer to the tag. + * \param [in] prevNext Pointer to the struct TagData.next pointer + * pointing to \pname{cur}. + * \returns True, since tag will definitely be removed. + */ + bool RemoveWriter (Tag & tag, bool preMerge, + struct TagData * cur, struct TagData ** prevNext); + /** + * Copy-on-write implementing Replace + * + * \param [in] tag The target tag type to replace + * \param [in] preMerge True if \pname{tag} was found before the first merge, + * false otherwise. + * \param [in] cur Pointer to the tag + * \param [in] prevNext Pointer to the struct TagData.next pointer + * 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); - static struct PacketTagList::TagData *g_free; - static uint32_t g_nfree; + enum PacketTagList_e + { + FREE_LIST_MAX = 1000 /**< Maximum size of free list */ + }; - struct TagData *m_next; + static struct TagData * g_free; /**< Head of #TagData free list */ + static uint32_t g_nfree; /**< Number of #TagData's on the free list */ + + struct TagData * m_next; /**< Pointer to first #TagData on the list */ }; } // namespace ns3 @@ -118,7 +362,7 @@ void PacketTagList::RemoveAll (void) { struct TagData *prev = 0; - for (struct TagData *cur = m_next; cur != 0; cur = cur->next) + for (struct TagData *cur = m_next; cur != 0; cur = cur->next) { cur->count--; if (cur->count > 0) diff --git a/src/network/model/packet.cc b/src/network/model/packet.cc index 1fcaff1c6..9fea897ae 100644 --- a/src/network/model/packet.cc +++ b/src/network/model/packet.cc @@ -38,19 +38,16 @@ ByteTagIterator::Item::GetTypeId (void) const uint32_t ByteTagIterator::Item::GetStart (void) const { - NS_LOG_FUNCTION (this); return m_start; } uint32_t ByteTagIterator::Item::GetEnd (void) const { - NS_LOG_FUNCTION (this); return m_end; } void ByteTagIterator::Item::GetTag (Tag &tag) const { - NS_LOG_FUNCTION (this << &tag); if (tag.GetInstanceTypeId () != GetTypeId ()) { NS_FATAL_ERROR ("The tag you provided is not of the right type."); @@ -63,7 +60,6 @@ ByteTagIterator::Item::Item (TypeId tid, uint32_t start, uint32_t end, TagBuffer m_end (end), m_buffer (buffer) { - NS_LOG_FUNCTION (this << tid << start << end << &buffer); } bool ByteTagIterator::HasNext (void) const @@ -73,35 +69,30 @@ ByteTagIterator::HasNext (void) const ByteTagIterator::Item ByteTagIterator::Next (void) { - NS_LOG_FUNCTION (this); ByteTagList::Iterator::Item i = m_current.Next (); return ByteTagIterator::Item (i.tid, - i.start-m_current.GetOffsetStart (), - i.end-m_current.GetOffsetStart (), + i.start - m_current.GetOffsetStart (), + i.end - m_current.GetOffsetStart (), i.buf); } ByteTagIterator::ByteTagIterator (ByteTagList::Iterator i) : m_current (i) { - NS_LOG_FUNCTION (this); } PacketTagIterator::PacketTagIterator (const struct PacketTagList::TagData *head) : m_current (head) { - NS_LOG_FUNCTION (this << head); } bool PacketTagIterator::HasNext (void) const { - NS_LOG_FUNCTION (this); return m_current != 0; } PacketTagIterator::Item PacketTagIterator::Next (void) { - NS_LOG_FUNCTION (this); NS_ASSERT (HasNext ()); const struct PacketTagList::TagData *prev = m_current; m_current = m_current->next; @@ -111,7 +102,6 @@ PacketTagIterator::Next (void) PacketTagIterator::Item::Item (const struct PacketTagList::TagData *data) : m_data (data) { - NS_LOG_FUNCTION (this << data); } TypeId PacketTagIterator::Item::GetTypeId (void) const @@ -121,9 +111,10 @@ PacketTagIterator::Item::GetTypeId (void) const void PacketTagIterator::Item::GetTag (Tag &tag) const { - NS_LOG_FUNCTION (this << &tag); NS_ASSERT (tag.GetInstanceTypeId () == m_data->tid); - tag.Deserialize (TagBuffer ((uint8_t*)m_data->data, (uint8_t*)m_data->data+PACKET_TAG_MAX_SIZE)); + tag.Deserialize (TagBuffer ((uint8_t*)m_data->data, + (uint8_t*)m_data->data + + PacketTagList::TagData::MAX_SIZE)); } @@ -133,7 +124,6 @@ Packet::Copy (void) const // we need to invoke the copy constructor directly // rather than calling Create because the copy constructor // is private. - NS_LOG_FUNCTION (this); return Ptr (new Packet (*this), false); } @@ -150,7 +140,6 @@ Packet::Packet () m_metadata (static_cast (Simulator::GetSystemId ()) << 32 | m_globalUid, 0), m_nixVector (0) { - NS_LOG_FUNCTION (this); m_globalUid++; } @@ -193,7 +182,6 @@ Packet::Packet (uint32_t size) m_metadata (static_cast (Simulator::GetSystemId ()) << 32 | m_globalUid, size), m_nixVector (0) { - NS_LOG_FUNCTION (this << size); m_globalUid++; } Packet::Packet (uint8_t const *buffer, uint32_t size, bool magic) @@ -203,7 +191,6 @@ Packet::Packet (uint8_t const *buffer, uint32_t size, bool magic) m_metadata (0,0), m_nixVector (0) { - NS_LOG_FUNCTION (this << &buffer << size << magic); NS_ASSERT (magic); Deserialize (buffer, size); } @@ -221,7 +208,6 @@ Packet::Packet (uint8_t const*buffer, uint32_t size) m_metadata (static_cast (Simulator::GetSystemId ()) << 32 | m_globalUid, size), m_nixVector (0) { - NS_LOG_FUNCTION (this << &buffer << size); m_globalUid++; m_buffer.AddAtStart (size); Buffer::Iterator i = m_buffer.Begin (); @@ -236,7 +222,6 @@ Packet::Packet (const Buffer &buffer, const ByteTagList &byteTagList, m_metadata (metadata), m_nixVector (0) { - NS_LOG_FUNCTION (this << &buffer << &byteTagList << &packetTagList << &metadata); } Ptr @@ -255,14 +240,12 @@ Packet::CreateFragment (uint32_t start, uint32_t length) const void Packet::SetNixVector (Ptr nixVector) { - NS_LOG_FUNCTION (this << nixVector); m_nixVector = nixVector; } Ptr Packet::GetNixVector (void) const { - NS_LOG_FUNCTION (this); return m_nixVector; } @@ -270,7 +253,7 @@ void Packet::AddHeader (const Header &header) { uint32_t size = header.GetSerializedSize (); - NS_LOG_FUNCTION (this << &header); + NS_LOG_FUNCTION (this << header.GetInstanceTypeId ().GetName () << size); uint32_t orgStart = m_buffer.GetCurrentStartOffset (); bool resized = m_buffer.AddAtStart (size); if (resized) @@ -285,7 +268,7 @@ uint32_t Packet::RemoveHeader (Header &header) { uint32_t deserialized = header.Deserialize (m_buffer.Begin ()); - NS_LOG_FUNCTION (this << &header); + NS_LOG_FUNCTION (this << header.GetInstanceTypeId ().GetName () << deserialized); m_buffer.RemoveAtStart (deserialized); m_metadata.RemoveHeader (header, deserialized); return deserialized; @@ -294,14 +277,14 @@ uint32_t Packet::PeekHeader (Header &header) const { uint32_t deserialized = header.Deserialize (m_buffer.Begin ()); - NS_LOG_FUNCTION (this << &header); + NS_LOG_FUNCTION (this << header.GetInstanceTypeId ().GetName () << deserialized); return deserialized; } void Packet::AddTrailer (const Trailer &trailer) { uint32_t size = trailer.GetSerializedSize (); - NS_LOG_FUNCTION (this << &trailer); + NS_LOG_FUNCTION (this << trailer.GetInstanceTypeId ().GetName () << size); uint32_t orgStart = m_buffer.GetCurrentStartOffset (); bool resized = m_buffer.AddAtEnd (size); if (resized) @@ -317,7 +300,7 @@ uint32_t Packet::RemoveTrailer (Trailer &trailer) { uint32_t deserialized = trailer.Deserialize (m_buffer.End ()); - NS_LOG_FUNCTION (this << &trailer); + NS_LOG_FUNCTION (this << trailer.GetInstanceTypeId ().GetName () << deserialized); m_buffer.RemoveAtEnd (deserialized); m_metadata.RemoveTrailer (trailer, deserialized); return deserialized; @@ -326,14 +309,14 @@ uint32_t Packet::PeekTrailer (Trailer &trailer) { uint32_t deserialized = trailer.Deserialize (m_buffer.End ()); - NS_LOG_FUNCTION (this << &trailer); + NS_LOG_FUNCTION (this << trailer.GetInstanceTypeId ().GetName () << deserialized); return deserialized; } void Packet::AddAtEnd (Ptr packet) { - NS_LOG_FUNCTION (this << packet); + NS_LOG_FUNCTION (this << packet << packet->GetSize ()); uint32_t aStart = m_buffer.GetCurrentStartOffset (); uint32_t bEnd = packet->m_buffer.GetCurrentEndOffset (); m_buffer.AddAtEnd (packet->m_buffer); @@ -390,35 +373,31 @@ Packet::PeekData (void) const uint32_t newStart = m_buffer.GetCurrentStartOffset (); // Update tag offsets if buffer offsets were changed - const_cast(m_byteTagList).AddAtStart (newStart - oldStart, newStart); + const_cast (m_byteTagList).AddAtStart (newStart - oldStart, newStart); return data; } uint32_t Packet::CopyData (uint8_t *buffer, uint32_t size) const { - NS_LOG_FUNCTION (this << &buffer << size); return m_buffer.CopyData (buffer, size); } void Packet::CopyData (std::ostream *os, uint32_t size) const { - NS_LOG_FUNCTION (this << &os << size); return m_buffer.CopyData (os, size); } uint64_t Packet::GetUid (void) const { - NS_LOG_FUNCTION (this); return m_metadata.GetUid (); } void Packet::PrintByteTags (std::ostream &os) const { - NS_LOG_FUNCTION (this << &os); ByteTagIterator i = GetByteTagIterator (); while (i.HasNext ()) { @@ -449,14 +428,14 @@ Packet::PrintByteTags (std::ostream &os) const void Packet::Print (std::ostream &os) const { - NS_LOG_FUNCTION (this << &os); PacketMetadata::ItemIterator i = m_metadata.BeginItem (m_buffer); while (i.HasNext ()) { PacketMetadata::Item item = i.Next (); if (item.isFragment) { - switch (item.type) { + switch (item.type) + { case PacketMetadata::Item::PAYLOAD: os << "Payload"; break; @@ -465,12 +444,13 @@ Packet::Print (std::ostream &os) const os << item.tid.GetName (); break; } - os << " Fragment [" << item.currentTrimedFromStart<<":" + os << " Fragment [" << item.currentTrimedFromStart << ":" << (item.currentTrimedFromStart + item.currentSize) << "]"; } else { - switch (item.type) { + switch (item.type) + { case PacketMetadata::Item::PAYLOAD: os << "Payload (size=" << item.currentSize << ")"; break; @@ -509,7 +489,8 @@ Packet::Print (std::ostream &os) const PacketMetadata::Item item = i.Next (); if (item.isFragment) { - switch (item.type) { + switch (item.type) + { case PacketMetadata::Item::PAYLOAD: os << "Payload"; break; @@ -518,12 +499,13 @@ Packet::Print (std::ostream &os) const os << item.tid.GetName (); break; } - os << " Fragment [" << item.currentTrimedFromStart<<":" + os << " Fragment [" << item.currentTrimedFromStart << ":" << (item.currentTrimedFromStart + item.currentSize) << "]"; } else { - switch (item.type) { + switch (item.type) + { case PacketMetadata::Item::PAYLOAD: os << "Payload (size=" << item.currentSize << ")"; break; @@ -567,7 +549,6 @@ Packet::Print (std::ostream &os) const PacketMetadata::ItemIterator Packet::BeginItem (void) const { - NS_LOG_FUNCTION (this); return m_metadata.BeginItem (m_buffer); } @@ -587,7 +568,6 @@ Packet::EnableChecking (void) uint32_t Packet::GetSerializedSize (void) const { - NS_LOG_FUNCTION (this); uint32_t size = 0; if (m_nixVector) @@ -631,7 +611,6 @@ uint32_t Packet::GetSerializedSize (void) const uint32_t Packet::Serialize (uint8_t* buffer, uint32_t maxSize) const { - NS_LOG_FUNCTION (this << &buffer << maxSize); uint32_t* p = reinterpret_cast (buffer); uint32_t size = 0; @@ -654,7 +633,7 @@ Packet::Serialize (uint8_t* buffer, uint32_t maxSize) const { // increment p by nixSize bytes // ensuring 4-byte boundary - p += ((nixSize+3) & (~3)) / 4; + p += ((nixSize + 3) & (~3)) / 4; } else { @@ -702,7 +681,7 @@ Packet::Serialize (uint8_t* buffer, uint32_t maxSize) const { // increment p by metaSize bytes // ensuring 4-byte boundary - p += ((metaSize+3) & (~3)) / 4; + p += ((metaSize + 3) & (~3)) / 4; } else { @@ -731,7 +710,7 @@ Packet::Serialize (uint8_t* buffer, uint32_t maxSize) const { // increment p by bufSize bytes // ensuring 4-byte boundary - p += ((bufSize+3) & (~3)) / 4; + p += ((bufSize + 3) & (~3)) / 4; } else { @@ -750,7 +729,7 @@ Packet::Serialize (uint8_t* buffer, uint32_t maxSize) const uint32_t Packet::Deserialize (const uint8_t* buffer, uint32_t size) { - NS_LOG_FUNCTION (this << &buffer << size); + NS_LOG_FUNCTION (this); const uint32_t* p = reinterpret_cast (buffer); @@ -832,7 +811,7 @@ Packet::Deserialize (const uint8_t* buffer, uint32_t size) void Packet::AddByteTag (const Tag &tag) const { - NS_LOG_FUNCTION (this << &tag); + NS_LOG_FUNCTION (this << tag.GetInstanceTypeId ().GetName () << tag.GetSerializedSize ()); ByteTagList *list = const_cast (&m_byteTagList); TagBuffer buffer = list->Add (tag.GetInstanceTypeId (), tag.GetSerializedSize (), m_buffer.GetCurrentStartOffset (), @@ -842,14 +821,12 @@ Packet::AddByteTag (const Tag &tag) const ByteTagIterator Packet::GetByteTagIterator (void) const { - NS_LOG_FUNCTION (this); return ByteTagIterator (m_byteTagList.Begin (m_buffer.GetCurrentStartOffset (), m_buffer.GetCurrentEndOffset ())); } bool Packet::FindFirstMatchingByteTag (Tag &tag) const { - NS_LOG_FUNCTION (this << &tag); TypeId tid = tag.GetInstanceTypeId (); ByteTagIterator i = GetByteTagIterator (); while (i.HasNext ()) @@ -867,20 +844,29 @@ Packet::FindFirstMatchingByteTag (Tag &tag) const void Packet::AddPacketTag (const Tag &tag) const { - NS_LOG_FUNCTION (this << &tag); + NS_LOG_FUNCTION (this << tag.GetInstanceTypeId ().GetName () << tag.GetSerializedSize ()); m_packetTagList.Add (tag); } + bool Packet::RemovePacketTag (Tag &tag) { - NS_LOG_FUNCTION (this << &tag); + NS_LOG_FUNCTION (this << tag.GetInstanceTypeId ().GetName () << tag.GetSerializedSize ()); bool found = m_packetTagList.Remove (tag); return found; } + +bool +Packet::ReplacePacketTag (Tag &tag) +{ + NS_LOG_FUNCTION (this << tag.GetInstanceTypeId ().GetName () << tag.GetSerializedSize ()); + bool found = m_packetTagList.Replace (tag); + return found; +} + bool Packet::PeekPacketTag (Tag &tag) const { - NS_LOG_FUNCTION (this << &tag); bool found = m_packetTagList.Peek (tag); return found; } @@ -894,7 +880,6 @@ Packet::RemoveAllPacketTags (void) void Packet::PrintPacketTags (std::ostream &os) const { - NS_LOG_FUNCTION (this << &os); PacketTagIterator i = GetPacketTagIterator (); while (i.HasNext ()) { @@ -918,7 +903,6 @@ Packet::PrintPacketTags (std::ostream &os) const PacketTagIterator Packet::GetPacketTagIterator (void) const { - NS_LOG_FUNCTION (this); return PacketTagIterator (m_packetTagList.Head ()); } diff --git a/src/network/model/packet.h b/src/network/model/packet.h index 539dac594..c04a2983e 100644 --- a/src/network/model/packet.h +++ b/src/network/model/packet.h @@ -43,7 +43,7 @@ namespace ns3 { /** * \ingroup packet - * \brief Iterator over the set of tags in a packet + * \brief Iterator over the set of byte tags in a packet * * This is a java-style iterator. */ @@ -51,7 +51,7 @@ class ByteTagIterator { public: /** - * Identifies a tag and a set of bytes within a packet + * Identifies a byte tag and a set of bytes within a packet * to which the tag applies. */ class Item @@ -74,12 +74,12 @@ public: */ uint32_t GetEnd (void) const; /** + * Read the requested tag and store it in the user-provided tag instance. + * * \param tag the user tag to which the data should be copied. * - * Read the requested tag and store it in the user-provided - * tag instance. This method will crash if the type of the - * tag provided by the user does not match the type of - * the underlying tag. + * This method will crash if the type of the tag provided + * by the user does not match the type of the underlying tag. */ void GetTag (Tag &tag) const; private: @@ -106,7 +106,7 @@ private: /** * \ingroup packet - * \brief Iterator over the set of 'packet' tags in a packet + * \brief Iterator over the set of packet tags in a packet * * This is a java-style iterator. */ @@ -114,7 +114,7 @@ class PacketTagIterator { public: /** - * Identifies a tag within a packet. + * Identifies a packet tag within a packet. */ class Item { @@ -124,12 +124,12 @@ public: */ TypeId GetTypeId (void) const; /** + * Read the requested tag and store it in the user-provided tag instance. + * * \param tag the user tag to which the data should be copied. * - * Read the requested tag and store it in the user-provided - * tag instance. This method will crash if the type of the - * tag provided by the user does not match the type of - * the underlying tag. + * This method will crash if the type of the tag provided + * by the user does not match the type of the underlying tag. */ void GetTag (Tag &tag) const; private: @@ -189,6 +189,11 @@ private: * qos class id set by an application and processed by a lower-level MAC * layer. * + * - Packet tags must have unique types; repeated tags of a single type + * (even with different values) can't be attached to a single packet. + * Packet tags must serialize to a finite maximum size, see + * PacketTagList::TagData. + * * Implementing a new type of Header or Trailer for a new protocol is * pretty easy and is a matter of creating a subclass of the ns3::Header * or of the ns3::Trailer base class, and implementing the methods @@ -197,13 +202,12 @@ private: * Implementing a new type of Tag requires roughly the same amount of * work and this work is described in the ns3::Tag API documentation. * - * The performance aspects of the Packet API are discussed in - * \ref packetperf + * The performance aspects copy-on-write semantics of the + * Packet API are discussed in \ref packetperf */ class Packet : public SimpleRefCount { public: - /** * Create an empty packet with a new uid (as returned * by getUid). @@ -320,7 +324,7 @@ public: void AddPaddingAtEnd (uint32_t size); /** * Remove size bytes from the end of the current packet - * It is safe to remove more bytes that what is present in + * It is safe to remove more bytes than arepresent in * the packet. * * \param size number of bytes from remove @@ -328,7 +332,7 @@ public: void RemoveAtEnd (uint32_t size); /** * Remove size bytes from the start of the current packet. - * It is safe to remove more bytes that what is present in + * It is safe to remove more bytes than are present in * the packet. * * \param size number of bytes from remove @@ -336,28 +340,35 @@ public: void RemoveAtStart (uint32_t size); /** + * \returns a pointer to the internal buffer of the packet. + * * If you try to change the content of the buffer * returned by this method, you will die. * Note that this method is now deprecated and will be removed in - * the next version of ns-3. If you need to get access to the content - * of the byte buffer of a packet, you need to call - * ns3::Packet::CopyData to perform an explicit copy. + * a future version of ns-3. To get access to the content + * of the byte buffer of a packet, call CopyData"()" to perform + * an explicit copy. * - * \returns a pointer to the internal buffer of the packet. */ - uint8_t const *PeekData (void) const NS_DEPRECATED; + uint8_t const * PeekData (void) const NS_DEPRECATED; /** + * Copy the packet contents to a byte buffer. + * * \param buffer a pointer to a byte buffer where the packet data * should be copied. * \param size the size of the byte buffer. * \returns the number of bytes read from the packet * - * No more than \b size bytes will be copied by this function. + * No more than \b size bytes will be copied by this function, + * even if the packet has more content. Use GetSize"()" to + * to find the total size of the buffer needed. */ uint32_t CopyData (uint8_t *buffer, uint32_t size) const; /** + * Copy the packet contents to an output stream. + * * \param os pointer to output stream in which we want * to write the packet data. * \param size the maximum number of bytes we want to write @@ -368,7 +379,7 @@ public: /** * \returns a COW copy of the packet. * - * The returns packet will behave like an independent copy of + * The returned packet will behave like an independent copy of * the original packet, even though they both share the * same datasets internally. */ @@ -432,30 +443,29 @@ public: static void EnableChecking (void); /** - * For packet serializtion, the total size is checked + * \returns number of bytes required for packet + * serialization + * + * For packet serialization, the total size is checked * in order to determine the size of the buffer * required for serialization - * - * \returns number of bytes required for packet - * serialization */ uint32_t GetSerializedSize (void) const; - /* + /** + * Serialize a packet, tags, and metadata into a byte buffer. + * * \param buffer a raw byte buffer to which the packet will be serialized * \param maxSize the max size of the buffer for bounds checking * - * A packet is completely serialized and placed into the raw byte buffer - * - * \returns zero if buffer size was too small + * \returns one if all data were serialized, zero if buffer size was too small. */ uint32_t Serialize (uint8_t* buffer, uint32_t maxSize) const; /** - * \param tag the new tag to add to this packet + * Tag each byte included in this packet with a new byte tag. * - * Tag each byte included in this packet with the - * new tag. + * \param tag the new tag to add to this packet * * Note that adding a tag is a const operation which is pretty * un-intuitive. The rationale is that the content and behavior of @@ -474,7 +484,7 @@ public: */ ByteTagIterator GetByteTagIterator (void) const; /** - * \param tag the tag to search in this packet + * \param tag the byte tag type to search in this packet * \returns true if the requested tag type was found, false otherwise. * * If the requested tag type is found, it is copied in the user's @@ -483,44 +493,54 @@ public: bool FindFirstMatchingByteTag (Tag &tag) const; /** - * Remove all the tags stored in this packet. + * Remove all byte tags stored in this packet. */ void RemoveAllByteTags (void); /** * \param os output stream in which the data should be printed. * - * Iterate over the tags present in this packet, and + * Iterate over the byte tags present in this packet, and * invoke the Print method of each tag stored in the packet. */ void PrintByteTags (std::ostream &os) const; /** - * \param tag the tag to store in this packet + * Add a packet tag. * - * Add a tag to this packet. This method calls the - * Tag::GetSerializedSize and, then, Tag::Serialize. + * \param tag the packet tag type to add. * * Note that this method is const, that is, it does not * modify the state of this packet, which is fairly - * un-intuitive. + * un-intuitive. See AddByteTag"()" discussion. */ void AddPacketTag (const Tag &tag) const; /** - * \param tag the tag to remove from this packet + * Remove a packet tag. + * + * \param tag the packet tag type to remove from this packet. + * The tag parameter is set to the value of the tag found. * \returns true if the requested tag is found, false * otherwise. - * - * Remove a tag from this packet. This method calls - * Tag::Deserialize if the tag is found. */ bool RemovePacketTag (Tag &tag); /** + * Replace the value of a packet tag. + * + * \param tag the packet tag type to replace. To get the old + * value of the tag, use PeekPacketTag first. + * \returns true if the requested tag is found, false otherwise. + * If the tag isn't found, Add is performed instead (so + * the packet is guaranteed to have the new tag value + * either way). + */ + bool ReplacePacketTag (Tag & tag); + /** + * Search a matching tag and call Tag::Deserialize if it is found. + * * \param tag the tag to search in this packet * \returns true if the requested tag is found, false * otherwise. - * - * Search a matching tag and call Tag::Deserialize if it is found. */ bool PeekPacketTag (Tag &tag) const; /** @@ -529,9 +549,9 @@ public: void RemoveAllPacketTags (void); /** - * \param os the stream in which we want to print data. + * Print the list of packet tags. * - * Print the list of 'packet' tags. + * \param os the stream on which to print the tags. * * \sa Packet::AddPacketTag, Packet::RemovePacketTag, Packet::PeekPacketTag, * Packet::RemoveAllPacketTags @@ -544,13 +564,22 @@ public: */ PacketTagIterator GetPacketTagIterator (void) const; - /* Note: These functions support a temporary solution + /** + * Set the packet nix-vector. + * + * Note: This function supports a temporary solution * to a specific problem in this generic class, i.e. * how to associate something specific like nix-vector * with a packet. This design methodology * should _not_ be followed, and is only here as an - * impetus to fix this general issue. */ + * impetus to fix this general issue. + */ void SetNixVector (Ptr); + /** + * Get the packet nix-vector. + * + * See the comment on SetNixVector + */ Ptr GetNixVector (void) const; private: @@ -590,6 +619,7 @@ std::ostream& operator<< (std::ostream& os, const Packet &packet); * - ns3::Packet::AddTrailer * - both versions of ns3::Packet::AddAtEnd * - ns3::Packet::RemovePacketTag + * - ns3::Packet::ReplacePacketTag * * Non-dirty operations: * - ns3::Packet::AddPacketTag @@ -614,6 +644,10 @@ std::ostream& operator<< (std::ostream& os, const Packet &packet); } // namespace ns3 +/**************************************************** + * Implementation of inline methods for performance + ****************************************************/ + namespace ns3 { uint32_t diff --git a/src/network/model/tag.h b/src/network/model/tag.h index f25cecf4f..678e1c915 100644 --- a/src/network/model/tag.h +++ b/src/network/model/tag.h @@ -31,7 +31,7 @@ namespace ns3 { * * \brief tag a set of bytes in a packet * - * New kinds of tags can be created by subclassing this base class. + * New kinds of tags can be created by subclassing from this abstract base class. */ class Tag : public ObjectBase { diff --git a/src/network/test/packet-test-suite.cc b/src/network/test/packet-test-suite.cc index ec995ac52..0a3020ac2 100644 --- a/src/network/test/packet-test-suite.cc +++ b/src/network/test/packet-test-suite.cc @@ -18,9 +18,12 @@ * Author: Mathieu Lacage */ #include "ns3/packet.h" +#include "ns3/packet-tag-list.h" #include "ns3/test.h" #include #include +#include +#include using namespace ns3; @@ -32,8 +35,14 @@ namespace { class ATestTagBase : public Tag { public: - ATestTagBase () : m_error (false) {} + ATestTagBase () : m_error (false), m_data (0) {} + ATestTagBase (uint8_t data) : m_error (false), m_data (data) {} + virtual int GetData () const { + int result = (int)m_data; + return result; + } bool m_error; + uint8_t m_data; }; template @@ -54,15 +63,17 @@ public: return GetTypeId (); } virtual uint32_t GetSerializedSize (void) const { - return N; + return N + sizeof(m_data); } virtual void Serialize (TagBuffer buf) const { + buf.WriteU8 (m_data); for (uint32_t i = 0; i < N; ++i) { buf.WriteU8 (N); } } virtual void Deserialize (TagBuffer buf) { + m_data = buf.ReadU8 (); for (uint32_t i = 0; i < N; ++i) { uint8_t v = buf.ReadU8 (); @@ -73,10 +84,12 @@ public: } } virtual void Print (std::ostream &os) const { - os << N; + os << N << "(" << m_data << ")"; } ATestTag () : ATestTagBase () {} + ATestTag (uint8_t data) + : ATestTagBase (data) {} }; class ATestHeaderBase : public Header @@ -435,6 +448,201 @@ PacketTest::DoRun (void) #endif } } +//-------------------------------------- +class PacketTagListTest : public TestCase +{ +public: + PacketTagListTest (); + virtual ~PacketTagListTest (); +private: + void DoRun (void); + void CheckRef (PacketTagList & ref, ATestTagBase & t, + const char * msg, bool miss = false); + void CheckRefList (PacketTagList & ref, const char * msg, int miss = 0); + void RemoveTime (const PacketTagList & ref, ATestTagBase & t, + const char * msg); + void AddRemoveTime (); +}; + +PacketTagListTest::PacketTagListTest () + : TestCase ("PacketTagListTest: ") +{ +} + +PacketTagListTest::~PacketTagListTest () +{ +} + +void +PacketTagListTest::CheckRef (PacketTagList & ref, ATestTagBase & t, + const char * msg, bool miss) +{ + int expect = t.GetData (); // the value we should find + bool found = ref.Peek (t); // rewrites t with actual value + NS_TEST_EXPECT_MSG_EQ (found, !miss, + msg << ": ref contains " + << t.GetTypeId ().GetName ()); + if (found) { + NS_TEST_EXPECT_MSG_EQ (t.GetData (), expect, + msg << ": ref " << t.GetTypeId ().GetName () + << " = " << expect); + } +} + + // A set of tags with data value 1, to check COW +#define MAKE_REF_TAGS \ + ATestTag<1> t1 (1); \ + ATestTag<2> t2 (1); \ + ATestTag<3> t3 (1); \ + ATestTag<4> t4 (1); \ + ATestTag<5> t5 (1); \ + ATestTag<6> t6 (1); \ + ATestTag<7> t7 (1) + +void +PacketTagListTest::CheckRefList (PacketTagList & ref, + const char * msg, + int miss /* = 0 */) +{ + MAKE_REF_TAGS ; + CheckRef (ref, t1, msg, miss == 1); + CheckRef (ref, t2, msg, miss == 2); + CheckRef (ref, t3, msg, miss == 3); + CheckRef (ref, t4, msg, miss == 4); + CheckRef (ref, t5, msg, miss == 5); + CheckRef (ref, t6, msg, miss == 6); + CheckRef (ref, t7, msg, miss == 7); +} + +void +PacketTagListTest::RemoveTime (const PacketTagList & ref, ATestTagBase & t, + const char * msg) +{ + const int reps = 10000; + std::vector< PacketTagList > ptv(reps, ref); + int start = clock (); + for (int i = 0; i < reps; ++i) { + ptv[i].Remove (t); + } + int stop = clock (); + std::cout << GetName () << "remove time: " << msg << ": " + << stop - start << " ticks" + << std::endl; +} + +void +PacketTagListTest::AddRemoveTime () +{ + /* old free list priming + const int nfree = 1000 - 10; + + // get some stuff on the free list + std::cout << GetName () << "free list: initial: " + << PacketTagList::GetNFree (); + std::vector< PacketTagList > ptv; + for (int i = 0; i < nfree; ++i) { + PacketTagList p; + p.Add (ATestTag <1> (i)); + ptv.push_back (p); + } + ptv.clear (); + std::cout << ", now: " + << PacketTagList::GetNFree () + << std::endl; + */ + + // timing + const int reps = 100000; + PacketTagList ptl; + ATestTag <2> t; + int start = clock (); + for (int i = 0; i < reps; ++i) { + ptl.Add (t); + ptl.Remove (t); + } + int stop = clock (); + std::cout << GetName () << "freelist time: " + << stop - start << " ticks" + << std::endl; +} + +void +PacketTagListTest::DoRun (void) +{ + std::cout << GetName () << "begin" << std::endl; + + MAKE_REF_TAGS ; + + PacketTagList ref; // empty list + ref.Add (t1); // last + ref.Add (t2); // post merge + ref.Add (t3); // merge successor + ref.Add (t4); // merge + ref.Add (t5); // merge precursor + ref.Add (t6); // pre-merge + ref.Add (t7); // first + + std::cout << GetName () << "missing tag" << std::endl;; + ATestTag<10> t10; + NS_TEST_EXPECT_MSG_EQ (ref.Peek (t10), false, "missing tag"); + + std::cout << GetName () << "copy and assignment" << std::endl; + { PacketTagList ptl (ref); + CheckRefList (ref, "copy ctor orig"); + CheckRefList (ptl, "copy ctor copy"); + } + { PacketTagList ptl = ref; + CheckRefList (ref, "assignment orig"); + CheckRefList (ptl, "assignment copy"); + } + + std::cout << GetName () << "remove" << std::endl; +#define RemoveCheck(n) \ + { PacketTagList p ## n = ref; \ + p ## n .Remove ( t ## n ); \ + CheckRefList (ref, "remove " #n " orig"); \ + CheckRefList (p ## n, "remove " #n " copy", n); \ + } + + RemoveCheck (1); + RemoveCheck (2); + RemoveCheck (3); + RemoveCheck (4); + RemoveCheck (5); + RemoveCheck (6); + RemoveCheck (7); + + std::cout << GetName () << "replace" << std::endl; +#define ReplaceCheck(n) \ + t ## n .m_data = 2; \ + { PacketTagList p ## n = ref; \ + p ## n .Replace ( t ## n ); \ + CheckRefList (ref, "replace " #n " orig"); \ + CheckRef (p ## n, t ## n, "replace " #n " copy"); \ + } + + ReplaceCheck (1); + ReplaceCheck (2); + ReplaceCheck (3); + ReplaceCheck (4); + ReplaceCheck (5); + ReplaceCheck (6); + ReplaceCheck (7); + + std::cout << GetName () << "freelist timing" << std::endl; + AddRemoveTime (); + + std::cout << GetName () << "remove timing" << std::endl; + RemoveTime (ref, t7, "t7"); + RemoveTime (ref, t6, "t6"); + RemoveTime (ref, t5, "t5"); + RemoveTime (ref, t4, "t4"); + RemoveTime (ref, t3, "t3"); + RemoveTime (ref, t2, "t2"); + RemoveTime (ref, t1, "t1"); + RemoveTime (ref, t7, "t7"); +} + //----------------------------------------------------------------------------- class PacketTestSuite : public TestSuite { @@ -445,7 +653,8 @@ public: PacketTestSuite::PacketTestSuite () : TestSuite ("packet", UNIT) { - AddTestCase (new PacketTest, TestCase::QUICK); + AddTestCase (new PacketTest); + AddTestCase (new PacketTagListTest); } static PacketTestSuite g_packetTestSuite; From a10b29e1fdbb98b89976233e4453e81d5e27eb30 Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Tue, 13 Nov 2012 23:51:56 -0800 Subject: [PATCH 2/8] Remove free list from PacketTagList. --- src/network/model/packet-tag-list.cc | 68 ++------------------------- src/network/model/packet-tag-list.h | 39 ++++----------- src/network/test/packet-test-suite.cc | 2 + 3 files changed, 15 insertions(+), 94 deletions(-) diff --git a/src/network/model/packet-tag-list.cc b/src/network/model/packet-tag-list.cc index 97ee625d6..229d02182 100644 --- a/src/network/model/packet-tag-list.cc +++ b/src/network/model/packet-tag-list.cc @@ -34,66 +34,6 @@ NS_LOG_COMPONENT_DEFINE ("PacketTagList"); namespace ns3 { -#ifndef USE_FREE_LIST -#define USE_FREE_LIST 0 -#endif - -#if USE_FREE_LIST - -struct PacketTagList::TagData * PacketTagList::g_free = 0; -uint32_t PacketTagList::g_nfree = 0; - -struct PacketTagList::TagData * -PacketTagList::AllocData (void) const -{ - NS_LOG_FUNCTION (g_nfree); - struct TagData * retval; - if (g_free != 0) - { - retval = g_free; - g_free = g_free->next; - g_nfree--; - } - else - { - retval = new struct TagData (); - } - return retval; -} - -void -PacketTagList::FreeData (struct TagData *data) const -{ - NS_LOG_FUNCTION (g_nfree << data); - if (g_nfree > FREE_LIST_MAX) - { - delete data; - return; - } - g_nfree++; - data->next = g_free; - g_free = data; - memset (data->data, 0, TagData::MAX_SIZE); - data->tid = TypeId (); - data->count = 0; -} -#else // if USE_FREE_LIST - -struct PacketTagList::TagData * -PacketTagList::AllocData (void) const -{ - NS_LOG_FUNCTION_NOARGS (); - return new struct TagData (); -} - -void -PacketTagList::FreeData (struct TagData *data) const -{ - NS_LOG_FUNCTION (data); - delete data; -} -#endif // if USE_FREE_LIST - bool PacketTagList::COWTraverse (Tag & tag, PacketTagList::COWWriter_fp Writer) { @@ -193,7 +133,7 @@ PacketTagList::COWTraverse (Tag & tag, PacketTagList::COWWriter_fp Writer) NS_ASSERT (cur != 0); NS_ASSERT (cur->count > 1); cur->count--; // unmerge cur - struct TagData * copy = AllocData (); + struct TagData * copy = new struct TagData (); copy->tid = cur->tid; copy->count = 1; memcpy (copy->data, cur->data, TagData::MAX_SIZE); @@ -237,7 +177,7 @@ PacketTagList::RemoveWriter (Tag & tag, bool preMerge, if (preMerge) { // found tid before first merge, so delete cur - FreeData (cur); + delete cur; } else { @@ -285,7 +225,7 @@ PacketTagList::ReplaceWriter (Tag & tag, bool preMerge, // cur is always a merge at this point // need to copy, replace, and link past cur cur->count--; // unmerge cur - struct TagData * copy = AllocData (); + struct TagData * copy = new struct TagData (); copy->tid = tag.GetInstanceTypeId (); copy->count = 1; tag.Serialize (TagBuffer (copy->data, @@ -309,7 +249,7 @@ PacketTagList::Add (const Tag &tag) const { NS_ASSERT (cur->tid != tag.GetInstanceTypeId ()); } - struct TagData * head = AllocData (); + struct TagData * head = new struct TagData (); head->count = 1; head->next = 0; head->tid = tag.GetInstanceTypeId (); diff --git a/src/network/model/packet-tag-list.h b/src/network/model/packet-tag-list.h index 2895b8f72..efc2f619c 100644 --- a/src/network/model/packet-tag-list.h +++ b/src/network/model/packet-tag-list.h @@ -123,12 +123,6 @@ class Tag; * * \par Memory Management: * \n - * If the preprocessor symbol \link packet-tag-list.cc#USE_FREE_LIST - * \c USE_FREE_LIST \endlink is true, #FreeData'd #TagData's - * are added to the static free list #g_free. The free list size is bounded. - * Using the free list avoids \c new'ing and \c delete'ing #TagData's - * every time a tag is added/removed from a PacketTagList. - * \n\n * Packet tags must serialize to a finite maximum size, see #TagData * * This documentation entitles the original author to a free beer. @@ -153,8 +147,7 @@ public: * in this constant. * * \intern - * Ideally, #TagData would be 32 bytes in size, so they - * can be added/removed from a single free list, and require + * 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, @@ -185,7 +178,7 @@ public: * \param [in] o The PacketTagList to copy. * * This makes a light-weight copy by #RemoveAll, then - * pointing to the same #TagData as \pname{o}. + * pointing to the same #struct TagData as \pname{o}. */ inline PacketTagList (PacketTagList const &o); /** @@ -194,7 +187,7 @@ public: * \param [in] o The PacketTagList to copy. * * This makes a light-weight copy by #RemoveAll, then - * pointing to the same #TagData as \pname{o}. + * pointing to the same #struct TagData as \pname{o}. */ inline PacketTagList &operator = (PacketTagList const &o); /** @@ -246,15 +239,6 @@ public: const struct PacketTagList::TagData *Head (void) const; private: - /** - * \returns A pointer to a new #TagData - */ - struct TagData * AllocData (void) const; - /** - * Free a #TagData, adding it to the free list. - */ - void FreeData (struct TagData * data) const; - /** * Typedef of method function pointer for copy-on-write operations * @@ -303,15 +287,10 @@ private: */ bool ReplaceWriter (Tag & tag, bool preMerge, struct TagData * cur, struct TagData ** prevNext); - enum PacketTagList_e - { - FREE_LIST_MAX = 1000 /**< Maximum size of free list */ - }; - - static struct TagData * g_free; /**< Head of #TagData free list */ - static uint32_t g_nfree; /**< Number of #TagData's on the free list */ - - struct TagData * m_next; /**< Pointer to first #TagData on the list */ + /** + * Pointer to first #struct TagData on the list + */ + struct TagData *m_next; }; } // namespace ns3 @@ -371,13 +350,13 @@ PacketTagList::RemoveAll (void) } if (prev != 0) { - FreeData (prev); + delete prev; } prev = cur; } if (prev != 0) { - FreeData (prev); + delete prev; } m_next = 0; } diff --git a/src/network/test/packet-test-suite.cc b/src/network/test/packet-test-suite.cc index 0a3020ac2..04fc019cc 100644 --- a/src/network/test/packet-test-suite.cc +++ b/src/network/test/packet-test-suite.cc @@ -630,7 +630,9 @@ PacketTagListTest::DoRun (void) ReplaceCheck (7); std::cout << GetName () << "freelist timing" << std::endl; + for (int i = 0; i < 100; ++i) { AddRemoveTime (); + } std::cout << GetName () << "remove timing" << std::endl; RemoveTime (ref, t7, "t7"); From 06addb49e0ac985049ce46e925333bd8b34312fd Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Tue, 13 Nov 2012 23:52:34 -0800 Subject: [PATCH 3/8] Minor doc corrections --- src/network/model/packet-tag-list.h | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/network/model/packet-tag-list.h b/src/network/model/packet-tag-list.h index efc2f619c..60878a3e0 100644 --- a/src/network/model/packet-tag-list.h +++ b/src/network/model/packet-tag-list.h @@ -79,22 +79,22 @@ class Tag; * }; * \enddot * - * - Tags are stored in serialized form in a tree of #TagData + * - Tags are stored in serialized form in a tree of TagData * structures. (T1-T7 in the diagram) * - * - Each #TagData points (\c next pointers in the diagram) + * - Each TagData points (\c next pointers in the diagram) * toward the root of the tree, which is a null pointer. * * - \c count is the number of incoming pointers to this - * #TagData. Branch points, where branches merge or join, have - * count \> 1 (\c T3, \c T5); succesive links along + * TagData. Branch points, where branches merge or join, have + * count \> 1 (\c T3, \c T5); successive links along * a branch have count = 1 (\c T1, \c T2, \c T4, \c T6, \c T7). * - * - Each PacketTagList points to a specific #TagData, + * - Each PacketTagList points to a specific TagData, * which is the most recent Tag added to the packet. (T5-T7) * * - Conceptually, therefore, each Packet has a PacketTagList which - * points to a singly-linked list of #TagData. + * points to a singly-linked list of TagData. * * \par Copy-on-write is implemented as follows: * @@ -123,7 +123,7 @@ class Tag; * * \par Memory Management: * \n - * Packet tags must serialize to a finite maximum size, see #TagData + * Packet tags must serialize to a finite maximum size, see TagData * * This documentation entitles the original author to a free beer. */ @@ -147,12 +147,12 @@ public: * in this constant. * * \intern - * Ideally, #TagData would be 32 bytes in size, so they require + * 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 + * 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. From b26e2d5fb2be7bb42376f69ddae762ea773cfe83 Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Tue, 13 Nov 2012 23:53:08 -0800 Subject: [PATCH 4/8] Add benchmark documentation, run timing. --- utils/bench-packets.cc | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/utils/bench-packets.cc b/utils/bench-packets.cc index ad951072c..f6a464c97 100644 --- a/utils/bench-packets.cc +++ b/utils/bench-packets.cc @@ -252,7 +252,10 @@ runBench (void (*bench) (uint32_t), uint32_t n, char const *name) double ps = n; ps *= 1000; ps /= deltaMs; - std::cout << name<<"=" << ps << " packets/s" << std::endl; + std::cout << ps << " packets/s" + << " (" << deltaMs << " ms elapsed)\t" + << name + << std::endl; } int main (int argc, char *argv[]) @@ -280,11 +283,12 @@ int main (int argc, char *argv[]) exit (1); } std::cout << "Running bench-packets with n=" << n << std::endl; + std::cout << "All tests begin by adding UDP and IPv4 headers." << std::endl; - runBench (&benchA, n, "a"); - runBench (&benchB, n, "b"); - runBench (&benchC, n, "c"); - runBench (&benchD, n, "d"); + runBench (&benchA, n, "Copy packet, remove headers"); + runBench (&benchB, n, "Just add headers"); + runBench (&benchC, n, "Remove by func call"); + runBench (&benchD, n, "Intermixed add/remove headers and tags"); return 0; } From 8a5e56e41472c01cf6f35cc043b7097d169ffa87 Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Tue, 13 Nov 2012 23:56:26 -0800 Subject: [PATCH 5/8] Measure micro-benchmark timing. --- src/network/test/packet-test-suite.cc | 53 +++++++++++++++++++-------- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/src/network/test/packet-test-suite.cc b/src/network/test/packet-test-suite.cc index 04fc019cc..49acffc44 100644 --- a/src/network/test/packet-test-suite.cc +++ b/src/network/test/packet-test-suite.cc @@ -20,6 +20,7 @@ #include "ns3/packet.h" #include "ns3/packet-tag-list.h" #include "ns3/test.h" +#include // std:numeric_limits #include #include #include @@ -459,9 +460,9 @@ private: void CheckRef (PacketTagList & ref, ATestTagBase & t, const char * msg, bool miss = false); void CheckRefList (PacketTagList & ref, const char * msg, int miss = 0); - void RemoveTime (const PacketTagList & ref, ATestTagBase & t, + int RemoveTime (const PacketTagList & ref, ATestTagBase & t, const char * msg); - void AddRemoveTime (); + int AddRemoveTime (); }; PacketTagListTest::PacketTagListTest () @@ -514,7 +515,7 @@ PacketTagListTest::CheckRefList (PacketTagList & ref, CheckRef (ref, t7, msg, miss == 7); } -void +int PacketTagListTest::RemoveTime (const PacketTagList & ref, ATestTagBase & t, const char * msg) { @@ -525,12 +526,14 @@ PacketTagListTest::RemoveTime (const PacketTagList & ref, ATestTagBase & t, ptv[i].Remove (t); } int stop = clock (); + int delta = stop - start; std::cout << GetName () << "remove time: " << msg << ": " - << stop - start << " ticks" + << delta << " ticks" << std::endl; + return delta; } -void +int PacketTagListTest::AddRemoveTime () { /* old free list priming @@ -561,9 +564,11 @@ PacketTagListTest::AddRemoveTime () ptl.Remove (t); } int stop = clock (); + int delta = stop - start; std::cout << GetName () << "freelist time: " - << stop - start << " ticks" + << delta << " ticks" << std::endl; + return delta; } void @@ -630,19 +635,37 @@ PacketTagListTest::DoRun (void) ReplaceCheck (7); std::cout << GetName () << "freelist timing" << std::endl; + int flm = std::numeric_limits::max (); for (int i = 0; i < 100; ++i) { - AddRemoveTime (); + int now = AddRemoveTime (); + if (now < flm) flm = now; } + std::cout << GetName () << "min freelist time: " + << flm << " ticks" << std::endl; std::cout << GetName () << "remove timing" << std::endl; - RemoveTime (ref, t7, "t7"); - RemoveTime (ref, t6, "t6"); - RemoveTime (ref, t5, "t5"); - RemoveTime (ref, t4, "t4"); - RemoveTime (ref, t3, "t3"); - RemoveTime (ref, t2, "t2"); - RemoveTime (ref, t1, "t1"); - RemoveTime (ref, t7, "t7"); + std::vector rmn (7, std::numeric_limits::max ()); + for (int i = 0; i < 100; ++i) { + for (int j = 1; j < 8; ++j) { + int now = 0; + switch (j) { + case 7: now = RemoveTime (ref, t7, "t7"); break; + case 6: now = RemoveTime (ref, t6, "t6"); break; + case 5: now = RemoveTime (ref, t5, "t5"); break; + case 4: now = RemoveTime (ref, t4, "t4"); break; + case 3: now = RemoveTime (ref, t3, "t3"); break; + case 2: now = RemoveTime (ref, t2, "t2"); break; + case 1: now = RemoveTime (ref, t1, "t1"); break; + } // switch + + if (now < rmn[j]) rmn[j] = now; + } // for tag j + } // for iteration i + for (int j = 7; j > 0; --j) { + std::cout << GetName () << "min remove time: t" + << j << ": " << rmn[j] << std::endl; + } + } //----------------------------------------------------------------------------- From 6a0691f12142783e2ebd19c07aed789b9050a483 Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Wed, 14 Nov 2012 00:03:41 -0800 Subject: [PATCH 6/8] Undo whitespace-only changes, to ease review --- src/network/model/byte-tag-list.h | 9 +- src/network/model/packet-tag-list.h | 1 - src/network/model/packet.cc | 29 ++- src/network/model/packet.h | 16 +- src/network/test/packet-test-suite.cc | 280 +++++++++++++++----------- 5 files changed, 182 insertions(+), 153 deletions(-) diff --git a/src/network/model/byte-tag-list.h b/src/network/model/byte-tag-list.h index 8d20e2117..96d28228f 100644 --- a/src/network/model/byte-tag-list.h +++ b/src/network/model/byte-tag-list.h @@ -68,9 +68,10 @@ struct ByteTagListData; class ByteTagList { public: + class Iterator { - public: +public: struct Item { TypeId tid; @@ -79,14 +80,14 @@ public: int32_t end; TagBuffer buf; Item (TagBuffer buf); - private: +private: friend class ByteTagList; friend class ByteTagList::Iterator; }; bool HasNext (void) const; struct ByteTagList::Iterator::Item Next (void); uint32_t GetOffsetStart (void) const; - private: +private: friend class ByteTagList; Iterator (uint8_t *start, uint8_t *end, int32_t offsetStart, int32_t offsetEnd); void PrepareForNext (void); @@ -157,7 +158,7 @@ private: bool IsDirtyAtStart (int32_t prependOffset); ByteTagList::Iterator BeginAll (void) const; - struct ByteTagListData * Allocate (uint32_t size); + struct ByteTagListData *Allocate (uint32_t size); void Deallocate (struct ByteTagListData *data); uint16_t m_used; diff --git a/src/network/model/packet-tag-list.h b/src/network/model/packet-tag-list.h index 60878a3e0..6402b3cd8 100644 --- a/src/network/model/packet-tag-list.h +++ b/src/network/model/packet-tag-list.h @@ -33,7 +33,6 @@ namespace ns3 { class Tag; - /** * \ingroup packet * diff --git a/src/network/model/packet.cc b/src/network/model/packet.cc index 9fea897ae..a325b12c0 100644 --- a/src/network/model/packet.cc +++ b/src/network/model/packet.cc @@ -71,8 +71,8 @@ ByteTagIterator::Next (void) { ByteTagList::Iterator::Item i = m_current.Next (); return ByteTagIterator::Item (i.tid, - i.start - m_current.GetOffsetStart (), - i.end - m_current.GetOffsetStart (), + i.start-m_current.GetOffsetStart (), + i.end-m_current.GetOffsetStart (), i.buf); } ByteTagIterator::ByteTagIterator (ByteTagList::Iterator i) @@ -373,7 +373,7 @@ Packet::PeekData (void) const uint32_t newStart = m_buffer.GetCurrentStartOffset (); // Update tag offsets if buffer offsets were changed - const_cast (m_byteTagList).AddAtStart (newStart - oldStart, newStart); + const_cast(m_byteTagList).AddAtStart (newStart - oldStart, newStart); return data; } @@ -434,8 +434,7 @@ Packet::Print (std::ostream &os) const PacketMetadata::Item item = i.Next (); if (item.isFragment) { - switch (item.type) - { + switch (item.type) { case PacketMetadata::Item::PAYLOAD: os << "Payload"; break; @@ -444,13 +443,12 @@ Packet::Print (std::ostream &os) const os << item.tid.GetName (); break; } - os << " Fragment [" << item.currentTrimedFromStart << ":" + os << " Fragment [" << item.currentTrimedFromStart<<":" << (item.currentTrimedFromStart + item.currentSize) << "]"; } else { - switch (item.type) - { + switch (item.type) { case PacketMetadata::Item::PAYLOAD: os << "Payload (size=" << item.currentSize << ")"; break; @@ -489,8 +487,7 @@ Packet::Print (std::ostream &os) const PacketMetadata::Item item = i.Next (); if (item.isFragment) { - switch (item.type) - { + switch (item.type) { case PacketMetadata::Item::PAYLOAD: os << "Payload"; break; @@ -499,13 +496,12 @@ Packet::Print (std::ostream &os) const os << item.tid.GetName (); break; } - os << " Fragment [" << item.currentTrimedFromStart << ":" + os << " Fragment [" << item.currentTrimedFromStart<<":" << (item.currentTrimedFromStart + item.currentSize) << "]"; } else { - switch (item.type) - { + switch (item.type) { case PacketMetadata::Item::PAYLOAD: os << "Payload (size=" << item.currentSize << ")"; break; @@ -633,7 +629,7 @@ Packet::Serialize (uint8_t* buffer, uint32_t maxSize) const { // increment p by nixSize bytes // ensuring 4-byte boundary - p += ((nixSize + 3) & (~3)) / 4; + p += ((nixSize+3) & (~3)) / 4; } else { @@ -681,7 +677,7 @@ Packet::Serialize (uint8_t* buffer, uint32_t maxSize) const { // increment p by metaSize bytes // ensuring 4-byte boundary - p += ((metaSize + 3) & (~3)) / 4; + p += ((metaSize+3) & (~3)) / 4; } else { @@ -710,7 +706,7 @@ Packet::Serialize (uint8_t* buffer, uint32_t maxSize) const { // increment p by bufSize bytes // ensuring 4-byte boundary - p += ((bufSize + 3) & (~3)) / 4; + p += ((bufSize+3) & (~3)) / 4; } else { @@ -855,7 +851,6 @@ Packet::RemovePacketTag (Tag &tag) bool found = m_packetTagList.Remove (tag); return found; } - bool Packet::ReplacePacketTag (Tag &tag) { diff --git a/src/network/model/packet.h b/src/network/model/packet.h index c04a2983e..954220de7 100644 --- a/src/network/model/packet.h +++ b/src/network/model/packet.h @@ -189,11 +189,6 @@ private: * qos class id set by an application and processed by a lower-level MAC * layer. * - * - Packet tags must have unique types; repeated tags of a single type - * (even with different values) can't be attached to a single packet. - * Packet tags must serialize to a finite maximum size, see - * PacketTagList::TagData. - * * Implementing a new type of Header or Trailer for a new protocol is * pretty easy and is a matter of creating a subclass of the ns3::Header * or of the ns3::Trailer base class, and implementing the methods @@ -208,6 +203,7 @@ private: class Packet : public SimpleRefCount { public: + /** * Create an empty packet with a new uid (as returned * by getUid). @@ -324,7 +320,7 @@ public: void AddPaddingAtEnd (uint32_t size); /** * Remove size bytes from the end of the current packet - * It is safe to remove more bytes than arepresent in + * It is safe to remove more bytes than are present in * the packet. * * \param size number of bytes from remove @@ -350,7 +346,7 @@ public: * an explicit copy. * */ - uint8_t const * PeekData (void) const NS_DEPRECATED; + uint8_t const *PeekData (void) const NS_DEPRECATED; /** * Copy the packet contents to a byte buffer. @@ -360,9 +356,7 @@ public: * \param size the size of the byte buffer. * \returns the number of bytes read from the packet * - * No more than \b size bytes will be copied by this function, - * even if the packet has more content. Use GetSize"()" to - * to find the total size of the buffer needed. + * No more than \b size bytes will be copied by this function. */ uint32_t CopyData (uint8_t *buffer, uint32_t size) const; @@ -379,7 +373,7 @@ public: /** * \returns a COW copy of the packet. * - * The returned packet will behave like an independent copy of + * The returns packet will behave like an independent copy of * the original packet, even though they both share the * same datasets internally. */ diff --git a/src/network/test/packet-test-suite.cc b/src/network/test/packet-test-suite.cc index 49acffc44..38aaf23c6 100644 --- a/src/network/test/packet-test-suite.cc +++ b/src/network/test/packet-test-suite.cc @@ -24,6 +24,7 @@ #include #include #include +#include #include using namespace ns3; @@ -457,12 +458,17 @@ public: virtual ~PacketTagListTest (); private: void DoRun (void); - void CheckRef (PacketTagList & ref, ATestTagBase & t, - const char * msg, bool miss = false); - void CheckRefList (PacketTagList & ref, const char * msg, int miss = 0); - int RemoveTime (const PacketTagList & ref, ATestTagBase & t, - const char * msg); - int AddRemoveTime (); + void CheckRef (const PacketTagList & ref, + ATestTagBase & t, + const char * msg, + bool miss = false); + void CheckRefList (const PacketTagList & ref, + const char * msg, + int miss = 0); + int RemoveTime (const PacketTagList & ref, + ATestTagBase & t, + const char * msg = 0); + int AddRemoveTime (const bool verbose = false); }; PacketTagListTest::PacketTagListTest () @@ -475,8 +481,10 @@ PacketTagListTest::~PacketTagListTest () } void -PacketTagListTest::CheckRef (PacketTagList & ref, ATestTagBase & t, - const char * msg, bool miss) +PacketTagListTest::CheckRef (const PacketTagList & ref, + ATestTagBase & t, + const char * msg, + bool miss) { int expect = t.GetData (); // the value we should find bool found = ref.Peek (t); // rewrites t with actual value @@ -491,7 +499,7 @@ PacketTagListTest::CheckRef (PacketTagList & ref, ATestTagBase & t, } // A set of tags with data value 1, to check COW -#define MAKE_REF_TAGS \ +#define MAKE_TEST_TAGS \ ATestTag<1> t1 (1); \ ATestTag<2> t2 (1); \ ATestTag<3> t3 (1); \ @@ -501,23 +509,24 @@ PacketTagListTest::CheckRef (PacketTagList & ref, ATestTagBase & t, ATestTag<7> t7 (1) void -PacketTagListTest::CheckRefList (PacketTagList & ref, +PacketTagListTest::CheckRefList (const PacketTagList & ptl, const char * msg, int miss /* = 0 */) { - MAKE_REF_TAGS ; - CheckRef (ref, t1, msg, miss == 1); - CheckRef (ref, t2, msg, miss == 2); - CheckRef (ref, t3, msg, miss == 3); - CheckRef (ref, t4, msg, miss == 4); - CheckRef (ref, t5, msg, miss == 5); - CheckRef (ref, t6, msg, miss == 6); - CheckRef (ref, t7, msg, miss == 7); + MAKE_TEST_TAGS ; + CheckRef (ptl, t1, msg, miss == 1); + CheckRef (ptl, t2, msg, miss == 2); + CheckRef (ptl, t3, msg, miss == 3); + CheckRef (ptl, t4, msg, miss == 4); + CheckRef (ptl, t5, msg, miss == 5); + CheckRef (ptl, t6, msg, miss == 6); + CheckRef (ptl, t7, msg, miss == 7); } int -PacketTagListTest::RemoveTime (const PacketTagList & ref, ATestTagBase & t, - const char * msg) +PacketTagListTest::RemoveTime (const PacketTagList & ref, + ATestTagBase & t, + const char * msg /* = 0 */) { const int reps = 10000; std::vector< PacketTagList > ptv(reps, ref); @@ -527,37 +536,21 @@ PacketTagListTest::RemoveTime (const PacketTagList & ref, ATestTagBase & t, } int stop = clock (); int delta = stop - start; - std::cout << GetName () << "remove time: " << msg << ": " - << delta << " ticks" + if (msg) { + std::cout << GetName () << "remove time: " << msg << ": " << std::setw (8) + << delta << " ticks to remove " + << reps << " times" << std::endl; + } return delta; } int -PacketTagListTest::AddRemoveTime () +PacketTagListTest::AddRemoveTime (const bool verbose /* = false */) { - /* old free list priming - const int nfree = 1000 - 10; - - // get some stuff on the free list - std::cout << GetName () << "free list: initial: " - << PacketTagList::GetNFree (); - std::vector< PacketTagList > ptv; - for (int i = 0; i < nfree; ++i) { - PacketTagList p; - p.Add (ATestTag <1> (i)); - ptv.push_back (p); - } - ptv.clear (); - std::cout << ", now: " - << PacketTagList::GetNFree () - << std::endl; - */ - - // timing const int reps = 100000; PacketTagList ptl; - ATestTag <2> t; + ATestTag <2> t(2); int start = clock (); for (int i = 0; i < reps; ++i) { ptl.Add (t); @@ -565,9 +558,12 @@ PacketTagListTest::AddRemoveTime () } int stop = clock (); int delta = stop - start; - std::cout << GetName () << "freelist time: " - << delta << " ticks" + if (verbose) { + std::cout << GetName () << "add/remove time: " << std::setw (8) + << delta << " ticks to add+remove " + << reps << " times" << std::endl; + } return delta; } @@ -576,7 +572,7 @@ PacketTagListTest::DoRun (void) { std::cout << GetName () << "begin" << std::endl; - MAKE_REF_TAGS ; + MAKE_TEST_TAGS ; PacketTagList ref; // empty list ref.Add (t1); // last @@ -587,85 +583,129 @@ PacketTagListTest::DoRun (void) ref.Add (t6); // pre-merge ref.Add (t7); // first - std::cout << GetName () << "missing tag" << std::endl;; - ATestTag<10> t10; - NS_TEST_EXPECT_MSG_EQ (ref.Peek (t10), false, "missing tag"); - - std::cout << GetName () << "copy and assignment" << std::endl; - { PacketTagList ptl (ref); - CheckRefList (ref, "copy ctor orig"); - CheckRefList (ptl, "copy ctor copy"); + { // Peek + std::cout << GetName () << "check Peek (missing tag) returns false" + << std::endl;; + ATestTag<10> t10; + NS_TEST_EXPECT_MSG_EQ (ref.Peek (t10), false, "missing tag"); } - { PacketTagList ptl = ref; - CheckRefList (ref, "assignment orig"); - CheckRefList (ptl, "assignment copy"); + + { // Copy ctor, assignment + std::cout << GetName () << "check copy and assignment" << std::endl; + { PacketTagList ptl (ref); + CheckRefList (ref, "copy ctor orig"); + CheckRefList (ptl, "copy ctor copy"); + } + { PacketTagList ptl = ref; + CheckRefList (ref, "assignment orig"); + CheckRefList (ptl, "assignment copy"); + } } - std::cout << GetName () << "remove" << std::endl; -#define RemoveCheck(n) \ - { PacketTagList p ## n = ref; \ - p ## n .Remove ( t ## n ); \ - CheckRefList (ref, "remove " #n " orig"); \ - CheckRefList (p ## n, "remove " #n " copy", n); \ - } - - RemoveCheck (1); - RemoveCheck (2); - RemoveCheck (3); - RemoveCheck (4); - RemoveCheck (5); - RemoveCheck (6); - RemoveCheck (7); - - std::cout << GetName () << "replace" << std::endl; -#define ReplaceCheck(n) \ - t ## n .m_data = 2; \ - { PacketTagList p ## n = ref; \ - p ## n .Replace ( t ## n ); \ - CheckRefList (ref, "replace " #n " orig"); \ - CheckRef (p ## n, t ## n, "replace " #n " copy"); \ - } - - ReplaceCheck (1); - ReplaceCheck (2); - ReplaceCheck (3); - ReplaceCheck (4); - ReplaceCheck (5); - ReplaceCheck (6); - ReplaceCheck (7); - - std::cout << GetName () << "freelist timing" << std::endl; - int flm = std::numeric_limits::max (); - for (int i = 0; i < 100; ++i) { - int now = AddRemoveTime (); - if (now < flm) flm = now; - } - std::cout << GetName () << "min freelist time: " - << flm << " ticks" << std::endl; - - std::cout << GetName () << "remove timing" << std::endl; - std::vector rmn (7, std::numeric_limits::max ()); - for (int i = 0; i < 100; ++i) { - for (int j = 1; j < 8; ++j) { - int now = 0; - switch (j) { - case 7: now = RemoveTime (ref, t7, "t7"); break; - case 6: now = RemoveTime (ref, t6, "t6"); break; - case 5: now = RemoveTime (ref, t5, "t5"); break; - case 4: now = RemoveTime (ref, t4, "t4"); break; - case 3: now = RemoveTime (ref, t3, "t3"); break; - case 2: now = RemoveTime (ref, t2, "t2"); break; - case 1: now = RemoveTime (ref, t1, "t1"); break; - } // switch - - if (now < rmn[j]) rmn[j] = now; - } // for tag j - } // for iteration i - for (int j = 7; j > 0; --j) { - std::cout << GetName () << "min remove time: t" - << j << ": " << rmn[j] << std::endl; + { // Removal +# define RemoveCheck(n) \ + { PacketTagList p ## n = ref; \ + p ## n .Remove ( t ## n ); \ + CheckRefList (ref, "remove " #n " orig"); \ + CheckRefList (p ## n, "remove " #n " copy", n); \ + } + + { // Remove single tags from list + std::cout << GetName () << "check removal of each tag" << std::endl; + RemoveCheck (1); + RemoveCheck (2); + RemoveCheck (3); + RemoveCheck (4); + RemoveCheck (5); + RemoveCheck (6); + RemoveCheck (7); + } + + { // Remove in the presence of a merge + std::cout << GetName () << "check removal doesn't disturb merge " + << std::endl; + PacketTagList ptl = ref; + ptl.Remove (t7); + ptl.Remove (t6); + ptl.Remove (t5); + + PacketTagList mrg = ptl; // merged list + ATestTag<8> m5 (1); + mrg.Add (m5); // ptl and mrg differ + ptl.Add (t5); + ptl.Add (t6); + ptl.Add (t7); + + CheckRefList (ref, "post merge, orig"); + CheckRefList (ptl, "post merge, long chain"); + const char * msg = "post merge, short chain"; + CheckRef (mrg, t1, msg, false); + CheckRef (mrg, t2, msg, false); + CheckRef (mrg, t3, msg, false); + CheckRef (mrg, t4, msg, false); + CheckRef (mrg, m5, msg, false); + } +# undef RemoveCheck + } // Removal + + { // Replace + + std::cout << GetName () << "check replacing each tag" << std::endl; + +# define ReplaceCheck(n) \ + t ## n .m_data = 2; \ + { PacketTagList p ## n = ref; \ + p ## n .Replace ( t ## n ); \ + CheckRefList (ref, "replace " #n " orig"); \ + CheckRef (p ## n, t ## n, "replace " #n " copy"); \ + } + + ReplaceCheck (1); + ReplaceCheck (2); + ReplaceCheck (3); + ReplaceCheck (4); + ReplaceCheck (5); + ReplaceCheck (6); + ReplaceCheck (7); } + { // Timing + std::cout << GetName () << "add+remove timing" << std::endl; + int flm = std::numeric_limits::max (); + for (int i = 0; i < 100; ++i) { + int now = AddRemoveTime (); + if (now < flm) flm = now; + } + std::cout << GetName () << "min add+remove time: " + << std::setw (8) << flm << " ticks" + << std::endl; + + std::cout << GetName () << "remove timing" << std::endl; + std::vector rmn (7, std::numeric_limits::max ()); + for (int i = 0; i < 100; ++i) { + for (int j = 1; j < 8; ++j) { + int now = 0; + switch (j) { + case 7: now = RemoveTime (ref, t7); break; + case 6: now = RemoveTime (ref, t6); break; + case 5: now = RemoveTime (ref, t5); break; + case 4: now = RemoveTime (ref, t4); break; + case 3: now = RemoveTime (ref, t3); break; + case 2: now = RemoveTime (ref, t2); break; + case 1: now = RemoveTime (ref, t1); break; + } // switch + + if (now < rmn[j]) rmn[j] = now; + } // for tag j + } // for iteration i + for (int j = 7; j > 0; --j) { + std::cout << GetName () << "min remove time: t" + << j << ": " + << std::setw (8) << rmn[j] << " ticks" + << std::endl; + } + } // Timing + } //----------------------------------------------------------------------------- From a1a1c6d36d1f6fe5b879921aa95eecc0b2144310 Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Wed, 14 Nov 2012 00:07:50 -0800 Subject: [PATCH 7/8] Delete _fp suffix, per code review --- src/network/model/packet-tag-list.cc | 2 +- src/network/model/packet-tag-list.h | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/network/model/packet-tag-list.cc b/src/network/model/packet-tag-list.cc index 229d02182..aff200f63 100644 --- a/src/network/model/packet-tag-list.cc +++ b/src/network/model/packet-tag-list.cc @@ -35,7 +35,7 @@ NS_LOG_COMPONENT_DEFINE ("PacketTagList"); namespace ns3 { bool -PacketTagList::COWTraverse (Tag & tag, PacketTagList::COWWriter_fp Writer) +PacketTagList::COWTraverse (Tag & tag, PacketTagList::COWWriter Writer) { TypeId tid = tag.GetInstanceTypeId (); NS_LOG_FUNCTION (this << tid); diff --git a/src/network/model/packet-tag-list.h b/src/network/model/packet-tag-list.h index 6402b3cd8..11f742523 100644 --- a/src/network/model/packet-tag-list.h +++ b/src/network/model/packet-tag-list.h @@ -249,7 +249,7 @@ private: * pointing to \pname{cur}. * \returns True if operation successful, false otherwise */ - typedef bool (PacketTagList::*COWWriter_fp) + typedef bool (PacketTagList::*COWWriter) (Tag & tag, bool preMerge, struct TagData * cur, struct TagData ** prevNext); /** @@ -259,7 +259,7 @@ private: * \param [in] Writer The copy-on-write function to use. * \returns True if \pname{tag} found, false otherwise. */ - bool COWTraverse (Tag & tag, PacketTagList::COWWriter_fp Writer); + bool COWTraverse (Tag & tag, PacketTagList::COWWriter Writer); /** * Copy-on-write implementing Remove. * From 4fc79291b9b68d0f6a99dc8223a5d2ffcffac817 Mon Sep 17 00:00:00 2001 From: "Peter D. Barnes, Jr." Date: Wed, 22 May 2013 12:57:58 -0700 Subject: [PATCH 8/8] Use PacketTagList::TagData::MAX_SIZE in DeviceNameTag --- src/network/utils/packet-socket.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/network/utils/packet-socket.cc b/src/network/utils/packet-socket.cc index 881bca33c..b8d81edc8 100644 --- a/src/network/utils/packet-socket.cc +++ b/src/network/utils/packet-socket.cc @@ -617,19 +617,20 @@ DeviceNameTag::GetInstanceTypeId (void) const uint32_t DeviceNameTag::GetSerializedSize (void) const { - uint32_t s = 1 + m_deviceName.size(); - return ( s >= PACKET_TAG_MAX_SIZE)?PACKET_TAG_MAX_SIZE:s; + 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 DeviceNameTag::Serialize (TagBuffer i) const { const char *n = m_deviceName.c_str(); - uint8_t l = (uint8_t) strlen (n); + uint8_t l = (uint8_t) m_deviceName.size (); - if ( ( 1 + l ) > PACKET_TAG_MAX_SIZE ) l = PACKET_TAG_MAX_SIZE - 1; + l = std::min ((uint32_t)l, (uint32_t)PacketTagList::TagData::MAX_SIZE - 1); i.WriteU8 (l); - i.Write ( (uint8_t*) n , (uint32_t) l ); + i.Write ( (uint8_t*) n , (uint32_t) l); } void DeviceNameTag::Deserialize (TagBuffer i)