Modbus should accept multiple ADUs per TCP frame

[Abestanis]

I have a Modbus client that is sending multiple commands with the click of a button to the Modbus server of OpenPLC. Sometimes, some of the last Modbus commands are not executed by OpenPLC.

Details

According to the Modbus TCP specification, a Modbus message is the combination of exactly one ADU per TCP frame, which contains exactly one PDU.

Screenshots from the spec

 

My client is a simple nodejs program that runs on a Windows machine and uses modbus-serial to communicate with the OpenPLC Modbus server. And unfortunately, my client is sometimes sending multiple ADUs per TCP frame.

Screenshot of Wireshark capture


Why this should be fixed in OpenPLC and not on the client

Now I know the OpenPLC Modbus server is conforming to the Modbus specification and it's the client that is not conformant (there is even an open but stagnant issue), but there are multiple reasons why I think this should be handled on the OpenPLC side:

1. The spec is not clear. On the one side, it specifically forbids multiple ADUs in one TCP frame:

   A TCP frame must transport only one MODBUS ADU.

   But then in the next sentence it seems to allow the possibility of sending multiple "Modbus requests" in one "TCP PDU":

   It is advised against sending multiple MODBUS requests or responses on the same TCP PDU.

   I'm pretty sure a "TCP PDU" is a TCP frame. I initially thought a "MODBUS requests" request refers to a Modbus PDU, but the possibility and framework of having multiple PDUs per ADU is mentioned nowhere else in the spec, so I believe a "MODBUS requests" should be a ADU, which seems to directly contradict the first sentence.

2. The requirement to have only one ADU per TCP frame is incredible hard to implement on a system with an operating system that has a TCP stack. TCP is first and foremost a streaming protocol with no real notion of packet boundaries, framing is an implementation detail that is not exposed to the layer above. None of the operating systems provide enough control over the framing process to guarantee not merging frames, so the only option is to bypass the TCP stack completely and use some form of raw socket, but this means the client has to implement most parts of the TCP protocol themselves, which seems unreasonable.
3. The OpenPLC Modbus server is actually also not spec compliant because it has exactly the same issue when responding to requests. Since the server is just using a regular socket, packets can be merged by the underlying operating system. Also, while the read seems to return the data one TCP frame at a time, there is actually no guarantee about that.
4. I think OpenPLC should follow the robustness principle "be conservative in what you send, be liberal in what you accept". Now we can't easily fix the sending part, but we can be more lenient in what we accept.
5. In addition to the nodejs library I tested two other Modbus client implementations (uModbus (Python) and libmodbus (C)) that are directly linked from the official Modbus website and I found that they also suffer from the same problem and can send multiple ADUs per TCP frame. I didn't invest the time to check the other implementations linked on that page, but I suspect they are not conformant too. So any client using these implementations would have this problem with OpenPLC.

Proposal

The Modbus implementation of OpenPLC should treat the incoming data as a stream and ignore TCP frame boundaries. We can use a ringbuffer to archive this. When more data becomes available, the server should try and execute multiple ADUs from the buffer until there is not enough data in the buffer to parse the next Modbus ADU.

I am willing to work on a PR if you agree with this path forward.

[thiagoralves]

Thanks for the info @Abestanis. Feel free to open a PR to fix this behavior. As long as it keeps working with devices that follow the specification, I'm fine with it.