From: Donald Sharp <sharpd@nvidia.com>
Date: Wed, 2 Feb 2022 18:21:52 +0000 (-0500)
Subject: zebra: Remove `struct nlsock` from dataplane information and use `int fd`
X-Git-Tag: pim6-testing-20220430~340^2~1
X-Git-Url: https://git.puffer.fish/?a=commitdiff_plain;h=d4000d7ba3242c0e6adfa56544a34312b0f566a9;p=matthieu%2Ffrr.git

zebra: Remove `struct nlsock` from dataplane information and use `int fd`

Store the fd that corresponds to the appropriate `struct nlsock` and pass
that around in the dplane context instead of the pointer to the nlsock.
Modify the kernel_netlink.c code to store in a hash the `struct nlsock`
with the socket fd as the key.

Why do this?  The dataplane context is used to pass around the `struct nlsock`
but the zebra code has a bug where the received buffer for kernel netlink
messages from the kernel is not big enough.  So we need to dynamically
grow the receive buffer per socket, instead of having a non-dynamic buffer
that we read into.  By passing around the fd we can look up the `struct nlsock`
that will soon have the associated buffer and not have to worry about `const`
issues that will arise.

Signed-off-by: Donald Sharp <sharpd@nvidia.com>
---

diff --git a/zebra/kernel_netlink.c b/zebra/kernel_netlink.c
index 5991194548..3650d87e0f 100644
--- a/zebra/kernel_netlink.c
+++ b/zebra/kernel_netlink.c
@@ -36,6 +36,7 @@
 #include "vrf.h"
 #include "mpls.h"
 #include "lib_errors.h"
+#include "hash.h"
 
 //#include "zebra/zserv.h"
 #include "zebra/zebra_router.h"
@@ -160,6 +161,7 @@ extern struct zebra_privs_t zserv_privs;
 
 DEFINE_MTYPE_STATIC(ZEBRA, NL_BUF, "Zebra Netlink buffers");
 
+struct hash *nlsock_hash;
 size_t nl_batch_tx_bufsize;
 char *nl_batch_tx_buf;
 
@@ -429,8 +431,10 @@ static int kernel_read(struct thread *thread)
  */
 int kernel_dplane_read(struct zebra_dplane_info *info)
 {
-	netlink_parse_info(dplane_netlink_information_fetch, &info->nls, info,
-			   5, false);
+	struct nlsock *nl = kernel_netlink_nlsock_lookup(info->sock);
+
+	netlink_parse_info(dplane_netlink_information_fetch, nl, info, 5,
+			   false);
 
 	return 0;
 }
