Trim excess copies from PacketTagList::Remove, add PacketTagList::Replace, documentation. ptl
authorPeter D. Barnes, Jr. <barnes26@llnl.gov>
Tue, 13 Nov 2012 23:44:58 -0800
branchptl
changeset 9763 038d5627c5a9
parent 9762 070c5caed391
child 9764 e5258de9a86c
Trim excess copies from PacketTagList::Remove, add PacketTagList::Replace, documentation.
doc/doxygen.conf
src/lte/model/lte-radio-bearer-tag.cc
src/lte/model/lte-radio-bearer-tag.h
src/lte/model/lte-rlc-um.cc
src/lte/model/lte-rlc-um.h
src/network/model/byte-tag-list.h
src/network/model/packet-tag-list.cc
src/network/model/packet-tag-list.h
src/network/model/packet.cc
src/network/model/packet.h
src/network/model/tag.h
src/network/test/packet-test-suite.cc
--- a/doc/doxygen.conf	Mon May 20 22:57:37 2013 +0200
+++ b/doc/doxygen.conf	Tue Nov 13 23:44:58 2012 -0800
@@ -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}=<span class=\"params\"><span class=\"paramname\">\1</span></span>"
+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}="<span class=\"params\"><span class=\"paramname\">\1</span></span>"
 
 # 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
--- a/src/network/model/byte-tag-list.h	Mon May 20 22:57:37 2013 +0200
+++ b/src/network/model/byte-tag-list.h	Tue Nov 13 23:44:58 2012 -0800
@@ -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 
@@ -68,10 +68,9 @@
 class ByteTagList
 {
 public:
-
   class Iterator
   {
-public:
+  public:
     struct Item 
     {
       TypeId tid;
@@ -80,14 +79,14 @@
       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 @@
   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;
--- a/src/network/model/packet-tag-list.cc	Mon May 20 22:57:37 2013 +0200
+++ b/src/network/model/packet-tag-list.cc	Tue Nov 13 23:44:58 2012 -0800
@@ -17,6 +17,12 @@
  *
  * Author: Mathieu Lacage <[email protected]>
  */
+
+/**
+\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 @@
 
 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 @@
 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;
+  g_free = data;
+  memset (data->data, 0, TagData::MAX_SIZE);
   data->tid = TypeId ();
-  g_free = data;
+  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)
+        {
+          // 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 = AllocData ();
       copy->tid = cur->tid;
       copy->count = 1;
-      copy->next = 0;
-      std::memcpy (copy->data, cur->data, PACKET_TAG_MAX_SIZE);
-      *prevNext = copy;
-      prevNext = &copy->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 = &copy->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
+      FreeData (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 = 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<PacketTagList *> (this)->m_next = head;
 }
@@ -152,14 +323,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 +344,5 @@
   return m_next;
 }
 
-} // namespace ns3
+} /* namespace ns3 */
 
--- a/src/network/model/packet-tag-list.h	Mon May 20 22:57:37 2013 +0200
+++ b/src/network/model/packet-tag-list.h	Tue Nov 13 23:44:58 2012 -0800
@@ -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 <stdint.h>
 #include <ostream>
 #include "ns3/type-id.h"
@@ -28,46 +33,285 @@
 
 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="<l> Other branch | <n> next | <c> ..." ];
+ *        PTL1 [ label="<l> PacketTagList A | <n> m_next" , shape=Mrecord];
+ *        PTL2 [ label="<l> PacketTagList B | <n> m_next" , shape=Mrecord];
+ *        oth:n  -> T7:l ;
+ *        PTL2:n -> T6:l ;
+ *        PTL1:n -> T5:l ;
+ *        T1   [ label="<l> T1  | <n> next | <c> count = 1" ];
+ *        T2   [ label="<l> T2  | <n> next | <c> count = 1" ];
+ *        T3   [ label="<l> T3  | <n> next | <c> count = 2" ];
+ *        T4   [ label="<l> T4  | <n> next | <c> count = 1" ];
+ *        T5   [ label="<l> T5  | <n> next | <c> count = 2" ];
+ *        T6   [ label="<l> T6  | <n> next | <c> count = 1" ];
+ *        T7   [ label="<l> T7  | <n> next | <c> 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. (<tt>T1-T7</tt> 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
+ *     <tt>count \> 1</tt> (\c T3, \c T5); succesive links along
+ *     a branch have <tt>count = 1</tt> (\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. (<tt>T5-T7</tt>)
+ *
+ *   - Conceptually, therefore, each Packet has a PacketTagList which
+ *     points to a singly-linked list of #TagData.
+ *
+ * \par <b> Copy-on-write </b> 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 <em> the first branch point: </em> \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 <em> the first branch point: </em> \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 <b> Memory Management: </b>
+ * \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.
  */
-#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
+     * 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:
-
-  bool Remove (TypeId tid);
-  struct PacketTagList::TagData *AllocData (void) const;
-  void FreeData (struct TagData *data) const;
+  /**
+   * \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;
 
-  static struct PacketTagList::TagData *g_free;
-  static uint32_t g_nfree;
+  /**
+   * 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);
 
-  struct TagData *m_next;
+  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 */
 };
 
 } // namespace ns3
@@ -118,7 +362,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) 
--- a/src/network/model/packet.cc	Mon May 20 22:57:37 2013 +0200
+++ b/src/network/model/packet.cc	Tue Nov 13 23:44:58 2012 -0800
@@ -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,35 +69,30 @@
 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::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<Packet> (new Packet (*this), false);
 }
 
@@ -150,7 +140,6 @@
     m_metadata (static_cast<uint64_t> (Simulator::GetSystemId ()) << 32 | m_globalUid, 0),
     m_nixVector (0)
 {
-  NS_LOG_FUNCTION (this);
   m_globalUid++;
 }
 
