Wireshark-bugs: [Wireshark-bugs] [Bug 7902] Improved Dissection of Modbus/TCP messages and added
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7902
--- Comment #17 from Chris Bontje <cbontje@xxxxxxxxx> 2012-10-23 07:36:22 PDT ---
(In reply to comment #14)
> Most of the dissection code you've added is not in the official Modbus/TCP
> specification
> (http://www.modbus.org/docs/Modbus_Application_Protocol_V1_1b.pdf). I know
> this particular dissection is used out in the real world, but do you have any
> specifications that document it?
There isn't really any "standard" documentation beyond the normal 16-bit
integer format of the registers that is defined by the documentation on
Modbus.org. All manufacturers implementing Modbus usually provide their own
documentation outlining the various "register types" they have implemented;
this documentation would typically indicate whether a particular address range
contains 16bit INT/UINT, 32-Bit INT/UINT, Float, etc - much more customization
beyond these somewhat "rudimentary" data-types is possible. I'll attach a
> I had modified the Modbus/TCP dissector to be broken up into 2 separate
> dissectors - Modbus/TCP + "Modbus" (PDU). The intention of this was that
> "Modbus" could be dissected with any "encapsulation" layer, be it Modbus/TCP,
> Modbus RTU or another dissector (like CIP)).
>
> The Modbus/TCP dissector dissects the MBAP Header and then sends the dissection
> to the "Modbus" dissector. If a Modbus message were coming over RTU/serial, I
> would expect a third dissector. The "Modbus RTU" dissector would dissect the
> Unit ID and then pass the data on to the "Modbus" dissector. I don't think
> there should be preferences for a "serial" protocol mixed with the TCP protocol
> that is Modbus/TCP. You also then have the freedom to better guess the "packet
> type" and pass it into the proto data like Modbus/TCP does. Because the
> dissector would be very small, I don't think it warrants a new file, but I
> think it should at least be separated out from Modbus/TCP.
Now that you mention it, I agree. It would be best for protocol documentation
purposes to leave the Modbus/TCP dissector as-is, augment the existing "Modbus"
dissector with the register payload decoding, and add a 3rd dissector for
Modbus-RTU (to handle the leading address byte, normal "Modbus"+Payload, and
CRC-16 Footer).
Modbus RTU, while being a serial protocol at it's core, is quite frequently
seen on Ethernet networks due to wide-spread usage of serial Port Servers and
wide-area-SCADA networks (such as Satellite, Ethernet radios, etc). The only
problem with "Modbus RTU" as a standard dissector would be that there is no
default port number - not really a big deal, do you think the dissector could
share the same user-configured port number as the Modbus/TCP dissector below?
> I like the TCP port preference addition as I know port 502 is reserved by IANA,
> but any port could be (and is) used. Again, however I don't see explicit
> mention of the use of UDP in the Modbus specification. It was in previous
> versions of Wireshark, but I intentionally removed for lack of specification
> (see bug 5923).
I have no issue with the removal of UDP, I have never specifically seen
Modbus/TCP (or Modbus RTU) tunneled over UDP packets. I found the
dissect_mbudp function still present when I got the latest SVN; is it just an
orphan function that never gets called?
> I like the "cleanup" of the field names, but your patch overall doesn't confirm
> to the whitespace formatting (modelines) of the rest of the dissector.
Not sure exactly what you're referring to; do you mean "space vs. tab"
white-space here? Which would you prefer I follow to match the pre-existing
driver format?
Chris
--
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
