Reviewer: Susan Hares Review result: Ready with Issues I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at <https://wiki.ietf.org/en/group/gen/GenArtFAQ>. Document: draft-ietf-manet-dlep-credit-flow-control-?? Reviewer: Susan Hares Review Date: 2024-08-12 IETF LC End Date: 2024-08-06 IESG Telechat date: Not scheduled for a telechat Status: Ready with Issues Summary: Referenced document (draft-ietf-manet-credit-window-07) points to unresolved issues and was declared dead by IESG. It is unclear how these issues were resolved in this draft. The credit window scheme is an important addition to both physical and virtual queues. I commend the authors and contributors for their work. Appendix A is appropriate. Major issues: The concept of credit-based flow control messages aligns with the basic concepts of the DLEP protocol [RFC8175] in a virtual or physical queue. This draft follows the concepts of credit window grant, credit window status, and credit request in draft-ietf-manet-credit-window/ Interestingly, draft-ietf-manet-credit-window-07 was declared dead by the IESG. The key points below from the TSVart and OPS-DIR reviews do not seem to have been resolved or listed in this draft. Details on Major issue: The two interesting points of the IESG evaluation for the concept found in draft-ietf-manet-credit-window are in the TSVart review (https://datatracker.ietf.org/doc/review-ietf-manet-credit-window-07-tsvart-telechat-scharf-2016-12-14/) and the OPS-DIR Review (https://datatracker.ietf.org/doc/review-ietf-manet-credit-window-07-opsdir-lc-schoenwaelder-2016-12-12/). The TSVart review of draft-ietf-manet-credit-window-07 states: "It is a bit surprising that the document does not discuss potential challenges and downsides of the flow control scheme. For instance, if the RF link capacity rapidly changes, what is the benefit of this flow control?" Sue's comment: Since the modem types are not specified, it could be RF modem, cable modem, wired modem, or virtual modem. It seems this comment is still relevant. Sue's suggested resolution: It seems the manageability section (2.4) gives several generic hints, but it might be wise to create an application document with more hints for operational deployment. The OPSDIR review of draft-ietf-manet-credit-window-07 states: a) "Section 9.2 discusses that knowledge about credits may get inconsistent. Mismatches may also include situations where packets are lost or dropped (e.g. checksum failures) - should this be mentioned as well?" d) "In the security considerations, I read 'The extension does not introduce any additional threats above those documented in [DLEP].' I wonder whether this is correct. Unless I have a secure transport for DLEP, can I not rather easily inject false window sizes to say prevent communication or to let a router overflow a modem?" Sue's comment: Different modems have different loss rates depending on physical media and modem functions. It seems that comment (a) is applicable to this specification. On comment b), RFC8175 states: "Implementations of DLEP SHOULD implement, and use, Transport Layer Security (TLS) [RFC5246] to protect the TCP session. The "dedicated deployments" discussed in "Implementation Scenarios" (Section 4) MAY consider the use of DLEP without TLS. For all "networked deployments" (again, discussed in "Implementation Scenarios"), the implementation and use of TLS are STRONGLY RECOMMENDED. If TLS is to be used, then the TLS session MUST be established before any Messages are passed between peers. Routers supporting TLS MUST prioritize connection points using TLS over those that do not." RFC8175 strongly recommends TLS to protect against the issues denoted in (b). draft-ietf-manet-manet-credit-window does not seem to emphasize this point in the security section. This document (draft-ietf-manet-dlep-credit-flow-control-15) also does not emphasize this point from RFC8175. In addition, the security comments in draft-ietf-manet-dlep-credit-flow-control-15.txt in the security section state: "These mechanisms expose vulnerabilities similar to existing DLEP messages, e.g., an injected message resizes a credit window to a value that results in a denial of service." The use of "e.g.,..." is unclear and further weakens the clarity of the point on TLS requirements from RFC8175, and the chance to expose vulnerabilities of RFC8175. Sue's resolution: If the authors rewrite the security section to contain clear language, it will go a long way to resolving these concerns. Manageability and security sections are like warning labels on tobacco products, it is necessary to warn people if they are doing something dangerous to one's health and well-being. Minor issues: Nits/editorial comments: 1. 7 places in the text use the grammar form "i.e.," to either the detriment of the clarity of the text or a misuse of the grammar form. place 1: section 2, paragraph 2, sentence 3, place 2: section 4, paragraph 4, sentence 3, - This use is closer, but I feel the text would be better if old text:/e.g., DSCPs,/ was replaced with: new text:/(for example, DSCPs) place 3: section 2.1, paragraph 1, sentence 3 old text: (e.g., framing, headers and trailers)/ Do you mean "framing, headers, and trailers" as a combination or "framing, headers, or trailers" as individual examples. Place 4: 2.3.1, paragraph 1, sentence Old text:/ In order to avoid errors caused by use of undefined FIDs or uninitialized credit windows, this Data Item SHOULD be included in any Session Initialization Response Message that also indicates support for an extension that requires support for the credit window control mechanisms defined in this document, e.g., see [I-D.ietf-manet-dlep-da-credit-extension]. Text that is confusing: /that requires support for the credit window control mechanisms defined in this document, e.g., see [I-D.ietf-manet-dlep-da-credit-extension]./ Comment: It appears you might have smashed two sentences together. The example is unclear given the lack of clarity of the preceding text. place 5 + 6: Section 2.4 - First and last sentence. place 7: Section 4, sentences 2. See my comment above in the major comments about the unclarity of the "e.g.," text. Since clarity in the manageability and security sections is important for this document, I suggest avoiding the use of "e.g.," text in places 5, 6, andf 7. 2. section 2.3.1, paragraph 5 (last paragraph), last sentence, lacks sentence clarity Old text:/ The modem SHOULD NOT issue additional credits for each affected FID until the associated affected Window has drained to be less than the new Credit Window Max Size, regardless of whether sufficient draining occurs before or after the modem receives that corresponding Session Update Response Message./ text that is difficult to parse: /after the modem receives that corresponding Session Update Response Message./ suggestion: I would suggest just rewriting the sentence into 2 parts (if possible). I had to re-read this multiple times. I think the algorithm is fine, but the sentence struggles. 3. section 2.3.2, problem grammar (commnets). Old text:/ Once the traffic classification information is located, the router MUST ensure that any data plane state, see Section 2.1, that is associated with the TID and its corresponding FIDs is updated as needed. / suggested revision-1:/ Once the traffic classification information is located, the router MUST ensure that any data plane state (per Section 2.1) that is associated with the TID and its corresponding FIDs are updated as needed. / alternate revision-2:/ Once the traffic classification information is located, the router MUST ensure that any data plane state that is associated with the TID and its corresponding FIDs are updated as needed (per Section 2.1). / -- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
