Re: [Netconf] Review of draft-ietf-netconf-partial-lock-02.txt

[Balazs Lengyel <balazs.lengyel at ericsson.com>]

Hello Mehmet,
Thanks for the comments. See also below.
Balazs

Ersue, Mehmet (NSN - DE/Muenich) wrote:
> Hi Balazs,
>
> I reviewed the draft "Partial Lock RPC for NETCONF".
> Please find my comments below.
>
> General comments:
>
> - The document looks good. Do you think we should bring it with
> an updated version to WGLC after the IETF meeting?
{BALAZS}: Yes, I believe it should go to WGLC.
> - Are we going to register the XML Schema at IANA as we did for
> RFC 5277? In any case I would suggest to have the Schema as
> self-explaining.
{BALAZS}: I will add comments, annotations
> Comments per chapter:
>
> 2.1. Overview
>
> - "The system MUST ensure . . ."
>
> I don't know how to ensure this if it is a non-NETCONF management system.
> The statement sounds like a normative statement but it does not cover
> any standard-relevant definition. It is actually a generic statement and
> I would suggest to find another formulation than "MUST".
{BALAZS}: I am not sure I understand your comment. The system (the netconf server) IS a NETCONF 
system, otherwise it would not implement Netconf partial locking. I see this as a requirement 
on the Netconf server, not just a generic statement so I think MUST is appropriate.
RFC4741 uses MUST when describing global lock "the server MUST prevent any changes to the 
locked resource".
Earlier I had "The system ensures" but someone had a comment to use MUST.
> - I would split the 3rd paragraph into two sentences for more clarity:
> "The duration of the partial lock is defined as beginning when the
> partial lock is granted.  The partial lock lasts until either the 
> corresponding
> <partial-unlock> operation succeeds or the NETCONF session terminates."
{BALAZS}: OK
> 2.2. Dependencies
>
> s/in _Section 2.4.1_ 
> <http://tools.ietf.org/html/draft-ietf-netconf-partial-lock-02#section-2.4.1>/in 
> _Section 2.4.1_ 
> <http://tools.ietf.org/html/draft-ietf-netconf-partial-lock-02#section-2.4.1>./ 
{BALAZS}: OK
> 2.4.1. <partial-lock>
>
> - I would indicate the beginning of the description part of the 
> <partial-lock>
> operation with: <Description:>
>
> - I would suggest to indicate that the the "portion of the data store" is a
> "coherent set of nodes".
{BALAZS}: Sorry what do you mean by COHERENT set of nodes? Could you explain it please.
The set of nodes can consist of 10 disconnected individual nodes/subtrees if the XPATH 
expressions are specified that way.
> - 6th paragraph: "If a top level node of a locked subtree is deleted, . 
> . ."
>
> Why is it necessary to keep the lock with a "nil scope" present?
> I would expect that a lock with a nil scope is not needed and should not
> be kept.
{BALAZS}: It is not nice if a lock just disappears. If the manager SW later comes and tries to 
unlock it and gets back an error he might be surprised. At this point the manager will need 
extra checks to see, whether the error was caused by a removed empty lock or other error.
But if the workgroup insists this could be changed.
> - s/A partial lock MUST fail/A partial lock operation MUST fail/
{BALAZS}: OK
> - 2nd bullet: " o  Any part of the scope to be locked is already locked 
> . . ."
>
> I think we should have a note here to explain why we think that a partial
> lock can be also introduced by a non-NETCONF management system and
> how this could be realized.
> I agree with an earlier comment that Netconf locking (both global and 
> partial)
> is creating new requirements for SNMP and other protocols. I would propose
> to explain the impact on other management protocols in this note.
{BALAZS}: I need to think about this comment a bit.
The new requirement is not really on the other protocol handlers, but the instrumentation 
behind the protocols. It needs to respect the Netconf partial-lock which is  stated in 2.1 
paragraph 2.
If the other protocols also want to use their own partial-locking, that is their own decision 
but that is not a _requirement_ for implementors of this draft. The other protocols can still 
use a global lock.
> - s/If the select expressions return/If the select expression returns/
{BALAZS}: OK
> 2.5. Modifications to Existing Operations
>
> I would reorganize the first sentence for more clarity, e.g.:
> "A granted partial-lock will cause operations to fail, if it wants to 
> modify
> the already locked area and is executed in a NETCONF session other
> than the one that owns the lock."
{BALAZS}: OK
> 3. Security Considerations
>
> Do the two sub-paragraphs below belong into the security considerations
> or should they be put to the end of the paragraph 2.4.2. <partial-unlock>?
> They have at least to do with releasing of locks.
>
> "The partial-lock is automatically released when . . .
> The <kill-session> operation allows terminating other users . . ."
{BALAZS}: Earlier in 2.1 para 3. we already stated that terminating the session releases the 
lock. These paragraphs are placed here to emphasize, how we can avoid a DOS attack based on 
partial-locking.
> 5. Appendix A - XML Schema for Partial Locking (normative)
>
> Please add the element descriptions in the YANG module also into the XML
> Schema as documentation to get it self-explaining.
{BALAZS}: OK
> Cheers,
> Mehmet

--
Balazs Lengyel                       Ericsson Hungary Ltd.
TSP System Manager
ECN: 831 7320                        Fax: +36 1 4377792
Tel: +36-1-437-7320     email: Balazs.Lengyel at ericsson.com
_______________________________________________
Netconf mailing list
Netconf at ietf.org
https://www.ietf.org/mailman/listinfo/netconf