[ovs-dev] [PATCH v1 1/2] netdev-dpdk: Fix netdev_dpdk_get_features().

Ian Stokes ian.stokes at intel.com
Thu Oct 25 14:17:40 UTC 2018


This commit fixes netdev_dpdk_get_features() by initializing a bitmap
that represents current features to zero and accounting for non defined
link speed values in the OpenFlow spec.

The current approach for retrieving netdev dpdk features uses a
pointer allocated in the stack without being initialized. As such there
is no guarantee that the bitmap will be accurate. Fix this by declaring
and initializing local variable 'feature' to be used when building the
bitmap, with its value then assigned to the pointer. Also account for
link speeds not defined in the OpenFlow spec by defaulting to
NETDEV_F_OTHER for undefined link speeds.

Fixes: 8a9562d21a40 ("dpif-netdev: Add DPDK netdev.")
Signed-off-by: Ian Stokes <ian.stokes at intel.com>
---
Flavio, this patch is based on suggestions from you

https://mail.openvswitch.org/pipermail/ovs-dev/2018-September/351809.html

I'd like to add

Co-authored-by: Flavio Leitner <fbl at sysclose.org>
Signed-off-by: Flavio Leitner <fbl at sysclose.org>

if you agree to sign off on this.
---
 lib/netdev-dpdk.c | 65 ++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 40 insertions(+), 25 deletions(-)

diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index f91aa27cd..35eb30f8d 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2707,43 +2707,58 @@ netdev_dpdk_get_features(const struct netdev *netdev,
 {
     struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
     struct rte_eth_link link;
+    uint32_t feature = 0;
 
     ovs_mutex_lock(&dev->mutex);
     link = dev->link;
     ovs_mutex_unlock(&dev->mutex);
 
-    if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
-        if (link.link_speed == ETH_SPEED_NUM_10M) {
-            *current = NETDEV_F_10MB_HD;
-        }
-        if (link.link_speed == ETH_SPEED_NUM_100M) {
-            *current = NETDEV_F_100MB_HD;
-        }
-        if (link.link_speed == ETH_SPEED_NUM_1G) {
-            *current = NETDEV_F_1GB_HD;
-        }
-    } else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
-        if (link.link_speed == ETH_SPEED_NUM_10M) {
-            *current = NETDEV_F_10MB_FD;
-        }
-        if (link.link_speed == ETH_SPEED_NUM_100M) {
-            *current = NETDEV_F_100MB_FD;
-        }
-        if (link.link_speed == ETH_SPEED_NUM_1G) {
-            *current = NETDEV_F_1GB_FD;
-        }
-        if (link.link_speed == ETH_SPEED_NUM_10G) {
-            *current = NETDEV_F_10GB_FD;
+    if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
+        switch (link.link_speed) {
+        /* OpenFlow defined values: see enum ofp_port_features */
+        case ETH_SPEED_NUM_10M:
+            feature |= NETDEV_F_10MB_FD;
+            break;
+        case ETH_SPEED_NUM_100M:
+            feature |= NETDEV_F_100MB_FD;
+            break;
+        case ETH_SPEED_NUM_1G:
+            feature |= NETDEV_F_1GB_FD;
+            break;
+        case ETH_SPEED_NUM_10G:
+            feature |= NETDEV_F_10GB_FD;
+            break;
+        case ETH_SPEED_NUM_40G:
+            feature |= NETDEV_F_40GB_FD;
+            break;
+        case ETH_SPEED_NUM_100G:
+            feature |= NETDEV_F_100GB_FD;
+            break;
+        default:
+            feature |= NETDEV_F_OTHER;
         }
-        if (link.link_speed == ETH_SPEED_NUM_40G) {
-            *current = NETDEV_F_40GB_FD;
+    }
+    else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
+        switch (link.link_speed) {
+        case ETH_SPEED_NUM_10M:
+            feature |= NETDEV_F_10MB_HD;
+            break;
+        case ETH_SPEED_NUM_100M:
+            feature |= NETDEV_F_100MB_HD;
+            break;
+        case ETH_SPEED_NUM_1G:
+            feature |= NETDEV_F_1GB_HD;
+            break;
+        default:
+            feature |= NETDEV_F_OTHER;
         }
     }
 
     if (link.link_autoneg) {
-        *current |= NETDEV_F_AUTONEG;
+        feature |= NETDEV_F_AUTONEG;
     }
 
+    *current = feature;
     *advertised = *supported = *peer = 0;
 
     return 0;
-- 
2.13.6



More information about the dev mailing list