Hi Ned,
Thanks for your review and comments. Please check inline below for responses.
The changes discussed below have been posted in this update: https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-ls-sr-policy-11
On Thu, Feb 6, 2025 at 5:01 AM Ned Smith via Datatracker <noreply@xxxxxxxx> wrote:
Reviewer: Ned Smith
Review result: Has Nits
Ned M. Smith Review
2025-02-05
I have reviewed draft-ietf-idr-bgp-ls-sr-policy revision 10, which specifies
BGP Link-State (BGP-LS) extensions for advertising Segment Routing (SR)
Policies. The draft appears to be ready but there are a few nits that caught my
attention. The authors may want to make changes or judge that for the intended
audience no changes are needed.
Notes:
1) "Flags: 1-octet field with following bit positions defined. Other bits MUST
be cleared by the originator and MUST be ignored by a receiver." [NMS] Use of
the word "cleared" may be ambiguous. Other similar language uses "set to 0".
There are multiple occurrences of this concern.
KT> When talking about bits, this document is pervasive (and I believe consistent?) in its use of the terms "set" and "clear". I fail to see the ambiguity as a protocol developer. That said, we have added a clarification at the start of the document
2) "Bandwidth: 4 octets which specify the desired bandwidth in unit of bytes
per second in IEEE floating point format." [NMS] This appears to be a normative
requirement on IEEE floating point format but doesn't cite the specification.
There are multiple occurrences of this.
KT> I have seen a similar comment raised on a different document which is also about a representation of bandwidth. One of the earlier RFCs that I know of where this came in was https://www.rfc-editor.org/rfc/rfc3630.html#section-2.5.6 - where no reference was provided. From there on, we've been using this without any reference quite widely across various routing protocol specs without thinking twice about it. That said, you have a good point and we have updated to refer to IEEE754 (courtesy Roman & Acee who provided the pointer on the other thread).
3) "4 octets which carry a 32-bit unsigned non-zero number"
[NMS] Using "number" may be ambiguous. Other text uses, e.g., "integer". The
byte order for multibyte numbers isn't explicitly specified. The reader might
presume big-endian from the contexts, but IMO it doesn't hurt to state
assumptions the authors are making.
KT> ACK on changing number -> integer. A strong NACK on getting into specification of byte-ordering. This should be clear to any implementer already. Adding anything here will be harmful since it raises question/doubt on the encoding of other similar information across protocols.
4) In the section 8.6. BGP-LS SR Policy Metric Type table, the code points 121
- 127 are omitted. Is this on purpose?
KT> Good catch. Thanks!
Thanks,
Ketan
-- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
