On 15/06/21(Tue) 22:52, David Gwynne wrote:
> On Mon, Jun 14, 2021 at 10:07:58AM +0200, Martin Pieuchot wrote:
> > On 10/06/21(Thu) 19:17, Alexander Bluhm wrote:
> [...]
> > > The in6_ functions need netlock. And driver SIOCSIFFLAGS ioctl
> > > must not have splnet().
> >
> > Why not? This is new since the introduction of intr_barrier() or this
> > is an old issue?
> >
> > > Is reducing splnet() the correct aproach?
>
> yes.
>
> > I doubt it is possible to answer this question without defining who owns
> > `if_flags' and how can it be read/written to.
>
> NET_LOCK is what "owns" updates to if_flags.
Why does reducing splnet() is the correct approach? It isn't clear to
me. What's splnet() protecting then?
> > > Index: net/if.c
> > > ===================================================================
> > > RCS file: /data/mirror/openbsd/cvs/src/sys/net/if.c,v
> > > retrieving revision 1.641
> > > diff -u -p -r1.641 if.c
> > > --- net/if.c 25 May 2021 22:45:09 -0000 1.641
> > > +++ net/if.c 10 Jun 2021 14:32:12 -0000
> > > @@ -3109,6 +3109,8 @@ ifnewlladdr(struct ifnet *ifp)
> > > short up;
> > > int s;
> > >
> > > + NET_ASSERT_LOCKED();
> > > +
> > > s = splnet();
> > > up = ifp->if_flags & IFF_UP;
> > >
> > > @@ -3116,11 +3118,14 @@ ifnewlladdr(struct ifnet *ifp)
> > > /* go down for a moment... */
> > > ifp->if_flags &= ~IFF_UP;
> > > ifrq.ifr_flags = ifp->if_flags;
> > > + splx(s);
> > > (*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t)&ifrq);
> > > + s = splnet();
> > > }
> > >
> > > ifp->if_flags |= IFF_UP;
> > > ifrq.ifr_flags = ifp->if_flags;
> > > + splx(s);
> > > (*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t)&ifrq);
> > >
> > > #ifdef INET6
> > > @@ -3139,11 +3144,12 @@ ifnewlladdr(struct ifnet *ifp)
> > > #endif
> > > if (!up) {
> > > /* go back down */
> > > + s = splnet();
> > > ifp->if_flags &= ~IFF_UP;
> > > ifrq.ifr_flags = ifp->if_flags;
> > > + splx(s);
> > > (*ifp->if_ioctl)(ifp, SIOCSIFFLAGS, (caddr_t)&ifrq);
> > > }
> > > - splx(s);
> > > }
> > >
> > > void
> > >
> >
>
