Message ID | 1550807549-22720-1-git-send-email-wenxu@ucloud.cn |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | [net-next] route: Add a new fib_multipath_hash_policy base on cpu id for tunnel packet | expand |
Hello! On 22.02.2019 6:52, wenxu@ucloud.cn wrote: > From: wenxu <wenxu@ucloud.cn> > > Current fib_multipath_hash_policy can make hash based on the L3 or > L4. But it only work on the outer IP. So a specific tunnel always > has the same hash value. But a specific tunnel may contain so many > inner connection. However there is no good ways for tunnel packet. Connections? Way? > A specific tunnel route based on the percpu dst_cache, It will not > lookup route table each packet. For each packet? > This patch provide a based cpu id hash policy. The different > connection run on differnt cpu and There will differnet hash Different. Will be? > value for percpu dst_cache. > > Signed-off-by: wenxu <wenxu@ucloud.cn> > --- > net/ipv4/route.c | 6 ++++++ > net/ipv4/sysctl_net_ipv4.c | 2 +- > 2 files changed, 7 insertions(+), 1 deletion(-) > > diff --git a/net/ipv4/route.c b/net/ipv4/route.c > index ecc12a7..6cf2fd4 100644 > --- a/net/ipv4/route.c > +++ b/net/ipv4/route.c [...] > @@ -1834,6 +1835,8 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4, > hash_keys.addrs.v4addrs.dst = fl4->daddr; > } > break; > + case 2: > + cpu = smp_processor_id() + 1; Should be a comment /* fall through */ if this fall thru isn't in error. > case 1: > /* skb is currently provided only when forwarding */ > if (skb) { [...] MBR, Sergei
On 2/21/19 10:52 PM, wenxu@ucloud.cn wrote: > From: wenxu <wenxu@ucloud.cn> > > Current fib_multipath_hash_policy can make hash based on the L3 or > L4. But it only work on the outer IP. So a specific tunnel always > has the same hash value. But a specific tunnel may contain so many > inner connection. However there is no good ways for tunnel packet. > A specific tunnel route based on the percpu dst_cache, It will not > lookup route table each packet. > > This patch provide a based cpu id hash policy. The different > connection run on differnt cpu and There will differnet hash > value for percpu dst_cache. > > Signed-off-by: wenxu <wenxu@ucloud.cn> > --- > net/ipv4/route.c | 6 ++++++ > net/ipv4/sysctl_net_ipv4.c | 2 +- > 2 files changed, 7 insertions(+), 1 deletion(-) > This multipath hash policy is global - for all fib lookups, not just tunnels. The suggestion by Nik is worth exploring - an option to add the mark to the hash (e.g., L3 header + mark) which makes this more generic. If the policy options are changed, the call to call_netevent_notifiers needs to be updated to handle a failure. For example, the mlxsw handler needs to be able to veto an option it does not support.
On Fri, Feb 22, 2019 at 10:19:41PM -0500, David Ahern wrote: > On 2/21/19 10:52 PM, wenxu@ucloud.cn wrote: > > From: wenxu <wenxu@ucloud.cn> > > > > Current fib_multipath_hash_policy can make hash based on the L3 or > > L4. But it only work on the outer IP. So a specific tunnel always > > has the same hash value. But a specific tunnel may contain so many > > inner connection. However there is no good ways for tunnel packet. > > A specific tunnel route based on the percpu dst_cache, It will not > > lookup route table each packet. > > > > This patch provide a based cpu id hash policy. The different > > connection run on differnt cpu and There will differnet hash > > value for percpu dst_cache. > > > > Signed-off-by: wenxu <wenxu@ucloud.cn> > > --- > > net/ipv4/route.c | 6 ++++++ > > net/ipv4/sysctl_net_ipv4.c | 2 +- > > 2 files changed, 7 insertions(+), 1 deletion(-) > > > > This multipath hash policy is global - for all fib lookups, not just > tunnels. > > The suggestion by Nik is worth exploring - an option to add the mark to > the hash (e.g., L3 header + mark) which makes this more generic. > > If the policy options are changed, the call to call_netevent_notifiers > needs to be updated to handle a failure. For example, the mlxsw handler > needs to be able to veto an option it does not support. Did the author consider using a UDP-based tunnel like FOU? Then L4 mode should work just fine. I believe most hardware routers do not take inner headers into account either. https://lwn.net/Articles/614348/ https://people.netfilter.org/pablo/netdev0.1/papers/UDP-Encapsulation-in-Linux.pdf
diff --git a/net/ipv4/route.c b/net/ipv4/route.c index ecc12a7..6cf2fd4 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -1821,6 +1821,7 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4, const struct sk_buff *skb, struct flow_keys *flkeys) { struct flow_keys hash_keys; + u32 cpu = 0; u32 mhash; switch (net->ipv4.sysctl_fib_multipath_hash_policy) { @@ -1834,6 +1835,8 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4, hash_keys.addrs.v4addrs.dst = fl4->daddr; } break; + case 2: + cpu = smp_processor_id() + 1; case 1: /* skb is currently provided only when forwarding */ if (skb) { @@ -1870,6 +1873,9 @@ int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4, } mhash = flow_hash_from_keys(&hash_keys); + if (cpu) + mhash = jhash_2words(mhash, cpu, 0); + return mhash >> 1; } #endif /* CONFIG_IP_ROUTE_MULTIPATH */ diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c index ba0fc4b..708bbbb 100644 --- a/net/ipv4/sysctl_net_ipv4.c +++ b/net/ipv4/sysctl_net_ipv4.c @@ -951,7 +951,7 @@ static int proc_fib_multipath_hash_policy(struct ctl_table *table, int write, .mode = 0644, .proc_handler = proc_fib_multipath_hash_policy, .extra1 = &zero, - .extra2 = &one, + .extra2 = &two, }, #endif {