proposal: nsswitch refactoring

proposal: nsswitch refactoring

[DJ Delorie]

Red Hat has had a long-standing request to make nsswitch.conf
reloadable.  It's come up again recently, so I'm looking to see what
it would take.

After discussing this with Carlos a bit, I've broken the task into
three phases.  I'd like some feedback on what the community (i.e. us)
thinks about my general direction on this, so I don't waste a lot of
time.

First, the data relating to the various shared objects that implement
the name services (libnss_foo.so) needs to be split out of the data
specific to nsswitch.conf's layout.  Why?  Because we still never
unload shared objects, so that data is effectively "write once" and
should never change.  Separating it from the changable data makes
sense.

Second, the nsswitch-specific data should be "compartmentalized"
somehow, so that we can reference count it, set aside the "old" copy
when we load a "new" copy, and clean it up after we know that there
are no further users of that data.

There are two schools of thought here - one memory chunk with all the
data in it, or spread out data with careful management of it.  One
chunk is easier to free up (just call free()) but has some overhead in
packing the data into it.  Spread out data is easier to understand and
manipulate, but you have to be more careful about cleanup.

The third phase is to consider expanding the syntax and functionality
of nsswitch.conf.  My "vision" on this is to compile the nsswitch
logic into a tiny p-code, and centralize running it.  That way, an API
(like getpwnam()) need only set up some data (query arguments, etc),
and pass said data and a callback function to this central logic
handler, which calls the callback when it decides which shared object
needs to be queried next.  This p-code could include new actions like
two-way branching, more interesting error handlers, etc.

Note: I had considered at one point allowing nsswitch to choose
handlers based on the query string itself, but feedback seems to be
that *that* kind of logic should be, itself, in a shared object
handler.

I don't know what other kinds of "smarts" should be in nsswitch.conf
itself, but I'm trying to consider future unknown additions :-)

Thoughts?

Re: proposal: nsswitch refactoring

[Florian Weimer]

* DJ Delorie:

> First, the data relating to the various shared objects that implement
> the name services (libnss_foo.so) needs to be split out of the data
> specific to nsswitch.conf's layout.  Why?  Because we still never
> unload shared objects, so that data is effectively "write once" and
> should never change.  Separating it from the changable data makes
> sense.

Right, unloading already-loaded modules would be the wrong thing to do
because dlclose could be problematic for NSS service modules.

We currently bind lookup functions lazily, so the module data is not
entirely write-once, more like a collection of ivars (called SyncVar
in Concurrent ML, basically futures without blocking).

One tricky aspect about the current framework is that the generic NSS
code does not know which lookup functions exist.  This is handled by
specific lookup implementations.  (For example, only the getaddrinfo
implementation and some of the NSS modules know about
“gethostbyname4_r” lookup functions, the rest of glibc does not care.)
We should probably change that because having a static list of
potentially available functions simplifies memory management.

> Second, the nsswitch-specific data should be "compartmentalized"
> somehow, so that we can reference count it, set aside the "old" copy
> when we load a "new" copy, and clean it up after we know that there
> are no further users of that data.

We already do this for the resolver configuration.  We could probably
reuse the same mechanism, and even the same data structure.

> There are two schools of thought here - one memory chunk with all the
> data in it, or spread out data with careful management of it.  One
> chunk is easier to free up (just call free()) but has some overhead in
> packing the data into it.  Spread out data is easier to understand and
> manipulate, but you have to be more careful about cleanup.

I don't think there is much choice unless we statically encode the set
of supported lookup functions.

> The third phase is to consider expanding the syntax and functionality
> of nsswitch.conf.  My "vision" on this is to compile the nsswitch
> logic into a tiny p-code, and centralize running it.  That way, an API
> (like getpwnam()) need only set up some data (query arguments, etc),
> and pass said data and a callback function to this central logic
> handler, which calls the callback when it decides which shared object
> needs to be queried next.  This p-code could include new actions like
> two-way branching, more interesting error handlers, etc.

