Re: Improving use of rt_refcnt

[Ryota Ozaki <ozaki-r%netbsd.org@localhost>]

On Wed, Jul 8, 2015 at 11:57 AM, Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
> On Mon, Jul 6, 2015 at 2:33 PM, Dennis Ferguson
> <dennis.c.ferguson%gmail.com@localhost> wrote:
>>
>> On 5 Jul, 2015, at 19:02 , Ryota Ozaki <ozaki-r%netbsd.org@localhost> wrote:
>>> On Sun, Jul 5, 2015 at 6:50 PM, Joerg Sonnenberger
>>> <joerg%britannica.bec.de@localhost> 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 <dyoung%pobox.com@localhost> 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.)

BTW how do you think of separating L2 tables (ARP/NDP) from the L3
routing tables? The separation gets rid of cloning/cloned route
features and that makes it easy to introduce locks in route.c.
(Currently rtrequest1 can be called recursively to remove cloned
routes and that makes it hard to use locks.) I read your paper
(BSDNetworking.pdf) and it seems to suggest to maintain L2 routes
in the common routing table (I may misunderstand your opinion).

Thanks,
  ozaki-r

>
>>
>> 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