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>

diff --git a/net/openvswitch/flow.c b/net/openvswitch/flow.c
index 6ef51e7643677cc9cd62c9edc0424e9c35228024..8c94cef25a72b685456b2ab3e7155b17f944abcf 100644 (file)
--- 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,
                           const struct sk_buff *skb)
 {
        struct flow_stats *stats;
-       int cpu = smp_processor_id();
+       unsigned int cpu = smp_processor_id();
        int len = skb->len + (skb_vlan_tag_present(skb) ? VLAN_HLEN : 0);
 
        stats = rcu_dereference(flow->stats[cpu]);
@@ -117,6 +117,7 @@ void ovs_flow_stats_update(struct sw_flow *flow, __be16 tcp_flags,
 
                                        rcu_assign_pointer(flow->stats[cpu],
                                                           new_stats);
+                                       cpumask_set_cpu(cpu, &flow->cpu_used_mask);
                                        goto unlock;
                                }
                        }
@@ -144,7 +145,7 @@ void ovs_flow_stats_get(const struct sw_flow *flow,
        memset(ovs_stats, 0, sizeof(*ovs_stats));
 
        /* We open code this to make sure cpu 0 is always considered */
-       for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, cpu_possible_mask)) {
+       for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, &flow->cpu_used_mask)) {
                struct flow_stats *stats = rcu_dereference_ovsl(flow->stats[cpu]);
 
                if (stats) {
@@ -168,7 +169,7 @@ void ovs_flow_stats_clear(struct sw_flow *flow)
        int cpu;
 
        /* We open code this to make sure cpu 0 is always considered */
-       for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, cpu_possible_mask)) {
+       for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, &flow->cpu_used_mask)) {
                struct flow_stats *stats = ovsl_dereference(flow->stats[cpu]);
 
                if (stats) {

diff --git a/net/openvswitch/flow.h b/net/openvswitch/flow.h
index a9bc1c875965c1814cd816446598e9602e8cc13a..1875bba4f865631f4e3122b4c53a734ced6772e7 100644 (file)
--- a/net/openvswitch/flow.h
+++ b/net/openvswitch/flow.h
@@ -31,6 +31,7 @@
 #include <linux/jiffies.h>
 #include <linux/time.h>
 #include <linux/flex_array.h>
+#include <linux/cpumask.h>
 #include <net/inet_ecn.h>
 #include <net/ip_tunnels.h>
 #include <net/dst_metadata.h>
@@ -219,6 +220,7 @@ struct sw_flow {
                                         */
        struct sw_flow_key key;
        struct sw_flow_id id;
+       struct cpumask cpu_used_mask;
        struct sw_flow_mask *mask;
        struct sw_flow_actions __rcu *sf_acts;
        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 ea7a8073fa02856469bf1f0c12ce9e5ac0fbbf5a..80ea2a71852e57b26936f3563527c066c4bc3312 100644 (file)
--- a/net/openvswitch/flow_table.c
+++ b/net/openvswitch/flow_table.c
@@ -98,6 +98,8 @@ struct sw_flow *ovs_flow_alloc(void)
 
        RCU_INIT_POINTER(flow->stats[0], stats);
 
+       cpumask_set_cpu(0, &flow->cpu_used_mask);
+
        return flow;
 err:
        kmem_cache_free(flow_cache, flow);
@@ -141,7 +143,7 @@ static void flow_free(struct sw_flow *flow)
        if (flow->sf_acts)
                ovs_nla_free_flow_actions((struct sw_flow_actions __force *)flow->sf_acts);
        /* We open code this to make sure cpu 0 is always considered */
-       for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, cpu_possible_mask))
+       for (cpu = 0; cpu < nr_cpu_ids; cpu = cpumask_next(cpu, &flow->cpu_used_mask))
                if (flow->stats[cpu])
                        kmem_cache_free(flow_stats_cache,
                                        (struct flow_stats __force *)flow->stats[cpu]);