On Mon, 2020-06-22 at 09:25 -0700, Brian Vazquez wrote:
> 
> Hi Paolo
> On Mon, Jun 22, 2020 at 3:13 AM Paolo Abeni <pab...@redhat.com> wrote:
> > Hi,
> > 
> > On Fri, 2020-06-19 at 20:14 -0700, Brian Vazquez wrote:
> > > @@ -111,11 +111,13 @@ struct dst_entry *fib6_rule_lookup(struct net *net, 
> > > struct flowi6 *fl6,
> > >       } else {
> > >               struct rt6_info *rt;
> > >  
> > > -             rt = lookup(net, net->ipv6.fib6_local_tbl, fl6, skb, flags);
> > > +             rt = pol_lookup_func(lookup,
> > > +                          net, net->ipv6.fib6_local_tbl, fl6, skb, 
> > > flags);
> > >               if (rt != net->ipv6.ip6_null_entry && rt->dst.error != 
> > > -EAGAIN)
> > >                       return &rt->dst;
> > >               ip6_rt_put_flags(rt, flags);
> > > -             rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, skb, flags);
> > > +             rt = pol_lookup_func(lookup,
> > > +                          net, net->ipv6.fib6_main_tbl, fl6, skb, flags);
> > >               if (rt->dst.error != -EAGAIN)
> > >                       return &rt->dst;
> > >               ip6_rt_put_flags(rt, flags);
> > 
> > Have you considered instead factoring out the slice of
> > fib6_rule_lookup() using indirect calls to an header file? it looks
> > like here (gcc 10.1.1) it sufficent let the compiler use direct calls
> > and will avoid the additional branches.
> > 
> 
> Sorry I couldn't get your point. Could you elaborate more, please? Or provide 
> an example?

I mean: I think we can avoid the indirect calls even without using the
ICW, just moving the relevant code around - in a inline function in the
header files.

e.g. with the following code - rough, build-tested only experiment -
the gcc 10.1.1 is able to use direct calls
for ip6_pol_route_lookup(), ip6_pol_route_output(), etc.

Cheers,