@@ -1035,9 +1039,9 @@ netlink_talk_info(int (*filter)(struct nlmsghdr *, ns_id_t, int startup),
 		  struct nlmsghdr *n, const struct zebra_dplane_info *dp_info,
 		  bool startup)
 {
-	const struct nlsock *nl;
+	struct nlsock *nl;
 
-	nl = &(dp_info->nls);
+	nl = kernel_netlink_nlsock_lookup(dp_info->sock);
 	n->nlmsg_seq = dp_info->seq;
 	n->nlmsg_pid = nl->snl.nl_pid;
 
@@ -1109,11 +1113,11 @@ static int nl_batch_read_resp(struct nl_batch *bth)
 	struct sockaddr_nl snl;
 	struct msghdr msg = {};
 	int status, seq;
-	const struct nlsock *nl;
+	struct nlsock *nl;
 	struct zebra_dplane_ctx *ctx;
 	bool ignore_msg;
 
-	nl = &(bth->zns->nls);
+	nl = kernel_netlink_nlsock_lookup(bth->zns->sock);
 
 	msg.msg_name = (void *)&snl;
 	msg.msg_namelen = sizeof(snl);
@@ -1295,13 +1299,15 @@ static void nl_batch_send(struct nl_batch *bth)
 	bool err = false;
 
 	if (bth->curlen != 0 && bth->zns != NULL) {
+		struct nlsock *nl =
+			kernel_netlink_nlsock_lookup(bth->zns->sock);
+
 		if (IS_ZEBRA_DEBUG_KERNEL)
 			zlog_debug("%s: %s, batch size=%zu, msg cnt=%zu",
-				   __func__, bth->zns->nls.name, bth->curlen,
+				   __func__, nl->name, bth->curlen,
 				   bth->msgcnt);
 
-		if (netlink_send_msg(&(bth->zns->nls), bth->buf, bth->curlen)
-		    == -1)
+		if (netlink_send_msg(nl, bth->buf, bth->curlen) == -1)
 			err = true;
 
 		if (!err) {
@@ -1334,6 +1340,7 @@ enum netlink_msg_status netlink_batch_add_msg(
 	int seq;
 	ssize_t size;
 	struct nlmsghdr *msgh;
+	struct nlsock *nl;
 
 	size = (*msg_encoder)(ctx, bth->buf_head, bth->bufsiz - bth->curlen);
 
@@ -1361,12 +1368,14 @@ enum netlink_msg_status netlink_batch_add_msg(
 	}
 
 	seq = dplane_ctx_get_ns(ctx)->seq;
+	nl = kernel_netlink_nlsock_lookup(dplane_ctx_get_ns_sock(ctx));
+
 	if (ignore_res)
 		seq++;
 
 	msgh = (struct nlmsghdr *)bth->buf_head;
 	msgh->nlmsg_seq = seq;
-	msgh->nlmsg_pid = dplane_ctx_get_ns(ctx)->nls.snl.nl_pid;
+	msgh->nlmsg_pid = nl->snl.nl_pid;
 
 	bth->zns = dplane_ctx_get_ns(ctx);
 	bth->buf_head = ((char *)bth->buf_head) + size;
@@ -1496,6 +1505,33 @@ void kernel_update_multi(struct dplane_ctx_q *ctx_list)
 	dplane_ctx_list_append(ctx_list, &handled_list);
 }
 
+struct nlsock *kernel_netlink_nlsock_lookup(int sock)
+{
+	struct nlsock lookup;
+
+	lookup.sock = sock;
+
+	return hash_lookup(nlsock_hash, &lookup);
+}
+
+static uint32_t kernel_netlink_nlsock_key(const void *arg)
+{
+	const struct nlsock *nl = arg;
+
+	return nl->sock;
+}
+
+static bool kernel_netlink_nlsock_hash_equal(const void *arg1, const void *arg2)
+{
+	const struct nlsock *nl1 = arg1;
+	const struct nlsock *nl2 = arg2;
+
+	if (nl1->sock == nl2->sock)
+		return true;
+
+	return false;
+}
+
 /* Exported interface function.  This function simply calls
    netlink_socket (). */
 void kernel_init(struct zebra_ns *zns)
@@ -1505,6 +1541,11 @@ void kernel_init(struct zebra_ns *zns)
 	int one, ret;
 #endif
 
+	if (!nlsock_hash)
+		nlsock_hash = hash_create_size(8, kernel_netlink_nlsock_key,
+					       kernel_netlink_nlsock_hash_equal,
+					       "Netlink Socket Hash");
+
 	/*
 	 * Initialize netlink sockets
 	 *
@@ -1537,6 +1578,7 @@ void kernel_init(struct zebra_ns *zns)
 			 zns->netlink.name);
 		exit(-1);
 	}
+	(void)hash_get(nlsock_hash, &zns->netlink, hash_alloc_intern);
 
 	snprintf(zns->netlink_cmd.name, sizeof(zns->netlink_cmd.name),
 		 "netlink-cmd (NS %u)", zns->ns_id);
@@ -1546,6 +1588,7 @@ void kernel_init(struct zebra_ns *zns)
 			 zns->netlink_cmd.name);
 		exit(-1);
 	}
+	(void)hash_get(nlsock_hash, &zns->netlink_cmd, hash_alloc_intern);
 
 	/* Outbound socket for dplane programming of the host OS. */
 	snprintf(zns->netlink_dplane_out.name,
@@ -1557,6 +1600,8 @@ void kernel_init(struct zebra_ns *zns)
 			 zns->netlink_dplane_out.name);
 		exit(-1);
 	}
+	(void)hash_get(nlsock_hash, &zns->netlink_dplane_out,
+		       hash_alloc_intern);
 
 	/* Inbound socket for OS events coming to the dplane. */
 	snprintf(zns->netlink_dplane_in.name,
@@ -1569,6 +1614,7 @@ void kernel_init(struct zebra_ns *zns)
 			 zns->netlink_dplane_in.name);
 		exit(-1);
 	}
+	(void)hash_get(nlsock_hash, &zns->netlink_dplane_in, hash_alloc_intern);
 
 	/*
 	 * SOL_NETLINK is not available on all platforms yet
@@ -1659,16 +1705,19 @@ void kernel_terminate(struct zebra_ns *zns, bool complete)
 	thread_cancel(&zns->t_netlink);
 
 	if (zns->netlink.sock >= 0) {
+		hash_release(nlsock_hash, &zns->netlink);
 		close(zns->netlink.sock);
 		zns->netlink.sock = -1;
 	}
 
 	if (zns->netlink_cmd.sock >= 0) {
+		hash_release(nlsock_hash, &zns->netlink_cmd);
 		close(zns->netlink_cmd.sock);
 		zns->netlink_cmd.sock = -1;
 	}
 
 	if (zns->netlink_dplane_in.sock >= 0) {
+		hash_release(nlsock_hash, &zns->netlink_dplane_in);
 		close(zns->netlink_dplane_in.sock);
 		zns->netlink_dplane_in.sock = -1;
 	}
@@ -1678,9 +1727,12 @@ void kernel_terminate(struct zebra_ns *zns, bool complete)
 	 */
 	if (complete) {
 		if (zns->netlink_dplane_out.sock >= 0) {
+			hash_release(nlsock_hash, &zns->netlink_dplane_out);
 			close(zns->netlink_dplane_out.sock);
 			zns->netlink_dplane_out.sock = -1;
 		}
+
+		hash_free(nlsock_hash);
 	}
 }
 #endif /* HAVE_NETLINK */
