From 14f31bb39f5d4b69c179d219833d7edb9b36ebd9 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Sat, 9 Apr 2016 00:03:28 +0800 Subject: [PATCH 1/6] bridge: simplify the flush_store by calling store_bridge_parm There are some repetitive codes in flush_store, we can remove them by calling store_bridge_parm, also, it would send rtnl notification after we add it in store_bridge_parm in the following patches. Signed-off-by: Xin Long Reviewed-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- net/bridge/br_sysfs_br.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c index 6b8091407ca31..c48f6b0b20229 100644 --- a/net/bridge/br_sysfs_br.c +++ b/net/bridge/br_sysfs_br.c @@ -336,17 +336,17 @@ static ssize_t group_addr_store(struct device *d, static DEVICE_ATTR_RW(group_addr); +static int set_flush(struct net_bridge *br, unsigned long val) +{ + br_fdb_flush(br); + return 0; +} + static ssize_t flush_store(struct device *d, struct device_attribute *attr, const char *buf, size_t len) { - struct net_bridge *br = to_bridge(d); - - if (!ns_capable(dev_net(br->dev)->user_ns, CAP_NET_ADMIN)) - return -EPERM; - - br_fdb_flush(br); - return len; + return store_bridge_parm(d, buf, len, set_flush); } static DEVICE_ATTR_WO(flush); From 347db6b49ec0ba5ee3c9d946d45b7db59cf40480 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Sat, 9 Apr 2016 00:03:29 +0800 Subject: [PATCH 2/6] bridge: simplify the forward_delay_store by calling store_bridge_parm There are some repetitive codes in forward_delay_store, we can remove them by calling store_bridge_parm. Signed-off-by: Xin Long Reviewed-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- net/bridge/br_sysfs_br.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c index c48f6b0b20229..137cd3bf2565d 100644 --- a/net/bridge/br_sysfs_br.c +++ b/net/bridge/br_sysfs_br.c @@ -160,29 +160,22 @@ static ssize_t group_fwd_mask_show(struct device *d, return sprintf(buf, "%#x\n", br->group_fwd_mask); } - -static ssize_t group_fwd_mask_store(struct device *d, - struct device_attribute *attr, - const char *buf, - size_t len) +static int set_group_fwd_mask(struct net_bridge *br, unsigned long val) { - struct net_bridge *br = to_bridge(d); - char *endp; - unsigned long val; - - if (!ns_capable(dev_net(br->dev)->user_ns, CAP_NET_ADMIN)) - return -EPERM; - - val = simple_strtoul(buf, &endp, 0); - if (endp == buf) - return -EINVAL; - if (val & BR_GROUPFWD_RESTRICTED) return -EINVAL; br->group_fwd_mask = val; - return len; + return 0; +} + +static ssize_t group_fwd_mask_store(struct device *d, + struct device_attribute *attr, + const char *buf, + size_t len) +{ + return store_bridge_parm(d, buf, len, set_group_fwd_mask); } static DEVICE_ATTR_RW(group_fwd_mask); From 4436156b6fbec746108d45431a9f1885de810ec1 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Sat, 9 Apr 2016 00:03:30 +0800 Subject: [PATCH 3/6] bridge: simplify the stp_state_store by calling store_bridge_parm There are some repetitive codes in stp_state_store, we can remove them by calling store_bridge_parm. Signed-off-by: Xin Long Reviewed-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- net/bridge/br_sysfs_br.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c index 137cd3bf2565d..f9d484ecae07b 100644 --- a/net/bridge/br_sysfs_br.c +++ b/net/bridge/br_sysfs_br.c @@ -128,27 +128,21 @@ static ssize_t stp_state_show(struct device *d, } -static ssize_t stp_state_store(struct device *d, - struct device_attribute *attr, const char *buf, - size_t len) +static int set_stp_state(struct net_bridge *br, unsigned long val) { - struct net_bridge *br = to_bridge(d); - char *endp; - unsigned long val; - - if (!ns_capable(dev_net(br->dev)->user_ns, CAP_NET_ADMIN)) - return -EPERM; - - val = simple_strtoul(buf, &endp, 0); - if (endp == buf) - return -EINVAL; - if (!rtnl_trylock()) return restart_syscall(); br_stp_set_enabled(br, val); rtnl_unlock(); - return len; + return 0; +} + +static ssize_t stp_state_store(struct device *d, + struct device_attribute *attr, const char *buf, + size_t len) +{ + return store_bridge_parm(d, buf, len, set_stp_state); } static DEVICE_ATTR_RW(stp_state); From 047831a9b9c3e34410025df84f629c005f437e42 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Sat, 9 Apr 2016 00:03:31 +0800 Subject: [PATCH 4/6] bridge: a netlink notification should be sent when those attributes are changed by br_sysfs_br Now when we change the attributes of bridge or br_port by netlink, a relevant netlink notification will be sent, but if we change them by ioctl or sysfs, no notification will be sent. We should ensure that whenever those attributes change internally or from sysfs/ioctl, that a netlink notification is sent out to listeners. Also, NetworkManager will use this in the future to listen for out-of-band bridge master attribute updates and incorporate them into the runtime configuration. This patch is used for br_sysfs_br. and we also need to remove some rtnl_trylock in old functions so that we can call it in a common one. For group_addr_store, we cannot make it use store_bridge_parm, because it's not a string-to-long convert, we will add notification on it individually. Signed-off-by: Xin Long Signed-off-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- net/bridge/br_sysfs_br.c | 21 +++++++++------------ net/bridge/br_vlan.c | 30 +++++------------------------- 2 files changed, 14 insertions(+), 37 deletions(-) diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c index f9d484ecae07b..70bddfd0f3e9f 100644 --- a/net/bridge/br_sysfs_br.c +++ b/net/bridge/br_sysfs_br.c @@ -43,7 +43,14 @@ static ssize_t store_bridge_parm(struct device *d, if (endp == buf) return -EINVAL; + if (!rtnl_trylock()) + return restart_syscall(); + err = (*set)(br, val); + if (!err) + netdev_state_change(br->dev); + rtnl_unlock(); + return err ? err : len; } @@ -101,15 +108,7 @@ static ssize_t ageing_time_show(struct device *d, static int set_ageing_time(struct net_bridge *br, unsigned long val) { - int ret; - - if (!rtnl_trylock()) - return restart_syscall(); - - ret = br_set_ageing_time(br, val); - rtnl_unlock(); - - return ret; + return br_set_ageing_time(br, val); } static ssize_t ageing_time_store(struct device *d, @@ -130,10 +129,7 @@ static ssize_t stp_state_show(struct device *d, static int set_stp_state(struct net_bridge *br, unsigned long val) { - if (!rtnl_trylock()) - return restart_syscall(); br_stp_set_enabled(br, val); - rtnl_unlock(); return 0; } @@ -315,6 +311,7 @@ static ssize_t group_addr_store(struct device *d, br->group_addr_set = true; br_recalculate_fwd_mask(br); + netdev_state_change(br->dev); rtnl_unlock(); diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c index 9309bb4f2a5b2..e001152d6ad1a 100644 --- a/net/bridge/br_vlan.c +++ b/net/bridge/br_vlan.c @@ -651,15 +651,7 @@ int __br_vlan_filter_toggle(struct net_bridge *br, unsigned long val) int br_vlan_filter_toggle(struct net_bridge *br, unsigned long val) { - int err; - - if (!rtnl_trylock()) - return restart_syscall(); - - err = __br_vlan_filter_toggle(br, val); - rtnl_unlock(); - - return err; + return __br_vlan_filter_toggle(br, val); } int __br_vlan_set_proto(struct net_bridge *br, __be16 proto) @@ -713,18 +705,10 @@ int __br_vlan_set_proto(struct net_bridge *br, __be16 proto) int br_vlan_set_proto(struct net_bridge *br, unsigned long val) { - int err; - if (val != ETH_P_8021Q && val != ETH_P_8021AD) return -EPROTONOSUPPORT; - if (!rtnl_trylock()) - return restart_syscall(); - - err = __br_vlan_set_proto(br, htons(val)); - rtnl_unlock(); - - return err; + return __br_vlan_set_proto(br, htons(val)); } static bool vlan_default_pvid(struct net_bridge_vlan_group *vg, u16 vid) @@ -855,21 +839,17 @@ int br_vlan_set_default_pvid(struct net_bridge *br, unsigned long val) if (val >= VLAN_VID_MASK) return -EINVAL; - if (!rtnl_trylock()) - return restart_syscall(); - if (pvid == br->default_pvid) - goto unlock; + goto out; /* Only allow default pvid change when filtering is disabled */ if (br->vlan_enabled) { pr_info_once("Please disable vlan filtering to change default_pvid\n"); err = -EPERM; - goto unlock; + goto out; } err = __br_vlan_set_default_pvid(br, pvid); -unlock: - rtnl_unlock(); +out: return err; } From bdaf0d5d98e1c42d3a48c5ce6db9d013cb882781 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Sat, 9 Apr 2016 00:03:32 +0800 Subject: [PATCH 5/6] bridge: a netlink notification should be sent when those attributes are changed by br_sysfs_if Now when we change the attributes of bridge or br_port by netlink, a relevant netlink notification will be sent, but if we change them by ioctl or sysfs, no notification will be sent. We should ensure that whenever those attributes change internally or from sysfs/ioctl, that a netlink notification is sent out to listeners. Also, NetworkManager will use this in the future to listen for out-of-band bridge master attribute updates and incorporate them into the runtime configuration. This patch is used for br_sysfs_if, and we also move br_ifinfo_notify out of store_flag. Signed-off-by: Xin Long Reviewed-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- net/bridge/br_sysfs_if.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c index efe415ad842a4..1e04d4d44273f 100644 --- a/net/bridge/br_sysfs_if.c +++ b/net/bridge/br_sysfs_if.c @@ -61,7 +61,6 @@ static int store_flag(struct net_bridge_port *p, unsigned long v, if (flags != p->flags) { p->flags = flags; br_port_flags_change(p, mask); - br_ifinfo_notify(RTM_NEWLINK, p); } return 0; } @@ -253,8 +252,10 @@ static ssize_t brport_store(struct kobject *kobj, spin_lock_bh(&p->br->lock); ret = brport_attr->store(p, val); spin_unlock_bh(&p->br->lock); - if (ret == 0) + if (!ret) { + br_ifinfo_notify(RTM_NEWLINK, p); ret = count; + } } rtnl_unlock(); } From bf871ad792e3c9f5dda0ef5bd519e0a2f1564001 Mon Sep 17 00:00:00 2001 From: Xin Long Date: Sat, 9 Apr 2016 00:03:33 +0800 Subject: [PATCH 6/6] bridge: a netlink notification should be sent when those attributes are changed by ioctl Now when we change the attributes of bridge or br_port by netlink, a relevant netlink notification will be sent, but if we change them by ioctl or sysfs, no notification will be sent. We should ensure that whenever those attributes change internally or from sysfs/ioctl, that a netlink notification is sent out to listeners. Also, NetworkManager will use this in the future to listen for out-of-band bridge master attribute updates and incorporate them into the runtime configuration. This patch is used for ioctl. Signed-off-by: Xin Long Reviewed-by: Nikolay Aleksandrov Signed-off-by: David S. Miller --- net/bridge/br_ioctl.c | 40 ++++++++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 16 deletions(-) diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c index 263b4de4de57c..f8fc6241469ad 100644 --- a/net/bridge/br_ioctl.c +++ b/net/bridge/br_ioctl.c @@ -112,7 +112,9 @@ static int add_del_if(struct net_bridge *br, int ifindex, int isadd) static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) { struct net_bridge *br = netdev_priv(dev); + struct net_bridge_port *p = NULL; unsigned long args[4]; + int ret = -EOPNOTSUPP; if (copy_from_user(args, rq->ifr_data, sizeof(args))) return -EFAULT; @@ -182,25 +184,29 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) if (!ns_capable(dev_net(dev)->user_ns, CAP_NET_ADMIN)) return -EPERM; - return br_set_forward_delay(br, args[1]); + ret = br_set_forward_delay(br, args[1]); + break; case BRCTL_SET_BRIDGE_HELLO_TIME: if (!ns_capable(dev_net(dev)->user_ns, CAP_NET_ADMIN)) return -EPERM; - return br_set_hello_time(br, args[1]); + ret = br_set_hello_time(br, args[1]); + break; case BRCTL_SET_BRIDGE_MAX_AGE: if (!ns_capable(dev_net(dev)->user_ns, CAP_NET_ADMIN)) return -EPERM; - return br_set_max_age(br, args[1]); + ret = br_set_max_age(br, args[1]); + break; case BRCTL_SET_AGEING_TIME: if (!ns_capable(dev_net(dev)->user_ns, CAP_NET_ADMIN)) return -EPERM; - return br_set_ageing_time(br, args[1]); + ret = br_set_ageing_time(br, args[1]); + break; case BRCTL_GET_PORT_INFO: { @@ -240,20 +246,19 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) return -EPERM; br_stp_set_enabled(br, args[1]); - return 0; + ret = 0; + break; case BRCTL_SET_BRIDGE_PRIORITY: if (!ns_capable(dev_net(dev)->user_ns, CAP_NET_ADMIN)) return -EPERM; br_stp_set_bridge_priority(br, args[1]); - return 0; + ret = 0; + break; case BRCTL_SET_PORT_PRIORITY: { - struct net_bridge_port *p; - int ret; - if (!ns_capable(dev_net(dev)->user_ns, CAP_NET_ADMIN)) return -EPERM; @@ -263,14 +268,11 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) else ret = br_stp_set_port_priority(p, args[2]); spin_unlock_bh(&br->lock); - return ret; + break; } case BRCTL_SET_PATH_COST: { - struct net_bridge_port *p; - int ret; - if (!ns_capable(dev_net(dev)->user_ns, CAP_NET_ADMIN)) return -EPERM; @@ -280,8 +282,7 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) else ret = br_stp_set_path_cost(p, args[2]); spin_unlock_bh(&br->lock); - - return ret; + break; } case BRCTL_GET_FDB_ENTRIES: @@ -289,7 +290,14 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) args[2], args[3]); } - return -EOPNOTSUPP; + if (!ret) { + if (p) + br_ifinfo_notify(RTM_NEWLINK, p); + else + netdev_state_change(br->dev); + } + + return ret; } static int old_deviceless(struct net *net, void __user *uarg)