diff mbox

[ovs-dev,PATCHv2] bridge: Prohibit "default" and "all" bridge name.

Message ID 1493472659-12834-1-git-send-email-u9012063@gmail.com
State Accepted
Headers show

Commit Message

William Tu April 29, 2017, 1:30 p.m. UTC 
Under Linux, when users create bridge named "default" or "all", although
ovs-vsctl fails but vswitchd in the background will keep retrying it,
causing the systemd-udev to reach 100% cpu utilization. The patch prevents
any attempt to create or open a netdev named "default" or "all" because
these two names are reserved on Linux due to
/proc/sys/net/ipv4/conf/ always contains directories by these names.

The reason for high CPU utilization is due to frequent calls into kernel's
register_netdevice function, which will invoke several kernel elements who
has registered on the netdevice notifier chain.  And due to creation failed,
OVS wakes up and re-recreate the device, which ends up as a high CPU loop.

VMWare-BZ: #1842388
Signed-off-by: William Tu <u9012063@gmail.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
Acked-by: Greg Rose <gvrose8192@gmail.com>
---
v1->v2: move to Linux specific implementation.
---
 lib/netdev-linux.c | 39 ++++++++++++++++++++++++++++++---------
 1 file changed, 30 insertions(+), 9 deletions(-)

Comments

Ben Pfaff May 1, 2017, 5:27 p.m. UTC | #1 
On Sat, Apr 29, 2017 at 06:30:59AM -0700, William Tu wrote:
> Under Linux, when users create bridge named "default" or "all", although
> ovs-vsctl fails but vswitchd in the background will keep retrying it,
> causing the systemd-udev to reach 100% cpu utilization. The patch prevents
> any attempt to create or open a netdev named "default" or "all" because
> these two names are reserved on Linux due to
> /proc/sys/net/ipv4/conf/ always contains directories by these names.
> 
> The reason for high CPU utilization is due to frequent calls into kernel's
> register_netdevice function, which will invoke several kernel elements who
> has registered on the netdevice notifier chain.  And due to creation failed,
> OVS wakes up and re-recreate the device, which ends up as a high CPU loop.
> 
> VMWare-BZ: #1842388
> Signed-off-by: William Tu <u9012063@gmail.com>
> Signed-off-by: Ben Pfaff <blp@ovn.org>
> Acked-by: Greg Rose <gvrose8192@gmail.com>
> ---
> v1->v2: move to Linux specific implementation.

Thanks!  I applied this to master, branch-2.7, and branch-2.6.
diff mbox

Patch

diff --git a/lib/netdev-linux.c b/lib/netdev-linux.c
index 9ff1333f8e85..79e827303d07 100644
--- a/lib/netdev-linux.c
+++ b/lib/netdev-linux.c
@@ -1,5 +1,5 @@ 
 /*
- * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016 Nicira, Inc.
+ * Copyright (c) 2009, 2010, 2011, 2012, 2013, 2014, 2015, 2016, 2017 Nicira, Inc.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -773,10 +773,28 @@  netdev_linux_alloc(void)
     return &netdev->up;
 }
 
-static void
-netdev_linux_common_construct(struct netdev_linux *netdev)
-{
+static int
+netdev_linux_common_construct(struct netdev *netdev_)
+{
+    /* Prevent any attempt to create (or open) a network device named "default"
+     * or "all".  These device names are effectively reserved on Linux because
+     * /proc/sys/net/ipv4/conf/ always contains directories by these names.  By
+     * itself this wouldn't call for any special treatment, but in practice if
+     * a program tries to create devices with these names, it causes the kernel
+     * to fire a "new device" notification event even though creation failed,
+     * and in turn that causes OVS to wake up and try to create them again,
+     * which ends up as a 100% CPU loop. */
+    struct netdev_linux *netdev = netdev_linux_cast(netdev_);
+    const char *name = netdev_->name;
+    if (!strcmp(name, "default") || !strcmp(name, "all")) {
+        static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 1);
+        VLOG_WARN_RL(&rl, "%s: Linux forbids network device with this name",
+                     name);
+        return EINVAL;
+    }
+
     ovs_mutex_init(&netdev->mutex);
+    return 0;
 }
 
 /* Creates system and internal devices. */
@@ -784,9 +802,10 @@  static int
 netdev_linux_construct(struct netdev *netdev_)
 {
     struct netdev_linux *netdev = netdev_linux_cast(netdev_);
-    int error;
-
-    netdev_linux_common_construct(netdev);
+    int error = netdev_linux_common_construct(netdev_);
+    if (error) {
+        return error;
+    }
 
     error = get_flags(&netdev->up, &netdev->ifi_flags);
     if (error == ENODEV) {
@@ -817,9 +836,11 @@  netdev_linux_construct_tap(struct netdev *netdev_)
     static const char tap_dev[] = "/dev/net/tun";
     const char *name = netdev_->name;
     struct ifreq ifr;
-    int error;
 
-    netdev_linux_common_construct(netdev);
+    int error = netdev_linux_common_construct(netdev_);
+    if (error) {
+        return error;
+    }
 
     /* Open tap device. */
     netdev->tap_fd = open(tap_dev, O_RDWR);