From 16018e8d7eb9fdbbe76f2e018efe3ef6b76f6cd1 Mon Sep 17 00:00:00 2001 From: Mathieu Lacage Date: Wed, 28 May 2008 08:51:27 -0700 Subject: [PATCH 01/13] add logging --- src/common/buffer.cc | 11 +++++++++++ src/common/packet.cc | 17 +++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/src/common/buffer.cc b/src/common/buffer.cc index 5c5bc3bb5..e75156fed 100644 --- a/src/common/buffer.cc +++ b/src/common/buffer.cc @@ -236,6 +236,7 @@ Buffer::CheckInternalState (void) const void Buffer::Initialize (uint32_t zeroSize) { + NS_LOG_FUNCTION (this << zeroSize); m_data = Buffer::Create (0); #ifdef BUFFER_HEURISTICS m_start = std::min (m_data->m_size, g_recommendedStart); @@ -336,6 +337,7 @@ Buffer::GetInternalEnd (void) const bool Buffer::AddAtStart (uint32_t start) { + NS_LOG_FUNCTION (this << start); bool dirty; NS_ASSERT (CheckInternalState ()); bool isDirty = m_data->m_count > 1 && m_start > m_data->m_dirtyStart; @@ -385,6 +387,7 @@ Buffer::AddAtStart (uint32_t start) bool Buffer::AddAtEnd (uint32_t end) { + NS_LOG_FUNCTION (this << end); bool dirty; NS_ASSERT (CheckInternalState ()); bool isDirty = m_data->m_count > 1 && m_end < m_data->m_dirtyEnd; @@ -438,6 +441,7 @@ Buffer::AddAtEnd (uint32_t end) void Buffer::AddAtEnd (const Buffer &o) { + NS_LOG_FUNCTION (this << &o); if (m_data->m_count == 1 && m_end == m_zeroAreaEnd && m_end == m_data->m_dirtyEnd && @@ -460,6 +464,7 @@ Buffer::AddAtEnd (const Buffer &o) Buffer::Iterator src = o.End (); src.Prev (endData); dst.Write (src, o.End ()); + NS_ASSERT (CheckInternalState ()); return; } @@ -471,11 +476,13 @@ Buffer::AddAtEnd (const Buffer &o) destStart.Prev (src.GetSize ()); destStart.Write (src.Begin (), src.End ()); *this = dst; + NS_ASSERT (CheckInternalState ()); } void Buffer::RemoveAtStart (uint32_t start) { + NS_LOG_FUNCTION (this << start); NS_ASSERT (CheckInternalState ()); uint32_t newStart = m_start + start; if (newStart <= m_zeroAreaStart) @@ -520,6 +527,7 @@ Buffer::RemoveAtStart (uint32_t start) void Buffer::RemoveAtEnd (uint32_t end) { + NS_LOG_FUNCTION (this << end); NS_ASSERT (CheckInternalState ()); uint32_t newEnd = m_end - std::min (end, m_end - m_start); if (newEnd > m_zeroAreaEnd) @@ -555,6 +563,7 @@ Buffer::RemoveAtEnd (uint32_t end) Buffer Buffer::CreateFragment (uint32_t start, uint32_t length) const { + NS_LOG_FUNCTION (this << start << length); NS_ASSERT (CheckInternalState ()); Buffer tmp = *this; tmp.RemoveAtStart (start); @@ -566,6 +575,7 @@ Buffer::CreateFragment (uint32_t start, uint32_t length) const Buffer Buffer::CreateFullCopy (void) const { + NS_LOG_FUNCTION (this); NS_ASSERT (CheckInternalState ()); if (m_zeroAreaEnd - m_zeroAreaStart != 0) { @@ -580,6 +590,7 @@ Buffer::CreateFullCopy (void) const Buffer::Iterator i = tmp.End (); i.Prev (dataEnd); i.Write (m_data->m_data+m_zeroAreaStart,dataEnd); + NS_ASSERT (tmp.CheckInternalState ()); return tmp; } NS_ASSERT (CheckInternalState ()); diff --git a/src/common/packet.cc b/src/common/packet.cc index a79ac2a7e..9244689dd 100644 --- a/src/common/packet.cc +++ b/src/common/packet.cc @@ -19,6 +19,9 @@ */ #include "packet.h" #include "ns3/assert.h" +#include "ns3/log.h" + +NS_LOG_COMPONENT_DEFINE ("Packet"); namespace ns3 { @@ -156,6 +159,7 @@ Packet::Packet (const Buffer &buffer, const TagList &tagList, const PacketMetad Ptr Packet::CreateFragment (uint32_t start, uint32_t length) const { + NS_LOG_FUNCTION (this << start << length); Buffer buffer = m_buffer.CreateFragment (start, length); NS_ASSERT (m_buffer.GetSize () >= start + length); uint32_t end = m_buffer.GetSize () - (start + length); @@ -174,6 +178,7 @@ Packet::GetSize (void) const void Packet::AddHeader (const Header &header) { + NS_LOG_FUNCTION (this << &header); uint32_t size = header.GetSerializedSize (); uint32_t orgStart = m_buffer.GetCurrentStartOffset (); bool resized = m_buffer.AddAtStart (size); @@ -188,6 +193,7 @@ Packet::AddHeader (const Header &header) uint32_t Packet::RemoveHeader (Header &header) { + NS_LOG_FUNCTION (this << &header); uint32_t deserialized = header.Deserialize (m_buffer.Begin ()); m_buffer.RemoveAtStart (deserialized); m_metadata.RemoveHeader (header, deserialized); @@ -196,6 +202,7 @@ Packet::RemoveHeader (Header &header) void Packet::AddTrailer (const Trailer &trailer) { + NS_LOG_FUNCTION (this << &trailer); uint32_t size = trailer.GetSerializedSize (); uint32_t orgEnd = m_buffer.GetCurrentEndOffset (); bool resized = m_buffer.AddAtEnd (size); @@ -211,6 +218,7 @@ Packet::AddTrailer (const Trailer &trailer) uint32_t Packet::RemoveTrailer (Trailer &trailer) { + NS_LOG_FUNCTION (this << &trailer); uint32_t deserialized = trailer.Deserialize (m_buffer.End ()); m_buffer.RemoveAtEnd (deserialized); m_metadata.RemoveTrailer (trailer, deserialized); @@ -220,6 +228,7 @@ Packet::RemoveTrailer (Trailer &trailer) void Packet::AddAtEnd (Ptr packet) { + NS_LOG_FUNCTION (this << packet); uint32_t aStart = m_buffer.GetCurrentStartOffset (); uint32_t bEnd = packet->m_buffer.GetCurrentEndOffset (); m_buffer.AddAtEnd (packet->m_buffer); @@ -235,6 +244,7 @@ Packet::AddAtEnd (Ptr packet) void Packet::AddPaddingAtEnd (uint32_t size) { + NS_LOG_FUNCTION (this << size); uint32_t orgEnd = m_buffer.GetCurrentEndOffset (); bool resized = m_buffer.AddAtEnd (size); if (resized) @@ -247,12 +257,14 @@ Packet::AddPaddingAtEnd (uint32_t size) void Packet::RemoveAtEnd (uint32_t size) { + NS_LOG_FUNCTION (this << size); m_buffer.RemoveAtEnd (size); m_metadata.RemoveAtEnd (size); } void Packet::RemoveAtStart (uint32_t size) { + NS_LOG_FUNCTION (this << size); m_buffer.RemoveAtStart (size); m_metadata.RemoveAtStart (size); } @@ -260,6 +272,7 @@ Packet::RemoveAtStart (uint32_t size) void Packet::RemoveAllTags (void) { + NS_LOG_FUNCTION (this); m_tagList.RemoveAll (); } @@ -408,12 +421,14 @@ Packet::BeginItem (void) const void Packet::EnableMetadata (void) { + NS_LOG_FUNCTION_NOARGS (); PacketMetadata::Enable (); } Buffer Packet::Serialize (void) const { + NS_LOG_FUNCTION (this); Buffer buffer; uint32_t reserve; @@ -441,6 +456,7 @@ Packet::Serialize (void) const void Packet::Deserialize (Buffer buffer) { + NS_LOG_FUNCTION (this); Buffer buf = buffer; // read size uint32_t packetSize = buf.Begin ().ReadU32 (); @@ -466,6 +482,7 @@ Packet::Deserialize (Buffer buffer) void Packet::AddTag (const Tag &tag) const { + NS_LOG_FUNCTION (this << &tag); TagList *list = const_cast (&m_tagList); TagBuffer buffer = list->Add (tag.GetInstanceTypeId (), tag.GetSerializedSize (), m_buffer.GetCurrentStartOffset (), From 0c34dea05b5764c947b9c3d10b34f94061aedbe6 Mon Sep 17 00:00:00 2001 From: Mathieu Lacage Date: Wed, 28 May 2008 10:10:47 -0700 Subject: [PATCH 02/13] kill dead code --- src/core/log.h | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/core/log.h b/src/core/log.h index 554d16815..41b190370 100644 --- a/src/core/log.h +++ b/src/core/log.h @@ -327,12 +327,10 @@ public: bool IsNoneEnabled (void) const; void Enable (enum LogLevel level); void Disable (enum LogLevel level); - bool Decorate (void) const; char const *Name (void) const; private: int32_t m_levels; char const *m_name; - bool m_decorate; }; class ParameterLogger : public std::ostream From 31870b3bc0a3b36ac639cf04eae36122709e82ac Mon Sep 17 00:00:00 2001 From: Mathieu Lacage Date: Wed, 28 May 2008 10:11:08 -0700 Subject: [PATCH 03/13] more logging code --- src/common/buffer.cc | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/common/buffer.cc b/src/common/buffer.cc index e75156fed..ad3421df7 100644 --- a/src/common/buffer.cc +++ b/src/common/buffer.cc @@ -203,11 +203,13 @@ Buffer::Create (uint32_t size) Buffer::Buffer () { + NS_LOG_FUNCTION (this); Initialize (0); } Buffer::Buffer (uint32_t dataSize) { + NS_LOG_FUNCTION (this << dataSize); Initialize (dataSize); } @@ -225,12 +227,15 @@ Buffer::CheckInternalState (void) const m_start <= m_data->m_size && m_zeroAreaStart <= m_data->m_size; - NS_ASSERT (offsetsOk); - NS_ASSERT (dirtyOk); - NS_ASSERT (internalSizeOk); - - return m_data->m_count > 0 && offsetsOk && dirtyOk && - internalSizeOk; + bool ok = m_data->m_count > 0 && offsetsOk && dirtyOk && internalSizeOk; + if (!ok) + { + LOG_INTERNAL_STATE ("check " << this << + ", " << (offsetsOk?"true":"false") << + ", " << (dirtyOk?"true":"false") << + ", " << (internalSizeOk?"true":"false") << " "); + } + return ok; } void @@ -262,6 +267,7 @@ Buffer::Buffer (Buffer const&o) m_start (o.m_start), m_end (o.m_end) { + NS_LOG_FUNCTION (this << &o); m_data->m_count++; NS_ASSERT (CheckInternalState ()); } @@ -269,6 +275,7 @@ Buffer::Buffer (Buffer const&o) Buffer & Buffer::operator = (Buffer const&o) { + NS_LOG_FUNCTION (this << &o); NS_ASSERT (CheckInternalState ()); if (m_data != o.m_data) { @@ -295,6 +302,8 @@ Buffer::operator = (Buffer const&o) Buffer::~Buffer () { + NS_LOG_FUNCTION (this); + NS_ASSERT (CheckInternalState ()); HEURISTICS (g_recommendedStart = std::max (g_recommendedStart, m_maxZeroAreaStart)); m_data->m_count--; if (m_data->m_count == 0) From ab01dd65a4aa11e7d54a63521d4ff95ac4ffc491 Mon Sep 17 00:00:00 2001 From: Mathieu Lacage Date: Wed, 28 May 2008 10:11:25 -0700 Subject: [PATCH 04/13] a failing testcase. --- src/common/packet-metadata-test.cc | 30 ++++++++++++++++++++++++++---- 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/src/common/packet-metadata-test.cc b/src/common/packet-metadata-test.cc index e8716e551..d86de1788 100644 --- a/src/common/packet-metadata-test.cc +++ b/src/common/packet-metadata-test.cc @@ -291,7 +291,7 @@ PacketMetadataTest::CheckHistory (Ptr p, const char *file, int line, uin if (!CheckHistory (p, __FILE__, \ __LINE__, __VA_ARGS__)) \ { \ - ok = false; \ + result = false; \ } \ Buffer buffer; \ buffer = p->Serialize (); \ @@ -300,7 +300,7 @@ PacketMetadataTest::CheckHistory (Ptr p, const char *file, int line, uin if (!CheckHistory (otherPacket, __FILE__, \ __LINE__, __VA_ARGS__)) \ { \ - ok = false; \ + result = false; \ } \ } @@ -315,7 +315,7 @@ PacketMetadataTest::DoAddHeader (Ptr p) bool PacketMetadataTest::RunTests (void) { - bool ok = true; + bool result = true; PacketMetadata::Enable (); @@ -624,7 +624,29 @@ PacketMetadataTest::RunTests (void) REM_HEADER (p3, 8); p2->AddAtEnd (p3); - return ok; + + p = Create (1000); + ADD_HEADER (p, 10); + ADD_TRAILER (p, 5); + p1 = p->Copy (); + ADD_HEADER (p1, 20); + REM_HEADER (p1, 20); + REM_TRAILER (p1, 5); + NS_TEST_ASSERT_EQUAL (p->GetSize (), 1015); + + + p = Create (1510); + ADD_HEADER (p, 8); + ADD_HEADER (p, 25); + REM_HEADER (p, 25); + ADD_HEADER (p, 1); + p1 = p->CreateFragment (0, 1500); + p2 = p1->Copy (); + ADD_HEADER (p2, 24); + NS_TEST_ASSERT_EQUAL (p->GetSize (), 1518); + + + return result; } static PacketMetadataTest g_packetHistoryTest; From 137ff47a46e2a015f789ee5b6dabe6d3f3bce8d1 Mon Sep 17 00:00:00 2001 From: Mathieu Lacage Date: Wed, 28 May 2008 10:31:09 -0700 Subject: [PATCH 05/13] fix the testcase --- src/common/packet-metadata-test.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/packet-metadata-test.cc b/src/common/packet-metadata-test.cc index d86de1788..1ccd92eb9 100644 --- a/src/common/packet-metadata-test.cc +++ b/src/common/packet-metadata-test.cc @@ -643,7 +643,7 @@ PacketMetadataTest::RunTests (void) p1 = p->CreateFragment (0, 1500); p2 = p1->Copy (); ADD_HEADER (p2, 24); - NS_TEST_ASSERT_EQUAL (p->GetSize (), 1518); + NS_TEST_ASSERT_EQUAL (p->GetSize (), 1519); return result; From 45793c3a02016df5d41053f790d7c36c6b6f36d5 Mon Sep 17 00:00:00 2001 From: Mathieu Lacage Date: Wed, 28 May 2008 10:47:14 -0700 Subject: [PATCH 06/13] fix new packet bug --- src/common/buffer.cc | 57 ++++++++++++++++++-------------------------- src/common/buffer.h | 16 +++++++++++-- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/src/common/buffer.cc b/src/common/buffer.cc index ad3421df7..a179e3f67 100644 --- a/src/common/buffer.cc +++ b/src/common/buffer.cc @@ -24,10 +24,10 @@ NS_LOG_COMPONENT_DEFINE ("Buffer"); -#define LOG_INTERNAL_STATE(y) \ -NS_LOG_LOGIC (y << "start="<m_ownerId; #ifdef BUFFER_HEURISTICS m_start = std::min (m_data->m_size, g_recommendedStart); m_maxZeroAreaStart = m_start; @@ -252,13 +243,12 @@ Buffer::Initialize (uint32_t zeroSize) m_zeroAreaStart = m_start; m_zeroAreaEnd = m_zeroAreaStart + zeroSize; m_end = m_zeroAreaEnd; - m_data->m_dirtyStart = m_start; - m_data->m_dirtyEnd = m_end; NS_ASSERT (CheckInternalState ()); } Buffer::Buffer (Buffer const&o) : m_data (o.m_data), + m_ownerId (o.m_ownerId), #ifdef BUFFER_HEURISTICS m_maxZeroAreaStart (o.m_zeroAreaStart), #endif @@ -286,6 +276,7 @@ Buffer::operator = (Buffer const&o) Recycle (m_data); } m_data = o.m_data; + m_ownerId = o.m_ownerId; m_data->m_count++; } HEURISTICS ( @@ -349,7 +340,7 @@ Buffer::AddAtStart (uint32_t start) NS_LOG_FUNCTION (this << start); bool dirty; NS_ASSERT (CheckInternalState ()); - bool isDirty = m_data->m_count > 1 && m_start > m_data->m_dirtyStart; + bool isDirty = m_data->m_count > 1 && m_ownerId != m_data->m_ownerId; if (m_start >= start && !isDirty) { /* enough space in the buffer and not dirty. @@ -357,9 +348,11 @@ Buffer::AddAtStart (uint32_t start) * Before: |*****---------***| * After: |***..---------***| */ - NS_ASSERT (m_data->m_count == 1 || m_start == m_data->m_dirtyStart); + NS_ASSERT (m_data->m_count == 1 || m_ownerId == m_data->m_ownerId); m_start -= start; - dirty = m_start > m_data->m_dirtyStart; + dirty = m_ownerId != m_data->m_ownerId; + m_ownerId++; + m_data->m_ownerId = m_ownerId; HEURISTICS (g_nAddNoRealloc++); } else @@ -373,6 +366,7 @@ Buffer::AddAtStart (uint32_t start) Buffer::Recycle (m_data); } m_data = newData; + m_ownerId = newData->m_ownerId; int32_t delta = start - m_start; m_start += delta; @@ -386,9 +380,6 @@ Buffer::AddAtStart (uint32_t start) HEURISTICS (g_nAddRealloc++); } HEURISTICS (m_maxZeroAreaStart = std::max (m_maxZeroAreaStart, m_zeroAreaStart)); - // update dirty area - m_data->m_dirtyStart = m_start; - m_data->m_dirtyEnd = m_end; LOG_INTERNAL_STATE ("add start=" << start << ", "); NS_ASSERT (CheckInternalState ()); return dirty; @@ -399,7 +390,7 @@ Buffer::AddAtEnd (uint32_t end) NS_LOG_FUNCTION (this << end); bool dirty; NS_ASSERT (CheckInternalState ()); - bool isDirty = m_data->m_count > 1 && m_end < m_data->m_dirtyEnd; + bool isDirty = m_data->m_count > 1 && m_ownerId != m_data->m_ownerId; if (GetInternalEnd () + end <= m_data->m_size && !isDirty) { /* enough space in buffer and not dirty @@ -407,10 +398,12 @@ Buffer::AddAtEnd (uint32_t end) * Before: |**----*****| * After: |**----...**| */ - NS_ASSERT (m_data->m_count == 1 || m_end == m_data->m_dirtyEnd); + NS_ASSERT (m_data->m_count == 1 || m_ownerId == m_data->m_ownerId); m_end += end; - dirty = m_end < m_data->m_dirtyEnd; + dirty = m_ownerId != m_data->m_ownerId; + m_ownerId++; + m_data->m_ownerId = m_ownerId; HEURISTICS (g_nAddNoRealloc++); } @@ -425,6 +418,7 @@ Buffer::AddAtEnd (uint32_t end) Buffer::Recycle (m_data); } m_data = newData; + m_ownerId = newData->m_ownerId; int32_t delta = -m_start; m_zeroAreaStart += delta; @@ -438,9 +432,6 @@ Buffer::AddAtEnd (uint32_t end) HEURISTICS (g_nAddRealloc++); } HEURISTICS (m_maxZeroAreaStart = std::max (m_maxZeroAreaStart, m_zeroAreaStart)); - // update dirty area - m_data->m_dirtyStart = m_start; - m_data->m_dirtyEnd = m_end; LOG_INTERNAL_STATE ("add end=" << end << ", "); NS_ASSERT (CheckInternalState ()); @@ -453,7 +444,7 @@ Buffer::AddAtEnd (const Buffer &o) NS_LOG_FUNCTION (this << &o); if (m_data->m_count == 1 && m_end == m_zeroAreaEnd && - m_end == m_data->m_dirtyEnd && + m_ownerId == m_data->m_ownerId && o.m_start == o.m_zeroAreaStart && o.m_zeroAreaEnd - o.m_zeroAreaStart > 0) { @@ -465,7 +456,6 @@ Buffer::AddAtEnd (const Buffer &o) uint32_t zeroSize = o.m_zeroAreaEnd - o.m_zeroAreaStart; m_zeroAreaEnd += zeroSize; m_end = m_zeroAreaEnd; - m_data->m_dirtyEnd = m_zeroAreaEnd; uint32_t endData = o.m_end - o.m_zeroAreaEnd; AddAtEnd (endData); Buffer::Iterator dst = End (); @@ -750,7 +740,6 @@ Buffer::Iterator::Write (Iterator start, Iterator end) NS_ASSERT (start.m_current <= end.m_current); NS_ASSERT (start.m_zeroStart == end.m_zeroStart); NS_ASSERT (start.m_zeroEnd == end.m_zeroEnd); - NS_ASSERT (m_data != start.m_data); uint32_t size = end.m_current - start.m_current; Iterator cur = start; for (uint32_t i = 0; i < size; i++) diff --git a/src/common/buffer.h b/src/common/buffer.h index 75f93f780..452e1b66f 100644 --- a/src/common/buffer.h +++ b/src/common/buffer.h @@ -58,8 +58,13 @@ namespace ns3 { * BufferData. If the BufferData::m_count field is one, it means that * there exist only one instance of Buffer which references the * BufferData instance so, it is safe to modify it. It is also - * safe to modify the content of a BufferData if the modification - * falls outside of the "dirty area" defined by the BufferData. + * safe to modify the content of a BufferData if someone has not yet + * made incompatible changes to the BufferData and we detect that + * case when the m_ownerId field is equal to the m_ownerId of the + * BufferData itself. Whenever a change potentially incompatible + * with other users is made to a BufferData object, we update the + * m_ownerId field of the BufferData to ensure that the other users + * will detect out modification. * In every other case, the BufferData must be copied before * being modified. * @@ -481,6 +486,13 @@ private: /* This structure is described in the buffer.cc file. */ struct BufferData *m_data; + /* id of the owner of the BufferData object. + * If this id matches the id stored in the BufferData object, + * we are the current owner of the data. If we are not the owner, + * and we need to acquire ownership before making modifications + * by doing a copy. + */ + uint32_t m_ownerId; #ifdef BUFFER_HEURISTICS /* keep track of the maximum value of m_zeroAreaStart across * the lifetime of a Buffer instance. This variable is used From 709f6a6d5bbe6c3d4b66ba17551ebaaa80fc1887 Mon Sep 17 00:00:00 2001 From: Mathieu Lacage Date: Wed, 28 May 2008 10:47:21 -0700 Subject: [PATCH 07/13] document --- src/devices/wifi/constant-rate-wifi-manager.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/devices/wifi/constant-rate-wifi-manager.cc b/src/devices/wifi/constant-rate-wifi-manager.cc index d8586c77b..7cf10936b 100644 --- a/src/devices/wifi/constant-rate-wifi-manager.cc +++ b/src/devices/wifi/constant-rate-wifi-manager.cc @@ -77,11 +77,11 @@ ConstantRateWifiManager::GetTypeId (void) static TypeId tid = TypeId ("ns3::ConstantRateWifiManager") .SetParent () .AddConstructor () - .AddAttribute ("DataMode", "XXX", + .AddAttribute ("DataMode", "The transmission mode to use for every data packet transmission", StringValue ("wifia-6mbs"), MakeWifiModeAccessor (&ConstantRateWifiManager::m_dataMode), MakeWifiModeChecker ()) - .AddAttribute ("ControlMode", "XXX", + .AddAttribute ("ControlMode", "The transmission mode to use for every control packet transmission.", StringValue ("wifia-6mbs"), MakeWifiModeAccessor (&ConstantRateWifiManager::m_ctlMode), MakeWifiModeChecker ()) From 27e56402011a1d5d6541837b5ded52f37748b375 Mon Sep 17 00:00:00 2001 From: Mathieu Lacage Date: Wed, 28 May 2008 11:28:06 -0700 Subject: [PATCH 08/13] the TypeId for Packet sockets is PacketSocketFactory, not PacketSocket. fix bug introduced by changeset c33b6d2775b7. --- examples/wifi-adhoc.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/examples/wifi-adhoc.cc b/examples/wifi-adhoc.cc index 1759f3643..09cded9f0 100644 --- a/examples/wifi-adhoc.cc +++ b/examples/wifi-adhoc.cc @@ -102,7 +102,7 @@ Experiment::ReceivePacket (Ptr socket) Ptr Experiment::SetupPacketReceive (Ptr node) { - TypeId tid = TypeId::LookupByName ("ns3::PacketSocket"); + TypeId tid = TypeId::LookupByName ("ns3::PacketSocketFactory"); Ptr sink = Socket::CreateSocket (node, tid); sink->Bind (); sink->SetRecvCallback (MakeCallback (&Experiment::ReceivePacket, this)); @@ -136,7 +136,7 @@ Experiment::Run (const WifiHelper &wifi) socket.SetPhysicalAddress (devices.Get (1)->GetAddress ()); socket.SetProtocol (1); - OnOffHelper onoff ("ns3::PacketSocket", Address (socket)); + OnOffHelper onoff ("ns3::PacketSocketFactory", Address (socket)); onoff.SetAttribute ("OnTime", RandomVariableValue (ConstantVariable (250))); onoff.SetAttribute ("OffTime", RandomVariableValue (ConstantVariable (0))); onoff.SetAttribute ("DataRate", DataRateValue (DataRate (60000000))); From 76dd4d1db490e6582942bcf1fa3bfcd3b7d38c00 Mon Sep 17 00:00:00 2001 From: Mathieu Lacage Date: Wed, 28 May 2008 13:03:29 -0700 Subject: [PATCH 09/13] Fix off-by-2 errors --- src/node/address.cc | 21 ++++++++++----------- src/node/address.h | 5 +++-- src/node/socket.cc | 19 ++----------------- 3 files changed, 15 insertions(+), 30 deletions(-) diff --git a/src/node/address.cc b/src/node/address.cc index fb49bf8ce..1902324d9 100644 --- a/src/node/address.cc +++ b/src/node/address.cc @@ -114,21 +114,20 @@ Address::GetSerializedSize (void) const } void -Address::Serialize (uint8_t* buf, uint32_t len) const +Address::Serialize (TagBuffer buffer) const { - NS_ASSERT (len >= static_cast (m_len + 2)); - buf[0] = m_type; - buf[1] = m_len; - for (uint8_t i = 0; i < m_len; i++) - { - buf[i+2] = m_data[i]; - } + buffer.WriteU8 (m_type); + buffer.WriteU8 (m_len); + buffer.Write (m_data, m_len); } -Address -Address::Deserialize (const uint8_t* buf) +void +Address::Deserialize (TagBuffer buffer) { - return Address (buf[0], buf + 2, buf[1]); + m_type = buffer.ReadU8 (); + m_len = buffer.ReadU8 (); + NS_ASSERT (m_len <= MAX_SIZE); + buffer.Read (m_data, m_len); } ATTRIBUTE_HELPER_CPP (Address); diff --git a/src/node/address.h b/src/node/address.h index 16a8dc555..d88d5643d 100644 --- a/src/node/address.h +++ b/src/node/address.h @@ -5,6 +5,7 @@ #include #include "ns3/attribute.h" #include "ns3/attribute-helper.h" +#include "ns3/tag-buffer.h" namespace ns3 { @@ -166,14 +167,14 @@ public: * \param buf output buffer that gets written with this Address * \param len length of output buffer */ - void Serialize (uint8_t* buf, uint32_t len) const; + void Serialize (TagBuffer buffer) const; /** * \param buf buffer to read address from * \returns an Address * * The input address buffer is expected to be in host byte order format. */ - static Address Deserialize (const uint8_t* buf); + void Deserialize (TagBuffer buffer); private: friend bool operator == (const Address &a, const Address &b); diff --git a/src/node/socket.cc b/src/node/socket.cc index 18a15b0e9..0a9aabc42 100644 --- a/src/node/socket.cc +++ b/src/node/socket.cc @@ -304,27 +304,12 @@ SocketRxAddressTag::GetSerializedSize (void) const void SocketRxAddressTag::Serialize (TagBuffer i) const { - uint8_t len = m_address.GetSerializedSize (); - uint8_t* buffer = new uint8_t[len]; - memset (buffer, 0, len); - m_address.Serialize (buffer, len); - i.Write (buffer, len); - delete [] buffer; + m_address.Serialize (i); } void SocketRxAddressTag::Deserialize (TagBuffer i) { - uint8_t type = i.ReadU8 (); - uint8_t len = i.ReadU8 (); - // Len is the length of the address starting from buffer[2] - NS_ASSERT (len >= 2); - uint8_t* buffer = new uint8_t[len]; - memset (buffer, 0, len); - buffer[0] = type; - buffer[1] = len; - i.Read (buffer+2, len); // ReadU8 consumes a byte - m_address = Address::Deserialize (buffer); - delete [] buffer; + m_address.Deserialize (i); } SocketIpTtlTag::SocketIpTtlTag () From c9ccce91ed0622469b293881bc2859de39ec3742 Mon Sep 17 00:00:00 2001 From: Mathieu Lacage Date: Wed, 28 May 2008 13:22:50 -0700 Subject: [PATCH 10/13] remove dead code --- src/node/socket-defaults.cc | 42 ------------------------------------- src/node/socket-defaults.h | 42 ------------------------------------- 2 files changed, 84 deletions(-) delete mode 100644 src/node/socket-defaults.cc delete mode 100644 src/node/socket-defaults.h diff --git a/src/node/socket-defaults.cc b/src/node/socket-defaults.cc deleted file mode 100644 index 20c97b980..000000000 --- a/src/node/socket-defaults.cc +++ /dev/null @@ -1,42 +0,0 @@ -/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */ -/* - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation; - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - * - */ -#include "socket-defaults.h" -#include "ns3/uinteger.h" - -namespace ns3 { - -NS_OBJECT_ENSURE_REGISTERED (SocketDefaults); - -TypeId SocketDefaults::GetTypeId (void) -{ - static TypeId tid = TypeId ("ns3::SocketDefaults") - .SetParent () - .AddAttribute ("DefaultSndBufLimit", - "Default maximum receive buffer size (bytes)", - UintegerValue (0xffffffffl), - MakeUintegerAccessor (&SocketDefaults::m_defaultSndBufLimit), - MakeUintegerChecker ()) - .AddAttribute ("DefaultRcvBufLimit", - "Default maximum receive buffer size (bytes)", - UintegerValue (0xffffffffl), - MakeUintegerAccessor (&SocketDefaults::m_defaultRcvBufLimit), - MakeUintegerChecker ()) - ; - return tid; -} - -} // namespace ns3 diff --git a/src/node/socket-defaults.h b/src/node/socket-defaults.h deleted file mode 100644 index e6eb290fb..000000000 --- a/src/node/socket-defaults.h +++ /dev/null @@ -1,42 +0,0 @@ -/* -*- Mode:C++; c-file-style:"gnu"; indent-tabs-mode:nil; -*- */ -/* - * This program is free software; you can redistribute it and/or modify - * it under the terms of the GNU General Public License version 2 as - * published by the Free Software Foundation; - * - * This program is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with this program; if not, write to the Free Software - * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA - */ -#ifndef SOCKET_DEFAULTS_H -#define SOCKET_DEFAULTS_H - -#include "ns3/object.h" - -namespace ns3 { - -/** - * \brief Object to hold socket option defaults - * - * This class can be aggregated to a Node and can be used to store - * socket defaults for a Node. - * - */ -class SocketDefaults : public Object -{ -public: - static TypeId GetTypeId (void); - -private: - uint32_t m_defaultSndBufLimit; - uint32_t m_defaultRcvBufLimit; -}; - -} // namespace ns3 - -#endif /* SOCKET_DEFAULTS_H */ From 08961f48837167b9cf0ee9a7207262619a9b9139 Mon Sep 17 00:00:00 2001 From: Mathieu Lacage Date: Wed, 28 May 2008 13:59:53 -0700 Subject: [PATCH 11/13] make sure that the mac low listener is destroyed _after_ the mac low itself. --- src/devices/wifi/dca-txop.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/devices/wifi/dca-txop.cc b/src/devices/wifi/dca-txop.cc index 9eee9cb9d..83282f011 100644 --- a/src/devices/wifi/dca-txop.cc +++ b/src/devices/wifi/dca-txop.cc @@ -139,10 +139,6 @@ void DcaTxop::DoDispose (void) { NS_LOG_FUNCTION (this); - delete m_transmissionListener; - delete m_dcf; - delete m_rng; - delete m_txMiddle; m_transmissionListener = 0; m_dcf = 0; m_rng = 0; @@ -150,6 +146,10 @@ DcaTxop::DoDispose (void) m_queue = 0; m_low = 0; m_stationManager = 0; + delete m_transmissionListener; + delete m_dcf; + delete m_rng; + delete m_txMiddle; } void From b851cc5b0ba65bf6134cfd6f3a2b7c2ad4e84c75 Mon Sep 17 00:00:00 2001 From: Tom Henderson Date: Wed, 28 May 2008 22:06:14 -0700 Subject: [PATCH 12/13] Undo changeset 3163 (breaks multicast) --- src/common/buffer.cc | 57 ++++++++++++++++++++++++++------------------ src/common/buffer.h | 16 ++----------- 2 files changed, 36 insertions(+), 37 deletions(-) diff --git a/src/common/buffer.cc b/src/common/buffer.cc index a179e3f67..ad3421df7 100644 --- a/src/common/buffer.cc +++ b/src/common/buffer.cc @@ -24,10 +24,10 @@ NS_LOG_COMPONENT_DEFINE ("Buffer"); -#define LOG_INTERNAL_STATE(y) \ - NS_LOG_LOGIC (y << "start="<m_ownerId; #ifdef BUFFER_HEURISTICS m_start = std::min (m_data->m_size, g_recommendedStart); m_maxZeroAreaStart = m_start; @@ -243,12 +252,13 @@ Buffer::Initialize (uint32_t zeroSize) m_zeroAreaStart = m_start; m_zeroAreaEnd = m_zeroAreaStart + zeroSize; m_end = m_zeroAreaEnd; + m_data->m_dirtyStart = m_start; + m_data->m_dirtyEnd = m_end; NS_ASSERT (CheckInternalState ()); } Buffer::Buffer (Buffer const&o) : m_data (o.m_data), - m_ownerId (o.m_ownerId), #ifdef BUFFER_HEURISTICS m_maxZeroAreaStart (o.m_zeroAreaStart), #endif @@ -276,7 +286,6 @@ Buffer::operator = (Buffer const&o) Recycle (m_data); } m_data = o.m_data; - m_ownerId = o.m_ownerId; m_data->m_count++; } HEURISTICS ( @@ -340,7 +349,7 @@ Buffer::AddAtStart (uint32_t start) NS_LOG_FUNCTION (this << start); bool dirty; NS_ASSERT (CheckInternalState ()); - bool isDirty = m_data->m_count > 1 && m_ownerId != m_data->m_ownerId; + bool isDirty = m_data->m_count > 1 && m_start > m_data->m_dirtyStart; if (m_start >= start && !isDirty) { /* enough space in the buffer and not dirty. @@ -348,11 +357,9 @@ Buffer::AddAtStart (uint32_t start) * Before: |*****---------***| * After: |***..---------***| */ - NS_ASSERT (m_data->m_count == 1 || m_ownerId == m_data->m_ownerId); + NS_ASSERT (m_data->m_count == 1 || m_start == m_data->m_dirtyStart); m_start -= start; - dirty = m_ownerId != m_data->m_ownerId; - m_ownerId++; - m_data->m_ownerId = m_ownerId; + dirty = m_start > m_data->m_dirtyStart; HEURISTICS (g_nAddNoRealloc++); } else @@ -366,7 +373,6 @@ Buffer::AddAtStart (uint32_t start) Buffer::Recycle (m_data); } m_data = newData; - m_ownerId = newData->m_ownerId; int32_t delta = start - m_start; m_start += delta; @@ -380,6 +386,9 @@ Buffer::AddAtStart (uint32_t start) HEURISTICS (g_nAddRealloc++); } HEURISTICS (m_maxZeroAreaStart = std::max (m_maxZeroAreaStart, m_zeroAreaStart)); + // update dirty area + m_data->m_dirtyStart = m_start; + m_data->m_dirtyEnd = m_end; LOG_INTERNAL_STATE ("add start=" << start << ", "); NS_ASSERT (CheckInternalState ()); return dirty; @@ -390,7 +399,7 @@ Buffer::AddAtEnd (uint32_t end) NS_LOG_FUNCTION (this << end); bool dirty; NS_ASSERT (CheckInternalState ()); - bool isDirty = m_data->m_count > 1 && m_ownerId != m_data->m_ownerId; + bool isDirty = m_data->m_count > 1 && m_end < m_data->m_dirtyEnd; if (GetInternalEnd () + end <= m_data->m_size && !isDirty) { /* enough space in buffer and not dirty @@ -398,12 +407,10 @@ Buffer::AddAtEnd (uint32_t end) * Before: |**----*****| * After: |**----...**| */ - NS_ASSERT (m_data->m_count == 1 || m_ownerId == m_data->m_ownerId); + NS_ASSERT (m_data->m_count == 1 || m_end == m_data->m_dirtyEnd); m_end += end; - dirty = m_ownerId != m_data->m_ownerId; - m_ownerId++; - m_data->m_ownerId = m_ownerId; + dirty = m_end < m_data->m_dirtyEnd; HEURISTICS (g_nAddNoRealloc++); } @@ -418,7 +425,6 @@ Buffer::AddAtEnd (uint32_t end) Buffer::Recycle (m_data); } m_data = newData; - m_ownerId = newData->m_ownerId; int32_t delta = -m_start; m_zeroAreaStart += delta; @@ -432,6 +438,9 @@ Buffer::AddAtEnd (uint32_t end) HEURISTICS (g_nAddRealloc++); } HEURISTICS (m_maxZeroAreaStart = std::max (m_maxZeroAreaStart, m_zeroAreaStart)); + // update dirty area + m_data->m_dirtyStart = m_start; + m_data->m_dirtyEnd = m_end; LOG_INTERNAL_STATE ("add end=" << end << ", "); NS_ASSERT (CheckInternalState ()); @@ -444,7 +453,7 @@ Buffer::AddAtEnd (const Buffer &o) NS_LOG_FUNCTION (this << &o); if (m_data->m_count == 1 && m_end == m_zeroAreaEnd && - m_ownerId == m_data->m_ownerId && + m_end == m_data->m_dirtyEnd && o.m_start == o.m_zeroAreaStart && o.m_zeroAreaEnd - o.m_zeroAreaStart > 0) { @@ -456,6 +465,7 @@ Buffer::AddAtEnd (const Buffer &o) uint32_t zeroSize = o.m_zeroAreaEnd - o.m_zeroAreaStart; m_zeroAreaEnd += zeroSize; m_end = m_zeroAreaEnd; + m_data->m_dirtyEnd = m_zeroAreaEnd; uint32_t endData = o.m_end - o.m_zeroAreaEnd; AddAtEnd (endData); Buffer::Iterator dst = End (); @@ -740,6 +750,7 @@ Buffer::Iterator::Write (Iterator start, Iterator end) NS_ASSERT (start.m_current <= end.m_current); NS_ASSERT (start.m_zeroStart == end.m_zeroStart); NS_ASSERT (start.m_zeroEnd == end.m_zeroEnd); + NS_ASSERT (m_data != start.m_data); uint32_t size = end.m_current - start.m_current; Iterator cur = start; for (uint32_t i = 0; i < size; i++) diff --git a/src/common/buffer.h b/src/common/buffer.h index 452e1b66f..75f93f780 100644 --- a/src/common/buffer.h +++ b/src/common/buffer.h @@ -58,13 +58,8 @@ namespace ns3 { * BufferData. If the BufferData::m_count field is one, it means that * there exist only one instance of Buffer which references the * BufferData instance so, it is safe to modify it. It is also - * safe to modify the content of a BufferData if someone has not yet - * made incompatible changes to the BufferData and we detect that - * case when the m_ownerId field is equal to the m_ownerId of the - * BufferData itself. Whenever a change potentially incompatible - * with other users is made to a BufferData object, we update the - * m_ownerId field of the BufferData to ensure that the other users - * will detect out modification. + * safe to modify the content of a BufferData if the modification + * falls outside of the "dirty area" defined by the BufferData. * In every other case, the BufferData must be copied before * being modified. * @@ -486,13 +481,6 @@ private: /* This structure is described in the buffer.cc file. */ struct BufferData *m_data; - /* id of the owner of the BufferData object. - * If this id matches the id stored in the BufferData object, - * we are the current owner of the data. If we are not the owner, - * and we need to acquire ownership before making modifications - * by doing a copy. - */ - uint32_t m_ownerId; #ifdef BUFFER_HEURISTICS /* keep track of the maximum value of m_zeroAreaStart across * the lifetime of a Buffer instance. This variable is used From 41b7eadd3397da53072d858d455c461240e6bbd6 Mon Sep 17 00:00:00 2001 From: Tom Henderson Date: Wed, 28 May 2008 22:45:59 -0700 Subject: [PATCH 13/13] undo changeset 3161 (to pass unit tests) --- src/common/packet-metadata-test.cc | 30 ++++-------------------------- 1 file changed, 4 insertions(+), 26 deletions(-) diff --git a/src/common/packet-metadata-test.cc b/src/common/packet-metadata-test.cc index 1ccd92eb9..e8716e551 100644 --- a/src/common/packet-metadata-test.cc +++ b/src/common/packet-metadata-test.cc @@ -291,7 +291,7 @@ PacketMetadataTest::CheckHistory (Ptr p, const char *file, int line, uin if (!CheckHistory (p, __FILE__, \ __LINE__, __VA_ARGS__)) \ { \ - result = false; \ + ok = false; \ } \ Buffer buffer; \ buffer = p->Serialize (); \ @@ -300,7 +300,7 @@ PacketMetadataTest::CheckHistory (Ptr p, const char *file, int line, uin if (!CheckHistory (otherPacket, __FILE__, \ __LINE__, __VA_ARGS__)) \ { \ - result = false; \ + ok = false; \ } \ } @@ -315,7 +315,7 @@ PacketMetadataTest::DoAddHeader (Ptr p) bool PacketMetadataTest::RunTests (void) { - bool result = true; + bool ok = true; PacketMetadata::Enable (); @@ -624,29 +624,7 @@ PacketMetadataTest::RunTests (void) REM_HEADER (p3, 8); p2->AddAtEnd (p3); - - p = Create (1000); - ADD_HEADER (p, 10); - ADD_TRAILER (p, 5); - p1 = p->Copy (); - ADD_HEADER (p1, 20); - REM_HEADER (p1, 20); - REM_TRAILER (p1, 5); - NS_TEST_ASSERT_EQUAL (p->GetSize (), 1015); - - - p = Create (1510); - ADD_HEADER (p, 8); - ADD_HEADER (p, 25); - REM_HEADER (p, 25); - ADD_HEADER (p, 1); - p1 = p->CreateFragment (0, 1500); - p2 = p1->Copy (); - ADD_HEADER (p2, 24); - NS_TEST_ASSERT_EQUAL (p->GetSize (), 1519); - - - return result; + return ok; } static PacketMetadataTest g_packetHistoryTest;