238 lines
7.3 KiB
Diff
238 lines
7.3 KiB
Diff
From 5010c42c47b7b5a3d68d83369d6c17ed0bc11cff Mon Sep 17 00:00:00 2001
|
|
From: Petr Mensik <pemensik@redhat.com>
|
|
Date: Wed, 17 Feb 2021 11:47:28 +0100
|
|
Subject: [PATCH] Correct occasional --bind-dynamic synchronization break
|
|
|
|
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.
|
|
|
|
Remove sleeping on netlink ENOBUFS
|
|
|
|
With reduced number of written events netlink should receive ENOBUFS
|
|
rarely. It does not make sense to wait if it is received. It is just a
|
|
signal some packets got missing. Fast reading all pending packets is required,
|
|
seq checking ensures it already. Finishes changes by
|
|
commit 1d07667ac77c55b9de56b1b2c385167e0e0ec27a.
|
|
|
|
Move restart from iface_enumerate to enumerate_interfaces
|
|
|
|
When ENOBUFS is received, restart of reading addresses is done. But
|
|
previously found addresses might not have been found this time. In order
|
|
to catch this, restart both IPv4 and IPv6 enumeration with clearing
|
|
found interfaces first. It should deliver up-to-date state also after
|
|
ENOBUFS.
|
|
|
|
Read all netlink messages before netlink restart
|
|
|
|
Before writing again into netlink socket, try fetching all pending
|
|
messages. They would be ignored, only might trigger new address
|
|
synchronization. Should ensure new try has better chance to succeed.
|
|
|
|
Request sending ENOBUFS again
|
|
|
|
ENOBUFS error handling was improved. Netlink is correctly drained before
|
|
sending a new request again. It seems ENOBUFS supression is no longer
|
|
necessary or wanted. Let kernel tell us when it failed and handle it a
|
|
good way.
|
|
---
|
|
src/netlink.c | 67 ++++++++++++++++++++++++++++++++++++---------------
|
|
src/network.c | 11 +++++++--
|
|
2 files changed, 57 insertions(+), 21 deletions(-)
|
|
|
|
diff --git a/src/netlink.c b/src/netlink.c
|
|
index ac1a1c5..f95f3e8 100644
|
|
--- a/src/netlink.c
|
|
+++ b/src/netlink.c
|
|
@@ -32,13 +32,21 @@
|
|
|
|
#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);
|
|
+static void nl_multicast_state(unsigned state);
|
|
|
|
void netlink_init(void)
|
|
{
|
|
@@ -135,7 +143,9 @@ static ssize_t netlink_recv(void)
|
|
|
|
|
|
/* family = AF_UNSPEC finds ARP table entries.
|
|
- family = AF_LOCAL finds MAC addresses. */
|
|
+ family = AF_LOCAL finds MAC addresses.
|
|
+ returns 0 on failure, 1 on success, -1 when restart is required
|
|
+*/
|
|
int iface_enumerate(int family, void *parm, int (*callback)())
|
|
{
|
|
struct sockaddr_nl addr;
|
|
@@ -143,6 +153,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;
|
|
@@ -154,7 +165,6 @@ int iface_enumerate(int family, void *parm, int (*callback)())
|
|
addr.nl_groups = 0;
|
|
addr.nl_pid = 0; /* address to kernel */
|
|
|
|
- again:
|
|
if (family == AF_UNSPEC)
|
|
req.nlh.nlmsg_type = RTM_GETNEIGH;
|
|
else if (family == AF_LOCAL)
|
|
@@ -181,8 +191,8 @@ int iface_enumerate(int family, void *parm, int (*callback)())
|
|
{
|
|
if (errno == ENOBUFS)
|
|
{
|
|
- sleep(1);
|
|
- goto again;
|
|
+ nl_multicast_state(state);
|
|
+ return -1;
|
|
}
|
|
return 0;
|
|
}
|
|
@@ -191,7 +201,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)
|
|
{
|
|
@@ -327,26 +337,36 @@ int iface_enumerate(int family, void *parm, int (*callback)())
|
|
}
|
|
}
|
|
|
|
-void netlink_multicast(void)
|
|
+static void nl_multicast_state(unsigned state)
|
|
{
|
|
ssize_t len;
|
|
struct nlmsghdr *h;
|
|
int flags;
|
|
-
|
|
- /* 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;
|
|
+
|
|
+ do {
|
|
+ /* don't risk blocking reading netlink messages here. */
|
|
+ while ((len = netlink_recv()) != -1)
|
|
|
|
- if ((len = netlink_recv()) != -1)
|
|
- for (h = (struct nlmsghdr *)iov.iov_base; NLMSG_OK(h, (size_t)len); h = NLMSG_NEXT(h, len))
|
|
- nl_async(h);
|
|
-
|
|
+ for (h = (struct nlmsghdr *)iov.iov_base; NLMSG_OK(h, (size_t)len); h = NLMSG_NEXT(h, len))
|
|
+ state = nl_async(h, state);
|
|
+ } while (errno == ENOBUFS);
|
|
+
|
|
/* restore non-blocking status */
|
|
fcntl(daemon->netlinkfd, F_SETFL, flags);
|
|
}
|
|
|
|
-static void nl_async(struct nlmsghdr *h)
|
|
+void netlink_multicast(void)
|
|
+{
|
|
+ unsigned state = 0;
|
|
+ nl_multicast_state(state);
|
|
+}
|
|
+
|
|
+
|
|
+static unsigned nl_async(struct nlmsghdr *h, unsigned state)
|
|
{
|
|
if (h->nlmsg_type == NLMSG_ERROR)
|
|
{
|
|
@@ -354,7 +374,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.
|
|
@@ -366,10 +387,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
|
|
|
|
diff --git a/src/network.c b/src/network.c
|
|
index c6e7d89..47caf38 100644
|
|
--- a/src/network.c
|
|
+++ b/src/network.c
|
|
@@ -656,7 +656,8 @@ int enumerate_interfaces(int reset)
|
|
|
|
if ((param.fd = socket(PF_INET, SOCK_DGRAM, 0)) == -1)
|
|
return 0;
|
|
-
|
|
+
|
|
+again:
|
|
/* Mark interfaces for garbage collection */
|
|
for (iface = daemon->interfaces; iface; iface = iface->next)
|
|
iface->found = 0;
|
|
@@ -709,10 +710,16 @@ int enumerate_interfaces(int reset)
|
|
|
|
#ifdef HAVE_IPV6
|
|
ret = iface_enumerate(AF_INET6, ¶m, iface_allowed_v6);
|
|
+ if (ret < 0)
|
|
+ goto again;
|
|
#endif
|
|
|
|
if (ret)
|
|
- ret = iface_enumerate(AF_INET, ¶m, iface_allowed_v4);
|
|
+ {
|
|
+ ret = iface_enumerate(AF_INET, ¶m, iface_allowed_v4);
|
|
+ if (ret < 0)
|
|
+ goto again;
|
|
+ }
|
|
|
|
errsave = errno;
|
|
close(param.fd);
|
|
--
|
|
2.26.2
|
|
|