Skip site navigation (1)Skip section navigation (2)
Date:      Sat, 27 Jun 2009 14:14:09 +0200
From:      Marko Zec <zec@freebsd.org>
To:        Julian Elischer <julian@elischer.org>
Cc:        Perforce Change Reviews <perforce@freebsd.org>
Subject:   Re: PERFORCE change 165251 for review
Message-ID:  <200906271414.09094.zec@freebsd.org>
In-Reply-To: <4A4541F5.1050301@elischer.org>
References:  <200906261413.n5QED9j7023013@repoman.freebsd.org> <4A4541F5.1050301@elischer.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Friday 26 June 2009 23:47:33 Julian Elischer wrote:
> Marko Zec wrote:
> > http://perforce.freebsd.org/chv.cgi?CH=165251
> >
> > Change 165251 by zec@zec_amdx4 on 2009/06/26 14:13:00
> >
> > 	Allow for rpc.statd and rpc.lockd to be started, but
> > 	without doing any functional testing.  Introduce a lot
> > 	curvnet recursions triggered by the above daemons that
> > 	have to be looked into and resolved.
> >
> > Affected files ...
> >
> > .. //depot/projects/vimage-commit2/src/sys/rpc/clnt_dg.c#6 edit
> > .. //depot/projects/vimage-commit2/src/sys/rpc/svc_dg.c#4 edit
> >
> > Differences ...
> >
> > ==== //depot/projects/vimage-commit2/src/sys/rpc/clnt_dg.c#6 (text+ko)
> > ====
> >
> > @@ -56,6 +56,7 @@
> >  #include <sys/socketvar.h>
> >  #include <sys/time.h>
> >  #include <sys/uio.h>
> > +#include <sys/vimage.h>
> >
> >  #include <rpc/rpc.h>
> >  #include <rpc/rpc_com.h>
> > @@ -197,11 +198,14 @@
> >  		return (NULL);
> >  	}
> >
> > +	CURVNET_SET(so->so_vnet);
> >  	if (!__rpc_socket2sockinfo(so, &si)) {
> >  		rpc_createerr.cf_stat = RPC_TLIERROR;
> >  		rpc_createerr.cf_error.re_errno = 0;
> > +		CURVNET_RESTORE();
> >  		return (NULL);
> >  	}
> > +	CURVNET_RESTORE();
> >
> >  	/*
> >  	 * Find the receive and the send size
> >
> > ==== //depot/projects/vimage-commit2/src/sys/rpc/svc_dg.c#4 (text+ko)
> > ====
> >
> > @@ -56,6 +56,7 @@
> >  #include <sys/sx.h>
> >  #include <sys/systm.h>
> >  #include <sys/uio.h>
> > +#include <sys/vimage.h>
> >
> >  #include <rpc/rpc.h>
> >
> > @@ -101,8 +102,10 @@
> >  	struct sockaddr* sa;
> >  	int error;
> >
> > +	CURVNET_SET(so->so_vnet);
> >  	if (!__rpc_socket2sockinfo(so, &si)) {
> >  		printf(svc_dg_str, svc_dg_err1);
> > +		CURVNET_RESTORE();
> >  		return (NULL);
> >  	}
> >  	/*
> > @@ -112,6 +115,7 @@
> >  	recvsize = __rpc_get_t_size(si.si_af, si.si_proto, (int)recvsize);
> >  	if ((sendsize == 0) || (recvsize == 0)) {
> >  		printf(svc_dg_str, svc_dg_err2);
> > +		CURVNET_RESTORE();
> >  		return (NULL);
> >  	}
> >
> > @@ -142,6 +146,7 @@
> >  	if (xprt) {
> >  		svc_xprt_free(xprt);
> >  	}
> > +	CURVNET_RESTORE();
> >  	return (NULL);
> >  }
>
> while leaving all your virtualization clues in place can we make it so
> that the nfs code always works on vnet0?
> I putr it to you that NFS itself should be virtualized as a separate
> major group than vnet.. but until that is done, use vnet0.

Well in this particular case using CURVNET_SET(vnet0) instead of 
CURVNET_SET(so->so_vnet) would be wrong, because so->so_vnet has already been 
set, possibly to vnet0, possibly to another vnet.

I think that we should use some other mechanism to (if we want to) enforce 
that NFS export / mount capabilities are available only to vnet0, possibly 
using priv().  I.e. if we'd allow for mount_nfs to be executed from within a 
non-default vnet, while hardcoding NFS to always work on vnet0, this would be 
clearly more dangerous than the current model that doesn't do any special 
casing re. in which vnet does a particular NFS mount / export exist.

Marko




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200906271414.09094.zec>