Re: kern/55777: more rump fixes

[Ruslan Nikolaev <nruslan_devel%yahoo.com@localhost>]

Christos,

Looks like this function (unp_sysctl_create) was added by you some time 
ago. I looked at the code and here is what I found:

1. liblocal uses uipc_usrreq.c (no uipc_domain.c)

2. libnet uses uipc_domain.c (no uipc_usrreq.c)

uipc_domain.c calls unp_sysctl_create which is defined uipc_usrreq.c. 
For that reason, libnet defines a dummy function in net_stub.c to avoid 
this dependency.

However, looking at the code, this function seems to simply call 
sysctl_createv. Do we really need it to be connected to uipc_domain.c 
anyhow? (I apologize if I miss something, I am not so familiar with that 
code.)

Below, I created a patch which separates uipc_usrreq.c from 
uipc_domain.c and removes a dummy function from net_stub.c. Is it 
acceptable or there some reason to not do it?

The code (at least) compiles without any issue on rumprun.

diff --git a/sys/kern/uipc_domain.c b/sys/kern/uipc_domain.c
index edd7d7a28df0..8a6b8bc1cb31 100644
--- a/sys/kern/uipc_domain.c
+++ b/sys/kern/uipc_domain.c
@@ -692,7 +692,6 @@ sysctl_net_setup(void)
���� ��� ������ SYSCTL_DESCR("SOCK_DGRAM protocol control block list"),
���� ��� ������ sysctl_unpcblist, 0, NULL, 0,
���� ��� ������ CTL_NET, PF_LOCAL, SOCK_DGRAM, CTL_CREATE, CTL_EOL);
-��� unp_sysctl_create(&domain_sysctllog);
�}

