[Ns-developers] icmpv4 stack for merging

[Tom Henderson]

Mathieu Lacage wrote:
> hi,
> 
> Since
> http://mailman.isi.edu/pipermail/ns-developers/2008-September/004751.html
> 
> I spent a while debugging this icmp stack and running it through quite a
> lot of testing. I made a couple of changes here and there, mostly to
> avoid generating icmp messages for multicast packets to avoid icmp
> storms. So far, the code has been reasonably stable so, this is a
> request for merging to craig and raj.
> 
> Two patches attached:
>   - icmp.patch contains all the changes for the src/node and
> src/internet-stack directories. 
>   - icmp-application.patch contains the ping application and the example
> code.
> 
> It would be helpful to review the first patch again. The second patch is
> both less risky and less intrusive.

Mathieu,
I read through your patches.  A few comments:

- I would suggest to rename SocketMtuDiscoverTag to 
SocketSetDontFragmentTag or something like that, so it is more explicit 
what it is functionally doing.

- likewise, in the patch, setting the UdpSocket MtuDiscover attribute 
currently just sets DF in the outbound direction but doesn't actually 
perform path MTU discovery AFAICS (it doesn't look like anything is done 
with a received DestUnreach)

I think the behavior you want to implement, if MtuDiscover attribute is 
set, is the following:
i) set the DF bit
ii) keep a member mtu variable around that is initialized to the 
outgoing interface MTU
iii) update this local mtu variable upon receipt of ICMP messages
iv) if (p->GetSize > mtu) then set EMSGSIZE errno

Otherwise, I think it looks good.  Although the ipv4-interface changes 
will have to be updated with the forthcoming ipv4 changes, I don't think 
anything needs to wait on the merge for ns-3.3.

- Tom