Hi Quifang, 

See inline. 

> On Jan 28, 2026, at 5:08 AM, maqiufang (A) <[email protected]> wrote:
> 
> Hi, Acee,
>  Thanks a lot for the careful review and comments, we prepared a PR to 
> address them at 
> :https://github.com/IETF-OPSAWG-WG/policy-based-network-acl/pull/146. Please 
> also see my reply below inline with [Qiufang]…
>  From: Acee Lindem [mailto:[email protected]]
> Sent: Tuesday, January 27, 2026 6:54 AM
> To: [email protected]; YANG Doctors <[email protected]>; 
> Mahesh Jethanandani <[email protected]>; [email protected]
> Subject: [yang-doctors] YANG Doctor review for "A YANG Data Model and RADIUS 
> Extension for Policy-based Network Access Control" - draft-ietf-opsawg-ucl-acl
>  Document: draft-ietf-opsawg-ucl-acl-11
> Reviewer: Acee Lindem
> Review Date: 2026-01-26
> IETF LC End Date: 2026-01-26
> Intended Status: STANDARD TRACK
> 
> This is a YANG doctor review on the YANG data module ietf-ucl-acl.yang.
> 
> 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? 



>  
> The following issues/questions also have to be addressed:
> 
> 1. In section 2, the formatting of "device group" and "application group"
>    are messed up. Also, there is an unresolved reference to {{sec-dg}} and
>    {{sec-ag}}. I guess you are not using the standard XML source.
> [Qiufang] Thanks, fixed at 
> https://github.com/IETF-OPSAWG-WG/policy-based-network-acl/pull/144/changes.
> 
> 2. Section 4.2.2 - I've never used a printer to send emails ;^)
> [Qiufang] Some printers support the scanning and can send scanned files via 
> email to a specified target. But I have removed this now;-)
> 
> 3. Section 4.3 - I believe you want to change "not differentiating" to
>    "differentiating" as this is prefaced by "run without requiring".
> [Qiufang] Fixed this with the following sentence:
> In some cases, applications and devices may operate and run without requiring 
> any user interventions, or they may require user authentication but access 
> rules do not differentiate between different users.
> 
> 4. Throughout, you hyphenate end-user but not end-device? I changed this
>    in my suggested edits.
> [Qiufang] Fixed, 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?


> 
> 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. 

Thanks,
Acee

> 
> 7. In section 8.1, I guess the PEP wouldn't need to implement anything
>    beyond standard ACLs, as long as the SDN controller maps the
>    group-id-based rule ACE to one or more standard ACEs - correct?
> [Qiufang] Yes, and the last sentence of the first paragraph have already 
> reflected this.
> 
> 8. In section 9, source-group-id and destination-group-id should both
>    in ACEs should both be addressed.
> [Qiufang] Yes, note they are inside 
> /acl:acls/acl:acl/acl:aces/acl:ace/acl:matches/ucl:endpoint-group and both 
> are covered in sec9.1.
> 
> 9. If the schedule-based ACEs are retained in this document, write access
>    could facilitate multiple attacks.
> [Qiufang] Have added one sentence to enforce the use of access control.
> 
> Consider:
> 
> I have some editorial suggestions for the draft that I've attached.
> [Qiufang] All should be incorporated, thanks.
> 
> Thanks,
> Acee
>  Best Regards,
> Qiufang
> _______________________________________________
> yang-doctors mailing list -- [email protected]
> To unsubscribe send an email to [email protected]


_______________________________________________
OPSAWG mailing list -- [email protected]
To unsubscribe send an email to [email protected]

Reply via email to