[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <786914f6-325c-4452-8d71-292ffb59a298@openvpn.net>
Date: Thu, 9 May 2024 16:53:42 +0200
From: Antonio Quartulli <antonio@...nvpn.net>
To: Sabrina Dubroca <sd@...asysnail.net>
Cc: netdev@...r.kernel.org, Jakub Kicinski <kuba@...nel.org>,
Sergey Ryazanov <ryazanov.s.a@...il.com>, Paolo Abeni <pabeni@...hat.com>,
Eric Dumazet <edumazet@...gle.com>, Andrew Lunn <andrew@...n.ch>,
Esben Haabendal <esben@...nix.com>
Subject: Re: [PATCH net-next v3 07/24] ovpn: introduce the ovpn_peer object
On 09/05/2024 16:36, Antonio Quartulli wrote:
> On 09/05/2024 16:17, Sabrina Dubroca wrote:
>> 2024-05-09, 15:44:26 +0200, Antonio Quartulli wrote:
>>> On 09/05/2024 15:04, Sabrina Dubroca wrote:
>>>>>>> +void ovpn_peer_release(struct ovpn_peer *peer)
>>>>>>> +{
>>>>>>> + call_rcu(&peer->rcu, ovpn_peer_release_rcu);
>>>>>>> +}
>>>>>>> +
>>>>>>> +/**
>>>>>>> + * ovpn_peer_delete_work - work scheduled to release peer in
>>>>>>> process context
>>>>>>> + * @work: the work object
>>>>>>> + */
>>>>>>> +static void ovpn_peer_delete_work(struct work_struct *work)
>>>>>>> +{
>>>>>>> + struct ovpn_peer *peer = container_of(work, struct ovpn_peer,
>>>>>>> + delete_work);
>>>>>>> + ovpn_peer_release(peer);
>>>>>>
>>>>>> Does call_rcu really need to run in process context?
>>>>>
>>>>> Reason for switching to process context is that we have to invoke
>>>>> ovpn_nl_notify_del_peer (that sends a netlink event to userspace)
>>>>> and the
>>>>> latter requires a reference to the peer.
>>>>
>>>> I'm confused. When you say "requires a reference to the peer", do you
>>>> mean accessing fields of the peer object? I don't see why this
>>>> requires ovpn_nl_notify_del_peer to to run from process context.
>>>
>>> ovpn_nl_notify_del_peer sends a netlink message to userspace and I
>>> was under
>>> the impression that it may block/sleep, no?
>>> For this reason I assumed it must be executed in process context.
>>
>> With s/GFP_KERNEL/GFP_ATOMIC/, it should be ok to run from whatever
>> context. Firing up a workqueue just to send a 100B netlink message
>> seems a bit overkill.
>
> Oh ok, I thought the send could be a problem too.
>
> Will test with GFP_ATOMIC then. Thanks for the hint.
I am back and unfortunately we also have (added by a later patch):
294 napi_disable(&peer->napi);
295 netif_napi_del(&peer->napi);
that need to be executed in process context.
So it seems I must fire up the worker anyway..
--
Antonio Quartulli
OpenVPN Inc.
Powered by blists - more mailing lists