Which aspect are you referring to?  The handling of struct pwd, struct
hostend, etc., including parsing file data?  Or forwarding a lookup
operation from a function such as getpwnam to an NSS module function
like _nss_files_getpwnam_r?

We currently generate code for this in glibc at compile time, via the
preprocessor, for both.  (But nscd has a separate mechanism for struct
marshalling.)  This seems independent from the other changes.

Re: proposal: nsswitch refactoring

[DJ Delorie]

Florian Weimer <fw@deneb.enyo.de> writes:
> We currently bind lookup functions lazily, so the module data is not
> entirely write-once, more like a collection of ivars (called SyncVar
> in Concurrent ML, basically futures without blocking).

But once we bind those and record their addresses, they won't change, so
I put them in the "write once" group.  The rest of the data needs to be
write-flush-rewrite capable, in order to unload nsswitch.conf data and
reload it.

> One tricky aspect about the current framework is that the generic NSS
> code does not know which lookup functions exist.

I had noted that, and figured some sort of cache/hash/whatever would be
needed somewhere, but elided the problem for now :-)

> We already do this for the resolver configuration.  We could probably
> reuse the same mechanism, and even the same data structure.

Thanks, I'll look at it.

> Which aspect are you referring to?  The handling of struct pwd, struct
> hostend, etc., including parsing file data?  Or forwarding a lookup
> operation from a function such as getpwnam to an NSS module function
> like _nss_files_getpwnam_r?

This doesn't include the shared objects like nss_files, nss_ldap, etc.
Just the core code that calls them all.  I figure the arguments would
need to be stored in a struct, possibly with some other stateful
information.  Like this pseudo-code:

static int
my_so_caller (nss_handler *shared_object, my_data *data)
{
  /* call the shared_object's functions, passing my_data */
}

int
my_api_handler (arg1, arg2, arg3)
{
  struct { ... } my_data;
  my_data.arg1 = arg1;
  ...
  nss_logic_handler (NSS_PWNAM, my_so_caller, &my_data);
  return my_data.rv;
}

This means that none of the individual API functions need to have code
to do the logic of nsswitch.conf.

Alternately, the core logic could be a stateful machine:

int
my_api_handler (arg1, arg2, arg3)
{
  nss_logic_state state;
  nss_logic_initialize (NSS_PWNAM, &state);

  while (nss_logic_handler (&state))
    state->rv = state->shared_object->do_something (arg1, arg2, arg3);

  return nss_logic_finalize (&state);
}

Re: proposal: nsswitch refactoring

