[Click] Packet::clone() potential problem

[Eddie Kohler]

Hi Barry,

> May I suggest the addition of Packet::copy() similar
> to Packet::clone() except that it uses skb_copy()
> instead of skb_clone(). 
> 
> I was using Packet::clone() and it was causing me
> problems since I was unaware that skb_clone() creates
> an skb that shares the same skb->data as the original.
> Thus, modifing the clone's skb->data before sending it
> to ToDevice was also modifying the original skb->data.
> The addition of Packet::copy() may make other people
> aware of this potential problem. 

Could you tell us how you actually modified the packet's data? It shouldn't
have been via Click Packet methods, since none of Packet's methods provide
access to mutable data (the data() method returns a 'const unsigned char
*', for example). To get mutable data, you need a WritablePacket.
WritablePackets are guaranteed not to be shared. They are created by the
'WritablePacket *Packet::uniqueify()' method, which basically does an
'skb_copy' if necessary.

I'm guessing you used 'skb()' to get a copy of the actual sk_buff. That is
not the "Click way". Keep yourself limited to Packet's methods if you can,
refrain from casting away const, and you won't run into this problem.

Sharing of Packets is described in the user manual (click.info); HTML at
file:///home/kohler/www-lcdf/clickweb/doc/progman.html#Packet%20Sharing .

You can get the effect of "copy()" with "WritablePacket *q =
p->clone()->uniqueify()". Nevertheless, if several people find that too
verbose, we could provide a Packet::copy() that did exactly that.

Eddie