git: ff5ad900d2a0 - main - netlink: refactor control data generation for recvmsg(2)

The branch main has been updated by glebius:

URL: https://cgit.FreeBSD.org/src/commit/?id=ff5ad900d2a0793659241eee96be53e6053b5081

commit ff5ad900d2a0793659241eee96be53e6053b5081
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2024-01-02 21:05:46 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2024-01-02 21:05:46 +0000

    netlink: refactor control data generation for recvmsg(2)
    
    Netlink should return a very simple control data on every recvmsg(2)
    syscall.  This data is associated with a syscall, not with an nlmsg,
    neither with internal our internal representation (nl_bufs).  There is
    no need to pre-allocate it in non-sleepable context and attach to
    nl_buf.  Allocate right in the syscall with M_WAITOK.  This also
    shaves lots of code and simplifies things.
    
    Reviewed by:            melifaro
    Differential Revision:  https://reviews.freebsd.org/D42989
---
 sys/netlink/netlink_domain.c | 64 +++++++++++++++++++++++---------------------
 sys/netlink/netlink_io.c     | 43 -----------------------------
 sys/netlink/netlink_var.h    |  2 --
 3 files changed, 34 insertions(+), 75 deletions(-)

diff --git a/sys/netlink/netlink_domain.c b/sys/netlink/netlink_domain.c
index 94d8377b9e15..cc7018ed2582 100644
--- a/sys/netlink/netlink_domain.c
+++ b/sys/netlink/netlink_domain.c
@@ -179,15 +179,6 @@ nl_get_groups_compat(struct nlpcb *nlp)
 	return (groups_mask);
 }
 
-static void
-nl_send_one_group(struct nl_writer *nw, struct nl_buf *nb, struct nlpcb *nlp)
-{
-	if (__predict_false(nlp->nl_flags & NLF_MSG_INFO))
-		nl_add_msg_info(nb);
-	nw->buf = nb;
-	(void)nl_send_one(nw);
-}
-
 static struct nl_buf *
 nl_buf_copy(struct nl_buf *nb)
 {
@@ -197,13 +188,6 @@ nl_buf_copy(struct nl_buf *nb)
 	if (__predict_false(copy == NULL))
 		return (NULL);
 	memcpy(copy, nb, sizeof(*nb) + nb->buflen);
-	if (nb->control != NULL) {
-		copy->control = m_copym(nb->control, 0, M_COPYALL, M_NOWAIT);
-		if (__predict_false(copy->control == NULL)) {
-			nl_buf_free(copy);
-			return (NULL);
-		}
-	}
 
 	return (copy);
 }
@@ -248,7 +232,8 @@ nl_send_group(struct nl_writer *nw)
 
 				copy = nl_buf_copy(nb);
 				if (copy != NULL) {
-					nl_send_one_group(nw, copy, nlp_last);
+					nw->buf = copy;
+					(void)nl_send_one(nw);
 				} else {
 					NLP_LOCK(nlp_last);
 					if (nlp_last->nl_socket != NULL)
@@ -259,9 +244,10 @@ nl_send_group(struct nl_writer *nw)
 			nlp_last = nlp;
 		}
 	}
-	if (nlp_last != NULL)
-		nl_send_one_group(nw, nb, nlp_last);
-	else
+	if (nlp_last != NULL) {
+		nw->buf = nb;
+		(void)nl_send_one(nw);
+	} else
 		nl_buf_free(nb);
 
 	NLCTL_RUNLOCK(ctl);
@@ -657,6 +643,30 @@ out:
 	return (error);
 }
 
+/* Create control data for recvmsg(2) on Netlink socket. */
+static struct mbuf *
+nl_createcontrol(struct nlpcb *nlp)
+{
+	struct {
+		struct nlattr nla;
+		uint32_t val;
+	} data[] = {
+		{
+			.nla.nla_len = sizeof(struct nlattr) + sizeof(uint32_t),
+			.nla.nla_type = NLMSGINFO_ATTR_PROCESS_ID,
+			.val = nlp->nl_process_id,
+		},
+		{
+			.nla.nla_len = sizeof(struct nlattr) + sizeof(uint32_t),
+			.nla.nla_type = NLMSGINFO_ATTR_PORT_ID,
+			.val = nlp->nl_port,
+		},
+	};
+
+	return (sbcreatecontrol(data, sizeof(data), NETLINK_MSG_INFO,
+	    SOL_NETLINK, M_WAITOK));
+}
+
 static int
 nl_soreceive(struct socket *so, struct sockaddr **psa, struct uio *uio,
     struct mbuf **mp, struct mbuf **controlp, int *flagsp)
@@ -667,6 +677,7 @@ nl_soreceive(struct socket *so, struct sockaddr **psa, struct uio *uio,
 		.nl_pid = 0 /* comes from the kernel */
 	};
 	struct sockbuf *sb = &so->so_rcv;
+	struct nlpcb *nlp = sotonlpcb(so);
 	struct nl_buf *first, *last, *nb, *next;
 	struct nlmsghdr *hdr;
 	int flags, error;
