[Click] set_ip_header semantics

[Eddie Kohler]

Hi Bart,

Bart Braem wrote:
> Students were doing exercise where they had to change an IP header of  
> a packet. What they did was use ip_header to get the current  
> ip_header, take a copy of that struct, modify it and then set it with  
> set_ip_header. And that did not work.
> 
> We were also confused in the beginning, until we realised the  
> semantics of set_ip_header: it only sets the pointers and it does not  
> copy anything. In fact, do we understand correctly that the  
> set_ip_header should be treated as an annotation function and that the  
> pointer set with it should lie within the packet boundaries?

That's exactly correct.  In fact, since commit 
f03d536cc8392f673fb90da298ebbacb77eb3ca1, we assert that the pointer is in range.

> We suggest to update the network header methods to better reflect  
> these semantics.
> We could introduce naming like set_ip_header_anno.
> But we also suggest to change the call itself and use an offset  
> parameter instead of a pointer to the data. This way confusion is  
> impossible because it is clear that the network_header methods work on  
> in-packet data.
> 
> What do you think?

I see what you mean, but don't you think the assertion is enough?  Perhaps a 
name change to _anno() would be useful, but set_ip_header() is very well 
established by this point.  My tendency is to leave things as they are.

Eddie


> Regards,
> Bart and Michael