Wireshark-bugs: [Wireshark-bugs] [Bug 7902] Improved Dissection of Modbus/TCP messages and added
Date: Tue, 23 Oct 2012 08:08:57 -0700 (PDT)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7902

--- Comment #19 from Michael Mann <mmann78@xxxxxxxxxxxx> 2012-10-23 08:08:56 PDT ---
(In reply to comment #17)
> (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 relook at the code when you submit the updated patch.  I didn't
necessarily object to the theory behind it, I was just tentative due to the
lack of an official "standard".  I know there are companies besides Schneider
Electric that have adopted similar derivations of the official standard.

> 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. 

Are you talking:
1. Modbus RTU over "raw" Ethernet (not TCP/IP)?  If that's the case the Modbus
RTU dissector would need an different/additional hook into the Ethernet frame,
not a port number.  It appears some of the captures provided display this
behavior.  I would differ to others as to the best way to do that.

2. Modbus RTU over TCP/IP where Unit ID + Modbus PDU is sent over Port 502
instead of the MBAP header?  I would technically consider this a "violation" as
port 502 is reserved for Modbus/TCP.  From an implementation standpoint I would
create a preference for the "Modbus RTU port", but default it to 0, not 502 to
prevent inadventent collisions.  Users would then have to pick between
Modbus/TCP and Modbus RTU by either changing ports or disabling one of the
protocols.  There is also the possibility of not assigning it a port at all and
just making it available to the "Decode As..." functionality.


> 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?

Yes, dissect_mbudp() should probably be removed.

> > 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?

Yes I was referring to the "space vs. tab" thing.  While Wireshark dissectors
have varied "space vs tab" alignment, you should keep the same alignment with
an already present dissector.

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.