253 lines
7.8 KiB
Diff
253 lines
7.8 KiB
Diff
From 4c0aecc68524c5fd74053244a605e905dc644228 Mon Sep 17 00:00:00 2001
|
|
From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= <pemensik@redhat.com>
|
|
Date: Tue, 2 Mar 2021 18:21:32 +0000
|
|
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.
|
|
|
|
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 | 72 +++++++++++++++++++++++++++++++++++++++--------------------
|
|
src/network.c | 14 ++++++++----
|
|
2 files changed, 58 insertions(+), 28 deletions(-)
|
|
|
|
diff --git a/src/netlink.c b/src/netlink.c
|
|
index 0494070..3ad18a6 100644
|
|
--- a/src/netlink.c
|
|
+++ b/src/netlink.c
|
|
@@ -41,19 +41,26 @@
|
|
|
|
#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);
|
|
|
|
char *netlink_init(void)
|
|
{
|
|
struct sockaddr_nl addr;
|
|
socklen_t slen = sizeof(addr);
|
|
- int opt = 1;
|
|
|
|
addr.nl_family = AF_NETLINK;
|
|
addr.nl_pad = 0;
|
|
@@ -92,10 +99,6 @@ char *netlink_init(void)
|
|
iov.iov_len = 100;
|
|
iov.iov_base = safe_malloc(iov.iov_len);
|
|
|
|
- if (daemon->kernel_version >= KERNEL_VERSION(2,6,30) &&
|
|
- setsockopt(daemon->netlinkfd, SOL_NETLINK, NETLINK_NO_ENOBUFS, &opt, sizeof(opt)) == -1)
|
|
- return _("warning: failed to set NETLINK_NO_ENOBUFS on netlink socket");
|
|
-
|
|
return NULL;
|
|
}
|
|
|
|
@@ -151,7 +154,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;
|
|
@@ -159,6 +164,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;
|
|
@@ -170,7 +176,6 @@ int iface_enumerate(int family, void *parm, int (*callback)())
|
|
|
|
addr.nl_family = AF_NETLINK;
|
|
|
|
- again:
|
|
if (family == AF_UNSPEC)
|
|
req.nlh.nlmsg_type = RTM_GETNEIGH;
|
|
else if (family == AF_LOCAL)
|
|
@@ -197,8 +202,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;
|
|
}
|
|
@@ -207,7 +212,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)
|
|
{
|
|
@@ -341,26 +346,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)
|
|
{
|
|
@@ -368,7 +383,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 +396,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 6cf15a9..77eb6ad 100644
|
|
--- a/src/network.c
|
|
+++ b/src/network.c
|
|
@@ -638,7 +638,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;
|
|
@@ -690,9 +691,14 @@ int enumerate_interfaces(int reset)
|
|
param.spare = spare;
|
|
|
|
ret = iface_enumerate(AF_INET6, ¶m, iface_allowed_v6);
|
|
-
|
|
- if (ret)
|
|
- ret = iface_enumerate(AF_INET, ¶m, iface_allowed_v4);
|
|
+ if (ret < 0)
|
|
+ goto again;
|
|
+ else if (ret)
|
|
+ {
|
|
+ ret = iface_enumerate(AF_INET, ¶m, iface_allowed_v4);
|
|
+ if (ret < 0)
|
|
+ goto again;
|
|
+ }
|
|
|
|
errsave = errno;
|
|
close(param.fd);
|
|
--
|
|
1.8.3.1
|
|
|