Re: Improving use of rt_refcnt

[Dennis Ferguson <dennis.c.ferguson%gmail.com@localhost>]

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.

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.

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