[ovs-dev] [PATCH] netdev-linux: Fix self-deadlocks in traffic control code.

[Ben Pfaff]

htb_parse_qdisc_details__(), which is called with the netdev mutex, called
netdev_get_mtu(), which tried to reacquire the mutex and thus deadlocked.
This commit fixes the problem and similar problems in
htb_parse_class_details__() and hfsc_parse_qdisc_details__().

Bug #19180.
Reported-by: Dhaval Badiani <dbadiani at vmware.com>
Signed-off-by: Ben Pfaff <blp at nicira.com>
---
 lib/netdev-linux.c |   41 ++++++++++++++++++++++++++++-------------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 9a80b67..c3684da 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1041,21 +1041,17 @@ netdev_linux_get_etheraddr(const struct netdev *netdev_,
     return error;
 }
 
-/* Returns the maximum size of transmitted (and received) packets on 'netdev',
- * in bytes, not including the hardware header; thus, this is typically 1500
- * bytes for Ethernet devices. */
 static int
-netdev_linux_get_mtu(const struct netdev *netdev_, int *mtup)
+netdev_linux_get_mtu__(struct netdev_linux *netdev, int *mtup)
+    OVS_REQUIRES(netdev->mutex)
 {
-    struct netdev_linux *netdev = netdev_linux_cast(netdev_);
     int error;
 
-    ovs_mutex_lock(&netdev->mutex);
     if (!(netdev->cache_valid & VALID_MTU)) {
         struct ifreq ifr;
 
         netdev->netdev_mtu_error = af_inet_ifreq_ioctl(
-            netdev_get_name(netdev_), &ifr, SIOCGIFMTU, "SIOCGIFMTU");
+            netdev_get_name(&netdev->up), &ifr, SIOCGIFMTU, "SIOCGIFMTU");
         netdev->mtu = ifr.ifr_mtu;
         netdev->cache_valid |= VALID_MTU;
     }
@@ -1064,6 +1060,21 @@ netdev_linux_get_mtu(const struct netdev *netdev_, int *mtup)
     if (!error) {
         *mtup = netdev->mtu;
     }
+
+    return error;
+}
+
+/* Returns the maximum size of transmitted (and received) packets on 'netdev',
+ * in bytes, not including the hardware header; thus, this is typically 1500
+ * bytes for Ethernet devices. */
+static int
+netdev_linux_get_mtu(const struct netdev *netdev_, int *mtup)
+{
+    struct netdev_linux *netdev = netdev_linux_cast(netdev_);
+    int error;
+
+    ovs_mutex_lock(&netdev->mutex);
+    error = netdev_linux_get_mtu__(netdev, mtup);
     ovs_mutex_unlock(&netdev->mutex);
 
     return error;
@@ -2707,7 +2718,7 @@ htb_setup_class__(struct netdev *netdev, unsigned int handle,
     int error;
     int mtu;
 
-    error = netdev_get_mtu(netdev, &mtu);
+    error = netdev_linux_get_mtu__(netdev_linux_cast(netdev), &mtu);
     if (error) {
         VLOG_WARN_RL(&rl, "cannot set up HTB on device %s that lacks MTU",
                      netdev_get_name(netdev));
@@ -2803,9 +2814,10 @@ htb_parse_tcmsg__(struct ofpbuf *tcmsg, unsigned int *queue_id,
 }
 
 static void
-htb_parse_qdisc_details__(struct netdev *netdev,
+htb_parse_qdisc_details__(struct netdev *netdev_,
                           const struct smap *details, struct htb_class *hc)
 {
+    struct netdev_linux *netdev = netdev_linux_cast(netdev_);
     const char *max_rate_s;
 
     max_rate_s = smap_get(details, "max-rate");
@@ -2813,7 +2825,8 @@ htb_parse_qdisc_details__(struct netdev *netdev,
     if (!hc->max_rate) {
         enum netdev_features current;
 
-        netdev_get_features(netdev, &current, NULL, NULL, NULL);
+        netdev_linux_read_features(netdev);
+        current = !netdev->get_features_error ? netdev->current : 0;
         hc->max_rate = netdev_features_to_bps(current, 100 * 1000 * 1000) / 8;
     }
     hc->min_rate = hc->max_rate;
@@ -2832,7 +2845,7 @@ htb_parse_class_details__(struct netdev *netdev,
     const char *priority_s = smap_get(details, "priority");
     int mtu, error;
 
-    error = netdev_get_mtu(netdev, &mtu);
+    error = netdev_linux_get_mtu__(netdev_linux_cast(netdev), &mtu);
     if (error) {
         VLOG_WARN_RL(&rl, "cannot parse HTB class on device %s that lacks MTU",
                      netdev_get_name(netdev));
@@ -3280,9 +3293,10 @@ hfsc_query_class__(const struct netdev *netdev, unsigned int handle,
 }
 
 static void
-hfsc_parse_qdisc_details__(struct netdev *netdev, const struct smap *details,
+hfsc_parse_qdisc_details__(struct netdev *netdev_, const struct smap *details,
                            struct hfsc_class *class)
 {
+    struct netdev_linux *netdev = netdev_linux_cast(netdev_);
     uint32_t max_rate;
     const char *max_rate_s;
 
@@ -3292,7 +3306,8 @@ hfsc_parse_qdisc_details__(struct netdev *netdev, const struct smap *details,
     if (!max_rate) {
         enum netdev_features current;
 
-        netdev_get_features(netdev, &current, NULL, NULL, NULL);
+        netdev_linux_read_features(netdev);
+        current = !netdev->get_features_error ? netdev->current : 0;
         max_rate = netdev_features_to_bps(current, 100 * 1000 * 1000) / 8;
     }
 
-- 
1.7.10.4