[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZwUrAn8xrF2BCrMp@nanopsycho.orion>
Date: Tue, 8 Oct 2024 14:52:18 +0200
From: Jiri Pirko <jiri@...nulli.us>
To: Antonio Quartulli <antonio@...nvpn.net>
Cc: ryazanov.s.a@...il.com, 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
Subject: Re: [PATCH net-next v8 03/24] ovpn: add basic netlink support
Tue, Oct 08, 2024 at 11:16:01AM CEST, antonio@...nvpn.net wrote:
>On 08/10/2024 10:58, Jiri Pirko wrote:
>> 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?
>
><CAHNKnsQnHAdxC-XhC9RP-cFp0d-E4YGb+7ie3WymXVL9N-QS6A@...l.gmail.com> from
>Sergey and subsequent replies.
>RTNL vs NL topic starts right after the definition of 'ovpn_link_ops'
Yeah, does not make sense to me. All devices should implement common
rtnl ops, the extra-config, if needed, could be on a separate channel.
I don't find Sergey's argumentation valid.
>
>Recently Kuniyuki commented on this topic as well in:
><20240919055259.17622-1-kuniyu@...zon.com>
>and that is why I added a default dellink implemetation.
Having dellink without newlink implemented is just wrong.
>
>>
>>
>> >
>> > 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
>
>All in all I tend to agree.
>
>>
>> >
>> > @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.
>
>The only counterargument I see is the one brought by Sergey: "the ovpn
>interface is not usable after creation, if no openvpn process is running".
>
>However, allowing to create "persistent interfaces" will define a use-case
>for having an ovpn device without any userspace process.
>
>@Sergey what is your opinion here? I am not sure persistent interfaces were
>discussed at the time you brought your point about RTNL vs NL.
>
>
>Regards,
>
>
>>
>> >
>> > 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.
>
>--
>Antonio Quartulli
>OpenVPN Inc.
Powered by blists - more mailing lists