git: a034c0aeccd8 - main - netlink: refactor writer initialization KPI

From: Gleb Smirnoff <glebius_at_FreeBSD.org>
Date: Tue, 03 Dec 2024 20:04:57 UTC
The branch main has been updated by glebius:

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

commit a034c0aeccd899a5872333625771df278e0a8ba7
Author:     Gleb Smirnoff <glebius@FreeBSD.org>
AuthorDate: 2024-12-03 20:04:18 +0000
Commit:     Gleb Smirnoff <glebius@FreeBSD.org>
CommitDate: 2024-12-03 20:04:18 +0000

    netlink: refactor writer initialization KPI
    
    o Allow callers to initialize a writer that will malloc(9) with M_WAITOK.
    o Use size_t for expected malloc size.
    o Use correct types to initialize a group writer.
    o Rename functions into nl_writer_ namespace instead of nlmsg_, cause
      they are working on nl_writer, not on nlmsg.
    o Make the KPI responsible to sparsely initialize the writer structure.
    o Garbage collect chain writer.  Fixes 17083b94a915.
    
    All current consumers are left as is, however some may benefit from
    M_WAITOK allocation as well as supplying a correct expected size.
    
    Reviewed by:            melifaro
    Differential Revision:  https://reviews.freebsd.org/D47549
---
 sys/modules/netlink/Makefile         |  1 -
 sys/netlink/netlink_generic.c        | 12 ++++++-----
 sys/netlink/netlink_glue.c           | 42 +++++++++++-------------------------
 sys/netlink/netlink_io.c             |  4 ++--
 sys/netlink/netlink_message_writer.c | 24 +++++++++++++--------
 sys/netlink/netlink_message_writer.h | 20 +++++++++--------
 sys/netlink/netlink_module.c         |  4 ++--
 sys/netlink/netlink_sysevent.c       |  5 +++--
 sys/netlink/netlink_var.h            |  7 +++---
 sys/netlink/route/iface.c            | 11 +++++-----
 sys/netlink/route/neigh.c            |  5 +++--
 sys/netlink/route/nexthop.c          | 11 +++++-----
 sys/netlink/route/rt.c               |  9 ++++----
 13 files changed, 76 insertions(+), 79 deletions(-)

diff --git a/sys/modules/netlink/Makefile b/sys/modules/netlink/Makefile
index 6835be6e7bd7..4abef5106899 100644
--- a/sys/modules/netlink/Makefile
+++ b/sys/modules/netlink/Makefile
@@ -10,7 +10,6 @@ SRCS+=	opt_inet.h opt_inet6.h opt_route.h
 CFLAGS+=	-DNETLINK_MODULE
 
 EXPORT_SYMS=
-EXPORT_SYMS+=	nlmsg_get_chain_writer
 EXPORT_SYMS+=	nlmsg_refill_buffer
 EXPORT_SYMS+=	nlmsg_end
 EXPORT_SYMS+=	nlmsg_flush
