diff options
author | Stephen Hemminger <shemminger@vyatta.com> | 2008-09-08 16:44:40 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2008-09-08 16:46:54 -0400 |
commit | 8d4698f7a54a492a1b96c505b30fe750ae3e61d5 (patch) | |
tree | 4bec6c7dcf06bb5df9645d702d554a071007b83a | |
parent | d315492b1a6ba29da0fa2860759505ae1b2db857 (diff) |
bridge: don't allow setting hello time to zero
Dushan Tcholich reports that on his system ksoftirqd can consume
between %6 to %10 of cpu time, and cause ~200 context switches per
second.
He then correlated this with a report by bdupree@techfinesse.com:
http://marc.info/?l=linux-kernel&m=119613299024398&w=2
and the culprit cause seems to be starting the bridge interface.
In particular, when starting the bridge interface, his scripts
are specifying a hello timer interval of "0".
The bridge hello time can't be safely set to values less than 1
second, otherwise it is possible to end up with a runaway timer.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r-- | net/bridge/br_ioctl.c | 8 | ||||
-rw-r--r-- | net/bridge/br_sysfs_br.c | 26 |
2 files changed, 25 insertions, 9 deletions
diff --git a/net/bridge/br_ioctl.c b/net/bridge/br_ioctl.c index eeee218eed80..5bbf07362172 100644 --- a/net/bridge/br_ioctl.c +++ b/net/bridge/br_ioctl.c | |||
@@ -188,15 +188,21 @@ static int old_dev_ioctl(struct net_device *dev, struct ifreq *rq, int cmd) | |||
188 | return 0; | 188 | return 0; |
189 | 189 | ||
190 | case BRCTL_SET_BRIDGE_HELLO_TIME: | 190 | case BRCTL_SET_BRIDGE_HELLO_TIME: |
191 | { | ||
192 | unsigned long t = clock_t_to_jiffies(args[1]); | ||
191 | if (!capable(CAP_NET_ADMIN)) | 193 | if (!capable(CAP_NET_ADMIN)) |
192 | return -EPERM; | 194 | return -EPERM; |
193 | 195 | ||
196 | if (t < HZ) | ||
197 | return -EINVAL; | ||
198 | |||
194 | spin_lock_bh(&br->lock); | 199 | spin_lock_bh(&br->lock); |
195 | br->bridge_hello_time = clock_t_to_jiffies(args[1]); | 200 | br->bridge_hello_time = t; |
196 | if (br_is_root_bridge(br)) | 201 | if (br_is_root_bridge(br)) |
197 | br->hello_time = br->bridge_hello_time; | 202 | br->hello_time = br->bridge_hello_time; |
198 | spin_unlock_bh(&br->lock); | 203 | spin_unlock_bh(&br->lock); |
199 | return 0; | 204 | return 0; |
205 | } | ||
200 | 206 | ||
201 | case BRCTL_SET_BRIDGE_MAX_AGE: | 207 | case BRCTL_SET_BRIDGE_MAX_AGE: |
202 | if (!capable(CAP_NET_ADMIN)) | 208 | if (!capable(CAP_NET_ADMIN)) |
diff --git a/net/bridge/br_sysfs_br.c b/net/bridge/br_sysfs_br.c index 27d6a511c8c1..158dee8b4965 100644 --- a/net/bridge/br_sysfs_br.c +++ b/net/bridge/br_sysfs_br.c | |||
@@ -29,11 +29,12 @@ | |||
29 | */ | 29 | */ |
30 | static ssize_t store_bridge_parm(struct device *d, | 30 | static ssize_t store_bridge_parm(struct device *d, |
31 | const char *buf, size_t len, | 31 | const char *buf, size_t len, |
32 | void (*set)(struct net_bridge *, unsigned long)) | 32 | int (*set)(struct net_bridge *, unsigned long)) |
33 | { | 33 | { |
34 | struct net_bridge *br = to_bridge(d); | 34 | struct net_bridge *br = to_bridge(d); |
35 | char *endp; | 35 | char *endp; |
36 | unsigned long val; | 36 | unsigned long val; |
37 | int err; | ||
37 | 38 | ||
38 | if (!capable(CAP_NET_ADMIN)) | 39 | if (!capable(CAP_NET_ADMIN)) |
39 | return -EPERM; | 40 | return -EPERM; |
@@ -43,9 +44,9 @@ static ssize_t store_bridge_parm(struct device *d, | |||
43 | return -EINVAL; | 44 | return -EINVAL; |
44 | 45 | ||
45 | spin_lock_bh(&br->lock); | 46 | spin_lock_bh(&br->lock); |
46 | (*set)(br, val); | 47 | err = (*set)(br, val); |
47 | spin_unlock_bh(&br->lock); | 48 | spin_unlock_bh(&br->lock); |
48 | return len; | 49 | return err ? err : len; |
49 | } | 50 | } |
50 | 51 | ||
51 | 52 | ||
@@ -56,12 +57,13 @@ static ssize_t show_forward_delay(struct device *d, | |||
56 | return sprintf(buf, "%lu\n", jiffies_to_clock_t(br->forward_delay)); | 57 | return sprintf(buf, "%lu\n", jiffies_to_clock_t(br->forward_delay)); |
57 | } | 58 | } |
58 | 59 | ||
59 | static void set_forward_delay(struct net_bridge *br, unsigned long val) | 60 | static int set_forward_delay(struct net_bridge *br, unsigned long val) |
60 | { | 61 | { |
61 | unsigned long delay = clock_t_to_jiffies(val); | 62 | unsigned long delay = clock_t_to_jiffies(val); |
62 | br->forward_delay = delay; | 63 | br->forward_delay = delay; |
63 | if (br_is_root_bridge(br)) | 64 | if (br_is_root_bridge(br)) |
64 | br->bridge_forward_delay = delay; | 65 | br->bridge_forward_delay = delay; |
66 | return 0; | ||
65 | } | 67 | } |
66 | 68 | ||
67 | static ssize_t store_forward_delay(struct device *d, | 69 | static ssize_t store_forward_delay(struct device *d, |
@@ -80,12 +82,17 @@ static ssize_t show_hello_time(struct device *d, struct device_attribute *attr, | |||
80 | jiffies_to_clock_t(to_bridge(d)->hello_time)); | 82 | jiffies_to_clock_t(to_bridge(d)->hello_time)); |
81 | } | 83 | } |
82 | 84 | ||
83 | static void set_hello_time(struct net_bridge *br, unsigned long val) | 85 | static int set_hello_time(struct net_bridge *br, unsigned long val) |
84 | { | 86 | { |
85 | unsigned long t = clock_t_to_jiffies(val); | 87 | unsigned long t = clock_t_to_jiffies(val); |
88 | |||
89 | if (t < HZ) | ||
90 | return -EINVAL; | ||
91 | |||
86 | br->hello_time = t; | 92 | br->hello_time = t; |
87 | if (br_is_root_bridge(br)) | 93 | if (br_is_root_bridge(br)) |
88 | br->bridge_hello_time = t; | 94 | br->bridge_hello_time = t; |
95 | return 0; | ||
89 | } | 96 | } |
90 | 97 | ||
91 | static ssize_t store_hello_time(struct device *d, | 98 | static ssize_t store_hello_time(struct device *d, |
@@ -104,12 +111,13 @@ static ssize_t show_max_age(struct device *d, struct device_attribute *attr, | |||
104 | jiffies_to_clock_t(to_bridge(d)->max_age)); | 111 | jiffies_to_clock_t(to_bridge(d)->max_age)); |
105 | } | 112 | } |
106 | 113 | ||
107 | static void set_max_age(struct net_bridge *br, unsigned long val) | 114 | static int set_max_age(struct net_bridge *br, unsigned long val) |
108 | { | 115 | { |
109 | unsigned long t = clock_t_to_jiffies(val); | 116 | unsigned long t = clock_t_to_jiffies(val); |
110 | br->max_age = t; | 117 | br->max_age = t; |
111 | if (br_is_root_bridge(br)) | 118 | if (br_is_root_bridge(br)) |
112 | br->bridge_max_age = t; | 119 | br->bridge_max_age = t; |
120 | return 0; | ||
113 | } | 121 | } |
114 | 122 | ||
115 | static ssize_t store_max_age(struct device *d, struct device_attribute *attr, | 123 | static ssize_t store_max_age(struct device *d, struct device_attribute *attr, |
@@ -126,9 +134,10 @@ static ssize_t show_ageing_time(struct device *d, | |||
126 | return sprintf(buf, "%lu\n", jiffies_to_clock_t(br->ageing_time)); | 134 | return sprintf(buf, "%lu\n", jiffies_to_clock_t(br->ageing_time)); |
127 | } | 135 | } |
128 | 136 | ||
129 | static void set_ageing_time(struct net_bridge *br, unsigned long val) | 137 | static int set_ageing_time(struct net_bridge *br, unsigned long val) |
130 | { | 138 | { |
131 | br->ageing_time = clock_t_to_jiffies(val); | 139 | br->ageing_time = clock_t_to_jiffies(val); |
140 | return 0; | ||
132 | } | 141 | } |
133 | 142 | ||
134 | static ssize_t store_ageing_time(struct device *d, | 143 | static ssize_t store_ageing_time(struct device *d, |
@@ -180,9 +189,10 @@ static ssize_t show_priority(struct device *d, struct device_attribute *attr, | |||
180 | (br->bridge_id.prio[0] << 8) | br->bridge_id.prio[1]); | 189 | (br->bridge_id.prio[0] << 8) | br->bridge_id.prio[1]); |
181 | } | 190 | } |
182 | 191 | ||
183 | static void set_priority(struct net_bridge *br, unsigned long val) | 192 | static int set_priority(struct net_bridge *br, unsigned long val) |
184 | { | 193 | { |
185 | br_stp_set_bridge_priority(br, (u16) val); | 194 | br_stp_set_bridge_priority(br, (u16) val); |
195 | return 0; | ||
186 | } | 196 | } |
187 | 197 | ||
188 | static ssize_t store_priority(struct device *d, struct device_attribute *attr, | 198 | static ssize_t store_priority(struct device *d, struct device_attribute *attr, |