Attachment 1757792 Details for Bug 1927973 – Main patch reducing netlink congestion

[patch] Main patch reducing netlink congestion

0001-Request-only-one-re-read-of-addresses-and-or-routes.patch (text/plain), 3.95 KB, created by Petr Menšík on 2021-02-18 13:27:09 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Petr Menšík

Created: 2021-02-18 13:27:09 UTC

Size: 3.95 KB

patch

obsolete

>From aada9425961b25e6c0b98c568b8a9ac91084e819 Mon Sep 17 00:00:00 2001
>From: Petr Mensik <pemensik@redhat.com>
>Date: Wed, 17 Feb 2021 11:47:28 +0100
>Subject: [PATCH 1/4] Request only one re-read of addresses and/or routes
>
>Previous implementation re-reads systemd addresses exactly the same
>number of time equal number of notifications received.
>This is not necessary, we need just notification of change, then re-read
>the current state and adapt listeners. Repeated re-reading slows netlink
>processing and highers CPU usage on mass interface changes.
>
>Continue reading multicast events from netlink, even when ENOBUFS
>arrive. Broadcasts are not trusted anyway and refresh would be done in
>iface_enumerate. Save queued events sent again.
>---
> src/netlink.c | 38 ++++++++++++++++++++++++++++----------
> 1 file changed, 28 insertions(+), 10 deletions(-)
>
>diff --git a/src/netlink.c b/src/netlink.c
>index 0494070..7ff829e 100644
>--- a/src/netlink.c
>+++ b/src/netlink.c
>@@ -41,13 +41,20 @@
> 
> #ifndef NDA_RTA
> #  define NDA_RTA(r) ((struct rtattr*)(((char*)(r)) + NLMSG_ALIGN(sizeof(struct ndmsg)))) 
>-#endif 
>+#endif
>+
>+/* Used to request refresh of addresses or routes just once,
>+ * when multiple changes might be announced. */
>+enum async_states {
>+  STATE_NEWADDR = (1 << 0),
>+  STATE_NEWROUTE = (1 << 1),
>+};
> 
> 
> static struct iovec iov;
> static u32 netlink_pid;
> 
>-static void nl_async(struct nlmsghdr *h);
>+static unsigned nl_async(struct nlmsghdr *h, unsigned state);
> 
> char *netlink_init(void)
> {
>@@ -159,6 +166,7 @@ int iface_enumerate(int family, void *parm, int (*callback)())
>   ssize_t len;
>   static unsigned int seq = 0;
>   int callback_ok = 1;
>+  unsigned state = 0;
> 
>   struct {
>     struct nlmsghdr nlh;
>@@ -207,7 +215,7 @@ int iface_enumerate(int family, void *parm, int (*callback)())
> 	if (h->nlmsg_pid != netlink_pid || h->nlmsg_type == NLMSG_ERROR)
> 	  {
> 	    /* May be multicast arriving async */
>-	    nl_async(h);
>+	    state = nl_async(h, state);
> 	  }
> 	else if (h->nlmsg_seq != seq)
> 	  {
>@@ -346,21 +354,22 @@ void netlink_multicast(void)
>   ssize_t len;
>   struct nlmsghdr *h;
>   int flags;
>+  unsigned state = 0;
>   
>   /* don't risk blocking reading netlink messages here. */
>   if ((flags = fcntl(daemon->netlinkfd, F_GETFL)) == -1 ||
>       fcntl(daemon->netlinkfd, F_SETFL, flags | O_NONBLOCK) == -1) 
>     return;
>   
>-  if ((len = netlink_recv()) != -1)
>+  while ((len = netlink_recv()) != -1 || errno == ENOBUFS)
>     for (h = (struct nlmsghdr *)iov.iov_base; NLMSG_OK(h, (size_t)len); h = NLMSG_NEXT(h, len))
>-      nl_async(h);
>+      state = nl_async(h, state);
>   
>   /* restore non-blocking status */
>   fcntl(daemon->netlinkfd, F_SETFL, flags);
> }
> 
>-static void nl_async(struct nlmsghdr *h)
>+static unsigned nl_async(struct nlmsghdr *h, unsigned state)
> {
>   if (h->nlmsg_type == NLMSG_ERROR)
>     {
>@@ -368,7 +377,8 @@ static void nl_async(struct nlmsghdr *h)
>       if (err->error != 0)
> 	my_syslog(LOG_ERR, _("netlink returns error: %s"), strerror(-(err->error)));
>     }
>-  else if (h->nlmsg_pid == 0 && h->nlmsg_type == RTM_NEWROUTE) 
>+  else if (h->nlmsg_pid == 0 && h->nlmsg_type == RTM_NEWROUTE &&
>+	   (state & STATE_NEWROUTE)==0)
>     {
>       /* We arrange to receive netlink multicast messages whenever the network route is added.
> 	 If this happens and we still have a DNS packet in the buffer, we re-send it.
>@@ -380,10 +390,18 @@ static void nl_async(struct nlmsghdr *h)
>       if (rtm->rtm_type == RTN_UNICAST && rtm->rtm_scope == RT_SCOPE_LINK &&
> 	  (rtm->rtm_table == RT_TABLE_MAIN ||
> 	   rtm->rtm_table == RT_TABLE_LOCAL))
>-	queue_event(EVENT_NEWROUTE);
>+	{
>+	  queue_event(EVENT_NEWROUTE);
>+	  state |= STATE_NEWROUTE;
>+	}
>+    }
>+  else if ((h->nlmsg_type == RTM_NEWADDR || h->nlmsg_type == RTM_DELADDR) &&
>+	   (state & STATE_NEWADDR)==0)
>+    {
>+      queue_event(EVENT_NEWADDR);
>+      state |= STATE_NEWADDR;
>     }
>-  else if (h->nlmsg_type == RTM_NEWADDR || h->nlmsg_type == RTM_DELADDR) 
>-    queue_event(EVENT_NEWADDR);
>+  return state;
> }
> #endif
> 
>-- 
>2.26.2
>

Actions: View | Diff

Attachments on bug 1927973: 1757792 | 1757794