openlcb / documents

The OpenLCB specification: standards, recommended practices and other documentation.
3 stars 7 forks source link

Tcp TN Draft: Flag bit discussion is wrong. #12

Closed RobertPHeller closed 4 years ago

RobertPHeller commented 5 years ago

In reading TcpTransferTN and TcpTransferS, I have found a somewhat major error:

TcpTransferS talks about the flag word at the start of a OpenLCB Tcp message from the high bit to the low bit, briefly:

8000 -- OpenLCB/Link 4000 -- Chain 3000 -- Zero 0C00 -- Multipart: 0b00 single, 0b01 first, 0b11 center, 0b10 last 03FF -- Zero

But TcpTransferTN talks about these bits in the opposite order (page 2, Section 2.3, the yellow text starting around line 35 though line 45). The whole yellow text section needs to be rewritten.

I would propose something like this:

Flags field.

The MSB 0x8000 is an expansion flag used to (eventually) add link control messages. These might be needed for e.g. turning gateway-filtering on and off, loading/resetting remote diagnostic information, etc.

The 0x4000 bit indicates "chaining": Another prefix is next, before the body of the message.

The 0x0C00 bits provide a way of extending a single OpenLCB message across multiple TCP transfers. This might be used to e.g. simplify buffering and framing in CAN-Ethernet gateways. A single OpenLCB message that extends across multiple CAN frames is best accumulated into a single TCP transmission. If that's not possible due to buffering or other issues, it can be sent as multiple TCP transmissions using the 0x0C00 bits. When the bits are zero it is a single part message, when 0x0400 it indicates the first part of the message (there can be only one first part), when 0x0C00, it is a middle part of the message (there can be multiple middle parts), when it is 0x0800 it is the last part (there can be only one last part). The multiple parts must have matching MTIs, Source NIDs, and for addressed messages matching Destination NIDs. The message fragments should be combined in the order received.

dpharris commented 5 years ago

Can we guarantee the order of TCP message-parts? If not, could we add sequence number?

Thanks David

On Thu, Mar 21, 2019 at 8:53 AM Robert Heller notifications@github.com wrote:

In reading TcpTransferTN and TcpTransferS, I have found a somewhat major error:

TcpTransferS talks about the flag word at the start of a OpenLCB Tcp message from the high bit to the low bit, briefly:

8000 -- OpenLCB/Link 4000 -- Chain 3000 -- Zero 0C00 -- Multipart: 0b00 single, 0b01 first, 0b11 center, 0b10 last 03FF -- Zero

But TcpTransferTN talks about these bits in the opposite order (page 2, Section 2.3, the yellow text starting around line 35 though line 45). The whole yellow text section needs to be rewritten.

I would propose something like this:

Flags field.

The MSB 0x8000 is an expansion flag used to (eventually) add link control messages. These might be needed for e.g. turning gateway-filtering on and off, loading/resetting remote diagnostic information, etc.

The 0x4000 bit indicates "chaining": Another prefix is next, before the body of the message.

The 0x0C00 bits provide a way of extending a single OpenLCB message across multiple TCP transfers. This might be used to e.g. simplify buffering and framing in CAN-Ethernet gateways. A single OpenLCB message that extends across multiple CAN frames is best accumulated into a single TCP transmission. If that's not possible due to buffering or other issues, it can be sent as multiple TCP transmissions using the 0x0C00 bits. When the bits are zero it is a single part message, when 0x0400 it indicates the first part of the message (there can be only one first part), when 0x0C00, it is a middle part of the message (there can be multiple middle parts), when it is 0x0800 it is the last part (there can be only one last part). The multiple parts must have matching MTIs, Source NIDs, and for addressed messages matching Destination NIDs. The message fragments should be combined in the order received.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openlcb/documents/issues/12, or mute the thread https://github.com/notifications/unsubscribe-auth/AAg4Sp9PuuQfL5k6if5N2el6dEyAgfsnks5vY6sBgaJpZM4cBxPW .

bakerstu commented 5 years ago

You absolutely can guarantee the order of TCP message parts. If you couldn’t, TCP would be absolutely useless as a protocol, might as well just use UDP.

RobertPHeller commented 5 years ago

It is my understanding that somewhere in the bowels of the Tcp/Ip stack is code dealing with packet ordering. A lot of the sort of low-level stuff that is a pain to deal with when writing drivers for CAN interfaces, is already handled when using Tcp/Ip.

