Dynamic Link Exchange Protocol (DLEP) Credit-Based Flow Control Messages and Data Items
draft-ietf-manet-dlep-credit-flow-control-19

Thank you for addressing both my discuss items and my comments.

I was unaware of this work in Manet, and thank you for making an interesting and useful specification. 

Thank you for addressing these concerns in rev-18:

1. The text says: “In the absence of a wildcard, a packet may not match any of the data items and, in this case, SHOULD be dropped by the router.”.- Why is this not a MUST? (If it needs to be a SHOULD, then please add text to explain what happens whene there is no match and explain whether sending this pacekt also consumens credit.)

2. If a packet is not matched and is dropped, ought it to be logged? (It would be logged if there was no FID). Either way, the specification ought to explain whether an entry in the log is to be expected.

3. I don’t think the security mechanisms are sufficiently defined to be useful. Please can you explain what threats these updates are expected to counter. This currently leads to two very ineffective security considerations.

# Gunter Van de Velde, RTG AD, comments for draft-ietf-manet-dlep-credit-flow-control-17

# The line numbers used are rendered from IETF idnits tool: https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-manet-dlep-credit-flow-control-17.txt

# Many thanks to Dhruv Dhody  for the RTGDIR review, and the follow up. 


# Detailed Review
# ===============

400	   Length:
401	      16
402
403	      Per [RFC8175] Length is the number of octets in the Data Item.  It
404	      MUST be equal to sixteen (16).

GV> This field seems slightly underspecified. Maybe point out that the Length is set in multiples of bytes, not bits.
GV> What must happen if the length is not set to value '16'? ignore? send a trap? reset the process? Please advice

496	   Length:
497	      2
498
499	      Per [RFC8175] Length is the number of octets in the Data Item.  It
500	      MUST be equal to two (2).

GV> similar observation as above. 

Kind Regards,
Gunter Van de Velde
Routing Area Director

Hi authors, 

Special thanks to Eric and the WG Chairs for he effort put in the last mile to finalize the specification (and others in the set). 

I only checked diff 18 vs. 17 as I already reviewed for OPSDIR both -15 and -17 (*).

Most of my comments raised in -15 were addressed. I hoped to see more about how to expose
default values/control the configuration parameters out there, but knowing the context of
this spec, I won't insist on this. So, I'm fine with this version to be published.

Cheers,
Med

(*)
  - 15: https://datatracker.ietf.org/doc/review-ietf-manet-dlep-credit-flow-control-15-opsdir-early-boucadair-2024-07-24/
  - 17: https://datatracker.ietf.org/doc/review-ietf-manet-dlep-credit-flow-control-17-opsdir-telechat-boucadair-2025-01-20/

I suspect Deb's DISCUSS.

Note the figure in Section 2.3.1 shows 2 32 bit credit fields but the text describes it as one 64 bit credit field. Please fix the figure.
Looking again, I see a ":" notation that I've never seen used before. I think the following is far cleaner:

    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    |                         Credit Value                          |
    |                                                               |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

This is also done in other sections, eg 2.3.3, 2.3.4, etc


In 2.3.2 the description of field "Data Item Type" should be "Data Item Type (TBA5)".

Flow Identifier (FID): should probably say it is a two octet value ?
I would also use the more common ~ instead of : syntax in the diagrams
to indicate variable length.

Section 4

Since this protocol has a window size of one (only one outstanding msg allowed),
couldn't an enduser/malicious code somehow block this traffic flow from happening
by sending bogus msgs that fill up the queue of size 1? How can an implementation
protect itself against this?

I support Deb Cooley's DISCUSS position.

Thank you to Sue Hares for the GENART review.  From her review, please consider:

** Section 2.4, Management Considerations, would benefit discuss of the "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?"

** Additional operational considerations: "knowledge about credits may get inconsistent. Mismatches may also include situations where packets are lost or dropped (e.g. checksum failures)"

I was going to COMMENT the same as Murray about the use of SHOULDs, so please consider his comment.

Thanks for carrying this document across the finish line. I have one question:

### Section 2.3.1

"Finally, a queue size (in octets) is provided for informational purposes."

Where?

I suggest reviewing several of the SHOULDs in this document.  For instance, there's a cluster of them in the last paragraph of Section 2.3.4.  Since SHOULD presents a choice to an implementer, I could decide to do none of these and still claim I'm compliant with this specification.  Is that the intent?  You might instead want to add some guidance about when you imagine an implementer might legitimately not do what you're saying here.  Or if you can't come up with such guidance, then consider that this really might be a MUST.

First of all, special thanks to David Black for his (as usual) excellent TSVART review and  to the authors for resolving those.

Nit: s/"Message Values" /" Message Type Values" in section 5.1. There is no "message values" registry defined by RFC8175