Re: [PATCH] bonding: Prevent deletion of a bond, or the last slave from a bond, with active usage.
flat view
From: Jay Vosburgh <hidden>
Date: 2016-09-06 15:09:06
Also in:
lkml
Kaur, Jasminder [off-list ref] wrote:
From: "Kaur, Jasminder" <redacted> If a bond is in use such as with IP address configured, removing it can result in application disruptions. If bond is used for cluster communication or network file system interfaces, removing it can cause system down time. An additional write option “?-” is added to sysfs bond interfaces as below, in order to prevent accidental deletions while bond is in use. In the absence of any usage, the below option proceeds with bond deletion. “ echo "?-bondX" > /sys/class/net/bonding_masters “ . If usage is detected such as an IP address configured, deletion is prevented with appropriate message logged to syslog.
The issue of interfaces being arbitrarily changed or deleted is not specific to bonding, and could affect any networking device (physical or virtual). Thus, if a facility such as this is to be provided, it should be generic, not specific to bonding. Separately, I'm not sure I see the value of such an option. Other than administrator error, I'm not sure when bonds (or other interfaces) would be randomly deleted. Are you seeing that happening? Also, this patch does not prevent other errors or malicious change, e.g., "ip link set bondX down" or "ip addr del 1.2.3.4/24" would still cause the service disruption you're trying to avoid. And, lastly, what Jiri said: use netlink for new bonding functionality, not sysfs. -J
quoted hunk ↗ jump to hunk
In the absence of any usage, the below option proceeds with deletion of slaves from a bond. “ echo "?-enoX" > /sys/class/net/bondX/bonding/slaves “ . If usage is detected such as an IP address configured on bond, deletion is prevented if the last slave is being removed from bond. An appropriate message is logged to syslog. Signed-off-by: Jasminder Kaur <redacted> --- drivers/net/bonding/bond_options.c | 24 ++++++++++++++++++++++-- drivers/net/bonding/bond_sysfs.c | 35 +++++++++++++++++++++++++++++++++-- 2 files changed, 55 insertions(+), 4 deletions(-)diff --git a/drivers/net/bonding/bond_options.c b/drivers/net/bonding/bond_options.c index 577e57c..e7640ea 100644 --- a/drivers/net/bonding/bond_options.c +++ b/drivers/net/bonding/bond_options.c@@ -1335,9 +1335,15 @@ static int bond_option_slaves_set(struct bonding *bond,struct net_device *dev; char *ifname; int ret; + struct in_device *in_dev; sscanf(newval->string, "%16s", command); /* IFNAMSIZ*/ - ifname = command + 1; + + if ((command[0] == '?') && (command[1] == '-')) + ifname = command + 2; + else + ifname = command + 1; + if ((strlen(command) <= 1) || !dev_valid_name(ifname)) goto err_no_cmd;@@ -1356,6 +1362,20 @@ static int bond_option_slaves_set(struct bonding *bond,ret = bond_enslave(bond->dev, dev); break; + case '?': + if (command[1] == '-') { + in_dev = __in_dev_get_rtnl(bond->dev); + if ((bond->slave_cnt == 1) && + ((in_dev->ifa_list) != NULL)) { + netdev_info(bond->dev, "attempt to remove last slave %s from bond.\n", + dev->name); + ret = -EBUSY; + break; + } + } else { + goto err_no_cmd; + } + case '-': netdev_info(bond->dev, "Removing slave %s\n", dev->name); ret = bond_release(bond->dev, dev);@@ -1369,7 +1389,7 @@ out:return ret; err_no_cmd: - netdev_err(bond->dev, "no command found in slaves file - use +ifname or -ifname\n"); + netdev_err(bond->dev, "no command found in slaves file - use +ifname or -ifname or ?-ifname\n"); ret = -EPERM; goto out; }diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c index e23c3ed..7c2ef64 100644 --- a/drivers/net/bonding/bond_sysfs.c +++ b/drivers/net/bonding/bond_sysfs.c@@ -102,7 +102,12 @@ static ssize_t bonding_store_bonds(struct class *cls,int rv, res = count; sscanf(buffer, "%16s", command); /* IFNAMSIZ*/ - ifname = command + 1; + + if ((command[0] == '?') && (command[1] == '-')) + ifname = command + 2; + else + ifname = command + 1; + if ((strlen(command) <= 1) || !dev_valid_name(ifname)) goto err_no_cmd;@@ -130,6 +135,32 @@ static ssize_t bonding_store_bonds(struct class *cls,res = -ENODEV; } rtnl_unlock(); + } else if ((command[0] == '?') && (command[1] == '-')) { + struct net_device *bond_dev; + + rtnl_lock(); + bond_dev = bond_get_by_name(bn, ifname); + + if (bond_dev) { + struct in_device *in_dev; + struct bonding *bond = netdev_priv(bond_dev); + + in_dev = __in_dev_get_rtnl(bond_dev); + + if (((in_dev->ifa_list) != NULL) && + (bond->slave_cnt > 0)) { + pr_err("%s is in use. Unconfigure IP %pI4 before deletion.\n", + ifname, &in_dev->ifa_list->ifa_local); + rtnl_unlock(); + return -EBUSY; + } + pr_info("%s is being deleted...\n", ifname); + unregister_netdevice(bond_dev); + } else { + pr_err("unable to delete non-existent %s\n", ifname); + res = -ENODEV; + } + rtnl_unlock(); } else goto err_no_cmd;@@ -139,7 +170,7 @@ static ssize_t bonding_store_bonds(struct class *cls,return res; err_no_cmd: - pr_err("no command found in bonding_masters - use +ifname or -ifname\n"); + pr_err("no command found in bonding_masters - use +ifname or -ifname or ?-ifname\n"); return -EPERM; } -- 1.8.3.1
--- -Jay Vosburgh, jay.vosburgh@canonical.com