pabender commented 5 years ago

On Mar 21, 2019, at 12:11 PM, David Harris notifications@github.com wrote: Can we guarantee the order of TCP message-parts? If not, could we add sequence number?

TCP guarantees in order delivery of all bytes in the byte stream. The trade off for this guarantee is latency. I.e. if a packet in the middle is late or goes missing on the network ( and has to be retransmitted ) later packets are also delayed.

Paul

balazsracz commented 5 years ago

On Thu, Mar 21, 2019 at 5:11 PM David Harris notifications@github.com wrote:

Can we guarantee the order of TCP message-parts? If not, could we add sequence number?

we can guarantee bytes come ordered, but nevertheless there is already a sequence number in the TcpTransferS.

Thanks David

On Thu, Mar 21, 2019 at 8:53 AM Robert Heller notifications@github.com wrote:

In reading TcpTransferTN and TcpTransferS, I have found a somewhat major error:

TcpTransferS talks about the flag word at the start of a OpenLCB Tcp message from the high bit to the low bit, briefly:

8000 -- OpenLCB/Link 4000 -- Chain 3000 -- Zero 0C00 -- Multipart: 0b00 single, 0b01 first, 0b11 center, 0b10 last 03FF -- Zero

But TcpTransferTN talks about these bits in the opposite order (page 2, Section 2.3, the yellow text starting around line 35 though line 45). The whole yellow text section needs to be rewritten.

I would propose something like this:

Flags field.

The MSB 0x8000 is an expansion flag used to (eventually) add link control messages. These might be needed for e.g. turning gateway-filtering on and off, loading/resetting remote diagnostic information, etc.

The 0x4000 bit indicates "chaining": Another prefix is next, before the body of the message.

The 0x0C00 bits provide a way of extending a single OpenLCB message across multiple TCP transfers. This might be used to e.g. simplify buffering and framing in CAN-Ethernet gateways. A single OpenLCB message that extends across multiple CAN frames is best accumulated into a single TCP transmission. If that's not possible due to buffering or other issues, it can be sent as multiple TCP transmissions using the 0x0C00 bits. When the bits are zero it is a single part message, when 0x0400 it indicates the first part of the message (there can be only one first part), when 0x0C00, it is a middle part of the message (there can be multiple middle parts), when it is 0x0800 it is the last part (there can be only one last part). The multiple parts must have matching MTIs, Source NIDs, and for addressed messages matching Destination NIDs. The message fragments should be combined in the order received.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openlcb/documents/issues/12, or mute the thread < https://github.com/notifications/unsubscribe-auth/AAg4Sp9PuuQfL5k6if5N2el6dEyAgfsnks5vY6sBgaJpZM4cBxPW

.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openlcb/documents/issues/12#issuecomment-475296155, or mute the thread https://github.com/notifications/unsubscribe-auth/ADjCCrpRwrjMm5yf_ArcJdwDyowBepR-ks5vY68ggaJpZM4cBxPW .

RobertPHeller commented 5 years ago

Is there any reason not to update the TcpTransferTN discussion of the flag bits to match what is in TcpTransferS? Riight now, the two documents are in disagreement about the flag bits and I suspect it is just a matter of typos in the TcpTransferTN document. Correcting that should be a trivial issue.

kiwi64ajs commented 5 years ago

Sounds good, thanks for doing this

Alex

On 3/07/2019, at 5:20 AM, Robert Heller notifications@github.com wrote:

Is there any reason not to update the TcpTransferTN discussion of the flag bits to match what is in TcpTransferS? Riight now, the two documents are in disagreement about the flag bits and I suspect it is just a matter of typos in the TcpTransferTN document. Correcting that should be a trivial issue.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/openlcb/documents/issues/12?email_source=notifications&email_token=AB5Y53MJAY3AL2ZBYAT6JSDP5OE7PA5CNFSM4HAHCPLKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZB7HTQ#issuecomment-507769806, or mute the thread https://github.com/notifications/unsubscribe-auth/AB5Y53IYCXZ27WCA6ZSJUBDP5OE7PANCNFSM4HAHCPLA.

kiwi64ajs commented 4 years ago

Has this been resolved with the PR here: https://github.com/openlcb/documents/pull/15 ? If yes then this can probably be closed unless there is some residual testing etc needing to be done

RobertPHeller commented 4 years ago

Yes, before Balazs refactored the repo, he merged the pull request. There isn't anything to test, it is just fix to the TN doc.