From: Antonio Quartulli <antonio@openvpn.net>
To: Sabrina Dubroca <sd@queasysnail.net>
Cc: ryazanov.s.a@gmail.com, netdev@vger.kernel.org,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Donald Hunter <donald.hunter@gmail.com>,
Shuah Khan <shuah@kernel.org>,
Andrew Lunn <andrew+netdev@lunn.ch>,
Simon Horman <horms@kernel.org>,
linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
Xiao Liang <shaw.leon@gmail.com>
Subject: Re: [PATCH net-next v18 20/25] ovpn: implement peer add/get/dump/delete via netlink
Date: Tue, 21 Jan 2025 11:10:00 +0100 [thread overview]
Message-ID: <bcb0ef0a-8c4c-404f-b171-7948e46da897@openvpn.net> (raw)
In-Reply-To: <Z49v7Ah4nUPMVgMM@hog>
On 21/01/2025 10:59, Sabrina Dubroca wrote:
> 2025-01-20, 22:20:40 +0100, Antonio Quartulli wrote:
>> On 20/01/2025 11:45, Antonio Quartulli wrote:
>> [...]
>>>>>>>> I'm not sure what this (and the peer flushing on NETDEV_DOWN) is
>>>>>>>> trying to accomplish. Is it a problem to keep peers
>>>>>>>> when the netdevice
>>>>>>>> is down?
>>>>>>>
>>>>>>> This is the result of my discussion with Sergey that
>>>>>>> started in v23 5/23:
>>>>>>>
>>>>>>> https://lore.kernel.org/r/netdev/20241029-b4-ovpn-v11-5-
>>>>>>> de4698c73a25@openvpn.net/
>>>>>>>
>>>>>>> The idea was to match operational state with actual
>>>>>>> connectivity to peer(s).
>>>>>>>
>>>>>>> Originally I wanted to simply kee the carrier always on,
>>>>>>> but after further
>>>>>>> discussion (including the meaning of the openvpn option
>>>>>>> --persist- tun) we
>>>>>>> agreed on following the logic where an UP device has a
>>>>>>> peer connected (logic
>>>>>>> is slightly different between MP and P2P).
>>>>>>>
>>>>>>> I am not extremely happy with the resulting complexity,
>>>>>>> but it seemed to be
>>>>>>> blocker for Sergey.
>>>>>>
>>>>>> [after re-reading that discussion with Sergey]
>>>>>>
>>>>>> I don't understand why "admin does 'ip link set tun0 down'" means "we
>>>>>> should get rid of all peers. For me the carrier situation goes the
>>>>>> other way: no peer, no carrier (as if I unplugged the cable from my
>>>>>> ethernet card), and it's independent of what the user does (ip link
>>>>>> set XXX up/down). You have that with netif_carrier_{on,off}, but
>>>>>> flushing peers when the admin does "ip link set tun0 down" is separate
>>>>>> IMO.
>>>>>
>>>>> The reasoning was "the user is asking the VPN to go down - it should be
>>>>> assumed that from that moment on no VPN traffic whatsoever
>>>>> should flow in
>>>>> either direction".
>>>>> Similarly to when you bring an Eth interface dwn - the phy link
>>>>> goes down as
>>>>> well.
>>>>>
>>>>> Does it make sense?
>>>>
>>>> I'm not sure. If I turn the ovpn interface down for a second, the
>>>> peers are removed. Will they come back when I bring the interface back
>>>> up? That'd have to be done by userspace (which could also watch for
>>>> the DOWN events and tell the kernel to flush the peers) - but some of
>>>> the peers could have timed out in the meantime.
>>>>
>>>> If I set the VPN interface down, I expect no packets flowing through
>>>> that interface (dropping the peers isn't necessary for that), but all
>>>> non-data (key exchange etc sent by openvpn's userspace) should still
>>>> go through, and IMO peer keepalive fits in that "non-data" category.
>>>
>>> This was my original thought too and my original proposal followed this
>>> idea :-)
>>>
>>> However Sergey had a strong opinion about "the user expect no traffic
>>> whatsoever".
>>>
>>> I'd be happy about going again with your proposed approach, but I need
>>> to be sure that on the next revision nobody will come asking to revert
>>> this logic again :(
>>>
>>>>
>>>>
>>>> What does openvpn currently do if I do
>>>> ip link set tun0 down ; sleep 5 ; ip link set tun0 up
>>>> with a tuntap interface?
>>>
>>> I think nothing happens, because userspace doesn't monitor the netdev
>>> status. Therefore, unless tun closed the socket (which I think it does
>>> only when the interface is destroyed), userspace does not even realize
>>> that the interface went down.
>>
>> What does IPsec do in this case? Does it keep connections open and
>> keepalives flowing?
>
> I don't think IPsec is a good comparison, because it can be used
> without any interface at all (and without UDP/TCP encap sockets), and
> they're not strongly tied to packet processing. If an interface is
> used, the implementation will send the packets through it, otherwise
> it's perfectly happy to send packets back and forth without it.
>
> MACsec is a bit more similar (all crypto state is bound to the macsec
> netdevice -- but no socket and no keepalive), and here the key
> exchange packets all flow directly through the real interface (eth0 or
> whatever), without worrying about the state of the macsec device
> (although I guess the userspace taking care of key exchange is free to
> stop sending when the admin turns the link down).
Thanks for the explanation!
>
>> One counter example we have in the kernel are 802.11 interfaces.
>> Any 802.11 interface must be brought up before you can possibly establish a
>> WiFi link. If you bring the interface down the link is closed and no 802.11
>> control packets flow anymore.
>>
>> However, 802.11 is different as we are controlling a "physical behaviour",
>> while in ovpn (like other tunneling modules) we are controlling a "virtual
>> behaviour".
>
> Agree, 802.11 is a bit special.
>
> (I see you already answered my previous message, but since I've
> written all this anyway... :))
>
Eheh
This confirms once more that I should go back to keeping peers alive on
ifdown.
Thanks a lot!
Regards,
--
Antonio Quartulli
OpenVPN Inc.
next prev parent reply other threads:[~2025-01-21 10:09 UTC|newest]
Thread overview: 71+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-01-13 9:31 [PATCH net-next v18 00/25] Introducing OpenVPN Data Channel Offload Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 01/25] net: introduce OpenVPN Data Channel Offload (ovpn) Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 02/25] ovpn: add basic netlink support Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 03/25] ovpn: add basic interface creation/destruction/management routines Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 04/25] ovpn: keep carrier always on for MP interfaces Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 05/25] ovpn: introduce the ovpn_peer object Antonio Quartulli
2025-01-17 11:58 ` Sabrina Dubroca
2025-01-17 12:26 ` Antonio Quartulli
2025-02-02 22:56 ` Sabrina Dubroca
2025-02-03 8:41 ` Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 06/25] ovpn: introduce the ovpn_socket object Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 07/25] ovpn: implement basic TX path (UDP) Antonio Quartulli
2025-02-03 9:52 ` Sabrina Dubroca
2025-02-04 16:18 ` Sabrina Dubroca
2025-02-05 9:12 ` Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 08/25] ovpn: implement basic RX " Antonio Quartulli
2025-02-03 9:30 ` Sabrina Dubroca
2025-02-03 9:58 ` Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 09/25] ovpn: implement packet processing Antonio Quartulli
2025-01-17 12:16 ` Sabrina Dubroca
2025-01-17 12:28 ` Antonio Quartulli
2025-02-05 21:50 ` Sabrina Dubroca
2025-02-07 13:13 ` Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 10/25] ovpn: store tunnel and transport statistics Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 11/25] ipv6: export inet6_stream_ops via EXPORT_SYMBOL_GPL Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 12/25] ovpn: implement TCP transport Antonio Quartulli
2025-01-15 17:25 ` Sabrina Dubroca
2025-01-15 17:55 ` Jakub Kicinski
2025-01-17 17:14 ` Sabrina Dubroca
2025-01-19 20:06 ` Antonio Quartulli
2025-01-20 14:12 ` Antonio Quartulli
2025-01-21 9:28 ` Sabrina Dubroca
2025-02-03 10:05 ` Sabrina Dubroca
2025-02-03 13:12 ` Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 13/25] skb: implement skb_send_sock_locked_with_flags() Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 14/25] ovpn: add support for MSG_NOSIGNAL in tcp_sendmsg Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 15/25] ovpn: implement multi-peer support Antonio Quartulli
2025-02-02 23:00 ` Sabrina Dubroca
2025-02-03 9:01 ` Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 16/25] ovpn: implement peer lookup logic Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 17/25] ovpn: implement keepalive mechanism Antonio Quartulli
2025-02-03 9:20 ` Sabrina Dubroca
2025-02-03 9:55 ` Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 18/25] ovpn: add support for updating local UDP endpoint Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 19/25] ovpn: add support for peer floating Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 20/25] ovpn: implement peer add/get/dump/delete via netlink Antonio Quartulli
2025-01-17 11:48 ` Sabrina Dubroca
2025-01-17 12:59 ` Antonio Quartulli
2025-01-17 17:12 ` Sabrina Dubroca
2025-01-19 13:12 ` Antonio Quartulli
2025-01-20 10:09 ` Sabrina Dubroca
2025-01-20 10:45 ` Antonio Quartulli
2025-01-20 21:20 ` Antonio Quartulli
2025-01-21 9:59 ` Sabrina Dubroca
2025-01-21 10:10 ` Antonio Quartulli [this message]
2025-01-21 9:39 ` Sabrina Dubroca
2025-01-21 9:48 ` Antonio Quartulli
2025-01-20 14:52 ` Antonio Quartulli
2025-01-21 23:26 ` Antonio Quartulli
2025-01-22 8:45 ` Sabrina Dubroca
2025-01-22 0:40 ` Antonio Quartulli
2025-01-22 8:51 ` Sabrina Dubroca
2025-01-22 9:00 ` Antonio Quartulli
2025-02-02 23:07 ` Sabrina Dubroca
2025-02-03 9:46 ` Antonio Quartulli
2025-02-03 10:42 ` Sabrina Dubroca
2025-01-13 9:31 ` [PATCH net-next v18 21/25] ovpn: implement key add/get/del/swap " Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 22/25] ovpn: kill key and notify userspace in case of IV exhaustion Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 23/25] ovpn: notify userspace when a peer is deleted Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 24/25] ovpn: add basic ethtool support Antonio Quartulli
2025-01-13 9:31 ` [PATCH net-next v18 25/25] testing/selftests: add test tool and scripts for ovpn module Antonio Quartulli
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=bcb0ef0a-8c4c-404f-b171-7948e46da897@openvpn.net \
--to=antonio@openvpn.net \
--cc=andrew+netdev@lunn.ch \
--cc=donald.hunter@gmail.com \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=kuba@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=ryazanov.s.a@gmail.com \
--cc=sd@queasysnail.net \
--cc=shaw.leon@gmail.com \
--cc=shuah@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).