Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 7902] Improved Dissection of Modbus/TCP messages and added support for standar

[bugzilla-daemon@xxxxxxxxxxxxx]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7902

--- Comment #29 from Chris Bontje <cbontje@xxxxxxxxx> 2012-10-24 14:18:57 PDT ---
(In reply to comment #28)
> (In reply to comment #26)
> > I'm not quite
> > clear as to where the modbus_request_info_t structure is defined (and how new
> > components like register_addr_type, etc got added); is it a dynamic data
> > structure we can just assign elements to?  
> 
> It's data associated with a protocol for a specific frame. It's one of the ways
> to pass data between dissectors (within the same frame).  If the Modbus
> dissector sees "context data" for its protocol, its acts accordingly, otherwise
> it defaults to "RAW, UINT16 registers".  You would add additional members to
> the modbus_request_info_t structure (in packet-mbtcp.h) if you felt the Modbus
> dissector needed more "context" from upper layer protocols (like Modbus/TCP and
> RTU).  This is different than the "conversation data" you'll need to match
> request/response.
> 
Understood - I'll take your markups and tackle the conversation data
implementation for Modbus/TCP, only in the case of Transaction ID's being
non-zero (to eliminate both our transID crossing over and "always-zero" problem
cases).  The example cases seem straight-forward enough and it would certainly
make things more robust.

> 
> > I was also considering going through
> > the filter names and adjusting things to split out the different categories - 
> 
> I tried to complete that thought, but maybe you were looking for more filters
> specific to each function code?  I waiver on how much the "modbus.data" filter
> should be used.  It's easier to write a filter for "modbus.data" == <some
> bytes> then modbus.holdingregister.data == <some bytes> ||
> modbus.inputregister.data == <some bytes>, etc, but the "modbus.data" filter
> also returns more results (and you can include the function code as part of the
> filter)
> 
I think the filtering as-is is pretty effective.  At the most, (in the real
world) it would be used to drill down on a specific unit ID and function code
(to pick out holding register reads, a coil set, register write, for example). 
Beyond that, I don't know that picking out actual register or coil values would
be as important.  Many times the contents of the registers themselves are less
important than confirming the command/write operation actually took place.

> > With Modbus RTU (at least) we can be guaranteed that there will be no
> > situations where two requests are issued sequentially; normal conversations
> > will always go poll->response, poll->response, etc.  
> 
> Wireshark can be used to troubleshoot when "conversations" aren't normal (like
> missed responses or duplicate responses), and you would hope the dissection
> would be as accurate as possible. Dissectors aren't written for just the
> "happy" case.
> 
> In that case, with a
> > response, couldn't we reference the packet context fields to simply step
> > backwards through the packet lists (from our current point), find the last
> > query from current dst addr, and take the reference_number base from there? 
> > This would just require us to store and then reference the reference_number -
> > or am I misunderstanding the capabilities of the packet context functionality?
> > "Transaction ID" only applies to Modbus/TCP messages and wouldn't to Modbus RTU
> > so we'll need a solution for both.  I've also got captures from systems where
> > the Transaction ID is zero, as you noted above so we aught to not use that. 
> > The Modbus/TCP systems I've worked with will typically still follow the same
> > poll->response pattern from the Modbus RTU world, do you have captures where
> > two different polls are issued sequentially?  I'll attach a typical capture
> > that I see, it has a master polling a slave with 4 different holding register
> > polls, each one seems to be split?
> 
> Once you are on TCP (and not a serial bus) you have the ability to have
> "multiple conversations" in a single capture.  So you could have 1 master
> polling 4 different slaves concurrently (with some of the requests being
> "outstanding" while other responses come in) or 4 separate masters each polling
> their own 1 slave.  This is where having a global varible that assumes a
> single, sequential "conversation" is dangerous.
> 
> You are also not guaranteed after the "first pass" over each frame that a
> dissector will access frames in sequential order.
> 
> Unfortunately, I just think matching request/response on the wire is a
> limitation of the protocol it terms of being able to do it deterministically.

Understood - for Modbus RTU I still want to leave in the register "indexing"
(just as a simple register counting mechanism to step through the packets), but
I'll just have it baseline to zero since we can't necessarily guarantee the
source of the actual offset/base.

Chris

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