diff --git a/sys/netlink/netlink_generic.c b/sys/netlink/netlink_generic.c
index 28f9fe108ef4..4a3c83da57e1 100644
--- a/sys/netlink/netlink_generic.c
+++ b/sys/netlink/netlink_generic.c
@@ -256,14 +256,16 @@ nlctrl_notify(void *arg __unused, const struct genl_family *gf, int cmd)
 {
 	struct nlmsghdr hdr = {.nlmsg_type = NETLINK_GENERIC };
 	struct genlmsghdr ghdr = { .cmd = cmd };
-	struct nl_writer nw = {};
+	struct nl_writer nw;
 
-	if (nlmsg_get_group_writer(&nw, NLMSG_SMALL, NETLINK_GENERIC, ctrl_group_id)) {
-		dump_family(&hdr, &ghdr, gf, &nw);
-		nlmsg_flush(&nw);
+	if (!nl_writer_group(&nw, NLMSG_SMALL, NETLINK_GENERIC, ctrl_group_id,
+	    false)) {
+		NL_LOG(LOG_DEBUG, "error allocating group writer");
 		return;
 	}
-	NL_LOG(LOG_DEBUG, "error allocating group writer");
+
+	dump_family(&hdr, &ghdr, gf, &nw);
+	nlmsg_flush(&nw);
 }
 
 static const struct genl_cmd nlctrl_cmds[] = {
diff --git a/sys/netlink/netlink_glue.c b/sys/netlink/netlink_glue.c
index 164bea1aba61..efb99a717b89 100644
--- a/sys/netlink/netlink_glue.c
+++ b/sys/netlink/netlink_glue.c
@@ -110,28 +110,15 @@ nlp_unconstrained_vnet(const struct nlpcb *nlp)
 /* Stub implementations for the loadable functions */
 
 static bool
-get_stub_writer(struct nl_writer *nw)
-{
-	bzero(nw, sizeof(*nw));
-	nw->enomem = true;
-
-	return (false);
-}
-
-static bool
-nlmsg_get_unicast_writer_stub(struct nl_writer *nw, int size, struct nlpcb *nlp)
+nl_writer_unicast_stub(struct nl_writer *nw, size_t size, struct nlpcb *nlp,
+    bool waitok)
 {
 	return (get_stub_writer(nw));
 }
 
 static bool
-nlmsg_get_group_writer_stub(struct nl_writer *nw, int size, int protocol, int group_id)
-{
-	return (get_stub_writer(nw));
-}
-
-static bool
-nlmsg_get_chain_writer_stub(struct nl_writer *nw, int size, struct mbuf **pm)
+nl_writer_group_stub(struct nl_writer *nw, size_t size, uint16_t protocol,
+    uint16_t group_id, bool waitok)
 {
 	return (get_stub_writer(nw));
 }
@@ -203,9 +190,8 @@ const static struct nl_function_wrapper nl_stub = {
 	.nlmsg_end = nlmsg_end_stub,
 	.nlmsg_abort = nlmsg_abort_stub,
 	.nlmsg_ignore_limit = nlmsg_ignore_limit_stub,
-	.nlmsg_get_unicast_writer = nlmsg_get_unicast_writer_stub,
-	.nlmsg_get_group_writer = nlmsg_get_group_writer_stub,
-	.nlmsg_get_chain_writer = nlmsg_get_chain_writer_stub,
+	.nl_writer_unicast = nl_writer_unicast_stub,
+	.nl_writer_group = nl_writer_group_stub,
 	.nlmsg_end_dump = nlmsg_end_dump_stub,
 	.nl_modify_ifp_generic = nl_modify_ifp_generic_stub,
 	.nl_store_ifp_cookie = nl_store_ifp_cookie_stub,
@@ -226,21 +212,17 @@ nl_set_functions(const struct nl_function_wrapper *nl)
 
 /* Function wrappers */
 bool
-nlmsg_get_unicast_writer(struct nl_writer *nw, int size, struct nlpcb *nlp)
-{
-	return (_nl->nlmsg_get_unicast_writer(nw, size, nlp));
-}
-
-bool
-nlmsg_get_group_writer(struct nl_writer *nw, int size, int protocol, int group_id)
+nl_writer_unicast(struct nl_writer *nw, size_t size, struct nlpcb *nlp,
+    bool waitok)
 {
-	return (_nl->nlmsg_get_group_writer(nw, size, protocol, group_id));
+	return (_nl->nl_writer_unicast(nw, size, nlp, waitok));
 }
 
 bool
-nlmsg_get_chain_writer(struct nl_writer *nw, int size, struct mbuf **pm)
+nl_writer_group(struct nl_writer *nw, size_t size, uint16_t protocol,
+    uint16_t group_id, bool waitok)
 {
-	return (_nl->nlmsg_get_chain_writer(nw, size, pm));
+	return (_nl->nl_writer_group(nw, size, protocol, group_id, waitok));
 }
 
 bool
diff --git a/sys/netlink/netlink_io.c b/sys/netlink/netlink_io.c
index 2dd49d5e8eb5..bc8d2f9fcc3f 100644
--- a/sys/netlink/netlink_io.c
+++ b/sys/netlink/netlink_io.c
@@ -317,13 +317,13 @@ npt_clear(struct nl_pstate *npt)
 static bool
 nl_process_nbuf(struct nl_buf *nb, struct nlpcb *nlp)
 {
+	struct nl_writer nw;
 	struct nlmsghdr *hdr;
 	int error;
 
 	NL_LOG(LOG_DEBUG3, "RX netlink buf %p on %p", nb, nlp->nl_socket);
 
-	struct nl_writer nw = {};
-	if (!nlmsg_get_unicast_writer(&nw, NLMSG_SMALL, nlp)) {
+	if (!nl_writer_unicast(&nw, NLMSG_SMALL, nlp, false)) {
 		NL_LOG(LOG_DEBUG, "error allocating socket writer");
 		return (true);
 	}
diff --git a/sys/netlink/netlink_message_writer.c b/sys/netlink/netlink_message_writer.c
index 59bc6aabaa3f..ee1dab42da74 100644
--- a/sys/netlink/netlink_message_writer.c
+++ b/sys/netlink/netlink_message_writer.c
@@ -73,22 +73,28 @@ nl_send_one(struct nl_writer *nw)
 }
 
 bool
-_nlmsg_get_unicast_writer(struct nl_writer *nw, int size, struct nlpcb *nlp)
+_nl_writer_unicast(struct nl_writer *nw, size_t size, struct nlpcb *nlp,
+    bool waitok)
 {
-	nw->nlp = nlp;
-	nw->cb = nl_send_one;
+	*nw = (struct nl_writer){
+		.nlp = nlp,
+		.cb = nl_send_one,
+	};
 
-	return (nlmsg_get_buf(nw, size, false));
+	return (nlmsg_get_buf(nw, size, waitok));
 }
 
 bool
-_nlmsg_get_group_writer(struct nl_writer *nw, int size, int protocol, int group_id)
+_nl_writer_group(struct nl_writer *nw, size_t size, uint16_t protocol,
+    uint16_t group_id, bool waitok)
 {
-	nw->group.proto = protocol;
-	nw->group.id = group_id;
-	nw->cb = nl_send_group;
+	*nw = (struct nl_writer){
+		.group.proto = protocol,
+		.group.id = group_id,
+		.cb = nl_send_group,
+	};
 
-	return (nlmsg_get_buf(nw, size, false));
+	return (nlmsg_get_buf(nw, size, waitok));
 }
 
 void
diff --git a/sys/netlink/netlink_message_writer.h b/sys/netlink/netlink_message_writer.h
index cb771cf93e53..8cf6951df40c 100644
--- a/sys/netlink/netlink_message_writer.h
+++ b/sys/netlink/netlink_message_writer.h
@@ -66,8 +66,8 @@ struct nl_writer {
 #if defined(NETLINK) || defined(NETLINK_MODULE)
 /* Provide optimized calls to the functions inside the same linking unit */
 
-bool _nlmsg_get_unicast_writer(struct nl_writer *nw, int expected_size, struct nlpcb *nlp);
-bool _nlmsg_get_group_writer(struct nl_writer *nw, int expected_size, int proto, int group_id);
+bool _nl_writer_unicast(struct nl_writer *, size_t, struct nlpcb *nlp, bool);
+bool _nl_writer_group(struct nl_writer *, size_t, uint16_t, uint16_t, bool);
 bool _nlmsg_flush(struct nl_writer *nw);
 void _nlmsg_ignore_limit(struct nl_writer *nw);
 
@@ -81,15 +81,17 @@ bool _nlmsg_end_dump(struct nl_writer *nw, int error, struct nlmsghdr *hdr);
 
 
 static inline bool
-nlmsg_get_unicast_writer(struct nl_writer *nw, int expected_size, struct nlpcb *nlp)
+nl_writer_unicast(struct nl_writer *nw, size_t size, struct nlpcb *nlp,
+    bool waitok)
 {
-	return (_nlmsg_get_unicast_writer(nw, expected_size, nlp));
+	return (_nl_writer_unicast(nw, size, nlp, waitok));
 }
 
 static inline bool
-nlmsg_get_group_writer(struct nl_writer *nw, int expected_size, int proto, int group_id)
+nl_writer_group(struct nl_writer *nw, size_t size, uint16_t proto,
+    uint16_t group_id, bool waitok)
 {
-	return (_nlmsg_get_group_writer(nw, expected_size, proto, group_id));
+	return (_nl_writer_group(nw, size, proto, group_id, waitok));
 }
 
 static inline bool
@@ -138,9 +140,9 @@ nlmsg_end_dump(struct nl_writer *nw, int error, struct nlmsghdr *hdr)
 #else
 /* Provide access to the functions via netlink_glue.c */
 
-bool nlmsg_get_unicast_writer(struct nl_writer *nw, int expected_size, struct nlpcb *nlp);
-bool nlmsg_get_group_writer(struct nl_writer *nw, int expected_size, int proto, int group_id);
-bool nlmsg_get_chain_writer(struct nl_writer *nw, int expected_size, struct mbuf **pm);
+bool nl_writer_unicast(struct nl_writer *, size_t, struct nlpcb *, bool waitok);
+bool nl_writer_group(struct nl_writer *, size_t, uint16_t, uint16_t,
+    bool waitok);
 bool nlmsg_flush(struct nl_writer *nw);
 void nlmsg_ignore_limit(struct nl_writer *nw);
 
diff --git a/sys/netlink/netlink_module.c b/sys/netlink/netlink_module.c
index ddae4488987b..cf119a74cfb7 100644
--- a/sys/netlink/netlink_module.c
+++ b/sys/netlink/netlink_module.c
@@ -179,8 +179,8 @@ const static struct nl_function_wrapper nl_module = {
 	.nlmsg_flush = _nlmsg_flush,
 	.nlmsg_end = _nlmsg_end,
 	.nlmsg_abort = _nlmsg_abort,
-	.nlmsg_get_unicast_writer = _nlmsg_get_unicast_writer,
-	.nlmsg_get_group_writer = _nlmsg_get_group_writer,
+	.nl_writer_unicast = _nl_writer_unicast,
+	.nl_writer_group = _nl_writer_group,
 	.nlmsg_end_dump = _nlmsg_end_dump,
 	.nl_modify_ifp_generic = _nl_modify_ifp_generic,
 	.nl_store_ifp_cookie = _nl_store_ifp_cookie,
diff --git a/sys/netlink/netlink_sysevent.c b/sys/netlink/netlink_sysevent.c
index 01d0837b14d2..3359c77fa303 100644
--- a/sys/netlink/netlink_sysevent.c
+++ b/sys/netlink/netlink_sysevent.c
@@ -80,9 +80,10 @@ static void
 sysevent_write(struct sysevent_group *se, const char *subsystem, const char *type,
     const char *data)
 {
-	struct nl_writer nw = {};
+	struct nl_writer nw;
 
-	if (!nlmsg_get_group_writer(&nw, NLMSG_LARGE, NETLINK_GENERIC, se->id)) {
+	if (!nl_writer_group(&nw, NLMSG_LARGE, NETLINK_GENERIC, se->id,
+	    false)) {
 		NL_LOG(LOG_DEBUG, "error allocating group writer");
 		return;
 	}
diff --git a/sys/netlink/netlink_var.h b/sys/netlink/netlink_var.h
index 7557bd2c06a5..c09e279205f0 100644
--- a/sys/netlink/netlink_var.h
+++ b/sys/netlink/netlink_var.h
@@ -184,9 +184,10 @@ struct nl_function_wrapper {
 	bool (*nlmsg_end)(struct nl_writer *nw);
 	void (*nlmsg_abort)(struct nl_writer *nw);
 	void (*nlmsg_ignore_limit)(struct nl_writer *nw);
-	bool (*nlmsg_get_unicast_writer)(struct nl_writer *nw, int size, struct nlpcb *nlp);
-	bool (*nlmsg_get_group_writer)(struct nl_writer *nw, int size, int protocol, int group_id);
-	bool (*nlmsg_get_chain_writer)(struct nl_writer *nw, int size, struct mbuf **pm);
+	bool (*nl_writer_unicast)(struct nl_writer *nw, size_t size,
+	    struct nlpcb *nlp, bool waitok);
+	bool (*nl_writer_group)(struct nl_writer *nw, size_t size,
+	    uint16_t protocol, uint16_t group_id, bool waitok);
 	bool (*nlmsg_end_dump)(struct nl_writer *nw, int error, struct nlmsghdr *hdr);
 	int (*nl_modify_ifp_generic)(struct ifnet *ifp, struct nl_parsed_link *lattrs,
 	    const struct nlattr_bmask *bm, struct nl_pstate *npt);
diff --git a/sys/netlink/route/iface.c b/sys/netlink/route/iface.c
index e6a3d7d00a8f..c19161f5bbb7 100644
--- a/sys/netlink/route/iface.c
+++ b/sys/netlink/route/iface.c
@@ -1363,7 +1363,7 @@ static void
 rtnl_handle_ifaddr(void *arg __unused, struct ifaddr *ifa, int cmd)
 {
 	struct nlmsghdr hdr = {};
-	struct nl_writer nw = {};
+	struct nl_writer nw;
 	uint32_t group = 0;
 
 	switch (ifa->ifa_addr->sa_family) {
@@ -1386,7 +1386,7 @@ rtnl_handle_ifaddr(void *arg __unused, struct ifaddr *ifa, int cmd)
 	if (!nl_has_listeners(NETLINK_ROUTE, group))
 		return;
 
-	if (!nlmsg_get_group_writer(&nw, NLMSG_LARGE, NETLINK_ROUTE, group)) {
+	if (!nl_writer_group(&nw, NLMSG_LARGE, NETLINK_ROUTE, group, false)) {
 		NL_LOG(LOG_DEBUG, "error allocating group writer");
 		return;
 	}
@@ -1401,13 +1401,14 @@ static void
 rtnl_handle_ifevent(if_t ifp, int nlmsg_type, int if_flags_mask)
 {
 	struct nlmsghdr hdr = { .nlmsg_type = nlmsg_type };
-	struct nl_writer nw = {};
+	struct nl_writer nw;
 
 	if (!nl_has_listeners(NETLINK_ROUTE, RTNLGRP_LINK))
 		return;
 
-	if (!nlmsg_get_group_writer(&nw, NLMSG_LARGE, NETLINK_ROUTE, RTNLGRP_LINK)) {
-		NL_LOG(LOG_DEBUG, "error allocating mbuf");
+	if (!nl_writer_group(&nw, NLMSG_LARGE, NETLINK_ROUTE, RTNLGRP_LINK,
+	    false)) {
+		NL_LOG(LOG_DEBUG, "error allocating group writer");
 		return;
 	}
 	dump_iface(&nw, ifp, &hdr, if_flags_mask);
diff --git a/sys/netlink/route/neigh.c b/sys/netlink/route/neigh.c
index 5be0c1f9d91f..c5003eddaba1 100644
--- a/sys/netlink/route/neigh.c
+++ b/sys/netlink/route/neigh.c
@@ -552,6 +552,7 @@ static const struct rtnl_cmd_handler cmd_handlers[] = {
 static void
 rtnl_lle_event(void *arg __unused, struct llentry *lle, int evt)
 {
+	struct nl_writer nw;
 	if_t ifp;
 	int family;
 
@@ -565,8 +566,8 @@ rtnl_lle_event(void *arg __unused, struct llentry *lle, int evt)
 
 	int nlmsgs_type = evt == LLENTRY_RESOLVED ? NL_RTM_NEWNEIGH : NL_RTM_DELNEIGH;
 
-	struct nl_writer nw = {};
-	if (!nlmsg_get_group_writer(&nw, NLMSG_SMALL, NETLINK_ROUTE, RTNLGRP_NEIGH)) {
+	if (!nl_writer_group(&nw, NLMSG_SMALL, NETLINK_ROUTE, RTNLGRP_NEIGH,
+	    false)) {
 		NL_LOG(LOG_DEBUG, "error allocating group writer");
 		return;
 	}
diff --git a/sys/netlink/route/nexthop.c b/sys/netlink/route/nexthop.c
index 0d6bb5c9ec84..90ed3497047e 100644
--- a/sys/netlink/route/nexthop.c
+++ b/sys/netlink/route/nexthop.c
@@ -506,7 +506,7 @@ static int
 delete_unhop(struct unhop_ctl *ctl, struct nlmsghdr *hdr, uint32_t uidx)
 {
 	struct user_nhop *unhop_ret, *unhop_base, *unhop_chain;
-
+	struct nl_writer nw;
 	struct user_nhop key = { .un_idx = uidx };
 
 	UN_WLOCK(ctl);
@@ -553,8 +553,8 @@ delete_unhop(struct unhop_ctl *ctl, struct nlmsghdr *hdr, uint32_t uidx)
 		.hdr.nlmsg_type = NL_RTM_DELNEXTHOP,
 	};
 
-	struct nl_writer nw = {};
-	if (!nlmsg_get_group_writer(&nw, NLMSG_SMALL, NETLINK_ROUTE, RTNLGRP_NEXTHOP)) {
+	if (!nl_writer_group(&nw, NLMSG_SMALL, NETLINK_ROUTE, RTNLGRP_NEXTHOP,
+	    false)) {
 		NL_LOG(LOG_DEBUG, "error allocating message writer");
 		return (ENOMEM);
 	}
@@ -881,6 +881,7 @@ static int
 rtnl_handle_newnhop(struct nlmsghdr *hdr, struct nlpcb *nlp,
     struct nl_pstate *npt)
 {
+	struct nl_writer nw;
 	struct user_nhop *unhop;
 	int error;
 
@@ -947,8 +948,8 @@ rtnl_handle_newnhop(struct nlmsghdr *hdr, struct nlpcb *nlp,
 		.hdr.nlmsg_type = NL_RTM_NEWNEXTHOP,
 	};
 
-	struct nl_writer nw = {};
-	if (!nlmsg_get_group_writer(&nw, NLMSG_SMALL, NETLINK_ROUTE, RTNLGRP_NEXTHOP)) {
+	if (!nl_writer_group(&nw, NLMSG_SMALL, NETLINK_ROUTE, RTNLGRP_NEXTHOP,
+	    false)) {
 		NL_LOG(LOG_DEBUG, "error allocating message writer");
 		return (ENOMEM);
 	}
diff --git a/sys/netlink/route/rt.c b/sys/netlink/route/rt.c
index 00315afbcb02..c9b0f56498ec 100644
--- a/sys/netlink/route/rt.c
+++ b/sys/netlink/route/rt.c
@@ -350,10 +350,10 @@ static void
 report_operation(uint32_t fibnum, struct rib_cmd_info *rc,
     struct nlpcb *nlp, struct nlmsghdr *hdr)
 {
-	struct nl_writer nw = {};
+	struct nl_writer nw;
 	uint32_t group_id = family_to_group(rt_get_family(rc->rc_rt));
 
-	if (nlmsg_get_group_writer(&nw, NLMSG_SMALL, NETLINK_ROUTE, group_id)) {
+	if (nl_writer_group(&nw, NLMSG_SMALL, NETLINK_ROUTE, group_id, false)) {
 		struct route_nhop_data rnd = {
 			.rnd_nhop = rc_get_nhop(rc),
 			.rnd_weight = rc->rc_nh_weight,
@@ -1043,7 +1043,7 @@ rtnl_handle_getroute(struct nlmsghdr *hdr, struct nlpcb *nlp, struct nl_pstate *
 void
 rtnl_handle_route_event(uint32_t fibnum, const struct rib_cmd_info *rc)
 {
-	struct nl_writer nw = {};
+	struct nl_writer nw;
 	int family, nlm_flags = 0;
 
 	family = rt_get_family(rc->rc_rt);
@@ -1082,7 +1082,8 @@ rtnl_handle_route_event(uint32_t fibnum, const struct rib_cmd_info *rc)
 	};
 
 	uint32_t group_id = family_to_group(family);
-	if (!nlmsg_get_group_writer(&nw, NLMSG_SMALL, NETLINK_ROUTE, group_id)) {
+	if (!nl_writer_group(&nw, NLMSG_SMALL, NETLINK_ROUTE, group_id,
+	    false)) {
 		NL_LOG(LOG_DEBUG, "error allocating event buffer");
 		return;
 	}