Re: MP-safe ifnet with psz & psref

2016-06-05 Thread Ryota Ozaki
Hi,

So I prepared patches:
  http://www.netbsd.org/~ozaki-r/m_set_rcvif.diff
  http://www.netbsd.org/~ozaki-r/m_get_rcvif.diff

The first patch introduces m_set_rcvif and m_reset_rcvif
that provide consistency API against m_get_rcvif,
hide internal of rcvif operation, and reduce the diff
of the second patch. No functional change.

The second patch realizes the proposal; store if_index
into mbuf instead of a pointer of an ifnet object and
look up an ifnet object by if_index when we get it from
a mbuf (*). The patch provides two APIs:
m_{get,put}_rcvif_psref that use psref(9) for sleep-able
critical sections and m_{get,put}_rcvif that use
pserialize(9) for other critical sections. The patch
also another API called m_get_rcvif_NOMPSAFE, that is NOT
MP-safe and for transition moratorium, i.e., it is intended
to be used for places where are not planned to be MP-ified
soon.

(*) In reality we need to preserve a pointer (void *)
for M_{SET,GET}CTX so that we keep it as a union
with if_index.

I measured the overhead of introducing psref in fast paths,
bridge forwarding and IP forwarding, and confirmed that
the overhead is negligible, 2% down at worst.

Thanks,
  ozaki-r

On Sun, May 22, 2016 at 6:10 PM, Ryota Ozaki  wrote:
> On Fri, May 20, 2016 at 6:11 PM, Ryota Ozaki  wrote:
>> On Fri, May 20, 2016 at 5:38 PM, Joerg Sonnenberger  wrote:
>>> On Fri, May 20, 2016 at 12:15:10PM +0900, Ryota Ozaki wrote:
 > Mbufs are slightly different. They can be queued on an interface for a
 > moderately long time. For this purpose, a "flush" hook would definitely
 > be needed. I think having such a hook would be useful for other purposes
 > as well, i.e. to trigger memory compaction. But it is a somewhat
 > expensive change in that it requires review of a lot of code.
 >
 > The final question for me is whether we can get away with just not
 > deallocating ifnet instances. Reusing them is fine in principle, the
 > problem is just keeping the memory type safe.

 What "keeping the memory type safe" means?
>>>
>>> Once allocated as ifnet, it will always be an ifnet.
>>
>> I see.
>>
>>>
 The problem "when we can reuse it?" seems to be similar to "when we can
 deallocate it?". Don't we need to do a similar solution I propose?
>>>
>>> The big question is what do we use rcvif for. If it is just a cookie
>>> used for preventing cycles, it doesn't matter what the content is.
>>
>> No, rcvif is used arbitrarily.
>>
>>> Otherwise we should have a field whether it was initialized completely
>>> or not.
>>
>> We can have IFF_INITIALIZED...oops, there is no space in if_flags :-<
>
> IIUC making such flag work requires some synchronization, a mutex or a psref,
> on an ifnet object to avoid race condition between clearing the initialized
> flag and proceeding destruction on a writer and checking the flag and
> referencing the object on a reader. Am I correct?
>
>   ozaki-r


Re: MP-safe ifnet with psz & psref

