Distributed Denial-of-Service Open Threat Signaling (DOTS) Signal Channel Configuration Attributes for Robust Block Transmission
draft-ietf-dots-robust-blocks-06

# Internet AD comments for {draft-ietf-dots-robust-blocks-05}
CC @ekline

## Comments

* I guess this work can be concluded when it eventually reinvents TCP.  ;-)

Section 7.  Recommend a pointer to explain the intent usage of the YANG module.

OLD
   This document defines YANG data structures that are meant to be used
   as an abstract representation in DOTS signal channel messages.  As
   such, the "ietf-dots-robust-trans" module (Section 5) does not
   introduce any new vulnerabilities beyond those specified above.

NEW

Consistent with Section 5 of [RFC9131], this YANG module is not intended to be used via NETCONF/RESTCONF for DOTS server management purposes.  It serves as an abstract representation in DOTS signal channel messages.   The "ietf-dots-robust-trans" module (Section 5) does not introduce any new vulnerabilities beyond those specified above.

# Éric Vyncke, INT AD, comments for draft-ietf-dots-robust-blocks-05
CC @evyncke

Thank you for the work put into this document. 

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education).

Special thanks to Valery Smyslov for the shepherd's detailed write-up including the WG consensus even if it lacks the justification of the intended status :-(

I hope that this review helps to improve the document,

Regards,

-éric

## COMMENTS

### Abstract

Should "in case of a DoS" be added to "should any of the blocks get lost in transmission" ? 

### Review by CORE WG ?

The shepherd's write-up mentions draft-ietf-core-new-block, but was this I-D formally reviewed by the CORE WG? It is usual to forward the WGLC to related WG; was it done ?

### Section 1

s/(i.e., responses may get dropped)/(e.g., responses may get dropped)/ ?

### Section 3, NON ?

The acronym "NON" is often used as a prefix, it would help the reader to provide the expansion of "NON" in the text.

### Security considerations

YANG modules often use https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines to build the associated security considerations; is there any reason why this template was not used ?

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. 

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments

Thanks for this spec. Also, thanks to Valery Smyslov for the carefully-
written shepherd writeup.

I have just one comment, and one question, about this in Section 3:

   max-payloads:  This attribute echoes the MAX_PAYLOADS parameter in
      [RFC9177].

      This is an optional attribute.  If the attribute is supplied in
      both 'idle-config' and 'mitigating-config', then it MUST convey
      the same value.  If the attribute is only provided as part of
      'idle-config' or 'mitigating-config', then the other (missing)
      definition MUST be updated to the same value.

I think what you're saying here is that if max-payloads is set for 
either idle-config or mitigating-config, then that parameter shall be
deemed set for both configs, and to the same quantity. I found the
way of saying it above to be quite confusing, especially the "(missing)"
thing, since if it's optional it can hardly be said to be "missing" just
because it isn't present.  

I also wonder if you need to supply some answer to the question of
what should happen if the MUST is violated. E.g. one option might be
to say that if two different values are supplied, the larger of the 
two shall be used for both.

# GEN AD review of draft-ietf-dots-robust-blocks-05

CC @larseggert

Thanks to Tim Evens for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/NJ_TPQmVJ7Ph620Xlka54ptz-f8).

## Comments

### IANA

The IANA review of this document seems to not have concluded yet.

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### Grammar/style

#### Section 3, paragraph 4
```
o limit the potential wait needed calculated when using PROBING_RATE. NON_PAR
                                  ^^^^^^^^^^
```
The double modal "needed calculated" is nonstandard (only accepted in certain
dialects). Consider "to be calculated".

#### Section 5, paragraph 11
```
 limit the potential wait needed calculated when using probing-rate."; choic
                                 ^^^^^^^^^^
```
The double modal "needed calculated" is nonstandard (only accepted in certain
dialects). Consider "to be calculated".

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

"NON_PROBING_WAIT:
is used to limit the potential wait needed calculated when using PROBING_RATE."

I think there is a grammar problem here, and I can't parse this definition.

HI,

Thanks for this document.

Only have a single, nit level, comment: When I was reading the field descriptions before the YANG modules, I was wondering what the units were, e.g., for the Dec64 values.  I noted that the units are specified in the YANG module, but possibly highlighting them in the earlier descriptions would also be helpful.

Regards,
Rob

I would just note that IANA review is not ready yet.