netdev - Re: [PATCH net-next v8 03/24] ovpn: add basic netlink support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZwT0SkGHu5VHQ9Hd@nanopsycho.orion>
Date: Tue, 8 Oct 2024 10:58:50 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Antonio Quartulli <antonio@...nvpn.net>
Cc: Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
	Paolo Abeni <pabeni@...hat.com>,
	Donald Hunter <donald.hunter@...il.com>,
	Shuah Khan <shuah@...nel.org>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
	sd@...asysnail.net, ryazanov.s.a@...il.com
Subject: Re: [PATCH net-next v8 03/24] ovpn: add basic netlink support

Tue, Oct 08, 2024 at 10:01:40AM CEST, antonio@...nvpn.net wrote:
>Hi,
>
>On 07/10/24 17:32, Jiri Pirko wrote:
>> Wed, Oct 02, 2024 at 11:02:17AM CEST, antonio@...nvpn.net wrote:
>> 
>> [...]
>> 
>> 
>> > +operations:
>> > +  list:
>> > +    -
>> > +      name: dev-new
>> > +      attribute-set: ovpn
>> > +      flags: [ admin-perm ]
>> > +      doc: Create a new interface of type ovpn
>> > +      do:
>> > +        request:
>> > +          attributes:
>> > +            - ifname
>> > +            - mode
>> > +        reply:
>> > +          attributes:
>> > +            - ifname
>> > +            - ifindex
>> > +    -
>> > +      name: dev-del
>> 
>> Why you expose new and del here in ovn specific generic netlink iface?
>> Why can't you use the exising RTNL api which is used for creation and
>> destruction of other types of devices?
>
>That was my original approach in v1, but it was argued that an ovpn interface
>needs a userspace program to be configured and used in a meaningful way,
>therefore it was decided to concentrate all iface mgmt APIs along with the
>others in the netlink family and to not expose any RTNL ops.

Can you please point me to the message id?


>
>However, recently we decided to add a dellink implementation for better
>integration with network namespaces and to allow the user to wipe a dangling
>interface.

Hmm, one more argument to have symmetric add/del impletentation in RTNL


>
>In the future we are planning to also add the possibility to create a
>"persistent interface", that is an interface created before launching any
>userspace program and that survives when the latter is stopped.
>I can guess this functionality may be better suited for RTNL, but I am not
>sure yet.

That would be quite confusing to have RTNL and genetlink iface to
add/del device. From what you described above, makes more sent to have
it just in RTNL

>
>@Jiri: do you have any particular opinion why we should use RTNL ops and not
>netlink for creating/destroying interfaces? I feel this is mostly a matter of
>taste, but maybe there are technical reasons we should consider.

Well. technically, you can probabaly do both. But it is quite common
that you can add/delete these kind of devices over RTNL. Lots of
examples. People are used to it, aligns with existing flows.

>
>Thanks a lot for your contribution.
>
>Regards,
>
>
>> 
>> 
>> ip link add [link DEV | parentdev NAME] [ name ] NAME
>> 		    [ txqueuelen PACKETS ]
>> 		    [ address LLADDR ]
>> 		    [ broadcast LLADDR ]
>> 		    [ mtu MTU ] [index IDX ]
>> 		    [ numtxqueues QUEUE_COUNT ]
>> 		    [ numrxqueues QUEUE_COUNT ]
>> 		    [ netns { PID | NETNSNAME | NETNSFILE } ]
>> 		    type TYPE [ ARGS ]
>> 
>> ip link delete { DEVICE | dev DEVICE | group DEVGROUP } type TYPE [ ARGS ]
>> 
>> Lots of examples of existing types creation is for example here:
>> https://developers.redhat.com/blog/2018/10/22/introduction-to-linux-interfaces-for-virtual-networking
>> 
>> 
>> 
>> > +      attribute-set: ovpn
>> > +      flags: [ admin-perm ]
>> > +      doc: Delete existing interface of type ovpn
>> > +      do:
>> > +        pre: ovpn-nl-pre-doit
>> > +        post: ovpn-nl-post-doit
>> > +        request:
>> > +          attributes:
>> > +            - ifindex
>> 
>> [...]
>
>-- 
>Antonio Quartulli
>OpenVPN Inc.