From: James Pearson <jcpearson@gmail.com>
To: NeilBrown <neilb@suse.de>
Cc: Linux NFS Mailing List <linux-nfs@vger.kernel.org>
Subject: Re: Changing the precedence order of client exports in /etc/exports
Date: Wed, 15 May 2024 16:22:43 +0100 [thread overview]
Message-ID: <CAK3fRr-vzHXwmnyztvAfVN3twO0ruWf5dtY5fPrWvr5qqsXQcg@mail.gmail.com> (raw)
In-Reply-To: <171557233775.4857.16486910988559286216@noble.neil.brown.name>
On Mon, 13 May 2024 at 04:52, NeilBrown <neilb@suse.de> wrote:
>
> On Sat, 27 Apr 2024, James Pearson wrote:
> > On Fri, 26 Apr 2024 at 05:43, NeilBrown <neilb@suse.de> wrote:
> > >
> > > On Thu, 25 Apr 2024, James Pearson wrote:
> > > > Many years ago, I asked on this list if it was possible to change the
> > > > precedence order of exports listed in /etc/exports where there is more
> > > > than one possible match (see the thread at:
> > > > https://marc.info/?l=linux-nfs&m=130565040627856&w=2) - and answer was
> > > > 'No'
> > > >
> > > > At that time, I used a simple hack to force the precedence order I
> > > > required (by modifying the 'MCL' enum order in nfs-utils
> > > > support/include/exportfs.h)
> > > >
> > > > However, the issue has come up again for me, so I thought I would see
> > > > if I could alter the precedence order by adding an exports 'priority='
> > > > option as suggested later in the above thread
> > > >
> > > > I started with the nfs-utils supplied with CentOS 7 (based on 1.3.0) -
> > > > and added logic to lookup_export() to check for client specifications
> > > > with a higher priority - but this didn't work - so looking for other
> > > > places that looped through MCL types, I added similar logic in
> > > > nfsd_fh() - which seems to work as I expected (I'm using NFSv3)
> > > >
> > > > However, adding similar logic to the nfs-utils supplied with Rocky 9
> > > > (based on 2.5.4) didn't work ...
> > > >
> > > > But comparing the code in nfsd_fh() in v1.3.0 and nfsd_handle_fh() in
> > > > v2.5.4, nfsd_fh() in v1.3.0 does the following towards the end of the
> > > > function - whereas nfsd_handle_fh() in v2.5.4 doesn't:
> > > >
> > > > if (cache_export_ent(buf, sizeof(buf), dom, found, found_path) < 0)
> > > > found = 0;
> > > >
> > > > By adding the above lines at a similar place in nfsd_handle_fh() in
> > > > v2.5.4, seems to 'fix' the issue and all works as I expected
> > > >
> > > > I don't fully understand what is going on under the hood with all
> > > > this, so no idea if what I've done is 'correct', or if there is a
> > > > better way of doing what I'm trying to achieve ?
> > > >
> > > > Below is a patch (made against the latest git nfs-utils) of what I've
> > > > done - could anyone let me know if I'm going along the right lines (or
> > > > not) ?
> > >
> > > The restored cache_export_ent() call has to go.
> > > You need to update init_exportent() to initialise the new field.
> > > You probably need to make some changes to auth_authenticate_newcache().
> > > Probably let the loop run all the way to MCL_MAXTYPES, and do a priority
> > > comparison if you find a new possible match.
> > > export_find() probably need some attention too.
> > >
> > > If you it still doesn't work after addressing those, I'll have a look
> > > and see if I can beat it into shape.
> >
> > Thanks for the pointers - new patch below
> >
> > I don't quite understand what export_find() is actually doing ?
> >
> > As far as I can tell, it is only used by exportfs when an export is
> > given on the command line - and only if that export is of type
> > MCL_FQDN - so I'm not sure it needs any changes to support these
> > priority additions ? (I might be completely wrong here ...)
>
> Sorry for the delay - been busy with other things.
>
> If you run
> exportfs -o options host:/path
>
> and /path is already exported to host via some netgroup or wildcard or
> similar, then exportfs will load the options for that other export,
> add in the "options" specified with -o, and then create a new export for
> just the host with the combined options.
>
> So this should use the same priority ordering as any other code.
>
>
> Is this patch now working for you? If so: great. We can talk about man
> page updates etc.
> If not, please tell me exactly how you are using it (e.g. /etc/exports
> contents) and I'll try to reproduce and see what happens.
Yes, my patch is working as expected for me - I've been testing by
doing something like the following:
/path exported read-only to subnet 10.64.0.0/16 and read-write to
*.web.example.com
As the subnet export is matched before the wildcard match, then any
client in the 10.64.0.0/16 subnet that also matches the wildcard won't
get write access
With the patch, I have /etc/exports containing:
/path 10.64.0.0/16(ro) *.web.example.com(rw,priority=100)
and then a client with an IP in the 10.64.0.0/16 subnet and a host
name in *.web.example.com does get write access (tested by simply
using touch to create a new file under the mount point on the client -
using both NFSv3 and NFSv4 mounts)
exportfs -v reports:
/path 10.64.0.0/16(sync,wdelay,hide,no_subtree_check,sec=sys,ro,secure,root_squash,no_all_squash)
/path *.web.example.com(sync,wdelay,hide,no_subtree_check,priority=100,sec=sys,rw,secure,root_squash,no_all_squash)
I can do the same with an empty /etc/exports file and explicitly
setting exports via exportfs (with and without priority settings etc)
As yet, I haven't come across any combinations of using the priority
option that didn't do as I expected e.g. adding or modifying exports
from the cmdline with or without exports in /etc/exports
I've also added netgroup and hostname exports to the mix - e.g.
exporting to a single hostname with no_root_squash in the
web.example.com domain:
exportfs -o rw,no_root_squash host.web.example.com:/path
In this case, the client doesn't get no_root_squash access to the
mount point - as its priority (0) is less than the domain match for
*.web.example.com (priority=100) - re-running the same exportfs with a
priority higher than 100 and the client gets no_root_squash access
So, I don't think any changes are needed to export_find() ?
If this is OK, I'll submit a patch, including a suitable update to the
export man page
Thanks
James Pearson
> > diff --git a/support/export/auth.c b/support/export/auth.c
> > index 2d7960f1..3d9e07b5 100644
> > --- a/support/export/auth.c
> > +++ b/support/export/auth.c
> > @@ -175,7 +175,7 @@ auth_authenticate_newcache(const struct sockaddr *caller,
> > const char *path, struct addrinfo *ai,
> > enum auth_error *error)
> > {
> > - nfs_export *exp;
> > + nfs_export *exp, *found;
> > int i;
> >
> > free(my_client.m_hostname);
> > @@ -189,6 +189,7 @@ auth_authenticate_newcache(const struct sockaddr *caller,
> > my_exp.m_client = &my_client;
> >
> > exp = NULL;
> > + found = NULL;
> > for (i = 0; !exp && i < MCL_MAXTYPES; i++)
> > for (exp = exportlist[i].p_head; exp; exp = exp->m_next) {
> > if (strcmp(path, exp->m_export.e_path))
> > @@ -198,8 +199,11 @@ auth_authenticate_newcache(const struct sockaddr *caller,
> > if (exp->m_export.e_flags & NFSEXP_V4ROOT)
> > /* not acceptable for v[23] export */
> > continue;
> > - break;
> > + /* we have a match - see if it is a higher priority */
> > + if (!found || exp->m_export.e_priority >
> > found->m_export.e_priority)
> > + found = exp;
> > }
> > + exp = found;
> > *error = not_exported;
> > if (!exp)
> > return NULL;
> > diff --git a/support/export/cache.c b/support/export/cache.c
> > index 6c0a44a3..dfb0051b 100644
> > --- a/support/export/cache.c
> > +++ b/support/export/cache.c
> > @@ -877,6 +877,14 @@ static int nfsd_handle_fh(int f, char *bp, int blen)
> > xlog(L_WARNING, "%s and %s have same
> > filehandle for %s, using first",
> > found_path, path, dom);
> > } else {
> > + /* same path, see if this one has a
> > higher export priority */
> > + if (exp->m_export.e_priority >
> > found->e_priority) {
> > + found = &exp->m_export;
> > + free(found_path);
> > + found_path = strdup(path);
> > + if (found_path == NULL)
> > + goto out;
> > + }
> > /* same path, if one is V4ROOT, choose
> > the other */
> > if (found->e_flags & NFSEXP_V4ROOT) {
> > found = &exp->m_export;
> > @@ -1178,6 +1186,12 @@ lookup_export(char *dom, char *path, struct addrinfo *ai)
> > found_type = i;
> > continue;
> > }
> > + /* see if this one has a higher export priority */
> > + if (exp->m_export.e_priority >
> > found->m_export.e_priority) {
> > + found = exp;
> > + found_type = i;
> > + continue;
> > + }
> > /* Always prefer non-V4ROOT exports */
> > if (exp->m_export.e_flags & NFSEXP_V4ROOT)
> > continue;
> > diff --git a/support/include/nfslib.h b/support/include/nfslib.h
> > index eff2a486..ab22ecaf 100644
> > --- a/support/include/nfslib.h
> > +++ b/support/include/nfslib.h
> > @@ -99,6 +99,7 @@ struct exportent {
> > unsigned int e_ttl;
> > char * e_realpath;
> > int e_reexport;
> > + int e_priority;
> > };
> >
> > struct rmtabent {
> > diff --git a/support/nfs/exports.c b/support/nfs/exports.c
> > index a6816e60..afc139db 100644
> > --- a/support/nfs/exports.c
> > +++ b/support/nfs/exports.c
> > @@ -106,6 +106,7 @@ static void init_exportent (struct exportent *ee,
> > int fromkernel)
> > ee->e_uuid = NULL;
> > ee->e_ttl = default_ttl;
> > ee->e_reexport = REEXP_NONE;
> > + ee->e_priority = 0;
> > }
> >
> > struct exportent *
> > @@ -374,6 +375,9 @@ putexportent(struct exportent *ep)
> > fprintf(fp, "%d,", id[i]);
> > }
> > fprintf(fp, "anonuid=%d,anongid=%d", ep->e_anonuid, ep->e_anongid);
> > + if (ep->e_priority) {
> > + fprintf(fp, ",priority=%d", ep->e_priority);
> > + }
> > secinfo_show(fp, ep);
> > xprtsecinfo_show(fp, ep);
> > fprintf(fp, ")\n");
> > @@ -834,6 +838,14 @@ bad_option:
> > setflags(NFSEXP_FSID, active, ep);
> >
> > saw_reexport = 1;
> > + } else if (strncmp(opt, "priority=", 9) == 0) {
> > + char *oe;
> > + ep->e_priority = strtol(opt+9, &oe, 10);
> > + if (opt[9]=='\0' || *oe != '\0') {
> > + xlog(L_ERROR, "%s: %d: bad priority \"%s\"\n",
> > + flname, flline, opt);
> > + goto bad_option;
> > + }
> > } else {
> > xlog(L_ERROR, "%s:%d: unknown keyword \"%s\"\n",
> > flname, flline, opt);
> > diff --git a/utils/exportfs/exportfs.c b/utils/exportfs/exportfs.c
> > index b03a047b..5e6a64b6 100644
> > --- a/utils/exportfs/exportfs.c
> > +++ b/utils/exportfs/exportfs.c
> > @@ -753,6 +753,8 @@ dump(int verbose, int export_format)
> > break;
> > #endif
> > }
> > + if (ep->e_priority)
> > + c = dumpopt(c, "priority=%d", ep->e_priority);
> > secinfo_show(stdout, ep);
> > xprtsecinfo_show(stdout, ep);
> > printf("%c\n", (c != '(')? ')' : ' ');
> >
>
prev parent reply other threads:[~2024-05-15 15:22 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <662a39f7.BuSJ6zaMPTMaMa7L%james-p@moving-picture.com>
2024-04-25 12:19 ` Changing the precedence order of client exports in /etc/exports James Pearson
2024-04-26 4:43 ` NeilBrown
2024-04-26 16:08 ` James Pearson
2024-05-08 9:49 ` James Pearson
2024-05-10 19:02 ` Steve Dickson
2024-05-13 3:52 ` NeilBrown
2024-05-15 15:22 ` James Pearson [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CAK3fRr-vzHXwmnyztvAfVN3twO0ruWf5dtY5fPrWvr5qqsXQcg@mail.gmail.com \
--to=jcpearson@gmail.com \
--cc=linux-nfs@vger.kernel.org \
--cc=neilb@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).