2021-10-19 06:10:32

by Ian Kent

[permalink] [raw]
Subject: [ANNOUNCE] autofs 5.1.8 release

Hi all,

It's time for a release, autofs-5.1.8.

The bulk of the changes in this release are improvements to the
internal hosts map and related code.

The internal hosts map was performing very poorly when there were a
very large number of exports from a server. This was because the
server exports are turned into an autofs map entry with an offset
for each of the exports and the offset handling code was written
with the assumption the number of offsets is small. After doing
as much optimization as possible with the existing code accessing
a server with 30k exports took more than 45 seconds on initial
access and shutting down autofs would take more than a minute (with
all offsets still present), not ok at all.

Because of need to handle offset mount trees as subtrees the existing
linear list implementation was complicated and hard to maintain so it
was re-written to leverage the recently added (in autofs-5.1.7) binary
tree implementation. The result is much simpler and cleaner and takes
10-15 seconds on initial access and about the same amount of time to
shutdown autofs for a server with 30k exports. That's not quite good
enough for interactive use but it's pretty close and, after all, there
are a "lot" of exports.

There are also quite a few bug fixes and minor improvements.

autofs
======

The package can be found at:
https://www.kernel.org/pub/linux/daemons/autofs/v5/

It is autofs-5.1.8.tar.[gz|xz]

No source rpm is there as it can be produced by using:

rpmbuild -ts autofs-5.1.8.tar.gz

and the binary rpm by using:

rpmbuild -tb autofs-5.1.8.tar.gz

Here are the entries from the CHANGELOG which outline the updates:

- add xdr_exports().
- remove mount.x and rpcgen dependencies.
- dont use realloc in host exports list processing.
- use sprintf() when constructing hosts mapent.
- fix mnts_remove_amdmount() uses wrong list.
- Fix option for master read wait.
- eliminate cache_lookup_offset() usage.
- fix is mounted check on non existent path.
- simplify cache_get_parent().
- set offset parent in update_offset_entry().
- remove redundant variables from mount_autofs_offset().
- remove unused parameter form do_mount_autofs_offset().
- refactor umount_multi_triggers().
- eliminate clean_stale_multi_triggers().
- simplify mount_subtree() mount check.
- fix mnts_get_expire_list() expire list construction.
- fix inconsistent locking in umount_subtree_mounts().
- fix return from umount_subtree_mounts() on offset list delete.
- pass mapent_cache to update_offset_entry().
- fix inconsistent locking in parse_mount().
- remove unused mount offset list lock functions.
- eliminate count_mounts() from expire_proc_indirect().
- eliminate some strlen calls in offset handling.
- don't add offset mounts to mounted mounts table.
- reduce umount EBUSY check delay.
- cleanup cache_delete() a little.
- rename path to m_offset in update_offset_entry().
- don't pass root to do_mount_autofs_offset().
- rename tree implementation functions.
- add some multi-mount macros.
- remove unused functions cache_dump_multi() and cache_dump_cache().
- add a len field to struct autofs_point.
- make tree implementation data independent.
- add mapent tree implementation.
- add tree_mapent_add_node().
- add tree_mapent_delete_offsets().
- add tree_mapent_traverse_subtree().
- fix mount_fullpath().
- add tree_mapent_cleanup_offsets().
- add set_offset_tree_catatonic().
- add mount and umount offsets functions.
- switch to use tree implementation for offsets.
- remove obsolete functions.
- remove redundant local var from sun_mount().
- use mount_fullpath() in one spot in parse_mount().
- pass root length to mount_fullpath().
- remove unused function master_submount_list_empty().
- move amd mounts removal into lib/mounts.c.
- check for offset with no mount location.
- remove mounts_mutex.
- remove unused variable from get_exports().
- add missing free in handle_mounts().
- remove redundant if check.
- fix possible memory leak in master_parse().
- fix possible memory leak in mnts_add_amdmount().
- fix double unlock in parse_mount().
- add length check in umount_subtree_mounts().
- fix flags check in umount_multi().
- dont try umount after stat() ENOENT fail.
- remove redundant assignment in master_add_amd_mount_section_mounts().
- fix dead code in mnts_add_mount().
- fix arg not used in error print.
- fix missing lock release in mount_subtree().
- fix double free in parse_mapent().
- refactor lookup_prune_one_cache() a bit.
- cater for empty mounts list in mnts_get_expire_list().
- add ext_mount_hash_mutex lock helpers.
- fix amd section mounts map reload.
- fix dandling symlink creation if nis support is not available.
- dont use AUTOFS_DEV_IOCTL_CLOSEMOUNT.
- fix lookup_prune_one_cache() refactoring change.
- fix amd hosts mount expire.
- fix offset entries order.
- use mapent tree root for tree_mapent_add_node().
- eliminate redundant cache lookup in tree_mapent_add_node().
- fix hosts map offset order.
- fix direct mount deadlock.
- add missing description of null map option.
- fix nonstrict offset mount fail handling.
- fix concat_options() error handling.
- eliminate some more alloca usage.
- use default stack size for threads.
- fix use of possibly NULL var in lookup_program.c:match_key().
- fix incorrect print format specifiers in get_pkt().
- add mapent path length check in handle_packet_expire_direct().
- add copy length check in umount_autofs_indirect().
- add some buffer length checks to master map parser.
- add buffer length check to rmdir_path().
- eliminate buffer usage from handle_mounts_cleanup().
- add buffer length checks to autofs mount_mount().
- make NFS version check flags consistent.
- refactor get_nfs_info().
- also require TCP_REQUESTED when setting NFS port.

