svn commit: r333967 - head/sys/netinet

[Marko Zec]

On Mon, 21 May 2018 07:12:06 +0000
Matt Macy <mmacy at 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);
>  	}
>