Hello Antonio, Donald,

On 18.09.2024 14:16, Antonio Quartulli wrote:

On 18/09/2024 12:07, Donald Hunter wrote:

quoted

Antonio Quartulli [off-list ref] writes:

quoted

quoted

quoted

+      -
+        name: local-ip
+        type: binary
+        doc: The local IP to be used to send packets to the peer 
(UDP only)
+        checks:
+          max-len: 16

It might be better to have separate attrs fopr local-ipv4 and
local-ipv6, to be consistent with vpn-ipv4 / vpn-ipv6

while it is possible for a peer to be dual stack and have both an 
IPv4 and IPv6 address assigned
to the VPN tunnel, the local transport endpoint can only be one 
(either v4 or v6).
This is why we have only one local_ip.
Does it make sense?

I was thinking that the two attributes would be mutually exclusive. You
could accept local-ipv4 OR local-ipv6. If both are provided then you can
report an extack error.

Ok then, I'll split the local-ip in two attrs.

It also gets cleaner as we have an explicit type definition, while right 
now we infer the type from the data length.

quoted

quoted

quoted

quoted

+      -
+        name: keyconf
+        type: nest
+        doc: Peer specific cipher configuration
+        nested-attributes: keyconf

Perhaps keyconf should just be used as a top-level attribute-set. The
only attr you'd need to duplicate would be peer-id? There are separate
ops for setting peers and for key configuration, right?

This is indeed a good point.
Yes, SET_PEER and SET_KEY are separate ops.

I could go with SET_PEER only, and let the user specify a keyconf 
within a peer (like now).

Or I could keep to SET_KEY, but then do as you suggest and move 
KEYCONF to the root level.

Is there any preferred approach?

I liked the separate ops for key management because the sematics are
explicit and it is very obvious that there is no op for reading keys. If
you also keep keyconf attrs separate from the peer attrs then it would be
obvious that the peer ops would never expose any keyconf attrs.

Ok, will move KEYCONF to the root level and will duplicate the PEER_ID.

Nice idea! A was about to suggest the same. Besides making semantic 
simple, what somehow subjective, it should make parsing way simple. Two 
nested attributes parsing calls will be saved.

--
Sergey