From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx3-rdu2.redhat.com ([66.187.233.73]:49560 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1752291AbeB1KFh (ORCPT ); Wed, 28 Feb 2018 05:05:37 -0500 Message-ID: <1519812335.2595.17.camel@redhat.com> Subject: Re: [PATCH net-next v2 5/5] ipv6: 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: Wed, 28 Feb 2018 11:05:35 +0100 In-Reply-To: <1519789965-3465-6-git-send-email-roopa@cumulusnetworks.com> References: <1519789965-3465-1-git-send-email-roopa@cumulusnetworks.com> <1519789965-3465-6-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 Tue, 2018-02-27 at 19:52 -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. > > Signed-off-by: Roopa Prabhu > --- > include/net/ip6_fib.h | 25 +++++++++++++++++++++++++ > include/net/ip6_route.h | 4 +++- > include/net/netns/ipv6.h | 3 ++- > net/ipv6/fib6_rules.c | 16 ++++++++++++++++ > net/ipv6/icmp.c | 2 +- > net/ipv6/route.c | 34 +++++++++++++++++++++++++--------- > 6 files changed, 72 insertions(+), 12 deletions(-) > > diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h > index 34ec321d..8d906a3 100644 > --- a/include/net/ip6_fib.h > +++ b/include/net/ip6_fib.h > @@ -415,6 +415,24 @@ void fib6_rules_cleanup(void); > bool fib6_rule_default(const struct fib_rule *rule); > int fib6_rules_dump(struct net *net, struct notifier_block *nb); > unsigned int fib6_rules_seq_read(struct net *net); > + > +static inline bool fib6_rules_early_flow_dissect(struct net *net, > + struct sk_buff *skb, > + struct flowi6 *fl6, > + struct flow_keys *flkeys) > +{ > + unsigned int flag = FLOW_DISSECTOR_F_STOP_AT_ENCAP; > + > + if (!net->ipv6.fib6_rules_require_fldissect) > + return false; > + > + skb_flow_dissect_flow_keys(skb, flkeys, flag); > + fl6->fl6_sport = flkeys->ports.src; > + fl6->fl6_dport = flkeys->ports.dst; > + fl6->flowi6_proto = flkeys->basic.ip_proto; > + > + return true; > +} > #else > static inline int fib6_rules_init(void) > { > @@ -436,5 +454,12 @@ static inline unsigned int fib6_rules_seq_read(struct net *net) > { > return 0; > } > +static inline bool fib6_rules_early_flow_dissect(struct net *net, > + struct sk_buff *skb, > + struct flowi6 *fl6, > + struct flow_keys *flkeys) > +{ > + return false; > +} > #endif > #endif > diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h > index 27d23a6..da2bde5 100644 > --- a/include/net/ip6_route.h > +++ b/include/net/ip6_route.h > @@ -127,7 +127,8 @@ static inline int ip6_route_get_saddr(struct net *net, struct rt6_info *rt, > > struct rt6_info *rt6_lookup(struct net *net, const struct in6_addr *daddr, > const struct in6_addr *saddr, int oif, int flags); > -u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb); > +u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb, > + struct flow_keys *hkeys); > > struct dst_entry *icmp6_dst_alloc(struct net_device *dev, struct flowi6 *fl6); > > @@ -266,4 +267,5 @@ static inline bool rt6_duplicate_nexthop(struct rt6_info *a, struct rt6_info *b) > ipv6_addr_equal(&a->rt6i_gateway, &b->rt6i_gateway) && > !lwtunnel_cmp_encap(a->dst.lwtstate, b->dst.lwtstate); > } > + > #endif > diff --git a/include/net/netns/ipv6.h b/include/net/netns/ipv6.h > index 987cc45..2b91942 100644 > --- a/include/net/netns/ipv6.h > +++ b/include/net/netns/ipv6.h > @@ -71,7 +71,8 @@ struct netns_ipv6 { > unsigned int ip6_rt_gc_expire; > unsigned long ip6_rt_last_gc; > #ifdef CONFIG_IPV6_MULTIPLE_TABLES > - bool fib6_has_custom_rules; > + unsigned int fib6_rules_require_fldissect; > + bool fib6_has_custom_rules; > struct rt6_info *ip6_prohibit_entry; > struct rt6_info *ip6_blk_hole_entry; > struct fib6_table *fib6_local_tbl; > diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c > index bcd1f22..04e5f52 100644 > --- a/net/ipv6/fib6_rules.c > +++ b/net/ipv6/fib6_rules.c > @@ -269,12 +269,26 @@ static int fib6_rule_configure(struct fib_rule *rule, struct sk_buff *skb, > rule6->dst.plen = frh->dst_len; > rule6->tclass = frh->tos; > > + if (fib_rule_requires_fldissect(rule)) > + net->ipv6.fib6_rules_require_fldissect++; > + > net->ipv6.fib6_has_custom_rules = true; > err = 0; > errout: > return err; > } > > +static int fib6_rule_delete(struct fib_rule *rule) > +{ > + struct net *net = rule->fr_net; > + > + if (net->ipv6.fib6_rules_require_fldissect && > + fib_rule_requires_fldissect(rule)) > + net->ipv6.fib6_rules_require_fldissect--; > + > + return 0; > +} > + > static int fib6_rule_compare(struct fib_rule *rule, struct fib_rule_hdr *frh, > struct nlattr **tb) > { > @@ -334,6 +348,7 @@ static const struct fib_rules_ops __net_initconst fib6_rules_ops_template = { > .match = fib6_rule_match, > .suppress = fib6_rule_suppress, > .configure = fib6_rule_configure, > + .delete = fib6_rule_delete, > .compare = fib6_rule_compare, > .fill = fib6_rule_fill, > .nlmsg_payload = fib6_rule_nlmsg_payload, > @@ -361,6 +376,7 @@ static int __net_init fib6_rules_net_init(struct net *net) > goto out_fib6_rules_ops; > > net->ipv6.fib6_rules_ops = ops; > + net->ipv6.fib6_rules_require_fldissect = 0; > out: > return err; > > diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c > index 4fa4f1b..b0778d3 100644 > --- a/net/ipv6/icmp.c > +++ b/net/ipv6/icmp.c > @@ -522,7 +522,7 @@ static void icmp6_send(struct sk_buff *skb, u8 type, u8 code, __u32 info, > fl6.fl6_icmp_type = type; > fl6.fl6_icmp_code = code; > fl6.flowi6_uid = sock_net_uid(net, NULL); > - fl6.mp_hash = rt6_multipath_hash(&fl6, skb); > + fl6.mp_hash = rt6_multipath_hash(&fl6, skb, NULL); > security_skb_classify_flow(skb, flowi6_to_flowi(&fl6)); > > sk = icmpv6_xmit_lock(net); > diff --git a/net/ipv6/route.c b/net/ipv6/route.c > index aa709b6..e2bb408 100644 > --- a/net/ipv6/route.c > +++ b/net/ipv6/route.c > @@ -460,7 +460,7 @@ static struct rt6_info *rt6_multipath_select(struct rt6_info *match, > * case it will always be non-zero. Otherwise now is the time to do it. > */ > if (!fl6->mp_hash) > - fl6->mp_hash = rt6_multipath_hash(fl6, NULL); > + fl6->mp_hash = rt6_multipath_hash(fl6, NULL, NULL); > > if (fl6->mp_hash <= atomic_read(&match->rt6i_nh_upper_bound)) > return match; > @@ -1786,10 +1786,12 @@ struct dst_entry *ip6_route_input_lookup(struct net *net, > EXPORT_SYMBOL_GPL(ip6_route_input_lookup); > > static void ip6_multipath_l3_keys(const struct sk_buff *skb, > - struct flow_keys *keys) > + struct flow_keys *keys, > + struct flow_keys *flkeys) > { > const struct ipv6hdr *outer_iph = ipv6_hdr(skb); > const struct ipv6hdr *key_iph = outer_iph; > + struct flow_keys *_flkeys = flkeys; > const struct ipv6hdr *inner_iph; > const struct icmp6hdr *icmph; > struct ipv6hdr _inner_iph; > @@ -1811,22 +1813,31 @@ static void ip6_multipath_l3_keys(const struct sk_buff *skb, > goto out; > > key_iph = inner_iph; > + _flkeys = NULL; > out: > memset(keys, 0, sizeof(*keys)); > keys->control.addr_type = FLOW_DISSECTOR_KEY_IPV6_ADDRS; > - keys->addrs.v6addrs.src = key_iph->saddr; > - keys->addrs.v6addrs.dst = key_iph->daddr; > - keys->tags.flow_label = ip6_flowinfo(key_iph); > - keys->basic.ip_proto = key_iph->nexthdr; > + if (_flkeys) { > + keys->addrs.v6addrs.src = _flkeys->addrs.v6addrs.src; > + keys->addrs.v6addrs.dst = _flkeys->addrs.v6addrs.dst; > + keys->tags.flow_label = _flkeys->tags.flow_label; > + keys->basic.ip_proto = _flkeys->basic.ip_proto; > + } else { > + keys->addrs.v6addrs.src = key_iph->saddr; > + keys->addrs.v6addrs.dst = key_iph->daddr; > + keys->tags.flow_label = ip6_flowinfo(key_iph); > + keys->basic.ip_proto = key_iph->nexthdr; > + } > } > > /* if skb is set it will be used and fl6 can be NULL */ > -u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb) > +u32 rt6_multipath_hash(const struct flowi6 *fl6, const struct sk_buff *skb, > + struct flow_keys *flkeys) > { > struct flow_keys hash_keys; > > if (skb) { > - ip6_multipath_l3_keys(skb, &hash_keys); > + ip6_multipath_l3_keys(skb, &hash_keys, flkeys); > return flow_hash_from_keys(&hash_keys) >> 1; > } > > @@ -1847,12 +1858,17 @@ void ip6_route_input(struct sk_buff *skb) > .flowi6_mark = skb->mark, > .flowi6_proto = iph->nexthdr, > }; > + struct flow_keys *flkeys = NULL, _flkeys; > > tun_info = skb_tunnel_info(skb); > if (tun_info && !(tun_info->mode & IP_TUNNEL_INFO_TX)) > fl6.flowi6_tun_key.tun_id = tun_info->key.tun_id; > + > + if (fib6_rules_early_flow_dissect(net, skb, &fl6, &_flkeys)) > + flkeys = &_flkeys; > + > if (unlikely(fl6.flowi6_proto == IPPROTO_ICMPV6)) > - fl6.mp_hash = rt6_multipath_hash(&fl6, skb); > + fl6.mp_hash = rt6_multipath_hash(&fl6, skb, flkeys); > skb_dst_drop(skb); > skb_dst_set(skb, ip6_route_input_lookup(net, skb->dev, &fl6, flags)); > } LGTM Acked-by: Paolo Abeni