Subject: [Netconf] WGLC: draft-ietf-netconf-partial-lock-05

From: "Bert Wijnen \(IETF\)" <[email-masked]>
Date: Mon, 19 Jan 2009 17:57:54 +0100

This new version is now available for over a month. I know that the holiday
season was part of that. Nevertheless, by now people should have had time
to check the new version and ensure that
- any earlier comments have been addressed or answered
- no new concerns have been introduced.

All WG members, pls review and make sure we have your input BEFORE
Feb 1st.

While looking at the diff file at:
http://tools.ietf.org/wg/netconf/draft-ietf-netconf-partial-lock/draft-ietf-netconf-partial-lock-05-from-04.diff.html

I immediately saw these 2 nits:

-  I notice in section 2.1 (page 4) that you changed 
      <partial-unlock> 
   into
       partial-unlock
   But in the last para of section 2,1 you do talk about <partial-lock>
   Inconsistent?

-  This sentence (3rd bullet page 7):
        The NETCONF server implements access control, and the locking user
        does not have sufficient access rights.  ....
   Do you mean s/does not/must/ ??

Bert

_______________________________________________
Netconf mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/netconf

---

From: "Bert Wijnen \(IETF\)" <[email-masked]>
Date: Mon, 19 Jan 2009 17:57:54 +0100

Bert

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Wed, 21 Jan 2009 10:15:05 +0100

> All WG members, pls review and make sure we have your input BEFORE

This is the first time I seriously read the document and here are the
comments/questions that popped up:

- What happens if one of the select expressions is invalid?

- What happens if a target datastore does not exist?

- Since new nodes are not added to the set of locked nodes, if I lock
  a subtree, can I run into race conditions? Suppose I am running a
  transaction that involves adding some nodes and doing some more work
  on them. Since there is no atomic way to add the nodes to the
  datastore and to add them to the lock set, I am concerned that race
  conditions might exist that can be exploited.
  
  Perhaps my problem is that I do not understand 2.4.1.2. What is the
  "the new section created"? Perhaps an example should be added to
  help me to understand how things work. If I have

  <top>
    <a>
      <b>1</b>
      <b>2</b>
    </a>
  </top>

  and I do a partial lock on /top, what is the node set returned? Is
  /top/a locked now and what does it mean for a non-leaf node to be
  locked? I think this needs to be clearly spelled out.

- I think the document should spell out how a lock affects other
  NETCONF operations. How does edit-config, copy-config etc fail if a
  locked node set if affected? This is related to me previous comment.

- I have not reviewed the XSD.

- The YANG module's organization, contact, description, and revision
  statements should in spirit follow RFC 4181.

- I like to see more consistent indentation, in particular indentation
  of description statements. For example, I prefer

   typedef lock-id-type {
     description 
      "A number identifying a specific partial-lock granted to a
       session.  It is allocated by the system, and SHOULD be used in
       the partial-unlock operation.";
     type uint32;
   }

   over what is in the current document.

- I am wondering whether the yang-types should provide a definition of
  a common type holding an xpath expression.

/js

-- 
Juergen Schoenwaelder           Jacobs University Bremen gGmbH
Phone: +49 421 200 3587         Campus Ring 1, 28759 Bremen, Germany
Fax:   +49 421 200 3103         <http://www.jacobs-university.de/>
_______________________________________________
Netconf mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/netconf

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Wed, 21 Jan 2009 10:15:05 +0100