@@ -681,6 +692,9 @@ nl_soreceive(struct socket *so, struct sockaddr **psa, struct uio *uio,
 		*psa = sodupsockaddr((const struct sockaddr *)&nl_empty_src,
 		    M_WAITOK);
 
+	if (controlp != NULL && (nlp->nl_flags & NLF_MSG_INFO))
+		*controlp = nl_createcontrol(nlp);
+
 	flags = flagsp != NULL ? *flagsp & ~MSG_TRUNC : 0;
 	trunc = flagsp != NULL ? *flagsp & MSG_TRUNC : false;
 	nonblock = (so->so_state & SS_NBIO) ||
@@ -691,9 +705,6 @@ nl_soreceive(struct socket *so, struct sockaddr **psa, struct uio *uio,
 	if (__predict_false(error))
 		return (error);
 
-	if (controlp != NULL)
-		*controlp = NULL;
-
 	len = 0;
 	overflow = 0;
 	msgrcv = 0;
@@ -798,13 +809,6 @@ nospace:
 
 	for (nb = first; nb != last; nb = next) {
 		next = TAILQ_NEXT(nb, tailq);
-
-		/* XXXGL multiple controls??? */
-		if (controlp != NULL && *controlp == NULL &&
-		    nb->control != NULL && !peek) {
-			*controlp = nb->control;
-			nb->control = NULL;
-		}
 		if (__predict_true(error == 0))
 			error = uiomove(&nb->data[nb->offset],
 			    (int)(nb->datalen - nb->offset), uio);
diff --git a/sys/netlink/netlink_io.c b/sys/netlink/netlink_io.c
index 56e430cdcfa8..fb8e0a46e8dd 100644
--- a/sys/netlink/netlink_io.c
+++ b/sys/netlink/netlink_io.c
@@ -62,7 +62,6 @@ nl_buf_alloc(size_t len, int mflag)
 	if (__predict_true(nb != NULL)) {
 		nb->buflen = len;
 		nb->datalen = nb->offset = 0;
-		nb->control = NULL;
 	}
 
 	return (nb);
@@ -72,51 +71,9 @@ void
 nl_buf_free(struct nl_buf *nb)
 {
 
-	if (nb->control)
-		m_freem(nb->control);
 	free(nb, M_NETLINK);
 }
 
-void
-nl_add_msg_info(struct nl_buf *nb)
-{
-	/* XXXGL pass nlp as arg? */
-	struct nlpcb *nlp = nl_get_thread_nlp(curthread);
-	NL_LOG(LOG_DEBUG2, "Trying to recover nlp from thread %p: %p",
-	    curthread, nlp);
-
-	if (nlp == NULL)
-		return;
-
-	/* Prepare what we want to encode - PID, socket PID & msg seq */
-	struct {
-		struct nlattr nla;
-		uint32_t val;
-	} data[] = {
-		{
-			.nla.nla_len = sizeof(struct nlattr) + sizeof(uint32_t),
-			.nla.nla_type = NLMSGINFO_ATTR_PROCESS_ID,
-			.val = nlp->nl_process_id,
-		},
-		{
-			.nla.nla_len = sizeof(struct nlattr) + sizeof(uint32_t),
-			.nla.nla_type = NLMSGINFO_ATTR_PORT_ID,
-			.val = nlp->nl_port,
-		},
-	};
-
-
-	nb->control = sbcreatecontrol(data, sizeof(data),
-	    NETLINK_MSG_INFO, SOL_NETLINK, M_NOWAIT);
-
-	if (__predict_true(nb->control != NULL))
-		NL_LOG(LOG_DEBUG2, "Storing %u bytes of control data, ctl: %p",
-		    (unsigned)sizeof(data), nb->control);
-	else
-		NL_LOG(LOG_DEBUG2, "Failed to allocate %u bytes of control",
-		    (unsigned)sizeof(data));
-}
-
 void
 nl_schedule_taskqueue(struct nlpcb *nlp)
 {
diff --git a/sys/netlink/netlink_var.h b/sys/netlink/netlink_var.h
index 97532c31e54b..c8f0d02a0dab 100644
--- a/sys/netlink/netlink_var.h
+++ b/sys/netlink/netlink_var.h
@@ -45,7 +45,6 @@ struct ucred;
 
 struct nl_buf {
 	TAILQ_ENTRY(nl_buf)	tailq;
-	struct mbuf		*control;
 	u_int			buflen;
 	u_int			datalen;
 	u_int			offset;
@@ -142,7 +141,6 @@ void nl_taskqueue_handler(void *_arg, int pending);
 void nl_schedule_taskqueue(struct nlpcb *nlp);
 void nl_process_receive_locked(struct nlpcb *nlp);
 void nl_set_source_metadata(struct mbuf *m, int num_messages);
-void nl_add_msg_info(struct nl_buf *nb);
 struct nl_buf *nl_buf_alloc(size_t len, int mflag);
 void nl_buf_free(struct nl_buf *nb);