diff options
author | Willem de Bruijn <willemb@google.com> | 2013-06-13 15:29:38 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2013-06-13 20:13:05 -0400 |
commit | 5f121b9a83b499a61ed44e5ba619c7de8f7271ad (patch) | |
tree | 6765d95f468e5c8c884af26f5fbfd2c6428d255d /net/core | |
parent | 85f16525a2eb66e6092cbd8dcf42371df8334ed0 (diff) |
net-rps: fixes for rps flow limit
Caught by sparse:
- __rcu: missing annotation to sd->flow_limit
- __user: direct access in cpumask_scnprintf
Also
- add endline character when printing bitmap if room in buffer
- avoid bucket overflow by reducing FLOW_LIMIT_HISTORY
The last item warrants some explanation. The hashtable buckets are
subject to overflow if FLOW_LIMIT_HISTORY is larger than or equal
to bucket size, since all packets may end up in a single bucket. The
current (rather arbitrary) history value of 256 happens to match the
buffer size (u8).
As a result, with a single flow, the first 128 packets are accepted
(correct), the second 128 packets dropped (correct) and then the
history[] array has filled, so that each subsequent new packet
causes an increment in the bucket for new_flow plus a decrement
for old_flow: a steady state.
This is fine if packets are dropped, as the steady state goes away
as soon as a mix of traffic reappears. But, because the 256th packet
overflowed the bucket to 0: no packets are dropped.
Instead of explicitly adding an overflow check, this patch changes
FLOW_LIMIT_HISTORY to never be able to overflow a single bucket.
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
(first item)
Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/core')
-rw-r--r-- | net/core/sysctl_net_core.c | 19 |
1 files changed, 16 insertions, 3 deletions
diff --git a/net/core/sysctl_net_core.c b/net/core/sysctl_net_core.c index 637a42e5d589..78c746e016ae 100644 --- a/net/core/sysctl_net_core.c +++ b/net/core/sysctl_net_core.c | |||
@@ -132,6 +132,8 @@ static int flow_limit_cpu_sysctl(struct ctl_table *table, int write, | |||
132 | write_unlock: | 132 | write_unlock: |
133 | mutex_unlock(&flow_limit_update_mutex); | 133 | mutex_unlock(&flow_limit_update_mutex); |
134 | } else { | 134 | } else { |
135 | char kbuf[128]; | ||
136 | |||
135 | if (*ppos || !*lenp) { | 137 | if (*ppos || !*lenp) { |
136 | *lenp = 0; | 138 | *lenp = 0; |
137 | goto done; | 139 | goto done; |
@@ -146,9 +148,20 @@ write_unlock: | |||
146 | } | 148 | } |
147 | rcu_read_unlock(); | 149 | rcu_read_unlock(); |
148 | 150 | ||
149 | len = cpumask_scnprintf(buffer, *lenp, mask); | 151 | len = min(sizeof(kbuf) - 1, *lenp); |
150 | *lenp = len + 1; | 152 | len = cpumask_scnprintf(kbuf, len, mask); |
151 | *ppos += len + 1; | 153 | if (!len) { |
154 | *lenp = 0; | ||
155 | goto done; | ||
156 | } | ||
157 | if (len < *lenp) | ||
158 | kbuf[len++] = '\n'; | ||
159 | if (copy_to_user(buffer, kbuf, len)) { | ||
160 | ret = -EFAULT; | ||
161 | goto done; | ||
162 | } | ||
163 | *lenp = len; | ||
164 | *ppos += len; | ||
152 | } | 165 | } |
153 | 166 | ||
154 | done: | 167 | done: |