# OPSDir review of draft-ietf-cdni-additional-footprint-types-04

Reviewer: Dhruv Dhody
Review Result: Minor issues

I have reviewed this document as part of the Operational directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written with the intent of improving the operational aspects of the IETF drafts. Comments that are not addressed in Telechat may be included in AD reviews during the IESG review.  Document editors and WG chairs should treat these comments just like any other Telechat review comments.

Also at https://notes.ietf.org/draft-ietf-cdni-additional-footprint-types

## Summary

This document defines two new CDNI Footprint Types (one for ISO3166-2 Subdivision Codes and one for union footprints). The document is easy to read and clear. The use of union could have some additional operational complexity. 

## Major

- None

## Minor
- Abstract should explicitly state that it is updating RFC 8006 and 8008.

- I think the I-D can use BCP 14 keywords (e.g. MAY), request authors to check. I see the use of "required" and "may" but in lowercase! 

- Section 3.1, while defining new footprint types in ALTO, RFC 9241 also explicitly states "Hierarchy and Inheritance" which this I-D skips. Just to be consistent, please add it and state that "There is no hierarchy or inheritance for properties associated with subdivision codes."  

- Section 4.1, Looking at the IANA registry https://www.iana.org/assignments/cdni-parameters/cdni-parameters.xhtml#metadata-footprint-types , I am not sure why FCI prefix ("FCI.xxx") have been added in this document. 
    - Also the description for subdivisioncode could be aligned to how the countrycode is defined "ISO 3166-1 alpha-2 code".

- Should there be an explicit statement that these are optional and not mandatory? 

- Are there any constraints to union that one needs to state? The use of the same type? Presence of multiple unions? Conflicts? Any operational issues? I could not think of any explicit one but wanted to highlight if others can. Perhaps adding some text on this could be beneficial. 


## Nits
- Fix the english "below is introduced the" in

````
   To overcome the described limitation and allow a list of footprint
   constraints that match both IPv4 and IPv6 client addresses, below is
   introduced the "footprintunion" footprint type. 
````

- In table 1, change "Specification" to "Reference" to align with the IANA registry. In table 2 also add a "Reference" column at the end. 

Thanks! 
Dhruv