Commit confirmed gNMI extension #198
Conversation
link fix entails->contains fixed typos
hellt
force-pushed
the
commit-confirmed
branch
from
19c8fc9
to
7a0e254
Compare
|
|
||
| #### 3.1 Initiating Commit Confirmed operation | ||
|
|
||
| To initiate a Commit Confirmed operation, the client MUST include the `CommitConfirmed` extension in the `SetRequest` message. The `CommitConfirmed` message MUST include the `COMMIT_CONFIRMED_ACTION_START` action to indicate that the network device MUST apply the configuration changes and start the commit rollback timeout waiting for a confirmation action. |
There was a problem hiding this comment.
We need to specify the locking here - i.e. the period from commit through to commit confirm (or abort or timeout) is a window in which the server is locked from the perspective of other sessions. I would prefer this to be explicit in the ACTION naming such as START_W_LOCK/CONFIRM_W_UNLOCK/REJECT_W_UNLOCK or a better suggestion may be:
ACTION_LOCK (instead of start)
ACTION_UNLOCK_REJECT
ACTION_UNLOCK_CONFIRM.
Failing this change, it should be very clear in this description and in the proto etc. that there is exclusive locking taking place across this 'confirmation exchange', as this is the first time gnmi has added a multi-step operation (i.e. all updates are single messages, with no lock, edit, edit, commit, unlock type flows like netconf).
|
|
||
| The commit identifier returned by the network device in the `SetResponse` message with `COMMIT_CONFIRMED_ACTION_START` action enables controlled session management and prevents erroneous actions to be taken by the network device due to race conditions. | ||
|
|
||
| For example, when commit_id is not used it is theoretically possible that the commit confirmed action will be processed by the network device after the timeout has expired, causing erroneous action to be taken. Or multiple actors will attempt to perform different actions on the same commit. |
There was a problem hiding this comment.
Add some comment about security here - is the commit-id sufficient to really identify the initial commit owner and are we happy with that? (vs constraining this to within a session so we can check some session params on the server as well as the id? or e.g. specify that you expect the server to store and check the AAA credentials of the confirm sender == the initiator??)
|
replaced by #196 |
Hi @robshakir, @kidiyoor
This is a proposal for gNMI Commit Confirmed extension as a response to #196 #197
It comes with the proposed extension proto spec in the accompanying PR openconfig/gnmi#154