[ovs-dev] [PATCH v2] netdev-linux: Avoid RTM_GETQDISC bug workaround on new-enough kernels.

[Ben Pfaff]

Otherwise we can't detect classless qdiscs.

This has no useful effect for the currently supported qdiscs, which all
have classes, but it makes it possible to add support for new classless
qdiscs.

This suddenly makes netdev-linux complain about qdiscs it doesn't know
about (e.g. pfifo_fast), which isn't too useful, so this commit also
demotes that INFO message to DBG level.

Reported-by: Jonathan Vestin <jonavest at kau.se>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
v1->v2: Do some more testing and figure out what's going on for sfq
  and pfifo_fast, then update treatment of tcm_parent and the "no reply"
  kernel case.

 AUTHORS            |    1 +
 lib/netdev-linux.c |   51 ++++++++++++++++++++++++++++++++++++++++-----------
 2 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/AUTHORS b/AUTHORS
index fe79acd..3b32ac6 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -267,6 +267,7 @@ Joan Cirer              joan at ev0.net
 John Darrington         john at darrington.wattle.id.au
 John Galgay             john at galgay.net
 John Hurley             john.hurley at netronome.com
+Jonathan Vestin         jonavest at kau.se
 K 華                    k940545 at hotmail.com
 Kevin Mancuso           kevin.mancuso at rackspace.com
 Kiran Shanbhog          kiran at vmware.com
diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 662ccc9..8c896d6 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -36,6 +36,7 @@
 #include <sys/types.h>
 #include <sys/ioctl.h>
 #include <sys/socket.h>
+#include <sys/utsname.h>
 #include <netpacket/packet.h>
 #include <net/if.h>
 #include <net/if_arp.h>
@@ -4390,6 +4391,31 @@ tc_del_qdisc(struct netdev *netdev_)
     return error;
 }
 
+static bool
+getqdisc_is_safe(void)
+{
+    static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER;
+    static bool safe = false;
+
+    if (ovsthread_once_start(&once)) {
+        struct utsname utsname;
+        int major, minor;
+
+        if (uname(&utsname) == -1) {
+            VLOG_WARN("uname failed (%s)", ovs_strerror(errno));
+        } else if (!ovs_scan(utsname.release, "%d.%d", &major, &minor)) {
+            VLOG_WARN("uname reported bad OS release (%s)", utsname.release);
+        } else if (major < 2 || (major == 2 && minor < 35)) {
+            VLOG_INFO("disabling unsafe RTM_GETQDISC in Linux kernel %s",
+                      utsname.release);
+        } else {
+            safe = true;
+        }
+        ovsthread_once_done(&once);
+    }
+    return safe;
+}
+
 /* If 'netdev''s qdisc type and parameters are not yet known, queries the
  * kernel to determine what they are.  Returns 0 if successful, otherwise a
  * positive errno value. */
@@ -4419,18 +4445,21 @@ tc_query_qdisc(const struct netdev *netdev_)
      * create will have a class with handle 1:0.  The built-in qdiscs only have
      * a class with handle 0:0.
      *
-     * We could check for Linux 2.6.35+ and use a more straightforward method
-     * there. */
+     * On Linux 2.6.35+ we use the straightforward method because it allows us
+     * to handle non-builtin qdiscs without handle 1:0 (e.g. codel).  However,
+     * in such a case we get no response at all from the kernel (!) if a
+     * builtin qdisc is in use (which is later caught by "!error &&
+     * !qdisc->size"). */
     tcmsg = tc_make_request(netdev_, RTM_GETQDISC, NLM_F_ECHO, &request);
     if (!tcmsg) {
         return ENODEV;
     }
-    tcmsg->tcm_handle = tc_make_handle(1, 0);
-    tcmsg->tcm_parent = 0;
+    tcmsg->tcm_handle = tc_make_handle(getqdisc_is_safe() ? 0 : 1, 0);
+    tcmsg->tcm_parent = getqdisc_is_safe() ? TC_H_ROOT : 0;
 
     /* Figure out what tc class to instantiate. */
     error = tc_transact(&request, &qdisc);
-    if (!error) {
+    if (!error && qdisc->size) {
         const char *kind;
 
         error = tc_parse_qdisc(qdisc, &kind, NULL);
@@ -4440,15 +4469,15 @@ tc_query_qdisc(const struct netdev *netdev_)
             ops = tc_lookup_linux_name(kind);
             if (!ops) {
                 static struct vlog_rate_limit rl2 = VLOG_RATE_LIMIT_INIT(1, 1);
-                VLOG_INFO_RL(&rl2, "unknown qdisc \"%s\"", kind);
+                VLOG_DBG_RL(&rl2, "unknown qdisc \"%s\"", kind);
 
                 ops = &tc_ops_other;
             }
         }
-    } else if (error == ENOENT) {
-        /* Either it's a built-in qdisc, or it's a qdisc set up by some
-         * other entity that doesn't have a handle 1:0.  We will assume
-         * that it's the system default qdisc. */
+    } else if ((!error && !qdisc->size) || error == ENOENT) {
+        /* Either it's a built-in qdisc, or (on Linux pre-2.6.35) it's a qdisc
+         * set up by some other entity that doesn't have a handle 1:0.  We will
+         * assume that it's the system default qdisc. */
         ops = &tc_ops_default;
         error = 0;
     } else {
-- 
1.7.10.4