Ian




2022-02-09 23:48:44

by NeilBrown

[permalink] [raw]
Subject: Re: [ANNOUNCE] autofs 5.1.8 release

On Tue, 19 Oct 2021, Ian Kent wrote:
> Hi all,
>
> It's time for a release, autofs-5.1.8.
>
...
> - also require TCP_REQUESTED when setting NFS port.

Unfortunately that last patch is buggy. TCP_REQUESTED is masked out in
the caller.

Maybe the following is best.

NeilBrown

From: NeilBrown <[email protected]>
Subject: [PATCH] Test TCP request correctly in nfs_get_info()

The TCP_REQUESTED flag is masked out by the caller, so it never gets to
nfs_get_info().
We can test if TCP was requested by examining the 'proto' parameter.

Signed-off-by: NeilBrown <[email protected]>

diff --git a/modules/replicated.c b/modules/replicated.c
index 09075dd0c1b4..3ac7ee432e73 100644
--- a/modules/replicated.c
+++ b/modules/replicated.c
@@ -291,7 +291,7 @@ static unsigned int get_nfs_info(unsigned logopt, struct host *host,

rpc_info->proto = proto;
if (port < 0) {
- if ((version & NFS4_REQUESTED) && (version & TCP_REQUESTED))
+ if ((version & NFS4_REQUESTED) && (proto == IPPROTO_TCP))
rpc_info->port = NFS_PORT;
else
port = 0;


2022-02-14 05:53:22

by Ian Kent

[permalink] [raw]
Subject: Re: [ANNOUNCE] autofs 5.1.8 release

On Thu, 2022-02-10 at 08:59 +1100, NeilBrown wrote:
> On Tue, 19 Oct 2021, Ian Kent wrote:
> > Hi all,
> >
> > It's time for a release, autofs-5.1.8.
> >
> ...
> > - also require TCP_REQUESTED when setting NFS port.
>
> Unfortunately that last patch is buggy.  TCP_REQUESTED is masked out
> in
> the caller.

Mmm ... sounds like I've made a mistake there.
I'll need to sort that out, thanks for pointing it out.

>
> Maybe the following is best.
>
> NeilBrown
>
> From: NeilBrown <[email protected]>
> Subject: [PATCH] Test TCP request correctly in nfs_get_info()
>
> The TCP_REQUESTED flag is masked out by the caller, so it never gets
> to
> nfs_get_info().

That wasn't my intent, I'll need to look at it again.
The case I'm trying to cover is fairly specific so I will need to
look at it again.

Ian

> We can test if TCP was requested by examining the 'proto' parameter.
>
> Signed-off-by: NeilBrown <[email protected]>
>
> diff --git a/modules/replicated.c b/modules/replicated.c
> index 09075dd0c1b4..3ac7ee432e73 100644
> --- a/modules/replicated.c
> +++ b/modules/replicated.c
> @@ -291,7 +291,7 @@ static unsigned int get_nfs_info(unsigned logopt,
> struct host *host,
>  
>         rpc_info->proto = proto;
>         if (port < 0) {
> -               if ((version & NFS4_REQUESTED) && (version &
> TCP_REQUESTED))
> +               if ((version & NFS4_REQUESTED) && (proto ==
> IPPROTO_TCP))
>                         rpc_info->port = NFS_PORT;
>                 else
>                         port = 0;
>

2022-02-15 00:08:43

by NeilBrown

[permalink] [raw]
Subject: Re: [ANNOUNCE] autofs 5.1.8 release

On Mon, 14 Feb 2022, Ian Kent wrote:
> On Thu, 2022-02-10 at 08:59 +1100, NeilBrown wrote:
> > On Tue, 19 Oct 2021, Ian Kent wrote:
> > > Hi all,
> > >
> > > It's time for a release, autofs-5.1.8.
> > >
> > ...
> > > - also require TCP_REQUESTED when setting NFS port.
> >
> > Unfortunately that last patch is buggy.  TCP_REQUESTED is masked out
> > in
> > the caller.
>
> Mmm ... sounds like I've made a mistake there.
> I'll need to sort that out, thanks for pointing it out.
>
> >
> > Maybe the following is best.
> >
> > NeilBrown
> >
> > From: NeilBrown <[email protected]>
> > Subject: [PATCH] Test TCP request correctly in nfs_get_info()
> >
> > The TCP_REQUESTED flag is masked out by the caller, so it never gets
> > to
> > nfs_get_info().
>
> That wasn't my intent, I'll need to look at it again.
> The case I'm trying to cover is fairly specific so I will need to
> look at it again.
>

I'm curious: What was the case you were trying to solve?? I couldn't
guess any justification for the change.

Thanks,
NeilBrown

2022-02-15 08:14:47

by Ian Kent

[permalink] [raw]
Subject: Re: [ANNOUNCE] autofs 5.1.8 release

On Tue, 2022-02-15 at 09:46 +1100, NeilBrown wrote:
> On Mon, 14 Feb 2022, Ian Kent wrote:
> > On Thu, 2022-02-10 at 08:59 +1100, NeilBrown wrote:
> > > On Tue, 19 Oct 2021, Ian Kent wrote:
> > > > Hi all,
> > > >
> > > > It's time for a release, autofs-5.1.8.
> > > >
> > > ...
> > > > - also require TCP_REQUESTED when setting NFS port.
> > >
> > > Unfortunately that last patch is buggy.  TCP_REQUESTED is masked
> > > out
> > > in
> > > the caller.
> >
> > Mmm ... sounds like I've made a mistake there.
> > I'll need to sort that out, thanks for pointing it out.
> >
> > >
> > > Maybe the following is best.
> > >
> > > NeilBrown
> > >
> > > From: NeilBrown <[email protected]>
> > > Subject: [PATCH] Test TCP request correctly in nfs_get_info()
> > >
> > > The TCP_REQUESTED flag is masked out by the caller, so it never
> > > gets
> > > to
> > > nfs_get_info().
> >
> > That wasn't my intent, I'll need to look at it again.
> > The case I'm trying to cover is fairly specific so I will need to
> > look at it again.
> >
>
> I'm curious: What was the case you were trying to solve??  I couldn't
> guess any justification for the change.

Somewhere along the way I broke NFSv4 mounts being able to be mounted
without the use of rpcbind.

I require the option fstype=nfs4 for this and if given the map entry
should be mountable without recall to any other services beside NFS.

So that option shouldn't be masked out since it allows automount
to identify (or should) this case.

Ian

2022-02-15 10:26:23

by Stanislav Levin

[permalink] [raw]
Subject: Re: [ANNOUNCE] autofs 5.1.8 release



10.02.2022 00:59, NeilBrown пишет:
> On Tue, 19 Oct 2021, Ian Kent wrote:
>> Hi all,
>>
>> It's time for a release, autofs-5.1.8.
>>
> ...
>> - also require TCP_REQUESTED when setting NFS port.
>
> Unfortunately that last patch is buggy. TCP_REQUESTED is masked out in
> the caller.
>
> Maybe the following is best.
>
> NeilBrown
>
> From: NeilBrown <[email protected]>
> Subject: [PATCH] Test TCP request correctly in nfs_get_info()
>
> The TCP_REQUESTED flag is masked out by the caller, so it never gets to
> nfs_get_info().
> We can test if TCP was requested by examining the 'proto' parameter.
>
> Signed-off-by: NeilBrown <[email protected]>
>
> diff --git a/modules/replicated.c b/modules/replicated.c
> index 09075dd0c1b4..3ac7ee432e73 100644
> --- a/modules/replicated.c
> +++ b/modules/replicated.c
> @@ -291,7 +291,7 @@ static unsigned int get_nfs_info(unsigned logopt, struct host *host,
>
> rpc_info->proto = proto;
> if (port < 0) {
> - if ((version & NFS4_REQUESTED) && (version & TCP_REQUESTED))
> + if ((version & NFS4_REQUESTED) && (proto == IPPROTO_TCP))
> rpc_info->port = NFS_PORT;
> else
> port = 0;
>


This seems duplicate of https://www.spinics.net/lists/autofs/msg02389.html


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2022-02-17 04:40:47

by NeilBrown

[permalink] [raw]
Subject: Re: [ANNOUNCE] autofs 5.1.8 release

On Tue, 15 Feb 2022, Stanislav Levin wrote:
>
>
> This seems duplicate of https://www.spinics.net/lists/autofs/msg02389.html
>

Yes it is - thanks for the link.
I wonder why the proposed fix isn't in git ....

Also, I cannot see that the new NS4_ONLY_REQUESTED is different from the
existing NFS4_VERS_MASK.
They are both set/cleared at exactly the same places.

Thanks,
NeilBrown

2022-02-18 07:08:54

by Ian Kent

[permalink] [raw]
Subject: Re: [ANNOUNCE] autofs 5.1.8 release

On Thu, 2022-02-17 at 13:57 +1100, NeilBrown wrote:
> On Tue, 15 Feb 2022, Stanislav Levin wrote:
> >
> >
> > This seems duplicate of
> > https://www.spinics.net/lists/autofs/msg02389.html
> >
>
> Yes it is - thanks for the link.
> I wonder why the proposed fix isn't in git ....

I think we are refering to commits:
fc4c067b53f7 autofs-5.1.7 - make NFS version check flags consistent
26fb6b5408be autofs-5.1.7 - refactor get_nfs_info()
606795ecfaa1 autofs-5.1.7 - also require TCP_REQUESTED when setting NFS
port

>
> Also, I cannot see that the new NS4_ONLY_REQUESTED is different from
> the
> existing NFS4_VERS_MASK.
> They are both set/cleared at exactly the same places.

Yes they are at the moment.

The aim there is, at some point, to have two seperate cases for NFSv4
mounts, one that may use rpcbind and one that does not such as when
traversing a firewall. Although I'm not clear on it myself the RFC
says, more or less, should (although that might have been must) not
need to use other than the NFS port, so no rpcbind should need to be
used.

Clearly the case seperation hasn't happened yet but I'm pretty sure
the current code did avoid the rpcbind usage for fstype=nfs4 which
is what was asked for at the time and what had been broken somewhere
along the line.

Ian