Date: Mon, 21 May 2018 09:47:34 +0200 From: Marko Zec <zec@fer.hr> To: Matt Macy <mmacy@freebsd.org> Cc: <src-committers@freebsd.org>, <svn-src-all@freebsd.org>, <svn-src-head@freebsd.org> Subject: Re: svn commit: r333967 - head/sys/netinet Message-ID: <20180521094734.3270cccb@x23.koncar-institut.local> In-Reply-To: <201805210712.w4L7C62h081191@repo.freebsd.org> References: <201805210712.w4L7C62h081191@repo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, 21 May 2018 07:12:06 +0000 Matt Macy <mmacy@freebsd.org> wrote: > Author: mmacy > Date: Mon May 21 07:12:06 2018 > New Revision: 333967 > URL: https://svnweb.freebsd.org/changeset/base/333967 > > Log: > ensure that vnet is set when doing in_leavegroup > > Modified: > head/sys/netinet/in_mcast.c > > Modified: head/sys/netinet/in_mcast.c > ============================================================================== > --- head/sys/netinet/in_mcast.c Mon May 21 05:20:23 > 2018 (r333966) +++ head/sys/netinet/in_mcast.c Mon May > 21 07:12:06 2018 (r333967) @@ -1664,6 +1664,8 @@ > inp_gcmoptions(epoch_context_t ctx) { > struct ip_moptions *imo; > struct in_mfilter *imf; > + struct in_multi *inm; > + struct ifnet *ifp; > size_t idx, nmships; > > imo = __containerof(ctx, struct ip_moptions, imo_epoch_ctx); > @@ -1673,7 +1675,13 @@ inp_gcmoptions(epoch_context_t ctx) > imf = imo->imo_mfilters ? &imo->imo_mfilters[idx] : > NULL; if (imf) > imf_leave(imf); > - (void)in_leavegroup(imo->imo_membership[idx], imf); > + inm = imo->imo_membership[idx]; > + ifp = inm->inm_ifp; > + if (ifp) > + CURVNET_SET(ifp->if_vnet); Unfortunately, this won't work because CURVNET_SET() expands to a sequence of declarations and assignments which are NOT enclosed in a single block. Instead, only the first statement in CURVNET_SET() sequence, which is an assert, will be executed conditionally only if ifp != NULL, while the rest of the CURVNET_SET() body will fall out of the scope of the if (ifp) conditional. I'd recommend backing out this patch, and instead extending the struct ip_moptions with an struct vnet * entry, which would be populated before scheduling inp_gcmoptions(). Then CURVNET_SET(imo->imo_vnet) could be called only once (and unconditionally) in gcmoptions(), instead of (attempts at) doing this multiple times in a for loop. Marko > + (void)in_leavegroup(inm, imf); > + if (ifp) > + CURVNET_RESTORE(); > if (imf) > imf_purge(imf); > } >
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180521094734.3270cccb>