From: Michal Kazior <[email protected]>

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.

This change requires a slight tweak to the netlink
abstraction code so that the events can be
discarded as soon as possible before getting a
chance to cause the churn.

Signed-off-by: Michal Kazior <[email protected]>
---

Notes:
    v2:
     - fix bracing style [0day robot / checkpatch]

 lib/netdev-linux.c     |  4 ++--
 lib/netlink-notifier.c |  4 +++-
 lib/rtnetlink.c        | 24 +++++++++++++++++++++---
 lib/rtnetlink.h        |  2 +-
 4 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 6be23dbee..301dd9060 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -742,7 +742,7 @@ netdev_linux_run(const struct netdev_class *netdev_class 
OVS_UNUSED)
         if (!error) {
             struct rtnetlink_change change;
 
-            if (rtnetlink_parse(&buf, &change)) {
+            if (rtnetlink_parse(&buf, &change) > 0) {
                 struct netdev *netdev_ = NULL;
                 char dev_name[IFNAMSIZ];
 
@@ -6343,7 +6343,7 @@ netdev_linux_update_via_netlink(struct netdev_linux 
*netdev)
         return error;
     }
 
-    if (rtnetlink_parse(reply, change)
+    if (rtnetlink_parse(reply, change) > 0
         && change->nlmsg_type == RTM_NEWLINK) {
         bool changed = false;
         error = 0;
diff --git a/lib/netlink-notifier.c b/lib/netlink-notifier.c
index dfecb9778..ab5a84abb 100644
--- a/lib/netlink-notifier.c
+++ b/lib/netlink-notifier.c
@@ -189,8 +189,10 @@ nln_run(struct nln *nln)
         if (!error) {
             int group = nln->parse(&buf, nln->change);
 
-            if (group != 0) {
+            if (group > 0) {
                 nln_report(nln, nln->change, group);
+            } else if (group == 0) {
+                /* ignore some events */
             } else {
                 VLOG_WARN_RL(&rl, "unexpected netlink message contents");
                 nln_report(nln, NULL, 0);
diff --git a/lib/rtnetlink.c b/lib/rtnetlink.c
index 125802925..316524c0f 100644
--- a/lib/rtnetlink.c
+++ b/lib/rtnetlink.c
@@ -82,7 +82,7 @@ rtnetlink_parse_link_info(const struct nlattr *nla,
 /* Parses a rtnetlink message 'buf' into 'change'.  If 'buf' is unparseable,
  * leaves 'change' untouched and returns false.  Otherwise, populates 'change'
  * and returns true. */
-bool
+int
 rtnetlink_parse(struct ofpbuf *buf, struct rtnetlink_change *change)
 {
     const struct nlmsghdr *nlmsg = buf->data;
@@ -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) {
+                return RTNLGRP_NONE;
+            }
+
             change->nlmsg_type     = nlmsg->nlmsg_type;
             change->if_index       = ifinfo->ifi_index;
             change->ifname         = nl_attr_get_string(attrs[IFLA_IFNAME]);
@@ -165,14 +183,14 @@ rtnetlink_parse(struct ofpbuf *buf, struct 
rtnetlink_change *change)
         }
     }
 
-    return parsed;
+    return parsed ? RTNLGRP_LINK : -1;
 }
 
 /* Return RTNLGRP_LINK on success, 0 on parse error. */
 static int
 rtnetlink_parse_cb(struct ofpbuf *buf, void *change)
 {
-    return rtnetlink_parse(buf, change) ? RTNLGRP_LINK : 0;
+    return rtnetlink_parse(buf, change);
 }
 
 /* Registers 'cb' to be called with auxiliary data 'aux' with network device
diff --git a/lib/rtnetlink.h b/lib/rtnetlink.h
index b6ddb4bd1..23921a63b 100644
--- a/lib/rtnetlink.h
+++ b/lib/rtnetlink.h
@@ -65,7 +65,7 @@ void rtnetlink_notify_func(const struct rtnetlink_change 
*change,
 
 bool rtnetlink_type_is_rtnlgrp_link(uint16_t type);
 bool rtnetlink_type_is_rtnlgrp_addr(uint16_t type);
-bool rtnetlink_parse(struct ofpbuf *buf, struct rtnetlink_change *change);
+int rtnetlink_parse(struct ofpbuf *buf, struct rtnetlink_change *change);
 struct nln_notifier *
 rtnetlink_notifier_create(rtnetlink_notify_func *, void *aux);
 void rtnetlink_notifier_destroy(struct nln_notifier *);
-- 
2.27.0

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to