Skip to content

comments from Paul - #161

Open
QiufangMa wants to merge 5 commits into
IETF-OPSAWG-WG:mainfrom
QiufangMa:main
Open

comments from Paul#161
QiufangMa wants to merge 5 commits into
IETF-OPSAWG-WG:mainfrom
QiufangMa:main

Conversation

@QiufangMa

Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread yang/ietf-ucl-acl.yang
"IETF OPSAWG (Operations and Management Area Working Group)";
contact
"WG Web: <https://datatracker.ietf.org/wg/opsawg/>
"WG Web: https://datatracker.ietf.org/wg/opsawg/

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Comment thread yang/ietf-ucl-acl.yang
description
"Indicates support of matching on endpoint groups.";
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expecte this to be fixed in the edited version by the RFC Editor, but OK

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Welcome back;-)
I aslo expect the editorial nits would be fixed by RFC editor, but since Paul has already point them out, we can maintain it here in case the editor fails to recognize them.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Authors should not defer work to the RFC Editor as the editor does not know the author's intentions and they may overlook or miss issues.

Comment thread yang/ietf-ucl-acl.yang
}
description
"Specifies the endpoint group type.";
"Specifies the type of the endpoint group (e.g., user,

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Comment thread yang/ietf-ucl-acl.yang
associated with the packet's source and/or destination
endpoint.

Note this container is only valid when the ACL type is

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK althought part of the description is redundant with teh when clause

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, the intention is to call out the implication that server needs to implement both features to ensure this container is valid.

Comment thread .note.xml
<note title="Discussion Venues" removeInRFC="true">
<t>Discussion of this document takes place on the
Operations and Management Area Working Group Working Group mailing list (opsawg@ietf.org),
Operations and Management Area Working Group mailing list (opsawg@ietf.org),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this note will be removed from the final RFC anyway.


device group:
: A collection of enterprise devices that share a common access control policies. Refer to {{sec-dg}} for more details.
Device group:

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK. I expect this to be fixed in the edited version bu the RFC Editor.

: Either the AAA server or the NAS notifies an SDN controller
of the mapping between the user group ID and related common packet
header attributes (e.g., the 5-tuple). The exact details of how such notification is performed are out scope of this specification.
header attributes (e.g., the 5-tuple). The exact details of how such notification is performed are out of scope for this specification.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will leave this one to the RFC Editor.

this attribute, including the Type, Length, Extended-Type, and the
"Value".
: The Length MUST be at most 67 octets. The maximum length is 67 octets to accommodate the maximum group ID of 64 octets plus one octet for Type, one octet for Length, and one octet for Extended-Length.
: The Length MUST be at most 67 octets. The maximum length is 67 octets to accommodate the maximum group ID of 64 octets plus one octet for Type, one octet for Length, and one octet for Extended-Type.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch


* /acl:acls/acl:acl/acl:aces/acl:ace/ucl:effective-schedule:
: It specifies the secheduling of ACLs. Unauthorized write access to this data node may allow intruders to
: It specifies the scheduling of ACLs. Unauthorized write access to this data node may allow intruders to

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

--- back

# Examples Usage
# Usage Examples

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Comment thread yang/ietf-ucl-acl.yang Outdated

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: there is trailing space after "group".

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8462028.

Comment thread yang/ietf-ucl-acl.yang Outdated
"Adds new match types.";
"Adds new match criteria based on the group identity
associated with the packet's source and/or destination
endpoint.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: there is trailing space after "endpoint."

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8462028. Thanks for catching them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants