Ethereal-dev: RE: [Ethereal-dev] Re: [PATCH] Order of subdissectors : suggestio n of a trick
Behold :)
Your fix will work for all TCP conversations captured from the initial SYN
onwards. Others won't work. (You stated this in your remarks too.)
The patch also assumes that a TCP Server always hosts an upper-layer server
too, which is not always true: the OTA-HTTP push mechanism defined in WAP
(now OMA) provides a scenario where a TCP Server hosts an HTTP Client :)
I'd prefer having the possibility to flag troublesome conversations and let
the end-user provide the correct dissection for those odd cases. This
however still requires some thinking :)
Regards,
Olivier
|-----Original Message-----
|From: metatech
|
|
|Hi list,
|
|>I wrote a small patch (about 20 lines in 3 files) to include
|the following
|>"trick" :
|>The goal is that the "tcp.port" field is only matched for the
|server port,
|>not for the client port. The only moment when the client and
|server behave
|>differently is during the TCP handshake : the client sends a
|SYN without
|>ACK, what the server never does at any moment (I think). For
|such a packet
|>the destination is known to be the server. At that moment the
|server port
|>needs to be stored in a new field of the "tcp_analysis" struct (in
|>packet-tcp) that holds a state of the TCP conversation. If
|the server port
|>is known by this trick, the dissector_try_port() is only
|called for the
|>server port, otherwise the normal code is executed (lowest
|port chosen).
|>
|>I think this makes things better in most cases.
|>
|>Limitations :
|>- does not work if the capture was started after the TCP
|connection was
|>started.
|>- might break a dissector that relies on the TCP port being a
|known value
|>for the client (does such a thing exist ?)
|>- a small speed overhead to retrieve the TCP conversation
|state on each
|>packet (actually this could be defined as a preference).
|
|As announced a month ago (!), I implemented the patch so that
|the TCP port
|detection is a bit more reliable if the TCP SYN was part of
|the conversation.
|
|I changed 3 files :
|epan/packet_info.h
|epan/packet.c
|packet-tcp.c
|
|Remarks :
|- Since the (sub-)sub-dissector might also be interested in
|knowing what is
|the server port, I put it into the packet_info structure.
|- In the packet-tcp.c I am not sure of where the check "if
|(tcp_mark_server_port)" should be. Maybe it should be a bit
|further down
|in that function ?
|- If the TCP SYN was not part of the conversation, the normal
|logic applies
|: the lowest port is tried.
|
|Patch against CVS tarball of 15/5/2004.
|
|Any comments are welcome,
|
|See you,
|
|metatech
|