Bug 624 - Unable to modify packet tag in RouteInput ()
Unable to modify packet tag in RouteInput ()
Status: RESOLVED INVALID
Product: ns-3
Classification: Unclassified
Component: routing
ns-3-dev
All All
: P1 normal
Assigned To: Tom Henderson
http://groups.google.com/group/ns-3-u...
: api, bug
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2009-07-05 08:43 EDT by wilson thong
Modified: 2009-10-01 05:05 EDT (History)
4 users (show)

See Also:


Attachments
patch for changing Packet::RemovePacketTag (Tag &tag) into a const method (1.32 KB, patch)
2009-07-05 11:17 EDT, wilson thong
Details | Diff
Patch RouteInput () to accept non-const packet (7.87 KB, patch)
2009-07-06 01:27 EDT, wilson thong
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description wilson thong 2009-07-05 08:43:52 EDT
I am doing a tag-based routing. My attempt is to add a tag to a packet in Ipv4RoutingProtocol::RouteOutput (), and the tag is read and modified by Ipv4RoutingProtocol::RouteInput () in all the subsequence hops the packet is traversing.

However, several issues prevent this from happening.
- Ipv4RoutingProtocol::RouteInput () receives Ptr<const Packet> as its input argument. This prevents me from using Packet::RemovePacketTag () to remove the original tag and add back a modified tag into the packet.
- Packet::PeekPacketTag (Tag &tag) only copies the tag to its output argument, instead of returning the reference to the actually tag. Thus, any changes to the "peeked" tag has no effect to the actual tag that is being "peeked".
Comment 1 wilson thong 2009-07-05 09:01:57 EDT
changeset = 2dddadf4248c

Thanks,
Wilson
Comment 2 wilson thong 2009-07-05 11:17:37 EDT
Created attachment 514 [details]
patch for changing Packet::RemovePacketTag (Tag &tag) into a const method

The patch changes the Packet::RemovePacketTag (Tag &tag) method into a Packet::RemovePacketTag (Tag &tag) const method.

Don't know if this is aligned with the design of Packet class.

Thanks,
Wilson
Comment 3 wilson thong 2009-07-06 01:27:49 EDT
Created attachment 515 [details]
Patch RouteInput () to accept non-const packet

This second patch is another option to allow modifying tag in RouteInput () method. This patch changes the RouteInput () signature so that the first argument becomes Ptr<Packet>, instead of its original Ptr<const Packet>

In contrast, the first patch mentioned in http://www.nsnam.org/bugzilla/show_bug.cgi?id=624#c2 changes the Packet::RemovePacketTag (Tag &tag) method to be a const method, so that packet tag is still able to be removed with when the packet is referenced by Ptr<const Packet>.

The two patches SHOULD NOT co-exist together, as the amount of changes is more than necessary. Nevertheless, compilation is success even if two patches are applied together.
Comment 4 Tom Henderson 2009-07-06 18:32:00 EDT
Can you not do a 
Ptr<Packet> p = packet->Copy();
and get a non-const packet to work with?  That is the API convention that I tried to align RouteInput() with.  See, for instance, Ipv4L3Protocol::LocalDeliver ().
Comment 5 wilson thong 2009-07-07 09:58:44 EDT
No problem~! Please give me some time to finish it~ ^^

(In reply to comment #4)
> Can you not do a 
> Ptr<Packet> p = packet->Copy();
> and get a non-const packet to work with?  That is the API convention that I
> tried to align RouteInput() with.  See, for instance,
> Ipv4L3Protocol::LocalDeliver ().
> 

Comment 6 Tom Henderson 2009-09-19 18:11:59 EDT
(In reply to comment #5)
> No problem~! Please give me some time to finish it~ ^^
> 
> (In reply to comment #4)
> > Can you not do a 
> > Ptr<Packet> p = packet->Copy();
> > and get a non-const packet to work with?  That is the API convention that I
> > tried to align RouteInput() with.  See, for instance,
> > Ipv4L3Protocol::LocalDeliver ().
> > 
> 


I'd like to mark this as INVALID by next week unless there is a test case posted.  I do not believe that there is a bug here.
Comment 7 wilson thong 2009-09-20 11:38:32 EDT
(In reply to comment #6)
> (In reply to comment #5)
> > No problem~! Please give me some time to finish it~ ^^
> > 
> > (In reply to comment #4)
> > > Can you not do a 
> > > Ptr<Packet> p = packet->Copy();
> > > and get a non-const packet to work with?  That is the API convention that I
> > > tried to align RouteInput() with.  See, for instance,
> > > Ipv4L3Protocol::LocalDeliver ().
> > > 
> > 
> 
> 
> I'd like to mark this as INVALID by next week unless there is a test case
> posted.  I do not believe that there is a bug here.
> 

I guess this should be an "enhancement bug" rather than a "normal bug".
Comment 8 Mathieu Lacage 2009-09-28 03:13:21 EDT
It was a design decision to not allow users to remove a tag or change the content of a tag in a packet if the packet is const. AddPacketTag is const because existing users of the packet will not be impacted by a new tag added. On the other hand, if you remove a tag which is used by some other users, then, you will impact them. This is why the current class does not have a const RemovePacketTag method. 

So, to summarize, if you want to remove a tag from a packet, you need a non-const packet so, RouteInput should give you a non-const packet.

Tom: I don't believe that your suggestion to call Packet::Copy will work because wilson wants to be able to modify the packet which is being used by the caller. Wilson: am I wrong ?
Comment 9 Tom Henderson 2009-09-28 03:54:10 EDT
(In reply to comment #8)
> It was a design decision to not allow users to remove a tag or change the
> content of a tag in a packet if the packet is const. AddPacketTag is const
> because existing users of the packet will not be impacted by a new tag added.
> On the other hand, if you remove a tag which is used by some other users, then,
> you will impact them. This is why the current class does not have a const
> RemovePacketTag method. 
> 
> So, to summarize, if you want to remove a tag from a packet, you need a
> non-const packet so, RouteInput should give you a non-const packet.
> 
> Tom: I don't believe that your suggestion to call Packet::Copy will work
> because wilson wants to be able to modify the packet which is being used by the
> caller. Wilson: am I wrong ?

I don't believe that the caller is using the packet.  The way that it works is that Ipv4L3Protocol::Receive() calls RouteInput() and then the method ends; there is no further use of that packet copy.  RouteInput later will call the callback to Ipv4L3Protocol::IpForward().  However, it (the routing protocol) can copy the packet to a non-const Ptr, remove the tag, and provide this copied packet to IpForward().

I don't care too much whether the routing system gets a const packet pointer or non-const packet pointer; certainly some routing systems will not need to touch the packet but others may add source routing headers, etc.  I was just trying to follow the API convention that packets are passed as Ptr<const Packet> across the stack layers and the callee does a copy to remove the constness if necessary.

Comment 10 Mathieu Lacage 2009-09-28 03:59:59 EDT
(In reply to comment #9)

> I don't believe that the caller is using the packet.  The way that it works is
> that Ipv4L3Protocol::Receive() calls RouteInput() and then the method ends;
> there is no further use of that packet copy.  RouteInput later will call the
> callback to Ipv4L3Protocol::IpForward().  However, it (the routing protocol)
> can copy the packet to a non-const Ptr, remove the tag, and provide this copied
> packet to IpForward().

Ok, marking the bug as INVALID then.
Comment 11 wilson thong 2009-10-01 05:05:24 EDT
Thanks Tom for teaching me how to modify packets.

To summarize, my desire is to allow modifying packet tags at each hop and the modification can be carried hop to hop from sources to destinations during routing.

I awared that using the Packet::Copy () method can modify a const packet but I didn't aware using callback to call the Ipv4L3Protocol::IpForward() for sending the modified packets to next hops.

Thanks Tom again. I will try using the "Copy () + IpForward () solution" instead of modifying the API.

Thanks,
Wilson