[rddp] Last call comments on MPA draft

MPA authors,

I am a little bit late for the last call comments (by 12 hours). So, my apologies for that! In any case, below are my comments on draft-ietf-rddp-mpa-02.txt:

1. Section 1.2, Page 7: Bullet 3 and Bullet 4 use terms “DDP data” and “DDP data chunks”. These are not defined terms. They should be replaced with “DDP Segments” that is defined in the DDP spec.

2. Section 1.2, Page 7: Bullet 6 refers to ULPDU_Length that is not defined at this point in the spec. Suggest using “MPA header” instead of “ULPDU_Length” here. Also, after PAD add the following “(if any)”.

3. Section 1.2, Page 8: Figure 1 says “IP etc.” at the lowest layer. Suggest removing “etc.” from the text as rddp protocols are designed for IP networks so the lowest layer in Fig 1 is “IP”.

4. Section 2, Page 10: FPDU Alignment and Header Alignment definition say “TCP segment begins with an FPDU”. TCP segment includes TCP header and TCP payload, so TCP segment always begins with TCP header. Suggest replacing text “TCP segment begins with an FPDU” with “TCP segment payload begins with an FPDU”.

5. Section 2, Page 10: MPA-aware TCP definition last line says “…TCP segments that begin with an FPDU”. Suggest replacing it with “… TCP segments with the payloads that begin with an FPDU.”

6. Section 2, Page 10: MPA definition says “Marker-based ULP PDU Aligned Framing for TCP protocol”. This is not consistent with the draft title. Suggest to use “ MPA – Marker PDU Aligned Framing for TCP” for the first sentence.

7. Section 3.1.1, Page 12, first paragraph: This section talks about the TCP transmit side. The last line in the first paragraph ends with “…. the DDP segments (FPDUs) posted for transmission.”. Suggest rephrasing the last part to “… FPDUs containing DDP Segments passed for transmission.”

8. Section 3.1.1, page 12, last paragraph, first sentence ends “…. TCP will use 1460 octet segments in creating FPDUs.” TCP does not create FPDUs, it is MPA that creates the FPDUs. Suggest rephrasing the end of the sentence.

9. Section 4, page 14, last paragraph: Last sentence say “The 16-bit “ULPDU_Length” field is large enough to support the largest IP datagram for IPv4 or IPv6.” ULPDU_Length supports FPDUs not IP datagrams. Suggest rephrasing the last sentence to “The 16-bit ULPDU_Length field is large enough to support the largest ULPDU that can fit in the largest IP datagram for IPv4 and IPv6.”

10. Section 4, Page 15: CRC definition need to have a reference to CRC32C algorithm when it is first used in the paragraph.

11. Section 4.1, page 15, last paragraph: This paragraph is vague in terms of that it does not say where the marker is. Suggest the following rephrasing “ FPDUPTR: The FPDU pointer is a relative pointer, 16-bits long, interpreted as an unsigned integer, that indicates the number of octets from the “ULPDU_Length” field of the FPDU that contains the marker to the first octet of the entire marker.”

12. Section 5.2, page 20: Figure 5 uses hexadecimal every where except for the size of Send Data. Suggest “Send Data (0x18 octets of zeros)”. Use “0x18” instead of “24”.

13. Section 5.3.2, page 23: When a marker falls between two FPDUs, then is it considered in the FPDU size calculation or not? My belief is it is not considered in the FPDU size. The specification neither says that nor clarifies that. I believe this section should specify the FPDU size calculation for that case.

14. Section 5.4, page 24: This section seems to suggest that CRC has to be valid for detecting FPDU start using one of the methods described in 1, 2, or 3. If the CRC is turned off, then which one of these methods should be used to detect the start of the FPDUs? Do all of them apply or a sub-set of them apply? The spec does not specify how to detect start of FPDUs when the CRC is turned off. If all the methods apply when the CRC is turned off, then add a note saying “There is no requirement to have valid CRCs for detecting the start of FPDUs using the above methods when the CRC is turned off.”

15. Section 6.1, page 27: The rules for MPA connection startup phase should go after the definition of “MPA Request frame” and “MPA Reply Frame”. The reason is the rules described use the fields of the MPA request and reply frames and these fields are defined after. Suggest moving section 6.1.1 before the rules.

16. Section 6.1.1: I think we need to add a note here to say “When markers are not enabled, then the MPA SHOULD NOT enable DDP to recover out-of-order segments.” Currently, the spec does not say that when the markers are not enabled, then out-of-order segments are not placed.

17. Section 6.1.1, M bit description: Last sentence in this paragraph needs rewording. Suggest “When both “MPA Request Frame” and “MPA Reply Frame” have M bit set to 1, then the markers MUST be added by both peers.”

Hemal