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

[Jeff Morriss <jeff.morriss@xxxxxxxxxxx>]

Sebastien Tandel wrote:
> 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. :-/
>   
It seems to have the right semantics but...

> > 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.
>   
OK, so I've checked in your original change (with some additional 
comments) in revision 20140.  Using in_error_pkt would just run the code 
for IPv6 too which wouldn't be good.