On Mon, 19 Dec 2022, Rick Macklem wrote:
Hi,
Kostik expressed some concern w.r.t. using a non-default VNET_NFSD kernel
build option and I understand his concern, given that many prefer to use
a GENERIC kernel and binary updates.
yes, I may have hinted towards that (at least in my mind) during looking
at the review.
There is a reason that (at least for now) I do like having it.
Personally (due to lack of time mostly) I haven't figured out if I would
want this to be a vnfs one day or part of vnet. The earlier (like the
kernel option) could more easily address possible security concerns to
people (especially while this is a moving target with more possibly
coming later).
Hence me having asked for a dedicated macro not mangling with the VNET
macros directly in place as this gives us a lot of flexibility to easily
move this around in the future if we wanted to.
Removing the option will make the code simpler.
Right now there are 29 NFS variables VNET_DEFINED() and several of them
are arrays currently sized at 500. One of the reasons for the non-default
VNET_NFSD kernel option was the bloat this caused to the vnet.
(Chris <bsd-li...@bsdforge.com> expressed concern that adding mountd/nfsd
to the vnet would result in bloat/overhead in a previous post to this
thread.)
Another issue with putting all these variables in the vnet is that the
nfsd.ko
cannot be loaded (it complains the vnet is out of space) and, as such,
options NFSD must be used with VNET_NFSD so that the nfsd is linked into the
kernel.
As such, I am wondering what others think of this alternate plan?
- Pull all the VNET_DEFINE()'d variable (except the 3 manipulated by
sysctls)
into a structure.
- Define a single VNET_DEFINE()'d variable that is a pointer to that
structure
and then malloc() the structure in the function called by VNET_SYSINIT().
This would result in a malloc'd structure for each vnet jail (for kernels
built with VMIMAGE), but would only add 4 variables to the vnet.
If a small C file that only consists of the VNET_DEFINE()s for the 4
variables
is linked into the kernel whenever the VIMAGE option is specified, then I
think nfsd.ko would be loadable.
It is not the number of variables added that is a problem; it is (as you
point out above) their size which is a problem.
So 500 uint8_t variables are as expensive as 1 uint8_t[500];
There's only a "small" allocated per-vnet to replicate state for
modules.
Multicast also had that problem needing huge junks and we eventually
switched to malloc to fix this and make the module loadable as well
again. (See 1a117215c7f90e6ef8c50ef3bfe099490aaa98f9 for one of the
changes -- I think I scrwed somehting up there about sizing so probably
follow-ups but it'll show the concept).
So wether you make this one huge malloc or multiple small ones for the
few big variables is up to you in the end. I also wonder what is easier
to deal with "vnet0/prison0/'nfs0-bits'" as NFS_ROOT and other parts
will need some of these things eventually to be in place early one for
the base.
Also sysctl and malloc and virtualisation can be a bit tricky.
The "common" theme would be to only malloc the complex data types but
leave the simple type variables being normal virtualised ones.
Unfortunately, this does not deal with vnet'ng the kgssapi, rpcsec_gss for
Kerberized mounts or vnet'ng NFS-over-TLS, but those could be handled in a
similar manner, I think?
Could be, yes.
So, what do others think of this alternate plan?
rick
ps: Every use of the vnet'd variables is currently wrapped in a macro called
NFSD_VNET(), so the change is pretty easy to do by just re-writing this
macro.
--
Bjoern A. Zeeb r15:7
