[ovs-dev] [PATCH v3] netlink: ignore IFLA_WIRELESS events
Michal Kazior
kazikcz at gmail.com
Thu Mar 4 14:42:43 UTC 2021
From: Michal Kazior <michal at plume.com>
Some older wireless drivers - ones relying on the
old and long deprecated wireless extension ioctl
system - can generate quite a bit of IFLA_WIRELESS
events depending on their configuration and
runtime conditions. These are delivered as
RTNLGRP_LINK via RTM_NEWLINK messages.
These tend to be relatively easily identifiable
because they report the change mask being 0. This
isn't guaranteed but in practice it shouldn't be a
problem. None of the wireless events that I ever
observed actually carry any unique information
about netdev states that ovs-vswitchd is
interested in. Hence ignoring these shouldn't
cause any problems.
These events can be responsible for a significant
CPU churn as ovs-vswitchd attempts to do plenty of
work for each and every netlink message regardless
of what that message carries. On low-end devices
such as consumer-grade routers these can lead to a
lot of CPU cycles being wasted, adding up to heat
output and reducing performance.
It could be argued that wireless drivers in
question should be fixed, but that isn't exactly a
trivial thing to do. Patching ovs seems far more
viable while still making sense.
Signed-off-by: Michal Kazior <michal at plume.com>
---
Notes:
v3:
- dont change rtnetlink_parse() semantics, instead
extend rtnetlink_change struct and update its
consumers [Ilya]
- adjusted commit log to reflect different approach
[Ilya]
v2:
- fix bracing style [0day robot / checkpatch]
lib/if-notifier.c | 7 ++++++-
lib/netdev-linux.c | 9 +++++++++
lib/route-table.c | 4 ++++
lib/rtnetlink.c | 18 ++++++++++++++++++
lib/rtnetlink.h | 3 +++
5 files changed, 40 insertions(+), 1 deletion(-)
diff --git a/lib/if-notifier.c b/lib/if-notifier.c
index 9a64f9b15..7ba9cc316 100644
--- a/lib/if-notifier.c
+++ b/lib/if-notifier.c
@@ -26,9 +26,14 @@ struct if_notifier {
};
static void
-if_notifier_cb(const struct rtnetlink_change *change OVS_UNUSED, void *aux)
+if_notifier_cb(const struct rtnetlink_change *change, void *aux)
{
struct if_notifier *notifier;
+
+ if (change->irrelevant) {
+ return;
+ }
+
notifier = aux;
notifier->cb(notifier->aux);
}
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 6be23dbee..388288f71 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -663,6 +663,10 @@ netdev_linux_update_lag(struct rtnetlink_change *change)
{
struct linux_lag_member *lag;
+ if (change->irrelevant) {
+ return;
+ }
+
if (change->sub && netdev_linux_kind_is_lag(change->sub)) {
lag = shash_find_data(&lag_shash, change->ifname);
@@ -887,6 +891,10 @@ netdev_linux_update(struct netdev_linux *dev, int nsid,
const struct rtnetlink_change *change)
OVS_REQUIRES(dev->mutex)
{
+ if (change->irrelevant) {
+ return;
+ }
+
if (netdev_linux_netnsid_is_eq(dev, nsid)) {
netdev_linux_update__(dev, change);
}
@@ -6344,6 +6352,7 @@ netdev_linux_update_via_netlink(struct netdev_linux *netdev)
}
if (rtnetlink_parse(reply, change)
+ && !change->irrelevant
&& change->nlmsg_type == RTM_NEWLINK) {
bool changed = false;
error = 0;
diff --git a/lib/route-table.c b/lib/route-table.c
index 6c82cdfdd..a4531df1e 100644
--- a/lib/route-table.c
+++ b/lib/route-table.c
@@ -342,6 +342,10 @@ static void
name_table_change(const struct rtnetlink_change *change,
void *aux OVS_UNUSED)
{
+ if (change->irrelevant) {
+ return;
+ }
+
/* Changes to interface status can cause routing table changes that some
* versions of the linux kernel do not advertise for some reason. */
route_table_valid = false;
diff --git a/lib/rtnetlink.c b/lib/rtnetlink.c
index 125802925..ebb032aa7 100644
--- a/lib/rtnetlink.c
+++ b/lib/rtnetlink.c
@@ -99,6 +99,7 @@ rtnetlink_parse(struct ofpbuf *buf, struct rtnetlink_change *change)
[IFLA_MTU] = { .type = NL_A_U32, .optional = true },
[IFLA_ADDRESS] = { .type = NL_A_UNSPEC, .optional = true },
[IFLA_LINKINFO] = { .type = NL_A_NESTED, .optional = true },
+ [IFLA_WIRELESS] = { .type = NL_A_UNSPEC, .optional = true },
};
struct nlattr *attrs[ARRAY_SIZE(policy)];
@@ -111,6 +112,23 @@ rtnetlink_parse(struct ofpbuf *buf, struct rtnetlink_change *change)
ifinfo = ofpbuf_at(buf, NLMSG_HDRLEN, sizeof *ifinfo);
+ /* Wireless events can be spammy and cause a
+ * lot of unnecessary churn and CPU load in
+ * ovs-vswitchd. The best way to filter them out
+ * is to rely on the IFLA_WIRELESS and
+ * ifi_change. As per rtnetlink_ifinfo_prep() in
+ * the kernel, the ifi_change = 0. That combined
+ * with the fact wireless events never really
+ * change interface state (as far as core
+ * networking is concerned) they can be ignored
+ * by ovs-vswitchd. It doesn't understand
+ * wireless extensions anyway and has no way of
+ * presenting these bits into ovsdb.
+ */
+ if (attrs[IFLA_WIRELESS] && ifinfo->ifi_change == 0) {
+ change->irrelevant = true;
+ }
+
change->nlmsg_type = nlmsg->nlmsg_type;
change->if_index = ifinfo->ifi_index;
change->ifname = nl_attr_get_string(attrs[IFLA_IFNAME]);
diff --git a/lib/rtnetlink.h b/lib/rtnetlink.h
index b6ddb4bd1..f103316f1 100644
--- a/lib/rtnetlink.h
+++ b/lib/rtnetlink.h
@@ -45,6 +45,9 @@ struct rtnetlink_change {
int mtu; /* Current MTU. */
struct eth_addr mac;
unsigned int ifi_flags; /* Flags of network device. */
+ bool irrelevant; /* Some events, notably wireless extensions,
+ don't really indicate real netdev change that
+ ovs should care about */
/* Network device address status. */
/* xxx To be added when needed. */
--
2.27.0
More information about the dev
mailing list