Re: [pim] [Last-Call] Last Call: <draft-ietf-pim-igmp-mld-snooping-yang-12.txt> (A Yang Data Model for IGMP and MLD Snooping) to Proposed Standard (tom petch)

[Hongji Zhao <[e-mail]>]

Hi Tom,

Please check inline. Thanks a lot!

BR/Hongji

Date: Thu, 11 Jun 2020 12:19:48 +0100
From: tom petch <[e-mail]>
To: [e-mail], [e-mail]
Subject: Re: [pim] [Last-Call] Last Call:
	<draft-ietf-pim-igmp-mld-snooping-yang-12.txt> (A Yang Data Model for
	IGMP and MLD Snooping) to Proposed Standard
Message-ID: <[e-mail]>
Content-Type: text/plain; charset=windows-1252; format=flowed

Separately, two more YANG quirks

The augment have no conditional - rather the YANG 'when' is one level down on the container, not something I am used to nor the practice in other pim yang.
[Authors] accepted
YANG must be plain text - [RFC 4541] looks like a markup
[Authors] accepted

Tom Petch


On 10/06/2020 12:44, tom petch wrote:

The RPC to clear the tables are somewhat insecure.  The default is to clear all so a mistake in entering a valid address will clear everything.  It would be more secure to have an explicit option for clear all.
[Authors] accepted

The RPC to clear the tables has no restrictions; a NACM deny all might be appropriate
[Authors] I have checked some vendor's document. NACM controls access right through users. Create an operator-defined NACM group and assign the user to the group.
In order for an operator-defined NACM group to have access rights associated with an operator-defined NACM rule list, create an operator-defined NACM rule list and assign the group to the rule list.
I think maybe this way could implement the restrictions. 

1.	To create a user account, include the following statement:
	Router(config)# system authentication user <user-name> password <pwdstring>

2.	To create an operator-defined NACM group and assign the operator-defined user to the group, include the following statement:
	Router(config)# nacm groups group <operator-defined-group>
	Router(config-group-<operator-defined-group>)# user-name <user-name>

3.	To create an operator-defined rule list with one rule, include the following statement:
	Note:  
	A rule list can contain multiple rules.
	Router(config)# nacm rule-list <operator-defined-rule-list>
	Router(config-rule-list-<operator-defined-rule-list>)# rule <operator-defined-rule1>
	Router(config-rule-<operator-defined-rule1>)# module-name <model-name>
	Router(config-rule-<operator-defined-rule1>)# path <xpath>
	Router(config-rule-<operator-defined-rule1>)# access-operations (* | read,exec | create,update,delete,exec)
	Router(config-rule-<operator-defined-rule1>)# action (permit | deny)

4.	To assign the operator-defined group to the operator-defined rule list, include the following statement:
	Router(config)# nacm rule-list <operator-defined-rule-list> 
	Router(config-rule-list-<operator-defined-rule-list>)# group <operator-defined-group>


RPC or ACTION?  the latter may be suitable.
[Authors] accepted

RFC5790 is referenced by the YANG module but is not in the I-D References
[Authors] accepted

feature feature* is unusual; that an identifier is a feature is usually apparent from the context in which the identifier is used and so including 'feature' in the name is redundant
[Authors] accepted

And to state the obvious the formatting lacks leading spaces which will doubtless get fixed some time.
[Authors] The RFC editor will fix it in the future.