Bug 624

changeset = 2dddadf4248c

Thanks,
Wilson

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

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/﷒0﷓ 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.

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 ().

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 ().
> 

(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.

(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".

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 ?

(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.

(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.

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