[ovs-dev] [PATCH] netdev-linux: Zero feature flags on error.

[Ben Pfaff]

Jesse Gross <jesse at nicira.com> writes:

> We claimed that if we got an error when retrieving the features
> for an interface we would zero out all the flags.  We weren't
> doing this, which lead to random data for pseudo-devices.

I think it would be better to do this in netdev.c, like so:

>From c4dd2c913718913792223d3020442cd19de16ba6 Mon Sep 17 00:00:00 2001
From: Ben Pfaff <blp at nicira.com>
Date: Thu, 19 Nov 2009 11:06:14 -0800
Subject: [PATCH] netdev: Really set output values to 0 on failure in netdev_get_features().

The comment on netdev_get_features() claimed that all of the passed-in
values were set to 0 on failure, but the implementation didn't live up
to the promise.

CC: Paul Ingram <paul at nicira.com>
---
 lib/netdev-linux.c |    3 +--
 lib/netdev.c       |   26 +++++++++++++++++++++-----
 2 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 7324703..3ccd6be 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -720,8 +720,7 @@ netdev_linux_get_stats(const struct netdev *netdev_, struct netdev_stats *stats)
 /* Stores the features supported by 'netdev' into each of '*current',
  * '*advertised', '*supported', and '*peer' that are non-null.  Each value is a
  * bitmap of "enum ofp_port_features" bits, in host byte order.  Returns 0 if
- * successful, otherwise a positive errno value.  On failure, all of the
- * passed-in values are set to 0. */
+ * successful, otherwise a positive errno value. */
 static int
 netdev_linux_get_features(struct netdev *netdev,
                           uint32_t *current, uint32_t *advertised,
diff --git a/lib/netdev.c b/lib/netdev.c
index 481671f..965bdbf 100644
--- a/lib/netdev.c
+++ b/lib/netdev.c
@@ -377,11 +377,27 @@ netdev_get_features(struct netdev *netdev,
                     uint32_t *supported, uint32_t *peer)
 {
     uint32_t dummy[4];
-    return netdev->class->get_features(netdev,
-                                       current ? current : &dummy[0],
-                                       advertised ? advertised : &dummy[1],
-                                       supported ? supported : &dummy[2],
-                                       peer ? peer : &dummy[3]);
+    int error;
+
+    if (!current) {
+        current = &dummy[0];
+    }
+    if (!advertised) {
+        advertised = &dummy[1];
+    }
+    if (!supported) {
+        supported = &dummy[2];
+    }
+    if (!peer) {
+        peer = &dummy[3];
+    }
+
+    error = netdev->class->get_features(netdev, current, advertised, supported,
+                                        peer);
+    if (error) {
+        *current = *advertised = *supported = *peer = 0;
+    }
+    return error;
 }
 
 /* Set the features advertised by 'netdev' to 'advertise'.  Returns 0 if
-- 
1.6.3.3