2024-05-09, 16:53:42 +0200, Antonio Quartulli wrote:

On 09/05/2024 16:36, Antonio Quartulli wrote:

quoted

On 09/05/2024 16:17, Sabrina Dubroca wrote:

quoted

2024-05-09, 15:44:26 +0200, Antonio Quartulli wrote:

quoted

On 09/05/2024 15:04, Sabrina Dubroca wrote:

quoted

quoted

quoted

quoted

+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);

Do you need the napi instance to be per peer, or can it be per
netdevice? If it's per netdevice you can clean it up in
->priv_destructor.

that need to be executed in process context.
So it seems I must fire up the worker anyway..

I hope with can simplify all that logic. There's some complexity
that's unavoidable in this kind of driver, but maybe not as much as
you've got here.

-- 
Sabrina