Bugzilla – Full Text Bug Listing |
Summary: | Packet metadata disabling on a per-packet basis | ||
---|---|---|---|
Product: | ns-3 | Reporter: | Tommaso Pecorella <tommaso.pecorella> |
Component: | network | Assignee: | ns-bugs <ns-bugs> |
Status: | PATCH PENDING --- | ||
Severity: | normal | CC: | tomh |
Priority: | P5 | ||
Version: | ns-3-dev | ||
Hardware: | All | ||
OS: | All | ||
Attachments: |
example script
possible patch revised patch for ns-3-dev tip New patch Newest patch |
Description
Tommaso Pecorella
2015-03-15 04:09:01 EDT
Created attachment 1991 [details]
example script
Test script (it could be reduced to "send an Icmpv6 Destination Unreachable")
Created attachment 1992 [details]
possible patch
This patch disables metadata removal from headers or trailers if the packet has been created from a plain byte buffer.
I still don't know why the
Packet::Packet (uint8_t const*buffer, uint32_t size)
constructor implicitly creates a "size" fake header metadata. Removing it is harmful.
I tried the patch on ns-3-dev and while I found that it fixed the example program, it caused other example/test failures. ./waf --run examples/ipv6/icmpv6-redirect 'build' finished successfully (4.052s) Node: 1, Time: +0.0s, Local time: +0.0s, Ipv6StaticRouting table Destination Next Hop Flag Met Ref Use If ::1/128 :: UH 0 - - 0 fe80::/64 :: U 0 - - 1 2001:1::/64 :: U 0 - - 1 2001:2::200:ff:fe00:5/128 fe80::200:ff:fe00:3 UH 0 - - 1 assert failed. cond="m_current >= delta", file=./ns3/buffer.h, line=859 terminate called without an active exception These are the new failures due to the patch I will attach. List of FAILed tests: packet-metadata List of CRASHed tests: examples/ipv6/icmpv6-redirect examples/ipv6/radvd examples/ipv6/radvd-two-prefix examples/routing/simple-routing-ping6 examples/routing/simple-routing-ping6.py sixlowpan-fragmentation Created attachment 2300 [details]
revised patch for ns-3-dev tip
Created attachment 2306 [details]
New patch
Changed the approach to much, much better one.
The developer have to *explicitly* mark the packet as non-metadata safe.
The previous idea was not considering the case of payloads built by a buffer with metadata-safe headers.
Note that there's still one problem, as is what to do with the case of mix-n-match.
Example: IP packet built from a buffer (non-metadata-safe). Then you add the Ethernet header / trailer. those *will* add metadata.
Upon removing the Ethernet header/trailer, metadata are not removed, because the packet is not safe.
So, the question is: shall we remove all the metadata if the packet is not safe ?
What to do when two packets are merged (AddAtEnd) and only one of them is non-metadata-safe ?
The actual patch fixes an actual bug, but the above points could come back sooner or later.
(In reply to Tommaso Pecorella from comment #5) > Created attachment 2306 [details] > New patch > > Changed the approach to much, much better one. > > The developer have to *explicitly* mark the packet as non-metadata safe. > The previous idea was not considering the case of payloads built by a buffer > with metadata-safe headers. What about making m_metadataDisabled an instance member of PacketMetadata instead, and then the only touch to class Packet is: /** * Disable, on a per-packet basis, the use of metadata. This must be used * if metadata checking is globally enabled, and if the packet is built in a * manner (e.g. from a raw byte buffer) such that the metadata is * incomplete. */ void Packet::DisableMetadata (void) { m_metadata.Disable (); } ('Disabled' seems a clearer term than 'Dirty' to me) (also, document that the Create<Packet> () method of packet creation from raw byte buffer will disable metadata even if it is globally enabled) > > Note that there's still one problem, as is what to do with the case of > mix-n-match. > Example: IP packet built from a buffer (non-metadata-safe). Then you add the > Ethernet header / trailer. those *will* add metadata. > Upon removing the Ethernet header/trailer, metadata are not removed, because > the packet is not safe. > > So, the question is: shall we remove all the metadata if the packet is not > safe ? Suggest to disable if this doesn't cause problems elsewhere (i.e. prevent mix-n-match from occurring). > What to do when two packets are merged (AddAtEnd) and only one of them is > non-metadata-safe ? Suggest to disable and document this as part of AddAtEnd(). > > The actual patch fixes an actual bug, but the above points could come back > sooner or later. Created attachment 2456 [details]
Newest patch
Trying again. This time the changes are (almost) only to PacketMetadata.
If the approach is ok, the only thing missing is to modify the metadata tests to check this new functionality.
|