On Thu, 4 Aug 2005 10:17:58 +0200
Robert Olsson <[EMAIL PROTECTED]> wrote:

> 
> Stephen Hemminger writes:
>  > Convert FIB Trie to use RCU for the normal lookup fast
>  > path. Use simple spin_lock for updates and dump access.
>  
>  Hello!
> 
>  Yes spent some days to get a complete RCU for the trie. Discussed
> with Patrick at OLS. This includes your start and also RCU versions
> of fib_alias handling and removes the fib_lock totally etc.
> 
>  Replace of fib_alias needs to be proctected via RCU and should be
> added.
> 
> > This needs more testing on systems with lareg numbers of routes
>  
>  Testing and verification is absolutely crucial... and is often much
> more work then doing a patch. And really testing & verification is a
> iterative part of the design.
> 
>  The patch below is current work it is tested/iterated with rDoS
> injection during both insert, delete and dump of full bgp on SMP
> machine.
> 
>

I have a merged patch set in the works. with Robert et al.
Some of what my differences are:

>  
> +/* Deal with better RCU integration later....  */
> +
> +/* Return the first fib alias matching TOS with
> + * priority less than or equal to PRIO.
> + */
> +struct fib_alias *fib_find_alias_rcu(struct list_head *fah, u8 tos,
> u32 prio) +{
> +     if (fah) {
> +             struct fib_alias *fa;
> +             list_for_each_entry_rcu(fa, fah, fa_list) {
> +                     if (fa->fa_tos > tos)
> +                             continue;
> +                     if (fa->fa_info->fib_priority >= prio ||
> +                         fa->fa_tos < tos)
> +                             return fa;
> +             }
> +     }
> +     return NULL;
> +}

fib_find_alias is only used in path's that have RT netlink
mutex held, so RCU is not needed.

> +int fib_semantic_match_rcu(struct list_head *head, const struct
> flowi *flp,
> +                    struct fib_result *res, __u32 zone, __u32
> mask, 
> +                     int prefixlen)
> +{


Why duplicate the code of of fib_semantic_match? It would be
better to just change fib_semantic_match to use the for_each_rcu().
The resulting code will be the same (except alpha) because the
only addition is a read_barrier_depends().

>  int fib_semantic_match(struct list_head *head, const struct flowi
> *flp, struct fib_result *res, __u32 zone, __u32 mask, 
>                       int prefixlen)


Also, please don't bother adding RCU to routines that are modifying
the tree. Only the lookup, dump, and /proc code paths should
use RCU.


-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to