socket: introduction of destructor callback

[micchie.gml_gmail.com]

Summary

A netmap extension to use the kernel stack (introduced in BSDCan 2018),
associates a socket with netmap port.
Thus, netmap would like to be notified of the socket close().

Test Plan

test with netmap as client: https://github.com/micchie/netmap/tree/stack

[jtl]

sys/kern/uipc_socket.c

1104 ↗

(On Diff #43458)

Is this the correct place to call the destructor? In this location, the destructor would get called when the user closes the socket; however, the protocol level can still hold a reference, which will delay the actual "destruction" of the socket (which, arguably, occurs in sofree).

sys/sys/socketvar.h

114 ↗

(On Diff #43458)

Is this required for both regular and listening sockets? If not, it should go in the one sub-structure that needs it.

[jtl]

The submitter spoke to me in person at BSDCan and answered my questions.

sys/kern/uipc_socket.c

1104 ↗

(On Diff #43458)

The submitter says the purpose is to be notified of the user's close() call, so this does indeed seem like the correct place to make the call.

sys/sys/socketvar.h

114 ↗

(On Diff #43458)

The submitter says this is applicable to both, so should be in the general section. I'll probably move it to be adjacent to so_emuldata to limit the possibility we create new alignment padding.

[rwatson]

Didn't quite catch this before it was committed. This isn't really a destructor, it's a close notification. Rather than confuse matters, as sockets have UMA destructors as well, this should probably be so_notify_close.

[jtl]

In D15706#334605, @rwatson wrote:

Didn't quite catch this before it was committed. This isn't really a destructor, it's a close notification. Rather than confuse matters, as sockets have UMA destructors as well, this should probably be so_notify_close.

Yes, I caught that when I asked the submitter about the placement of the "destructor". But, once I figured out it was a close notification, I should have changed the name. Mea cupla.

Before changing this, let me see if I can confirm what the Linux implementation does.

[micchie.gml_gmail.com]

In D15706#334711, @jtl wrote:

In D15706#334605, @rwatson wrote:

Didn't quite catch this before it was committed. This isn't really a destructor, it's a close notification. Rather than confuse matters, as sockets have UMA destructors as well, this should probably be so_notify_close.

Yes, I caught that when I asked the submitter about the placement of the "destructor". But, once I figured out it was a close notification, I should have changed the name. Mea cupla.

Before changing this, let me see if I can confirm what the Linux implementation does.

Thanks, it is intended to be equivalent to sk_destruct() callback in struct sock in Linux.

[rwatson]

In D15706#334870, @micchie.gml_gmail.com wrote:

In D15706#334711, @jtl wrote:

In D15706#334605, @rwatson wrote:

Didn't quite catch this before it was committed. This isn't really a destructor, it's a close notification. Rather than confuse matters, as sockets have UMA destructors as well, this should probably be so_notify_close.

Yes, I caught that when I asked the submitter about the placement of the "destructor". But, once I figured out it was a close notification, I should have changed the name. Mea cupla.

Before changing this, let me see if I can confirm what the Linux implementation does.

Thanks, it is intended to be equivalent to sk_destruct() callback in struct sock in Linux.

Is the Linux sk_destruct called on socket close, or on socket destruction? If we want an actual socket destructor, we can add one of those [instead / as well], called from the socket destructor function.

[micchie.gml_gmail.com]

In D15706#335628, @rwatson wrote:

In D15706#334870, @micchie.gml_gmail.com wrote:

In D15706#334711, @jtl wrote:

In D15706#334605, @rwatson wrote:

Didn't quite catch this before it was committed. This isn't really a destructor, it's a close notification. Rather than confuse matters, as sockets have UMA destructors as well, this should probably be so_notify_close.

Yes, I caught that when I asked the submitter about the placement of the "destructor". But, once I figured out it was a close notification, I should have changed the name. Mea cupla.

Before changing this, let me see if I can confirm what the Linux implementation does.

Thanks, it is intended to be equivalent to sk_destruct() callback in struct sock in Linux.

Is the Linux sk_destruct called on socket close, or on socket destruction? If we want an actual socket destructor, we can add one of those [instead / as well], called from the socket destructor function.

It is literally destructor, which is called when the final reference count drops to zero. So probably we should define so_dtor() as destructor (so keep the name) and call it after SOCK_UNLOCK() in sofree(). (in Linux it is invoked without locking socket).

[jtl]

In D15706#336213, @micchie.gml_gmail.com wrote:

In D15706#335628, @rwatson wrote:

In D15706#334870, @micchie.gml_gmail.com wrote:

In D15706#334711, @jtl wrote:

In D15706#334605, @rwatson wrote:

Didn't quite catch this before it was committed. This isn't really a destructor, it's a close notification. Rather than confuse matters, as sockets have UMA destructors as well, this should probably be so_notify_close.

Yes, I caught that when I asked the submitter about the placement of the "destructor". But, once I figured out it was a close notification, I should have changed the name. Mea cupla.

Before changing this, let me see if I can confirm what the Linux implementation does.

Thanks, it is intended to be equivalent to sk_destruct() callback in struct sock in Linux.

Is the Linux sk_destruct called on socket close, or on socket destruction? If we want an actual socket destructor, we can add one of those [instead / as well], called from the socket destructor function.

It is literally destructor, which is called when the final reference count drops to zero. So probably we should define so_dtor() as destructor (so keep the name) and call it after SOCK_UNLOCK() in sofree(). (in Linux it is invoked without locking socket).

Which behavior do you want? As @rwatson indicated, you can both have a destructor and a close callback.

[micchie.gml_gmail.com]

In D15706#336214, @jtl wrote:

In D15706#336213, @micchie.gml_gmail.com wrote:

In D15706#335628, @rwatson wrote:

In D15706#334870, @micchie.gml_gmail.com wrote:

In D15706#334711, @jtl wrote:

In D15706#334605, @rwatson wrote:

Didn't quite catch this before it was committed. This isn't really a destructor, it's a close notification. Rather than confuse matters, as sockets have UMA destructors as well, this should probably be so_notify_close.

Yes, I caught that when I asked the submitter about the placement of the "destructor". But, once I figured out it was a close notification, I should have changed the name. Mea cupla.

Before changing this, let me see if I can confirm what the Linux implementation does.

Thanks, it is intended to be equivalent to sk_destruct() callback in struct sock in Linux.

Is the Linux sk_destruct called on socket close, or on socket destruction? If we want an actual socket destructor, we can add one of those [instead / as well], called from the socket destructor function.

It is literally destructor, which is called when the final reference count drops to zero. So probably we should define so_dtor() as destructor (so keep the name) and call it after SOCK_UNLOCK() in sofree(). (in Linux it is invoked without locking socket).

Which behavior do you want? As @rwatson indicated, you can both have a destructor and a close callback.

Just the destructor behaviour should suffice, but let me come back soon after checking.

[micchie.gml_gmail.com]

In D15706#336215, @micchie.gml_gmail.com wrote:

In D15706#336214, @jtl wrote:

In D15706#336213, @micchie.gml_gmail.com wrote:

In D15706#335628, @rwatson wrote:

In D15706#334870, @micchie.gml_gmail.com wrote:

In D15706#334711, @jtl wrote:

In D15706#334605, @rwatson wrote:

Didn't quite catch this before it was committed. This isn't really a destructor, it's a close notification. Rather than confuse matters, as sockets have UMA destructors as well, this should probably be so_notify_close.

Yes, I caught that when I asked the submitter about the placement of the "destructor". But, once I figured out it was a close notification, I should have changed the name. Mea cupla.

Before changing this, let me see if I can confirm what the Linux implementation does.

Thanks, it is intended to be equivalent to sk_destruct() callback in struct sock in Linux.

Is the Linux sk_destruct called on socket close, or on socket destruction? If we want an actual socket destructor, we can add one of those [instead / as well], called from the socket destructor function.

It is literally destructor, which is called when the final reference count drops to zero. So probably we should define so_dtor() as destructor (so keep the name) and call it after SOCK_UNLOCK() in sofree(). (in Linux it is invoked without locking socket).

Which behavior do you want? As @rwatson indicated, you can both have a destructor and a close callback.

Just the destructor behaviour should suffice, but let me come back soon after checking.

I confirmed that the destructor behavior i.e., invoke it after SOCK_UNLOCK() in sofree() as below.
Or should I submit a new patch?

--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1025,6 +1025,9 @@ sofree(struct socket *so)
                so->so_error = ECONNABORTED;
        SOCK_UNLOCK(so);
 
+       if (so->so_dtor != NULL)
+               so->so_dtor(so);
+
        VNET_SO_ASSERT(so);
        if (pr->pr_flags & PR_RIGHTS && pr->pr_domain->dom_dispose != NULL)
                (*pr->pr_domain->dom_dispose)(so);
@@ -1101,8 +1104,6 @@ soclose(struct socket *so)
 drop:
        if (so->so_proto->pr_usrreqs->pru_close != NULL)
                (*so->so_proto->pr_usrreqs->pru_close)(so);
-       if (so->so_dtor != NULL)
-               so->so_dtor(so);
 
        SOCK_LOCK(so);
        if ((listening = (so->so_options & SO_ACCEPTCONN))) {