From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:58160 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752414AbeBZJKw (ORCPT ); Mon, 26 Feb 2018 04:10:52 -0500 Message-ID: <1519636250.2733.11.camel@redhat.com> Subject: Re: [PATCH net-next 4/5] ipv4: route: dissect flow in input path if fib rules need it From: Paolo Abeni To: Roopa Prabhu , davem@davemloft.net, netdev@vger.kernel.org Cc: dsa@cumulusnetworks.com, nikolay@cumulusnetworks.com, idosch@mellanox.com Date: Mon, 26 Feb 2018 10:10:50 +0100 In-Reply-To: <1519537482-6861-5-git-send-email-roopa@cumulusnetworks.com> References: <1519537482-6861-1-git-send-email-roopa@cumulusnetworks.com> <1519537482-6861-5-git-send-email-roopa@cumulusnetworks.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org List-ID: On Sat, 2018-02-24 at 21:44 -0800, Roopa Prabhu wrote: > From: Roopa Prabhu > > Dissect flow in fwd path if fib rules require it. Controlled by > a flag to avoid penatly for the common case. Flag is set when fib > rules with sport, dport and proto match that require flow dissect > are installed. Also passes the dissected hash keys to the multipath > hash function when applicable to avoid dissecting the flow again. > icmp packets will continue to use inner header for hash > calculations (Thanks to Nikolay Aleksandrov for some review here). > > Signed-off-by: Roopa Prabhu > --- > include/net/ip_fib.h | 2 +- > include/net/netns/ipv4.h | 1 + > net/ipv4/fib_rules.c | 6 ++++++ > net/ipv4/fib_semantics.c | 2 +- > net/ipv4/route.c | 52 +++++++++++++++++++++++++++++++++++------------- > 5 files changed, 47 insertions(+), 16 deletions(-) > > diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h > index f805243..5ada772 100644 > --- a/include/net/ip_fib.h > +++ b/include/net/ip_fib.h > @@ -371,7 +371,7 @@ int fib_sync_up(struct net_device *dev, unsigned int nh_flags); > > #ifdef CONFIG_IP_ROUTE_MULTIPATH > int fib_multipath_hash(const struct fib_info *fi, const struct flowi4 *fl4, > - const struct sk_buff *skb); > + const struct sk_buff *skb, struct flow_keys *flkeys); > #endif > void fib_select_multipath(struct fib_result *res, int hash); > void fib_select_path(struct net *net, struct fib_result *res, > diff --git a/include/net/netns/ipv4.h b/include/net/netns/ipv4.h > index 44668c2..87b8fdc 100644 > --- a/include/net/netns/ipv4.h > +++ b/include/net/netns/ipv4.h > @@ -52,6 +52,7 @@ struct netns_ipv4 { > #ifdef CONFIG_IP_MULTIPLE_TABLES > struct fib_rules_ops *rules_ops; > bool fib_has_custom_rules; > + bool fib_rules_require_fldissect; > struct fib_table __rcu *fib_main; > struct fib_table __rcu *fib_default; > #endif > diff --git a/net/ipv4/fib_rules.c b/net/ipv4/fib_rules.c > index 9d55c90..83aa786 100644 > --- a/net/ipv4/fib_rules.c > +++ b/net/ipv4/fib_rules.c > @@ -253,6 +253,11 @@ static int fib4_rule_configure(struct fib_rule *rule, struct sk_buff *skb, > } > #endif > > + if (rule->ip_proto || > + fib_rule_port_range_valid(&rule->sport_range) || > + fib_rule_port_range_valid(&rule->dport_range)) > + net->ipv4.fib_rules_require_fldissect = true; > + > rule4->src_len = frh->src_len; > rule4->srcmask = inet_make_mask(rule4->src_len); > rule4->dst_len = frh->dst_len; What about using 'fib_rules_require_fldissect' to conditionally avoid all the tests introduced by patch 2/5 ? Perhaps even using a static key for that? It would be great if the kernel would be able to clear this flag when no more needed. I know it's not the current behaviour for other similar flags, but I hope we can improve ;) Both points above also apply to the ipv6 code path. Thanks, Paolo