[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