Netdev flags are queried multiple times per poll loop. Sending a GETLINK
command for each can be very inefficient if there is RTNL lock
contention.

Cache the result of the query as we do with other netdev attributes.

Therefore, "netdev_get_flags" no longer fully populates the full
internal netdev state. It only updates the flags, if needed.

While testing, "insert_ipdev__()" was flagged as (incorrectly)
relying on the old behavior, so this patch also makes sure the
netdev's seq is cached only after the relevant internal fields
have been fully updated.

Signed-off-by: Adrian Moreno <[email protected]>
---
 lib/netdev-linux.c | 14 +++++++++++---
 lib/tnl-ports.c    |  2 +-
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index eefe80865..a7c0f5451 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -265,6 +265,7 @@ enum {
     VALID_DRVINFO           = 1 << 6,
     VALID_FEATURES          = 1 << 7,
     VALID_NUMA_ID           = 1 << 8,
+    VALID_FLAGS             = 1 << 9,
 };
 
 /* Linux 4.4 introduced the ability to skip the internal stats gathering
@@ -833,7 +834,7 @@ netdev_linux_run(const struct netdev_class *netdev_class 
OVS_UNUSED)
 
                 ovs_mutex_lock(&netdev->mutex);
                 netdev_linux_update_via_ioctl(netdev);
-                netdev_linux_changed(netdev, 0);
+                netdev_linux_changed(netdev, VALID_FLAGS);
                 ovs_mutex_unlock(&netdev->mutex);
 
                 netdev_close(netdev_);
@@ -908,6 +909,7 @@ netdev_linux_update__(struct netdev_linux *dev,
 
             dev->ifindex = change->if_index;
             dev->cache_valid |= VALID_IFINDEX;
+            dev->cache_valid |= VALID_FLAGS;
             dev->get_ifindex_error = 0;
             dev->present = true;
         } else {
@@ -3881,7 +3883,10 @@ netdev_linux_update_via_ioctl(struct netdev_linux 
*netdev)
     int error;
 
     error = get_flags(&netdev->up, &ifi_flags);
-    netdev_linux_set_flags(netdev, ifi_flags);
+    if (!error) {
+        netdev_linux_set_flags(netdev, ifi_flags);
+        netdev->cache_valid |= VALID_FLAGS;
+    }
     return error;
 }
 
@@ -3902,7 +3907,8 @@ netdev_linux_update_flags(struct netdev *netdev_, enum 
netdev_flags off,
         error = modify_flags(netdev, off, on, old_flagsp);
     } else {
         /* Try reading flags over netlink, or fall back to ioctl. */
-        if (netdev_linux_update_via_netlink(netdev)) {
+        if (!(netdev->cache_valid & VALID_FLAGS) &&
+            netdev_linux_update_via_netlink(netdev)) {
             error = netdev_linux_update_via_ioctl(netdev);
         }
         *old_flagsp = iff_to_nd_flags(netdev->ifi_flags);
@@ -6969,6 +6975,8 @@ netdev_linux_update_via_netlink(struct netdev_linux 
*netdev)
             netdev->ifi_flags = change->ifi_flags;
             changed = true;
         }
+        netdev->cache_valid |= VALID_FLAGS;
+
         if (change->mtu && change->mtu != netdev->mtu) {
             netdev->mtu = change->mtu;
             netdev->cache_valid |= VALID_MTU;
diff --git a/lib/tnl-ports.c b/lib/tnl-ports.c
index 56119b300..67b6ebcf7 100644
--- a/lib/tnl-ports.c
+++ b/lib/tnl-ports.c
@@ -402,13 +402,13 @@ insert_ipdev__(struct netdev *dev,
 
     ip_dev = xzalloc(sizeof *ip_dev);
     ip_dev->dev = netdev_ref(dev);
-    ip_dev->change_seq = netdev_get_change_seq(dev);
     error = netdev_get_etheraddr(ip_dev->dev, &ip_dev->mac);
     if (error) {
         goto err_free_ipdev;
     }
     ip_dev->addr = addr;
     ip_dev->n_addr = n_addr;
+    ip_dev->change_seq = netdev_get_change_seq(dev);
     ovs_strlcpy(ip_dev->dev_name, netdev_get_name(dev), sizeof 
ip_dev->dev_name);
     ovs_list_insert(&addr_list, &ip_dev->node);
     map_insert_ipdev(ip_dev);
-- 
2.54.0

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

Reply via email to