diff options
| author | Tonghao Zhang <xiangxia.m.yue@gmail.com> | 2017-07-18 02:28:06 -0400 |
|---|---|---|
| committer | David S. Miller <davem@davemloft.net> | 2017-07-19 16:49:39 -0400 |
| commit | c4b2bf6b4a35348fe6d1eb06928eb68d7b9d99a9 (patch) | |
| tree | a4fee7f327cb884723fcb54f03e2170c7b4ca995 /net/openvswitch | |
| parent | c57c054eb5b1ccf230c49f736f7a018fcbc3e952 (diff) | |
openvswitch: Optimize operations for OvS flow_stats.
When calling the flow_free() to free the flow, we call many times
(cpu_possible_mask, eg. 128 as default) cpumask_next(). That will
take up our CPU usage if we call the flow_free() frequently.
When we put all packets to userspace via upcall, and OvS will send
them back via netlink to ovs_packet_cmd_execute(will call flow_free).
The test topo is shown as below. VM01 sends TCP packets to VM02,
and OvS forward packtets. When testing, we use perf to report the
system performance.
VM01 --- OvS-VM --- VM02
Without this patch, perf-top show as below: The flow_free() is
3.02% CPU usage.
4.23% [kernel] [k] _raw_spin_unlock_irqrestore
3.62% [kernel] [k] __do_softirq
3.16% [kernel] [k] __memcpy
3.02% [kernel] [k] flow_free
2.42% libc-2.17.so [.] __memcpy_ssse3_back
2.18% [kernel] [k] copy_user_generic_unrolled
2.17% [kernel] [k] find_next_bit
When applied this patch, perf-top show as below: Not shown on
the list anymore.
4.11% [kernel] [k] _raw_spin_unlock_irqrestore
3.79% [kernel] [k] __do_softirq
3.46% [kernel] [k] __memcpy
2.73% libc-2.17.so [.] __memcpy_ssse3_back
2.25% [kernel] [k] copy_user_generic_unrolled
1.89% libc-2.17.so [.] _int_malloc
1.53% ovs-vswitchd [.] xlate_actions
With this patch, the TCP throughput(we dont use Megaflow Cache
+ Microflow Cache) between VMs is 1.18Gbs/sec up to 1.30Gbs/sec
(maybe ~10% performance imporve).
This patch adds cpumask struct, the cpu_used_mask stores the cpu_id
that the flow used. And we only check the flow_stats on the cpu we
used, and it is unncessary to check all possible cpu when getting,
cleaning, and updating the flow_stats. Adding the cpu_used_mask to
sw_flow struct does’t increase the cacheline number.
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Acked-by: Pravin B Shelar <pshelar@ovn.org>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/openvswitch')
| -rw-r--r-- | net/openvswitch/flow.c | 7 | ||||
| -rw-r--r-- | net/openvswitch/flow.h | 2 | ||||
| -rw-r--r-- | net/openvswitch/flow_table.c | 4 |
3 files changed, 9 insertions, 4 deletions
diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c index 6ef51e764367..8c94cef25a72 100644 --- a/net/openvswitch/flow.c +++ b/net/openvswitch/flow.c | |||
| @@ -72,7 +72,7 @@ void ovs_flow_stats_update(struct sw_flow *flow, __be16 tcp_flags, | |||
| 72 | const struct sk_buff *skb) | 72 | const struct sk_buff *skb) |
| 73 | { | 73 | { |
| 74 | struct flow_stats *stats; | 74 | struct flow_stats *stats; |
| 75 | int cpu = smp_processor_id(); | 75 | unsigned int cpu = smp_processor_id(); |
| 76 | int len = skb->len + (skb_vlan_tag_present(skb) ? VLAN_HLEN : 0); | 76 | int len = skb->len + (skb_vlan_tag_present(skb) ? VLAN_HLEN : 0); |
| 77 | 77 | ||
| 78 | stats = rcu_dereference(flow->stats[cpu]); | 78 | stats = rcu_dereference(flow->stats[cpu]); |
| @@ -117,6 +117,7 @@ void ovs_flow_stats_update(struct sw_flow *flow, __be16 tcp_flags, | |||
| 117 | 117 | ||
| 118 | rcu_assign_pointer(flow->stats[cpu], | 118 | rcu_assign_pointer(flow->stats[cpu], |
| 119 | new_stats); | 119 | new_stats); |
| 120 | cpumask_set_cpu(cpu, &flow->cpu_used_mask); | ||
| 120 | goto unlock; | 121 | goto unlock; |
| 121 | } | 122 | } |
| 122 | } | 123 | } |
| @@ -144,7 +145,7 @@ void ovs_flow_stats_get(const struct sw_flow *flow, | |||
| 144 | memset(ovs_stats, 0, sizeof(*ovs_stats)); | 145 | memset(ovs_stats, 0, sizeof(*ovs_stats)); |
| 145 | 146 | ||
| 146 | /* We open code this to make sure cpu 0 is always considered */ | 147 | /* We open code this to make sure cpu 0 is always considered */ |
| 147 | for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, cpu_possible_mask)) { | 148 | for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, &flow->cpu_used_mask)) { |
| 148 | struct flow_stats *stats = rcu_dereference_ovsl(flow->stats[cpu]); | 149 | struct flow_stats *stats = rcu_dereference_ovsl(flow->stats[cpu]); |
| 149 | 150 | ||
| 150 | if (stats) { | 151 | if (stats) { |
| @@ -168,7 +169,7 @@ void ovs_flow_stats_clear(struct sw_flow *flow) | |||
| 168 | int cpu; | 169 | int cpu; |
| 169 | 170 | ||
| 170 | /* We open code this to make sure cpu 0 is always considered */ | 171 | /* We open code this to make sure cpu 0 is always considered */ |
| 171 | for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, cpu_possible_mask)) { | 172 | for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, &flow->cpu_used_mask)) { |
| 172 | struct flow_stats *stats = ovsl_dereference(flow->stats[cpu]); | 173 | struct flow_stats *stats = ovsl_dereference(flow->stats[cpu]); |
| 173 | 174 | ||
| 174 | if (stats) { | 175 | if (stats) { |
diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h index a9bc1c875965..1875bba4f865 100644 --- a/net/openvswitch/flow.h +++ b/net/openvswitch/flow.h | |||
| @@ -31,6 +31,7 @@ | |||
| 31 | #include <linux/jiffies.h> | 31 | #include <linux/jiffies.h> |
| 32 | #include <linux/time.h> | 32 | #include <linux/time.h> |
| 33 | #include <linux/flex_array.h> | 33 | #include <linux/flex_array.h> |
| 34 | #include <linux/cpumask.h> | ||
| 34 | #include <net/inet_ecn.h> | 35 | #include <net/inet_ecn.h> |
| 35 | #include <net/ip_tunnels.h> | 36 | #include <net/ip_tunnels.h> |
| 36 | #include <net/dst_metadata.h> | 37 | #include <net/dst_metadata.h> |
| @@ -219,6 +220,7 @@ struct sw_flow { | |||
| 219 | */ | 220 | */ |
| 220 | struct sw_flow_key key; | 221 | struct sw_flow_key key; |
| 221 | struct sw_flow_id id; | 222 | struct sw_flow_id id; |
| 223 | struct cpumask cpu_used_mask; | ||
| 222 | struct sw_flow_mask *mask; | 224 | struct sw_flow_mask *mask; |
| 223 | struct sw_flow_actions __rcu *sf_acts; | 225 | struct sw_flow_actions __rcu *sf_acts; |
| 224 | struct flow_stats __rcu *stats[]; /* One for each CPU. First one | 226 | struct flow_stats __rcu *stats[]; /* One for each CPU. First one |
diff --git a/net/openvswitch/flow_table.c b/net/openvswitch/flow_table.c index ea7a8073fa02..80ea2a71852e 100644 --- a/net/openvswitch/flow_table.c +++ b/net/openvswitch/flow_table.c | |||
| @@ -98,6 +98,8 @@ struct sw_flow *ovs_flow_alloc(void) | |||
| 98 | 98 | ||
| 99 | RCU_INIT_POINTER(flow->stats[0], stats); | 99 | RCU_INIT_POINTER(flow->stats[0], stats); |
| 100 | 100 | ||
| 101 | cpumask_set_cpu(0, &flow->cpu_used_mask); | ||
| 102 | |||
| 101 | return flow; | 103 | return flow; |
| 102 | err: | 104 | err: |
| 103 | kmem_cache_free(flow_cache, flow); | 105 | kmem_cache_free(flow_cache, flow); |
| @@ -141,7 +143,7 @@ static void flow_free(struct sw_flow *flow) | |||
| 141 | if (flow->sf_acts) | 143 | if (flow->sf_acts) |
| 142 | ovs_nla_free_flow_actions((struct sw_flow_actions __force *)flow->sf_acts); | 144 | ovs_nla_free_flow_actions((struct sw_flow_actions __force *)flow->sf_acts); |
| 143 | /* We open code this to make sure cpu 0 is always considered */ | 145 | /* We open code this to make sure cpu 0 is always considered */ |
| 144 | for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, cpu_possible_mask)) | 146 | for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, &flow->cpu_used_mask)) |
| 145 | if (flow->stats[cpu]) | 147 | if (flow->stats[cpu]) |
| 146 | kmem_cache_free(flow_stats_cache, | 148 | kmem_cache_free(flow_stats_cache, |
| 147 | (struct flow_stats __force *)flow->stats[cpu]); | 149 | (struct flow_stats __force *)flow->stats[cpu]); |
