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.