<top>
    <a>
      <b>1</b>
      <b>2</b>
    </a>
  </top>

   typedef lock-id-type {
     description 

/js

---

From: WashamFan <[email-masked]>
Date: Thu, 22 Jan 2009 09:33:44 +0800

> On Mon, Jan 19, 2009 at 05:57:54PM +0100, Bert Wijnen (IETF) wrote:
>   
I think this select expr would return empty.

>  - What happens if a target datastore does not exist?
I think all select expr evaluated for this datastore would return empty.

>    locked? I think this needs to be clearly spelled out.
I'd like try to answer this question, although I am not sure I completely understand what you are asking. If you do a partial lock on /top, the returned node set, which is termed as *scope of lock*, is <top> node, but the *protected area* is <top> node and all nodes underneath it.

I'd like explain sec 2.4.1.2 using your example. If I want to create a <c> directly under <top>, I can't lock /top/c, as <c> is non-existent. What I should do is lock <top> first, then create <c>, then, lock /top/c and unlock /top, after that I can operate on /top/c.

I hope that helps.

washam

>  Fax:   +49 421 200 3103         <

---

From: WashamFan <[email-masked]>
Date: Thu, 22 Jan 2009 09:33:44 +0800

I hope that helps.

washam

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 22 Jan 2009 08:05:21 +0100 (CET)

> Hi,

I think Juergen meant what happens if the expression isn't even valid
XPath syntax.  In this case an rpc-error should be returned with an
'invalid-value' error.

> I think all select expr evaluated for this datastore would return empty.

I think this should also generate an 'invalid-value' error.

> >    "the new section created"?

Maybe write "newly created subtree" instead?  At least
s/section/subtree/.

> >    locked node set if affected? This is related to me previous comment.

This is supposed to be coverd in 2.5.   Is more needed?

> >    a common type holding an xpath expression.

I think that is a good idea.

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 22 Jan 2009 08:05:21 +0100 (CET)

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Thu, 22 Jan 2009 08:29:01 +0100

> This is supposed to be coverd in 2.5.   Is more needed?

For example, how do locking errors relate to the error-option of
edit-config?  Consider an edit-config modifying a node-set of which a
subset is locked. Does the edit-config fail in all cases? Or does lets
say continue-on-error still cause those nodes not locked to be
changed? I think it is good to have these details documented.

/js

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Thu, 22 Jan 2009 08:29:01 +0100

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 22 Jan 2009 08:36:53 +0100 (CET)

> changed? I think it is good to have these details documented.

Good point!  This hasn't been discussed at all in the WG.  One problem
here is that the meaning of "continue-on-error" is unclear; this has
been discussed several times.  Specifically, is the "error" part of
"continue-on-error" supposed to cover this kind of transient errors?

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Thu, 22 Jan 2009 08:36:53 +0100 (CET)

/martin

---

From: WashamFan <[email-masked]>
Date: Fri, 23 Jan 2009 13:41:34 +0800

> Juergen Schoenwaelder <[email-masked]> wrote:
>  > edit-config?  
Why should we treat errors caused by lock differently. "continue-on-error" take effect on all errors including those caused by lock.

>Consider an edit-config modifying a node-set of which 
> a
Yes, the error caused by lock is not the real error, but operator could recognize it in <rpc-error> response. The error lead to <edit-config> partial failure at least. IMO, it is covered by "error" part of "continue-on-error".

washam

---

From: WashamFan <[email-masked]>
Date: Fri, 23 Jan 2009 13:41:34 +0800

washam

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 23 Jan 2009 14:12:06 +0100

See below!
Balazs

>    But in the last para of section 2,1 you do talk about <partial-lock>

BALAZS: Yes inconsistent, changed back to <partial-lock>

>    Do you mean s/does not/must/ ??

BALAZS: We meant "does not" as written. This is not a requirement, we just state that if access 
rights happen to be insufficient, partial-lock fails.
_______________________________________________
Netconf mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/netconf

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 23 Jan 2009 14:12:06 +0100

See below!
Balazs

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 23 Jan 2009 14:16:09 +0100

Yes you understood it correctly. Thanks.
Balazs

WashamFan wrote:
> I'd like try to answer this question, although I am not sure I completely 
> understand what you are asking. If you do a partial lock on /top, the 
> returned node set, which is termed as *scope of lock*, is <top> node, 
 > but the *protected area* is <top> node and all nodes underneath it.
> 
> I'd like explain sec 2.4.1.2 using your example. If I want to create 
 > a <c> directly under <top>, I can't lock /top/c, as <c> is
 > non-existent. What I should do is lock <top> first, then create <c>,
 > then, lock /top/c and unlock /top, after that I can operate on /top/c.
> 
> I hope that helps.
>

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 23 Jan 2009 14:16:09 +0100

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Fri, 23 Jan 2009 14:47:11 +0100

> Hello Washam,

But I fail to read this from the document. I think the document needs
to better spell out what it means to lock a container vs.  locking a
leaf. From the example given here, I deduce that locking a container
means you can't create child nodes. Thats all fine but I like to have
written this down in the specification.

/js

>> I'd like try to answer this question, although I am not sure I 
>> completely understand what you are asking. If you do a partial lock on 
>> /top, the returned node set, which is termed as *scope of lock*, is 
>> <top> node,

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Fri, 23 Jan 2009 14:47:11 +0100

/js

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 23 Jan 2009 15:45:30 +0100

I agree with Washam. It would not be nice to treat the failure due to partial locking as a 
special case. I think the text in 4741 is clear enough. If there is any need for clarification, 
  I would rather say that the 4741bis effort should write some generic text about all error cases.

However we could add to chapter 2.5 something like:
If partial lock prevents edit-config from modifying some data, but the operation includes the 
continue-on-error option, modification of other parts of the datastore, which not protected by 
partial locking, might still succeed.

Opinion?

Is there anything else missing from chapter 2.5?

Balazs

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

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 23 Jan 2009 15:45:30 +0100

Opinion?

Balazs

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Fri, 23 Jan 2009 15:55:14 +0100

> If partial lock prevents edit-config from modifying some data, but the 
> operation includes the continue-on-error option, modification of other 
> parts of the datastore, which not protected by partial locking, might 
> still succeed.

I think being explicit is important. So I am in favour of adding such
a clarification.

/js

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Fri, 23 Jan 2009 15:55:14 +0100

/js

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 23 Jan 2009 17:25:05 +0100

Balazs

Juergen Schoenwaelder wrote:
> On Fri, Jan 23, 2009 at 03:45:30PM +0100, Balazs Lengyel wrote:
>

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 23 Jan 2009 17:25:05 +0100

Balazs

---

From: WashamFan <[email-masked]>
Date: Sat, 24 Jan 2009 09:31:44 +0800

Hi,
> On Fri, Jan 23, 2009 at 02:16:09PM +0100, Balazs Lengyel wrote:
>  > Hello Washam,
Locking a container *implies* other sessions can't *change* anything included in the container, e.g, you can't create child nodes in the container locked by other session.
IMO, sec 2.4.1.2 explictly addressed this concern.

washam
>  >>>       Perhaps my problem is that I do not understand 2.4.1.2. 
> What is 
>  >>> the
>  >>>     and I do a partial lock on /top, what is the node set 
> returned? 
>  >>> Is

---

From: WashamFan <[email-masked]>
Date: Sat, 24 Jan 2009 09:31:44 +0800

Hi,

washam

---

From: Balazs Lengyel <[email-masked]>
Date: Mon, 26 Jan 2009 16:15:38 +0100

Hello Jurgen,
If I would put the following example in an appendix and reference it from 2.4.1.2
would that be OK?

What do you think of the example below?
regards Balazs

=================================================

As an example, lets say we have the data model

container allUsers {
    list users {
      key name  { type string; }
      ... other data ...
    }
}

and the datastore currently contains

<allUsers>
    <user>
      <name>Fred</Name>
      ... other data ...
    </user>
</allUsers>

We want to add user Joe and lock him immediately for further editing.
The following is the proposed procedure to do it:

Use <partial-lock> with select=/.../allUsers. (Full RCP)
This will return the node <allUsers> at /.../allUsers as the only node in the scope of the
lock, but it will protect all nodes under allUsers from modification.

Next create Joe. Now the datastore contains:

<allUsers>
    <user>
      <name>Fred</Name>
      ... other data ...
    </user>
    <user>
      <name>Joe</Name>
      ... other data ...
    </user>
</allUsers>

Use <partial-lock> with select=/.../allUsers/user[name="Joe"]. (Full RCP)
This will return the <user> node at /.../allUsers/user[name="Joe"] as the only node in the
scope of the lock. If any nodes exist under this node, they are protected by both partial locks.

Remove the first partial lock that locked <allUsers>. (Full RCP)
The <user> node at /.../allUsers/user[name="Joe"] and all nodes below it are still protected by 
the second partial
lock.

---

From: Balazs Lengyel <[email-masked]>
Date: Mon, 26 Jan 2009 16:15:38 +0100

Hello Jurgen,

container allUsers {
    list users {

<allUsers>
    <user>
      <name>Fred</Name>
      ... other data ...
    </user>
</allUsers>

<allUsers>
    <user>
      <name>Fred</Name>
      ... other data ...
    </user>
    <user>
      <name>Joe</Name>
      ... other data ...
    </user>
</allUsers>

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Mon, 26 Jan 2009 18:00:51 +0100

On Mon, Jan 26, 2009 at 04:15:38PM +0100, Balazs Lengyel wrote:

> If I would put the following example in an appendix and reference it
> from 2.4.1.2 would that be OK?

> What do you think of the example below?

I would phrase the example by just showing the instance document to
avoid any data modelling language issues. And it might be good to be
consistent with the examples used in RFC 4741.

> Use <partial-lock> with select=/.../allUsers. (Full RCP)

You mean you want to show the full RPC message?

> scope of the lock. If any nodes exist under this node, they are protected by both partial locks.

What I do not understand is that on the one hand the lock on allUsers
is claimed to protect everything below allUsers but then you can
acquire a lock on Joe even though I thought it is locked. I am missing
a clear explanation what a lock on a leaf implies and what a lock on a
contained implies. Perhaps the idea is to say that a lock on a
container locks the container for additions/removals but you wrote the
lock on allUsers "protect all nodes under allUsers from modification"
and this is where I am getting confused about the partial lock
semantics.

> The <user> node at /.../allUsers/user[name="Joe"] and all nodes below it 
> are still protected by the second partial
> lock.

If you say so...

/js

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Mon, 26 Jan 2009 18:00:51 +0100

If you say so...

/js

---

From: Andy Bierman <[email-masked]>
Date: Mon, 26 Jan 2009 09:17:23 -0800

Juergen Schoenwaelder wrote:
>....

I do not like this 'feature' at all.
Obviously only the current lock holder is allowed
to take out another partial lock on nodes it already
has locked.  Otherwise the entire draft is pointless.

But since the only one allowed to take out additional
locks already knows exactly what nodes are currently locked,
that application should simply avoid requesting these nodes.

(I've seen projects where they move as much complexity
to the agent as possible.  It never works out so well.
Good luck this time around.)

> /js

Andy

---

From: Andy Bierman <[email-masked]>
Date: Mon, 26 Jan 2009 09:17:23 -0800

Andy

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 26 Jan 2009 21:21:08 +0100 (CET)

Andy Bierman <[email-masked]> wrote:
> >> The <user> node at /.../allUsers/user[name="Joe"] and all nodes below
> >> it are still protected by the second partial
> >> lock.
> > If you say so...
> > 
> 
> 

Yes you're right.

> that application should simply avoid requesting these nodes.

I don't understand what you mean.  The example does this:

  lock /top/allUsers  --> lock-id=1
  edit-config create /top/allUsers/user[name='joe']  --> ok
  lock /top/allUsers/user[name='joe']  --> lock-id=2
  unlock lock-id=1
  ... many edit-configs towards new user joe... 
  unlock lock-id=2

> Good luck this time around.)

We implemented this even before the draft was done.  Compared to the
other capabilities, this was one of the more trivial to implement
efficiently (ok, global lock is even simpler).

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Mon, 26 Jan 2009 21:21:08 +0100 (CET)

Yes you're right.

/martin

---

From: Andy Bierman <[email-masked]>
Date: Mon, 26 Jan 2009 12:46:32 -0800

>   ... many edit-configs towards new user joe... 

This is a totally different use-case.
This is fine (I guess).  Granting a lock to a node that
is not currently locked is not the same thing
as granting a node a lock that is already locked.

The new node is not locked by rules of the I-D,
but IMO those rules are not intuitive at all.
In practice, any other session must access
/top/allUsers before it can access /top/allUsers/user,
so the fact that user 'joe' is not locked is not relevant.

What would be better (IMO) is a 'create-and-lock'
operation attribute value for <edit-config>,
so /top/allUsers never needs to be locked in the first place.

> efficiently (ok, global lock is even simpler).

The draft came from your implementation,
so I fail to see how that is relevant.

Andy

---

From: Andy Bierman <[email-masked]>
Date: Mon, 26 Jan 2009 12:46:32 -0800

Andy

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 27 Jan 2009 09:36:30 +0100

> so the fact that user 'joe' is not locked is not relevant.

I do not understand the semantics. In the example, it was said that
the lock on allUsers locks everything below allUsers. If that were
true, I would assume that the new user would automatically be locked
as well.

This might have all been easy to implement but the written down words
make it hard for me to understand what a lock on a container really
means.

/js

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 27 Jan 2009 09:36:30 +0100

/js

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 27 Jan 2009 10:42:00 +0100 (CET)

> true, I would assume that the new user would automatically be locked

Sure.

I think we need to backtrack a bit.  The original question was that
given that non-existing nodes cannot be locked, how can we create a
new user 'joe' w/o risking someone else modifying him.  The simple
approach of:

    create joe  --> ok
    lock joe --> ok
    <edit joe>
    unlock joe --> ok

is not safe, since someone else might edit joe before the lock is
granted.

So what you can do is simply:

    lock users --> ok
    create joe --> ok
    <edit joe>
    unlock users --> ok

this is safe, but has the drawback of locking *all* users.

To minimize the time all users are locked, you can do:

    lock users --> ok
    create joe --> ok
    lock joe --> ok
    unlock users --> ok
    <edit joe>
    unlock joe --> ok

This is what Balazs' example shows.

/martin

---

From: Martin Bjorklund <[email-masked]>
Date: Tue, 27 Jan 2009 10:42:00 +0100 (CET)

Sure.

    create joe  --> ok
    lock joe --> ok
    <edit joe>
    unlock joe --> ok

    lock users --> ok
    create joe --> ok
    <edit joe>
    unlock users --> ok

    lock users --> ok
    create joe --> ok
    lock joe --> ok
    unlock users --> ok
    <edit joe>
    unlock joe --> ok

/martin

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 27 Jan 2009 11:52:23 +0100

> This is what Balazs' example shows.

But this requires to spell out in detail under which conditions you
can lock something that is already locked.

/js

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Tue, 27 Jan 2009 11:52:23 +0100

/js

---

From: Andy Bierman <[email-masked]>
Date: Tue, 27 Jan 2009 04:45:57 -0800

> can lock something that is already locked.

I agree the details must be in normative text.

I really do not like this precedent, especially
since the eventual access control model might attempt
to be compatible with partial locking.

IMO, the main idea is broken -- that a lock on '/top/users'
does not mean all the nodes under /top/users are locked.
Instead it means that the lock holder can keep
locking nodes under /top/users.

This is different than a global lock.  A 2nd call to lock,
even by the lock-holder, is considered an application error,
and the lock request is rejected.  Changing this behavior
for partial-lock is inconsistent.  It would be better
to treat this an an application error as well.
If any node in the path from root to the selected node
is already locked, then the new lock request MUST fail.

Andy

---

From: Andy Bierman <[email-masked]>
Date: Tue, 27 Jan 2009 04:45:57 -0800

Andy

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 30 Jan 2009 17:32:02 +0100

Hello Andy,

> I agree the details must be in normative text.

BALAZS: I also agree, but IMO all this is covered in the normative text. What I propose is just 
an example not more. What is missing from the text?

> and the lock request is rejected.  

BALAZS: Other people had earlier explicitly requested to allow the 2nd lock (and the way global 
locking works is actually not changed.)
- It gives you a way to safely reduce the scope of your lock
- it also allows a management application to lock user-joe without considering whether allUsers 
is already locked.

Changing this behavior

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 30 Jan 2009 17:32:02 +0100

Hello Andy,

Changing this behavior

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 30 Jan 2009 17:41:09 +0100

> consistent with the examples used in RFC 4741.

BALAZS: OK

> acquire a lock on Joe even though I thought it is locked. 

BALAZS: from the draft:
"When a NETCONF session holds a lock on a node, no other session or
    non-NETCONF mechanism of the system can change that node or any node
    in the hierarchy of nodes beneath it."

By locking a node you don't actually change it, so this feature does not block that.

later from the draft:

"A partial lock operation MUST fail if:

    o  Any part of the area to be protected is already locked (or
       protected by partial locking) by another management session,
       including other NETCONF sessions using <partial-lock> or any other
       non-NETCONF management method."

It states that a partial lock only blocks another partial lock if it executed "by another 
management session".

> I am missing

---

From: Balazs Lengyel <[email-masked]>
Date: Fri, 30 Jan 2009 17:41:09 +0100

BALAZS: OK

BALAZS: from the draft:

later from the draft:

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Sat, 31 Jan 2009 00:39:13 +0100

On Fri, Jan 30, 2009 at 05:41:09PM +0100, Balazs Lengyel wrote:

> By locking a node you don't actually change it, so this feature does
> not block that.  later from the draft:
>
> It states that a partial lock only blocks another partial lock if it 
> executed "by another management session".

I found the behaviour irritating and it would have helped me a lot if
the text in the document would have been more explicit that it is OK
to lock something that is already locked by the same management
session and that the system has to maintain multiple locks even if
they cover the same "region" (I do not think this is stated anywhere;
or again I have to read between the lines somewhere).

/js

---

From: Juergen Schoenwaelder <[email-masked]>
Date: Sat, 31 Jan 2009 00:39:13 +0100

/js

---

From: WashamFan <[email-masked]>
Date: Sat, 31 Jan 2009 18:07:13 +0800

Hi,
I think this proposal is based on "Partial lock cannot be used to lock non-existent nodes, which would effectively attempt to reserve them for future use." stated in sec 2.4.1.2. Why locking non-existent nodes is not allowed currently, could anyone elaborate it?

washam

---

From: WashamFan <[email-masked]>
Date: Sat, 31 Jan 2009 18:07:13 +0800

Hi,

washam