@@ -193,7 +182,6 @@
     m_metadata (static_cast<uint64_t> (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<uint64_t> (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<Packet>
@@ -255,14 +240,12 @@
 void
 Packet::SetNixVector (Ptr<NixVector> nixVector)
 {
-  NS_LOG_FUNCTION (this << nixVector);
   m_nixVector = nixVector;
 }
 
 Ptr<NixVector>
 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<const Packet> 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 @@
   uint32_t newStart = m_buffer.GetCurrentStartOffset ();
  
   // Update tag offsets if buffer offsets were changed
-  const_cast<ByteTagList &>(m_byteTagList).AddAtStart (newStart - oldStart, newStart);
+  const_cast<ByteTagList &> (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 @@
 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 @@
               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 @@
       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 @@
               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 @@
 PacketMetadata::ItemIterator 
 Packet::BeginItem (void) const
 {
-  NS_LOG_FUNCTION (this);
   return m_metadata.BeginItem (m_buffer);
 }
 
@@ -587,7 +568,6 @@
 
 uint32_t Packet::GetSerializedSize (void) const
 {
-  NS_LOG_FUNCTION (this);
   uint32_t size = 0;
 
   if (m_nixVector)
@@ -631,7 +611,6 @@
 uint32_t 
 Packet::Serialize (uint8_t* buffer, uint32_t maxSize) const
 {
-  NS_LOG_FUNCTION (this << &buffer << maxSize);
   uint32_t* p = reinterpret_cast<uint32_t *> (buffer);
   uint32_t size = 0;
 
@@ -654,7 +633,7 @@
             {
               // increment p by nixSize bytes
               // ensuring 4-byte boundary
-              p += ((nixSize+3) & (~3)) / 4;
+              p += ((nixSize + 3) & (~3)) / 4;
             }
           else
             {
@@ -702,7 +681,7 @@
         {
           // increment p by metaSize bytes
           // ensuring 4-byte boundary
-          p += ((metaSize+3) & (~3)) / 4;
+          p += ((metaSize + 3) & (~3)) / 4;
         }
       else
         {
@@ -731,7 +710,7 @@
         {
           // increment p by bufSize bytes
           // ensuring 4-byte boundary
-          p += ((bufSize+3) & (~3)) / 4;
+          p += ((bufSize + 3) & (~3)) / 4;
         }
       else 
         {
@@ -750,7 +729,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<const uint32_t *> (buffer);
 
@@ -832,7 +811,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<ByteTagList *> (&m_byteTagList);
   TagBuffer buffer = list->Add (tag.GetInstanceTypeId (), tag.GetSerializedSize (), 
                                 m_buffer.GetCurrentStartOffset (),
@@ -842,14 +821,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 +844,29 @@
 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 @@
 void 
 Packet::PrintPacketTags (std::ostream &os) const
 {
-  NS_LOG_FUNCTION (this << &os);
   PacketTagIterator i = GetPacketTagIterator ();
   while (i.HasNext ())
     {
@@ -918,7 +903,6 @@
 PacketTagIterator 
 Packet::GetPacketTagIterator (void) const
 {
-  NS_LOG_FUNCTION (this);
   return PacketTagIterator (m_packetTagList.Head ());
 }
 
--- a/src/network/model/packet.h	Mon May 20 22:57:37 2013 +0200
+++ b/src/network/model/packet.h	Tue Nov 13 23:44:58 2012 -0800
@@ -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:
@@ -189,6 +189,11 @@
  * 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 @@
  * 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<Packet>
 {
 public:
-
   /**
    * Create an empty packet with a new uid (as returned
    * by getUid).
@@ -320,7 +324,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 arepresent in
    * the packet.
    *
    * \param size number of bytes from remove
@@ -328,7 +332,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,28 +340,35 @@
   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 @@
   /**
    * \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 @@
   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 @@
    */
   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 @@
   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 @@
   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 @@
    */
   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<NixVector>);
+  /**
+   * Get the packet nix-vector.
+   *
+   * See the comment on SetNixVector
+   */
   Ptr<NixVector> GetNixVector (void) const; 
 
 private:
@@ -590,6 +619,7 @@
  *   - 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 @@
 
 } // namespace ns3
 
+/****************************************************
+ *  Implementation of inline methods for performance
+ ****************************************************/
+
 namespace ns3 {
 
 uint32_t 
--- a/src/network/model/tag.h	Mon May 20 22:57:37 2013 +0200
+++ b/src/network/model/tag.h	Tue Nov 13 23:44:58 2012 -0800
@@ -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
 {
--- a/src/network/test/packet-test-suite.cc	Mon May 20 22:57:37 2013 +0200
+++ b/src/network/test/packet-test-suite.cc	Tue Nov 13 23:44:58 2012 -0800
@@ -18,9 +18,12 @@
  * Author: Mathieu Lacage <[email protected]>
  */
 #include "ns3/packet.h"
+#include "ns3/packet-tag-list.h"
 #include "ns3/test.h"
 #include <string>
 #include <cstdarg>
+#include <iostream>
+#include <ctime>
 
 using namespace ns3;
 
@@ -32,8 +35,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 <int N>
@@ -54,15 +63,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 +84,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 +448,201 @@
 #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 @@
 PacketTestSuite::PacketTestSuite ()
   : TestSuite ("packet", UNIT)
 {
-  AddTestCase (new PacketTest, TestCase::QUICK);
+  AddTestCase (new PacketTest);
+  AddTestCase (new PacketTagListTest);
 }
 
 static PacketTestSuite g_packetTestSuite;