Paolo
---
diff --git a/include/net/ip6_fib.h b/include/net/ip6_fib.h
index 3f615a29766e..c1b5ac890cd2 100644
--- a/include/net/ip6_fib.h
+++ b/include/net/ip6_fib.h
@@ -430,9 +430,6 @@ struct fib6_entry_notifier_info {
 
 struct fib6_table *fib6_get_table(struct net *net, u32 id);
 struct fib6_table *fib6_new_table(struct net *net, u32 id);
-struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
-                                  const struct sk_buff *skb,
-                                  int flags, pol_lookup_t lookup);
 
 /* called with rcu lock held; can return error pointer
  * caller needs to select path
diff --git a/include/net/ip6_route.h b/include/net/ip6_route.h
index 2a5277758379..fe1c2102ffe8 100644
--- a/include/net/ip6_route.h
+++ b/include/net/ip6_route.h
@@ -336,4 +336,50 @@ u32 ip6_mtu_from_fib6(const struct fib6_result *res,
 struct neighbour *ip6_neigh_lookup(const struct in6_addr *gw,
                                   struct net_device *dev, struct sk_buff *skb,
                                   const void *daddr);
+
+#ifdef CONFIG_IPV6_MULTIPLE_TABLES
+struct rt6_info *__fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
+                                   const struct sk_buff *skb,
+                                   int flags, pol_lookup_t lookup);
+static inline struct dst_entry *
+fib6_rule_lookup(struct net *net, struct flowi6 *fl6, const struct sk_buff 
*skb,
+                int flags, pol_lookup_t lookup)
+{
+       struct rt6_info *rt;
+
+       if (!net->ipv6.fib6_has_custom_rules) {
+               rt = lookup(net, net->ipv6.fib6_local_tbl, fl6, skb, flags);
+               if (rt != net->ipv6.ip6_null_entry && rt->dst.error != -EAGAIN)
+                       return &rt->dst;
+               ip6_rt_put_flags(rt, flags);
+               rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, skb, flags);
+               if (rt->dst.error != -EAGAIN)
+                       return &rt->dst;
+               ip6_rt_put_flags(rt, flags);
+       } else {
+               rt = __fib6_rule_lookup(net, fl6, skb, flags, lookup);
+       }
+
+       if (!(flags & RT6_LOOKUP_F_DST_NOREF))
+               dst_hold(&net->ipv6.ip6_null_entry->dst);
+       return &net->ipv6.ip6_null_entry->dst;
+}
+#else
+static inline struct dst_entry *
+fib6_rule_lookup(struct net *net, struct flowi6 *fl6, const struct sk_buff 
*skb,
+                int flags, pol_lookup_t lookup)
+{
+       struct rt6_info *rt;
+
+       rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, skb, flags);
+       if (rt->dst.error == -EAGAIN) {
+               ip6_rt_put_flags(rt, flags);
+               rt = net->ipv6.ip6_null_entry;
+               if (!(flags & RT6_LOOKUP_F_DST_NOREF))
+                       dst_hold(&rt->dst);
+       }
+
+       return &rt->dst;
+}
+#endif
 #endif
diff --git a/net/ipv6/fib6_rules.c b/net/ipv6/fib6_rules.c
index fafe556d21e0..4672cb04c562 100644
--- a/net/ipv6/fib6_rules.c
+++ b/net/ipv6/fib6_rules.c
@@ -87,43 +87,27 @@ int fib6_lookup(struct net *net, int oif, struct flowi6 
*fl6,
        return err;
 }
 
-struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
-                                  const struct sk_buff *skb,
-                                  int flags, pol_lookup_t lookup)
+struct rt6_info *__fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
+                                 const struct sk_buff *skb,
+                                 int flags, pol_lookup_t lookup)
 {
-       if (net->ipv6.fib6_has_custom_rules) {
-               struct fib6_result res = {};
-               struct fib_lookup_arg arg = {
-                       .lookup_ptr = lookup,
-                       .lookup_data = skb,
-                       .result = &res,
-                       .flags = FIB_LOOKUP_NOREF,
-               };
-
-               /* update flow if oif or iif point to device enslaved to l3mdev 
*/
-               l3mdev_update_flow(net, flowi6_to_flowi(fl6));
-
-               fib_rules_lookup(net->ipv6.fib6_rules_ops,
-                                flowi6_to_flowi(fl6), flags, &arg);
-
-               if (res.rt6)
-                       return &res.rt6->dst;
-       } else {
-               struct rt6_info *rt;
-
-               rt = lookup(net, net->ipv6.fib6_local_tbl, fl6, skb, flags);
-               if (rt != net->ipv6.ip6_null_entry && rt->dst.error != -EAGAIN)
-                       return &rt->dst;
-               ip6_rt_put_flags(rt, flags);
-               rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, skb, flags);
-               if (rt->dst.error != -EAGAIN)
-                       return &rt->dst;
-               ip6_rt_put_flags(rt, flags);
-       }
-
-       if (!(flags & RT6_LOOKUP_F_DST_NOREF))
-               dst_hold(&net->ipv6.ip6_null_entry->dst);
-       return &net->ipv6.ip6_null_entry->dst;
+       struct fib6_result res = {};
+       struct fib_lookup_arg arg = {
+               .lookup_ptr = lookup,
+               .lookup_data = skb,
+               .result = &res,
+               .flags = FIB_LOOKUP_NOREF,
+       };
+
+       /* update flow if oif or iif point to device enslaved to l3mdev */
+       l3mdev_update_flow(net, flowi6_to_flowi(fl6));
+
+       fib_rules_lookup(net->ipv6.fib6_rules_ops,
+                        flowi6_to_flowi(fl6), flags, &arg);
+
+       if (res.rt6)
+               return res.rt6;
+       return NULL;
 }
 
 static int fib6_rule_saddr(struct net *net, struct fib_rule *rule, int flags,
diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index 49ee89bbcba0..242d4cdd2666 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -308,23 +308,6 @@ struct fib6_table *fib6_get_table(struct net *net, u32 id)
          return net->ipv6.fib6_main_tbl;
 }
 
-struct dst_entry *fib6_rule_lookup(struct net *net, struct flowi6 *fl6,
-                                  const struct sk_buff *skb,
-                                  int flags, pol_lookup_t lookup)
-{
-       struct rt6_info *rt;
-
-       rt = lookup(net, net->ipv6.fib6_main_tbl, fl6, skb, flags);
-       if (rt->dst.error == -EAGAIN) {
-               ip6_rt_put_flags(rt, flags);
-               rt = net->ipv6.ip6_null_entry;
-               if (!(flags & RT6_LOOKUP_F_DST_NOREF))
-                       dst_hold(&rt->dst);
-       }
-
-       return &rt->dst;
-}
-
 /* called with rcu lock held; no reference taken on fib6_info */
 int fib6_lookup(struct net *net, int oif, struct flowi6 *fl6,
                struct fib6_result *res, int flags)

Reply via email to