[yang-doctors] Re: YANG Doctor review for "A YANG Data Model and RADIUS Extension for Policy-based Network Access Control" - draft-ietf-opsawg-ucl-acl

["maqiufang (A)" <[e-mail]>]

Hi, Acee,

Thanks for the follow-up, please see below inline with [Qiufang]. My co-authors may chime in if they wish.

> I have one major concern with this document. The YANG model adds 
> generalized schedule-based ACEs, yet this is not reflected in the YANG 
> model name,  draft title, or abstract. This should at least be in a 
> separate YANG model and possibly in a separate draft since it appears 
> to have been added as an afterthought and, IMO, it is much more 
> important than the group-based access control.
> [Qiufang] Note that this is not an afterthought design, it is there when the draft was -00. We split the scheduling related definition into a separate I-D to define common groupings (see https://datatracker.ietf.org/doc/draft-ietf-netmod-schedule-yang/) based on the WG feedback.
> I agree with you that the extension regarding scheduling is of great importance, it addresses the requirement of “when to take effect”, together with endpoint group based matching, which resolves the requirement of “for whom to take effect”, they both constitutes the data model, separating them could fragment the policy model. Hopefully this clarifies the intention.
> To enhance the visibility of the scheduling feature, we have updated both the abstract and introduction sections to reflect it explicitly.

Will you also have a separate YANG model? For example, ietf-acl-ace-sched.yang? 
[Qiufang] As clarified, creating a separate model might introduce unnecessary fragmentation. Note we already use if-feature to make each augment conditional ("match-on-group" feature for endpoint group based augmentation and "schedule" feature for date and time based ACLs), this allows the potential for reuse that a separate model could provide, especially for implementations that wants to support time-based ACLs without the complexity of endpoint group matching. Thanks.



> 
> 5. How did you decide on 64 octets for the group identifier string
>    maximum?
> [Qiufang] As specified in the YANG module, the “group-id” is defined as a string type with a length constraint of "1..64". This is to align with that.

Right - I'm asking how you came up with 64 octets as a limit?
[Qiufang] sorry for misunderstanding. 64 is not arbitrary, see https://www.rfc-editor.org/rfc/rfc7950.html#section-6.2: "Implementations MUST support identifiers up to 64 characters in length and MAY support longer identifiers."
And also https://datatracker.ietf.org/doc/html/draft-ietf-netmod-rfc8407bis-28#section-4.3: "All YANG identifiers in published modules MUST be between 1 and 64 characters in length."

> 
> 6. In section 6, I would have expected the attribute to be the first
>    column in table 4.
> [Qiufang] Review of RADIUS-related RFCs (e.g., RFC 8044, RFC 2865) reveals no mandatory requirement that the attribute column be placed as the first column in the table. Since the table focuses solely on the single attribute "User-Access-Group-ID"—with no need to distinguish between multiple attributes—placing the attribute column last does not obscure the key information.
> There are also some input received from RADEXT WG, see some previous discussion at : https://mailarchive.ietf.org/arch/msg/opsawg/EWuvlu623PgapTPSB4s8LM6lc5M/.

But English is normally left-to-right and one would expect the attribute to be in the first column. 
[Qiufang] Thanks, Acee. I don’t really have a strong feeling regarding this. While I checked some existing RFCs that register the RADIUS attribute type from "Radius Attribute Types", it seems that most of the RFCs put the attribute as the last column, e.g., https://datatracker.ietf.org/doc/html/rfc5176#section-3.6, https://datatracker.ietf.org/doc/html/rfc8658#Table3, https://datatracker.ietf.org/doc/html/rfc5090#section-5, and  https://datatracker.ietf.org/doc/html/rfc9445#table-1, perhaps aligning with existing RFCs would help improve consistency?

Best Regards,
Qiufang