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

[Sebastien Tandel <sebastien@xxxxxxxxx>]

Hi Jeff,


Jeff Morriss wrote:
> 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.
>   
I didn't see the in_error_pkt.  I think it would probably work but did
you grep in_error_pkt = TRUE ? I don't know the semantic of the others
dissectors using this field. :-/

> 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.
>   
The ICMPv6 is built after receiving your TCP segment ... we can guess
that (at least) the tcp seq will be reported as you will put in the
ICMPv6 packet as much data as there are in the original packet without
exceeding the IPv6 MTU and the ICMPv6 header is not so long (64bits). In
the current code of the tcp dissector, we have to read 10 bytes more to
add tcp seq to the tree. The minimum MTU for IPv4 equipments is of 68
bytes (ip header included) ... I then guess that we will show the tcp
seq as if in normal conditions.

>> 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.
>
> _______________________________________________
> Wireshark-dev mailing list
> Wireshark-dev@xxxxxxxxxxxxx
> http://www.wireshark.org/mailman/listinfo/wireshark-dev
>   

Regards,
Sebastien Tandel