Reviewer: Jean-Michel Combes Review result: Ready with Nits Hi, At first, my apologies for my delayed review. I am an assigned INT directorate reviewer for draft-ietf-detnet-yang-19.txt. These comments were written primarily for the benefit of the Internet Area Directors. Document editors and shepherd(s) should treat these comments just like they would treat comments from any other IETF contributors and resolve them along with any other Last Call comments that have been received. For more details on the INT Directorate, see https://datatracker.ietf.org/group/intdir/about/ <https://datatracker.ietf.org/group/intdir/about/>. Based on my review, if I was on the IESG I would ballot this document as NO OBJECTION. The following are minor issues (typos, misspelling, minor text improvements) with the document: *** Section 8, p.14 *** typedef ipsec-spi { type uint32 { range "1..max"; } description "IPsec Security Parameters Index. A 32 bit value where 0 is reserved."; reference "IETF RFC 4303 Encapsulating Security Payload (ESP)."; } IMHO, RFC 4301 should be referenced instead of RFC 4303 because: - SPI is defined for IPsec in RFC 4301 - SPI is also used by AH [RFC 4302] <snip> *** Section 8, p.18 *** grouping ip-header { description "This grouping captures the IPv4/IPv6 packet header information. It is modeled after existing fields."; leaf src-ip-address { type inet:ip-address-no-zone; description "The source IP address in the header."; reference "RFC 6991 Common YANG Data Types"; } leaf dest-ip-address { type inet:ip-address-no-zone; description "The destination IP address in the header."; reference "RFC 6991 Common YANG Data Types"; } leaf protocol-next-header { type uint8; description "Internet Protocol number. Refers to the protocol of the payload. In IPv6, this field is known as 'next-header', and if extension headers are present, the protocol is present in the 'upper-layer' header."; reference "RFC 791: Internet Protocol RFC 8200: Internet Protocol, Version 6 (IPv6) Specification."; } “In IPv6, this field is known as 'next-header', and if extension headers are present, the protocol is present in the 'upper-layer' header.";” From my point of view, this sentence is not really clear. IMHO, you should copy/paste what is written in RFC 8200 (Section 4, p.7): “When processing a sequence of Next Header values in a packet, the first one that is not an extension header [IANA-EH] indicates that the next item in the packet is the corresponding upper-layer header.” <snip> *** Section 8, p.20 *** grouping ip-flow-id { description "The IPv4/IPv6 packet header identification information."; leaf src-ip-prefix { type inet:ip-prefix; description "The source IP prefix."; reference "RFC 6991 Common YANG Data Types"; } leaf dest-ip-prefix { type inet:ip-prefix; description "The destination IP prefix."; reference "RFC 6991 Common YANG Data Types"; } leaf protocol-next-header { type uint8; description "Internet Protocol number. Refers to the protocol of the payload. In IPv6, this field is known as 'next-header', and if extension headers are present, the protocol is present in the 'upper-layer' header."; reference "RFC 791: Internet Protocol RFC 8200: Internet Protocol, Version 6 (IPv6) Specification."; } Same comment as *** Section 8, p.18 *** leaf dscp { type inet:dscp; description "The traffic class value in the header."; reference "RFC 6991 Common YANG Data Types"; } leaf flow-label { type inet:ipv6-flow-label; description "The flow label value of the header."; reference "RFC 6991 Common YANG Data Types"; } uses source-ip-port-id; uses destination-ip-port-id; leaf ipsec-spi { type ipsec-spi; description "IPsec Security Parameters Index of the Security Association."; reference "IETF RFC 4303 Encapsulating Security Payload (ESP)."; } Same comment as *** Section 8, p.14 *** <snip> Best regards, JMC. -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
