Shawn Bohrer <sbohrer@xxxxxxxxxxxxxx> wrote: > This includes a couple of fixes that I found while investigating the > crash I was experiencing. The crash is fixed in the first patch. > > Additionally as I'm not terribly familiar with the netfilter code, I have > some questions that might lead to additional fixes. > > 1. Should nf_conncount_destroy() acquire the nf_conncount_locks > spinlock? This updates the tree by calling rb_erase() so what keeps it > from racing with insert_tree() or tree_gc_worker()? nf_conncount_destroy must never be called while packets can still enter the tree. The GC worker must be stopped already by the time the trees are removed. > 2. I'm in no way an RCU expert, but I don't think rb_erase() is RCU > safe. Under the covers it eventually calls __rb_change_child() and not > __rb_change_child_rcu(). Actually same thing for rb_insert_color(). > I'll note there are very few users in the kernel currently using RCU > with rbtrees. Yes, this is rather new. Looks like we need to add rcu variants for those. > 3. As an optimization it might be possible to use rb_replace_node_rcu() > when an exact match is found, but the node is dead. I haven't spent > enough time thinking about how to rework the code to do this. I would not think in terms of 'optimization' for such a rare case. If it simplifies code, go for it, else i would not bother.
