Wireshark · Wireshark-dev: Re: [Wireshark-dev] [PATCH] Improved support for MIPv4

[Sebastien Tandel <sebastien@xxxxxxxxx>]

Hi,

Some quick comments :
   1) please use proto_tree_add_item whenever possible. Don't use  
tvb_get_* if you don't intend to use the retrieved value for another  
purpose than inserting it in the tree.
   2) Why have you changed the length type of some fields  
(icmp.mip.flags, icmp.mip.r, icmp.mip.h, ...)? Do we really want to  
be unable to parse correctly mip captures which contains these fields  
but of previous lengths?
	Whatever the answer is, please propagate the changes when inserting  
in the tree. At least icmp.mip.flags is inserted once in the tree  
with a display length field of 1.

Some questions :
   - Do you have traces you can share? If so, can you put them on  
the captures wiki page?
   - Is there a description on the wiki page?
   - Have you fuzz tested your changes?


Regards,
Sebastien Tandel

On 16 May 2007, at 05:21, Ville Nuorvala wrote:

> Hello,
>
> attached is a patch that adds support for the following RFCs (and  
> RFC-to-be):
>
> RFC 3519 Mobile IP Traversal of Network Address Translation (NAT)  
> Devices
>
> RFC 3543 Registration Revocation in Mobile IPv4
>
> RFC 4433 Mobile IPv4 Dynamic Home Agent (HA) Assignment (including the
> not yet published errata about the message extension being using the
> short message extension format)
>
> draft-ietf-mip4-message-string-ext Mobile IPv4 Message String  
> Extension