Received-SPF: pass (google.com: domain of linux-nfs+bounces-3244-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
MIME-Version: 1.0
From: "NeilBrown" <neilb@suse.de>
To: "James Pearson" <jcpearson@gmail.com>
Cc: "Linux NFS Mailing List" <linux-nfs@vger.kernel.org>
Subject: Re: Changing the precedence order of client exports in /etc/exports
In-reply-to:
 <CAK3fRr9kngizdGsAU8NpzBjpPSh4k6-23wpwfpZUHJeEoU7yVg@mail.gmail.com>
References:
 <>, <CAK3fRr9kngizdGsAU8NpzBjpPSh4k6-23wpwfpZUHJeEoU7yVg@mail.gmail.com>
Date: Mon, 13 May 2024 13:52:17 +1000
Message-id: <171557233775.4857.16486910988559286216@noble.neil.brown.name>

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=3Dlinux-nfs&m=3D130565040627856&w=3D2) - and answe=
r 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=3D'
> > > 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 =3D 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.
>=20
> Thanks for the pointers - new patch below
>=20
> I don't quite understand what export_find() is actually doing ?
>=20
> 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.

Thanks,
NeilBrown


>=20
> Thanks
>=20
> James Pearson
>=20
> 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 *calle=
r,
>                            const char *path, struct addrinfo *ai,
>                            enum auth_error *error)
>  {
> -       nfs_export *exp;
> +       nfs_export *exp, *found;
>         int i;
>=20
>         free(my_client.m_hostname);
> @@ -189,6 +189,7 @@ auth_authenticate_newcache(const struct sockaddr *calle=
r,
>         my_exp.m_client =3D &my_client;
>=20
>         exp =3D NULL;
> +       found =3D NULL;
>         for (i =3D 0; !exp && i < MCL_MAXTYPES; i++)
>                 for (exp =3D exportlist[i].p_head; exp; exp =3D exp->m_next=
) {
>                         if (strcmp(path, exp->m_export.e_path))
> @@ -198,8 +199,11 @@ auth_authenticate_newcache(const struct sockaddr *call=
er,
>                         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 =3D exp;
>                 }
> +       exp =3D found;
>         *error =3D 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 =3D &exp->m_export;
> +                                       free(found_path);
> +                                       found_path =3D strdup(path);
> +                                       if (found_path =3D=3D NULL)
> +                                               goto out;
> +                               }
>                                 /* same path, if one is V4ROOT, choose
> the other */
>                                 if (found->e_flags & NFSEXP_V4ROOT) {
>                                         found =3D &exp->m_export;
> @@ -1178,6 +1186,12 @@ lookup_export(char *dom, char *path, struct addrinfo=
 *ai)
>                                 found_type =3D i;
>                                 continue;
>                         }
> +                       /* see if this one has a higher export priority */
> +                       if (exp->m_export.e_priority >
> found->m_export.e_priority) {
> +                               found =3D exp;
> +                               found_type =3D 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;
>  };
>=20
>  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 =3D NULL;
>         ee->e_ttl =3D default_ttl;
>         ee->e_reexport =3D REEXP_NONE;
> +       ee->e_priority =3D 0;
>  }
>=20
>  struct exportent *
> @@ -374,6 +375,9 @@ putexportent(struct exportent *ep)
>                                 fprintf(fp, "%d,", id[i]);
>         }
>         fprintf(fp, "anonuid=3D%d,anongid=3D%d", ep->e_anonuid, ep->e_anong=
id);
> +       if (ep->e_priority) {
> +               fprintf(fp, ",priority=3D%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);
>=20
>                         saw_reexport =3D 1;
> +               } else if (strncmp(opt, "priority=3D", 9) =3D=3D 0) {
> +                       char *oe;
> +                       ep->e_priority =3D strtol(opt+9, &oe, 10);
> +                       if (opt[9]=3D=3D'\0' || *oe !=3D '\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 =3D dumpopt(c, "priority=3D%d", ep->e_pri=
ority);
>                         secinfo_show(stdout, ep);
>                         xprtsecinfo_show(stdout, ep);
>                         printf("%c\n", (c !=3D '(')? ')' : ' ');
>=20