# HG changeset patch # User Peter D. Barnes, Jr. # Date 1369260244 25200 # Node ID ddea5860c58c4ab717b79cd9646c7c2b56750ff0 # Parent 070c5caed3915165b38b4fac3b2a6cb119dffec7# Parent d9bc7131bb68013e63ed9037ca17abe4496f0168 Merge packet-tag-list refactoring diff -r 070c5caed391 -r ddea5860c58c doc/doxygen.conf --- a/doc/doxygen.conf Mon May 20 22:57:37 2013 +0200 +++ b/doc/doxygen.conf Wed May 22 15:04:04 2013 -0700 @@ -193,9 +193,14 @@ # 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 -r 070c5caed391 -r ddea5860c58c src/lte/model/lte-radio-bearer-tag.cc diff -r 070c5caed391 -r ddea5860c58c src/lte/model/lte-radio-bearer-tag.h diff -r 070c5caed391 -r ddea5860c58c src/lte/model/lte-rlc-um.cc diff -r 070c5caed391 -r ddea5860c58c src/lte/model/lte-rlc-um.h diff -r 070c5caed391 -r ddea5860c58c src/network/model/byte-tag-list.h --- a/src/network/model/byte-tag-list.h Mon May 20 22:57:37 2013 +0200 +++ b/src/network/model/byte-tag-list.h Wed May 22 15:04:04 2013 -0700 @@ -31,7 +31,7 @@ /** * \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 @@ * 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 @@ * 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 diff -r 070c5caed391 -r ddea5860c58c src/network/model/packet-tag-list.cc --- a/src/network/model/packet-tag-list.cc Mon May 20 22:57:37 2013 +0200 +++ b/src/network/model/packet-tag-list.cc Wed May 22 15:04:04 2013 -0700 @@ -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,123 +34,228 @@ namespace ns3 { -#ifdef 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_NOARGS (); - struct PacketTagList::TagData *retval; - if (g_free != 0) - { - retval = g_free; - g_free = g_free->m_next; - g_nfree--; - } - else - { - retval = new struct PacketTagList::TagData (); - } - return retval; -} - -void -PacketTagList::FreeData (struct TagData *data) const +bool +PacketTagList::COWTraverse (Tag & tag, PacketTagList::COWWriter Writer) { - NS_LOG_FUNCTION (data); - if (g_nfree > 1000) - { - delete data; - return; - } - g_nfree++; - data->next = g_free; - data->tid = TypeId (); - g_free = data; -} -#else -struct PacketTagList::TagData * -PacketTagList::AllocData (void) const -{ - NS_LOG_FUNCTION (this); - struct PacketTagList::TagData *retval; - retval = new struct PacketTagList::TagData (); - return retval; -} + TypeId tid = tag.GetInstanceTypeId (); + NS_LOG_FUNCTION (this << tid); + NS_LOG_INFO ("looking for " << tid); -void -PacketTagList::FreeData (struct TagData *data) const -{ - NS_LOG_FUNCTION (this << data); - delete data; -} -#endif - -bool -PacketTagList::Remove (Tag &tag) -{ - 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) + // 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) + { + // found merge + NS_LOG_INFO ("found initial merge before tid"); + break; + } + else if (cur->tid == tid) + { + NS_LOG_INFO ("found tid before initial merge, calling writer"); + found = (this->*Writer)(tag, true, cur, prevNext); + break; + } + else { - /** - * 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; + // 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; } - struct TagData *copy = AllocData (); + } + 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 = new struct TagData (); 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; + } + // 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 + delete cur; } - *prevNext = 0; - RemoveAll (); - m_next = start; - return true; + 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 = new struct TagData (); + 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 = new struct TagData (); 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 +263,14 @@ 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 +284,5 @@ return m_next; } -} // namespace ns3 +} /* namespace ns3 */ diff -r 070c5caed391 -r ddea5860c58c src/network/model/packet-tag-list.h --- a/src/network/model/packet-tag-list.h Mon May 20 22:57:37 2013 +0200 +++ b/src/network/model/packet-tag-list.h Wed May 22 15:04:04 2013 -0700 @@ -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" @@ -29,44 +34,261 @@ class Tag; /** - * \ingroup constants - * \brief Tag maximum size - * The maximum size (in bytes) of a Tag is stored - * in this constant. + * \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); 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, + * 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 + * Packet tags must serialize to a finite maximum size, see TagData + * + * This documentation entitles the original author to a free beer. */ -#define PACKET_TAG_MAX_SIZE 20 - 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 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 #struct 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 #struct 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: + /** + * 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) + (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 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); - bool Remove (TypeId tid); - struct PacketTagList::TagData *AllocData (void) const; - void FreeData (struct TagData *data) const; - - static struct PacketTagList::TagData *g_free; - static uint32_t g_nfree; - + /** + * Pointer to first #struct TagData on the list + */ struct TagData *m_next; }; @@ -118,7 +340,7 @@ 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) @@ -127,13 +349,13 @@ } if (prev != 0) { - FreeData (prev); + delete prev; } prev = cur; } if (prev != 0) { - FreeData (prev); + delete prev; } m_next = 0; } diff -r 070c5caed391 -r ddea5860c58c src/network/model/packet.cc --- a/src/network/model/packet.cc Mon May 20 22:57:37 2013 +0200 +++ b/src/network/model/packet.cc Wed May 22 15:04:04 2013 -0700 @@ -38,19 +38,16 @@ 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 @@ m_end (end), m_buffer (buffer) { - NS_LOG_FUNCTION (this << tid << start << end << &buffer); } bool ByteTagIterator::HasNext (void) const @@ -73,7 +69,6 @@ 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 (), @@ -83,25 +78,21 @@ 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::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 @@ 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 @@ // 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 @@ m_metadata (static_cast (Simulator::GetSystemId ()) << 32 | m_globalUid, 0), m_nixVector (0) { - NS_LOG_FUNCTION (this); m_globalUid++; } @@ -193,7 +182,6 @@ 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 @@ m_metadata (0,0), m_nixVector (0) { - NS_LOG_FUNCTION (this << &buffer << size << magic); NS_ASSERT (magic); Deserialize (buffer, size); } @@ -221,7 +208,6 @@ 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 @@ m_metadata (metadata), m_nixVector (0) { - NS_LOG_FUNCTION (this << &buffer << &byteTagList << &packetTagList << &metadata); } Ptr @@ -255,14 +240,12 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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); @@ -397,28 +380,24 @@ 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,7 +428,6 @@ void Packet::Print (std::ostream &os) const { - NS_LOG_FUNCTION (this << &os); PacketMetadata::ItemIterator i = m_metadata.BeginItem (m_buffer); while (i.HasNext ()) { @@ -567,7 +545,6 @@ PacketMetadata::ItemIterator Packet::BeginItem (void) const { - NS_LOG_FUNCTION (this); return m_metadata.BeginItem (m_buffer); } @@ -587,7 +564,6 @@ uint32_t Packet::GetSerializedSize (void) const { - NS_LOG_FUNCTION (this); uint32_t size = 0; if (m_nixVector) @@ -631,7 +607,6 @@ 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; @@ -750,7 +725,7 @@ 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 +807,7 @@ 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 +817,12 @@ 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 +840,28 @@ 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 +875,6 @@ void Packet::PrintPacketTags (std::ostream &os) const { - NS_LOG_FUNCTION (this << &os); PacketTagIterator i = GetPacketTagIterator (); while (i.HasNext ()) { @@ -918,7 +898,6 @@ PacketTagIterator Packet::GetPacketTagIterator (void) const { - NS_LOG_FUNCTION (this); return PacketTagIterator (m_packetTagList.Head ()); } diff -r 070c5caed391 -r ddea5860c58c src/network/model/packet.h --- a/src/network/model/packet.h Mon May 20 22:57:37 2013 +0200 +++ b/src/network/model/packet.h Wed May 22 15:04:04 2013 -0700 @@ -43,7 +43,7 @@ /** * \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 @@ { 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 @@ */ 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 @@ /** * \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 @@ { public: /** - * Identifies a tag within a packet. + * Identifies a packet tag within a packet. */ class Item { @@ -124,12 +124,12 @@ */ 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: @@ -197,8 +197,8 @@ * 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 { @@ -320,7 +320,7 @@ 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 are present in * the packet. * * \param size number of bytes from remove @@ -328,7 +328,7 @@ 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,18 +336,21 @@ 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; /** + * 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. @@ -358,6 +361,8 @@ 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 @@ -432,30 +437,29 @@ 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 +478,7 @@ */ 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 +487,54 @@ 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 +543,9 @@ 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 +558,22 @@ */ 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 +613,7 @@ * - ns3::Packet::AddTrailer * - both versions of ns3::Packet::AddAtEnd * - ns3::Packet::RemovePacketTag + * - ns3::Packet::ReplacePacketTag * * Non-dirty operations: * - ns3::Packet::AddPacketTag @@ -614,6 +638,10 @@ } // namespace ns3 +/**************************************************** + * Implementation of inline methods for performance + ****************************************************/ + namespace ns3 { uint32_t diff -r 070c5caed391 -r ddea5860c58c src/network/model/tag.h --- a/src/network/model/tag.h Mon May 20 22:57:37 2013 +0200 +++ b/src/network/model/tag.h Wed May 22 15:04:04 2013 -0700 @@ -31,7 +31,7 @@ * * \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 -r 070c5caed391 -r ddea5860c58c src/network/test/packet-test-suite.cc --- a/src/network/test/packet-test-suite.cc Mon May 20 22:57:37 2013 +0200 +++ b/src/network/test/packet-test-suite.cc Wed May 22 15:04:04 2013 -0700 @@ -18,9 +18,14 @@ * Author: Mathieu Lacage */ #include "ns3/packet.h" +#include "ns3/packet-tag-list.h" #include "ns3/test.h" +#include // std:numeric_limits #include #include +#include +#include +#include using namespace ns3; @@ -32,8 +37,14 @@ 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 +65,17 @@ 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 +86,12 @@ } } 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 +450,264 @@ #endif } } +//-------------------------------------- +class PacketTagListTest : public TestCase +{ +public: + PacketTagListTest (); + virtual ~PacketTagListTest (); +private: + void DoRun (void); + 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 () + : TestCase ("PacketTagListTest: ") +{ +} + +PacketTagListTest::~PacketTagListTest () +{ +} + +void +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 + 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_TEST_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 (const PacketTagList & ptl, + const char * msg, + int miss /* = 0 */) +{ + 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 /* = 0 */) +{ + 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 (); + int delta = stop - start; + if (msg) { + std::cout << GetName () << "remove time: " << msg << ": " << std::setw (8) + << delta << " ticks to remove " + << reps << " times" + << std::endl; + } + return delta; +} + +int +PacketTagListTest::AddRemoveTime (const bool verbose /* = false */) +{ + const int reps = 100000; + PacketTagList ptl; + ATestTag <2> t(2); + int start = clock (); + for (int i = 0; i < reps; ++i) { + ptl.Add (t); + ptl.Remove (t); + } + int stop = clock (); + int delta = stop - start; + if (verbose) { + std::cout << GetName () << "add/remove time: " << std::setw (8) + << delta << " ticks to add+remove " + << reps << " times" + << std::endl; + } + return delta; +} + +void +PacketTagListTest::DoRun (void) +{ + std::cout << GetName () << "begin" << std::endl; + + MAKE_TEST_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 + + { // 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"); + } + + { // 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"); + } + } + + { // 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 + +} + //----------------------------------------------------------------------------- class PacketTestSuite : public TestSuite { @@ -445,7 +718,8 @@ PacketTestSuite::PacketTestSuite () : TestSuite ("packet", UNIT) { - AddTestCase (new PacketTest, TestCase::QUICK); + AddTestCase (new PacketTest); + AddTestCase (new PacketTagListTest); } static PacketTestSuite g_packetTestSuite; diff -r 070c5caed391 -r ddea5860c58c src/network/utils/packet-socket.cc --- a/src/network/utils/packet-socket.cc Mon May 20 22:57:37 2013 +0200 +++ b/src/network/utils/packet-socket.cc Wed May 22 15:04:04 2013 -0700 @@ -617,19 +617,20 @@ 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) diff -r 070c5caed391 -r ddea5860c58c utils/bench-packets.cc --- a/utils/bench-packets.cc Mon May 20 22:57:37 2013 +0200 +++ b/utils/bench-packets.cc Wed May 22 15:04:04 2013 -0700 @@ -252,7 +252,10 @@ 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 @@ 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; }