diff --git a/zebra/kernel_netlink.h b/zebra/kernel_netlink.h
index cf8b8c785e..ae88f3372b 100644
--- a/zebra/kernel_netlink.h
+++ b/zebra/kernel_netlink.h
@@ -146,6 +146,7 @@ extern int netlink_config_write_helper(struct vty *vty);
 extern void netlink_set_batch_buffer_size(uint32_t size, uint32_t threshold,
 					  bool set);
 
+extern struct nlsock *kernel_netlink_nlsock_lookup(int sock);
 #endif /* HAVE_NETLINK */
 
 #ifdef __cplusplus
diff --git a/zebra/rt_netlink.c b/zebra/rt_netlink.c
index 9ead1a768e..c6423dce99 100644
--- a/zebra/rt_netlink.c
+++ b/zebra/rt_netlink.c
@@ -1898,6 +1898,7 @@ ssize_t netlink_route_multipath_msg_encode(int cmd,
 	union g_addr src;
 	const struct prefix *p, *src_p;
 	uint32_t table_id;
+	struct nlsock *nl;
 
 	struct {
 		struct nlmsghdr n;
@@ -1911,6 +1912,8 @@ ssize_t netlink_route_multipath_msg_encode(int cmd,
 	if (datalen < sizeof(*req))
 		return 0;
 
+	nl = kernel_netlink_nlsock_lookup(dplane_ctx_get_ns_sock(ctx));
+
 	memset(req, 0, sizeof(*req));
 
 	bytelen = (p->family == AF_INET ? 4 : 16);
@@ -1924,7 +1927,7 @@ ssize_t netlink_route_multipath_msg_encode(int cmd,
 
 	req->n.nlmsg_type = cmd;
 
-	req->n.nlmsg_pid = dplane_ctx_get_ns(ctx)->nls.snl.nl_pid;
+	req->n.nlmsg_pid = nl->snl.nl_pid;
 
 	req->r.rtm_family = p->family;
 	req->r.rtm_dst_len = p->prefixlen;
@@ -2360,6 +2363,8 @@ ssize_t netlink_nexthop_msg_encode(uint16_t cmd,
 	int type = dplane_ctx_get_nhe_type(ctx);
 	struct rtattr *nest;
 	uint16_t encap;
+	struct nlsock *nl =
+		kernel_netlink_nlsock_lookup(dplane_ctx_get_ns_sock(ctx));
 
 	if (!id) {
 		flog_err(
@@ -2402,7 +2407,7 @@ ssize_t netlink_nexthop_msg_encode(uint16_t cmd,
 		req->n.nlmsg_flags |= NLM_F_REPLACE;
 
 	req->n.nlmsg_type = cmd;
-	req->n.nlmsg_pid = dplane_ctx_get_ns(ctx)->nls.snl.nl_pid;
+	req->n.nlmsg_pid = nl->snl.nl_pid;
 
 	req->nhm.nh_family = AF_UNSPEC;
 	/* TODO: Scope? */
@@ -4283,6 +4288,8 @@ ssize_t netlink_mpls_multipath_msg_encode(int cmd, struct zebra_dplane_ctx *ctx,
 	const char *routedesc;
 	int route_type;
 	struct prefix p = {0};
+	struct nlsock *nl =
+		kernel_netlink_nlsock_lookup(dplane_ctx_get_ns_sock(ctx));
 
 	struct {
 		struct nlmsghdr n;
@@ -4325,7 +4332,7 @@ ssize_t netlink_mpls_multipath_msg_encode(int cmd, struct zebra_dplane_ctx *ctx,
 	req->n.nlmsg_len = NLMSG_LENGTH(sizeof(struct rtmsg));
 	req->n.nlmsg_flags = NLM_F_CREATE | NLM_F_REQUEST;
 	req->n.nlmsg_type = cmd;
-	req->n.nlmsg_pid = dplane_ctx_get_ns(ctx)->nls.snl.nl_pid;
+	req->n.nlmsg_pid = nl->snl.nl_pid;
 
 	req->r.rtm_family = AF_MPLS;
 	req->r.rtm_table = RT_TABLE_MAIN;
diff --git a/zebra/zebra_dplane.c b/zebra/zebra_dplane.c
index 656ebcf3b7..05297e143b 100644
--- a/zebra/zebra_dplane.c
+++ b/zebra/zebra_dplane.c
@@ -1465,6 +1465,13 @@ const struct zebra_dplane_info *dplane_ctx_get_ns(
 	return &(ctx->zd_ns_info);
 }
 
+int dplane_ctx_get_ns_sock(const struct zebra_dplane_ctx *ctx)
+{
+	DPLANE_CTX_VALID(ctx);
+
+	return ctx->zd_ns_info.sock;
+}
+
 /* Accessors for nexthop information */
 uint32_t dplane_ctx_get_nhe_id(const struct zebra_dplane_ctx *ctx)
 {
@@ -4844,7 +4851,7 @@ static void dplane_info_from_zns(struct zebra_dplane_info *ns_info,
 
 #if defined(HAVE_NETLINK)
 	ns_info->is_cmd = true;
-	ns_info->nls = zns->netlink_dplane_out;
+	ns_info->sock = zns->netlink_dplane_out.sock;
 #endif /* NETLINK */
 }
 
@@ -4861,7 +4868,7 @@ static int dplane_incoming_read(struct thread *event)
 
 	/* Re-start read task */
 	thread_add_read(zdplane_info.dg_master, dplane_incoming_read, zi,
-			zi->info.nls.sock, &zi->t_read);
+			zi->info.sock, &zi->t_read);
 
 	return 0;
 }
@@ -4906,13 +4913,13 @@ void zebra_dplane_ns_enable(struct zebra_ns *zns, bool enabled)
 		/* Make sure we're up-to-date with the zns object */
 #if defined(HAVE_NETLINK)
 		zi->info.is_cmd = false;
-		zi->info.nls = zns->netlink_dplane_in;
+		zi->info.sock = zns->netlink_dplane_in.sock;
 
 		/* Start read task for the dplane pthread. */
 		if (zdplane_info.dg_master)
 			thread_add_read(zdplane_info.dg_master,
-					dplane_incoming_read, zi,
-					zi->info.nls.sock, &zi->t_read);
+					dplane_incoming_read, zi, zi->info.sock,
+					&zi->t_read);
 #endif
 	} else if (zi) {
 		if (IS_ZEBRA_DEBUG_DPLANE)
@@ -5935,7 +5942,7 @@ void zebra_dplane_start(void)
 	frr_each (zns_info_list, &zdplane_info.dg_zns_list, zi) {
 #if defined(HAVE_NETLINK)
 		thread_add_read(zdplane_info.dg_master, dplane_incoming_read,
-				zi, zi->info.nls.sock, &zi->t_read);
+				zi, zi->info.sock, &zi->t_read);
 #endif
 	}
 
diff --git a/zebra/zebra_dplane.h b/zebra/zebra_dplane.h
index 69ea9c7fd9..a7a5f99e45 100644
--- a/zebra/zebra_dplane.h
+++ b/zebra/zebra_dplane.h
@@ -41,7 +41,7 @@ struct zebra_dplane_info {
 	ns_id_t ns_id;
 
 #if defined(HAVE_NETLINK)
-	struct nlsock nls;
+	int sock;
 	int seq;
 	bool is_cmd;
 #endif
@@ -57,10 +57,10 @@ zebra_dplane_info_from_zns(struct zebra_dplane_info *zns_info,
 #if defined(HAVE_NETLINK)
 	zns_info->is_cmd = is_cmd;
 	if (is_cmd) {
-		zns_info->nls = zns->netlink_cmd;
+		zns_info->sock = zns->netlink_cmd.sock;
 		zns_info->seq = zns->netlink_cmd.seq;
 	} else {
-		zns_info->nls = zns->netlink;
+		zns_info->sock = zns->netlink.sock;
 		zns_info->seq = zns->netlink.seq;
 	}
 #endif /* NETLINK */
@@ -564,9 +564,10 @@ dplane_ctx_gre_get_mtu(const struct zebra_dplane_ctx *ctx);
 const struct zebra_l2info_gre *
 dplane_ctx_gre_get_info(const struct zebra_dplane_ctx *ctx);
 
-/* Namespace info - esp. for netlink communication */
+/* Namespace fd info - esp. for netlink communication */
 const struct zebra_dplane_info *dplane_ctx_get_ns(
 	const struct zebra_dplane_ctx *ctx);
+int dplane_ctx_get_ns_sock(const struct zebra_dplane_ctx *ctx);
 
 /* Indicates zebra shutdown/exit is in progress. Some operations may be
  * simplified or skipped during shutdown processing.