Bugzilla – Bug 2221
expanding packet tag maximum size
Last modified: 2016-11-27 11:42:34 EST
changeset 11638:553cf6229117 bumped up the maximum size of a packet tag from 20 to 21 bytes, to accommodate a larger WifiTxVector size. The documentation still says 20 bytes: * \internal * Ideally, TagData would be 32 bytes in size, so they require * no padding on 64-bit architectures. (The architecture * affects the size because of the \c #next pointer.) * This would leave 18 bytes for \c #data. However, * ns3:Ipv6PacketInfoTag needs 19 bytes! The current * implementation allows 20 bytes, which gives TagData * a size of 30 bytes on 32-byte machines (which gets * padded with 2 bytes), and 34 bytes on 64-bit machines, which * gets padded to 40 bytes. We should verify that 21 and perhaps 22 avoids padding on 32-byte machines, and also align the documentation.
some information in this thread suggests that expanding to 22 is costly for 64-bit machines, and maybe 26 is a better limit: https://groups.google.com/forum/#!topic/ns-3-users/uSiRMf8bkxk Mathieu also remarks about removing this constraint: "Long term, I think that finding a way to make PacketTagList not have a tag size constraint is the way to go. Namely, I have tried to be careful to make the chunk cache separate from the rest of the code and I would be happy to see hard numbers with real simulation scenario benchmarks that show how much it would cost us to remove the constant-sized chunks and rely on malloc's size optimizations. In the past, I did benchmark it and I obviously measured a real gain for the approach I implemented but this was a long time ago and things change."
I marked as "PATCH WANTED" but I think we need to decide whether to change the maximum, or go for the longer term solution, or both (short term and long term).
*** Bug 2361 has been marked as a duplicate of this bug. ***
Created attachment 2672 [details] Patch implementing variable size PacketTagList::TagData This patch supports variable (dynamic) sized TagData buffers. It uses the Tag::GetSerializedSize() to malloc the internal data array. Passes all tests with gcc-5.4.0.
Created attachment 2673 [details] Revised to use placement new
I don't have any relevant performance tests. I'd be surprised if we could measure this change except in a very localized PacketTagList benchmark. Since we took out the local allocator, the fixed size is an anachronism IMO.
I'd like to last call this since bug 2551 depends on it and bug 2551 merge is holding up some other patches from Sebastien. I have tested this patch on clang and gcc today and found no problems.
+1
committed as changeset 12425:1313ad34c26a (test case added one commit later).