Hi Antonio,

On 23.09.2024 15:59, Antonio Quartulli wrote:

On 23/09/2024 01:20, Sergey Ryazanov wrote:

quoted

On 17.09.2024 04:07, Antonio Quartulli wrote:

quoted

+    -
+      name: set-peer
+      attribute-set: ovpn
+      flags: [ admin-perm ]
+      doc: Add or modify a remote peer

As Donald already mentioned, the typical approach to manage objects 
via Netlink is to provide an interface with four commands: New, Set, 
Get, Del. Here, peer created implicitely using the "set" comand. Out 
of curiosity, what the reason to create peers in the such way?

To be honest, I just wanted to keep the API as concise as possible and 
having ADD and SET looked like duplicating methods, from a conceptual 
perspective.

Could you elaborate, what is wrong with separated NEW and SET method 
conceptually?

 From the implementation point of view I can see that both methods can 
setup a same set of object properties. What can be resolved using a 
shared (between NEW and SET) peer configuration method.

What userspace wants is "ensure we have a peer with ID X and these 
attributes". If this ID was already known is not extremely important.

I can understand in other contexts knowing if an object already exists 
can be crucial.

Looks like you want a "self synchronizing" API that automatically 
recovers synchronization between userspace and kernel.

On one hand this approach can mask potential bug. E.g. management 
application assumes that a peer was not configured and trying to 
configure it and kernel quietly reconfigure earlier known peer. Shall we 
in that case loudly inform everyone that something already went wrong?

On another hand, I see that current implementation does not do this. The 
SET method handler works differently depending on prior peer existence. 
The SET method will not allow an existing peer reconfiguration since it 
will trigger error due to inability to update "VPN" IPv4/IPv6 address. 
So looks like we have two different methods merged into the single 
function with complex behaviour.

BTW, if you want an option to recreate a peer, did you consider the 
NLM_F_REPLACE flag support in the NEW method?

quoted

Is the reason to create keys also implicitly same?

basically yes: userspace tells kernelspace "this is what I have 
configured in my slots - make sure to have the same"
(this statement also goes back to the other reply I have sent regarding 
changing the KEY APIs)

If we save the current conception of slots, then yes it make sense.

quoted

quoted

+      do:
+        pre: ovpn-nl-pre-doit
+        post: ovpn-nl-post-doit
+        request:
+          attributes:
+            - ifindex
+            - peer
+    -
+      name: get-peer
+      attribute-set: ovpn
+      flags: [ admin-perm ]
+      doc: Retrieve data about existing remote peers (or a specific 
one)
+      do:
+        pre: ovpn-nl-pre-doit
+        post: ovpn-nl-post-doit
+        request:
+          attributes:
+            - ifindex
+            - peer
+        reply:
+          attributes:
+            - peer
+      dump:
+        request:
+          attributes:
+            - ifindex
+        reply:
+          attributes:
+            - peer
+    -
+      name: del-peer
+      attribute-set: ovpn
+      flags: [ admin-perm ]
+      doc: Delete existing remote peer
+      do:
+        pre: ovpn-nl-pre-doit
+        post: ovpn-nl-post-doit
+        request:
+          attributes:
+            - ifindex
+            - peer
+    -
+      name: set-key
+      attribute-set: ovpn
+      flags: [ admin-perm ]
+      doc: Add or modify a cipher key for a specific peer
+      do:
+        pre: ovpn-nl-pre-doit
+        post: ovpn-nl-post-doit
+        request:
+          attributes:
+            - ifindex
+            - peer
+    -
+      name: swap-keys
+      attribute-set: ovpn
+      flags: [ admin-perm ]
+      doc: Swap primary and secondary session keys for a specific peer
+      do:
+        pre: ovpn-nl-pre-doit
+        post: ovpn-nl-post-doit
+        request:
+          attributes:
+            - ifindex
+            - peer
+    -
+      name: del-key
+      attribute-set: ovpn
+      flags: [ admin-perm ]
+      doc: Delete cipher key for a specific peer
+      do:
+        pre: ovpn-nl-pre-doit
+        post: ovpn-nl-post-doit
+        request:
+          attributes:
+            - ifindex
+            - peer
+

--
Sergey