experiments/clixon
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

* Netconf lock/unlock behaviour changed to adhere to RFC 6241

Browse source 
* Changed commit lock error tag from "lock denied" to "in-use".
  * Changed unlock error message from "lock is already held" to #lock not active" or "lock held by other session".
  * Fixed [lock candidate succeeded even though it is modified #110](https://github.com/clicon/clixon/issues/110)
This commit is contained in:
Olof hagsand 2020-07-28 13:56:44 +02:00
parent 6bd9306423
commit 23bcee8185
7 changed files with 178 additions and 69 deletions
Download patch file Download diff file Expand all files Collapse all files

43
apps/backend/backend_client.c
View file

@ -273,11 +273,10 @@ clixon_stats_get_db(clicon_handle h,
cxobj *xt = NULL; cxobj *xt = NULL;
uint64_t nr = 0; uint64_t nr = 0;
size_t sz = 0; size_t sz = 0;
db_elmnt *de = NULL;
/* This is the db cache */ /* This is the db cache */
if ((de = clicon_db_elmnt_get(h, dbname)) == NULL || if ((xt = xmldb_cache_get(h, dbname)) == NULL){
(xt = de->de_xml) == NULL){
cprintf(cb, "<datastore><name>%s</name><nr>0</nr><size>0</size></datastore>", dbname); cprintf(cb, "<datastore><name>%s</name><nr>0</nr><size>0</size></datastore>", dbname);
} }
else{ else{
@ -606,6 +605,7 @@ from_client_edit_config(clicon_handle h,
goto done; goto done;
goto ok; goto ok;
} }
if ((x = xpath_first(xn, NULL, "default-operation")) != NULL){ if ((x = xpath_first(xn, NULL, "default-operation")) != NULL){
if (xml_operation(xml_body(x), &operation) < 0){ if (xml_operation(xml_body(x), &operation) < 0){
if (netconf_invalid_value(cbret, "protocol", "Wrong operation")< 0) if (netconf_invalid_value(cbret, "protocol", "Wrong operation")< 0)
@ -618,7 +618,6 @@ from_client_edit_config(clicon_handle h,
goto done; goto done;
goto ok; goto ok;
} }
else{
/* <config> yang spec may be set to anyxml by ingress yang check,...*/ /* <config> yang spec may be set to anyxml by ingress yang check,...*/
if (xml_spec(xc) != NULL) if (xml_spec(xc) != NULL)
xml_spec_set(xc, NULL); xml_spec_set(xc, NULL);
@ -660,7 +659,7 @@ from_client_edit_config(clicon_handle h,
} }
if (ret == 0) if (ret == 0)
goto ok; goto ok;
} xmldb_modified_set(h, target, 1); /* mark as dirty */
assert(cbuf_len(cbret) == 0); assert(cbuf_len(cbret) == 0);
cprintf(cbret, "<rpc-reply><ok/></rpc-reply>"); cprintf(cbret, "<rpc-reply><ok/></rpc-reply>");
ok: ok:
@ -742,6 +741,7 @@ from_client_copy_config(clicon_handle h,
goto done; goto done;
goto ok; goto ok;
} }
xmldb_modified_set(h, target, 1); /* mark as dirty */
cprintf(cbret, "<rpc-reply><ok/></rpc-reply>"); cprintf(cbret, "<rpc-reply><ok/></rpc-reply>");
ok: ok:
retval = 0; retval = 0;
@ -808,6 +808,7 @@ from_client_delete_config(clicon_handle h,
goto done; goto done;
goto ok; goto ok;
} }
xmldb_modified_set(h, target, 1); /* mark as dirty */
cprintf(cbret, "<rpc-reply><ok/></rpc-reply>"); cprintf(cbret, "<rpc-reply><ok/></rpc-reply>");
ok: ok:
retval = 0; retval = 0;
@ -859,21 +860,26 @@ from_client_lock(clicon_handle h,
/* /*
* A lock MUST not be granted if either of the following conditions is true: * A lock MUST not be granted if either of the following conditions is true:
* 1) A lock is already held by any NETCONF session or another entity. * 1) A lock is already held by any NETCONF session or another entity.
* 2) The target configuration is <candidate>, it has already been modified, and
* these changes have not been committed or rolled back.
*/ */
iddb = xmldb_islocked(h, db); if ((iddb = xmldb_islocked(h, db)) != 0){
if (iddb != 0){
cprintf(cbx, "<session-id>%u</session-id>", iddb); cprintf(cbx, "<session-id>%u</session-id>", iddb);
if (netconf_lock_denied(cbret, cbuf_get(cbx), "Operation failed, lock is already held") < 0) if (netconf_lock_denied(cbret, cbuf_get(cbx), "Operation failed, lock is already held") < 0)
goto done; goto done;
goto ok; goto ok;
} }
else{ /* 2) The target configuration is <candidate>, it has already been modified, and
* these changes have not been committed or rolled back.
*/
if (strcmp(db, "candidate") == 0 &&
xmldb_modified_get(h, db)){
if (netconf_lock_denied(cbret, "<session-id>0</session-id>",
"Operation failed, candidate has already been modified and the changes have not been committed or rolled back (RFC 6241 7.5)") < 0)
goto done;
goto ok;
}
if (xmldb_lock(h, db, id) < 0) if (xmldb_lock(h, db, id) < 0)
goto done; goto done;
cprintf(cbret, "<rpc-reply><ok/></rpc-reply>"); cprintf(cbret, "<rpc-reply><ok/></rpc-reply>");
}
ok: ok:
retval = 0; retval = 0;
done: done:
@ -926,12 +932,19 @@ from_client_unlock(clicon_handle h,
* An unlock operation will not succeed if any of the following * An unlock operation will not succeed if any of the following
* conditions are true: * conditions are true:
* 1) the specified lock is not currently active * 1) the specified lock is not currently active
* 2) the session issuing the <unlock> operation is not the same */
if (iddb == 0){
cprintf(cbx, "<session-id>0</session-id>");
if (netconf_lock_denied(cbret, cbuf_get(cbx), "Unlock failed, lock is not currently active") < 0)
goto done;
goto ok;
}
/* 2) the session issuing the <unlock> operation is not the same
* session that obtained the lock * session that obtained the lock
*/ */
if (iddb == 0 || iddb != id){ else if (iddb != id){
cprintf(cbx, "<session-id>id=%u iddb=%d</session-id>", id, iddb); cprintf(cbx, "<session-id>%u</session-id>", iddb);
if (netconf_lock_denied(cbret, cbuf_get(cbx), "Unlock failed, lock is already held") < 0) if (netconf_lock_denied(cbret, cbuf_get(cbx), "Unlock failed, lock held by other session") < 0)
goto done; goto done;
goto ok; goto ok;
} }

4
apps/backend/backend_commit.c
View file

@ -572,6 +572,7 @@ candidate_commit(clicon_handle h,
*/ */
if (xmldb_copy(h, candidate, "running") < 0) if (xmldb_copy(h, candidate, "running") < 0)
goto done; goto done;
xmldb_modified_set(h, candidate, 0); /* reset dirty bit */
/* Here pointers to old (source) tree are obsolete */ /* Here pointers to old (source) tree are obsolete */
if (td->td_dvec){ if (td->td_dvec){
td->td_dlen = 0; td->td_dlen = 0;
@ -640,7 +641,7 @@ from_client_commit(clicon_handle h,
goto done; goto done;
} }
cprintf(cbx, "<session-id>%u</session-id>", iddb); cprintf(cbx, "<session-id>%u</session-id>", iddb);
if (netconf_lock_denied(cbret, cbuf_get(cbx), "Operation failed, lock is already held") < 0) if (netconf_in_use(cbret, cbuf_get(cbx), "Operation failed, lock is already held") < 0)
goto done; goto done;
goto ok; goto ok;
} }
@ -703,6 +704,7 @@ from_client_discard_changes(clicon_handle h,
goto done; goto done;
goto ok; goto ok;
} }
xmldb_modified_set(h, "candidate", 0); /* reset dirty bit */
cprintf(cbret, "<rpc-reply><ok/></rpc-reply>"); cprintf(cbret, "<rpc-reply><ok/></rpc-reply>");
ok: ok:
retval = 0; retval = 0;

2
apps/backend/backend_main.c
View file

@ -871,6 +871,8 @@ main(int argc,
/* Initiate the shared candidate. */ /* Initiate the shared candidate. */
if (xmldb_copy(h, "running", "candidate") < 0) if (xmldb_copy(h, "running", "candidate") < 0)
goto done; goto done;
xmldb_modified_set(h, "candidate", 0);
/* Set startup status */ /* Set startup status */
if (clicon_startup_status_set(h, status) < 0) if (clicon_startup_status_set(h, status) < 0)
goto done; goto done;

7
lib/clixon/clixon_data.h
View file

@ -46,10 +46,15 @@
/* /*
* Types * Types
*/ */
/* Struct per database in hash */ /* Struct per database in hash
* Semantics of de_modified is to implement this from RFC 6241 Sec 7.5:
* The target configuration is <candidate>, it has already been
* modified, and these changes have not been committed or rolled back.
*/
typedef struct { typedef struct {
uint32_t de_id; /* session id */ uint32_t de_id; /* session id */
cxobj *de_xml; /* cache */ cxobj *de_xml; /* cache */
int de_modified;
} db_elmnt; } db_elmnt;
/* /*

6
lib/clixon/clixon_datastore.h
View file

@ -69,4 +69,10 @@ int xmldb_create(clicon_handle h, const char *db);
/* utility functions */ /* utility functions */
int xmldb_db_reset(clicon_handle h, char *db); int xmldb_db_reset(clicon_handle h, char *db);
cxobj *xmldb_cache_get(clicon_handle h, const char *db);
int xmldb_modified_get(clicon_handle h, const char *db);
int xmldb_modified_set(clicon_handle h, const char *db, int value);
#endif /* _CLIXON_DATASTORE_H */ #endif /* _CLIXON_DATASTORE_H */

64
lib/src/clixon_datastore.c
View file

@ -329,7 +329,7 @@ xmldb_unlock_all(clicon_handle h,
* @param[in] db Database * @param[in] db Database
* @retval -1 Error * @retval -1 Error
* @retval 0 Not locked * @retval 0 Not locked
* @retval >0 Id of locker * @retval >0 Session id of locker
*/ */
uint32_t uint32_t
xmldb_islocked(clicon_handle h, xmldb_islocked(clicon_handle h,
@ -479,3 +479,65 @@ xmldb_db_reset(clicon_handle h,
return -1; return -1;
return 0; return 0;
} }
/*! Get datastore XML cache
* @param[in] h Clicon handle
* @param[in] db Database name
* @retval xml XML cached tree or NULL
*/
cxobj *
xmldb_cache_get(clicon_handle h,
const char *db)
{
db_elmnt *de;
if ((de = clicon_db_elmnt_get(h, db)) == NULL)
return NULL;
return de->de_xml;
}
/*! Get modified flag from datastore
* @param[in] h Clicon handle
* @param[in] db Database name
* @retval -1 Error (datastore does not exist)
* @retval 0 Db is not modified
* @retval 1 Db is modified
* @note This only makes sense for "candidate", see RFC 6241 Sec 7.5
* @note This only works if db cache is used,...
*/
int
xmldb_modified_get(clicon_handle h,
const char *db)
{
db_elmnt *de;
if ((de = clicon_db_elmnt_get(h, db)) == NULL){
clicon_err(OE_CFG, EFAULT, "datastore %s does not exist", db);
return -1;
}
return de->de_modified;
}
/*! Get modified flag from datastore
* @param[in] h Clicon handle
* @param[in] db Database name
* @param[in] value 0 or 1
* @retval -1 Error (datastore does not exist)
* @retval 0 OK
* @note This only makes sense for "candidate", see RFC 6241 Sec 7.5
* @note This only works if db cache is used,...
*/
int
xmldb_modified_set(clicon_handle h,
const char *db,
int value)
{
db_elmnt *de;
if ((de = clicon_db_elmnt_get(h, db)) == NULL){
clicon_err(OE_CFG, EFAULT, "datastore %s does not exist", db);
return -1;
}
de->de_modified = value;
return 0;
}

27
test/test_netconf.sh
View file

@ -164,14 +164,20 @@ expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></
new "netconf get state operation" new "netconf get state operation"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><get><filter type=\"xpath\" select=\"/if:interfaces\" xmlns:if=\"urn:ietf:params:xml:ns:yang:ietf-interfaces\" /></get></rpc>]]>]]>" '^<rpc-reply><data><interfaces xmlns="urn:ietf:params:xml:ns:yang:ietf-interfaces"><interface><name>eth1</name><type>ex:eth</type><enabled>true</enabled><oper-status>up</oper-status><ex:my-status xmlns:ex="urn:example:clixon"><ex:int>42</ex:int><ex:str>foo</ex:str></ex:my-status></interface></interfaces></data></rpc-reply>]]>]]>$' expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><get><filter type=\"xpath\" select=\"/if:interfaces\" xmlns:if=\"urn:ietf:params:xml:ns:yang:ietf-interfaces\" /></get></rpc>]]>]]>" '^<rpc-reply><data><interfaces xmlns="urn:ietf:params:xml:ns:yang:ietf-interfaces"><interface><name>eth1</name><type>ex:eth</type><enabled>true</enabled><oper-status>up</oper-status><ex:my-status xmlns:ex="urn:example:clixon"><ex:int>42</ex:int><ex:str>foo</ex:str></ex:my-status></interface></interfaces></data></rpc-reply>]]>]]>$'
new "netconf lock"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><lock><target><candidate/></target></lock></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
new "netconf unlock"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><unlock><target><candidate/></target></unlock></rpc>]]>]]>" "^<rpc-reply><rpc-error><error-type>protocol</error-type><error-tag>lock-denied</error-tag><error-info><session-id>0</session-id></error-info><error-severity>error</error-severity><error-message>Unlock failed, lock is not currently active</error-message></rpc-error></rpc-reply>]]>]]>$"
new "netconf lock/unlock" new "netconf lock/unlock"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><lock><target><candidate/></target></lock></rpc>]]>]]><rpc><unlock><target><candidate/></target></unlock></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]><rpc-reply><ok/></rpc-reply>]]>]]>$" expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><lock><target><candidate/></target></lock></rpc>]]>]]><rpc><unlock><target><candidate/></target></unlock></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]><rpc-reply><ok/></rpc-reply>]]>]]>$"
new "netconf lock/lock" new "netconf lock/lock"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><lock><target><candidate/></target></lock></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><lock><target><candidate/></target></lock></rpc>]]>]]><rpc><lock><target><candidate/></target></lock></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]><rpc-reply><rpc-error><error-type>protocol</error-type><error-tag>lock-denied</error-tag><error-info><session-id>"
new "netconf lock" new "netconf unlock/unlock"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><lock><target><candidate/></target></lock></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><unlock><target><candidate/></target></unlock></rpc>]]>]]><rpc><unlock><target><candidate/></target></unlock></rpc>]]>]]>" "^<rpc-reply><rpc-error><error-type>protocol</error-type><error-tag>lock-denied</error-tag><error-info><session-id>0</session-id></error-info><error-severity>error</error-severity><error-message>Unlock failed, lock is not currently active</error-message></rpc-error></rpc-reply>]]>]]><rpc-reply><rpc-error><error-type>protocol</error-type><error-tag>lock-denied</error-tag><error-info><session-id>0</session-id></error-info><error-severity>error</error-severity><error-message>Unlock failed, lock is not currently active</error-message></rpc-error></rpc-reply>]]>]]>$"
new "close-session" new "close-session"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><close-session/></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><close-session/></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
@ -180,7 +186,20 @@ expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><close-session/></rpc>]]>]]>" "^<rp
new "kill-session" new "kill-session"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><kill-session><session-id>44</session-id></kill-session></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><kill-session><session-id>44</session-id></kill-session></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
new "copy startup" # modify candidate, then lock, should fail.
new "netconf edit config"
expecteof "$clixon_netconf -qf $cfg" 0 '<rpc><edit-config><target><candidate/></target><config><table xmlns="urn:example:clixon"><parameter><name>a</name></parameter></table></config></edit-config></rpc>]]>]]>' "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
new "netconf lock: should fail"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><lock><target><candidate/></target></lock></rpc>]]>]]>" "^<rpc-reply><rpc-error><error-type>protocol</error-type><error-tag>lock-denied</error-tag><error-info><session-id>0</session-id></error-info><error-severity>error</error-severity><error-message>Operation failed, candidate has already been modified and the changes have not been committed or rolled back (RFC 6241 7.5)</error-message></rpc-error></rpc-reply>]]>]]>$"
new "netconf discard-changes"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><discard-changes/></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
new "netconf lock"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><lock><target><candidate/></target></lock></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
new "copy startup to candidate"
expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><copy-config><target><startup/></target><source><candidate/></source></copy-config></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$" expecteof "$clixon_netconf -qf $cfg" 0 "<rpc><copy-config><target><startup/></target><source><candidate/></source></copy-config></rpc>]]>]]>" "^<rpc-reply><ok/></rpc-reply>]]>]]>$"
new "netconf get startup" new "netconf get startup"