2016-05-22 Thread Ryota Ozaki
On Fri, May 20, 2016 at 6:11 PM, Ryota Ozaki  wrote:
> On Fri, May 20, 2016 at 5:38 PM, Joerg Sonnenberger  wrote:
>> On Fri, May 20, 2016 at 12:15:10PM +0900, Ryota Ozaki wrote:
>>> > Mbufs are slightly different. They can be queued on an interface for a
>>> > moderately long time. For this purpose, a "flush" hook would definitely
>>> > be needed. I think having such a hook would be useful for other purposes
>>> > as well, i.e. to trigger memory compaction. But it is a somewhat
>>> > expensive change in that it requires review of a lot of code.
>>> >
>>> > The final question for me is whether we can get away with just not
>>> > deallocating ifnet instances. Reusing them is fine in principle, the
>>> > problem is just keeping the memory type safe.
>>>
>>> What "keeping the memory type safe" means?
>>
>> Once allocated as ifnet, it will always be an ifnet.
>
> I see.
>
>>
>>> The problem "when we can reuse it?" seems to be similar to "when we can
>>> deallocate it?". Don't we need to do a similar solution I propose?
>>
>> The big question is what do we use rcvif for. If it is just a cookie
>> used for preventing cycles, it doesn't matter what the content is.
>
> No, rcvif is used arbitrarily.
>
>> Otherwise we should have a field whether it was initialized completely
>> or not.
>
> We can have IFF_INITIALIZED...oops, there is no space in if_flags :-<

IIUC making such flag work requires some synchronization, a mutex or a psref,
on an ifnet object to avoid race condition between clearing the initialized
flag and proceeding destruction on a writer and checking the flag and
referencing the object on a reader. Am I correct?

  ozaki-r


Re: MP-safe ifnet with psz & psref

2016-05-20 Thread Ryota Ozaki
On Fri, May 20, 2016 at 5:38 PM, Joerg Sonnenberger  wrote:
> On Fri, May 20, 2016 at 12:15:10PM +0900, Ryota Ozaki wrote:
>> > Mbufs are slightly different. They can be queued on an interface for a
>> > moderately long time. For this purpose, a "flush" hook would definitely
>> > be needed. I think having such a hook would be useful for other purposes
>> > as well, i.e. to trigger memory compaction. But it is a somewhat
>> > expensive change in that it requires review of a lot of code.
>> >
>> > The final question for me is whether we can get away with just not
>> > deallocating ifnet instances. Reusing them is fine in principle, the
>> > problem is just keeping the memory type safe.
>>
>> What "keeping the memory type safe" means?
>
> Once allocated as ifnet, it will always be an ifnet.

I see.

>
>> The problem "when we can reuse it?" seems to be similar to "when we can
>> deallocate it?". Don't we need to do a similar solution I propose?
>
> The big question is what do we use rcvif for. If it is just a cookie
> used for preventing cycles, it doesn't matter what the content is.

No, rcvif is used arbitrarily.

> Otherwise we should have a field whether it was initialized completely
> or not.

We can have IFF_INITIALIZED...oops, there is no space in if_flags :-<

  ozaki-r


Re: MP-safe ifnet with psz & psref

2016-05-20 Thread Joerg Sonnenberger
On Fri, May 20, 2016 at 12:15:10PM +0900, Ryota Ozaki wrote:
> > Mbufs are slightly different. They can be queued on an interface for a
> > moderately long time. For this purpose, a "flush" hook would definitely
> > be needed. I think having such a hook would be useful for other purposes
> > as well, i.e. to trigger memory compaction. But it is a somewhat
> > expensive change in that it requires review of a lot of code.
> >
> > The final question for me is whether we can get away with just not
> > deallocating ifnet instances. Reusing them is fine in principle, the
> > problem is just keeping the memory type safe.
> 
> What "keeping the memory type safe" means?

Once allocated as ifnet, it will always be an ifnet.

> The problem "when we can reuse it?" seems to be similar to "when we can
> deallocate it?". Don't we need to do a similar solution I propose?

The big question is what do we use rcvif for. If it is just a cookie
used for preventing cycles, it doesn't matter what the content is.
Otherwise we should have a field whether it was initialized completely
or not.

Joerg


Re: MP-safe ifnet with psz & psref

2016-05-19 Thread Taylor R Campbell
   Date: Fri, 20 May 2016 11:51:24 +0900
   From: Ryota Ozaki 

   On Fri, May 20, 2016 at 3:01 AM, Taylor R Campbell