�void
diff --git a/sys/kern/uipc_usrreq.c b/sys/kern/uipc_usrreq.c
index 6c1ca5a1cb3d..3d8d4118c12d 100644
--- a/sys/kern/uipc_usrreq.c
+++ b/sys/kern/uipc_usrreq.c
@@ -111,6 +111,7 @@ __KERNEL_RCSID(0, "$NetBSD: uipc_usrreq.c,v 1.199 
2020/08/26 22:54:30 christos E
�#include <sys/socket.h>
�#include <sys/socketvar.h>
�#include <sys/unpcb.h>
+#include <sys/sysctl.h>
�#include <sys/un.h>
�#include <sys/namei.h>
�#include <sys/vnode.h>
@@ -197,6 +198,9 @@ static lwp_t *unp_thread_lwp;
�static SLIST_HEAD(,file) unp_thread_discard;
�static int unp_defer;

+static struct sysctllog *usrreq_sysctllog;
+static void unp_sysctl_create(void);
+
�/* Compat interface */

�struct mbuf * stub_compat_70_unp_addsockcred(lwp_t *, struct mbuf *);
@@ -219,6 +223,8 @@ uipc_init(void)
�{
���� int error;

+��� unp_sysctl_create();
+
���� uipc_lock = mutex_obj_alloc(MUTEX_DEFAULT, IPL_NONE);
���� cv_init(&unp_thread_cv, "unpgc");

@@ -1988,40 +1994,42 @@ unp_discard_later(file_t *fp)
���� mutex_exit(&filelist_lock);
�}

-void
-unp_sysctl_create(struct sysctllog **clog)
+static void
+unp_sysctl_create(void)
�{
-��� sysctl_createv(clog, 0, NULL, NULL,
+
+��� KASSERT(usrreq_sysctllog == NULL);
+��� sysctl_createv(&usrreq_sysctllog, 0, NULL, NULL,
���� ��� ������ CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
���� ��� ������ CTLTYPE_LONG, "sendspace",
���� ��� ������ SYSCTL_DESCR("Default stream send space"),
���� ��� ������ NULL, 0, &unpst_sendspace, 0,
���� ��� ������ CTL_NET, PF_LOCAL, SOCK_STREAM, CTL_CREATE, CTL_EOL);
-��� sysctl_createv(clog, 0, NULL, NULL,
+��� sysctl_createv(&usrreq_sysctllog, 0, NULL, NULL,
���� ��� ������ CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
���� ��� ������ CTLTYPE_LONG, "recvspace",
���� ��� ������ SYSCTL_DESCR("Default stream recv space"),
���� ��� ������ NULL, 0, &unpst_recvspace, 0,
���� ��� ������ CTL_NET, PF_LOCAL, SOCK_STREAM, CTL_CREATE, CTL_EOL);
-��� sysctl_createv(clog, 0, NULL, NULL,
+��� sysctl_createv(&usrreq_sysctllog, 0, NULL, NULL,
���� ��� ������ CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
���� ��� ������ CTLTYPE_LONG, "sendspace",
���� ��� ������ SYSCTL_DESCR("Default datagram send space"),
���� ��� ������ NULL, 0, &unpdg_sendspace, 0,
���� ��� ������ CTL_NET, PF_LOCAL, SOCK_DGRAM, CTL_CREATE, CTL_EOL);
-��� sysctl_createv(clog, 0, NULL, NULL,
+��� sysctl_createv(&usrreq_sysctllog, 0, NULL, NULL,
���� ��� ������ CTLFLAG_PERMANENT|CTLFLAG_READWRITE,
���� ��� ������ CTLTYPE_LONG, "recvspace",
���� ��� ������ SYSCTL_DESCR("Default datagram recv space"),
���� ��� ������ NULL, 0, &unpdg_recvspace, 0,
���� ��� ������ CTL_NET, PF_LOCAL, SOCK_DGRAM, CTL_CREATE, CTL_EOL);
-��� sysctl_createv(clog, 0, NULL, NULL,
+��� sysctl_createv(&usrreq_sysctllog, 0, NULL, NULL,
���� ��� ������ CTLFLAG_PERMANENT|CTLFLAG_READONLY,
���� ��� ������ CTLTYPE_INT, "inflight",
���� ��� ������ SYSCTL_DESCR("File descriptors in flight"),
���� ��� ������ NULL, 0, &unp_rights, 0,
���� ��� ������ CTL_NET, PF_LOCAL, CTL_CREATE, CTL_EOL);
-��� sysctl_createv(clog, 0, NULL, NULL,
+��� sysctl_createv(&usrreq_sysctllog, 0, NULL, NULL,
���� ��� ������ CTLFLAG_PERMANENT|CTLFLAG_READONLY,
���� ��� ������ CTLTYPE_INT, "deferred",
���� ��� ������ SYSCTL_DESCR("File descriptors deferred for close"),
diff --git a/sys/rump/librump/rumpnet/net_stub.c 
b/sys/rump/librump/rumpnet/net_stub.c
index e43cdec4ab3f..9810adf9aa13 100644
--- a/sys/rump/librump/rumpnet/net_stub.c
+++ b/sys/rump/librump/rumpnet/net_stub.c
@@ -34,8 +34,6 @@ __KERNEL_RCSID(0, "$NetBSD: net_stub.c,v 1.40 
2020/09/27 00:34:44 roy Exp $");
�#include <sys/socketvar.h>
�#include <sys/pslist.h>
�#include <sys/psref.h>
-#include <sys/sysctl.h>
-#include <sys/un.h>

�#include <net/if.h>
�#include <net/route.h>
@@ -88,12 +86,6 @@ int ipsec_used;
�percpu_t *ipsecstat_percpu;
�u_int ipsec_spdgen;

-/* sysctl */
-void
-unp_sysctl_create(struct sysctllog **clog)
-{
-}
-
�__weak_alias(ah4_ctlinput,rumpnet_stub);
�__weak_alias(ah6_ctlinput,rumpnet_stub);
�__weak_alias(esp4_ctlinput,rumpnet_stub);
diff --git a/sys/sys/un.h b/sys/sys/un.h
index 2533ac7f2169..b5dde2de339d 100644
--- a/sys/sys/un.h
+++ b/sys/sys/un.h
@@ -90,7 +90,6 @@ int��� unp_connect(struct socket *, struct sockaddr *, 
struct lwp *);
�int��� unp_connect2(struct socket *, struct socket *);
�void ��� unp_dispose(struct mbuf *);
�int ��� unp_externalize(struct mbuf *, struct lwp *, int);
-void��� unp_sysctl_create(struct sysctllog **);

�#else /* !_KERNEL */


On 11/3/20 12:06 PM, Christos Zoulas wrote:
> > On Nov 3, 2020, at 11:17 AM, Jason Thorpe <thorpej%me.com@localhost> wrote:
> >
> > Seems like we should just put it all into a single rump library.  If it's a static link, it'll naturally reduce the code slurped in, and if it's a shared library, who cares?
> Unfortunately in practice it does not reduce the code slurped in because of the same variable interdependencies. It is also better to reduce the memory size of the image so that the minimum memory requirements to run the process are smaller. We have been fixing those duplications as we find them, but we have not proactively tried to eliminate them.
>
> christos