On Mon, Jul 6, 2015 at 2:33 PM, Dennis Ferguson <dennis.c.fergu...@gmail.com> wrote: > > On 5 Jul, 2015, at 19:02 , Ryota Ozaki <ozak...@netbsd.org> wrote: >> On Sun, Jul 5, 2015 at 6:50 PM, Joerg Sonnenberger >> <jo...@britannica.bec.de> wrote: >>> On Sun, Jul 05, 2015 at 02:12:18PM +0900, Ryota Ozaki wrote: >>>> On Sun, Jul 5, 2015 at 2:35 AM, David Young <dyo...@pobox.com> wrote: >>>>> On Sat, Jul 04, 2015 at 09:52:56PM +0900, Ryota Ozaki wrote: >>>>>> I'm trying to improve use of rt_refcnt: reducing >>>>>> abuse of it, e.g., rt_refcnt++/rt_refcnt-- outside >>>>>> route.c and extending it to treat referencing >>>>>> during packet processing (IOW, references from >>>>>> local variables) -- currently it handles only >>>>>> references between routes. The latter is needed for >>>>>> MP-safe networking. >>>>> >>>>> Do you propose to increase/decrease rt_refcnt in the packet processing >>>>> path, using atomic instructions? >>>> >>>> Atomic instructions aren't used yet, i.e., softnet_lock is still needed. >>>> I will introduce them later. (Using refcount(9) by riastradh would be >>>> good once it is committed.) >>> >>> I think the main point that David wanted to raise is that the normal >>> path for packets should *not* do any ref count changes at all. >> >> Why? rtentry can be freed during the normal path in MP-safe world. >> Do you suggest using pserialize instead? > > I don't think either a reference count or pserialize, or anything else > that is non-blocking for readers, can be used to protect the data > structure rtentry's are now stored in.
I'm sorry for confusing you, our first attempt doesn't intend to provide non-blocking reader operations. But yet I misunderstood about the characteristic of the routing table you described below. > > If you want readers to continue while a data structure is being modified > then the modification must be implemented so that concurrent readers see > the structure in a consistent state (i.e. one that produces either the > before-the-change or the after-the-change result) at every point during > the change. Since the current radix tree does not work this way the only > way to make a change to it is to block the readers while the change is > being made, i.e. with a lock. An rtentry will hence never be freed while > the normal (reader) path is looking at it since you'll be preventing those > readers from looking at anything in that structure while you are changing it. I got what you mean. The current implementation just doesn't free a rtentry if there are references to it but does modify a rtentry regardless of refcnt of it. I'll use a lock somehow to prevent the latter. Nonetheless, I think my patch is still useful to prevent the former. (And anyway we have to reduce awkward use of refcnt.) > > If you don't want it to work this way then you'll need to replace the > radix tree with something that permits changes while readers are > concurrently operating. IIUC, your rttree(9) satisfies the requirement, right? We're evaluating rttree(9) as a replacement of the current radix tree. Do you have any updates on rttree(9)? ozaki-r > To take best advantage of a more modern data > structure, however, you are still not going to want readers to ever > write the shared data structure if that can be avoided. The two > atomic operations needed to increment and decrement a reference count > greatly exceed the cost of a (well-cached) route lookup. > > Dennis Ferguson