On Tue, Nov 28, 2017 at 6:40 PM, Ryota Ozaki <ozaki.ry...@gmail.com> wrote: > On Mon, Nov 27, 2017 at 12:24 AM, Nick Hudson <sk...@netbsd.org> wrote: >> On 11/17/17 07:44, Ryota Ozaki wrote: >>> >>> On Wed, Nov 15, 2017 at 1:53 PM, Ryota Ozaki <ozaki.ry...@gmail.com> >>> wrote: >>>> >>>> On Fri, Nov 10, 2017 at 6:35 PM, Ryota Ozaki <ozaki.ry...@gmail.com> >>>> wrote: >>>>> >>>>> Hi, >>>>> >>>>> http://www.netbsd.org/~ozaki-r/IFEF_MPSAFE.diff >>>>> >>>>> I'm going to commit the above change that integrates >>>>> IFEF_OUTPUT_MPSAFE and IFEF_START_MPSAFE flags into >>>>> IFEF_MPSAFE. >>>>> >>>>> The motivation is to not waste if_extflags bits. I'm now >>>>> trying to make if_ioctl() hold KERNEL_LOCK selectively >>>>> for some reasons as well as if_start() and if_output(). >>>> >>>> BTW this is a patch for this plan: >>>> http://www.netbsd.org/~ozaki-r/if_ioctl-no-KERNEL_LOCK.diff >>>> >>>> It removes KERNEL_LOCK for if_ioctl from soo_ioctl and >>>> selectively takes it in doifioctl. To this end, some fine-grain >>>> KERNEL_LOCKs have to be added where calling components/functions >>>> that aren't MP-safe. >>> >>> Revised rebased on latest NET_MPSAFE macros. The new patch also provides >>> similar macros: >>> http://www.netbsd.org/~ozaki-r/if_ioctl-no-KERNEL_LOCK.revised.diff >>> >>> ozaki-r >>> >> Hi, > > (Sorry for late replying. I was sick in bed for days...) > >> >> Can you document the protection requirements of the struct ifnet members, >> e.g. if_flags. >> >> Ideally by annotating it like struct proc >> >> http://src.illumos.org/source/xref/netbsd-src/sys/sys/proc.h#230 > > OK. I'm trying to add annotations like that.
Some more changes are needed though, I fixed some race conditions on ifnet and wrote the lock requirements of ifnet: http://www.netbsd.org/~ozaki-r/ifnet-locks.diff There remain some unsafe members. Those for carp, agr and pf should be fixed when making them MP-safe. Another remainder is a set of statistic counters, which will be MP-safe (by making them per-CPU counters) sooner or later. if_flags should be modified with holding if_ioct_lock. If an interface enables IEFE_MPSAFE, the interface must meet the contract, which enforces the interface to update if_flags only in XXX_ioctl and also the interface has to give up using IFF_OACTIVE flag which doesn't suit for multi-core systems and hardware multi-queue NICs. I'm not sure yet we have to do synchronization between updates and reads on if_flags. (Anyway we should NOT do that because the synchronization will hurt performance.) Thanks, ozaki-r