123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375376377378379380381382383384385386387388389390391392393394395396397398399400401402403 |
- From: Alexander Duyck <alexander.h.duyck@redhat.com>
- Date: Wed, 31 Dec 2014 10:56:24 -0800
- Subject: [PATCH] fib_trie: Push rcu_read_lock/unlock to callers
- This change is to start cleaning up some of the rcu_read_lock/unlock
- handling. I realized while reviewing the code there are several spots that
- I don't believe are being handled correctly or are masking warnings by
- locally calling rcu_read_lock/unlock instead of calling them at the correct
- level.
- A common example is a call to fib_get_table followed by fib_table_lookup.
- The rcu_read_lock/unlock ought to wrap both but there are several spots where
- they were not wrapped.
- Signed-off-by: Alexander Duyck <alexander.h.duyck@redhat.com>
- Signed-off-by: David S. Miller <davem@davemloft.net>
- ---
- --- a/include/net/ip_fib.h
- +++ b/include/net/ip_fib.h
- @@ -222,16 +222,19 @@ static inline struct fib_table *fib_new_
- static inline int fib_lookup(struct net *net, const struct flowi4 *flp,
- struct fib_result *res)
- {
- - struct fib_table *table;
- + int err = -ENETUNREACH;
-
- - table = fib_get_table(net, RT_TABLE_LOCAL);
- - if (!fib_table_lookup(table, flp, res, FIB_LOOKUP_NOREF))
- - return 0;
- -
- - table = fib_get_table(net, RT_TABLE_MAIN);
- - if (!fib_table_lookup(table, flp, res, FIB_LOOKUP_NOREF))
- - return 0;
- - return -ENETUNREACH;
- + rcu_read_lock();
- +
- + if (!fib_table_lookup(fib_get_table(net, RT_TABLE_LOCAL), flp, res,
- + FIB_LOOKUP_NOREF) ||
- + !fib_table_lookup(fib_get_table(net, RT_TABLE_MAIN), flp, res,
- + FIB_LOOKUP_NOREF))
- + err = 0;
- +
- + rcu_read_unlock();
- +
- + return err;
- }
-
- #else /* CONFIG_IP_MULTIPLE_TABLES */
- @@ -247,20 +250,25 @@ static inline int fib_lookup(struct net
- struct fib_result *res)
- {
- if (!net->ipv4.fib_has_custom_rules) {
- + int err = -ENETUNREACH;
- +
- + rcu_read_lock();
- +
- res->tclassid = 0;
- - if (net->ipv4.fib_local &&
- - !fib_table_lookup(net->ipv4.fib_local, flp, res,
- - FIB_LOOKUP_NOREF))
- - return 0;
- - if (net->ipv4.fib_main &&
- - !fib_table_lookup(net->ipv4.fib_main, flp, res,
- - FIB_LOOKUP_NOREF))
- - return 0;
- - if (net->ipv4.fib_default &&
- - !fib_table_lookup(net->ipv4.fib_default, flp, res,
- - FIB_LOOKUP_NOREF))
- - return 0;
- - return -ENETUNREACH;
- + if ((net->ipv4.fib_local &&
- + !fib_table_lookup(net->ipv4.fib_local, flp, res,
- + FIB_LOOKUP_NOREF)) ||
- + (net->ipv4.fib_main &&
- + !fib_table_lookup(net->ipv4.fib_main, flp, res,
- + FIB_LOOKUP_NOREF)) ||
- + (net->ipv4.fib_default &&
- + !fib_table_lookup(net->ipv4.fib_default, flp, res,
- + FIB_LOOKUP_NOREF)))
- + err = 0;
- +
- + rcu_read_unlock();
- +
- + return err;
- }
- return __fib_lookup(net, flp, res);
- }
- --- a/net/ipv4/fib_frontend.c
- +++ b/net/ipv4/fib_frontend.c
- @@ -109,6 +109,7 @@ struct fib_table *fib_new_table(struct n
- return tb;
- }
-
- +/* caller must hold either rtnl or rcu read lock */
- struct fib_table *fib_get_table(struct net *net, u32 id)
- {
- struct fib_table *tb;
- @@ -119,15 +120,11 @@ struct fib_table *fib_get_table(struct n
- id = RT_TABLE_MAIN;
- h = id & (FIB_TABLE_HASHSZ - 1);
-
- - rcu_read_lock();
- head = &net->ipv4.fib_table_hash[h];
- hlist_for_each_entry_rcu(tb, head, tb_hlist) {
- - if (tb->tb_id == id) {
- - rcu_read_unlock();
- + if (tb->tb_id == id)
- return tb;
- - }
- }
- - rcu_read_unlock();
- return NULL;
- }
- #endif /* CONFIG_IP_MULTIPLE_TABLES */
- @@ -167,16 +164,18 @@ static inline unsigned int __inet_dev_ad
- if (ipv4_is_multicast(addr))
- return RTN_MULTICAST;
-
- + rcu_read_lock();
- +
- local_table = fib_get_table(net, RT_TABLE_LOCAL);
- if (local_table) {
- ret = RTN_UNICAST;
- - rcu_read_lock();
- if (!fib_table_lookup(local_table, &fl4, &res, FIB_LOOKUP_NOREF)) {
- if (!dev || dev == res.fi->fib_dev)
- ret = res.type;
- }
- - rcu_read_unlock();
- }
- +
- + rcu_read_unlock();
- return ret;
- }
-
- @@ -923,7 +922,7 @@ no_promotions:
- #undef BRD1_OK
- }
-
- -static void nl_fib_lookup(struct fib_result_nl *frn, struct fib_table *tb)
- +static void nl_fib_lookup(struct net *net, struct fib_result_nl *frn)
- {
-
- struct fib_result res;
- @@ -933,6 +932,11 @@ static void nl_fib_lookup(struct fib_res
- .flowi4_tos = frn->fl_tos,
- .flowi4_scope = frn->fl_scope,
- };
- + struct fib_table *tb;
- +
- + rcu_read_lock();
- +
- + tb = fib_get_table(net, frn->tb_id_in);
-
- frn->err = -ENOENT;
- if (tb) {
- @@ -949,6 +953,8 @@ static void nl_fib_lookup(struct fib_res
- }
- local_bh_enable();
- }
- +
- + rcu_read_unlock();
- }
-
- static void nl_fib_input(struct sk_buff *skb)
- @@ -956,7 +962,6 @@ static void nl_fib_input(struct sk_buff
- struct net *net;
- struct fib_result_nl *frn;
- struct nlmsghdr *nlh;
- - struct fib_table *tb;
- u32 portid;
-
- net = sock_net(skb->sk);
- @@ -972,9 +977,7 @@ static void nl_fib_input(struct sk_buff
- nlh = nlmsg_hdr(skb);
-
- frn = (struct fib_result_nl *) nlmsg_data(nlh);
- - tb = fib_get_table(net, frn->tb_id_in);
- -
- - nl_fib_lookup(frn, tb);
- + nl_fib_lookup(net, frn);
-
- portid = NETLINK_CB(skb).portid; /* netlink portid */
- NETLINK_CB(skb).portid = 0; /* from kernel */
- --- a/net/ipv4/fib_rules.c
- +++ b/net/ipv4/fib_rules.c
- @@ -81,27 +81,25 @@ static int fib4_rule_action(struct fib_r
- break;
-
- case FR_ACT_UNREACHABLE:
- - err = -ENETUNREACH;
- - goto errout;
- + return -ENETUNREACH;
-
- case FR_ACT_PROHIBIT:
- - err = -EACCES;
- - goto errout;
- + return -EACCES;
-
- case FR_ACT_BLACKHOLE:
- default:
- - err = -EINVAL;
- - goto errout;
- + return -EINVAL;
- }
-
- + rcu_read_lock();
- +
- tbl = fib_get_table(rule->fr_net, rule->table);
- - if (!tbl)
- - goto errout;
- + if (tbl)
- + err = fib_table_lookup(tbl, &flp->u.ip4,
- + (struct fib_result *)arg->result,
- + arg->flags);
-
- - err = fib_table_lookup(tbl, &flp->u.ip4, (struct fib_result *) arg->result, arg->flags);
- - if (err > 0)
- - err = -EAGAIN;
- -errout:
- + rcu_read_unlock();
- return err;
- }
-
- --- a/net/ipv4/fib_trie.c
- +++ b/net/ipv4/fib_trie.c
- @@ -1181,72 +1181,6 @@ err:
- return err;
- }
-
- -/* should be called with rcu_read_lock */
- -static int check_leaf(struct fib_table *tb, struct trie *t, struct tnode *l,
- - t_key key, const struct flowi4 *flp,
- - struct fib_result *res, int fib_flags)
- -{
- - struct leaf_info *li;
- - struct hlist_head *hhead = &l->list;
- -
- - hlist_for_each_entry_rcu(li, hhead, hlist) {
- - struct fib_alias *fa;
- -
- - if (l->key != (key & li->mask_plen))
- - continue;
- -
- - list_for_each_entry_rcu(fa, &li->falh, fa_list) {
- - struct fib_info *fi = fa->fa_info;
- - int nhsel, err;
- -
- - if (fa->fa_tos && fa->fa_tos != flp->flowi4_tos)
- - continue;
- - if (fi->fib_dead)
- - continue;
- - if (fa->fa_info->fib_scope < flp->flowi4_scope)
- - continue;
- - fib_alias_accessed(fa);
- - err = fib_props[fa->fa_type].error;
- - if (unlikely(err < 0)) {
- -#ifdef CONFIG_IP_FIB_TRIE_STATS
- - this_cpu_inc(t->stats->semantic_match_passed);
- -#endif
- - return err;
- - }
- - if (fi->fib_flags & RTNH_F_DEAD)
- - continue;
- - for (nhsel = 0; nhsel < fi->fib_nhs; nhsel++) {
- - const struct fib_nh *nh = &fi->fib_nh[nhsel];
- -
- - if (nh->nh_flags & RTNH_F_DEAD)
- - continue;
- - if (flp->flowi4_oif && flp->flowi4_oif != nh->nh_oif)
- - continue;
- -
- -#ifdef CONFIG_IP_FIB_TRIE_STATS
- - this_cpu_inc(t->stats->semantic_match_passed);
- -#endif
- - res->prefixlen = li->plen;
- - res->nh_sel = nhsel;
- - res->type = fa->fa_type;
- - res->scope = fi->fib_scope;
- - res->fi = fi;
- - res->table = tb;
- - res->fa_head = &li->falh;
- - if (!(fib_flags & FIB_LOOKUP_NOREF))
- - atomic_inc(&fi->fib_clntref);
- - return 0;
- - }
- - }
- -
- -#ifdef CONFIG_IP_FIB_TRIE_STATS
- - this_cpu_inc(t->stats->semantic_match_miss);
- -#endif
- - }
- -
- - return 1;
- -}
- -
- static inline t_key prefix_mismatch(t_key key, struct tnode *n)
- {
- t_key prefix = n->key;
- @@ -1254,6 +1188,7 @@ static inline t_key prefix_mismatch(t_ke
- return (key ^ prefix) & (prefix | -prefix);
- }
-
- +/* should be called with rcu_read_lock */
- int fib_table_lookup(struct fib_table *tb, const struct flowi4 *flp,
- struct fib_result *res, int fib_flags)
- {
- @@ -1263,14 +1198,12 @@ int fib_table_lookup(struct fib_table *t
- #endif
- const t_key key = ntohl(flp->daddr);
- struct tnode *n, *pn;
- + struct leaf_info *li;
- t_key cindex;
- - int ret = 1;
- -
- - rcu_read_lock();
-
- n = rcu_dereference(t->trie);
- if (!n)
- - goto failed;
- + return -EAGAIN;
-
- #ifdef CONFIG_IP_FIB_TRIE_STATS
- this_cpu_inc(stats->gets);
- @@ -1350,7 +1283,7 @@ backtrace:
-
- pn = node_parent_rcu(pn);
- if (unlikely(!pn))
- - goto failed;
- + return -EAGAIN;
- #ifdef CONFIG_IP_FIB_TRIE_STATS
- this_cpu_inc(stats->backtrack);
- #endif
- @@ -1368,12 +1301,62 @@ backtrace:
-
- found:
- /* Step 3: Process the leaf, if that fails fall back to backtracing */
- - ret = check_leaf(tb, t, n, key, flp, res, fib_flags);
- - if (unlikely(ret > 0))
- - goto backtrace;
- -failed:
- - rcu_read_unlock();
- - return ret;
- + hlist_for_each_entry_rcu(li, &n->list, hlist) {
- + struct fib_alias *fa;
- +
- + if ((key ^ n->key) & li->mask_plen)
- + continue;
- +
- + list_for_each_entry_rcu(fa, &li->falh, fa_list) {
- + struct fib_info *fi = fa->fa_info;
- + int nhsel, err;
- +
- + if (fa->fa_tos && fa->fa_tos != flp->flowi4_tos)
- + continue;
- + if (fi->fib_dead)
- + continue;
- + if (fa->fa_info->fib_scope < flp->flowi4_scope)
- + continue;
- + fib_alias_accessed(fa);
- + err = fib_props[fa->fa_type].error;
- + if (unlikely(err < 0)) {
- +#ifdef CONFIG_IP_FIB_TRIE_STATS
- + this_cpu_inc(stats->semantic_match_passed);
- +#endif
- + return err;
- + }
- + if (fi->fib_flags & RTNH_F_DEAD)
- + continue;
- + for (nhsel = 0; nhsel < fi->fib_nhs; nhsel++) {
- + const struct fib_nh *nh = &fi->fib_nh[nhsel];
- +
- + if (nh->nh_flags & RTNH_F_DEAD)
- + continue;
- + if (flp->flowi4_oif && flp->flowi4_oif != nh->nh_oif)
- + continue;
- +
- + if (!(fib_flags & FIB_LOOKUP_NOREF))
- + atomic_inc(&fi->fib_clntref);
- +
- + res->prefixlen = li->plen;
- + res->nh_sel = nhsel;
- + res->type = fa->fa_type;
- + res->scope = fi->fib_scope;
- + res->fi = fi;
- + res->table = tb;
- + res->fa_head = &li->falh;
- +#ifdef CONFIG_IP_FIB_TRIE_STATS
- + this_cpu_inc(stats->semantic_match_passed);
- +#endif
- + return err;
- + }
- + }
- +
- +#ifdef CONFIG_IP_FIB_TRIE_STATS
- + this_cpu_inc(stats->semantic_match_miss);
- +#endif
- + }
- + goto backtrace;
- }
- EXPORT_SYMBOL_GPL(fib_table_lookup);
-
|