[Carlos O'Donell]

On 3/18/19 7:57 PM, DJ Delorie wrote:
> First, the data relating to the various shared objects that implement
> the name services (libnss_foo.so) needs to be split out of the data
> specific to nsswitch.conf's layout.  Why?  Because we still never
> unload shared objects, so that data is effectively "write once" and
> should never change.  Separating it from the changable data makes
> sense.

As Florian points out the binding to the service API is lazy, and worse
the list is not static, it's computed by the calling function.

It seems like the refactoring could take two distinct approaches:

* Refactor code to make the list of NSS lookups functions static and
   knowable at compiler time. We don't *need* a dynamic list of them,
   because the code must know which to call, it's dynamic simple by virtue
   of the existing design (a bunch of code which calls a function after
   making some choices).

* Refactor code to use a ref-count mechanism like in the resolver
   configuration case (reusing as much ref-count stuff as possible or
   generalizing it).

* Implement the reloading.

After these two things are done (a lot of work) we'll be in a very good
position to review future work against the implementation.

> The third phase is to consider expanding the syntax and functionality
> of nsswitch.conf.  My "vision" on this is to compile the nsswitch
> logic into a tiny p-code, and centralize running it.  That way, an API
> (like getpwnam()) need only set up some data (query arguments, etc),
> and pass said data and a callback function to this central logic
> handler, which calls the callback when it decides which shared object
> needs to be queried next.  This p-code could include new actions like
> two-way branching, more interesting error handlers, etc.

This need not be a "third phase", but an entirely distinct project and
discussion around the extension of the nsswitch.conf language to support
various use cases.

-- 
Cheers,
Carlos.

Re: proposal: nsswitch refactoring

[Florian Weimer]

* DJ Delorie:

>> We already do this for the resolver configuration.  We could probably
>> reuse the same mechanism, and even the same data structure.
>
> Thanks, I'll look at it.

It's struct resolv_conf, by the way.  struct resolv_context is the
temporary thing.  Allocating it prevents the struct resolv_conf object
from going away, among other things.

>> Which aspect are you referring to?  The handling of struct pwd, struct
>> hostend, etc., including parsing file data?  Or forwarding a lookup
>> operation from a function such as getpwnam to an NSS module function
>> like _nss_files_getpwnam_r?
>
> This doesn't include the shared objects like nss_files, nss_ldap, etc.
> Just the core code that calls them all.  I figure the arguments would
> need to be stored in a struct, possibly with some other stateful
> information.  Like this pseudo-code:
>
> static int
> my_so_caller (nss_handler *shared_object, my_data *data)
> {
>   /* call the shared_object's functions, passing my_data */
> }
>
> int
> my_api_handler (arg1, arg2, arg3)
> {
>   struct { ... } my_data;
>   my_data.arg1 = arg1;
>   ...
>   nss_logic_handler (NSS_PWNAM, my_so_caller, &my_data);
>   return my_data.rv;
> }
>
> This means that none of the individual API functions need to have code
> to do the logic of nsswitch.conf.
>
> Alternately, the core logic could be a stateful machine:
>
> int
> my_api_handler (arg1, arg2, arg3)
> {
>   nss_logic_state state;
>   nss_logic_initialize (NSS_PWNAM, &state);
>
>   while (nss_logic_handler (&state))
>     state->rv = state->shared_object->do_something (arg1, arg2, arg3);
>
>   return nss_logic_finalize (&state);
> }

We use the second approach today, I think, and nss_logic_handler is
called nss_next_action.

Re: proposal: nsswitch refactoring

[DJ Delorie]

"Carlos O'Donell" <codonell@redhat.com> writes:
> * Refactor code to make the list of NSS lookups functions static and
>    knowable at compiler time. We don't *need* a dynamic list of them,

Can we use linker magic to collect this list?

The other alternatives are either preprocessor magic, or a centralized
master list.

Re: proposal: nsswitch refactoring

[Florian Weimer]

* Carlos O'Donell:

> * Refactor code to make the list of NSS lookups functions static and
>    knowable at compiler time. We don't *need* a dynamic list of them,
>    because the code must know which to call, it's dynamic simple by virtue
>    of the existing design (a bunch of code which calls a function after
>    making some choices).

I'm not sure if this is strictly necessary.

Consider the handling of nip in git/nss/getXXbyYY_r.c.  Maybe it is
possible to retain that processing and just have calls to obtain an
initial nip and clean it afterwards?  Then this could main the
registration with the global configuration object.

I'm not saying that switching to central tables wouldn't be useful.  I
expect it will be.  But maybe it's possible to find a way to implement
the reload protection without that.

> * Refactor code to use a ref-count mechanism like in the resolver
>    configuration case (reusing as much ref-count stuff as possible or
>    generalizing it).

We might just put the whole thing into the same struct.  It will
introduce a tiny additional overhead because an application call
getpwnam locally will now load /etc/resolv.conf as well.  (The reverse
case, an application use libresolv without any NSS functions, is
really limited.)

Re: proposal: nsswitch refactoring

[Florian Weimer]

* Florian Weimer:

> We use the second approach today, I think, and nss_logic_handler is
> called nss_next_action.

And __nss_next2 and probably some other constructs. 8-/