wrote:
   > Can you explain why there are long-term mbufs and rtentries that may
   > continue to point to ifnets that are gone?  Can you instead arrange to
   > (1) prevent an ifnet from being used, and (2) flush all mbufs and
   > rtentries pointing at it, before (3) destroying the ifnet?

   For mbufs, currently we do nothing when destroying an interface object.
   We can flush mbufs queued in its if_percpuq, but it's difficult to all
   mbufs pointing to the interface, for example mbufs in other queues such
   as pktqueues for ip and gif. (Well, we may be able to flush such mbufs
   in every existing queues, I'm not sure it's a realistic approach.)
   And we also have to wait for in-flight mbufs, which are dequeued and
   in use, are disappeared somehow.

Fair enough.  I guess just recursively flushing every queue that might
be fed into by the interface's queue is not doable, since there are
cycles in the transitive closure of that relation.


Re: MP-safe ifnet with psz & psref

2016-05-19 Thread Ryota Ozaki
On Fri, May 20, 2016 at 3:13 AM, Joerg Sonnenberger  wrote:
> On Thu, May 19, 2016 at 04:11:56PM +0900, Ryota Ozaki wrote:
>> I'm working on a next task that replaces an ifnet pointer embedded
>> in other objects, say mbuf and rtentry, with an interface index
>> (ifnet#if_index) and when we take an ifnet object from such objects,
>> look it up (with psz or psref) from the interface collection
>> (ifindex2ifnet, just an array). By doing so, we can ensure a taken
>> ifnet object isn't freed during manipulating it. And if an interface
>> which we're obtaining is being destroyed, a look-up just returns NULL.
>
> Similar to Taylor's question, I wonder if this is really the correct
> direction. Routing table entries should never point to dead interfaces.
> If they do, there is a synchronisation bug already. Introducing a layer
> of indirection seems to be a pure penalty for the hot path.

Sure. It should be guaranteed by rtentries (the routing table).

>
> Mbufs are slightly different. They can be queued on an interface for a
> moderately long time. For this purpose, a "flush" hook would definitely
> be needed. I think having such a hook would be useful for other purposes
> as well, i.e. to trigger memory compaction. But it is a somewhat
> expensive change in that it requires review of a lot of code.
>
> The final question for me is whether we can get away with just not
> deallocating ifnet instances. Reusing them is fine in principle, the
> problem is just keeping the memory type safe.

What "keeping the memory type safe" means?

The problem "when we can reuse it?" seems to be similar to "when we can
deallocate it?". Don't we need to do a similar solution I propose?

Thanks,
  ozaki-r


Re: MP-safe ifnet with psz & psref

2016-05-19 Thread Ryota Ozaki
On Fri, May 20, 2016 at 3:01 AM, Taylor R Campbell
 wrote:
>Date: Thu, 19 May 2016 16:11:56 +0900
>From: Ryota Ozaki 
>
>I'm working on a next task that replaces an ifnet pointer embedded
>in other objects, say mbuf and rtentry, with an interface index
>(ifnet#if_index) and when we take an ifnet object from such objects,
>look it up (with psz or psref) from the interface collection
>(ifindex2ifnet, just an array). By doing so, we can ensure a taken
>ifnet object isn't freed during manipulating it. And if an interface
>which we're obtaining is being destroyed, a look-up just returns NULL.
>
> Can you explain why there are long-term mbufs and rtentries that may
> continue to point to ifnets that are gone?  Can you instead arrange to
> (1) prevent an ifnet from being used, and (2) flush all mbufs and
> rtentries pointing at it, before (3) destroying the ifnet?

For mbufs, currently we do nothing when destroying an interface object.
We can flush mbufs queued in its if_percpuq, but it's difficult to all
mbufs pointing to the interface, for example mbufs in other queues such
as pktqueues for ip and gif. (Well, we may be able to flush such mbufs
in every existing queues, I'm not sure it's a realistic approach.)
And we also have to wait for in-flight mbufs, which are dequeued and
in use, are disappeared somehow.

For rtentries, hmm I'm wrong :-/ The problem should be handled by
rtentries (the routing table), not ifnet. (joerg's comment awakes me.)

Thanks,
  ozaki-r


Re: MP-safe ifnet with psz & psref

2016-05-19 Thread Joerg Sonnenberger
On Thu, May 19, 2016 at 04:11:56PM +0900, Ryota Ozaki wrote:
> I'm working on a next task that replaces an ifnet pointer embedded
> in other objects, say mbuf and rtentry, with an interface index
> (ifnet#if_index) and when we take an ifnet object from such objects,
> look it up (with psz or psref) from the interface collection
> (ifindex2ifnet, just an array). By doing so, we can ensure a taken
> ifnet object isn't freed during manipulating it. And if an interface
> which we're obtaining is being destroyed, a look-up just returns NULL.

Similar to Taylor's question, I wonder if this is really the correct
direction. Routing table entries should never point to dead interfaces.
If they do, there is a synchronisation bug already. Introducing a layer
of indirection seems to be a pure penalty for the hot path.

Mbufs are slightly different. They can be queued on an interface for a
moderately long time. For this purpose, a "flush" hook would definitely
be needed. I think having such a hook would be useful for other purposes
as well, i.e. to trigger memory compaction. But it is a somewhat
expensive change in that it requires review of a lot of code.

The final question for me is whether we can get away with just not
deallocating ifnet instances. Reusing them is fine in principle, the
problem is just keeping the memory type safe.

Joerg


Re: MP-safe ifnet with psz & psref

2016-05-19 Thread Taylor R Campbell
   Date: Thu, 19 May 2016 16:11:56 +0900
   From: Ryota Ozaki 

   I'm working on a next task that replaces an ifnet pointer embedded
   in other objects, say mbuf and rtentry, with an interface index
   (ifnet#if_index) and when we take an ifnet object from such objects,
   look it up (with psz or psref) from the interface collection
   (ifindex2ifnet, just an array). By doing so, we can ensure a taken
   ifnet object isn't freed during manipulating it. And if an interface
   which we're obtaining is being destroyed, a look-up just returns NULL.

Can you explain why there are long-term mbufs and rtentries that may
continue to point to ifnets that are gone?  Can you instead arrange to
(1) prevent an ifnet from being used, and (2) flush all mbufs and
rtentries pointing at it, before (3) destroying the ifnet?


Re: MP-safe ifnet with psz & psref

2016-05-19 Thread Ryota Ozaki
Hi,

I'm working on a next task that replaces an ifnet pointer embedded
in other objects, say mbuf and rtentry, with an interface index
(ifnet#if_index) and when we take an ifnet object from such objects,
look it up (with psz or psref) from the interface collection
(ifindex2ifnet, just an array). By doing so, we can ensure a taken
ifnet object isn't freed during manipulating it. And if an interface
which we're obtaining is being destroyed, a look-up just returns NULL.

This needs huge amount of changes, so I want suggestions and comments
before proceeding the task. Any thoughts?

Thanks,
  ozaki-r


Re: MP-safe ifnet with psz & psref

2016-05-13 Thread Ryota Ozaki
On Tue, May 10, 2016 at 7:19 PM, Ryota Ozaki  wrote:
> Yet another patch is here:
>   http://www.netbsd.org/~ozaki-r/if_get.diff
>
> This trial patch introduces if_get that returns an ifnet
> object with holding psref and if_put that releases psref
> of the ifnet object. The patch replaces ifnet_lock with
> if_get/if_put and a simple mutex. The change fixes an
> issue that normal ioctls and if_clone_destroy on the
> same interface can run in parallel and it causes a panic.

Patches updated:
  http://www.netbsd.org/~ozaki-r/if_get.diff
  http://www.netbsd.org/~ozaki-r/retire-ifnet_lock.diff

The new one survived a stress test for a day (the old one
was still racy).

Thanks,
  ozaki-r


Re: MP-safe ifnet with psz & psref

2016-05-11 Thread Ryota Ozaki
On Thu, May 12, 2016 at 11:36 AM, Christos Zoulas  wrote:
> In article 
> ,
> Ryota Ozaki   wrote:
>>On Wed, May 11, 2016 at 4:18 AM, matthew green  wrote:
>>> Thor Lancelot Simon writes:
 I do not think you should do any extra work to support kvm grovelling.
>>
>>Actually I do nothing other than leaving the original list
>>as it is. I thinks it's reasonable as what we do now.
>>
>>>
>>> existing tool functionality should not be broken, however.  the
>>> netstat -i groveller code should be either updated to the new
>>> method or converted to a real sysctl, though the current work
>>> around would be ok for now.  simply breaking netstat -i is not
>>> an acceptable solution.
>>
>>Yeah, someone should sweep kvm(3) users in the future...
>>
>>BTW netstat already has a feature to retrieve information via
>>sysctl but its man page warns that it lacks some information
>>compared to kvm(3). Do anyone know what's lost?
>
> grep the netstat code for XXX

Thanks.

Hmm, if_snd.ifq_drops. We now have percpu RX queues and will have
multiple TX queues (in each device driver, not ifnet though).
We would need to revise how to show queue information of interfaces.

  ozaki-r


Re: MP-safe ifnet with psz & psref

2016-05-11 Thread Christos Zoulas
In article ,
Ryota Ozaki   wrote:
>On Wed, May 11, 2016 at 4:18 AM, matthew green  wrote:
>> Thor Lancelot Simon writes:
>>> I do not think you should do any extra work to support kvm grovelling.
>
>Actually I do nothing other than leaving the original list
>as it is. I thinks it's reasonable as what we do now.
>
>>
>> existing tool functionality should not be broken, however.  the
>> netstat -i groveller code should be either updated to the new
>> method or converted to a real sysctl, though the current work
>> around would be ok for now.  simply breaking netstat -i is not
>> an acceptable solution.
>
>Yeah, someone should sweep kvm(3) users in the future...
>
>BTW netstat already has a feature to retrieve information via
>sysctl but its man page warns that it lacks some information
>compared to kvm(3). Do anyone know what's lost?

grep the netstat code for XXX

christos



Re: MP-safe ifnet with psz & psref

2016-05-11 Thread Christos Zoulas
In article <11591.1462907...@splode.eterna.com.au>,
matthew green   wrote:
>Thor Lancelot Simon writes:
>> I do not think you should do any extra work to support kvm grovelling.
>
>existing tool functionality should not be broken, however.  the
>netstat -i groveller code should be either updated to the new
>method or converted to a real sysctl, though the current work
>around would be ok for now.  simply breaking netstat -i is not
>an acceptable solution.

I will fix netstat, don't worry about it.

christos



Re: MP-safe ifnet with psz & psref

2016-05-11 Thread Ryota Ozaki
Sorry for consecutive patches but one more patch is here:
  http://www.netbsd.org/~ozaki-r/if_get_byindex.diff

The patch implements if_get_byindex that is if_byindex
with psref. Same as if_get it's used together with
if_put. The patch also replaces some if_byindex with it
while some if_byindex are kept because just adding
pserialize is enough.

Any comments?

  ozaki-r


On Wed, May 11, 2016 at 6:53 PM, Ryota Ozaki  wrote:
> http://www.netbsd.org/~ozaki-r/psref-ifnet.diff
>
> One more update. Use IFNET_LOCK to protect whole if_getindex
> that updates global shared objects and retire index_gen_mtx
> that is used only for index_gen that is also updated in
> if_getindex.
>
>   ozaki-r
>
>
> On Wed, May 11, 2016 at 10:03 AM, Ryota Ozaki  wrote:
>> On Wed, May 11, 2016 at 4:18 AM, matthew green  wrote:
>>> Thor Lancelot Simon writes:
 I do not think you should do any extra work to support kvm grovelling.
>>
>> Actually I do nothing other than leaving the original list
>> as it is. I thinks it's reasonable as what we do now.
>>
>>>
>>> existing tool functionality should not be broken, however.  the
>>> netstat -i groveller code should be either updated to the new
>>> method or converted to a real sysctl, though the current work
>>> around would be ok for now.  simply breaking netstat -i is not
>>> an acceptable solution.
>>
>> Yeah, someone should sweep kvm(3) users in the future...
>>
>> BTW netstat already has a feature to retrieve information via
>> sysctl but its man page warns that it lacks some information
>> compared to kvm(3). Do anyone know what's lost?
>>
>>   ozaki-r


Re: MP-safe ifnet with psz & psref

2016-05-11 Thread Ryota Ozaki
http://www.netbsd.org/~ozaki-r/psref-ifnet.diff

One more update. Use IFNET_LOCK to protect whole if_getindex
that updates global shared objects and retire index_gen_mtx
that is used only for index_gen that is also updated in
if_getindex.

  ozaki-r


On Wed, May 11, 2016 at 10:03 AM, Ryota Ozaki  wrote:
> On Wed, May 11, 2016 at 4:18 AM, matthew green  wrote:
>> Thor Lancelot Simon writes:
>>> I do not think you should do any extra work to support kvm grovelling.
>
> Actually I do nothing other than leaving the original list
> as it is. I thinks it's reasonable as what we do now.
>
>>
>> existing tool functionality should not be broken, however.  the
>> netstat -i groveller code should be either updated to the new
>> method or converted to a real sysctl, though the current work
>> around would be ok for now.  simply breaking netstat -i is not
>> an acceptable solution.
>
> Yeah, someone should sweep kvm(3) users in the future...
>
> BTW netstat already has a feature to retrieve information via
> sysctl but its man page warns that it lacks some information
> compared to kvm(3). Do anyone know what's lost?
>
>   ozaki-r


Re: MP-safe ifnet with psz & psref

2016-05-10 Thread Ryota Ozaki
On Wed, May 11, 2016 at 4:18 AM, matthew green  wrote:
> Thor Lancelot Simon writes:
>> I do not think you should do any extra work to support kvm grovelling.

Actually I do nothing other than leaving the original list
as it is. I thinks it's reasonable as what we do now.

>
> existing tool functionality should not be broken, however.  the
> netstat -i groveller code should be either updated to the new
> method or converted to a real sysctl, though the current work
> around would be ok for now.  simply breaking netstat -i is not
> an acceptable solution.

Yeah, someone should sweep kvm(3) users in the future...

BTW netstat already has a feature to retrieve information via
sysctl but its man page warns that it lacks some information
compared to kvm(3). Do anyone know what's lost?

  ozaki-r


re: MP-safe ifnet with psz & psref

2016-05-10 Thread matthew green
Thor Lancelot Simon writes:
> I do not think you should do any extra work to support kvm grovelling.

existing tool functionality should not be broken, however.  the
netstat -i groveller code should be either updated to the new
method or converted to a real sysctl, though the current work
around would be ok for now.  simply breaking netstat -i is not
an acceptable solution.


.mrg.

> On Tue, May 10, 2016 at 12:07:12PM +0900, Ryota Ozaki wrote:
> > On Thu, Apr 28, 2016 at 4:04 PM, Ryota Ozaki  wrote:
> > > Hi,
> > >
> > > This proposal is the first step toward making interfaces
> > > (struct ifnet) MP-safe; it applies psz and psref to ifnet
> > > and ifnet_list to safely look up ifnets from ifnet_list
> > > and safely remove an ifnet.
> > >
> > > Here is a patch:
> > >   http://www.netbsd.org/~ozaki-r/psref-ifnet.diff
> > 
> > I restored the original ifnet_list because the change broke
> > netstat -i (kvm(3)). So we now maintain two ifnet lists while
> > the original list is only added/removed elements and not
> > referenced by say IFNET_FOREACH. It's redundant but its overhead
> > is negligible and it's the easiest way to keep backward
> > compatibility. Of course, ideally we should kill kvm(3) users
> > and remove the original list, but it's not now, I think.
> > 
> > Objection?
> > 
> >   ozaki-r
> > 
> > >
> > > One concern is m_reclaim that can be run in hardware
> > > interrupt and so the patch skips if_drain if it's
> > > running in hardware interrupt because pserialize is
> > > basically designed used in lwp or softint (and now
> > > psref is used with IPL_SOFTNET). Is there a better
> > > solution?
> > >
> > > And any other suggestions and comments would be
> > > appreciated.
> > >
> > > Thanks,
> > >   ozaki-r
> > >
> > > P.S. From today for ten days, I'll be unable to use
> > > my main development machines, I wouldn't be able to
> > > update and test patches reflected your feedbacks enough.
> 
> -- 
>   Thor Lancelot Simon  t...@panix.com
> 
>   "We cannot usually in social life pursue a single value or a single moral
>aim, untroubled by the need to compromise with others."  - H.L.A. Hart
> 


Re: MP-safe ifnet with psz & psref

2016-05-10 Thread Thor Lancelot Simon
I do not think you should do any extra work to support kvm grovelling.

On Tue, May 10, 2016 at 12:07:12PM +0900, Ryota Ozaki wrote:
> On Thu, Apr 28, 2016 at 4:04 PM, Ryota Ozaki  wrote:
> > Hi,
> >
> > This proposal is the first step toward making interfaces
> > (struct ifnet) MP-safe; it applies psz and psref to ifnet
> > and ifnet_list to safely look up ifnets from ifnet_list
> > and safely remove an ifnet.
> >
> > Here is a patch:
> >   http://www.netbsd.org/~ozaki-r/psref-ifnet.diff
> 
> I restored the original ifnet_list because the change broke
> netstat -i (kvm(3)). So we now maintain two ifnet lists while
> the original list is only added/removed elements and not
> referenced by say IFNET_FOREACH. It's redundant but its overhead
> is negligible and it's the easiest way to keep backward
> compatibility. Of course, ideally we should kill kvm(3) users
> and remove the original list, but it's not now, I think.
> 
> Objection?
> 
>   ozaki-r
> 
> >
> > One concern is m_reclaim that can be run in hardware
> > interrupt and so the patch skips if_drain if it's
> > running in hardware interrupt because pserialize is
> > basically designed used in lwp or softint (and now
> > psref is used with IPL_SOFTNET). Is there a better
> > solution?
> >
> > And any other suggestions and comments would be
> > appreciated.
> >
> > Thanks,
> >   ozaki-r
> >
> > P.S. From today for ten days, I'll be unable to use
> > my main development machines, I wouldn't be able to
> > update and test patches reflected your feedbacks enough.

-- 
  Thor Lancelot Simont...@panix.com

  "We cannot usually in social life pursue a single value or a single moral
   aim, untroubled by the need to compromise with others."  - H.L.A. Hart


Re: MP-safe ifnet with psz & psref

2016-05-10 Thread Ryota Ozaki
Yet another patch is here:
  http://www.netbsd.org/~ozaki-r/if_get.diff

This trial patch introduces if_get that returns an ifnet
object with holding psref and if_put that releases psref
of the ifnet object. The patch replaces ifnet_lock with
if_get/if_put and a simple mutex. The change fixes an
issue that normal ioctls and if_clone_destroy on the
same interface can run in parallel and it causes a panic.

Let me know if I'm missing something.

The patch just demonstrates fixing the issue though,
we have to replace ifunit with if_get and if_put anyway.
And also we need to apply psref to if_byindex as well.

  ozaki-r

On Tue, May 10, 2016 at 12:07 PM, Ryota Ozaki  wrote:
> On Thu, Apr 28, 2016 at 4:04 PM, Ryota Ozaki  wrote:
>> Hi,
>>
>> This proposal is the first step toward making interfaces
>> (struct ifnet) MP-safe; it applies psz and psref to ifnet
>> and ifnet_list to safely look up ifnets from ifnet_list
>> and safely remove an ifnet.
>>
>> Here is a patch:
>>   http://www.netbsd.org/~ozaki-r/psref-ifnet.diff
>
> I restored the original ifnet_list because the change broke
> netstat -i (kvm(3)). So we now maintain two ifnet lists while
> the original list is only added/removed elements and not
> referenced by say IFNET_FOREACH. It's redundant but its overhead
> is negligible and it's the easiest way to keep backward
> compatibility. Of course, ideally we should kill kvm(3) users
> and remove the original list, but it's not now, I think.
>
> Objection?
>
>   ozaki-r
>
>>
>> One concern is m_reclaim that can be run in hardware
>> interrupt and so the patch skips if_drain if it's
>> running in hardware interrupt because pserialize is
>> basically designed used in lwp or softint (and now
>> psref is used with IPL_SOFTNET). Is there a better
>> solution?
>>
>> And any other suggestions and comments would be
>> appreciated.
>>
>> Thanks,
>>   ozaki-r
>>
>> P.S. From today for ten days, I'll be unable to use
>> my main development machines, I wouldn't be able to
>> update and test patches reflected your feedbacks enough.


Re: MP-safe ifnet with psz & psref

2016-05-09 Thread Ryota Ozaki
On Thu, Apr 28, 2016 at 4:04 PM, Ryota Ozaki  wrote:
> Hi,
>
> This proposal is the first step toward making interfaces
> (struct ifnet) MP-safe; it applies psz and psref to ifnet
> and ifnet_list to safely look up ifnets from ifnet_list
> and safely remove an ifnet.
>
> Here is a patch:
>   http://www.netbsd.org/~ozaki-r/psref-ifnet.diff

I restored the original ifnet_list because the change broke
netstat -i (kvm(3)). So we now maintain two ifnet lists while
the original list is only added/removed elements and not
referenced by say IFNET_FOREACH. It's redundant but its overhead
is negligible and it's the easiest way to keep backward
compatibility. Of course, ideally we should kill kvm(3) users
and remove the original list, but it's not now, I think.

Objection?

  ozaki-r

>
> One concern is m_reclaim that can be run in hardware
> interrupt and so the patch skips if_drain if it's
> running in hardware interrupt because pserialize is
> basically designed used in lwp or softint (and now
> psref is used with IPL_SOFTNET). Is there a better
> solution?
>
> And any other suggestions and comments would be
> appreciated.
>
> Thanks,
>   ozaki-r
>
> P.S. From today for ten days, I'll be unable to use
> my main development machines, I wouldn't be able to
> update and test patches reflected your feedbacks enough.


MP-safe ifnet with psz & psref

2016-04-28 Thread Ryota Ozaki
Hi,

This proposal is the first step toward making interfaces
(struct ifnet) MP-safe; it applies psz and psref to ifnet
and ifnet_list to safely look up ifnets from ifnet_list
and safely remove an ifnet.

Here is a patch:
  http://www.netbsd.org/~ozaki-r/psref-ifnet.diff

One concern is m_reclaim that can be run in hardware
interrupt and so the patch skips if_drain if it's
running in hardware interrupt because pserialize is
basically designed used in lwp or softint (and now
psref is used with IPL_SOFTNET). Is there a better
solution?

And any other suggestions and comments would be
appreciated.

Thanks,
  ozaki-r

P.S. From today for ten days, I'll be unable to use
my main development machines, I wouldn't be able to
update and test patches reflected your feedbacks enough.