Wireshark · Wireshark-dev: Re: [Wireshark-dev] [PATCH] bugfix : ICMP unreachable and tcp seq not shown

[Jeff Morriss <jeff.morriss@xxxxxxxxxxx>]

Sebastien Tandel wrote:
> I am not sure it's broken ...
>
> ICMP and ICMPv6 are rather different ...
> - ICMP states that you have to put the IP header + 64 bits of data
> - ICMPv6 RFC states, and I quote, you have to put
> "As much of invoking packet as will fit without the ICMPv6 packet
> exceeding the minimum IPv6 MTU [IPv6]"
>
> IPv6 MTU may vary ... but should certainly include the TCP seq number.
> For that field, IMHO, I think we are safe.

Sorry, I guess I wasn't clear.  Your code will not show the sequence 
number in IPv6 because you're searching for the string "icmp:ip" whereas 
in IPv6 it'll be "icmpv6:ipv6".

Anyway, I think a better way for the patch to work would be to check the 
"pinfo->in_error_pkt" field (set to TRUE by ICMP before calling the IP 
subdissector).  I'll try that tonight.

However, you raise an interesting point for IPv6: what if there's enough 
TCP in there that the regular TCP dissection puts (again) the sequence 
number in the tree?  I don't know what the chances of that are.

> Nevertheless, if you want *all* the potential fields, wireshark is not
> since IPv6 MTU is not a *fixed* parameter. Therefore the solution would
> be to do the check for every item which is not added directly to the
> tree. I don't know if it has a real interest ... it will probably mess a
> little bit more the code of the TCP dissector.

True, probably not worth the effort.